Set the site instance in all cases when we start a new load. If you navigate away from the new tab page (or another DOMUI page) before the DOMUI page commits, the subsequent page will inherit the SiteInstance of the original page.

BUG=12718,40575
TEST=included unit test
Review URL: http://codereview.chromium.org/1519025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43879 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 26 insertions, 6 deletions

diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index 2619b67..a07f4fb 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -339,9 +339,7 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
   // 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 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
     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;
     }
diff --git a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc
index 668406d..5dc2213 100644
--- a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -230,7 +230,7 @@ TEST_F(RenderViewHostManagerTest, DOMUI) {
 
   manager.Init(profile_.get(), instance, MSG_ROUTING_NONE);
 
-  GURL url("chrome://newtab");
+  GURL url(chrome::kChromeUINewTabURL);
   NavigationEntry entry(NULL /* instance */, -1 /* page_id */, url,
                         GURL() /* referrer */, string16() /* title */,
                         PageTransition::TYPED);
@@ -242,6 +242,13 @@ TEST_F(RenderViewHostManagerTest, DOMUI) {
   EXPECT_TRUE(manager.pending_dom_ui());
   EXPECT_FALSE(manager.dom_ui());
 
+  // It's important that the site instance get set on the DOM UI page as soon
+  // as the navigation starts, rather than lazily after it commits, so we don't
+  // try to re-use the SiteInstance/process for non DOM-UI things that may
+  // get loaded in between.
+  EXPECT_TRUE(host->site_instance()->has_site());
+  EXPECT_EQ(url, host->site_instance()->site());
+
   // Commit.
   manager.DidNavigateMainFrame(host);