Fix flaky SSLUI tests

chrome/browser/ssl/ssl_browser_tests.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 "base/command_line.h"
 #include "base/time.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator.h"
@@ skipping 81 matching lines @@
     ASSERT_TRUE(entry);
     EXPECT_EQ(interstitial ?
                   content::PAGE_TYPE_INTERSTITIAL : content::PAGE_TYPE_NORMAL,
               entry->GetPageType());
     EXPECT_EQ(content::SECURITY_STYLE_AUTHENTICATION_BROKEN,
               entry->GetSSL().security_style);
     // CERT_STATUS_UNABLE_TO_CHECK_REVOCATION doesn't lower the security style
     // to SECURITY_STYLE_AUTHENTICATION_BROKEN.
     ASSERT_NE(net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION, error);
     EXPECT_EQ(error,
+              error &
               entry->GetSSL().cert_status & net::CERT_STATUS_ALL_ERRORS);

[wtc, 2012/02/17 00:14:07]

If you want to allow extraneous certificate errors, you can
just say:
  EXPECT_EQ(error, entry->GetSSL().cert_status & error);

[tpayne, 2012/02/17 19:25:47]

Done.

     EXPECT_FALSE(!!(entry->GetSSL().content_status &
                     SSLStatus::DISPLAYED_INSECURE_CONTENT));
-    EXPECT_EQ(ran_insecure_content, 
+    EXPECT_EQ(ran_insecure_content,
         !!(entry->GetSSL().content_status & SSLStatus::RAN_INSECURE_CONTENT));
+    unsigned int extra_cert_errors = error ^ (entry->GetSSL().cert_status &

[wtc, 2012/02/17 00:14:07]

Nit: it looks nicer to use net::CertStatus instead of unsigned int.

[tpayne, 2012/02/17 19:25:47]

Done.

+                                              net::CERT_STATUS_ALL_ERRORS);
+    if (extra_cert_errors) {
+      LOG(WARNING) << "Got unexpected cert error: " << extra_cert_errors;
+    }

[wtc, 2012/02/17 00:14:07]

Nit: this file seems to omit curly braces for one-liner if
statements, so we should follow that style.

[tpayne, 2012/02/17 19:25:47]

Done.

   }
 
   void CheckWorkerLoadResult(WebContents* tab, bool expectLoaded) {
     // Workers are async and we don't have notifications for them passing
     // messages since they do it between renderer and worker processes.
     // So have a polling loop, check every 200ms, timeout at 30s.
     const int timeout_ms = 200;
     base::Time timeToQuit = base::Time::Now() +
         base::TimeDelta::FromMilliseconds(30000);
 
@@ skipping 190 matching lines @@
                                  true);  // Interstitial showing
 
   ProceedThroughInterstitial(tab);
 
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  false);  // No interstitial showing
 }
 
 // Visits a page with https error and don't proceed (and ensure we can still
 // navigate at that point):
-// Disabled, flakily exceeds test timeout, http://crbug.com/43575.
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestHTTPSExpiredCertAndDontProceed) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndDontProceed) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // First navigate to an OK page.
   ui_test_utils::NavigateToURL(browser(),
                                https_server_.GetURL("files/ssl/google.html"));
 
   WebContents* tab = browser()->GetSelectedWebContents();
   NavigationEntry* entry = tab->GetController().GetActiveEntry();
@@ skipping 24 matching lines @@
   // We should be back to the original good page.
   CheckAuthenticatedState(tab, false);
 
   // Try to navigate to a new page. (to make sure bug 5800 is fixed).
   ui_test_utils::NavigateToURL(browser(),
                                test_server()->GetURL("files/ssl/google.html"));
   CheckUnauthenticatedState(tab);
 }
 
 // Visits a page with https error and then goes back using Browser::GoBack.
 // See bug 114185.

[dpapad, 2012/02/16 23:48:22]

Could you also remove this comment and add the bug number to the cl description?

[wtc, 2012/02/17 00:14:07]

I believe we need to keep the "See bug 114185." comment
because this unit test is a regression test for that bug.

[tpayne, 2012/02/17 19:25:47]

I think Demetrios is right. Bug 114185 seems to be a duplicate of 40932.

[tpayne, 2012/02/17 19:25:47]

Done.

-// Marked as flaky, see bug 40932.
 IN_PROC_BROWSER_TEST_F(SSLUITest,
-                       DISABLED_TestHTTPSExpiredCertAndGoBackViaButton) {
+                       TestHTTPSExpiredCertAndGoBackViaButton) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // First navigate to an HTTP page.
   ui_test_utils::NavigateToURL(browser(),
       test_server()->GetURL("files/ssl/google.html"));
   WebContents* tab = browser()->GetSelectedWebContents();
   NavigationEntry* entry = tab->GetController().GetActiveEntry();
   ASSERT_TRUE(entry);
 
@@ skipping 15 matching lines @@
   load_failed_observer.Wait();
   EXPECT_FALSE(tab->GetRenderViewHost()->is_swapped_out());
 
   // We should be back at the original good page.
   EXPECT_FALSE(browser()->GetSelectedWebContents()->GetInterstitialPage());
   CheckUnauthenticatedState(tab);
 }
 
 // Visits a page with https error and then goes back using GoToOffset.
 // See bug 114185.
-// Marked as flaky, see bug 40932.
 IN_PROC_BROWSER_TEST_F(SSLUITest,
-                       DISABLED_TestHTTPSExpiredCertAndGoBackViaMenu) {
+                       TestHTTPSExpiredCertAndGoBackViaMenu) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // First navigate to an HTTP page.
   ui_test_utils::NavigateToURL(browser(),
       test_server()->GetURL("files/ssl/google.html"));
   WebContents* tab = browser()->GetSelectedWebContents();
   NavigationEntry* entry = tab->GetController().GetActiveEntry();
   ASSERT_TRUE(entry);
 
   // Now go to a bad HTTPS page that shows an interstitial.
   ui_test_utils::NavigateToURL(browser(),
       https_server_expired_.GetURL("files/ssl/google.html"));
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  true);  // Interstitial showing
 
   // Simulate user clicking and holding on back button (crbug.com/37215).
   tab->GetController().GoToOffset(-1);
 
   // We should be back at the original good page.
   EXPECT_FALSE(browser()->GetSelectedWebContents()->GetInterstitialPage());
   CheckUnauthenticatedState(tab);
 }
 
 // Visits a page with https error and then goes forward using GoToOffset.
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestHTTPSExpiredCertAndGoForward) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoForward) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // First navigate to two HTTP pages.
   ui_test_utils::NavigateToURL(browser(),
       test_server()->GetURL("files/ssl/google.html"));
   WebContents* tab = browser()->GetSelectedWebContents();
   NavigationEntry* entry1 = tab->GetController().GetActiveEntry();
   ASSERT_TRUE(entry1);
   ui_test_utils::NavigateToURL(browser(),
@@ skipping 141 matching lines @@
   ui_test_utils::NavigateToURL(browser(), https_server_.GetURL(
       "files/ssl/page_runs_insecure_content.html"));
 
   CheckAuthenticationBrokenState(browser()->GetSelectedWebContents(), 0, true,
                                  false);
 }
 
 // Visits a page with unsafe content and make sure that:
 // - frames content is replaced with warning
 // - images and scripts are filtered out entirely
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnsafeContents) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContents) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   std::string replacement_path;
   ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
       "files/ssl/page_with_unsafe_contents.html",
       https_server_expired_.host_port_pair(),
       &replacement_path));
   ui_test_utils::NavigateToURL(browser(),
                                https_server_.GetURL(replacement_path));
@@ skipping 323 matching lines @@
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  true);  // Interstitial showing.
 
   ProceedThroughInterstitial(tab);
 
   // We have been redirected to the good page.
   CheckAuthenticatedState(tab, false);
 }
 
 // Visit a page over good https that is a redirect to a page with bad https.
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestRedirectGoodToBadHTTPS) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectGoodToBadHTTPS) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   GURL url1 = https_server_.GetURL("server-redirect?");
   GURL url2 = https_server_expired_.GetURL("files/ssl/google.html");
   ui_test_utils::NavigateToURL(browser(), GURL(url1.spec() + url2.spec()));
 
   WebContents* tab = browser()->GetSelectedWebContents();
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  true);  // Interstitial showing.
@@ skipping 15 matching lines @@
   GURL http_url = test_server()->GetURL("server-redirect?");
   GURL good_https_url =
       https_server_.GetURL("files/ssl/google.html");
 
   ui_test_utils::NavigateToURL(browser(),
                                GURL(http_url.spec() + good_https_url.spec()));
   CheckAuthenticatedState(tab, false);
 }
 
 // Visit a page over http that is a redirect to a page with bad HTTPS.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestRedirectHTTPToBadHTTPS) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectHTTPToBadHTTPS) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   WebContents* tab = browser()->GetSelectedWebContents();
 
   GURL http_url = test_server()->GetURL("server-redirect?");
   GURL bad_https_url =
       https_server_expired_.GetURL("files/ssl/google.html");
   ui_test_utils::NavigateToURL(browser(),
                                GURL(http_url.spec() + bad_https_url.spec()));
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  true);  // Interstitial showing.
 
   ProceedThroughInterstitial(tab);
 
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  false);  // No interstitial showing.
 }
 
 // Visit a page over https that is a redirect to a page with http (to make sure
 // we don't keep the secure state).
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestRedirectHTTPSToHTTP) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectHTTPSToHTTP) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_.Start());
 
   GURL https_url = https_server_.GetURL("server-redirect?");
   GURL http_url = test_server()->GetURL("files/ssl/google.html");
 
   ui_test_utils::NavigateToURL(browser(),
                                GURL(https_url.spec() + http_url.spec()));
   CheckUnauthenticatedState(browser()->GetSelectedWebContents());
 }
@@ skipping 110 matching lines @@
         content::NOTIFICATION_LOAD_STOP,
         content::Source<NavigationController>(&tab->GetController()));
     tab->GetController().GoBack();
     observer.Wait();
   }
   CheckAuthenticatedState(tab, true);
 }
 
 // From a bad HTTPS top frame:
 // - navigate to an OK HTTPS frame (expected to be still authentication broken).
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestBadFrameNavigation) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestBadFrameNavigation) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   std::string top_frame_path;
   ASSERT_TRUE(GetTopFramePath(*test_server(),
                               https_server_,
                               https_server_expired_,
                               &top_frame_path));
 
   WebContents* tab = browser()->GetSelectedWebContents();
@@ skipping 78 matching lines @@
   bool is_content_evil = true;
   std::wstring content_frame_xpath(L"html/frameset/frame[2]");
   std::wstring is_evil_js(L"window.domAutomationController.send("
                           L"document.getElementById('evilDiv') != null);");
   EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
       tab->GetRenderViewHost(), content_frame_xpath, is_evil_js,
       &is_content_evil));
   EXPECT_FALSE(is_content_evil);
 }
 
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnsafeContentsInWorkerFiltered) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContentsInWorkerFiltered) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // This page will spawn a Worker which will try to load content from
   // BadCertServer.
   std::string page_with_unsafe_worker_path;
   ASSERT_TRUE(GetPageWithUnsafeWorkerPath(https_server_expired_,
                                           &page_with_unsafe_worker_path));
   ui_test_utils::NavigateToURL(browser(), https_server_.GetURL(
       page_with_unsafe_worker_path));
   WebContents* tab = browser()->GetSelectedWebContents();
   // Expect Worker not to load insecure content.
   CheckWorkerLoadResult(tab, false);
   // The bad content is filtered, expect the state to be authenticated.
   CheckAuthenticatedState(tab, false);
 }
 
-// Marked as flaky, see bug 40932.
-IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnsafeContentsInWorker) {
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContentsInWorker) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(https_server_expired_.Start());
 
   // Navigate to an unsafe site. Proceed with interstitial page to indicate
   // the user approves the bad certificate.
   ui_test_utils::NavigateToURL(browser(),
       https_server_expired_.GetURL("files/ssl/blank_page.html"));
   WebContents* tab = browser()->GetSelectedWebContents();
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
                                  true);  // Interstitial showing
@@ skipping 76 matching lines @@
 
 // Visit a page over https that contains a frame with a redirect.
 
 // XMLHttpRequest insecure content in synchronous mode.
 
 // XMLHttpRequest insecure content in asynchronous mode.
 
 // XMLHttpRequest over bad ssl in synchronous mode.
 
 // XMLHttpRequest over OK ssl in synchronous mode.