Attempt to fix a renderer crash in NetErrorHelperCore.

chrome/browser/errorpage_browsertest.cc

Index: chrome/browser/errorpage_browsertest.cc
diff --git a/chrome/browser/errorpage_browsertest.cc b/chrome/browser/errorpage_browsertest.cc
index 763d5f9d1b72210e3afd4ffa5b0cab5de330fec7..dc484753e7e253a1dfab9077fd4a404c92e5507f 100644
--- a/chrome/browser/errorpage_browsertest.cc
+++ b/chrome/browser/errorpage_browsertest.cc
@@ -164,6 +164,9 @@ void ExpectDisplayingNavigationCorrections(Browser* browser,
   // The diagnostics button isn't displayed when corrections were
   // retrieved from a remote server.
   EXPECT_FALSE(IsDisplayingDiagnosticsButton(browser));
+
+  // Close help box again, to return page to original state.
+  ToggleHelpBox(browser);
 }
 
 std::string GetShowSavedButtonLabel() {
@@ -740,6 +743,51 @@ IN_PROC_BROWSER_TEST_F(ErrorPageTest, DNSError_DoReload) {
   EXPECT_EQ(3, link_doctor_interceptor()->num_requests());
 }
 
+// Test that the reload button on a DNS error page works after a same page
+// navigation on the error page.  Error pages don't seem to do this, but some
+// traces indicate this may actually happen.  This test may hang on regression.
+IN_PROC_BROWSER_TEST_F(ErrorPageTest,
+                       DNSError_DoReloadAfterSamePageNavigation) {
+  // The first navigation should fail, and the second one should be the error
+  // page.
+  ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
+       browser(), GetDnsErrorURL(), 2);
+  ExpectDisplayingNavigationCorrections(browser(), net::ERR_NAME_NOT_RESOLVED);
+  EXPECT_EQ(1, link_doctor_interceptor()->num_requests());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Do a same page navigation.
+  content::TestNavigationObserver nav_observer1(web_contents, 1);
+  web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
+      base::ASCIIToUTF16("document.location='#';"));
+  // The same page navigation counts as a single navigation as far as the
+  // TestNavigationObserver is concerned.
+  nav_observer1.Wait();
+  // Page being displayed should not change.
+  ExpectDisplayingNavigationCorrections(browser(), net::ERR_NAME_NOT_RESOLVED);
+  // No new requests should have been issues.

[Randy Smith (Not in Mondays), 2015/12/09 21:23:46]

nit:issued.

[mmenke, 2015/12/10 15:52:23]

Done.

+  EXPECT_EQ(1, link_doctor_interceptor()->num_requests());
+
+  // Clicking the reload button should load the error page again, and there
+  // should be two commits, as before.
+  content::TestNavigationObserver nav_observer2(web_contents, 2);
+  // Can't use content::ExecuteScript because it waits for scripts to send
+  // notification that they've run, and scripts that trigger a navigation may
+  // not send that notification.
+  web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
+      base::ASCIIToUTF16("document.getElementById('reload-button').click();"));
+  nav_observer2.Wait();
+  ExpectDisplayingNavigationCorrections(browser(), net::ERR_NAME_NOT_RESOLVED);
+
+  // There should have two more requests to the correction service:  One for the
+  // new error page, and one for tracking purposes.  Have to make sure to wait
+  // for the tracking request, since the new error page does not depend on it.
+  link_doctor_interceptor()->WaitForRequests(3);
+  EXPECT_EQ(3, link_doctor_interceptor()->num_requests());
+}
+
 // Test that clicking links on a DNS error page works.
 IN_PROC_BROWSER_TEST_F(ErrorPageTest, DNSError_DoClickLink) {
   // The first navigation should fail, and the second one should be the error
@@ -1063,6 +1111,41 @@ IN_PROC_BROWSER_TEST_F(ErrorPageAutoReloadTest, ManualReloadNotSuppressed) {
   EXPECT_FALSE(IsDisplayingText(browser(), "error.page.auto.reload"));
 }
 
+// Make sure that a same page navigation does not cause issues with the
+// auto-reload timer.  Note that this test was added due to this case causing
+// a crash.  On regression, this test may hang due to a crashed renderer.
+IN_PROC_BROWSER_TEST_F(ErrorPageAutoReloadTest, IgnoresSamePageNavigation) {

[mmenke, 2015/12/09 20:18:33]

I'm a big concerned that this may timeout on OSX official builders, like the
first reload test (Which also goes through 2 auto reloads).  We may want to look
into workaround at some point.

+  GURL test_url("http://error.page.auto.reload");
+  InstallInterceptor(test_url, 2);
+
+  // Wait for the error page and first autoreload, which happens immediately.
+  ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
+      browser(), test_url, 2);
+
+  EXPECT_EQ(2, interceptor()->failures());
+  EXPECT_EQ(2, interceptor()->requests());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::TestNavigationObserver observer(web_contents, 1);
+  web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
+      base::ASCIIToUTF16("document.location='#';"));
+  // The same page navigation counts as a navigation as far as the
+  // TestNavigationObserver is concerned.
+  observer.Wait();
+
+  // No new requests should have been issued.
+  EXPECT_EQ(2, interceptor()->failures());
+  EXPECT_EQ(2, interceptor()->requests());
+
+  // Wait for the second auto reload, which succeeds.
+  content::TestNavigationObserver observer2(web_contents, 1);
+  observer2.Wait();
+
+  EXPECT_EQ(2, interceptor()->failures());
+  EXPECT_EQ(3, interceptor()->requests());
+}
+
 // Interceptor that fails all requests with net::ERR_ADDRESS_UNREACHABLE.
 class AddressUnreachableInterceptor : public net::URLRequestInterceptor {
  public: