Fix flaky SSLUI tests

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 7501bfd5efc8215cf630dd18e479c416bf4d0e40..2f3594524f1cbf0819254be200a607d4c644cd6a 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -99,11 +99,17 @@ class SSLUITest : public InProcessBrowserTest {
     // 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) {
@@ -314,9 +320,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndProceed) {
 
 // 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());
@@ -362,9 +366,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestHTTPSExpiredCertAndDontProceed) {
 
 // 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());
 
@@ -400,9 +403,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest,
 
 // 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());
 
@@ -428,8 +430,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest,
 }
 
 // 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());
 
@@ -591,8 +592,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest,
 // 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());
 
@@ -936,8 +936,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectBadToGoodHTTPS) {
 }
 
 // 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());
 
@@ -973,7 +972,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectHTTPToGoodHTTPS) {
 }
 
 // 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());
 
@@ -995,8 +994,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestRedirectHTTPToBadHTTPS) {
 
 // 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());
 
@@ -1127,8 +1125,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestGoodFrameNavigation) {
 
 // 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());
 
@@ -1227,8 +1224,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnauthenticatedFrameNavigation) {
   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());
 
@@ -1246,8 +1242,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnsafeContentsInWorkerFiltered) {
   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());