[PasswordManager] Fix Flaky BrowserTest related to dynamically created

chrome/browser/password_manager/password_manager_browsertest.cc

Index: chrome/browser/password_manager/password_manager_browsertest.cc
diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc
index ccaae8d9a21c623c6f5582ae3bac48a0e81e56eb..8613a288cf1913f3f40e4e426c7eb91268dbf9e1 100644
--- a/chrome/browser/password_manager/password_manager_browsertest.cc
+++ b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -231,18 +231,27 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
   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';"
@@ -1393,24 +1402,35 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
 
 // 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();
@@ -1419,8 +1439,6 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
   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");