Downgrade security state while displaying an SB interstitial

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

Index: chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
index e4e535bf1680978e31be1d012f794d8195354354..13f53b938f0dd1040eb4e79170387e18fabcfff5 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -571,6 +571,15 @@ class SafeBrowsingBlockingPageBrowserTest
     EXPECT_EQ(expected_tag_name, actual_resource.tag_name());
   }
 
+  void ExpectSecurityIndicatorDowngrade(content::WebContents* tab) {
+    ChromeSecurityStateModelClient* model_client =
+        ChromeSecurityStateModelClient::FromWebContents(tab);
+    ASSERT_TRUE(model_client);
+    EXPECT_EQ(security_state::SecurityStateModel::SECURITY_ERROR,
+              model_client->GetSecurityInfo().security_level);
+    EXPECT_TRUE(model_client->GetSecurityInfo().fails_malware_check);
+  }
+
  protected:
   TestThreatDetailsFactory details_factory_;
 
@@ -1047,36 +1056,36 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, WhitelistUnsaved) {
 // Safe Browsing interstitial.
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
                        SecurityState_HTTP) {
+  // The security indicator should be downgraded while the interstitial shows.
   SetupWarningAndNavigate();
+  WebContents* error_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(error_tab);
+  ExpectSecurityIndicatorDowngrade(error_tab);
+
+  // The security indicator should still be downgraded post-interstitial.
   EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
   AssertNoInterstitial(true);
-
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  ASSERT_TRUE(tab);
-  ChromeSecurityStateModelClient* model_client =
-      ChromeSecurityStateModelClient::FromWebContents(tab);
-  ASSERT_TRUE(model_client);
-  EXPECT_EQ(security_state::SecurityStateModel::SECURITY_ERROR,
-            model_client->GetSecurityInfo().security_level);
-  EXPECT_TRUE(model_client->GetSecurityInfo().fails_malware_check);
+  WebContents* post_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(post_tab);
+  ExpectSecurityIndicatorDowngrade(post_tab);
 }
 
 // Test that the security indicator is downgraded even if the website has valid
 // HTTPS (meaning that the SB state overrides the HTTPS state).
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
                        SecurityState_ValidHTTPS) {
+  // The security indicator should be downgraded while the interstitial shows.
   SetupWarningAndNavigateToValidHTTPS();
+  WebContents* error_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(error_tab);
+  ExpectSecurityIndicatorDowngrade(error_tab);
+
+  // The security indicator should still be downgraded post-interstitial.
   EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
   AssertNoInterstitial(true);
-
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  ASSERT_TRUE(tab);
-  ChromeSecurityStateModelClient* model_client =
-      ChromeSecurityStateModelClient::FromWebContents(tab);
-  ASSERT_TRUE(model_client);
-  EXPECT_EQ(security_state::SecurityStateModel::SECURITY_ERROR,
-            model_client->GetSecurityInfo().security_level);
-  EXPECT_TRUE(model_client->GetSecurityInfo().fails_malware_check);
+  WebContents* post_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(post_tab);
+  ExpectSecurityIndicatorDowngrade(post_tab);
   EXPECT_EQ(0u, model_client->GetSecurityInfo().cert_status);
 }
 
@@ -1084,18 +1093,18 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
 // are shown in a row (one for Safe Browsing, one for invalid HTTPS).
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
                        SecurityState_InvalidHTTPS) {
+  // The security indicator should be downgraded while the interstitial shows.
   SetupWarningAndNavigateToInvalidHTTPS();
+  WebContents* error_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(error_tab);
+  ExpectSecurityIndicatorDowngrade(error_tab);
+
+  // The security indicator should still be downgraded post-interstitial.
   EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
   AssertNoInterstitial(true);
-
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  ASSERT_TRUE(tab);
-  ChromeSecurityStateModelClient* model_client =
-      ChromeSecurityStateModelClient::FromWebContents(tab);
-  ASSERT_TRUE(model_client);
-  EXPECT_EQ(security_state::SecurityStateModel::SECURITY_ERROR,
-            model_client->GetSecurityInfo().security_level);
-  EXPECT_TRUE(model_client->GetSecurityInfo().fails_malware_check);
+  WebContents* post_tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(post_tab);
+  ExpectSecurityIndicatorDowngrade(post_tab);
   EXPECT_NE(0u, model_client->GetSecurityInfo().cert_status);

[felt, 2016/08/25 06:10:33]

Note: this line (which depends on having a model_client) needs to move up into
ExpectSecurityIndicatorDowngrade(), but I'm leaving it here for now while I
figure out why that check is failing in the dependent CL
(https://codereview.chromium.org/2270283002/).

 }