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

components/autofill/content/browser/request_autocomplete_manager.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/browser/request_autocomplete_manager.h"
 
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/common/autofill_data_validation.h"
@@ skipping 17 matching lines @@
     case AutofillClient::AutocompleteResultErrorCancel:
       return blink::WebFormElement::AutocompleteResultErrorCancel;
     case AutofillClient::AutocompleteResultErrorInvalid:
       return blink::WebFormElement::AutocompleteResultErrorInvalid;
   }
 
   NOTREACHED();
   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(
     ContentAutofillDriver* autofill_driver)
     : autofill_driver_(autofill_driver), weak_ptr_factory_(this) {
   DCHECK(autofill_driver_);
 }
 
 RequestAutocompleteManager::~RequestAutocompleteManager() {}
 
@@ skipping 24 matching lines @@
     return;
 
   content::RenderViewHost* host =
       autofill_driver_->web_contents()->GetRenderViewHost();
   if (!host)
     return;
 
   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(
       host->GetRoutingID(),
       ToWebkitAutocompleteResult(result),
       debug_message,
       form_data));
 }
 
 void RequestAutocompleteManager::ShowRequestAutocompleteDialog(
     const FormData& form,
     const GURL& source_url,
     const AutofillClient::ResultCallback& callback) {
   AutofillClient* client = autofill_driver_->autofill_manager()->client();
   client->ShowRequestAutocompleteDialog(form, source_url, callback);
 }
 
 }  // namespace autofill