Destroy SSLErrorHandler on new navigations so that it can properly be recreated.

chrome/browser/captive_portal/captive_portal_browsertest.cc

 // Copyright (c) 2012 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 <map>
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 799 matching lines @@
   }
 }
 
 // This observer waits for the SSLErrorHandler to fire an interstitial timer for
 // the given web contents.
 class SSLInterstitialTimerObserver {
  public:
   explicit SSLInterstitialTimerObserver(content::WebContents* web_contents);
   ~SSLInterstitialTimerObserver();
 
   // Waits until the interstitial delay timer in SSLErrorHandler is fired.

[mmenke, 2015/03/20 13:33:50]

While you're here, can you replace all these "fired" with "started".  Had to
trace my way back through the code to figure out that this didn't mean what it
sounded like.

[meacer, 2015/03/20 17:46:01]

Done.

   void WaitForTimerFired();
 
  private:
   void OnTimerFired(content::WebContents* web_contents);
 
   const content::WebContents* web_contents_;
   SSLErrorHandler::TimerStartedCallback callback_;
 
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
 
@@ skipping 110 matching lines @@
   // there's no detected captive portal.
   void FastTimeoutNoCaptivePortal(Browser* browser,
                                   CaptivePortalResult expected_result);
 
   // Navigates the active tab to a slow loading SSL page, which will then
   // trigger a captive portal test.  The test is expected to find a captive
   // portal.  The slow loading page will continue to load after the function
   // returns, until URLRequestTimeoutOnDemandJob::FailJobs() is called,
   // at which point it will timeout.
   //
-  // When |expect_login_tab| is false, no login tab is expected to be opened,
-  // because one already exists, and the function returns once the captive
-  // portal test is complete.
+  // When |expect_open_login_tab| is false, no login tab is expected to be
+  // opened, because one already exists, and the function returns once the
+  // captive portal test is complete.
   //
-  // If |expect_login_tab| is true, a login tab is then expected to be opened.
-  // It waits until both the login tab has finished loading, and two captive
-  // portal tests complete.  The second test is triggered by the load of the
-  // captive portal tab completing.
+  // If |expect_open_login_tab| is true, a login tab is then expected to be
+  // opened. It waits until both the login tab has finished loading, and two
+  // captive portal tests complete.  The second test is triggered by the load of
+  // the captive portal tab completing.
   //
   // This function must not be called when the active tab is currently loading.
   // Waits for the hanging request to be issued, so other functions can rely
   // on URLRequestTimeoutOnDemandJob::WaitForJobs having been called.
-  void SlowLoadBehindCaptivePortal(Browser* browser, bool expect_login_tab);
+  void SlowLoadBehindCaptivePortal(Browser* browser,
+                                   bool expect_open_login_tab);
 
   // Same as above, but takes extra parameters.
   //
   // |hanging_url| should either be kMockHttpsUrl or redirect to kMockHttpsUrl.
   //
   // |expected_portal_checks| and |expected_login_tab_navigations| allow
   // client-side redirects to be tested.  |expected_login_tab_navigations| is
   // ignored when |expect_open_login_tab| is false.
   void SlowLoadBehindCaptivePortal(Browser* browser,
                                    bool expect_open_login_tab,
@@ skipping 1089 matching lines @@
   EXPECT_EQ(1, portal_observer2.num_results_received());
   EXPECT_EQ(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL,
             portal_observer2.captive_portal_result());
   EXPECT_EQ(0, NumLoadingTabs());
   EXPECT_FALSE(CheckPending(browser()));
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
             GetStateOfTabReloaderAt(browser(), 0));
 }
 
-// This test is very flaky on Linux and is disabled.
-// https://crbug.com/453875
-#if defined(OS_LINUX)
-#define MAYBE_InterstitialTimerReloadWhileLoading \
-        DISABLED_InterstitialTimerReloadWhileLoading
-#else
-#define MAYBE_InterstitialTimerReloadWhileLoading \
-        InterstitialTimerReloadWhileLoading
-#endif
 // Same as above, but instead of stopping, the loading page is reloaded. The end
 // result is the same. (i.e. page load stops, no interstitials shown)
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,
-                       MAYBE_InterstitialTimerReloadWhileLoading) {
+                       InterstitialTimerReloadWhileLoading) {
   net::SpawnedTestServer::SSLOptions https_options;
   https_options.server_certificate =
       net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
   net::SpawnedTestServer https_server(
       net::SpawnedTestServer::TYPE_HTTPS, https_options,
       base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
   ASSERT_TRUE(https_server.Start());
   // The path does not matter.
   GURL cert_error_url = https_server.GetURL(kTestServerLoginPath);
 
@@ skipping 32 matching lines @@
   EXPECT_EQ(1, portal_observer2.num_results_received());
   EXPECT_EQ(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL,
             portal_observer2.captive_portal_result());
   EXPECT_EQ(0, NumLoadingTabs());
   EXPECT_FALSE(CheckPending(browser()));
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
             GetStateOfTabReloaderAt(browser(), 0));
 }
 
-// This test is very flaky on Linux and is disabled.
-// https://crbug.com/453875
-#if defined(OS_LINUX)
-#define MAYBE_InterstitialTimerNavigateAwayWhileLoading \
-        DISABLED_InterstitialTimerNavigateAwayWhileLoading
-#else
-#define MAYBE_InterstitialTimerNavigateAwayWhileLoading \
-        InterstitialTimerNavigateAwayWhileLoading
-#endif
-
-// Same as above, but instead of reloading, the page is navigated away. The new
-// page should load, and no interstitials should be shown.
+// Same as |InterstitialTimerReloadWhileLoading_NoSSLError|, but instead of
+// reloading, the page is navigated away. The new page should load, and no
+// interstitials should be shown.
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,
-                       MAYBE_InterstitialTimerNavigateAwayWhileLoading) {
+                       InterstitialTimerNavigateAwayWhileLoading) {
   net::SpawnedTestServer::SSLOptions https_options;
   https_options.server_certificate =
       net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
   net::SpawnedTestServer https_server(
       net::SpawnedTestServer::TYPE_HTTPS, https_options,
       base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
   ASSERT_TRUE(https_server.Start());
   // The path does not matter.
   GURL cert_error_url = https_server.GetURL(kTestServerLoginPath);
 
@@ skipping 40 matching lines @@
   EXPECT_EQ(1, portal_observer2.num_results_received());
   EXPECT_EQ(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL,
             portal_observer2.captive_portal_result());
   EXPECT_EQ(0, NumLoadingTabs());
   EXPECT_FALSE(CheckPending(browser()));
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
             GetStateOfTabReloaderAt(browser(), 0));
 }
 
+// Same as above, but the hanging load is interrupted by a navigation to the
+// same page, this time committing the navigation. This should end up with an
+// SSL interstitial when not behind a captive portal. This ensures that a new
+// |SSLErrorHandler| is created on a new navigation, even though the tab's
+// WebContents doesn't change.
+IN_PROC_BROWSER_TEST_F(
+    CaptivePortalBrowserTest,
+    InterstitialTimerNavigateWhileLoading_EndWithSSLInterstitial) {
+  net::SpawnedTestServer::SSLOptions https_options;
+  https_options.server_certificate =
+      net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
+  net::SpawnedTestServer https_server(
+      net::SpawnedTestServer::TYPE_HTTPS, https_options,
+      base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(https_server.Start());
+  // The path does not matter.
+  GURL cert_error_url = https_server.GetURL(kTestServerLoginPath);
+  URLRequestMockCaptivePortalJobFactory::SetBehindCaptivePortal(false);
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+  WebContents* broken_tab_contents = tab_strip_model->GetActiveWebContents();
+
+  FastErrorWithInterstitialTimer(browser(), cert_error_url);
+  // Page appears loading. Turn on response to probe request again, and navigate
+  // to the same page. This should result in a cert error which should
+  // instantiate an |SSLErrorHandler| and end up showing an SSL.

[mmenke, 2015/03/20 13:33:50]

"showing an SSL interstitial"?

[meacer, 2015/03/20 17:46:01]

Done.

+  RespondToProbeRequests(true);
+  // Can't have ui_test_utils do the navigation because it will wait for loading
+  // tabs to stop loading before navigating.
+  CaptivePortalObserver portal_observer(browser()->profile());
+  MultiNavigationObserver test_navigation_observer;
+  browser()->OpenURL(content::OpenURLParams(cert_error_url, content::Referrer(),
+                                            CURRENT_TAB,
+                                            ui::PAGE_TRANSITION_TYPED, false));
+  // Expect two navigations: First one for stopping the hanging page, second one
+  // for completing the load of the above navigation.
+  test_navigation_observer.WaitForNavigations(2);
+  // Should end up with an SSL interstitial.
+  WaitForInterstitialAttach(broken_tab_contents);
+  EXPECT_TRUE(broken_tab_contents->ShowingInterstitialPage());

[mmenke, 2015/03/20 13:33:50]

ASSERT_TRUE?

[meacer, 2015/03/20 17:46:01]

Done.

+  EXPECT_EQ(SSLBlockingPage::kTypeForTesting,
+            broken_tab_contents->GetInterstitialPage()
+                ->GetDelegateForTesting()
+                ->GetTypeForTesting());
+  EXPECT_FALSE(broken_tab_contents->IsLoading());
+  EXPECT_EQ(1, portal_observer.num_results_received());
+  EXPECT_EQ(captive_portal::RESULT_INTERNET_CONNECTED,
+            portal_observer.captive_portal_result());
+  EXPECT_EQ(0, NumLoadingTabs());
+  EXPECT_FALSE(CheckPending(browser()));
+  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+  EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
+            GetStateOfTabReloaderAt(browser(), 0));
+}
+
+// Same as above, but this time behind a captive portal.
+IN_PROC_BROWSER_TEST_F(
+    CaptivePortalBrowserTest,
+    InterstitialTimerNavigateWhileLoading_EndWithCaptivePortalInterstitial) {
+  net::SpawnedTestServer::SSLOptions https_options;
+  https_options.server_certificate =
+      net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
+  net::SpawnedTestServer https_server(
+      net::SpawnedTestServer::TYPE_HTTPS, https_options,
+      base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(https_server.Start());
+  // The path does not matter.
+  GURL cert_error_url = https_server.GetURL(kTestServerLoginPath);
+  URLRequestMockCaptivePortalJobFactory::SetBehindCaptivePortal(true);
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+  WebContents* broken_tab_contents = tab_strip_model->GetActiveWebContents();
+  int initial_tab_count = tab_strip_model->count();
+
+  FastErrorWithInterstitialTimer(browser(), cert_error_url);
+  // Page appears loading. Turn on response to probe request again, and navigate
+  // to the same page. This should result in a cert error which should
+  // instantiate an |SSLErrorHandler| and end up showing an SSL.
+  RespondToProbeRequests(true);
+  // Can't have ui_test_utils do the navigation because it will wait for loading
+  // tabs to stop loading before navigating.
+  CaptivePortalObserver portal_observer(browser()->profile());
+  MultiNavigationObserver test_navigation_observer;
+  browser()->OpenURL(content::OpenURLParams(cert_error_url, content::Referrer(),
+                                            CURRENT_TAB,
+                                            ui::PAGE_TRANSITION_TYPED, false));
+  // Expect three navigations:
+  // 1- For stopping the hanging page.
+  // 2- For completing the load of the above navigation.
+  // 3- For completing the load of the login tab.
+  test_navigation_observer.WaitForNavigations(3);
+  // Should end up with a captive portal interstitial and a new login tab.
+  WaitForInterstitialAttach(broken_tab_contents);
+  EXPECT_TRUE(broken_tab_contents->ShowingInterstitialPage());

[mmenke, 2015/03/20 13:33:50]

ASSERT_TRUE?

[meacer, 2015/03/20 17:46:01]

Done.

+  EXPECT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
+            broken_tab_contents->GetInterstitialPage()
+                ->GetDelegateForTesting()
+                ->GetTypeForTesting());
+  ASSERT_EQ(initial_tab_count + 1, tab_strip_model->count());
+  EXPECT_EQ(initial_tab_count, tab_strip_model->active_index());
+  EXPECT_FALSE(broken_tab_contents->IsLoading());
+  EXPECT_EQ(1, portal_observer.num_results_received());
+  EXPECT_EQ(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL,
+            portal_observer.captive_portal_result());
+  EXPECT_EQ(0, NumLoadingTabs());
+  EXPECT_FALSE(CheckPending(browser()));
+  EXPECT_EQ(CaptivePortalTabReloader::STATE_BROKEN_BY_PORTAL,
+            GetStateOfTabReloaderAt(browser(), 0));
+  EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
+            GetStateOfTabReloaderAt(browser(), 1));
+  EXPECT_TRUE(IsLoginTab(tab_strip_model->GetWebContentsAt(1)));
+}
+
 // A cert error triggers a captive portal check and results in opening a login
 // tab.  The user then logs in and the page with the error is reloaded.
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, SSLCertErrorLogin) {
   // Need an HTTP TestServer to handle a dynamically created server redirect.
   ASSERT_TRUE(test_server()->Start());
 
   net::SpawnedTestServer::SSLOptions https_options;
   https_options.server_certificate =
       net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
   net::SpawnedTestServer https_server(
@@ skipping 508 matching lines @@
 
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NEEDS_RELOAD,
             GetStateOfTabReloaderAt(browser(), broken_tab_index));
 
   WaitForInterstitialAttach(broken_tab_contents);
   portal_observer.WaitForResults(1);
 
   EXPECT_EQ(SSLBlockingPage::kTypeForTesting,
             GetInterstitialType(broken_tab_contents));
 }