Autofill database migration to clean up bogus profiles.

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 287ce9484f4b200d272bfa591b9b67acf57ff668..a09381459ca780876a025451e288fa4ea67d9d42 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -11,12 +11,15 @@
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill-inl.h"
+#include "chrome/browser/autofill/autofill_country.h"
 #include "chrome/browser/autofill/autofill_field.h"
 #include "chrome/browser/autofill/autofill_metrics.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/browser/autofill/phone_number.h"
+#include "chrome/browser/autofill/select_control_handler.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/browser_thread.h"
@@ -82,6 +85,14 @@ bool IsValidEmail(const string16& value) {
   return re.match(WebKit::WebString(StringToLowerASCII(value))) != -1;
 }
 
+bool IsValidZip(const string16& value) {

[Ilya Sherman, 2011/03/16 04:58:57]

nit: Perhaps "IsValidUSZip()"?

[dhollowa, 2011/03/16 16:13:35]

I added comment, but I prefer as is.

+  // Basic US zip code matching.
+  const string16 kZipPattern = ASCIIToUTF16("^\\d{5}(-\\d{4})?$");
+  WebKit::WebRegularExpression re(WebKit::WebString(kZipPattern),
+                                  WebKit::WebTextCaseInsensitive);
+  return re.match(WebKit::WebString(StringToLowerASCII(value))) != -1;
+}
+
 // Returns true if minimum requirements for import of a given |profile| have
 // been met.  An address submitted via a form must have at least these fields
 // filled.  No verification of validity of the contents is preformed.  This is
@@ -253,11 +264,14 @@ bool PersonalDataManager::ImportFormData(
           }
         }
 
-        if (field_type.field_type() == EMAIL_ADDRESS && !IsValidEmail(value))
-          continue;
+        // Reject profiles with invalid country information.
+        if (field_type.field_type() == ADDRESS_HOME_COUNTRY &&
+            !value.empty() && !AutofillCountry::IsValidCountry(value)) {
+          imported_profile.reset();
+          break;
+        }

[Ilya Sherman, 2011/03/16 04:58:57]

Checking whether a string is a country name is a little expensive, so we should
move this below the call to SetInfo(), as

// Reject profiles with invalid country information.
if (field_type.field_type() == ADDRESS_HOME_COUNTRY &&
    !value.empty()  && imported_profile->CountryCode().empty())
  ...
}

[dhollowa, 2011/03/16 16:13:35]

Done.

 
-        imported_profile->SetInfo(AutofillType(field_type.field_type()),
-                                   value);
+        imported_profile->SetInfo(AutofillType(field_type.field_type()), value);
         ++importable_fields;
       }
     }
@@ -270,8 +284,11 @@ bool PersonalDataManager::ImportFormData(
   if (importable_credit_card_fields < kMinCreditCardImportSize)
     local_imported_credit_card.reset();
 
-  if (imported_profile.get() && !IsMinimumAddress(*imported_profile.get()))
+  // Reject if minimum address and validation requirements are not met.
+  if (imported_profile.get() &&
+      !IsSuitableMergeProfile(*imported_profile.get())) {
     imported_profile.reset();
+  }
 
   if (local_imported_credit_card.get() &&
       !CreditCard::IsCreditCardNumber(local_imported_credit_card->GetFieldText(
@@ -448,6 +465,7 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) {
 
   wds->UpdateAutofillProfile(profile);
   FOR_EACH_OBSERVER(Observer, observers_, OnPersonalDataChanged());
+  EmptyMigrationTrash();
 }
 
 void PersonalDataManager::RemoveProfile(const std::string& guid) {
@@ -629,6 +647,88 @@ void PersonalDataManager::Init(Profile* profile) {
   LoadCreditCards();
 }
 
+// static
+bool PersonalDataManager::IsSuitableMergeProfile(
+    const AutofillProfile& profile) {
+  if (!IsMinimumAddress(profile))
+    return false;
+
+  string16 email = profile.GetFieldText(AutofillType(EMAIL_ADDRESS));
+  if (!email.empty() && !IsValidEmail(email))
+    return false;
+
+  // Reject profiles with invalid US state information.
+  string16 state = profile.GetFieldText(AutofillType(ADDRESS_HOME_STATE));
+  if (profile.CountryCode() == "US" &&
+      !state.empty() && !autofill::IsValidState(state)) {
+    return false;
+  }
+
+  // Reject profiles with invalid US zip information.
+  string16 zip = profile.GetFieldText(AutofillType(ADDRESS_HOME_ZIP));
+  if (profile.CountryCode() == "US" && !zip.empty() && !IsValidZip(zip))
+    return false;
+
+  return true;
+}
+
+// static
+bool PersonalDataManager::MergeProfile(
+    const AutofillProfile& profile,
+    const std::vector<AutofillProfile*>& existing_profiles,
+    std::vector<AutofillProfile>* merged_profiles) {
+  DCHECK(merged_profiles);
+  merged_profiles->clear();
+
+  // Set to true if |profile| is merged into |existing_profiles|.
+  bool merged = false;
+
+  // First preference is to add missing values to an existing profile.
+  // Only merge with the first match.
+  for (std::vector<AutofillProfile*>::const_iterator iter =
+           existing_profiles.begin();
+       iter != existing_profiles.end(); ++iter) {
+    if (!merged) {
+      if (profile.IsSubsetOf(**iter)) {
+        // In this case, the existing profile already contains all of the data
+        // in |profile|, so consider the profiles already merged.
+        merged = true;
+      } else if ((*iter)->IntersectionOfTypesHasEqualValues(profile)) {
+        // |profile| contains all of the data in this profile, plus more.
+        merged = true;
+        (*iter)->MergeWith(profile);
+      }
+    }
+    merged_profiles->push_back(**iter);
+  }
+
+  // The second preference, if not merged above, is to alter non-primary values
+  // where the primary values match.
+  // Again, only merge with the first match.
+  if (!merged) {
+    merged_profiles->clear();
+    for (std::vector<AutofillProfile*>::const_iterator iter =
+             existing_profiles.begin();
+         iter != existing_profiles.end(); ++iter) {
+      if (!merged) {
+        if (!profile.PrimaryValue().empty() &&
+            (*iter)->PrimaryValue() == profile.PrimaryValue()) {
+          merged = true;
+          (*iter)->OverwriteWith(profile);
+        }
+      }
+      merged_profiles->push_back(**iter);
+    }
+  }
+
+  // Finally, if the new profile was not merged with an existing profile then
+  // add the new profile to the list.
+  if (!merged)
+    merged_profiles->push_back(profile);
+
+  return merged;
+}
+
 void PersonalDataManager::LoadProfiles() {
   WebDataService* web_data_service =
       profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
@@ -679,6 +779,7 @@ void PersonalDataManager::ReceiveLoadedProfiles(WebDataService::Handle h,
   }
 
   LogProfileCount();
+  EmptyMigrationTrash();
 }
 
 void PersonalDataManager::ReceiveLoadedCreditCards(
@@ -719,62 +820,6 @@ void PersonalDataManager::SaveImportedProfile(
   AddProfile(imported_profile);
 }
 
-bool PersonalDataManager::MergeProfile(
-    const AutofillProfile& profile,
-    const std::vector<AutofillProfile*>& existing_profiles,
-    std::vector<AutofillProfile>* merged_profiles) {
-  DCHECK(merged_profiles);
-  merged_profiles->clear();
-
-  // Set to true if |profile| is merged into |existing_profiles|.
-  bool merged = false;
-
-  // First preference is to add missing values to an existing profile.
-  // Only merge with the first match.
-  for (std::vector<AutofillProfile*>::const_iterator iter =
-           existing_profiles.begin();
-       iter != existing_profiles.end(); ++iter) {
-    if (!merged) {
-      if (profile.IsSubsetOf(**iter)) {
-        // In this case, the existing profile already contains all of the data
-        // in |profile|, so consider the profiles already merged.
-        merged = true;
-      } else if ((*iter)->IntersectionOfTypesHasEqualValues(profile)) {
-        // |profile| contains all of the data in this profile, plus more.
-        merged = true;
-        (*iter)->MergeWith(profile);
-      }
-    }
-    merged_profiles->push_back(**iter);
-  }
-
-  // The second preference, if not merged above, is to alter non-primary values
-  // where the primary values match.
-  // Again, only merge with the first match.
-  if (!merged) {
-    merged_profiles->clear();
-    for (std::vector<AutofillProfile*>::const_iterator iter =
-             existing_profiles.begin();
-         iter != existing_profiles.end(); ++iter) {
-      if (!merged) {
-        if (!profile.PrimaryValue().empty() &&
-            (*iter)->PrimaryValue() == profile.PrimaryValue()) {
-          merged = true;
-          (*iter)->OverwriteWith(profile);
-        }
-      }
-      merged_profiles->push_back(**iter);
-    }
-  }
-
-  // Finally, if the new profile was not merged with an existing profile then
-  // add the new profile to the list.
-  if (!merged)
-    merged_profiles->push_back(profile);
-
-  return merged;
-}
-
 
 void PersonalDataManager::SaveImportedCreditCard(
     const CreditCard& imported_credit_card) {
@@ -814,6 +859,27 @@ void PersonalDataManager::SaveImportedCreditCard(
   SetCreditCards(&creditcards);
 }
 
+void PersonalDataManager::EmptyMigrationTrash() const {
+  if (!profile_ || profile_->IsOffTheRecord())
+    return;
+
+  WebDataService* web_data_service =
+      profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
+  if (!web_data_service) {
+    NOTREACHED();
+    return;
+  }
+
+  ProfileSyncService* sync_service = profile_->GetProfileSyncService();
+  if (!sync_service)
+    return;
+
+  if (!sync_service->HasSyncSetupCompleted()) {
+    web_data_service->EmptyMigrationTrash(false);
+  } else if (sync_service->ShouldPushChanges()) {
+    web_data_service->EmptyMigrationTrash(true);
+  }
+}
 
 void PersonalDataManager::LogProfileCount() const {
   if (!g_has_logged_profile_count) {