Field's server type mapping (using Autofill server response) for forms with checkable elements/password fields is off

BUG=244290

Review URL: https://chromiumcodereview.appspot.com/16164003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202684 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 69 insertions, 10 deletions

diff --git a/components/autofill/browser/form_structure.cc b/components/autofill/browser/form_structure.cc
index cec92b0..02a581f 100644
--- a/components/autofill/browser/form_structure.cc
+++ b/components/autofill/browser/form_structure.cc
@@ -305,11 +305,7 @@ FormStructure::FormStructure(const FormData& form,
            form.fields.begin();
        field != form.fields.end(); field++) {
 
-    // Skip checkable and password elements when Autocheckout is not enabled,
-    // else these fields will interfere with existing field signatures with
-    // Autofill servers.
-    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 +555,10 @@ 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) {
+      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 +578,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 +658,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") &&
+      !IsAutocheckoutEnabled();
+}
+
 size_t FormStructure::RequiredFillableFields() const {
   return IsAutocheckoutEnabled() ? 0 : kRequiredAutofillFields;
 }
@@ -1026,10 +1032,7 @@ bool FormStructure::EncodeFormRequest(
         EncodeFieldForUpload(*field, encompassing_xml_element);
         break;
       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;
diff --git a/components/autofill/browser/form_structure.h b/components/autofill/browser/form_structure.h
index 3c4541f..9372ce5 100644
--- a/components/autofill/browser/form_structure.h
+++ b/components/autofill/browser/form_structure.h
@@ -201,6 +201,9 @@ class FormStructure {
 
   bool IsAutocheckoutEnabled() const;
 
+  // Returns true if field should be skipped when talking to Autofill server.
+  bool ShouldSkipField(const FormFieldData field) const;
+
   // Returns the minimal number of fillable fields required to start autofill.
   size_t RequiredFillableFields() const;
   size_t active_field_count() const;
diff --git a/components/autofill/browser/form_structure_unittest.cc b/components/autofill/browser/form_structure_unittest.cc
index 9fde80f..4c6af54 100644
--- a/components/autofill/browser/form_structure_unittest.cc
+++ b/components/autofill/browser/form_structure_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "components/autofill/browser/autocheckout_page_meta_data.h"
 #include "components/autofill/browser/autofill_metrics.h"
 #include "components/autofill/common/form_data.h"
 #include "components/autofill/common/form_field_data.h"
@@ -2387,4 +2388,56 @@ TEST(FormStructureTest, ToFormData) {
   EXPECT_NE(form, FormStructure(form, std::string()).ToFormData());
 }
 
+TEST(FormStructureTest, SkipFieldTest) {
+  FormData form;
+  form.name = ASCIIToUTF16("the-name");
+  form.method = ASCIIToUTF16("POST");
+  form.origin = GURL("http://cool.com");
+  form.action = form.origin.Resolve("/login");
+
+  FormFieldData field;
+  field.label = ASCIIToUTF16("username");
+  field.name = ASCIIToUTF16("username");
+  field.form_control_type = "text";
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("password");
+  field.name = ASCIIToUTF16("password");
+  field.form_control_type = "password";
+  form.fields.push_back(field);
+
+  field.label = base::string16();
+  field.name = ASCIIToUTF16("email");
+  field.form_control_type = "text";
+  form.fields.push_back(field);
+
+  ScopedVector<FormStructure> forms;
+  forms.push_back(new FormStructure(form, std::string()));
+  std::vector<std::string> encoded_signatures;
+  std::string encoded_xml;
+
+  const char * const kSignature = "18006745212084723782";
+  const char * const kResponse =
+      "<\?xml version=\"1.0\" encoding=\"UTF-8\"?><autofillquery "
+      "clientversion=\"6.1.1715.1442/en (GGLL)\" accepts=\"e\"><form "
+      "signature=\"18006745212084723782\"><field signature=\"239111655\"/>"
+      "<field signature=\"420638584\"/></form></autofillquery>";
+  ASSERT_TRUE(FormStructure::EncodeQueryRequest(forms.get(),
+                                                &encoded_signatures,
+                                                &encoded_xml));
+  ASSERT_EQ(1U, encoded_signatures.size());
+  EXPECT_EQ(kSignature, encoded_signatures[0]);
+  EXPECT_EQ(kResponse, encoded_xml);
+
+  AutocheckoutPageMetaData page_meta_data;
+  const char * const kServerResponse =
+      "<autofillqueryresponse><field autofilltype=\"3\" />"
+      "<field autofilltype=\"9\" /></autofillqueryresponse>";
+  FormStructure::ParseQueryResponse(kServerResponse, forms.get(),
+                                    &page_meta_data, TestAutofillMetrics());
+  ASSERT_EQ(NAME_FIRST, forms[0]->field(0)->server_type());
+  ASSERT_EQ(NO_SERVER_DATA, forms[0]->field(1)->server_type());
+  ASSERT_EQ(EMAIL_ADDRESS, forms[0]->field(2)->server_type());
+}
+
 }  // namespace autofill