Clear wallet data from autofill when disabled.

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

Index: components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
diff --git a/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc b/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
index dcfff3f28f8f1e151912b8f174049fb6fffb66b7..ddd89dc97525120fc76321e797fbb18ef8585fff 100644
--- a/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
+++ b/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.

[Evan Stade, 2015/03/17 01:26:53]

I suspect this is true of many places in Autofill :/

set is probably more often used for the uniqueness property than for any
performance reason. Is there a std:: or base::dumbset we can use?

[brettw, 2015/03/19 16:39:39]

No, there's base::SmallMap but no set version. This does brute-force up to some
limit which is kind of what we want. I got curious so checked some options more
than was necessary:

First, I tried using this using true as the "value" in the map. It generates
twice as many compares as my brute force version since each insert also checks
equality. It also generates quite a bit more code since there's a bunch of
templates to due both brute-force and tree-based searching for this small
container, and requires this shocking template declaration:
base::SmallMap<std::map<const Data*, bool, AutofillDataPtrLessThan<Data>>, 8,
AutofillDataPtrEqualTo<Data>> existing_data_set;
and an extra 

Then I tried using find_if with a lambda. This was fun to write but I find hard
to read:
 if (std::find_if(existing_data.begin(), existing_data.end(),
                       [new_data](const Data* d) { return d->Compare(new_data)
== 0; })
          == existing_data.end()) {
and still generates more machine code than my simple brute force version.

So I prefer to keep my simple brute-force version.

+    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 says on the client when

[Evan Stade, 2015/03/17 01:26:53]

nit:says->stays

+  // 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(