Autofill: Set requirements for number of recognized fields in an autofillable form

BUG=447332

Review URL: https://codereview.chromium.org/853523004

Cr-Commit-Position: refs/heads/master@{#313551}

chrome/test/data/autofill/heuristics/input/bug_447332.html

+<form>
+  <label for="test0">I intend to address this</label>
+  <textarea name="test0"></textarea>
+  <label for="test1">I can haz city</label>
+  <textarea name="test1"></textarea>
+  <label for="test2">State your intentions</label>
+  <textarea name="test2"></textarea>
+  <label for="test3">Test3</label>
+  <textarea name="test3"></textarea>
+</form>

chrome/test/data/autofill/heuristics/output/bug_447332.out

+UNKNOWN_TYPE | test0 | I intend to address this |  | test0_1-default
+UNKNOWN_TYPE | test1 | I can haz city |  | test0_1-default
+UNKNOWN_TYPE | test2 | State your intentions |  | test0_1-default
+UNKNOWN_TYPE | test3 | Test3 |  | test0_1-default

components/autofill/content/common/autofill_param_traits_macros.h

@@ -19,6 +19,7 @@ IPC_STRUCT_TRAITS_BEGIN(autofill::FormData)
   IPC_STRUCT_TRAITS_MEMBER(origin)
   IPC_STRUCT_TRAITS_MEMBER(action)
   IPC_STRUCT_TRAITS_MEMBER(user_submitted)
+  IPC_STRUCT_TRAITS_MEMBER(is_form_tag)
   IPC_STRUCT_TRAITS_MEMBER(fields)
 IPC_STRUCT_TRAITS_END()
 

components/autofill/content/renderer/form_autofill_util.cc

@@ -1157,6 +1157,7 @@ bool UnownedFormElementsAndFieldSetsToFormData(
     FormFieldData* field) {
   form->origin = origin;
   form->user_submitted = false;
+  form->is_form_tag = false;
 
   return FormOrFieldsetsToFormData(nullptr, element, fieldsets,
                                    control_elements, requirements, extract_mask,

components/autofill/core/browser/form_field.cc

@@ -40,7 +40,10 @@ bool ShouldBeIgnored(const AutofillField* field) {
 
 // static
 void FormField::ParseFormFields(const std::vector<AutofillField*>& fields,
+                                bool is_form_tag,
                                 ServerFieldTypeMap* map) {
+  DCHECK(map->empty());
+
   // Set up a working copy of the fields to be processed.
   std::vector<AutofillField*> remaining_fields(fields.size());
   std::copy(fields.begin(), fields.end(), remaining_fields.begin());
@@ -50,8 +53,11 @@ void FormField::ParseFormFields(const std::vector<AutofillField*>& fields,
                      ShouldBeIgnored),
       remaining_fields.end());
 
+  ServerFieldTypeMap saved_map = *map;
+
   // Email pass.
   ParseFormFieldsPass(EmailField::Parse, &remaining_fields, map);
+  size_t email_count = map->size();
 
   // Phone pass.
   ParseFormFieldsPass(PhoneField::Parse, &remaining_fields, map);
@@ -64,6 +70,16 @@ void FormField::ParseFormFields(const std::vector<AutofillField*>& fields,
 
   // Name pass.
   ParseFormFieldsPass(NameField::Parse, &remaining_fields, map);
+
+  // Do not autofill a form if there are less than 3 recognized fields.
+  // Otherwise it is very easy to have false positives. http://crbug.com/447332
+  // For <form> tags, make an exception for email fields, which are commonly the
+  // only recognized field on account registration sites.
+  size_t kThreshold = 3;
+  bool accept_parsing = (map->size() >= kThreshold ||
+                         (is_form_tag && email_count > 0));
+  if (!accept_parsing)
+    *map = saved_map;
 }
 
 // static
@@ -130,17 +146,17 @@ bool FormField::Match(const AutofillField* field,
                       const base::string16& pattern,
                       int match_type) {
   if ((match_type & FormField::MATCH_LABEL) &&
-      autofill::MatchesPattern(field->label, pattern)) {
+      MatchesPattern(field->label, pattern)) {
     return true;
   }
 
   if ((match_type & FormField::MATCH_NAME) &&
-      autofill::MatchesPattern(field->name, pattern)) {
+      MatchesPattern(field->name, pattern)) {
     return true;
   }
 
   if ((match_type & FormField::MATCH_VALUE) &&
-      autofill::MatchesPattern(field->value, pattern)) {
+      MatchesPattern(field->value, pattern)) {
     return true;
   }
 

components/autofill/core/browser/form_field.h

@@ -29,6 +29,7 @@ class FormField {
   // The association is stored into |map|.  Each field has a derived unique name
   // that is used as the key into the |map|.
   static void ParseFormFields(const std::vector<AutofillField*>& fields,
+                              bool is_form_tag,
                               ServerFieldTypeMap* map);
 
  protected:

components/autofill/core/browser/form_field_unittest.cc

@@ -135,15 +135,24 @@ TEST(FormFieldTest, ParseFormFields) {
   field_data.label = ASCIIToUTF16("Is PO Box");
   fields.push_back(new AutofillField(field_data, field_data.label));
 
-  // reset is_checkable to false.
+  // reset |is_checkable| to false.
   field_data.is_checkable = false;
+
   field_data.label = ASCIIToUTF16("Address line2");
   fields.push_back(new AutofillField(field_data, field_data.label));
 
   ServerFieldTypeMap field_type_map;
-  FormField::ParseFormFields(fields.get(), &field_type_map);
+  FormField::ParseFormFields(fields.get(), true, &field_type_map);
+  // Does not parse since there are only 2 recognized fields.
+  ASSERT_EQ(0U, field_type_map.size());
+
+  field_data.label = ASCIIToUTF16("City");
+  fields.push_back(new AutofillField(field_data, field_data.label));
+
   // Checkable element shouldn't interfere with inference of Address line2.
-  EXPECT_EQ(2U, field_type_map.size());
+  field_type_map.clear();
+  FormField::ParseFormFields(fields.get(), true, &field_type_map);
+  ASSERT_EQ(3U, field_type_map.size());
 
   EXPECT_EQ(ADDRESS_HOME_LINE1,
             field_type_map.find(ASCIIToUTF16("Address line1"))->second);

components/autofill/core/browser/form_structure.cc

@@ -353,7 +353,8 @@ FormStructure::FormStructure(const FormData& form)
       active_field_count_(0),
       upload_required_(USE_UPLOAD_RATES),
       has_author_specified_types_(false),
-      has_password_field_(false) {
+      has_password_field_(false),
+      is_form_tag_(form.is_form_tag) {
   // Copy the form fields.
   std::map<base::string16, size_t> unique_names;
   for (std::vector<FormFieldData>::const_iterator field =
@@ -397,7 +398,7 @@ void FormStructure::DetermineHeuristicTypes() {
 
   if (!has_author_specified_types_) {
     ServerFieldTypeMap field_type_map;
-    FormField::ParseFormFields(fields_.get(), &field_type_map);
+    FormField::ParseFormFields(fields_.get(), is_form_tag_, &field_type_map);
     for (size_t i = 0; i < field_count(); ++i) {
       AutofillField* field = fields_[i];
       ServerFieldTypeMap::iterator iter =

components/autofill/core/browser/form_structure.h

@@ -43,7 +43,7 @@ struct FormDataPredictions;
 // in the fields along with additional information needed by Autofill.
 class FormStructure {
  public:
-  FormStructure(const FormData& form);
+  explicit FormStructure(const FormData& form);
   virtual ~FormStructure();
 
   // Runs several heuristics against the form fields to determine their possible
@@ -254,6 +254,9 @@ class FormStructure {
   // True if the form contains at least one password field.
   bool has_password_field_;
 
+  // True if the form is a <form>.
+  bool is_form_tag_;
+
   DISALLOW_COPY_AND_ASSIGN(FormStructure);
 };
 

components/autofill/core/browser/form_structure_unittest.cc

@@ -94,6 +94,16 @@ TEST(FormStructureTest, AutofillCount) {
   field.form_control_type = "password";
   form.fields.push_back(field);
 
+  field.label = ASCIIToUTF16("email");
+  field.name = ASCIIToUTF16("email");
+  field.form_control_type = "text";
+  form.fields.push_back(field);
+
+  field.label = ASCIIToUTF16("city");
+  field.name = ASCIIToUTF16("city");
+  field.form_control_type = "text";
+  form.fields.push_back(field);
+
   field.label = ASCIIToUTF16("state");
   field.name = ASCIIToUTF16("state");
   field.form_control_type = "select-one";
@@ -107,7 +117,7 @@ TEST(FormStructureTest, AutofillCount) {
   // Only text and select fields that are heuristically matched are counted.
   form_structure.reset(new FormStructure(form));
   form_structure->DetermineHeuristicTypes();
-  EXPECT_EQ(1U, form_structure->autofill_count());
+  EXPECT_EQ(3U, form_structure->autofill_count());
 
   // Add a field with should_autocomplete=false. This should not be considered a
   // fillable field.
@@ -119,14 +129,14 @@ TEST(FormStructureTest, AutofillCount) {
 
   form_structure.reset(new FormStructure(form));
   form_structure->DetermineHeuristicTypes();
-  EXPECT_EQ(2U, form_structure->autofill_count());
+  EXPECT_EQ(4U, form_structure->autofill_count());
 
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kRespectAutocompleteOffForAutofill);
 
   form_structure.reset(new FormStructure(form));
   form_structure->DetermineHeuristicTypes();
-  EXPECT_EQ(1U, form_structure->autofill_count());
+  EXPECT_EQ(3U, form_structure->autofill_count());
 }
 
 TEST(FormStructureTest, SourceURL) {
@@ -2257,7 +2267,6 @@ TEST(FormStructureTest, CheckFormSignature) {
       std::string("https://login.facebook.com&login_form&email&first&"
                   "random1234&random&1random&random")),
       form_structure->FormSignature());
-
 }
 
 TEST(FormStructureTest, ToFormData) {
@@ -2420,6 +2429,8 @@ TEST(FormStructureTest, ParseQueryResponse) {
 
   FormStructure::ParseQueryResponse(response, forms.get());
 
+  ASSERT_GE(forms[0]->field_count(), 2U);
+  ASSERT_GE(forms[1]->field_count(), 2U);
   EXPECT_EQ(7, forms[0]->field(0)->server_type());
   EXPECT_EQ(30, forms[0]->field(1)->server_type());
   EXPECT_EQ(9, forms[1]->field(0)->server_type());
@@ -2455,6 +2466,7 @@ TEST(FormStructureTest, ParseQueryResponseAuthorDefinedTypes) {
 
   FormStructure::ParseQueryResponse(response, forms.get());
 
+  ASSERT_GE(forms[0]->field_count(), 2U);
   EXPECT_EQ(NO_SERVER_DATA, forms[0]->field(0)->server_type());
   EXPECT_EQ(76, forms[0]->field(1)->server_type());
 }

components/autofill/core/common/form_data.cc

@@ -13,7 +13,7 @@ namespace autofill {
 
 namespace {
 
-const int kPickleVersion = 2;
+const int kPickleVersion = 3;
 
 bool ReadGURL(PickleIterator* iter, GURL* url) {
   std::string spec;
@@ -56,7 +56,8 @@ void LogDeserializationError(int version) {
 }  // namespace
 
 FormData::FormData()
-    : user_submitted(false) {
+    : user_submitted(false),
+      is_form_tag(true) {
 }
 
 FormData::FormData(const FormData& data)
@@ -64,6 +65,7 @@ FormData::FormData(const FormData& data)
       origin(data.origin),
       action(data.action),
       user_submitted(data.user_submitted),
+      is_form_tag(data.is_form_tag),
       fields(data.fields) {
 }
 
@@ -75,6 +77,7 @@ bool FormData::SameFormAs(const FormData& form) const {
       origin != form.origin ||
       action != form.action ||
       user_submitted != form.user_submitted ||
+      is_form_tag != form.is_form_tag ||
       fields.size() != form.fields.size())
     return false;
   for (size_t i = 0; i < fields.size(); ++i) {
@@ -93,6 +96,8 @@ bool FormData::operator<(const FormData& form) const {
     return action < form.action;
   if (user_submitted != form.user_submitted)
     return user_submitted < form.user_submitted;
+  if (is_form_tag != form.is_form_tag)
+    return is_form_tag < form.is_form_tag;
   return fields < form.fields;
 }
 
@@ -101,6 +106,7 @@ std::ostream& operator<<(std::ostream& os, const FormData& form) {
      << form.origin << " "
      << form.action << " "
      << form.user_submitted << " "
+     << form.is_form_tag << " "
      << "Fields:";
   for (size_t i = 0; i < form.fields.size(); ++i) {
     os << form.fields[i] << ",";
@@ -115,6 +121,7 @@ void SerializeFormData(const FormData& form_data, Pickle* pickle) {
   pickle->WriteString(form_data.action.spec());
   pickle->WriteBool(form_data.user_submitted);
   SerializeFormFieldDataVector(form_data.fields, pickle);
+  pickle->WriteBool(form_data.is_form_tag);
 }
 
 bool DeserializeFormData(PickleIterator* iter, FormData* form_data) {
@@ -124,35 +131,41 @@ bool DeserializeFormData(PickleIterator* iter, FormData* form_data) {
     return false;
   }
 
-  switch (version) {
-    case 1: {
-      base::string16 method;
-      if (!iter->ReadString16(&form_data->name) ||
-          !iter->ReadString16(&method) ||
-          !ReadGURL(iter, &form_data->origin) ||
-          !ReadGURL(iter, &form_data->action) ||
-          !iter->ReadBool(&form_data->user_submitted) ||
-          !DeserializeFormFieldDataVector(iter, &form_data->fields)) {
-        LogDeserializationError(version);
-        return false;
-      }
-      break;
+  if (version < 1 || version > kPickleVersion) {
+    DVLOG(1) << "Unknown FormData pickle version " << version;
+    return false;
+  }
+
+  if (!iter->ReadString16(&form_data->name)) {
+    LogDeserializationError(version);
+    return false;
+  }
+
+  if (version == 1) {
+    base::string16 method;
+    if (!iter->ReadString16(&method)) {
+      LogDeserializationError(version);
+      return false;
     }
-    case 2:
-      if (!iter->ReadString16(&form_data->name) ||
-          !ReadGURL(iter, &form_data->origin) ||
-          !ReadGURL(iter, &form_data->action) ||
-          !iter->ReadBool(&form_data->user_submitted) ||
-          !DeserializeFormFieldDataVector(iter, &form_data->fields)) {
-        LogDeserializationError(version);
-        return false;
-      }
-      break;
-    default: {
-      DVLOG(1) << "Unknown FormData pickle version " << version;
+  }
+
+  if (!ReadGURL(iter, &form_data->origin) ||
+      !ReadGURL(iter, &form_data->action) ||
+      !iter->ReadBool(&form_data->user_submitted) ||
+      !DeserializeFormFieldDataVector(iter, &form_data->fields)) {
+    LogDeserializationError(version);
+    return false;
+  }
+
+  if (version == 3) {
+    if (!iter->ReadBool(&form_data->is_form_tag)) {
+      LogDeserializationError(version);
       return false;
     }
+  } else {
+    form_data->is_form_tag = true;
   }
+
   return true;
 }
 

components/autofill/core/common/form_data.h

@@ -34,6 +34,8 @@ struct FormData {
   GURL action;
   // true if this form was submitted by a user gesture and not javascript.
   bool user_submitted;
+  // true if this form is a form tag.
+  bool is_form_tag;
   // A vector of all the input fields in the form.
   std::vector<FormFieldData> fields;
 };

components/autofill/core/common/form_data_unittest.cc

@@ -9,13 +9,15 @@
 #include "components/autofill/core/common/form_field_data.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace autofill {
+
 namespace {
 
 // This function serializes the form data into the pickle in version one format.
 // It should always be possible to deserialize it using DeserializeFormData(),
 // even when version changes. See kPickleVersion in form_data.cc.
-void SerializeInVersion1Format(const autofill::FormData& form_data,
-                               Pickle* pickle) {
+void SerializeInVersion1Format(const FormData& form_data, Pickle* pickle) {
+  DCHECK_EQ(true, form_data.is_form_tag);
   pickle->WriteInt(1);
   pickle->WriteString16(form_data.name);
   base::string16 method(base::ASCIIToUTF16("POST"));
@@ -29,10 +31,9 @@ void SerializeInVersion1Format(const autofill::FormData& form_data,
   }
 }
 
-// This function serializes the form data into the pickle in incorrect format
-// (no version number).
-void SerializeIncorrectFormat(const autofill::FormData& form_data,
-                               Pickle* pickle) {
+void SerializeInVersion2Format(const FormData& form_data, Pickle* pickle) {
+  DCHECK_EQ(true, form_data.is_form_tag);
+  pickle->WriteInt(2);
   pickle->WriteString16(form_data.name);
   pickle->WriteString(form_data.origin.spec());
   pickle->WriteString(form_data.action.spec());
@@ -43,16 +44,24 @@ void SerializeIncorrectFormat(const autofill::FormData& form_data,
   }
 }
 
+// This function serializes the form data into the pickle in incorrect format
+// (no version number).
+void SerializeIncorrectFormat(const FormData& form_data, Pickle* pickle) {
+  pickle->WriteString16(form_data.name);
+  pickle->WriteString(form_data.origin.spec());
+  pickle->WriteString(form_data.action.spec());
+  pickle->WriteBool(form_data.user_submitted);
+  pickle->WriteInt(static_cast<int>(form_data.fields.size()));
+  for (size_t i = 0; i < form_data.fields.size(); ++i) {
+    SerializeFormFieldData(form_data.fields[i], pickle);
+  }
 }
 
-namespace autofill {
-
-TEST(FormDataTest, SerializeAndDeserialize) {
-  FormData data;
-  data.name = base::ASCIIToUTF16("name");
-  data.origin = GURL("origin");
-  data.action = GURL("action");
-  data.user_submitted = true;
+void FillInDummyFormData(FormData* data) {
+  data->name = base::ASCIIToUTF16("name");
+  data->origin = GURL("origin");
+  data->action = GURL("action");
+  data->user_submitted = true;
 
   FormFieldData field_data;
   field_data.label = base::ASCIIToUTF16("label");
@@ -72,13 +81,21 @@ TEST(FormDataTest, SerializeAndDeserialize) {
   field_data.option_contents.push_back(base::ASCIIToUTF16("First"));
   field_data.option_contents.push_back(base::ASCIIToUTF16("Second"));
 
-  data.fields.push_back(field_data);
+  data->fields.push_back(field_data);
 
   // Change a few fields.
   field_data.max_length = 150;
   field_data.option_values.push_back(base::ASCIIToUTF16("Third"));
   field_data.option_contents.push_back(base::ASCIIToUTF16("Third"));
-  data.fields.push_back(field_data);
+  data->fields.push_back(field_data);
+}
+
+}  // namespace
+
+TEST(FormDataTest, SerializeAndDeserialize) {
+  FormData data;
+  FillInDummyFormData(&data);
+  data.is_form_tag = false;
 
   Pickle pickle;
   SerializeFormData(data, &pickle);
@@ -92,36 +109,7 @@ TEST(FormDataTest, SerializeAndDeserialize) {
 
 TEST(FormDataTest, Serialize_v1_Deserialize_vCurrent) {
   FormData data;
-  data.name = base::ASCIIToUTF16("name");
-  data.origin = GURL("origin");
-  data.action = GURL("action");
-  data.user_submitted = true;
-
-  FormFieldData field_data;
-  field_data.label = base::ASCIIToUTF16("label");
-  field_data.name = base::ASCIIToUTF16("name");
-  field_data.value = base::ASCIIToUTF16("value");
-  field_data.form_control_type = "password";
-  field_data.autocomplete_attribute = "off";
-  field_data.max_length = 200;
-  field_data.is_autofilled = true;
-  field_data.is_checked = true;
-  field_data.is_checkable = true;
-  field_data.is_focusable = true;
-  field_data.should_autocomplete = false;
-  field_data.text_direction = base::i18n::RIGHT_TO_LEFT;
-  field_data.option_values.push_back(base::ASCIIToUTF16("First"));
-  field_data.option_values.push_back(base::ASCIIToUTF16("Second"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("First"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("Second"));
-
-  data.fields.push_back(field_data);
-
-  // Change a few fields.
-  field_data.max_length = 150;
-  field_data.option_values.push_back(base::ASCIIToUTF16("Third"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("Third"));
-  data.fields.push_back(field_data);
+  FillInDummyFormData(&data);
 
   Pickle pickle;
   SerializeInVersion1Format(data, &pickle);
@@ -133,38 +121,23 @@ TEST(FormDataTest, Serialize_v1_Deserialize_vCurrent) {
   EXPECT_TRUE(actual.SameFormAs(data));
 }
 
-TEST(FormDataTest, SerializeIncorrectFormatAndDeserialize) {
+TEST(FormDataTest, Serialize_v2_Deserialize_vCurrent) {
   FormData data;
-  data.name = base::ASCIIToUTF16("name");
-  data.origin = GURL("origin");
-  data.action = GURL("action");
-  data.user_submitted = true;
+  FillInDummyFormData(&data);
 
-  FormFieldData field_data;
-  field_data.label = base::ASCIIToUTF16("label");
-  field_data.name = base::ASCIIToUTF16("name");
-  field_data.value = base::ASCIIToUTF16("value");
-  field_data.form_control_type = "password";
-  field_data.autocomplete_attribute = "off";
-  field_data.max_length = 200;
-  field_data.is_autofilled = true;
-  field_data.is_checked = true;
-  field_data.is_checkable = true;
-  field_data.is_focusable = true;
-  field_data.should_autocomplete = false;
-  field_data.text_direction = base::i18n::RIGHT_TO_LEFT;
-  field_data.option_values.push_back(base::ASCIIToUTF16("First"));
-  field_data.option_values.push_back(base::ASCIIToUTF16("Second"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("First"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("Second"));
+  Pickle pickle;
+  SerializeInVersion2Format(data, &pickle);
 
-  data.fields.push_back(field_data);
+  PickleIterator iter(pickle);
+  FormData actual;
+  EXPECT_TRUE(DeserializeFormData(&iter, &actual));
 
-  // Change a few fields.
-  field_data.max_length = 150;
-  field_data.option_values.push_back(base::ASCIIToUTF16("Third"));
-  field_data.option_contents.push_back(base::ASCIIToUTF16("Third"));
-  data.fields.push_back(field_data);
+  EXPECT_TRUE(actual.SameFormAs(data));
+}
+
+TEST(FormDataTest, SerializeIncorrectFormatAndDeserialize) {
+  FormData data;
+  FillInDummyFormData(&data);
 
   Pickle pickle;
   SerializeIncorrectFormat(data, &pickle);