Adapt additional SSL interstitial tests to committed interstitials

(Rebase of crrev.com/c/827676)

This adapts remaining SSL interstitial tests in policy_browsertest.cc,
captive_portal_blocking_page_browsertest.cc, and
security_state_tab_helper_browsertest.cc to run with committed interstitials
enabled.

There is a small handful of other relevant tests scattered throughout the
codebase, but those should be straightforward to adapt when we turn on committed
interstitials by default.

Bug: 752372
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic83cd3be0c2f4f95876d0bafbe8922932fa60f3c
Reviewed-on: https://chromium-review.googlesource.com/1041126
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557763}

chrome/browser/ssl/security_state_tab_helper.cc

@@ -141,7 +141,8 @@ void SecurityStateTabHelper::DidFinishNavigation(
         "https://goo.gl/y8SRRv.");
   }
   if (net::IsCertStatusError(security_info.cert_status) &&
-      !net::IsCertStatusMinorError(security_info.cert_status)) {
+      !net::IsCertStatusMinorError(security_info.cert_status) &&
+      !navigation_handle->IsErrorPage()) {
     // Record each time a user visits a site after having clicked through a
     // certificate warning interstitial. This is used as a baseline for
     // interstitial.ssl.did_user_revoke_decision2 in order to determine how

chrome/browser/ssl/ssl_browsertest.cc

@@ -3018,8 +3018,7 @@ IN_PROC_BROWSER_TEST_P(SSLUITest, TestCloseTabWithUnsafePopup) {
 }
 
 // Visit a page over bad https that is a redirect to a page with good https.
-// TODO(estark): switch to SSLUITest when https://crbug.com/792221 is fixed.
-IN_PROC_BROWSER_TEST_F(SSLUITestBase, TestRedirectBadToGoodHTTPS) {
+IN_PROC_BROWSER_TEST_P(SSLUITest, TestRedirectBadToGoodHTTPS) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
@@ -3441,11 +3440,10 @@ IN_PROC_BROWSER_TEST_P(SSLUITest, TestUnauthenticatedFrameNavigation) {
 enum class OffMainThreadFetchMode { kEnabled, kDisabled };
 enum class SSLUIWorkerFetchTestType { kUseFetch, kUseImportScripts };
 
-// TODO(estark): adapt this test class to work with committed interstitials.
-// https://crbug.com/752327
-class SSLUIWorkerFetchTest
-    : public testing::WithParamInterface<SSLUIWorkerFetchTestType>,
-      public SSLUITestBase {
+class SSLUIWorkerFetchTest : public testing::WithParamInterface<
+                                 std::pair<SSLUIWorkerFetchTestType,
+                                           bool /* committed interstitials */>>,
+                             public SSLUITestBase {
  public:
   SSLUIWorkerFetchTest() {
     EXPECT_TRUE(tmp_dir_.CreateUniqueTempDir());
@@ -3453,6 +3451,12 @@ class SSLUIWorkerFetchTest
 
   ~SSLUIWorkerFetchTest() override {}
 
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    if (GetParam().second) {
+      command_line->AppendSwitch(switches::kCommittedInterstitials);
+    }
+  }
+
  protected:
   void WriteFile(const base::FilePath::StringType& filename,
                  base::StringPiece contents) {
@@ -3471,7 +3475,7 @@ class SSLUIWorkerFetchTest
               "    'message',"
               "    event => { document.title = event.data; });"
               "</script>");
-    switch (GetParam()) {
+    switch (GetParam().first) {
       case SSLUIWorkerFetchTestType::kUseFetch:
         WriteFile(FILE_PATH_LITERAL("worker_test_data.txt.mock-http-headers"),
                   "HTTP/1.1 200 OK\n"
@@ -3658,7 +3662,18 @@ IN_PROC_BROWSER_TEST_P(SSLUIWorkerFetchTest,
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID,
                                  AuthState::SHOWING_INTERSTITIAL);
-  ProceedThroughInterstitial(tab);
+  if (AreCommittedInterstitialsEnabled()) {
+    content::TestNavigationObserver nav_observer(tab, 1);
+    security_interstitials::SecurityInterstitialTabHelper* helper =
+        security_interstitials::SecurityInterstitialTabHelper::FromWebContents(
+            tab);
+    helper->GetBlockingPageForCurrentlyCommittedNavigationForTesting()
+        ->CommandReceived(
+            base::IntToString(security_interstitials::CMD_PROCEED));
+    nav_observer.Wait();
+  } else {
+    ProceedThroughInterstitial(tab);
+  }
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID,
                                  AuthState::NONE);
 
@@ -3882,8 +3897,15 @@ IN_PROC_BROWSER_TEST_P(SSLUIWorkerFetchTest, MAYBE_MixedContentSubFrame) {
 INSTANTIATE_TEST_CASE_P(
     /* no prefix */,
     SSLUIWorkerFetchTest,
-    ::testing::Values(SSLUIWorkerFetchTestType::kUseFetch,
-                      SSLUIWorkerFetchTestType::kUseImportScripts));
+    ::testing::Values(
+        std::make_pair(SSLUIWorkerFetchTestType::kUseFetch,
+                       false /* committed interstitials */),
+        std::make_pair(SSLUIWorkerFetchTestType::kUseImportScripts,
+                       false /* committed interstitials */),
+        std::make_pair(SSLUIWorkerFetchTestType::kUseFetch,
+                       true /* committed interstitials */),
+        std::make_pair(SSLUIWorkerFetchTestType::kUseImportScripts,
+                       true /* committed interstititals */)));
 
 // Visits a page with unsafe content and makes sure that if a user exception
 // to the certificate error is present, the image is loaded and script

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -18,8 +18,12 @@
 -MediaGalleriesPlatformAppBrowserTest.ToURL
 -NetInternalsTest.netInternalsSessionBandwidthSucceed
 -NewlibPackagedAppTest.SuccessfulLoad
--PKPModelClientTest.PKPBypass
--PKPModelClientTest.PKPEnforced
+-PKPModelClientTest.PKPBypass/0
+-PKPModelClientTest.PKPBypass/1
+-PKPModelClientTest.PKPEnforced/0
+-PKPModelClientTest.PKPEnforced/1
+-PlatformAppBrowserTest.AppWindowAdjustBoundsToBeVisibleOnScreen
+-PlatformAppBrowserTest.CreateAndCloseAppWindow
 -PolicyTest.DefaultCookiesSetting
 -PrefetchBrowserTestPredictionDisabled.ExperimentDisabled
 -PreviewsOptimizationGuideBrowserTest.NoScriptPreviewsEnabledByWhitelist
@@ -83,6 +87,9 @@
 # crbug.com/776589 Intercepting requests with net::URLRequestFilter.
 -ContinueWhereILeftOffTest.CookiesClearedOnExit
 -DownloadExtensionTest.DownloadExtensionTest_Download_FileSystemURL
+-MimeHandlerViewTests/MimeHandlerViewTest.SingleRequest/0
+-PolicyTest.CertificateTransparencyEnforcementDisabledForUrls/0
+-PolicyTest.CertificateTransparencyEnforcementDisabledForUrls/1
 -PolicyTest.ExtensionInstallSources
 -PolicyTest.ForceGoogleSafeSearch
 -ProfileBrowserTest.SendHPKPReport
@@ -231,7 +238,8 @@
 # https://crbug.com/797292
 -ProcessManagerBrowserTest.NestedURLNavigationsToExtensionBlocked
 -ProcessManagerBrowserTest.NestedURLDownloadsToExtensionAllowed
--SecurityStateTabHelperTest.DefaultSecurityLevelOnSecureFilesystemUrl
+-SecurityStateTabHelperTest.DefaultSecurityLevelOnSecureFilesystemUrl/0
+-SecurityStateTabHelperTest.DefaultSecurityLevelOnSecureFilesystemUrl/1
 
 # Tests check state of NQE in the browser process, but need to be updated to
 # watch NQE in the renderer process.