[Password Manager] Autofill forms with field name and id attributes missing.

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 6a1f0b8c3d1bf7a86ea39317e89351d79a8fa5a4..fa8fe13fcaba399d0508d3f1541def6f9ef6d773 100644
--- a/chrome/browser/password_manager/password_manager_browsertest.cc
+++ b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -131,6 +131,39 @@ void CheckThatCredentialsStored(
   EXPECT_EQ(password, form.password_value);
 }
 
+void FillAndSubmitPasswordForm(

[vabr (Chromium), 2015/08/26 09:36:00]

Please remove this function and use the store directly to save the credential,
rather than doing that through JavaScript. (Search for AddLogin in this file to
see examples of how to do that.)

We should only use JavaScript and HTML interaction to test code, using it to
prepare the test state will result in slower and more flaky tests.

[Pritam Nikam, 2015/08/26 13:25:40]

Done.

But unfortunately that didn't work for me. With new patch set I've added
necessary changes where we do |password_store->AddLogin()| to store passwords.
Atop executing a javascript to retrieve form field values. I'm not able to
figure out the problem. Any pointers here would be a great help.

[vabr (Chromium), 2015/08/26 13:39:00]

I'm afraid I don't understand your sentence "Atop ... values," so I'm not sure I
understand your problem here. If the problem is that the saved passwords are not
filled, I suggest debugging the issue, you should be able to figure out if the
passwords are not stored, the password forms are not matching enough, or if
there is another failed check.

[Pritam Nikam, 2015/08/27 11:42:02]

Acknowledged.

Able to debug the problem. We had 2 problems here:
1. |password_store->AddLogin()| asynchronously adds password form to store.
2. If we store login form having only |username_value|, |password_value| and
|signon_realm| don't actually prefill fields (Ref [1]).

Added fix for both the cases.

[1].
https://code.google.com/p/chromium/codesearch#chromium/src/components/passwor...

+    Profile* profile,
+    content::WebContents* web_contents,
+    content::RenderViewHost* render_view_host) {
+  password_manager::TestPasswordStore* password_store =
+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              profile, ServiceAccessType::IMPLICIT_ACCESS)
+              .get());
+
+  EXPECT_TRUE(password_store->IsEmpty());
+
+  // Fill a form and submit through a <input type="submit"> button.
+  NavigationObserver observer(web_contents);
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(web_contents));
+  std::string fill_and_submit =
+      "document.getElementById('username_field').value = 'myusername';"
+      "document.getElementById('password_field').value = 'mypassword';"
+      "document.getElementById('input_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(render_view_host, fill_and_submit));
+  observer.Wait();
+
+  prompt_observer->Accept();
+
+  // Spin the message loop to make sure the password store had a chance to save
+  // the password.
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+
+  EXPECT_FALSE(password_store->IsEmpty());
+}
+
 }  // namespace
 
 namespace password_manager {
@@ -2126,4 +2159,74 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
 }
 #endif
 
+// Test whether the password form with the username and password fields having
+// ambiguity in id attribute gets autofill correctly.

[vabr (Chromium), 2015/08/26 09:36:00]

nit: autofill -> autofilled

[Pritam Nikam, 2015/08/26 13:25:40]

Done.

+IN_PROC_BROWSER_TEST_F(
+    PasswordManagerBrowserTestBase,
+    AutofillSuggetionsForPasswordFormWithAmbiguousIdAttribute) {
+  NavigateToFile("/password/password_form.html");
+  FillAndSubmitPasswordForm(browser()->profile(), WebContents(),
+                            RenderViewHost());
+
+  // Now, navigate to the password form having ambiguous Ids for username and
+  // password fields and verify whether username and password is autofilled.
+  NavigateToFile("/password/ambiguous_password_form.html");
+
+  // Let the user interact with the page, so that DOM gets modification events,
+  // needed for autofilling fields.
+  content::SimulateMouseClickAt(
+      WebContents(), 0, blink::WebMouseEvent::ButtonLeft, gfx::Point(1, 1));
+
+  std::string get_username =
+      "window.domAutomationController.send("
+      "  document.getElementById('ambiguous_form').elements[0].value);";
+
+  std::string actual_username;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      RenderViewHost(), get_username, &actual_username));
+  ASSERT_EQ("myusername", actual_username);

[vabr (Chromium), 2015/08/26 09:36:00]

EXPECT_EQ seems more appropriate here and on lines 2194, 2222, 2229, there is no
need to abort the whole test if this check fails.

[Pritam Nikam, 2015/08/26 13:25:40]

Done.

+  std::string get_password =
+      "window.domAutomationController.send("
+      "  document.getElementById('ambiguous_form').elements[1].value);";
+  std::string actual_password;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      RenderViewHost(), get_password, &actual_password));
+  ASSERT_EQ("mypassword", actual_password);
+}
+
+// Test whether the password form having username and password fields without
+// name and id attribute  gets autofill correctly.

[vabr (Chromium), 2015/08/26 09:36:00]

nit: autofill -> autofilled

[vabr (Chromium), 2015/08/26 09:36:00]

nit: Please remove extra space between "attribute" and "gets".

[Pritam Nikam, 2015/08/26 13:25:40]

Done.

[Pritam Nikam, 2015/08/26 13:25:40]

Done.

+IN_PROC_BROWSER_TEST_F(
+    PasswordManagerBrowserTestBase,
+    AutofillSuggetionsForPasswordFormWithoutNameOrIdAttribute) {
+  NavigateToFile("/password/password_form.html");
+  FillAndSubmitPasswordForm(browser()->profile(), WebContents(),
+                            RenderViewHost());
+
+  // Now, navigate to the password form having no Ids for username and password
+  // fields and verify whether username and password is autofilled.
+  NavigateToFile("/password/ambiguous_password_form.html");
+
+  // Let the user interact with the page, so that DOM gets modification events,
+  // needed for autofilling fields.
+  content::SimulateMouseClickAt(
+      WebContents(), 0, blink::WebMouseEvent::ButtonLeft, gfx::Point(1, 1));
+
+  std::string get_username =
+      "window.domAutomationController.send("
+      "  document.getElementById('no_username_id_form').elements[0].value);";
+
+  std::string actual_username;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      RenderViewHost(), get_username, &actual_username));
+  ASSERT_EQ("myusername", actual_username);
+  std::string get_password =
+      "window.domAutomationController.send("
+      "  document.getElementById('no_username_id_form').elements[1].value);";
+  std::string actual_password;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      RenderViewHost(), get_password, &actual_password));
+  ASSERT_EQ("mypassword", actual_password);
+}
+
 }  // namespace password_manager