Attach a SecurityStyle to each request in ResourceLoader

chrome/browser/ui/browser_browsertest.cc

Index: chrome/browser/ui/browser_browsertest.cc
diff --git a/chrome/browser/ui/browser_browsertest.cc b/chrome/browser/ui/browser_browsertest.cc
index e1390ac329148cbc079329c20a036d81901202f0..058784ac4188618de291c96c28c96ff5f3beabee 100644
--- a/chrome/browser/ui/browser_browsertest.cc
+++ b/chrome/browser/ui/browser_browsertest.cc
@@ -384,7 +384,8 @@ class SecurityStyleTestObserver : public WebContentsObserver {
 
 // Check that |observer|'s latest event was for an expired certificate
 // and that it saw the proper SecurityStyle and explanations.
-void CheckExpiredSecurityStyle(const SecurityStyleTestObserver& observer) {
+void CheckBrokenSecurityStyle(const SecurityStyleTestObserver& observer,
+                              int error) {
   EXPECT_EQ(content::SECURITY_STYLE_AUTHENTICATION_BROKEN,
             observer.latest_security_style());
 
@@ -397,8 +398,7 @@ void CheckExpiredSecurityStyle(const SecurityStyleTestObserver& observer) {
   EXPECT_EQ(l10n_util::GetStringUTF8(IDS_CERTIFICATE_CHAIN_ERROR),
             expired_explanation.broken_explanations[0].summary);
 
-  base::string16 error_string =
-      base::UTF8ToUTF16(net::ErrorToString(net::ERR_CERT_DATE_INVALID));
+  base::string16 error_string = base::UTF8ToUTF16(net::ErrorToString(error));
   EXPECT_EQ(l10n_util::GetStringFUTF8(
                 IDS_CERTIFICATE_CHAIN_ERROR_DESCRIPTION_FORMAT, error_string),
             expired_explanation.broken_explanations[0].description);
@@ -2950,6 +2950,33 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, SecurityStyleChangedObserver) {
   EXPECT_EQ(0u, observer.latest_explanations().warning_explanations.size());
   EXPECT_EQ(0u, observer.latest_explanations().broken_explanations.size());
 
+  // Navigate to a bad HTTPS page on a different host, and then click
+  // Back to verify that the good security style is seen again.

[davidben, 2015/07/22 20:56:57]

Optional: the good -> the previous good? Makes it clearer that you're not
expecting a good security style from the bad HTTPS page, but from the valid
HTTPS URL from earlier.

[estark, 2015/07/22 22:56:55]

Done.

+  GURL expired_url(https_test_server_expired.GetURL(std::string()));
+  host_resolver()->AddRule("www.example_expired.test", "127.0.0.1");
+  GURL::Replacements replace_expired;
+  replace_expired.SetHostStr("www.example_expired.test");
+  GURL expired_url_different_host =
+      expired_url.ReplaceComponents(replace_expired);

[davidben, 2015/07/22 20:56:57]

Is the base URL being expired_url important, rather than using
https_test_server? Not a huge deal, but it seems you're just interested in the
name mismatch, and not that it's expired.

[estark, 2015/07/22 22:56:54]

Done. Using https_test_server is probably much better because it means the test
doesn't rely on Chrome prioritizing COMMON_NAME_INVALID over DATE_INVALID.

+  ui_test_utils::NavigateToURL(browser(), expired_url_different_host);
+  CheckBrokenSecurityStyle(observer, net::ERR_CERT_COMMON_NAME_INVALID);
+  ProceedThroughInterstitial(web_contents);
+  CheckBrokenSecurityStyle(observer, net::ERR_CERT_COMMON_NAME_INVALID);
+
+  content::WindowedNotificationObserver back_nav_load_observer(
+      content::NOTIFICATION_LOAD_STOP,
+      content::Source<NavigationController>(&browser()
+                                                 ->tab_strip_model()
+                                                 ->GetActiveWebContents()
+                                                 ->GetController()));
+  chrome::GoBack(browser(), CURRENT_TAB);
+  back_nav_load_observer.Wait();
+
+  EXPECT_EQ(content::SECURITY_STYLE_AUTHENTICATED,
+            observer.latest_security_style());
+  EXPECT_EQ(0u, observer.latest_explanations().warning_explanations.size());
+  EXPECT_EQ(0u, observer.latest_explanations().broken_explanations.size());
+
   // Visit an (otherwise valid) HTTPS page that displays mixed content.
   std::string replacement_path;
   ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
@@ -2970,14 +2997,13 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, SecurityStyleChangedObserver) {
   EXPECT_EQ(0u, mixed_content_explanation.broken_explanations.size());
 
   // Visit a broken HTTPS url.
-  GURL expired_url(https_test_server_expired.GetURL(std::string()));
   ui_test_utils::NavigateToURL(browser(), expired_url);
 
   // An interstitial should show, and an event for the lock icon on the
   // interstitial should fire.
   content::WaitForInterstitialAttach(web_contents);
   EXPECT_TRUE(web_contents->ShowingInterstitialPage());
-  CheckExpiredSecurityStyle(observer);
+  CheckBrokenSecurityStyle(observer, net::ERR_CERT_DATE_INVALID);
 
   // Before clicking through, navigate to a different page, and then go
   // back to the interstitial.
@@ -2992,16 +3018,16 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, SecurityStyleChangedObserver) {
   ui_test_utils::NavigateToURL(browser(), expired_url);
   content::WaitForInterstitialAttach(web_contents);
   EXPECT_TRUE(web_contents->ShowingInterstitialPage());
-  CheckExpiredSecurityStyle(observer);
+  CheckBrokenSecurityStyle(observer, net::ERR_CERT_DATE_INVALID);
 
   // Since the next expected style is the same as the previous, clear
   // the observer (to make sure that the event fires twice and we don't
   // just see the previous event's style).
   observer.ClearLatestSecurityStyleAndExplanations();
 
-  // Other conditions cannot be tested after clicking through because
-  // once the interstitial is clicked through, all URLs for this host
-  // will remain in a broken state.
+  // Other conditions cannot be tested on this host after clicking
+  // through because once the interstitial is clicked through, all URLs
+  // for this host will remain in a broken state.
   ProceedThroughInterstitial(web_contents);
-  CheckExpiredSecurityStyle(observer);
+  CheckBrokenSecurityStyle(observer, net::ERR_CERT_DATE_INVALID);
 }