diff options
| -rw-r--r-- | chrome/browser/renderer_host/test/render_process_host_browsertest.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/ui/browser_navigator.cc | 20 | ||||
| -rw-r--r-- | content/browser/tab_contents/render_view_host_manager.cc | 46 |
3 files changed, 38 insertions, 32 deletions
diff --git a/chrome/browser/renderer_host/test/render_process_host_browsertest.cc b/chrome/browser/renderer_host/test/render_process_host_browsertest.cc index aa75b51..17eb30e 100644 --- a/chrome/browser/renderer_host/test/render_process_host_browsertest.cc +++ b/chrome/browser/renderer_host/test/render_process_host_browsertest.cc @@ -183,9 +183,7 @@ IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, DevToolsOnSelfInOwnProcess) { // sharing behaves correctly. In particular, this test is verifying that even // when we hit the max process limit, that renderers of each type will wind up // in a process of that type, even if that means creating a new process. -// TODO(erikkay) crbug.com/43448 - disabled for now until we can get a -// reasonable implementation in place. -IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, DISABLED_ProcessOverflow) { +IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, ProcessOverflow) { // Set max renderers to 1 to force running out of processes. RenderProcessHost::SetMaxRendererProcessCount(1); diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc index f009028..16ee1fd 100644 --- a/chrome/browser/ui/browser_navigator.cc +++ b/chrome/browser/ui/browser_navigator.cc @@ -23,9 +23,17 @@ namespace { -// Returns the SiteInstance for |source_contents| if it represents the same -// website as |url|, or NULL otherwise. |source_contents| cannot be NULL. -SiteInstance* GetSiteInstance(TabContents* source_contents, const GURL& url) { +// Returns an appropriate SiteInstance for WebUI URLs, or the SiteInstance for +// |source_contents| if it represents the same website as |url|. Returns NULL +// otherwise. +SiteInstance* GetSiteInstance(TabContents* source_contents, Profile* profile, + const GURL& url) { + // If url is a WebUI or extension, we need to be sure to use the right type + // of renderer process up front. Otherwise, we create a normal SiteInstance + // as part of creating the tab. + if (WebUIFactory::UseWebUIForURL(profile, url)) + return SiteInstance::CreateSiteInstanceForURL(profile, url); + if (!source_contents) return NULL; @@ -399,13 +407,15 @@ void Navigate(NavigateParams* params) { // If no target TabContents was specified, we need to construct one if we are // supposed to target a new tab; unless it's a singleton that already exists. if (!params->target_contents && singleton_index < 0) { + GURL url = params->url.is_empty() ? params->browser->GetHomePage() + : params->url; if (params->disposition != CURRENT_TAB) { TabContents* source_contents = params->source_contents ? params->source_contents->tab_contents() : NULL; params->target_contents = Browser::TabContentsFactory( params->browser->profile(), - GetSiteInstance(source_contents, params->url), + GetSiteInstance(source_contents, params->browser->profile(), url), MSG_ROUTING_NONE, source_contents, NULL); @@ -434,8 +444,6 @@ void Navigate(NavigateParams* params) { } // Perform the actual navigation. - GURL url = params->url.is_empty() ? params->browser->GetHomePage() - : params->url; params->target_contents->controller().LoadURL(url, params->referrer, params->transition); } else { diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc index 556c3b7..c31dc76 100644 --- a/content/browser/tab_contents/render_view_host_manager.cc +++ b/content/browser/tab_contents/render_view_host_manager.cc @@ -363,6 +363,8 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( return curr_instance; const GURL& dest_url = entry.url(); + NavigationController& controller = delegate_->GetControllerForRenderManager(); + Profile* profile = controller.profile(); // If we haven't used our SiteInstance (and thus RVH) yet, then we can use it // for this entry. We won't commit the SiteInstance to this site until the @@ -375,28 +377,28 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( // to compare against the current URL and not the SiteInstance's site. In // this case, there is no current URL, so comparing against the site is ok. // See additional comments below.) - if (curr_instance->HasRelatedSiteInstance(dest_url)) { + if (curr_instance->HasRelatedSiteInstance(dest_url)) return curr_instance->GetRelatedSiteInstance(dest_url); - } else { - // Normally the "site" on the SiteInstance is set lazily when the load - // actually commits. This is to support better process sharing in case - // the site redirects to some other site: we want to use the destination - // site in the site instance. - // - // In the case of session restore, as it loads all the pages immediately - // we need to set the site first, otherwise after a restore none of the - // pages would share renderers. - // - // For Web UI (this mostly comes up for the new tab page), the - // SiteInstance has special meaning: we never want to reassign the - // process. If you navigate to another site before the Web UI commits, - // we still want to create a new process rather than re-using the - // existing Web UI process. - if (entry.restore_type() != NavigationEntry::RESTORE_NONE || - WebUIFactory::HasWebUIScheme(dest_url)) - curr_instance->SetSite(dest_url); - return curr_instance; - } + + // For extensions and Web UI URLs (such as the new tab page), we do not + // want to use the curr_instance if it has no site, since it will have a + // RenderProcessHost of TYPE_NORMAL. Create a new SiteInstance for this + // URL instead (with the correct process type). + if (WebUIFactory::UseWebUIForURL(profile, dest_url)) + return SiteInstance::CreateSiteInstanceForURL(profile, dest_url); + + // Normally the "site" on the SiteInstance is set lazily when the load + // actually commits. This is to support better process sharing in case + // the site redirects to some other site: we want to use the destination + // site in the site instance. + // + // In the case of session restore, as it loads all the pages immediately + // we need to set the site first, otherwise after a restore none of the + // pages would share renderers in process-per-site. + if (entry.restore_type() != NavigationEntry::RESTORE_NONE) + curr_instance->SetSite(dest_url); + + return curr_instance; } // Otherwise, only create a new SiteInstance for cross-site navigation. @@ -409,7 +411,6 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( // For now, though, we're in a hybrid model where you only switch // SiteInstances if you type in a cross-site URL. This means we have to // compare the entry's URL to the last committed entry's URL. - NavigationController& controller = delegate_->GetControllerForRenderManager(); NavigationEntry* curr_entry = controller.GetLastCommittedEntry(); if (interstitial_page_) { // The interstitial is currently the last committed entry, but we want to @@ -428,7 +429,6 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( // practice.) const GURL& current_url = (curr_entry) ? curr_entry->url() : curr_instance->site(); - Profile* profile = controller.profile(); if (SiteInstance::IsSameWebSite(profile, current_url, dest_url)) { return curr_instance; |