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

components/autofill/content/renderer/password_autofill_agent.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 "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/i18n/case_conversion.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 37 matching lines @@
 // The size above which we stop triggering autocomplete.
 static const size_t kMaximumTextSizeForAutocomplete = 1000;
 
 // Experiment information
 const char kFillOnAccountSelectFieldTrialName[] = "FillOnAccountSelect";
 const char kFillOnAccountSelectFieldTrialEnabledWithHighlightGroup[] =
     "EnableWithHighlight";
 const char kFillOnAccountSelectFieldTrialEnabledWithNoHighlightGroup[] =
     "EnableWithNoHighlight";
 
+const char kUsernameField[] = "anonymous_username";
+const char kPasswordField[] = "anonymous_password";
+const char kInputTypePassword[] = "password";

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

nit: You can just inline this constant in the helper function below. The type
name will not change frequently, as it is part of the HTML norm.

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

Done.

+
 // Maps element names to the actual elements to simplify form filling.
 typedef std::map<base::string16, blink::WebInputElement> FormInputElementMap;
 
 // Use the shorter name when referencing SavePasswordProgressLogger::StringID
 // values to spare line breaks. The code provides enough context for that
 // already.
 typedef SavePasswordProgressLogger Logger;
 
 // Utility struct for form lookup and autofill. When we parse the DOM to look up
 // a form, in addition to action and origin URL's we have to compare all
 // necessary form elements. To avoid having to look these up again when we want
 // to fill the form, the FindFormElements function stores the pointers
 // in a FormElements* result, referenced to ensure they are safe to use.
 struct FormElements {
   blink::WebFormElement form_element;
   FormInputElementMap input_elements;
 };
 
 typedef std::vector<FormElements*> FormElementsList;
 
 bool FillDataContainsFillableUsername(const PasswordFormFillData& fill_data) {
   return !fill_data.username_field.name.empty() &&
          (!fill_data.additional_logins.empty() ||
           !fill_data.username_field.value.empty());
 }
 
+// Returns true if password form has username and password fields with either
+// same or no name and id attributes supplied.
+bool PasswordFormWithAmbiguousOrNoNameAndIdAttributes(
+    const PasswordFormFillData& fill_data) {
+  return fill_data.username_field.name == fill_data.password_field.name;
+}
+
+bool IsPasswordField(const FormFieldData& field) {
+  return (field.form_control_type == kInputTypePassword);
+}
+
+// Returns the |field|'s autofillable name. If no name or id attribute is
+// specified returns a dummy name.
+base::string16 FieldName(const FormFieldData& field,
+                         bool ambiguousOrNoNameAndIdAttribute) {

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

style: Please use lowercase_with_underscore, not camelCase. (Also below.)

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

Done.

+  return ambiguousOrNoNameAndIdAttribute
+             ? IsPasswordField(field) ? base::ASCIIToUTF16(kPasswordField)
+                                      : base::ASCIIToUTF16(kUsernameField)
+             : field.name;
+}
+
+// Helper function to check whether the |element| qualifies for autofilling.
+bool IsFillableInputElement(const blink::WebInputElement& element,
+                            const FormFieldData& field) {
+  return element.hasHTMLTagName("input") && element.isTextField() &&
+         element.isPasswordField() == IsPasswordField(field);
+}
+
+// Helper function to get the password form control elements. On supplied with

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

Please reformulate the second sentence. You probably mean something like: "If
|field_name| is ... then only returns the fields with that name." Also, it is
not clear, what is meant by "valid". Do you mean non-empty?

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

Done.

valid -> non-empty and non-ambiguous.

+// valid |field_name| gets the elements matching it.

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

You should comment on what the bool argument is used for (and possibly update
the whole comment if it is outdated).

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

Done.

+void GetFormControlElements(
+    blink::WebFormElement* form_element,
+    const base::string16& field_name,
+    bool ambiguousOrNoNameAndIdAttribute,
+    blink::WebVector<blink::WebFormControlElement>* elements) {
+  ambiguousOrNoNameAndIdAttribute

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

Please use if-else here, do not create a value just to throw it out and only use
its side-effects.

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

Done.

+      ? form_element->getFormControlElements(*elements)
+      : form_element->getNamedElements(
+            field_name, (blink::WebVector<blink::WebNode>&)*elements);
+}
+
 // Utility function to find the unique entry of the |form_element| for the
 // specified input |field|. On successful find, adds it to |result| and returns
 // |true|. Otherwise clears the references from each |HTMLInputElement| from
 // |result| and returns |false|.
 bool FindFormInputElement(blink::WebFormElement* form_element,
                           const FormFieldData& field,
+                          bool ambiguousOrNoNameAndIdAttribute,
                           FormElements* result) {
-  blink::WebVector<blink::WebNode> temp_elements;
-  form_element->getNamedElements(field.name, temp_elements);
+  blink::WebVector<blink::WebFormControlElement> input_elements;
+  GetFormControlElements(form_element, field.name,
+                         ambiguousOrNoNameAndIdAttribute, &input_elements);
 
   // Match the first input element, if any.
-  // |getNamedElements| may return non-input elements where the names match,
-  // so the results are filtered for input elements.
-  // If more than one match is made, then we have ambiguity (due to misuse
-  // of "name" attribute) so is it considered not found.
+  // |GetFormControlElements| may return non-input elements where the names
+  // match, so the results are filtered for input elements.
   bool found_input = false;
-  for (size_t i = 0; i < temp_elements.size(); ++i) {
-    if (temp_elements[i].to<blink::WebElement>().hasHTMLTagName("input")) {
+  base::string16 field_name = FieldName(field, ambiguousOrNoNameAndIdAttribute);
+  for (size_t i = 0; i < input_elements.size(); ++i) {
+    blink::WebInputElement input_element =
+        input_elements[i].to<blink::WebInputElement>();
+    if (IsFillableInputElement(input_element, field)) {
       // Check for a non-unique match.
       if (found_input) {
         found_input = false;
         break;
       }
 
-      // Only fill saved passwords into password fields and usernames into
-      // text fields.
-      blink::WebInputElement input_element =
-          temp_elements[i].to<blink::WebInputElement>();
-      if (input_element.isPasswordField() !=
-          (field.form_control_type == "password"))
-        continue;
-
       // This element matched, add it to our temporary result. It's possible
       // there are multiple matches, but for purposes of identifying the form
       // one suffices and if some function needs to deal with multiple
       // matching elements it can get at them through the FormElement*.
       // Note: This assignment adds a reference to the InputElement.
-      result->input_elements[field.name] = input_element;
+      result->input_elements[field_name] = input_element;
       found_input = true;
     }
   }
 
   // A required element was not found. This is not the right form.
   // Make sure no input elements from a partially matched form in this
   // iteration remain in the result set.
   // Note: clear will remove a reference from each InputElement.
   if (!found_input) {
     result->input_elements.clear();
@@ skipping 41 matching lines @@
   }
 
   return group_name !=
          kFillOnAccountSelectFieldTrialEnabledWithNoHighlightGroup;
 }
 
 // Helper to search the given form element for the specified input elements in
 // |data|, and add results to |result|.
 bool FindFormInputElements(blink::WebFormElement* form_element,
                            const PasswordFormFillData& data,
+                           bool ambiguousOrNoNameAndIdAttribute,
                            FormElements* result) {
-  return FindFormInputElement(form_element, data.password_field, result) &&
-         (!FillDataContainsFillableUsername(data) ||
-          FindFormInputElement(form_element, data.username_field, result));
+  return FindFormInputElement(form_element, data.password_field,
+                              ambiguousOrNoNameAndIdAttribute, result) &&
+         (!(ambiguousOrNoNameAndIdAttribute ||
+            FillDataContainsFillableUsername(data)) ||
+          FindFormInputElement(form_element, data.username_field,
+                               ambiguousOrNoNameAndIdAttribute, result));
 }
 
 // Helper to locate form elements identified by |data|.
 void FindFormElements(content::RenderFrame* render_frame,
                       const PasswordFormFillData& data,
+                      bool ambiguousOrNoNameAndIdAttribute,
                       FormElementsList* results) {
   DCHECK(results);
 
   blink::WebDocument doc = render_frame->GetWebFrame()->document();
   if (!doc.isHTMLDocument())
     return;
 
   if (data.origin != GetCanonicalOriginForDocument(doc))
     return;
 
   blink::WebVector<blink::WebFormElement> forms;
   doc.forms(forms);
 
   for (size_t i = 0; i < forms.size(); ++i) {
     blink::WebFormElement fe = forms[i];
 
     // Action URL must match.
     if (data.action != GetCanonicalActionForForm(fe))
       continue;
 
     scoped_ptr<FormElements> curr_elements(new FormElements);
-    if (!FindFormInputElements(&fe, data, curr_elements.get()))
+    if (!FindFormInputElements(&fe, data, ambiguousOrNoNameAndIdAttribute,
+                               curr_elements.get())) {
       continue;
+    }
 
     // We found the right element.
     // Note: this assignment adds a reference to |fe|.
     curr_elements->form_element = fe;
     results->push_back(curr_elements.release());
   }
 }
 
 bool IsElementEditable(const blink::WebInputElement& element) {
   return element.isEnabled() && !element.isReadOnly();
@@ skipping 236 matching lines @@
   while (cur_frame->parent()) {
     cur_frame = cur_frame->parent();
     if (!bottom_frame_origin.equals(cur_frame->securityOrigin().toString()))
       return false;
   }
 
   // If we can't modify the password, don't try to set the username
   if (!IsElementAutocompletable(password_element))
     return false;
 
-  bool form_contains_fillable_username_field =
-      FillDataContainsFillableUsername(fill_data);
+  bool ambiguousOrNoNameAndIdAttribute =
+      PasswordFormWithAmbiguousOrNoNameAndIdAttributes(fill_data);
+  base::string16 username_field_name =
+      FieldName(fill_data.username_field, ambiguousOrNoNameAndIdAttribute);
   // 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
   //   (b) The username element isn't prefilled
   //
   // If (a) is false, then just mark the username element as autofilled if the
   // user is not in the "no highlighting" group and return so the fill step is
   // skipped.
   //
   // If there is no autocompletable username field, and (a) is false, then the
   // username element cannot be autofilled, but the user should still be able to
   // select to fill the password element, so the password element must be marked
   // as autofilled and the fill step should also be skipped if the user is not
   // 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()) {
     if (!ShouldHighlightFields()) {
       return false;
     }
 
     if (form_has_fillable_username) {
       username_element.setAutofilled(true);
     } else if (username_element.isNull() ||
@@ skipping 657 matching lines @@
   }
 
   // This is a new navigation, so require a new user gesture before filling in
   // passwords.
   gatekeeper_.Reset();
 }
 
 void PasswordAutofillAgent::OnFillPasswordForm(
     int key,
     const PasswordFormFillData& form_data) {
-
+  bool ambiguousOrNoNameAndIdAttribute =
+      PasswordFormWithAmbiguousOrNoNameAndIdAttributes(form_data);
   FormElementsList forms;
   // We own the FormElements* in forms.
-  FindFormElements(render_frame(), form_data, &forms);
+  FindFormElements(render_frame(), form_data, ambiguousOrNoNameAndIdAttribute,
+                   &forms);
   FormElementsList::iterator iter;
   for (iter = forms.begin(); iter != forms.end(); ++iter) {
     scoped_ptr<FormElements> form_elements(*iter);
 
+    base::string16 username_field_name =
+        FieldName(form_data.username_field, ambiguousOrNoNameAndIdAttribute);
+    base::string16 password_field_name =
+        FieldName(form_data.password_field, ambiguousOrNoNameAndIdAttribute);
+
     // Attach autocomplete listener to enable selecting alternate logins.
     blink::WebInputElement username_element, password_element;
 
     // Check whether the password form has a username input field.
-    bool form_contains_fillable_username_field =
-        FillDataContainsFillableUsername(form_data);
-    if (form_contains_fillable_username_field) {
-      username_element =
-          form_elements->input_elements[form_data.username_field.name];
+    if (!username_field_name.empty()) {
+      username_element = form_elements->input_elements[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
     // with identical markup.
     if (login_to_password_info_.find(username_element) !=
         login_to_password_info_.end())
       continue;
 
     // Get pointer to password element. (We currently only support single
     // password forms).
-    password_element =
-        form_elements->input_elements[form_data.password_field.name];
+    password_element = form_elements->input_elements[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.
     if (!form_data.wait_for_username) {
       FillFormOnPasswordReceived(
           form_data,
           username_element,
           password_element,
           &nonscript_modified_values_,
           base::Bind(&PasswordValueGatekeeper::RegisterElement,
@@ skipping 185 matching lines @@
 void PasswordAutofillAgent::LegacyPasswordAutofillAgent::DidStopLoading() {
   agent_->DidStopLoading();
 }
 
 void PasswordAutofillAgent::LegacyPasswordAutofillAgent::
     DidStartProvisionalLoad(blink::WebLocalFrame* navigated_frame) {
   agent_->LegacyDidStartProvisionalLoad(navigated_frame);
 }
 
 }  // namespace autofill