Autofill should reject aggregated profiles where email address is found in non-email fields

chrome/browser/autofill/personal_data_manager.cc

Index: chrome/browser/autofill/personal_data_manager.cc
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 13168791a9cf8e792cc2455289c710c6d693cf04..31350449f7baecf470b9c4c555b54dca5c692607 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -102,6 +102,29 @@ bool IsMinimumAddress(const AutofillProfile& profile) {
          !profile.GetInfo(ADDRESS_HOME_ZIP).empty();
 }
 
+// Return true if the |field_type| and |value| are valid within the context
+// of importing a form.
+bool IsValidFieldTypeAndValue(const std::set<AutofillFieldType>& types_seen,
+                              AutofillFieldType field_type,
+                              const string16& value) {
+  // Abandon the import if two fields of the same type are encountered.
+  // This indicates ambiguous data or miscategorization of types.
+  // Make an exception for PHONE_HOME_NUMBER however as both prefix and
+  // suffix are stored against this type.
+  if (types_seen.count(field_type) &&
+      field_type != PHONE_HOME_NUMBER &&
+      field_type != PHONE_FAX_NUMBER) {
+    return false;
+  }
+
+  // Abandon the import if an email address value shows up in a field that is
+  // not an email address.
+  if (field_type != EMAIL_ADDRESS && IsValidEmail(value))
+    return false;

[Ilya Sherman, 2011/05/12 21:40:03]

It's a little confusing to me that this check is here, but the complementary
check is in IsValidLearnableProfile().  If you're not opposed, let's have them
live side by side -- either here or there is fine by me.

[dhollowa, 2011/05/12 23:24:12]

Done.

+
+  return true;
+}
+
 }  // namespace
 
 PersonalDataManager::~PersonalDataManager() {
@@ -214,36 +237,30 @@ bool PersonalDataManager::ImportFormData(
     AutofillFieldType field_type = field->type();
     FieldTypeGroup group(AutofillType(field_type).group());
 
-    // Abandon the import if two fields of the same type are encountered.
-    // This indicates ambiguous data or miscategorization of types.
-    // Make an exception for PHONE_HOME_NUMBER however as both prefix and
-    // suffix are stored against this type.
-    if (types_seen.count(field_type) &&
-        field_type != PHONE_HOME_NUMBER  &&
-        field_type != PHONE_FAX_NUMBER) {
+    // If the |field_type| and |value| don't pass basic validity checks then
+    // abandon the import.
+    if (!IsValidFieldTypeAndValue(types_seen, field_type, value)) {
       imported_profile.reset();
       local_imported_credit_card.reset();
       break;
-    } else {
-      types_seen.insert(field_type);
     }
 
+    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 (!HasPassword()) {
-        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);
+      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);
         }
-        ++importable_credit_card_fields;
+        local_imported_credit_card->SetInfo(field_type, value);
       }
+      ++importable_credit_card_fields;
     } else {
       // In the case of a phone number, if the whole phone number was entered
       // into a single field, then parse it and set the sub components.
@@ -536,10 +553,6 @@ void PersonalDataManager::GetNonEmptyTypes(
   }
 }
 
-bool PersonalDataManager::HasPassword() {
-  return !password_hash_.empty();
-}
-
 bool PersonalDataManager::IsDataLoaded() const {
   return is_data_loaded_;
 }