Fix flaky browser test SSLUITest.BadCertFollowedByGoodCert

chrome/browser/ssl/ssl_browser_tests.cc

Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 59da844d52c303a549a9c989817e7fc46c86922c..51fad59a179de749e09d39dd53ae19e8887d368d 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -273,33 +273,6 @@ class MockSSLCertReporter : public SSLCertReporter {
 
 }  // namespace CertificateReporting
 
-void RootCertsChangedOnIOThread(
-    const scoped_refptr<net::URLRequestContextGetter> context_getter) {
-  net::CertDatabase::GetInstance()->NotifyObserversOfCACertChanged(NULL);
-  context_getter->GetURLRequestContext()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->CloseAllConnections();
-}
-
-// Alerts the URLRequestContext for the given WebContents that a root
-// certificate has changed state or been removed. This, in turn, clears any
-// cached certificate validation in the cert verifier. This will also close all
-// connections in the socket pool of |contents|, so calls to this should be made
-// with care.
-void RootCertsChanged(WebContents* contents) {
-  scoped_refptr<net::URLRequestContextGetter> url_request_context =
-      contents->GetBrowserContext()->GetRequestContextForRenderProcess(
-          contents->GetRenderProcessHost()->GetID());
-  base::RunLoop run_loop;
-  content::BrowserThread::PostTaskAndReply(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&RootCertsChangedOnIOThread, url_request_context),
-      run_loop.QuitClosure());
-  run_loop.Run();
-  base::RunLoop().RunUntilIdle();
-}
-
 }  // namespace
 
 class SSLUITest : public InProcessBrowserTest {
@@ -2349,29 +2322,31 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, InterstitialNotAffectedByHideShow) {
 // However, if this is followed by another visit, and a good certificate
 // is seen for the same host, the original exception is forgotten.
 IN_PROC_BROWSER_TEST_F(SSLUITest, BadCertFollowedByGoodCert) {
+  // It is necessary to use |https_server_expired_| rather than
+  // |https_server_mismatched| because the former shares a host with
+  // |https_server_| and cert exceptions are per host.
+  ASSERT_TRUE(https_server_expired_.Start());
   ASSERT_TRUE(https_server_.Start());
+
+  std::string https_server_expired_host =
+      https_server_.GetURL("files/ssl/google.html").host();
   std::string https_server_host =
       https_server_.GetURL("files/ssl/google.html").host();
+  ASSERT_EQ(https_server_expired_host, https_server_host);

[Ryan Sleevi, 2015/04/24 11:03:40]

This isn't part of the contract of net::TestServer, so I'm uneasy about
depending on it.

I mean, it'll work, but it just adds to the 'hidden API' cost. David probably
has thouhts here.

[davidben, 2015/04/24 14:53:00]

I think this is fine. We'll notice if it breaks and "CERT_EXPIRED and CERT_OK
differ only the notAfter field" is a fine assumption to make, IMO.

 
   WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  net::TestRootCerts* root_certs = net::TestRootCerts::GetInstance();
-
-  ASSERT_TRUE(root_certs);
-  root_certs->Clear();
 
   Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
   ChromeSSLHostStateDelegate* state =
       reinterpret_cast<ChromeSSLHostStateDelegate*>(
           profile->GetSSLHostStateDelegate());
 
-  ui_test_utils::NavigateToURL(browser(),
-                               https_server_.GetURL("files/ssl/google.html"));
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_expired_.GetURL("files/ssl/google.html"));
 
   ProceedThroughInterstitial(tab);
   EXPECT_TRUE(state->HasAllowException(https_server_host));
 
-  ASSERT_TRUE(https_server_.LoadTestRootCert());
-  RootCertsChanged(tab);
   ui_test_utils::NavigateToURL(browser(),
                                https_server_.GetURL("files/ssl/google.html"));
   ASSERT_FALSE(tab->GetInterstitialPage());