Clear wallet data from autofill when disabled.

Re-mask all unmasked cards when the user disables autofill integration.

Delete all server cards and addresses when the user disables the sync type or stops syncing.

Fix a bug where addresses were always written to the DB on startup because profiles were being created differently from protos than they were from sqlite so wouldn't match.

Optimize autofill profile comparisons. I was offended by the number of autofill profile and card comparisons we do at startup, the small number of items in the list means std::set is operating at a worst-case. I switched to brute-force with a "give up and rewrite the data" escape valve if you have a large number of addresses.

BUG=450843

Review URL: https://codereview.chromium.org/1007003007

Cr-Commit-Position: refs/heads/master@{#321406}

components/autofill/core/browser/personal_data_manager.cc

@@ -1342,6 +1342,11 @@ std::string PersonalDataManager::MostCommonCountryCodeFromProfiles() const {
 
 void PersonalDataManager::EnabledPrefChanged() {
   default_country_code_.clear();
+  if (!pref_service_->GetBoolean(prefs::kAutofillWalletImportEnabled)) {
+    // Re-mask all server cards when the user turns off wallet card
+    // integration.
+    ResetFullServerCards();
+  }
   NotifyPersonalDataChanged();
 }
 

components/autofill/core/browser/personal_data_manager.h

@@ -125,7 +125,6 @@ class PersonalDataManager : public KeyedService,
   void ResetFullServerCard(const std::string& guid);
 
   // Resets all unmasked cards to the masked state.
-  // TODO(estade): remove this.
   void ResetFullServerCards();
 
   // Returns the credit card with the specified |guid|, or NULL if there is

components/autofill/core/browser/webdata/autofill_table.cc

@@ -952,6 +952,10 @@ bool AutofillTable::GetServerProfiles(std::vector<AutofillProfile*>* profiles) {
 
 void AutofillTable::SetServerProfiles(
     const std::vector<AutofillProfile>& profiles) {
+  sql::Transaction transaction(db_);
+  if (!transaction.Begin())
+    return;
+
   // Delete all old ones first.
   sql::Statement delete_old(db_->GetUniqueStatement(
       "DELETE FROM server_addresses"));
@@ -996,6 +1000,8 @@ void AutofillTable::SetServerProfiles(
     insert.Run();
     insert.Reset(true);
   }
+
+  transaction.Commit();
 }
 
 bool AutofillTable::UpdateAutofillProfile(const AutofillProfile& profile) {

components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc

@@ -84,7 +84,6 @@ AutofillProfile ProfileFromSpecifics(
   profile.SetRawInfo(ADDRESS_HOME_STREET_ADDRESS,
                      base::UTF8ToUTF16(JoinString(street_address, '\n')));
 
-  profile.SetRawInfo(NAME_FULL, base::UTF8ToUTF16(address.recipient_name()));
   profile.SetRawInfo(COMPANY_NAME, base::UTF8ToUTF16(address.company_name()));
   profile.SetRawInfo(ADDRESS_HOME_STATE,
                      base::UTF8ToUTF16(address.address_1()));
@@ -99,27 +98,30 @@ AutofillProfile ProfileFromSpecifics(
                      base::UTF8ToUTF16(address.sorting_code()));
   profile.SetRawInfo(ADDRESS_HOME_COUNTRY,
                      base::UTF8ToUTF16(address.country_code()));
-  profile.SetRawInfo(PHONE_HOME_WHOLE_NUMBER,
-                     base::UTF8ToUTF16(address.phone_number()));
   profile.set_language_code(address.language_code());
 
+  // SetInfo instead of SetRawInfo so the constituent pieces will be parsed
+  // for these data types.
+  profile.SetInfo(AutofillType(NAME_FULL),
+                  base::UTF8ToUTF16(address.recipient_name()),
+                  profile.language_code());
+  profile.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
+                  base::UTF8ToUTF16(address.phone_number()),
+                  profile.language_code());
+
   return profile;
 }
 
-// Implements operator< for two objects given their pointers.
-template<class Data> struct AutofillDataPtrLessThan {
-  bool operator()(const Data* a, const Data* b) const {
-    return a->Compare(*b) < 0;
-  }
-};
-
 // This function handles conditionally updating the AutofillTable with either
 // a set of CreditCards or AutocompleteProfiles only when the existing data
 // doesn't match.
 //
 // It's passed the getter and setter function on the AutofillTable for the
-// corresponding data type, and expects the types to implement a server_id()
-// and a Compare function.
+// corresponding data type, and expects the types to implement a Compare
+// function. Note that the guid can't be used here as a key since these are
+// generated locally and will be different each time, and the server ID can't
+// be used because it's empty for addresses (though it could be used for credit
+// cards if we wanted separate implementations).
 //
 // Returns true if anything changed. The previous number of items in the table
 // (for sync tracking) will be placed into *prev_item_count.
@@ -134,18 +136,31 @@ bool SetDataIfChanged(
   (table->*getter)(&existing_data.get());
   *prev_item_count = existing_data.size();
 
-  bool difference_found = true;
-  if (existing_data.size() == data.size()) {
-    difference_found = false;
+  // If the user has a large number of addresses, don't bother verifying
+  // anything changed and just rewrite the data.
+  const size_t kTooBigToCheckThreshold = 8;
 
-    // Implement this set with pointers using our custom operator that takes
-    // pointers which avoids any copies.
-    std::set<const Data*, AutofillDataPtrLessThan<Data>> existing_data_set;
-    for (const Data* cur : existing_data)
-      existing_data_set.insert(cur);
+  bool difference_found;
+  if (existing_data.size() != data.size() ||
+      data.size() > kTooBigToCheckThreshold) {
+    difference_found = true;
+  } else {
+    difference_found = false;
 
-    for (const Data& new_data : data) {
-      if (existing_data_set.find(&new_data) == existing_data_set.end()) {
+    // Implement brute-force searching. Address and card counts are typically
+    // small, and comparing them is relatively expensive (many string
+    // compares). A std::set only uses operator< requiring multiple calls to
+    // check equality, giving 8 compares for 2 elements and 16 for 3. For these
+    // set sizes, brute force O(n^2) is faster.
+    for (const Data* cur_existing : existing_data) {
+      bool found_match_for_cur_existing = false;
+      for (const Data& cur_new : data) {
+        if (cur_existing->Compare(cur_new) == 0) {
+          found_match_for_cur_existing = true;
+          break;
+        }
+      }
+      if (!found_match_for_cur_existing) {
         difference_found = true;
         break;
       }
@@ -184,6 +199,12 @@ AutofillWalletSyncableService::MergeDataAndStartSyncing(
 void AutofillWalletSyncableService::StopSyncing(syncer::ModelType type) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(type, syncer::AUTOFILL_WALLET_DATA);
+  sync_processor_.reset();
+
+  // This data type is special. Normal sync data stays on the client when
+  // sync is disabled, but this one is supposed to represent the data you
+  // have on the server. Explicitly clear our local copy.
+  SetSyncData(syncer::SyncDataList());
 }
 
 syncer::SyncDataList AutofillWalletSyncableService::GetAllSyncData(