AutoFill: Fix IsAutoFillable to only count fields that are heuristically

matched.  Move heuristic matching into the constructor of FormStructure and
update the tests accordingly.

BUG=41255
TEST=FormStructureTest.*

Review URL: http://codereview.chromium.org/1906001

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

5 files changed, 106 insertions, 201 deletions

diff --git a/chrome/browser/autofill/autofill_download_unittest.cc b/chrome/browser/autofill/autofill_download_unittest.cc
index fe09261a..3738213 100644
--- a/chrome/browser/autofill/autofill_download_unittest.cc
+++ b/chrome/browser/autofill/autofill_download_unittest.cc
@@ -106,6 +106,14 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
                                                ASCIIToUTF16("username"),
                                                string16(),
                                                ASCIIToUTF16("text")));
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("First Name"),
+                                               ASCIIToUTF16("firstname"),
+                                               string16(),
+                                               ASCIIToUTF16("text")));
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Last Name"),
+                                               ASCIIToUTF16("lastname"),
+                                               string16(),
+                                               ASCIIToUTF16("text")));
   form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("email"),
                                                ASCIIToUTF16("email"),
                                                string16(),
@@ -162,6 +170,8 @@ TEST(AutoFillDownloadTest, QueryAndUploadTest) {
   const char *responses[] = {
     "<autofillqueryresponse>"
       "<field autofilltype=\"0\" />"
+      "<field autofilltype=\"3\" />"
+      "<field autofilltype=\"5\" />"
       "<field autofilltype=\"9\" />"
       "<field autofilltype=\"0\" />"
       "<field autofilltype=\"30\" />"
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index bd53f2d..d2ddf6b 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -393,10 +393,6 @@ bool AutoFillManager::IsAutoFillEnabled() const {
 
 void AutoFillManager::DeterminePossibleFieldTypes(
     FormStructure* form_structure) {
-  // TODO(jhawkins): Update field text.
-
-  form_structure->GetHeuristicAutoFillTypes();
-
   for (size_t i = 0; i < form_structure->field_count(); i++) {
     const AutoFillField* field = form_structure->field(i);
     FieldTypeSet field_types;
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index 71d3c9c..460557e 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -69,12 +69,12 @@ FormStructure::FormStructure(const FormData& form)
     // store all fields in order to match the fields stored in the FormManager.
     // We don't append other field types to the form signature though in order
     // to match the form signature of the AutoFill servers.
-    if (IsAutoFillableField(field->form_control_type())) {
+    if (LowerCaseEqualsASCII(field->form_control_type(), kControlTypeText) ||
+        LowerCaseEqualsASCII(field->form_control_type(), kControlTypeSelect)) {
       // Add all supported form fields (including with empty names) to
       // signature.  This is a requirement for AutoFill servers.
       form_signature_field_names_.append("&");
       form_signature_field_names_.append(UTF16ToUTF8(field->name()));
-      ++autofill_count_;
     }
 
     // Generate a unique name for this field by appending a counter to the name.
@@ -84,6 +84,7 @@ FormStructure::FormStructure(const FormData& form)
 
   // Terminate the vector with a NULL item.
   fields_.push_back(NULL);
+  GetHeuristicAutoFillTypes();
 
   std::string method = UTF16ToUTF8(form.method);
   if (StringToLowerASCII(method) == kFormMethodPost) {
@@ -95,12 +96,6 @@ FormStructure::FormStructure(const FormData& form)
   }
 }
 
-bool FormStructure::IsAutoFillableField(const string16& field_type) const {
-  // We currently only handle text and select fields.
-  return LowerCaseEqualsASCII(field_type, kControlTypeText) ||
-         LowerCaseEqualsASCII(field_type, kControlTypeSelect);
-}
-
 bool FormStructure::EncodeUploadRequest(bool auto_fill_used,
                                         std::string* encoded_xml) const {
   bool auto_fillable = IsAutoFillable();
@@ -165,36 +160,6 @@ bool FormStructure::EncodeQueryRequest(const ScopedVector<FormStructure>& forms,
   return true;
 }
 
-void FormStructure::GetHeuristicAutoFillTypes() {
-  has_credit_card_field_ = false;
-  has_autofillable_field_ = false;
-
-  FieldTypeMap field_type_map;
-  GetHeuristicFieldInfo(&field_type_map);
-
-  for (size_t index = 0; index < field_count(); index++) {
-    AutoFillField* field = fields_[index];
-    // TODO(dhollowa): Defensive check for crash happening in the field.
-    // See http://crbug.com/42211
-    CHECK(field);
-    FieldTypeMap::iterator iter = field_type_map.find(field->unique_name());
-
-    AutoFillFieldType heuristic_auto_fill_type;
-    if (iter == field_type_map.end())
-      heuristic_auto_fill_type = UNKNOWN_TYPE;
-    else
-      heuristic_auto_fill_type = iter->second;
-
-    field->set_heuristic_type(heuristic_auto_fill_type);
-
-    AutoFillType autofill_type(field->type());
-    if (autofill_type.group() == AutoFillType::CREDIT_CARD)
-      has_credit_card_field_ = true;
-    if (autofill_type.field_type() != UNKNOWN_TYPE)
-      has_autofillable_field_ = true;
-  }
-}
-
 std::string FormStructure::FormSignature() const {
   std::string form_string = target_url_.scheme() +
                             "://" +
@@ -226,11 +191,8 @@ bool FormStructure::HasAutoFillableValues() const {
   for (std::vector<AutoFillField*>::const_iterator iter = begin();
        iter != end(); ++iter) {
     AutoFillField* field = *iter;
-    if (field &&
-        !field->IsEmpty() &&
-        IsAutoFillableField(field->form_control_type())) {
+    if (field && !field->IsEmpty() && field->IsFieldFillable())
       return true;
-    }
   }
   return false;
 }
@@ -291,6 +253,38 @@ bool FormStructure::operator!=(const FormData& form) const {
   return !operator==(form);
 }
 
+void FormStructure::GetHeuristicAutoFillTypes() {
+  has_credit_card_field_ = false;
+  has_autofillable_field_ = false;
+
+  FieldTypeMap field_type_map;
+  GetHeuristicFieldInfo(&field_type_map);
+
+  for (size_t index = 0; index < field_count(); index++) {
+    AutoFillField* field = fields_[index];
+    // TODO(dhollowa): Defensive check for crash happening in the field.
+    // See http://crbug.com/42211
+    CHECK(field);
+    FieldTypeMap::iterator iter = field_type_map.find(field->unique_name());
+
+    AutoFillFieldType heuristic_auto_fill_type;
+    if (iter == field_type_map.end()) {
+      heuristic_auto_fill_type = UNKNOWN_TYPE;
+    } else {
+      heuristic_auto_fill_type = iter->second;
+      ++autofill_count_;
+    }
+
+    field->set_heuristic_type(heuristic_auto_fill_type);
+
+    AutoFillType autofill_type(field->type());
+    if (autofill_type.group() == AutoFillType::CREDIT_CARD)
+      has_credit_card_field_ = true;
+    if (autofill_type.field_type() != UNKNOWN_TYPE)
+      has_autofillable_field_ = true;
+  }
+}
+
 void FormStructure::GetHeuristicFieldInfo(FieldTypeMap* field_type_map) {
   FormFieldSet fields(this);
 
diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h
index 98fa9a9..f429c3c 100644
--- a/chrome/browser/autofill/form_structure.h
+++ b/chrome/browser/autofill/form_structure.h
@@ -48,10 +48,6 @@ class FormStructure {
   static bool EncodeQueryRequest(const ScopedVector<FormStructure>& forms,
                                  std::string* encoded_xml);
 
-  // Runs several heuristics against the form fields to determine their possible
-  // types.
-  void GetHeuristicAutoFillTypes();
-
   // The unique signature for this form, composed of the target url domain,
   // the form name, and the form field names in a 64-bit hash.
   std::string FormSignature() const;
@@ -93,8 +89,9 @@ class FormStructure {
     UPLOAD,
   };
 
-  // Returns true if we can autofill the given field type.
-  bool IsAutoFillableField(const string16& field_type) const;
+  // Runs several heuristics against the form fields to determine their possible
+  // types.
+  void GetHeuristicAutoFillTypes();
 
   // Associates the field with the heuristic type for each of the field views.
   void GetHeuristicFieldInfo(FieldTypeMap* field_types_map);
diff --git a/chrome/browser/autofill/form_structure_unittest.cc b/chrome/browser/autofill/form_structure_unittest.cc
index 5324304..4835509 100644
--- a/chrome/browser/autofill/form_structure_unittest.cc
+++ b/chrome/browser/autofill/form_structure_unittest.cc
@@ -79,8 +79,8 @@ TEST(FormStructureTest, AutoFillCount) {
                                                ASCIIToUTF16("submit")));
   FormStructure form_structure(form);
 
-  // Only text and select fields are counted.
-  EXPECT_EQ(2U, form_structure.autofill_count());
+  // Only text and select fields that are heuristically matched are counted.
+  EXPECT_EQ(1U, form_structure.autofill_count());
 }
 
 TEST(FormStructureTest, ConvertToFormData) {
@@ -126,8 +126,8 @@ TEST(FormStructureTest, HasAutoFillableValues) {
   EXPECT_FALSE(form_structure->HasAutoFillableValues());
 
   // Empty text/select fields are also not used when saving auto fill data.
-  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("First Name"),
-                                               ASCIIToUTF16("firstname"),
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Email"),
+                                               ASCIIToUTF16("email"),
                                                string16(),
                                                ASCIIToUTF16("text")));
   form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("state"),
@@ -138,9 +138,36 @@ TEST(FormStructureTest, HasAutoFillableValues) {
   EXPECT_FALSE(form_structure->HasAutoFillableValues());
 
   // Non-empty fields can be saved in auto fill profile.
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("First Name"),
+                                               ASCIIToUTF16("firstname"),
+                                               ASCIIToUTF16("John"),
+                                               ASCIIToUTF16("text")));
   form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Last Name"),
                                                ASCIIToUTF16("lastname"),
-                                               ASCIIToUTF16("John"),
+                                               ASCIIToUTF16("Dear"),
+                                               ASCIIToUTF16("text")));
+  form_structure.reset(new FormStructure(form));
+  EXPECT_TRUE(form_structure->HasAutoFillableValues());
+
+  // The fields must be recognized heuristically by AutoFill in addition to
+  // being text or select and non-empty.
+  form.fields.clear();
+  form.fields.push_back(webkit_glue::FormField(string16(),
+                                               ASCIIToUTF16("Field1"),
+                                               string16(),
+                                               ASCIIToUTF16("text")));
+  form.fields.push_back(webkit_glue::FormField(string16(),
+                                               ASCIIToUTF16("Field2"),
+                                               ASCIIToUTF16("dummy value"),
+                                               ASCIIToUTF16("text")));
+  form_structure.reset(new FormStructure(form));
+  EXPECT_FALSE(form_structure->HasAutoFillableValues());
+
+  // Add a field that we match heuristically, verify that this form has
+  // auto-fillable values.
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Full Name"),
+                                               ASCIIToUTF16("fullname"),
+                                               ASCIIToUTF16("John Dear"),
                                                ASCIIToUTF16("text")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->HasAutoFillableValues());
@@ -167,7 +194,7 @@ TEST(FormStructureTest, IsAutoFillable) {
   form_structure.reset(new FormStructure(form));
   EXPECT_FALSE(form_structure->IsAutoFillable());
 
-  // We now have three text fields.
+  // We now have three text fields, but only two auto-fillable fields.
   form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("First Name"),
                                                ASCIIToUTF16("firstname"),
                                                string16(),
@@ -177,6 +204,14 @@ TEST(FormStructureTest, IsAutoFillable) {
                                                string16(),
                                                ASCIIToUTF16("text")));
   form_structure.reset(new FormStructure(form));
+  EXPECT_FALSE(form_structure->IsAutoFillable());
+
+  // We now have three auto-fillable fields.
+  form.fields.push_back(webkit_glue::FormField(ASCIIToUTF16("Email"),
+                                               ASCIIToUTF16("email"),
+                                               string16(),
+                                               ASCIIToUTF16("text")));
+  form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
 
   // The method must be 'post'.
@@ -241,28 +276,9 @@ TEST(FormStructureTest, HeuristicsContactInfo) {
   EXPECT_TRUE(form_structure->IsAutoFillable());
 
   // Expect the correct number of fields.
-  ASSERT_EQ(9UL, form_structure->field_count());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
   ASSERT_EQ(9U, form_structure->field_count());
+  ASSERT_EQ(8U, form_structure->autofill_count());
 
-  // Check that heuristics are no longer UNKNOWN_TYPE.
   // First name.
   EXPECT_EQ(NAME_FIRST, form_structure->field(0)->heuristic_type());
   // Last name.
@@ -341,27 +357,9 @@ TEST(FormStructureTest, HeuristicsSample8) {
                              ASCIIToUTF16("submit")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
   ASSERT_EQ(10U, form_structure->field_count());
+  ASSERT_EQ(9U, form_structure->autofill_count());
 
-  // Check that heuristics are no longer UNKNOWN_TYPE.
   // First name.
   EXPECT_EQ(NAME_FIRST, form_structure->field(0)->heuristic_type());
   // Last name.
@@ -429,27 +427,9 @@ TEST(FormStructureTest, HeuristicsSample6) {
                              ASCIIToUTF16("submit")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
   ASSERT_EQ(7U, form_structure->field_count());
+  ASSERT_EQ(5U, form_structure->autofill_count());
 
-  // Check that heuristics are no longer UNKNOWN_TYPE.
   // Email.
   EXPECT_EQ(EMAIL_ADDRESS, form_structure->field(0)->heuristic_type());
   // Full name.
@@ -512,30 +492,9 @@ TEST(FormStructureTest, HeuristicsLabelsOnly) {
                                                ASCIIToUTF16("submit")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
-
-  // Expect the correct number of fields.
-  ASSERT_EQ(9UL, form_structure->field_count());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
   ASSERT_EQ(9U, form_structure->field_count());
+  ASSERT_EQ(8U, form_structure->autofill_count());
 
-  // Check that heuristics are no longer UNKNOWN_TYPE.
   // First name.
   EXPECT_EQ(NAME_FIRST, form_structure->field(0)->heuristic_type());
   // Last name.
@@ -588,28 +547,8 @@ TEST(FormStructureTest, HeuristicsCreditCardInfo) {
                                                ASCIIToUTF16("submit")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
-
-  // Expect the correct number of fields.
-  ASSERT_EQ(6UL, form_structure->field_count());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
   ASSERT_EQ(6U, form_structure->field_count());
+  ASSERT_EQ(5U, form_structure->autofill_count());
 
   // Credit card name.
   EXPECT_EQ(CREDIT_CARD_NAME, form_structure->field(0)->heuristic_type());
@@ -664,28 +603,8 @@ TEST(FormStructureTest, HeuristicsCreditCardInfoWithUnknownCardField) {
                                                ASCIIToUTF16("submit")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
-
-  // Expect the correct number of fields.
-  ASSERT_EQ(7UL, form_structure->field_count());
-
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
-  ASSERT_EQ(7UL, form_structure->field_count());
+  ASSERT_EQ(7U, form_structure->field_count());
+  ASSERT_EQ(5U, form_structure->autofill_count());
 
   // Credit card name.
   EXPECT_EQ(CREDIT_CARD_NAME, form_structure->field(0)->heuristic_type());
@@ -725,35 +644,24 @@ TEST(FormStructureTest, ThreeAddressLines) {
                              ASCIIToUTF16("Address"),
                              string16(),
                              ASCIIToUTF16("text")));
+  form.fields.push_back(
+      webkit_glue::FormField(ASCIIToUTF16("City"),
+                             ASCIIToUTF16("city"),
+                             string16(),
+                             ASCIIToUTF16("text")));
   form_structure.reset(new FormStructure(form));
   EXPECT_TRUE(form_structure->IsAutoFillable());
+  ASSERT_EQ(4U, form_structure->field_count());
+  ASSERT_EQ(3U, form_structure->autofill_count());
 
-  // Check that heuristics are initialized as UNKNOWN_TYPE.
-  std::vector<AutoFillField*>::const_iterator iter;
-  size_t i;
-  for (iter = form_structure->begin(), i = 0;
-       iter != form_structure->end();
-       ++iter, ++i) {
-    // Expect last element to be NULL.
-    if (i == form_structure->field_count()) {
-      ASSERT_EQ(static_cast<AutoFillField*>(NULL), *iter);
-    } else {
-      ASSERT_NE(static_cast<AutoFillField*>(NULL), *iter);
-      EXPECT_EQ(UNKNOWN_TYPE, (*iter)->heuristic_type());
-    }
-  }
-
-  // Compute heuristic types.
-  form_structure->GetHeuristicAutoFillTypes();
-  ASSERT_EQ(3U, form_structure->field_count());
-
-  // Check that heuristics are no longer UNKNOWN_TYPE.
   // Address Line 1.
   EXPECT_EQ(ADDRESS_HOME_LINE1, form_structure->field(0)->heuristic_type());
   // Address Line 2.
   EXPECT_EQ(ADDRESS_HOME_LINE2, form_structure->field(1)->heuristic_type());
   // Address Line 3.
   EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(2)->heuristic_type());
+  // City.
+  EXPECT_EQ(ADDRESS_HOME_CITY, form_structure->field(3)->heuristic_type());
 }
 
 }  // namespace