Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3324)

Unified Diff: chrome/browser/tab_contents/render_view_host_manager.cc

Issue 1519025: Set the site instance in all cases when we start a new load. If you navigate ... (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: '' Created 10 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/tab_contents/render_view_host_manager_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/tab_contents/render_view_host_manager.cc
===================================================================
--- chrome/browser/tab_contents/render_view_host_manager.cc (revision 43614)
+++ chrome/browser/tab_contents/render_view_host_manager.cc (working copy)
@@ -339,9 +339,7 @@
// 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
// navigation commits (in DidNavigate), unless the navigation entry was
- // restored. As session restore loads all the pages immediately we need to set
- // the site first, otherwise after a restore none of the pages would share
- // renderers.
+ // restored or it's a DOM UI as described below.
if (!curr_instance->has_site()) {
// If we've already created a SiteInstance for our destination, we don't
// want to use this unused SiteInstance; use the existing one. (We don't
@@ -352,7 +350,22 @@
if (curr_instance->HasRelatedSiteInstance(dest_url)) {
return curr_instance->GetRelatedSiteInstance(dest_url);
} else {
- if (entry.restore_type() != NavigationEntry::RESTORE_NONE)
+ // 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 DOM 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 DOM UI commits,
+ // we still want to create a new process rather than re-using the
+ // existing DOM UI process.
+ if (entry.restore_type() != NavigationEntry::RESTORE_NONE ||
+ DOMUIFactory::HasDOMUIScheme(dest_url))
curr_instance->SetSite(dest_url);
return curr_instance;
}
« no previous file with comments | « no previous file | chrome/browser/tab_contents/render_view_host_manager_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698