[Password manager] Relplace the FormFieldData vector from autofill::FormData with named fields…

components/autofill/content/browser/request_autocomplete_manager.cc

Index: components/autofill/content/browser/request_autocomplete_manager.cc
diff --git a/components/autofill/content/browser/request_autocomplete_manager.cc b/components/autofill/content/browser/request_autocomplete_manager.cc
index 89c08bf7823ff9dbc28dcca21e1f39e509bfe101..bcfd0ada4700bf757979385dad173c63220ff8be 100644
--- a/components/autofill/content/browser/request_autocomplete_manager.cc
+++ b/components/autofill/content/browser/request_autocomplete_manager.cc
@@ -35,6 +35,27 @@ blink::WebFormElement::AutocompleteResult ToWebkitAutocompleteResult(
   return blink::WebFormElement::AutocompleteResultErrorDisabled;
 }
 
+// Helper function to check whether form having input |field| or not.

[vabr (Chromium), 2014/10/13 12:30:52]

nit: form -> a form, having -> has
Having said that, the comment is confusing (which form does it talk about?). If
you address the comment about renaming the function (below), I'd suggest just
dropping this comment entirely.

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

+bool HasInputField(const FormFieldData& field) {

[vabr (Chromium), 2014/10/13 12:30:52]

The name does not match what is computed.
What about: HasName?

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

+  return !field.name.empty();
+}
+
+// Helper function to set or reset the |is_autofilled| property.

[vabr (Chromium), 2014/10/13 12:30:52]

|is_autofilled| property... of what?
Please specify.

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

+void SetFormFieldsAutocompleted(FormData& form_data, bool is_autofilled) {

[vabr (Chromium), 2014/10/13 12:30:52]

Not non-const references. Use FormData* instead of FormData&.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Reference_Arg...

[vabr (Chromium), 2014/10/13 12:30:52]

So is it Autocompleted or autofilled? :)
Please be consistent.

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

+  if (HasInputField(form_data.username))
+    form_data.username.is_autofilled = is_autofilled;
+
+  if (HasInputField(form_data.password))
+    form_data.password.is_autofilled = is_autofilled;
+
+  // TODO (pritam.nikam): Make sure we remove |fields|, and add form fields
+  // for all possible autofillable input fields.
+  for (size_t i = 0; i < form_data.fields.size(); ++i) {
+    if (!form_data.fields[i].value.empty())

[vabr (Chromium), 2014/10/13 12:30:52]

It looks suspicious, that above the FormFieldData::name is checked against
emptiness, and here the FormFieldData::value is.

[Pritam Nikam, 2014/10/16 12:55:12]

Done.

+      form_data.fields[i].is_autofilled = is_autofilled;
+  }
+}

[Ilya Sherman, 2014/10/13 23:49:32]

Are the username and password fields ever used with requestAutocomplete?  I
don't believe that they are.

[Pritam Nikam, 2014/10/16 12:55:13]

Yes, I didn't see this is getting called.

+
 }  // namespace
 
 RequestAutocompleteManager::RequestAutocompleteManager(
@@ -79,10 +100,7 @@ void RequestAutocompleteManager::ReturnAutocompleteResult(
   FormData form_data;
   if (form_structure) {
     form_data = form_structure->ToFormData();
-    for (size_t i = 0; i < form_data.fields.size(); ++i) {
-      if(!form_data.fields[i].value.empty())
-        form_data.fields[i].is_autofilled = true;
-    }
+    SetFormFieldsAutocompleted(form_data, true);
   }
 
   host->Send(new AutofillMsg_RequestAutocompleteResult(