Prevent a crash when visiting about:hang in two NTPs.

Fixes how we assign about:hang to renderers and a related sanity check.

BUG=59859
TEST=NewTabUITest.AboutHangInNTP

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

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

5 files changed, 57 insertions, 8 deletions

diff --git a/chrome/browser/dom_ui/dom_ui_factory.cc b/chrome/browser/dom_ui/dom_ui_factory.cc
index fbdb42b..d27329e 100644
--- a/chrome/browser/dom_ui/dom_ui_factory.cc
+++ b/chrome/browser/dom_ui/dom_ui_factory.cc
@@ -234,6 +234,20 @@ bool DOMUIFactory::UseDOMUIForURL(Profile* profile, const GURL& url) {
 }
 
 // static
+bool DOMUIFactory::IsURLAcceptableForDOMUI(Profile* profile, const GURL& url) {
+  return UseDOMUIForURL(profile, url) ||
+      // javacsript: URLs are allowed to run in DOM UI pages
+      url.SchemeIs(chrome::kJavaScriptScheme) ||
+      // It's possible to load about:blank in a DOM UI renderer.
+      // See http://crbug.com/42547
+      url.spec() == chrome::kAboutBlankURL ||
+      // about:crash, about:hang, and about:shorthang are allowed.
+      url.spec() == chrome::kAboutCrashURL ||
+      url.spec() == chrome::kAboutHangURL ||
+      url.spec() == chrome::kAboutShorthangURL;
+}
+
+// static
 DOMUI* DOMUIFactory::CreateDOMUIForURL(TabContents* tab_contents,
                                        const GURL& url) {
   DOMUIFactoryFunction function = GetDOMUIFactoryFunction(
diff --git a/chrome/browser/dom_ui/dom_ui_factory.h b/chrome/browser/dom_ui/dom_ui_factory.h
index 3a7bfc6..1e1ed87 100644
--- a/chrome/browser/dom_ui/dom_ui_factory.h
+++ b/chrome/browser/dom_ui/dom_ui_factory.h
@@ -35,9 +35,14 @@ class DOMUIFactory {
   // to determine security policy.
   static bool HasDOMUIScheme(const GURL& url);
 
-  // Returns true if the given URL will use the DOM UI system.
+  // Returns true if the given URL must use the DOM UI system.
   static bool UseDOMUIForURL(Profile* profile, const GURL& url);
 
+  // Returns true if the given URL can be loaded by DOM UI system.  This
+  // includes URLs that can be loaded by normal tabs as well, such as
+  // javascript: URLs or about:hang.
+  static bool IsURLAcceptableForDOMUI(Profile* profile, const GURL& url);
+
   // Allocates a new DOMUI object for the given URL, and returns it. If the URL
   // is not a DOM UI URL, then it will return NULL. When non-NULL, ownership of
   // the returned pointer is passed to the caller.
diff --git a/chrome/browser/dom_ui/new_tab_ui_uitest.cc b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
index 5be42f5..f50e179 100644
--- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc
+++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/sync/signin_manager.h"
 #include "chrome/common/json_pref_store.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/automation/window_proxy.h"
@@ -83,6 +84,30 @@ TEST_F(NewTabUITest, NTPHasLoginName) {
   EXPECT_EQ(L"user@gmail.com", displayed_username);
 }
 
+// Loads about:hang into two NTP tabs, ensuring we don't crash.
+// See http://crbug.com/59859.
+TEST_F(NewTabUITest, AboutHangInNTP) {
+  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 about:hang again in another NTP.  Don't bother waiting for the
+  // NTP to load, because it's hung.
+  ASSERT_TRUE(window->RunCommand(IDC_NEW_TAB));
+  scoped_refptr<TabProxy> tab2 = window->GetActiveTab();
+  ASSERT_TRUE(tab2.get());
+  ASSERT_TRUE(tab2->NavigateToURLAsync(GURL(chrome::kAboutHangURL)));
+}
+
 // 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));
diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index 8883540..664b7f5 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -310,9 +310,15 @@ bool RenderViewHostManager::ShouldSwapProcessesForNavigation(
   // 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()) !=
-      DOMUIFactory::UseDOMUIForURL(profile, new_entry->url()))
-    return true;
+  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.
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index d9522e1..b6bc9ae 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -864,12 +864,11 @@ bool TabContents::NavigateToEntry(
         kPreferredSizeWidth | kPreferredSizeHeightThisIsSlow);
   }
 
-  // For security, we should never send non-DOM-UI URLs (other than about:blank)
-  // to a DOM UI renderer.  Double check that here.
+  // For security, we should never send non-DOM-UI URLs to a DOM UI renderer.
+  // Double check that here.
   int enabled_bindings = dest_render_view_host->enabled_bindings();
   bool is_allowed_in_dom_ui_renderer =
-      DOMUIFactory::UseDOMUIForURL(profile(), entry.url()) ||
-      entry.url() == GURL(chrome::kAboutBlankURL);
+      DOMUIFactory::IsURLAcceptableForDOMUI(profile(), entry.url());
   CHECK(!BindingsPolicy::is_dom_ui_enabled(enabled_bindings) ||
         is_allowed_in_dom_ui_renderer);