[sync] Handle local changes in AutocompleteSyncBridge

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

Index: components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
diff --git a/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc b/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
index 12702868ce18b6f1d878ccbdccc6db9bb03d10ec..621fe9e91dc39f56718d03cc8f8ce6605cefa4f2 100644
--- a/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
+++ b/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
@@ -293,6 +293,8 @@ AutocompleteSyncBridge::AutocompleteSyncBridge(
   DCHECK(web_data_backend_);
 
   scoped_observer_.Add(web_data_backend_);
+
+  LoadMetadata();

[skym, 2017/01/14 00:04:33]

As far as I can tell, no unit tests check that this was called.

[Patrick Noland, 2017/01/17 22:59:55]

Done.

 }
 
 AutocompleteSyncBridge::~AutocompleteSyncBridge() {
@@ -390,6 +392,62 @@ void AutocompleteSyncBridge::GetAllData(DataCallback callback) {
   callback.Run(std::move(batch));
 }
 
+void AutocompleteSyncBridge::ActOnLocalChanges(
+    const AutofillChangeList& changes) {
+  if (!change_processor()->IsTrackingMetadata()) {
+    return;
+  }
+
+  std::unique_ptr<AutofillMetadataChangeList> change_list =

[maxbogue, 2017/01/14 00:39:02]

auto mcl = base::MakeUnique...

auto because MakeUnique makes the type clear, and mcl because to me it's
actually more descriptive than change_list (less confuse-able with non-metadata
changes)

[skym, 2017/01/17 17:57:21]

I'm not a huge fan of abbreviations. I'd prefer metadata_change_list or
metadata_changes, both of which we have precedent using. But I'm okay with
whatever else is chosen.

[Patrick Noland, 2017/01/17 22:59:55]

Done. I also don't love abbreviations whose meanings aren't obvious, so I went
with metadata_change_list.

+      base::MakeUnique<AutofillMetadataChangeList>(GetAutofillTable(),
+                                                   syncer::AUTOFILL);
+  for (const auto& change : changes) {
+    const std::string storage_key = GetStorageKeyFromModel(change.key());
+    switch (change.type()) {
+      case AutofillChange::ADD:
+      case AutofillChange::UPDATE: {
+        base::Time date_created, date_last_used;
+        bool success = GetAutofillTable()->GetAutofillTimestamps(
+            change.key().name(), change.key().value(), &date_created,
+            &date_last_used);
+        if (!success) {
+          change_processor()->ReportError(
+              FROM_HERE, "Failed reading autofill entry from WebDatabase.");
+          return;
+        }
+
+        AutofillEntry entry(change.key(), date_created, date_last_used);

[skym, 2017/01/14 00:04:33]

If you're going to const the storage_key, might as well const this too, right?

[Patrick Noland, 2017/01/17 22:59:55]

Done.

+        std::unique_ptr<syncer::EntityData> entity_data =

[skym, 2017/01/14 00:04:33]

I'd personally inline this, you're only saving a single character switching from
"CreateEntityData(entry)" to "std::move(entity_data)"

[maxbogue, 2017/01/14 00:39:02]

+1

[Patrick Noland, 2017/01/17 22:59:55]

Done.

+            CreateEntityData(entry);
+        change_processor()->Put(storage_key, std::move(entity_data),
+                                change_list.get());
+        break;
+      }
+      case AutofillChange::REMOVE: {
+        change_processor()->Delete(storage_key, change_list.get());
+        break;
+      }
+    }
+  }
+
+  Optional<ModelError> error = change_list->TakeError();
+  if (error) {
+    change_processor()->ReportError(

[skym, 2017/01/14 00:04:33]

You should give |error| (via error.value()) to ReportError() here. That way we
preserve the original FROM_HERE and hopefully a more useful message.

[maxbogue, 2017/01/14 00:39:02]

+1

[Patrick Noland, 2017/01/17 22:59:55]

Done.

+        FROM_HERE, "Failed writing autofill metadata to WebDatabase.");
+  }
+}
+
+void AutocompleteSyncBridge::LoadMetadata() {
+  std::unique_ptr<syncer::MetadataBatch> batch =

[skym, 2017/01/14 00:04:33]

auto!

[Patrick Noland, 2017/01/17 22:59:55]

Done.

+      base::MakeUnique<syncer::MetadataBatch>();
+  if (!GetAutofillTable()->GetAllSyncMetadata(syncer::AUTOFILL, batch.get())) {
+    change_processor()->ReportError(
+        FROM_HERE, "Failed reading autofill metadata from WebDatabase.");
+    return;
+  }
+  change_processor()->OnMetadataLoaded(std::move(batch));
+}
+
 std::string AutocompleteSyncBridge::GetClientTag(
     const EntityData& entity_data) {
   DCHECK(entity_data.specifics.has_autofill());
@@ -411,6 +469,7 @@ std::string AutocompleteSyncBridge::GetStorageKey(
 void AutocompleteSyncBridge::AutofillEntriesChanged(
     const AutofillChangeList& changes) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  ActOnLocalChanges(changes);
 }
 
 AutofillTable* AutocompleteSyncBridge::GetAutofillTable() const {