Ensure that normal URLs are not loaded in the NTP process in process-per-tab.

BUG=69224
TEST=NewTabUIProcessPerTabTest.NavBeforeNTPCommits

Review URL: http://codereview.chromium.org/6335014

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

chrome/browser/dom_ui/new_tab_ui_uitest.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/dom_ui/new_tab_ui.h"
 #include "chrome/browser/prefs/pref_value_store.h"
 #include "chrome/browser/sync/signin_manager.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/json_pref_store.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
@@ -109,6 +110,42 @@ TEST_F(NewTabUITest, AboutHangInNTP) {
   ASSERT_TRUE(tab2->NavigateToURLAsync(GURL(chrome::kAboutHangURL)));
 }
 
+// Allows testing NTP in process-per-tab mode.
+class NewTabUIProcessPerTabTest : public NewTabUITest {
+ public:
+  NewTabUIProcessPerTabTest() : NewTabUITest() {}
+
+ protected:
+  virtual void SetUp() {
+    launch_arguments_.AppendSwitch(switches::kProcessPerTab);
+    UITest::SetUp();
+  }
+};
+
+// Navigates away from NTP before it commits, in process-per-tab mode.
+// Ensures that we don't load the normal page in the NTP process (and thus
+// crash), as in http://crbug.com/69224.
+TEST_F(NewTabUIProcessPerTabTest, NavBeforeNTPCommits) {
+  scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(window.get());
+
+  // Bring up a new tab page.
+  ASSERT_TRUE(window->RunCommand(IDC_NEW_TAB));
+  int load_time;
+  ASSERT_TRUE(automation()->WaitForInitialNewTabUILoad(&load_time));
+  scoped_refptr<TabProxy> tab = window->GetActiveTab();
+  ASSERT_TRUE(tab.get());
+
+  // Navigate to about:hang to stall the process.
+  ASSERT_TRUE(tab->NavigateToURLAsync(GURL(chrome::kAboutHangURL)));
+
+  // Visit a normal URL in another NTP that hasn't committed.
+  ASSERT_TRUE(window->RunCommand(IDC_NEW_TAB));
+  scoped_refptr<TabProxy> tab2 = window->GetActiveTab();
+  ASSERT_TRUE(tab2.get());
+  ASSERT_TRUE(tab2->NavigateToURL(GURL("data:text/html,hello world")));
+}
+
 // Fails about ~5% of the time on all platforms. http://crbug.com/45001
 TEST_F(NewTabUITest, FLAKY_ChromeInternalLoadsNTP) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));

chrome/browser/tab_contents/render_view_host_manager.cc

@@ -292,18 +292,32 @@ bool RenderViewHostManager::ShouldSwapProcessesForNavigation(
     const NavigationEntry* new_entry) const {
   DCHECK(new_entry);
 
+  // Check for reasons to swap processes even if we are in a process model that
+  // doesn't usually swap (e.g., process-per-tab).
+
+  // For security, we should transition between processes when one is a DOM UI
+  // page and one isn't.  If there's no cur_entry, check the current RVH's
+  // site, which might already be committed to a DOM UI URL (such as the NTP).
+  const GURL& current_url = (cur_entry) ? cur_entry->url() :
+      render_view_host_->site_instance()->site();
+  Profile* profile = delegate_->GetControllerForRenderManager().profile();
+  if (DOMUIFactory::UseDOMUIForURL(profile, current_url)) {
+    // Force swap if it's not an acceptable URL for DOM UI.
+    if (!DOMUIFactory::IsURLAcceptableForDOMUI(profile, new_entry->url()))
+      return true;
+  } else {
+    // Force swap if it's a DOM UI URL.
+    if (DOMUIFactory::UseDOMUIForURL(profile, new_entry->url()))
+      return true;
+  }
+
   if (!cur_entry) {
     // Always choose a new process when navigating to extension URLs. The
     // process grouping logic will combine all of a given extension's pages
     // into the same process.
     if (new_entry->url().SchemeIs(chrome::kExtensionScheme))
       return true;
-    // When a tab is created, it starts as TYPE_NORMAL. If the new entry is a
-    // DOM UI page, it needs to be grouped with other DOM UI pages. This matches
-    // the logic when transitioning between DOM UI and normal pages.
-    Profile* profile = delegate_->GetControllerForRenderManager().profile();
-    if (DOMUIFactory::UseDOMUIForURL(profile, new_entry->url()))
-      return true;
+
     return false;
   }
 
@@ -314,19 +328,6 @@ bool RenderViewHostManager::ShouldSwapProcessesForNavigation(
   if (cur_entry->IsViewSourceMode() != new_entry->IsViewSourceMode())
     return true;
 
-  // For security, we should transition between processes when one is a DOM UI
-  // page and one isn't.
-  Profile* profile = delegate_->GetControllerForRenderManager().profile();
-  if (DOMUIFactory::UseDOMUIForURL(profile, cur_entry->url())) {
-    // Force swap if it's not an acceptable URL for DOM UI.
-    if (!DOMUIFactory::IsURLAcceptableForDOMUI(profile, new_entry->url()))
-      return true;
-  } else {
-    // Force swap if it's a DOM UI URL.
-    if (DOMUIFactory::UseDOMUIForURL(profile, new_entry->url()))
-      return true;
-  }
-
   // Also, we must switch if one is an extension and the other is not the exact
   // same extension.
   if (cur_entry->url().SchemeIs(chrome::kExtensionScheme) ||