[PasswordManager] Fix Flaky BrowserTest related to dynamically created

chrome/browser/password_manager/password_manager_browsertest.cc

 // Copyright 2013 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 <string>
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/path_service.h"
@@ skipping 213 matching lines @@
       PromptObserver::Create(WebContents()));
   std::string fill_and_submit =
       "document.getElementById('username_field').value = 'temp';"
       "document.getElementById('password_field').value = 'random';"
       "document.getElementById('submit_button').click()";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
   observer.Wait();
   EXPECT_TRUE(prompt_observer->IsShowingPrompt());
 }
 
-// Flaky: crbug.com/301547, observed on win and mac. Probably happens on all
-// platforms.
-IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
-                       DISABLED_PromptForDynamicForm) {
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, PromptForDynamicForm) {
   NavigateToFile("/password/dynamic_password_form.html");
 
+  // Create password form.

[vabr (Chromium), 2015/08/05 08:37:35]

nit: This comment is not necessary, you named the string variable well enough,
so it is clear what it does. (Also below.)

[xunlu, 2015/08/05 18:37:12]

Done.

+  std::string create_form =
+      "document.getElementById('create_form_button').click();";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), create_form));
+
+  // Make sure PasswordStore::GetLogins() returned before submitting the form.

[vabr (Chromium), 2015/08/05 08:37:35]

This comment can be misunderstood. The observer sadly cannot observe the
GetLogins issued by the PasswordFormManager for the added form, but only its
own. It is a clever hack to use a response to a second GetLogins as an
approximation for knowing that the first one has been processed, but we need to
document this in case something goes wrong. Namely, it is still possible that
the message about creating the form in the renderer gets to the browser later
than the observer issues its GetLogin. In that case, the rest of the test might
still be running too early. Therefore I suggest the following comment instead:

// Issue an artificial PasswordStore::GetLogins() request and wait until the
store returns a response. Hopefully, at that point, the PasswordFormManager
created for the new form above will have received and processed the store's
response as well.

(Also below.)

[xunlu, 2015/08/05 18:37:12]

Your version is much more clear, thanks! Done

+  password_manager::TestPasswordStore* password_store =

[vabr (Chromium), 2015/08/05 08:37:35]

(1) You do not need TestPasswordStore interface, just use PasswordStore.
(2) Please keep the store in a scoped_refptr:
scoped_refptr<PasswordStore> password_store =
PasswordStoreFactory::GetForProfile(...);

(Also below.)

[xunlu, 2015/08/05 18:37:12]

Done.

+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS).get());
+  PasswordStoreObserver store_observer(password_store);
+  store_observer.Wait();
+
   // Fill the dynamic password form and submit.
   NavigationObserver observer(WebContents());
   scoped_ptr<PromptObserver> prompt_observer(
       PromptObserver::Create(WebContents()));
   std::string fill_and_submit =
-      "document.getElementById('create_form_button').click();"
       "window.setTimeout(function() {"
       "  document.dynamic_form.username.value = 'tempro';"
       "  document.dynamic_form.password.value = 'random';"
       "  document.dynamic_form.submit();"
       "}, 0)";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
   observer.Wait();
   EXPECT_TRUE(prompt_observer->IsShowingPrompt());
 }
 
@@ skipping 1130 matching lines @@
   // needed for autofilling fields.
   content::SimulateMouseClickAt(
       WebContents(), 0, blink::WebMouseEvent::ButtonLeft, gfx::Point(1, 1));
 
   // Wait until that interaction causes the password value to be revealed.
   WaitForElementValue("password", "mypassword");
 }
 
 // Test that if a form gets autofilled, then it gets autofilled on re-creation
 // as well.
-// TODO(vabr): This is flaky everywhere. http://crbug.com/442704
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
-                       DISABLED_ReCreatedFormsGetFilled) {
+                       ReCreatedFormsGetFilled) {
   NavigateToFile("/password/dynamic_password_form.html");
 
+  // Create password form.
+  std::string create_form =
+      "document.getElementById('create_form_button').click();";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), create_form));
+
+  // Make sure PasswordStore::GetLogins() returned before submitting form.
+  password_manager::TestPasswordStore* password_store =
+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS).get());
+  PasswordStoreObserver store_observer(password_store);
+  store_observer.Wait();
+
   // Fill in the credentials, and make sure they are saved.
   NavigationObserver form_submit_observer(WebContents());
   scoped_ptr<PromptObserver> prompt_observer(
       PromptObserver::Create(WebContents()));
-  std::string create_fill_and_submit =
-      "document.getElementById('create_form_button').click();"
+  std::string fill_and_submit =
       "window.setTimeout(function() {"
       "  var form = document.getElementById('dynamic_form_id');"
       "  form.username.value = 'temp';"
       "  form.password.value = 'random';"
       "  form.submit();"
       "}, 0)";
-  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), create_fill_and_submit));
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
   form_submit_observer.Wait();
   EXPECT_TRUE(prompt_observer->IsShowingPrompt());
   prompt_observer->Accept();
 
   // Reload the original page to have the saved credentials autofilled.
   NavigationObserver reload_observer(WebContents());
   NavigateToFile("/password/dynamic_password_form.html");
   reload_observer.Wait();
-  std::string create_form =
-      "document.getElementById('create_form_button').click();";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), create_form));
   // Wait until the username is filled, to make sure autofill kicked in.
   WaitForElementValue("username_id", "temp");
 
   // Now the form gets deleted and created again. It should get autofilled
   // again.
   std::string delete_form =
       "var form = document.getElementById('dynamic_form_id');"
       "form.parentNode.removeChild(form);";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), delete_form));
@@ skipping 520 matching lines @@
                                                                      top + 1));
   // Verify password has been autofilled
   WaitForElementValue("iframe", "password_field", "pa55w0rd");
 
   // Verify username has been autofilled
   CheckElementValue("iframe", "username_field", "temp");
 
 }
 
 }  // namespace password_manager