Fix flaky password visibility browser test

chrome/browser/ssl/security_state_tab_helper_browser_tests.cc

 // Copyright 2015 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 "chrome/browser/ssl/security_state_tab_helper.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/strings/string_split.h"
@@ skipping 50 matching lines @@
 enum CertificateStatus { VALID_CERTIFICATE, INVALID_CERTIFICATE };
 
 const base::FilePath::CharType kDocRoot[] =
     FILE_PATH_LITERAL("chrome/test/data");
 
 // Inject a script into the page. Used by tests that check for visible
 // password fields to wait for notifications about these
 // fields. Notifications about visible password fields are queued at the
 // end of the event loop, so waiting for a dummy script to run ensures
 // that these notifcations have been sent.
-void InjectScript(content::WebContents* contents) {
+void InjectScript(const content::ToRenderFrameHost& adapter) {
   bool js_result = false;
   EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
-      contents, "window.domAutomationController.send(true);", &js_result));
+      adapter, "window.domAutomationController.send(true);", &js_result));
   EXPECT_TRUE(js_result);
 }
 
 // A WebContentsObserver useful for testing the DidChangeVisibleSecurityState()
 // method: it keeps track of the latest security style and explanation that was
 // fired.
 class SecurityStyleTestObserver : public content::WebContentsObserver {
  public:
   explicit SecurityStyleTestObserver(content::WebContents* web_contents)
       : content::WebContentsObserver(web_contents),
@@ skipping 1045 matching lines @@
   ASSERT_TRUE(contents);
 
   SecurityStateTabHelper* helper =
       SecurityStateTabHelper::FromWebContents(contents);
   ASSERT_TRUE(helper);
 
   ui_test_utils::NavigateToURL(
       browser(),
       GetURLWithNonLocalHostname(embedded_test_server(),
                                  "/password/simple_password_in_iframe.html"));
-  InjectScript(contents);
+  // Inject a dummy script into each frame to ensure that password notifications

[estark, 2017/02/10 00:55:34]

note: this test was not marked as flaky. I'm guessing this is because it's an
OOPIF iframe, so we don't technically need to inject into all the frames, but I
figured it was better for consistency.

[meacer, 2017/02/10 01:07:40]

Would it make sense to move this code to InjectScript and make it take a
WebContents so that it injects into all frames automatically? 

[estark, 2017/02/10 01:45:13]

Done.

+  // from any subframe have been sent.
+  for (const auto& frame : contents->GetAllFrames()) {
+    InjectScript(content::ToRenderFrameHost(frame));
+  }
   security_state::SecurityInfo security_info;
   helper->GetSecurityInfo(&security_info);
   EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
 
   content::NavigationEntry* entry = contents->GetController().GetVisibleEntry();
   ASSERT_TRUE(entry);
   EXPECT_TRUE(entry->GetSSL().content_status &
               content::SSLStatus::DISPLAYED_PASSWORD_FIELD_ON_HTTP);
 }
 
 // Tests that when a visible password field is detected inside an iframe
 // on an HTTP page load, and when the command-line flag is set, the
 // security level is downgraded to HTTP_SHOW_WARNING, even if the iframe
 // itself was loaded over HTTPS.
-#if defined(OS_LINUX)
-// Flaky on Linux. See https://crbug.com/662485.
-#define MAYBE_PasswordSecurityLevelDowngradedFromHttpsIframe \
-  DISABLED_PasswordSecurityLevelDowngradedFromHttpsIframe
-#else
-#define MAYBE_PasswordSecurityLevelDowngradedFromHttpsIframe \
-  PasswordSecurityLevelDowngradedFromHttpsIframe
-#endif
 IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTestWithPasswordCcSwitch,
-                       MAYBE_PasswordSecurityLevelDowngradedFromHttpsIframe) {
+                       PasswordSecurityLevelDowngradedFromHttpsIframe) {
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(contents);
 
   SecurityStateTabHelper* helper =
       SecurityStateTabHelper::FromWebContents(contents);
   ASSERT_TRUE(helper);
 
   // Navigate to an HTTP URL, which loads an iframe using the host and port of
   // |https_server_|.
   std::string replacement_path;
   GetFilePathWithHostAndPortReplacement(
       "/password/simple_password_in_https_iframe.html",
       https_server_.host_port_pair(), &replacement_path);
   ui_test_utils::NavigateToURL(
       browser(),
       GetURLWithNonLocalHostname(embedded_test_server(), replacement_path));
-  InjectScript(contents);
+  // Inject a dummy script into each frame to ensure that password notifications
+  // from any subframe have been sent.

[meacer, 2017/02/10 01:07:40]

nit: any subframe or all subframes? They sound different to me.

[estark, 2017/02/10 01:45:13]

Probably should have been "all subframes", but n/a now

+  for (const auto& frame : contents->GetAllFrames()) {
+    InjectScript(content::ToRenderFrameHost(frame));
+  }
   security_state::SecurityInfo security_info;
   helper->GetSecurityInfo(&security_info);
   EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level);
 
   content::NavigationEntry* entry = contents->GetController().GetVisibleEntry();
   ASSERT_TRUE(entry);
   EXPECT_TRUE(entry->GetSSL().content_status &
               content::SSLStatus::DISPLAYED_PASSWORD_FIELD_ON_HTTP);
 }
 
@@ skipping 965 matching lines @@
   SecurityStateTabHelper* helper =
       SecurityStateTabHelper::FromWebContents(web_contents);
   ASSERT_TRUE(helper);
   security_state::SecurityInfo security_info;
   helper->GetSecurityInfo(&security_info);
   EXPECT_EQ(security_state::SECURE, security_info.security_level);
   EXPECT_EQ(kTestSCTStatuses, security_info.sct_verify_statuses);
 }
 
 }  // namespace