[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..cf3345924e72a8b62ec3b063aeefdaf5972b89d8 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();
 }
 
 AutocompleteSyncBridge::~AutocompleteSyncBridge() {
@@ -390,6 +392,57 @@ void AutocompleteSyncBridge::GetAllData(DataCallback callback) {
   callback.Run(std::move(batch));
 }
 
+void AutocompleteSyncBridge::ActOnLocalChanges(
+    const AutofillChangeList& changes) {
+  if (!change_processor()->IsTrackingMetadata()) {
+    return;
+  }
+
+  auto 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;
+        }
+
+        const AutofillEntry entry(change.key(), date_created, date_last_used);
+        change_processor()->Put(storage_key, CreateEntityData(entry),
+                                metadata_change_list.get());
+        break;
+      }
+      case AutofillChange::REMOVE: {
+        change_processor()->Delete(storage_key, metadata_change_list.get());
+        break;
+      }
+    }
+  }
+
+  Optional<ModelError> error = metadata_change_list->TakeError();
+  if (error) {

[skym, 2017/01/17 23:50:10]

What do you think of inlining this?

if (Optional<ModelError> error = metadata_change_list->TakeError()) {
  change_processor()->ReportError(error.value());
}

I know, using the return value of an assignment is weird, but with optionals,
suddenly I feel like it becomes kinda idiomatic.

[Peter Kasting, 2017/01/18 00:33:27]

At least in the past, this could cause compiler warnings on some platforms.

If the {} are removed from the one-line body here, I think it reads OK without
doing this.  I always struggle to read assignment-inside-conditional, FWIW.

[Patrick Noland, 2017/01/19 19:11:49]

I like this structure for optionals. Done.

+    change_processor()->ReportError(error.value());
+  }
+}
+
+void AutocompleteSyncBridge::LoadMetadata() {
+  auto batch = 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 +464,7 @@ std::string AutocompleteSyncBridge::GetStorageKey(
 void AutocompleteSyncBridge::AutofillEntriesChanged(
     const AutofillChangeList& changes) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  ActOnLocalChanges(changes);
 }
 
 AutofillTable* AutocompleteSyncBridge::GetAutofillTable() const {