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

components/autofill/core/browser/form_field.cc

Index: components/autofill/core/browser/form_field.cc
diff --git a/components/autofill/core/browser/form_field.cc b/components/autofill/core/browser/form_field.cc
index 191be8d62f4d80459c572f0179a930c0afde2e18..0b669b637d03bf56a4229f40f64994f66860efb9 100644
--- a/components/autofill/core/browser/form_field.cc
+++ b/components/autofill/core/browser/form_field.cc
@@ -40,6 +40,7 @@ bool ShouldBeIgnored(const AutofillField* field) {
 
 // static
 void FormField::ParseFormFields(const std::vector<AutofillField*>& fields,
+                                bool is_unowned_form,
                                 ServerFieldTypeMap* map) {
   // Set up a working copy of the fields to be processed.
   std::vector<AutofillField*> remaining_fields(fields.size());
@@ -50,20 +51,36 @@ void FormField::ParseFormFields(const std::vector<AutofillField*>& fields,
                      ShouldBeIgnored),
       remaining_fields.end());
 
+  ServerFieldTypeMap saved_map = *map;
+  size_t email_count = 0;
+  size_t count = 0;
+
   // Email pass.
-  ParseFormFieldsPass(EmailField::Parse, &remaining_fields, map);
+  email_count += ParseFormFieldsPass(EmailField::Parse, &remaining_fields, map);
+  count += email_count;
 
   // Phone pass.
-  ParseFormFieldsPass(PhoneField::Parse, &remaining_fields, map);
+  count += ParseFormFieldsPass(PhoneField::Parse, &remaining_fields, map);
 
   // Address pass.
-  ParseFormFieldsPass(AddressField::Parse, &remaining_fields, map);
+  count += ParseFormFieldsPass(AddressField::Parse, &remaining_fields, map);
 
   // Credit card pass.
-  ParseFormFieldsPass(CreditCardField::Parse, &remaining_fields, map);
+  count += ParseFormFieldsPass(CreditCardField::Parse, &remaining_fields, map);
 
   // Name pass.
-  ParseFormFieldsPass(NameField::Parse, &remaining_fields, map);
+  count += 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
+  size_t kThreshold = 3;
+  bool ignore_parsing = (count < kThreshold);

[Evan Stade, 2015/01/21 22:52:49]

nit: no parens

+  // For <form> tags, make an exception for email fields, which are commonly the
+  // only recognized field on account registration sites.
+  if (!is_unowned_form)

[Evan Stade, 2015/01/21 22:52:49]

&= is a confusing operator. How about:

bool should_parse = count >= kThreshold ||
    (is_in_form && email_count > 0);
if (!should_parse)
  *map = saved_map;

[Lei Zhang, 2015/01/22 08:07:37]

Done.

+    ignore_parsing &= (email_count == 0);
+  if (ignore_parsing)
+    *map = saved_map;
 }
 
 // static
@@ -130,17 +147,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;
   }
 
@@ -148,12 +165,14 @@ bool FormField::Match(const AutofillField* field,
 }
 
 // static
-void FormField::ParseFormFieldsPass(ParseFunction parse,
-                                    std::vector<AutofillField*>* fields,
-                                    ServerFieldTypeMap* map) {
+size_t FormField::ParseFormFieldsPass(ParseFunction parse,
+                                      std::vector<AutofillField*>* fields,
+                                      ServerFieldTypeMap* map) {
   // Store unmatched fields for further processing by the caller.
   std::vector<AutofillField*> remaining_fields;
 
+  ScopedVector<FormField> form_fields;
+  size_t field_count = 0;
   AutofillScanner scanner(*fields);
   while (!scanner.IsEnd()) {
     scoped_ptr<FormField> form_field(parse(&scanner));
@@ -163,12 +182,18 @@ void FormField::ParseFormFieldsPass(ParseFunction parse,
       continue;
     }
 
+    field_count += form_field->FieldCount();

[Evan Stade, 2015/01/21 22:52:49]

instead of adding this function, couldn't you more or less just look at the
difference in size |fields| has at the beginning and end of this function?

[Lei Zhang, 2015/01/22 08:07:37]

Isn't it possible for functions like AddressField::ParseAddressLines() to
consume fields without parsing them into anything usable?

[Evan Stade, 2015/01/22 17:17:05]

perhaps... but you could check the size of |map| after classifications?

[Lei Zhang, 2015/01/22 23:14:15]

Sounds better, done.

+    form_fields.push_back(form_field.release());
+  }
+
+  for (const auto& form_field : form_fields) {

[Evan Stade, 2015/01/22 17:17:05]

why is this a separate loop now?

also, I think auto is great for iterators, but less clear for simple types like
const FormField&

[Lei Zhang, 2015/01/22 23:14:15]

I was experimenting with different ways to count fields earlier. Reverted.
Sure.

     // Add entries into the map for each field type found in |form_field|.
     bool ok = form_field->ClassifyField(map);
     DCHECK(ok);
   }
 
   std::swap(*fields, remaining_fields);
+  return field_count;
 }
 
 bool FormField::MatchesFormControlType(const std::string& type,