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

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

Index: chrome/renderer/autofill/password_autofill_agent_browsertest.cc
diff --git a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
index 01562c4c97913e6b23967874cb226a119dfc9f68..08a5787b80d31ed3f66ec1f67dc3a1c62bbd7536 100644
--- a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
+++ b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
@@ -2085,4 +2085,100 @@ TEST_F(PasswordAutofillAgentTest,
   ASSERT_FALSE(message);
 }
 
+TEST_F(
+    PasswordAutofillAgentTest,
+    SuggestionsOnSubmittingPasswordFormHavingFieldsWithAmbiguousOrNoNameOrIdAttributes) {

[vabr (Chromium), 2015/09/18 14:41:38]

nit: This name is a bit too long. :) You might want to shorten it and add a
comment with the details about what the test does.

[Pritam Nikam, 2015/09/21 10:51:17]

Done.

+  const char kDummyUsernameField[] = "anonymous_username";
+  const char kDummyPasswordField[] = "anonymous_password";
+  const char kPasswordFormWithNoNameOrIdHTML[] =
+      "<FORM name='WithoutNameIdForm' action='http://www.bidule.com'>"
+      "  <INPUT type='text' placeholder='username'/>"
+      "  <INPUT type='password' placeholder='Password'/>"
+      "  <INPUT type='submit' />"
+      "</FORM>";
+
+  const char kChangePasswordFormWithNoNameOrIdHTML[] =
+      "<FORM name='WithoutNameIdForm' action='http://www.bidule.com'>"
+      "  <INPUT type='text' placeholder='username'/>"

[vabr (Chromium), 2015/09/18 14:41:38]

Again, this looks like a sign-up form.
I would expect the change-password form to have an old password and a new
password. The username is optional and so is the confirmation of the new
password.

I defer to Vadym to chose the best type (and if he already chose this one, then
just ignore me here and in the other test).

[dvadym, 2015/09/18 15:59:00]

A password change form may or may not contain old password. So the current HTML
corresponds to the form which we consider to be password change form. But I
agree it would be more clear if the fields have name with reference to old and
new password.

[Pritam Nikam, 2015/09/21 10:51:17]

Done.

Added attributes |autocomplete='current-password'| for old password fields and
|autocomplete='new-password'| for new password fields.

Also added related tests, ptal.

+      "  <INPUT type='password' placeholder='Password'/>"
+      "  <INPUT type='password' placeholder='Confirm Password'/>"
+      "  <INPUT type='submit' />"
+      "</FORM>";
+
+  const char kPasswordFormWithAmbiguousNameOrIdHTML[] =
+      "<FORM name='AmbiguousNameIdForm' action='http://www.bidule.com'>"
+      "  <INPUT type='text' id='credentials' placeholder='username'/>"
+      "  <INPUT type='password' id='credentials' placeholder='Password'/>"
+      "  <INPUT type='submit' />"
+      "</FORM>";
+
+  const struct {
+    const char* html_form;
+    const char* fill_data_username_field_name;
+    const char* fill_data_password_field_name;
+    const char* expected_username_suggestions;
+    const char* expected_password_suggestions;
+    bool expected_is_username_autofillable;
+    bool expected_is_password_autofillable;
+  } test_cases[] = {
+      // Password form without name or id attributes specified for the input
+      // fields.
+      {kPasswordFormWithNoNameOrIdHTML, kDummyUsernameField,
+       kDummyPasswordField, kAliceUsername, kAlicePassword, true, true},
+
+      // Change password form without name or id attributes specified for the
+      // input fields.
+      {kChangePasswordFormWithNoNameOrIdHTML, kDummyUsernameField,
+       kDummyPasswordField, kAliceUsername, kAlicePassword, true, true},
+
+      // Password form with ambiguous name or id attributes specified for the
+      // input fields.
+      {kPasswordFormWithAmbiguousNameOrIdHTML, "credentials", "credentials",
+       kAliceUsername, kAlicePassword, true, true},
+  };
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {

[vabr (Chromium), 2015/09/18 14:41:38]

You can spare yourself the arraysize if you write:

for (const auto& test_case : test_cases) {...}

[Pritam Nikam, 2015/09/21 10:51:17]

Done.

+    SCOPED_TRACE(testing::Message()
+                 << "Iteration: " << i
+                 << ", html_form: " << test_cases[i].html_form
+                 << ", fill_data_username_field_name: "
+                 << test_cases[i].fill_data_username_field_name
+                 << ", fill_data_password_field_name: "
+                 << test_cases[i].fill_data_password_field_name);
+
+    // Load a password form.
+    LoadHTML(test_cases[i].html_form);
+
+    blink::WebDocument document = GetMainFrame()->document();
+    blink::WebVector<blink::WebFormElement> forms;
+    document.forms(forms);
+    blink::WebFormElement form_element = forms[0];
+    std::vector<blink::WebFormControlElement> control_elements =
+        ExtractAutofillableElementsInForm(form_element);
+    username_element_ = control_elements[0].to<blink::WebInputElement>();
+    password_element_ = control_elements[1].to<blink::WebInputElement>();
+
+    UpdateOriginForHTML(test_cases[i].html_form);
+    fill_data_.username_field.name =
+        ASCIIToUTF16(test_cases[i].fill_data_username_field_name);
+    fill_data_.password_field.name =
+        ASCIIToUTF16(test_cases[i].fill_data_password_field_name);
+    fill_data_.additional_logins.clear();
+    fill_data_.other_possible_usernames.clear();
+
+    ClearUsernameAndPasswordFields();
+
+    // Simulate the browser sending back the login info, it triggers the
+    // autocomplete.
+    SimulateOnFillPasswordForm(fill_data_);
+    SimulateSuggestionChoice(username_element_);
+
+    // The username and password should now have been autocompleted.
+    CheckTextFieldsDOMState(test_cases[i].expected_username_suggestions,
+                            test_cases[i].expected_is_username_autofillable,
+                            test_cases[i].expected_password_suggestions,
+                            test_cases[i].expected_is_password_autofillable);
+  }
+}
+
 }  // namespace autofill