Field's server type mapping (using Autofill server response) for forms with checkable elements/pass…

components/autofill/browser/form_structure.cc

Index: components/autofill/browser/form_structure.cc
diff --git a/components/autofill/browser/form_structure.cc b/components/autofill/browser/form_structure.cc
index cec92b09f11eba2632a3f142afb072487e0b1e07..8ed74247f7936effe8228df8109a7de0cd803e55 100644
--- a/components/autofill/browser/form_structure.cc
+++ b/components/autofill/browser/form_structure.cc
@@ -308,8 +308,7 @@ FormStructure::FormStructure(const FormData& form,
     // Skip checkable and password elements when Autocheckout is not enabled,
     // else these fields will interfere with existing field signatures with
     // Autofill servers.

[Evan Stade, 2013/05/28 19:24:07]

I don't think the comment above each call to ShouldSkipField is necessary now
that you have one centralized place for docs (the declaration of this fn)

[Raman Kakilate, 2013/05/28 20:38:44]

Done.

-    if ((!field->is_checkable && field->form_control_type != "password") ||
-        IsAutocheckoutEnabled()) {
+    if (!ShouldSkipField(*field)) {
       // Add all supported form fields (including with empty names) to the
       // signature.  This is a requirement for Autofill servers.
       form_signature_field_names_.append("&");
@@ -559,7 +558,12 @@ void FormStructure::ParseQueryResponse(
     form->server_experiment_id_ = experiment_id;
 
     for (std::vector<AutofillField*>::iterator field = form->fields_.begin();
-         field != form->fields_.end(); ++field, ++current_info) {
+         field != form->fields_.end(); ++field) {
+      // Skip putting checkable and password fields in the request if
+      // Autocheckout is not enabled.
+      if (form->ShouldSkipField(**field))
+        continue;
+
       // In some cases *successful* response does not return all the fields.
       // Quit the update of the types then.
       if (current_info == field_infos.end())
@@ -579,6 +583,8 @@ void FormStructure::ParseQueryResponse(
       // Copy default value into the field if available.
       if (!current_info->default_value.empty())
         (*field)->set_default_value(current_info->default_value);
+
+      ++current_info;
     }
 
     form->UpdateAutofillCount();
@@ -657,6 +663,11 @@ bool FormStructure::IsAutocheckoutEnabled() const {
   return !autocheckout_url_prefix_.empty();
 }
 
+bool FormStructure::ShouldSkipField(const FormFieldData field) const {
+  return ((field.is_checkable || field.form_control_type == "password") &&

[Evan Stade, 2013/05/28 19:24:07]

nit: no need for outer parens

[Raman Kakilate, 2013/05/28 20:38:44]

Done.

+      !IsAutocheckoutEnabled());
+}
+
 size_t FormStructure::RequiredFillableFields() const {
   return IsAutocheckoutEnabled() ? 0 : kRequiredAutofillFields;
 }
@@ -1028,8 +1039,7 @@ bool FormStructure::EncodeFormRequest(
       case FormStructure::QUERY:
         // Skip putting checkable and password fields in the request if
         // Autocheckout is not enabled.
-        if ((field->is_checkable || field->form_control_type == "password") &&
-            !IsAutocheckoutEnabled())
+        if (ShouldSkipField(*field))
           continue;
         EncodeFieldForQuery(*field, encompassing_xml_element);
         break;