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

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 28a5398b82ea6dccf6c5749aa7ff9b313fcc7e3e..e811647439dd81a9299b115e718cde209fef1468 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -54,6 +54,8 @@ const char kFillOnAccountSelectFieldTrialEnabledWithHighlightGroup[] =
     "EnableWithHighlight";
 const char kFillOnAccountSelectFieldTrialEnabledWithNoHighlightGroup[] =
     "EnableWithNoHighlight";
+const char kDummyUsernameField[] = "anonymous_username";
+const char kDummyPasswordField[] = "anonymous_password";
 
 // Maps element names to the actual elements to simplify form filling.
 typedef std::map<base::string16, blink::WebInputElement> FormInputElementMap;
@@ -84,6 +86,32 @@ bool IsNamedElementVisible(
   return false;
 }
 
+// Returns true if password form has username and password fields with either
+// same or no name and id attributes supplied.
+bool PasswordFormWithAmbiguousOrNoNameAndIdAttribute(

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

Usually, predicates are named as the questions they answer. Here, I suggest:

DoesFormContainAmbiguousOrEmptyNames

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

Done.

+    const PasswordFormFillData& fill_data) {
+  return (fill_data.username_field.name == fill_data.password_field.name) ||
+         (fill_data.password_field.name ==
+              base::ASCIIToUTF16(kDummyPasswordField) &&
+          (!FillDataContainsFillableUsername(fill_data) ||
+           fill_data.username_field.name ==
+               base::ASCIIToUTF16(kDummyUsernameField)));
+}
+
+bool IsPasswordField(const FormFieldData& field) {
+  return (field.form_control_type == "password");
+}
+
+// Returns the |field|'s autofillable name. If no name or id attribute is

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

The second sentence should reference the bool argument instead of its expected
meaning.

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

Done.

+// specified returns a dummy name.
+base::string16 FieldName(const FormFieldData& field,
+                         bool ambiguous_or_no_name_and_id_attribute) {
+  return ambiguous_or_no_name_and_id_attribute
+             ? IsPasswordField(field) ? base::ASCIIToUTF16(kDummyPasswordField)
+                                      : base::ASCIIToUTF16(kDummyUsernameField)
+             : field.name;
+}
+
 // Utility function to find the unique entry of |control_elements| for the
 // specified input |field|. On successful find, adds it to |result| and returns
 // |true|. Otherwise clears the references from each |HTMLInputElement| from
@@ -91,33 +119,40 @@ bool IsNamedElementVisible(
 bool FindFormInputElement(
     const std::vector<blink::WebFormControlElement>& control_elements,
     const FormFieldData& field,
+    bool ambiguous_or_no_name_and_id_attribute,
     FormInputElementMap* result) {
   // Match the first input element, if any.
-  // If more than one match is made, then we have ambiguity (due to misuse
-  // of "name" attribute) so is it considered not found.
   bool found_input = false;
+  base::string16 field_name =
+      FieldName(field, ambiguous_or_no_name_and_id_attribute);
   for (size_t i = 0; i < control_elements.size(); ++i) {
-    if (control_elements[i].nameForAutofill() != field.name)
+    if (!ambiguous_or_no_name_and_id_attribute &&
+        control_elements[i].nameForAutofill() != field_name) {
       continue;
+    }
 
     if (!control_elements[i].hasHTMLTagName("input"))
       continue;
 
+    // Only fill saved passwords into password fields and usernames into text
+    // fields.
+    const blink::WebInputElement input_element =
+        control_elements[i].toConst<blink::WebInputElement>();
+    bool is_password_field = IsPasswordField(field);
+    if (input_element.isPasswordField() != is_password_field)
+      continue;
+
     // Check for a non-unique match.
     if (found_input) {
+      // For change password form keep only the first password field entry.
+      if (ambiguous_or_no_name_and_id_attribute && is_password_field)
+        continue;
+
       found_input = false;
       break;
     }
 
-    // Only fill saved passwords into password fields and usernames into
-    // text fields.
-    const blink::WebInputElement input_element =
-        control_elements[i].toConst<blink::WebInputElement>();
-    if (input_element.isPasswordField() !=
-        (field.form_control_type == "password"))
-      continue;
-
-    (*result)[field.name] = input_element;
+    (*result)[field_name] = input_element;
     found_input = true;
   }
 
@@ -179,15 +214,19 @@ bool ShouldHighlightFields() {
 bool FindFormInputElements(
     const std::vector<blink::WebFormControlElement>& control_elements,
     const PasswordFormFillData& data,
+    bool ambiguous_or_no_name_and_id_attribute,
     FormInputElementMap* result) {
-  return FindFormInputElement(control_elements, data.password_field, result) &&
+  return FindFormInputElement(control_elements, data.password_field,
+                              ambiguous_or_no_name_and_id_attribute, result) &&
          (!FillDataContainsFillableUsername(data) ||
-          FindFormInputElement(control_elements, data.username_field, result));
+          FindFormInputElement(control_elements, data.username_field,
+                               ambiguous_or_no_name_and_id_attribute, result));
 }
 
 // Helper to locate form elements identified by |data|.
 void FindFormElements(content::RenderFrame* render_frame,
                       const PasswordFormFillData& data,
+                      bool ambiguous_or_no_name_and_id_attribute,
                       FormElementsList* results) {
   DCHECK(results);
 
@@ -211,7 +250,8 @@ void FindFormElements(content::RenderFrame* render_frame,
     std::vector<blink::WebFormControlElement> control_elements =
         ExtractAutofillableElementsInForm(fe);
     FormInputElementMap cur_map;
-    if (FindFormInputElements(control_elements, data, &cur_map))
+    if (FindFormInputElements(control_elements, data,
+                              ambiguous_or_no_name_and_id_attribute, &cur_map))
       results->push_back(cur_map);
   }
   // If the element to be filled are not in a <form> element, the "action" and
@@ -222,7 +262,9 @@ void FindFormElements(content::RenderFrame* render_frame,
   std::vector<blink::WebFormControlElement> control_elements =
       GetUnownedAutofillableFormFieldElements(doc.all(), nullptr);
   FormInputElementMap unowned_elements_map;
-  if (FindFormInputElements(control_elements, data, &unowned_elements_map))
+  if (FindFormInputElements(control_elements, data,
+                            ambiguous_or_no_name_and_id_attribute,
+                            &unowned_elements_map))
     results->push_back(unowned_elements_map);
 }
 
@@ -453,6 +495,13 @@ bool FillFormOnPasswordReceived(
 
   bool form_contains_fillable_username_field =
       FillDataContainsFillableUsername(fill_data);
+  bool ambiguous_or_no_name_and_id_attribute =
+      PasswordFormWithAmbiguousOrNoNameAndIdAttribute(fill_data);
+  base::string16 username_field_name;
+  if (form_contains_fillable_username_field)
+    username_field_name = FieldName(fill_data.username_field,
+                                    ambiguous_or_no_name_and_id_attribute);
+
   // If the form contains an autocompletable username field, try to set the
   // username to the preferred name, but only if:
   //   (a) The fill-on-account-select flag is not set, and
@@ -469,7 +518,7 @@ bool FillFormOnPasswordReceived(
   // in the "no highlighting" group.
   //
   // In all other cases, do nothing.
-  bool form_has_fillable_username = form_contains_fillable_username_field &&
+  bool form_has_fillable_username = !username_field_name.empty() &&
                                     IsElementAutocompletable(username_element);
 
   if (ShouldFillOnAccountSelect()) {
@@ -815,8 +864,11 @@ bool PasswordAutofillAgent::ShowSuggestions(
       !IsElementAutocompletable(password_info->password_field))
     return true;
 
-  if (element.nameForAutofill().isEmpty())
+  if (element.nameForAutofill().isEmpty() &&
+      !PasswordFormWithAmbiguousOrNoNameAndIdAttribute(
+          password_info->fill_data)) {
     return false;  // If the field has no name, then we won't have values.
+  }
 
   // Don't attempt to autofill with values that are too large.
   if (element.value().length() > kMaximumTextSizeForAutocomplete)
@@ -1195,24 +1247,32 @@ void PasswordAutofillAgent::LegacyDidStartProvisionalLoad(
 void PasswordAutofillAgent::OnFillPasswordForm(
     int key,
     const PasswordFormFillData& form_data) {
-
+  bool ambiguous_or_no_name_and_id_attribute =
+      PasswordFormWithAmbiguousOrNoNameAndIdAttribute(form_data);
   FormElementsList forms;
-  FindFormElements(render_frame(), form_data, &forms);
+  FindFormElements(render_frame(), form_data,
+                   ambiguous_or_no_name_and_id_attribute, &forms);
   FormElementsList::iterator iter;
   for (iter = forms.begin(); iter != forms.end(); ++iter) {
     // Attach autocomplete listener to enable selecting alternate logins.
     blink::WebInputElement username_element, password_element;
 
-    // Check whether the password form has a username input field.
+    base::string16 username_field_name, password_field_name;
+    password_field_name = FieldName(form_data.password_field,
+                                    ambiguous_or_no_name_and_id_attribute);
     bool form_contains_fillable_username_field =
         FillDataContainsFillableUsername(form_data);
-    if (form_contains_fillable_username_field) {
-      username_element =
-          (*iter)[form_data.username_field.name];
+    if (form_contains_fillable_username_field)
+      username_field_name = FieldName(form_data.username_field,
+                                      ambiguous_or_no_name_and_id_attribute);
+
+    // Check whether the password form has a username input field.
+    if (!username_field_name.empty()) {
+      username_element = (*iter)[username_field_name];
     }
 
     // No password field, bail out.
-    if (form_data.password_field.name.empty())
+    if (password_field_name.empty())
       break;
 
     // We might have already filled this form if there are two <form> elements
@@ -1223,7 +1283,7 @@ void PasswordAutofillAgent::OnFillPasswordForm(
 
     // Get pointer to password element. (We currently only support single
     // password forms).
-    password_element = (*iter)[form_data.password_field.name];
+    password_element = (*iter)[password_field_name];
 
     // If wait_for_username is true, we don't want to initially fill the form
     // until the user types in a valid username.