diff options
15 files changed, 88 insertions, 192 deletions
diff --git a/chrome/browser/dom_ui/dom_ui_factory.cc b/chrome/browser/dom_ui/dom_ui_factory.cc index fb41ac2..f776baa 100644 --- a/chrome/browser/dom_ui/dom_ui_factory.cc +++ b/chrome/browser/dom_ui/dom_ui_factory.cc @@ -18,77 +18,102 @@ #include "chrome/common/url_constants.h" #include "googleurl/src/gurl.h" -const DOMUITypeID DOMUIFactory::kNoDOMUI = NULL; - -typedef DOMUI* (*DOMUIFactoryFunction)(TabContents* tab_contents, - const GURL& url); - -// Template for defining DOMUIFactoryFunction. -template<class T> -DOMUI* NewDOMUI(TabContents* contents, const GURL& url) { - return new T(contents); -} - -// Special case for extensions. -template<> -DOMUI* NewDOMUI<ExtensionDOMUI>(TabContents* contents, const GURL& url) { - // Don't use a DOMUI for non-existent extensions. - ExtensionsService* service = contents->profile()->GetExtensionsService(); - bool valid_extension = (service && service->GetExtensionById(url.host())); - if (valid_extension) - return new ExtensionDOMUI(contents); - return NULL; -} - -static DOMUIFactoryFunction GetDOMUIFactoryFunction(const GURL& url) { - // Currently, any gears: URL means an HTML dialog. - if (url.SchemeIs(chrome::kGearsScheme)) - return &NewDOMUI<HtmlDialogUI>; - - if (url.SchemeIs(chrome::kExtensionScheme)) - return &NewDOMUI<ExtensionDOMUI>; +// Backend for both querying for and creating new DOMUI objects. If you're just +// querying whether there's a DOM UI for the given URL, pass NULL for both the +// web contents and the new_ui. The return value will indiacate whether a DOM UI +// exists for the given URL. +// +// If you want to create a DOM UI, pass non-NULL pointers for both tab_contents +// and new_ui. The *new_ui pointer will be filled with the created UI if it +// succeeds (indicated by a return value of true). The caller owns the *new_ui +// pointer. +static bool CreateDOMUI(const GURL& url, TabContents* tab_contents, + DOMUI** new_ui) { + // Currently, any gears: URL means an HTML dialog. + if (url.SchemeIs(chrome::kGearsScheme)) { + if (new_ui) + *new_ui = new HtmlDialogUI(tab_contents); + return true; + } + + if (url.SchemeIs(chrome::kExtensionScheme)) { + // We assume we have a valid extension unless we have a TabContents which + // can prove us wrong. This can can happen if the user types in a URL + // manually. + bool valid_extension = true; + if (tab_contents) { + // TabContents can be NULL if we're doing a quick UseDOMUIForURL check. + ExtensionsService* service = + tab_contents->profile()->GetExtensionsService(); + valid_extension = (service && service->GetExtensionById(url.host())); + } + if (valid_extension) { + if (new_ui) + *new_ui = new ExtensionDOMUI(tab_contents); + return true; + } + return false; + } // TODO(mhm) Make sure this ifdef is removed once print is complete. #if !defined(GOOGLE_CHROME_BUILD) - if (url.SchemeIs(chrome::kPrintScheme)) - return &NewDOMUI<PrintUI>; + if (url.SchemeIs(chrome::kPrintScheme)) { + if (new_ui) + *new_ui = new PrintUI(tab_contents); + return true; + } #endif // This will get called a lot to check all URLs, so do a quick check of other // schemes (gears was handled above) to filter out most URLs. if (!url.SchemeIs(chrome::kChromeInternalScheme) && !url.SchemeIs(chrome::kChromeUIScheme)) - return NULL; + return false; - if (url.host() == chrome::kSyncResourcesHost) - return &NewDOMUI<HtmlDialogUI>; + if (url.host() == chrome::kSyncResourcesHost) { + if (new_ui) + *new_ui = new HtmlDialogUI(tab_contents); + return true; + } // Special case the new tab page. In older versions of Chrome, the new tab // page was hosted at chrome-internal:<blah>. This might be in people's saved // sessions or bookmarks, so we say any URL with that scheme triggers the new // tab page. if (url.host() == chrome::kChromeUINewTabHost || - url.SchemeIs(chrome::kChromeInternalScheme)) - return &NewDOMUI<NewTabUI>; + url.SchemeIs(chrome::kChromeInternalScheme)) { + if (new_ui) + *new_ui = new NewTabUI(tab_contents); + return true; + } // We must compare hosts only since some of the DOM UIs append extra stuff // after the host name. - if (url.host() == chrome::kChromeUIHistoryHost) - return &NewDOMUI<HistoryUI>; - if (url.host() == chrome::kChromeUIDownloadsHost) - return &NewDOMUI<DownloadsUI>; - if (url.host() == chrome::kChromeUIExtensionsHost) - return &NewDOMUI<ExtensionsUI>; - if (url.host() == chrome::kChromeUIDevToolsHost) - return &NewDOMUI<DevToolsUI>; - - return NULL; -} + if (url.host() == chrome::kChromeUIHistoryHost) { + if (new_ui) + *new_ui = new HistoryUI(tab_contents); + return true; + } + + if (url.host() == chrome::kChromeUIDownloadsHost) { + if (new_ui) + *new_ui = new DownloadsUI(tab_contents); + return true; + } + + if (url.host() == chrome::kChromeUIExtensionsHost) { + if (new_ui) + *new_ui = new ExtensionsUI(tab_contents); + return true; + } + + if (url.host() == chrome::kChromeUIDevToolsHost) { + if (new_ui) + *new_ui = new DevToolsUI(tab_contents); + return true; + } -// static -DOMUITypeID DOMUIFactory::GetDOMUIType(const GURL& url) { - DOMUIFactoryFunction function = GetDOMUIFactoryFunction(url); - return function ? function : kNoDOMUI; + return false; } // static @@ -101,16 +126,16 @@ bool DOMUIFactory::HasDOMUIScheme(const GURL& url) { // static bool DOMUIFactory::UseDOMUIForURL(const GURL& url) { - return GetDOMUIFactoryFunction(url) != NULL; + return CreateDOMUI(url, NULL, NULL); } // static DOMUI* DOMUIFactory::CreateDOMUIForURL(TabContents* tab_contents, const GURL& url) { - DOMUIFactoryFunction function = GetDOMUIFactoryFunction(url); - if (!function) + DOMUI* dom_ui; + if (!CreateDOMUI(url, tab_contents, &dom_ui)) return NULL; - return (*function)(tab_contents, url); + return dom_ui; } // static diff --git a/chrome/browser/dom_ui/dom_ui_factory.h b/chrome/browser/dom_ui/dom_ui_factory.h index e52a557..e4e36c3 100644 --- a/chrome/browser/dom_ui/dom_ui_factory.h +++ b/chrome/browser/dom_ui/dom_ui_factory.h @@ -11,17 +11,8 @@ class DOMUI; class GURL; class TabContents; -typedef void* DOMUITypeID; - class DOMUIFactory { public: - static const DOMUITypeID kNoDOMUI; - - // Returns a type identifier indicating what DOMUI we would use for the - // given URL. This is useful for comparing the potential DOMUIs for two URLs. - // Returns kNoDOMUI if the given URL will not use the DOM UI system. - static DOMUITypeID GetDOMUIType(const GURL& url); - // Returns true if the given URL's scheme would trigger the DOM UI system. // This is a less precise test than UseDONUIForURL, which tells you whether // that specific URL matches a known one. This one is faster and can be used diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc index 2a25bc7..88da761 100644 --- a/chrome/browser/extensions/extension_browsertests_misc.cc +++ b/chrome/browser/extensions/extension_browsertests_misc.cc @@ -501,59 +501,3 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, UninstallDisabled) { EXPECT_EQ(0u, service->extensions()->size()); EXPECT_EQ(0u, service->disabled_extensions()->size()); } - -// Tests that an extension page can call window.open to an extension URL and -// the new window has extension privileges. -IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenExtension) { - ASSERT_TRUE(LoadExtension( - test_data_dir_.AppendASCII("uitest").AppendASCII("window_open"))); - - ui_test_utils::NavigateToURL( - browser(), - GURL("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/test.html")); - - bool result = false; - ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents()->render_view_host(), L"", - L"testWindowOpen('newtab.html')", &result); - ASSERT_TRUE(result); - - // Now the current tab should be the new tab. - ui_test_utils::WaitForNavigation( - &browser()->GetSelectedTabContents()->controller()); - ASSERT_EQ(browser()->GetSelectedTabContents()->GetURL().path(), - "/newtab.html"); - - result = false; - ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents()->render_view_host(), L"", - L"testExtensionApi()", &result); - EXPECT_TRUE(result); -} - -// Tests that if an extension page calls window.open to an invalid extension -// URL, the browser doesn't crash. -IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenInvalidExtension) { - ASSERT_TRUE(LoadExtension( - test_data_dir_.AppendASCII("uitest").AppendASCII("window_open"))); - - ui_test_utils::NavigateToURL( - browser(), - GURL("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/test.html")); - - bool result = false; - ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents()->render_view_host(), L"", - L"testWindowOpen('" - L"chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab/newtab.html" - L"')", &result); - ASSERT_TRUE(result); - - // Now the current tab should be the new tab. - ui_test_utils::WaitForNavigation( - &browser()->GetSelectedTabContents()->controller()); - ASSERT_EQ(browser()->GetSelectedTabContents()->GetURL().path(), - "/newtab.html"); - - // If we got to this point, we didn't crash, so we're good. -} diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc index c3741c7..044a28a 100644 --- a/chrome/browser/extensions/extension_dom_ui.cc +++ b/chrome/browser/extensions/extension_dom_ui.cc @@ -40,7 +40,7 @@ void ExtensionDOMUI::ResetExtensionFunctionDispatcher( // Use the NavigationController to get the URL rather than the TabContents // since this is the real underlying URL (see HandleChromeURLOverride). NavigationController& controller = tab_contents()->controller(); - const GURL& url = controller.GetActiveEntry()->url(); + const GURL& url = controller.pending_entry()->url(); extension_function_dispatcher_.reset( new ExtensionFunctionDispatcher(render_view_host, this, url)); } diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 8690e6a..60aa0d3 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -232,7 +232,7 @@ void ExtensionHost::CreateNewWindow(int route_id, base::WaitableEvent* modal_dialog_event) { delegate_view_helper_.CreateNewWindow( route_id, modal_dialog_event, render_view_host()->process()->profile(), - site_instance(), DOMUIFactory::GetDOMUIType(url_)); + site_instance()); } void ExtensionHost::CreateNewWidget(int route_id, bool activatable) { diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc index 2f0eef8..184fc5e 100644 --- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc +++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc @@ -21,7 +21,7 @@ void RenderViewHostDelegateViewHelper::CreateNewWindow(int route_id, base::WaitableEvent* modal_dialog_event, Profile* profile, - SiteInstance* site, DOMUITypeID domui_type) { + SiteInstance* site) { // Create the new web contents. This will automatically create the new // TabContentsView. In the future, we may want to create the view separately. TabContents* new_contents = @@ -29,7 +29,6 @@ void RenderViewHostDelegateViewHelper::CreateNewWindow(int route_id, site, route_id, modal_dialog_event); - new_contents->set_opener_dom_ui_type(domui_type); TabContentsView* new_view = new_contents->view(); // TODO(brettw) it seems bogus that we have to call this function on the diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.h b/chrome/browser/tab_contents/render_view_host_delegate_helper.h index 215cddcc..4f9e600 100644 --- a/chrome/browser/tab_contents/render_view_host_delegate_helper.h +++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.h @@ -10,7 +10,6 @@ #include "base/basictypes.h" #include "base/gfx/rect.h" #include "base/waitable_event.h" -#include "chrome/browser/dom_ui/dom_ui_factory.h" #include "webkit/glue/webpreferences.h" #include "webkit/glue/window_open_disposition.h" @@ -32,8 +31,7 @@ class RenderViewHostDelegateViewHelper { virtual void CreateNewWindow(int route_id, base::WaitableEvent* modal_dialog_event, - Profile* profile, SiteInstance* site, - DOMUITypeID domui_type); + Profile* profile, SiteInstance* site); virtual RenderWidgetHostView* CreateNewWidget(int route_id, bool activatable, RenderProcessHost* process); virtual TabContents* GetCreatedWindow(int route_id); diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc index 6b3866f..101c1f8 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.cc +++ b/chrome/browser/tab_contents/render_view_host_manager.cc @@ -187,11 +187,6 @@ void RenderViewHostManager::DidNavigateMainFrame( } } -void RenderViewHostManager::SetDOMUIPostCommit(DOMUI* dom_ui) { - DCHECK(!dom_ui_.get()); - dom_ui_.reset(dom_ui); -} - void RenderViewHostManager::RendererAbortedProvisionalLoad( RenderViewHost* render_view_host) { // We used to cancel the pending renderer here for cross-site downloads. diff --git a/chrome/browser/tab_contents/render_view_host_manager.h b/chrome/browser/tab_contents/render_view_host_manager.h index c562628..8c8dde2 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.h +++ b/chrome/browser/tab_contents/render_view_host_manager.h @@ -127,11 +127,6 @@ class RenderViewHostManager // Called when a renderer's main frame navigates. void DidNavigateMainFrame(RenderViewHost* render_view_host); - // Set the DOMUI after committing a page load. This is useful for navigations - // initiated from a renderer, where we want to give the new renderer DOMUI - // privileges from the originating renderer. - void SetDOMUIPostCommit(DOMUI* dom_ui); - // Called when a provisional load on the given renderer is aborted. void RendererAbortedProvisionalLoad(RenderViewHost* render_view_host); diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 2a9ef7d..cfdde6c 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -258,8 +258,7 @@ TabContents::TabContents(Profile* profile, message_box_active_(CreateEvent(NULL, TRUE, FALSE, NULL)), #endif last_javascript_message_dismissal_(), - suppress_javascript_messages_(false), - opener_dom_ui_type_(DOMUIFactory::kNoDOMUI) { + suppress_javascript_messages_(false) { pending_install_.page_id = 0; pending_install_.callback_functor = NULL; @@ -1272,21 +1271,6 @@ DOMUI* TabContents::GetDOMUIForCurrentState() { void TabContents::DidNavigateMainFramePostCommit( const NavigationController::LoadCommittedDetails& details, const ViewHostMsg_FrameNavigate_Params& params) { - if (opener_dom_ui_type_ != DOMUIFactory::kNoDOMUI) { - // If this is a window.open navigation, use the same DOMUI as the renderer - // that opened the window, as long as both renderers have the same - // privileges. - if (opener_dom_ui_type_ == DOMUIFactory::GetDOMUIType(GetURL())) { - DOMUI* dom_ui = DOMUIFactory::CreateDOMUIForURL(this, GetURL()); - // dom_ui might be NULL if the URL refers to a non-existent extension. - if (dom_ui) { - render_manager_.SetDOMUIPostCommit(dom_ui); - dom_ui->RenderViewCreated(render_view_host()); - } - } - opener_dom_ui_type_ = DOMUIFactory::kNoDOMUI; - } - if (details.is_user_initiated_main_frame_load()) { // Clear the status bubble. This is a workaround for a bug where WebKit // doesn't let us know that the cursor left an element during a diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 3786f68..3b53b52 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -18,7 +18,6 @@ #include "base/scoped_ptr.h" #include "chrome/browser/autocomplete/autocomplete_edit.h" #include "chrome/browser/cancelable_request.h" -#include "chrome/browser/dom_ui/dom_ui_factory.h" #include "chrome/browser/download/save_package.h" #include "chrome/browser/fav_icon_helper.h" #include "chrome/browser/find_notification_details.h" @@ -593,10 +592,6 @@ class TabContents : public PageNavigator, return &renderer_preferences_; } - void set_opener_dom_ui_type(DOMUITypeID opener_dom_ui_type) { - opener_dom_ui_type_ = opener_dom_ui_type; - } - private: friend class NavigationController; // Used to access the child_windows_ (ConstrainedWindowList) for testing @@ -1119,10 +1114,6 @@ class TabContents : public PageNavigator, // Settings that get passed to the renderer process. RendererPreferences renderer_preferences_; - // If this tab was created from a renderer using window.open, this will be - // non-NULL and represent the DOMUI of the opening renderer. - DOMUITypeID opener_dom_ui_type_; - // --------------------------------------------------------------------------- DISALLOW_COPY_AND_ASSIGN(TabContents); diff --git a/chrome/browser/tab_contents/tab_contents_view.cc b/chrome/browser/tab_contents/tab_contents_view.cc index ecdf697..78d084a 100644 --- a/chrome/browser/tab_contents/tab_contents_view.cc +++ b/chrome/browser/tab_contents/tab_contents_view.cc @@ -32,10 +32,9 @@ void TabContentsView::UpdatePreferredWidth(int pref_width) { void TabContentsView::CreateNewWindow(int route_id, base::WaitableEvent* modal_dialog_event) { - delegate_view_helper_.CreateNewWindow( - route_id, modal_dialog_event, - tab_contents_->profile(), tab_contents_->GetSiteInstance(), - DOMUIFactory::GetDOMUIType(tab_contents_->GetURL())); + delegate_view_helper_.CreateNewWindow(route_id, modal_dialog_event, + tab_contents_->profile(), + tab_contents_->GetSiteInstance()); } void TabContentsView::CreateNewWidget(int route_id, bool activatable) { diff --git a/chrome/test/data/extensions/uitest/window_open/manifest.json b/chrome/test/data/extensions/uitest/window_open/manifest.json deleted file mode 100644 index bcdc692..0000000 --- a/chrome/test/data/extensions/uitest/window_open/manifest.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": "1.0.0.0", - "name": "window.open test", - "description": "An extension for an extension UITest.", - "permissions": ["tabs"] -} diff --git a/chrome/test/data/extensions/uitest/window_open/newtab.html b/chrome/test/data/extensions/uitest/window_open/newtab.html deleted file mode 100644 index db1eb11..0000000 --- a/chrome/test/data/extensions/uitest/window_open/newtab.html +++ /dev/null @@ -1,11 +0,0 @@ -<script> -function testExtensionApi() { - try { - chrome.tabs.getAllInWindow(null, function() { - window.domAutomationController.send(true); - }); - } catch (e) { - window.domAutomationController.send(false); - } -} -</script> diff --git a/chrome/test/data/extensions/uitest/window_open/test.html b/chrome/test/data/extensions/uitest/window_open/test.html deleted file mode 100644 index c267f19..0000000 --- a/chrome/test/data/extensions/uitest/window_open/test.html +++ /dev/null @@ -1,8 +0,0 @@ -HOWDIE!!! - -<script> -function testWindowOpen(url) { - window.open(url); - window.domAutomationController.send(true); -} -</script> |