Consolidate Autofill possible type detection code, and enforce greater match precision.

* Only consider submitted field values to match locally stored data if the strings match exactly
  + This means we will send fewer false positives to the server, at the cost of some false negatives.  Most importantly, we should stop identifying ADDRESS_LINE_2 fields as ADDRESS_LINE_1.
  + There are a few excpetions for structured fields with canonicalized values, like countries (which are canonicalized to country codes)
* Eliminate FormGroup::GetMatchingTypes() -- all of this work is now done in a consistent way within AutofillManager
* Refactor FormGroup::GetNonEmptyTypes() to be shared code.

BUG=81867, 76992
TEST=unit_tests --gtest_filter=AutofillManagerTest.DeterminePossibleFieldTypesForUpload


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

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

1 files changed, 19 insertions, 66 deletions

diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 91c44f3..65b0e9a 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -166,8 +166,8 @@ void PersonalDataManager::OnWebDataServiceRequestDone(
 // PersonalDataManager,
 // views::ButtonListener implementations
 void PersonalDataManager::SetObserver(PersonalDataManager::Observer* observer) {
-  // TODO: RemoveObserver is for compatibility with old code, it should be
-  // nuked.
+  // TODO(dhollowa): RemoveObserver is for compatibility with old code, it
+  // should be nuked.
   observers_.RemoveObserver(observer);
   observers_.AddObserver(observer);
 }
@@ -244,17 +244,11 @@ bool PersonalDataManager::ImportFormData(
     types_seen.insert(field_type);
 
     if (group == AutofillType::CREDIT_CARD) {
-      // If the user has a password set, we have no way of setting credit
-      // card numbers.
       if (LowerCaseEqualsASCII(field->form_control_type, "month")) {
         DCHECK_EQ(CREDIT_CARD_EXP_MONTH, field_type);
         local_imported_credit_card->SetInfoForMonthInputType(value);
       } else {
-        if (field_type == CREDIT_CARD_NUMBER) {
-          // Clean up any imported credit card numbers.
-          value = CreditCard::StripSeparators(value);
-        }
-        local_imported_credit_card->SetInfo(field_type, value);
+        local_imported_credit_card->SetCanonicalizedInfo(field_type, value);
       }
       ++importable_credit_card_fields;
     } else {
@@ -263,7 +257,7 @@ bool PersonalDataManager::ImportFormData(
       // If the fields are not the phone fields in question both home.SetInfo()
       // and fax.SetInfo() are going to return false.
       if (!home.SetInfo(field_type, value) && !fax.SetInfo(field_type, value))
-        imported_profile->SetInfo(field_type, value);
+        imported_profile->SetCanonicalizedInfo(field_type, value);
 
       // Reject profiles with invalid country information.
       if (field_type == ADDRESS_HOME_COUNTRY &&
@@ -274,31 +268,25 @@ bool PersonalDataManager::ImportFormData(
     }
   }
 
-  // Build phone numbers if they are from parts.
-  if (imported_profile.get()) {
+  // Construct the phone and fax numbers.  Reject the profile if either number
+  // is invalid.
+  if (imported_profile.get() && !home.IsEmpty()) {
     string16 constructed_number;
-    if (!home.empty()) {
-      if (!home.ParseNumber(imported_profile->CountryCode(),
-                            &constructed_number)) {
-        imported_profile.reset();
-      } else {
-        imported_profile->SetInfo(PHONE_HOME_WHOLE_NUMBER, constructed_number);
-      }
-    }
-    if (!fax.empty()) {
-      if (!fax.ParseNumber(imported_profile->CountryCode(),
-                           &constructed_number)) {
-        imported_profile.reset();
-      } else {
-        imported_profile->SetInfo(PHONE_FAX_WHOLE_NUMBER, constructed_number);
-      }
+    if (!home.ParseNumber(imported_profile->CountryCode(),
+                          &constructed_number) ||
+        !imported_profile->SetCanonicalizedInfo(PHONE_HOME_WHOLE_NUMBER,
+                                                constructed_number)) {
+      imported_profile.reset();
     }
   }
-  // Normalize phone numbers.
-  if (imported_profile.get()) {
-    // Reject profile if even one of the phones is invalid.
-    if (!imported_profile->NormalizePhones())
+  if (imported_profile.get() && !fax.IsEmpty()) {
+    string16 constructed_number;
+    if (!fax.ParseNumber(imported_profile->CountryCode(),
+                         &constructed_number) ||
+        !imported_profile->SetCanonicalizedInfo(PHONE_FAX_WHOLE_NUMBER,
+                                                constructed_number)) {
       imported_profile.reset();
+    }
   }
 
   // Reject the profile if minimum address and validation requirements are not
@@ -486,41 +474,6 @@ CreditCard* PersonalDataManager::GetCreditCardByGUID(const std::string& guid) {
   return NULL;
 }
 
-void PersonalDataManager::GetMatchingTypes(const string16& text,
-                                           FieldTypeSet* matching_types) const {
-  string16 clean_info = StringToLowerASCII(CollapseWhitespace(text, false));
-  if (clean_info.empty()) {
-    matching_types->insert(EMPTY_TYPE);
-    return;
-  }
-
-  const std::vector<AutofillProfile*>& profiles = this->profiles();
-  for (std::vector<AutofillProfile*>::const_iterator iter = profiles.begin();
-       iter != profiles.end(); ++iter) {
-    const FormGroup* profile = *iter;
-    if (!profile) {
-      DLOG(ERROR) << "NULL information in profiles list";
-      continue;
-    }
-
-    profile->GetMatchingTypes(clean_info, matching_types);
-  }
-
-  for (ScopedVector<CreditCard>::const_iterator iter = credit_cards_.begin();
-       iter != credit_cards_.end(); ++iter) {
-    const FormGroup* credit_card = *iter;
-    if (!credit_card) {
-      DLOG(ERROR) << "NULL information in credit cards list";
-      continue;
-    }
-
-    credit_card->GetMatchingTypes(clean_info, matching_types);
-  }
-
-  if (matching_types->empty())
-    matching_types->insert(UNKNOWN_TYPE);
-}
-
 void PersonalDataManager::GetNonEmptyTypes(
     FieldTypeSet* non_empty_types) const {
   const std::vector<AutofillProfile*>& profiles = this->profiles();