Fix the process creation problem. This forces transitions between

BrowsingInstances when we force a transition for DOM UIs.

BUG=9364
TEST=see bug
Review URL: http://codereview.chromium.org/67201

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

3 files changed, 66 insertions, 5 deletions

diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index be813aa..a178356 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -277,7 +277,7 @@ bool RenderViewHostManager::ShouldTransitionCrossSite() {
   return !CommandLine::ForCurrentProcess()->HasSwitch(switches::kProcessPerTab);
 }
 
-bool RenderViewHostManager::ShouldSwapRenderViewsForNavigation(
+bool RenderViewHostManager::ShouldSwapProcessesForNavigation(
     const NavigationEntry* cur_entry,
     const NavigationEntry* new_entry) const {
   if (!cur_entry || !new_entry)
@@ -378,6 +378,15 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
 
   if (SiteInstance::IsSameWebSite(current_url, dest_url)) {
     return curr_instance;
+  } else if (ShouldSwapProcessesForNavigation(curr_entry, &entry)) {
+    // When we're swapping, we need to force the site instance AND browsing
+    // instance to be new ones. This addresses special cases where we use a
+    // single BrowsingInstance for all pages of a certain type (e.g., New Tab
+    // Pages), keeping them in the same process. When you navigate away from
+    // that page, we want to explicity ignore that BrowsingInstance and make a
+    // new process.
+    return SiteInstance::CreateSiteInstance(
+        delegate_->GetControllerForRenderManager()->profile());
   } else {
     // Start the new renderer in a new SiteInstance, but in the current
     // BrowsingInstance.  It is important to immediately give this new
@@ -489,7 +498,7 @@ RenderViewHost* RenderViewHostManager::UpdateRendererStateForNavigate(
     new_instance = GetSiteInstanceForEntry(entry, curr_instance);
 
   if (new_instance != curr_instance ||
-      ShouldSwapRenderViewsForNavigation(
+      ShouldSwapProcessesForNavigation(
           delegate_->GetLastCommittedNavigationEntryForRenderManager(),
           &entry)) {
     // New SiteInstance.
diff --git a/chrome/browser/tab_contents/render_view_host_manager.h b/chrome/browser/tab_contents/render_view_host_manager.h
index 09bd4ea..b3a5b25 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.h
+++ b/chrome/browser/tab_contents/render_view_host_manager.h
@@ -186,9 +186,12 @@ class RenderViewHostManager : public NotificationObserver {
   bool ShouldTransitionCrossSite();
 
   // Returns true if the two navigation entries are incompatible in some way
-  // other than site instances. This will cause us to swap RenderViewHosts even
-  // if the site instances are the same. Either of the entries may be NULL.
-  bool ShouldSwapRenderViewsForNavigation(
+  // other than site instances. Cases where this can happen include DOM UI
+  // to regular web pages. It will cause us to swap RenderViewHosts (and hence
+  // RenderProcessHosts) even if the site instance would otherwise be the same.
+  // As part of this, we'll also force new SiteInstances and BrowsingInstances.
+  // Either of the entries may be NULL.
+  bool ShouldSwapProcessesForNavigation(
       const NavigationEntry* cur_entry,
       const NavigationEntry* new_entry) const;
 
diff --git a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc
new file mode 100644
index 0000000..02f5473
--- /dev/null
+++ b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc
@@ -0,0 +1,49 @@
+// Copyright (c) 2009 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.
+
+#include "chrome/browser/renderer_host/test_render_view_host.h"
+#include "chrome/common/url_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class RenderViewHostManagerTest : public RenderViewHostTestHarness {
+
+};
+
+// Tests that when you navigate from the New TabPage to another page, and
+// then do that same thing in another tab, that the two resulting pages have
+// different SiteInstances, BrowsingInstances, and RenderProcessHosts. This is
+// a regression test for bug 9364.
+TEST_F(RenderViewHostManagerTest, NewTabPageProcesses) {
+  GURL ntp(chrome::kChromeUINewTabURL);
+  GURL dest("http://www.google.com/");
+
+  // Navigate our first tab to the new tab page and then to the destination.
+  NavigateAndCommit(ntp);
+  NavigateAndCommit(dest);
+
+  // Make a second tab.
+  WebContents* contents2 = new TestWebContents(profile_.get(), NULL);
+  NavigationController* controller2 =
+      new NavigationController(contents2, profile_.get());
+  contents2->set_controller(controller2);
+
+  // Load the two URLs in the second tab. Note that the first navigation creates
+  // a RVH that's not pending (since there is no cross-site transition), so
+  // we use the committed one, but the second one is the opposite.
+  contents2->controller()->LoadURL(ntp, GURL(), PageTransition::LINK);
+  static_cast<TestRenderViewHost*>(contents2->render_manager()->
+     current_host())->SendNavigate(100, ntp);
+  contents2->controller()->LoadURL(dest, GURL(), PageTransition::LINK);
+  static_cast<TestRenderViewHost*>(contents2->render_manager()->
+      pending_render_view_host())->SendNavigate(101, dest);
+
+  // The two RVH's should be different in every way.
+  EXPECT_NE(rvh()->process(), contents2->render_view_host()->process());
+  EXPECT_NE(rvh()->site_instance(),
+      contents2->render_view_host()->site_instance());
+  EXPECT_NE(rvh()->site_instance()->browsing_instance(),
+      contents2->render_view_host()->site_instance()->browsing_instance());
+
+  contents2->CloseContents();
+}