[sync] Handle local changes in AutocompleteSyncBridge

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

Index: components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc
diff --git a/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc b/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc
index 515b874815c6918f967a21bb325e5954b0a078c2..3cda2365add69b35a6643b913dd6abe69e2b68ba 100644
--- a/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc
+++ b/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc
@@ -16,18 +16,22 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
+#include "components/autofill/core/browser/webdata/autofill_entry.h"
 #include "components/autofill/core/browser/webdata/autofill_table.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_backend.h"
-#include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
+#include "components/autofill/core/browser/webdata/autofill_webdata_backend_impl.h"
 #include "components/sync/model/data_batch.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/model_error.h"
+#include "components/sync/model/recording_model_type_change_processor.h"
 #include "components/webdata/common/web_database.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using base::UTF8ToUTF16;
 using base::ScopedTempDir;
 using base::Time;
+using base::TimeDelta;
 using sync_pb::AutofillSpecifics;
 using sync_pb::EntitySpecifics;
 using syncer::DataBatch;
@@ -79,16 +83,9 @@ void VerifyDataBatch(std::map<std::string, AutofillSpecifics> expected,
   EXPECT_TRUE(expected.empty());
 }
 
-std::unique_ptr<ModelTypeChangeProcessor> CreateModelTypeChangeProcessor(
-    ModelType type,
-    ModelTypeSyncBridge* bridge) {
-  return base::MakeUnique<FakeModelTypeChangeProcessor>();
-}
-
-AutofillEntry CreateAutofillEntry(
-    const sync_pb::AutofillSpecifics& autofill_specifics) {
-  AutofillKey key(base::UTF8ToUTF16(autofill_specifics.name()),
-                  base::UTF8ToUTF16(autofill_specifics.value()));
+AutofillEntry CreateAutofillEntry(const AutofillSpecifics& autofill_specifics) {
+  AutofillKey key(UTF8ToUTF16(autofill_specifics.name()),
+                  UTF8ToUTF16(autofill_specifics.value()));
   Time date_created, date_last_used;
   const google::protobuf::RepeatedField<int64_t>& timestamps =
       autofill_specifics.usage_timestamp();
@@ -99,7 +96,6 @@ AutofillEntry CreateAutofillEntry(
   return AutofillEntry(key, date_created, date_last_used);
 }
 
-// Creates an EntityData/EntityDataPtr around a copy of the given specifics.
 EntityDataPtr SpecificsToEntity(const AutofillSpecifics& specifics) {
   EntityData data;
   data.client_tag_hash = "ignored";
@@ -135,15 +131,19 @@ class AutocompleteSyncBridgeTest : public testing::Test {
       db_.Init(temp_dir_.GetPath().AppendASCII("SyncTestWebDatabase"));
       backend_.SetWebDatabase(&db_);
 
+      sync_pb::ModelTypeState model_type_state;
+      model_type_state.set_initial_sync_done(true);
+      table_.UpdateModelTypeState(syncer::AUTOFILL, model_type_state);
+
       bridge_.reset(new AutocompleteSyncBridge(
-          &backend_, base::Bind(&CreateModelTypeChangeProcessor)));
+          &backend_,
+          base::Bind(
+              &AutocompleteSyncBridgeTest::CreateModelTypeChangeProcessor,
+              base::Unretained(this))));
     }
   }
   ~AutocompleteSyncBridgeTest() override {}
 
- protected:
-  AutocompleteSyncBridge* bridge() { return bridge_.get(); }
-
   void SaveSpecificsToTable(
       const std::vector<AutofillSpecifics>& specifics_list) {
     std::vector<AutofillEntry> new_entries;
@@ -217,6 +217,30 @@ class AutocompleteSyncBridgeTest : public testing::Test {
     bridge()->GetAllData(base::Bind(&VerifyDataBatch, ExpectedMap(expected)));
   }
 
+  void VerifyProcessorRecordedPut(const AutofillSpecifics& specifics,
+                               int position = 0) {
+    const std::string storage_key = GetStorageKey(specifics);
+    auto recorded_specifics_iterator =
+        processor()->put_multimap().find(storage_key);
+
+    EXPECT_NE(processor()->put_multimap().end(), recorded_specifics_iterator);
+    while (position > 0) {
+      recorded_specifics_iterator++;
+      EXPECT_NE(processor()->put_multimap().end(), recorded_specifics_iterator);
+      position--;
+    }
+
+    AutofillSpecifics recorded_specifics =
+        recorded_specifics_iterator->second->specifics.autofill();
+    VerifyEqual(recorded_specifics, specifics);
+  }
+
+  AutocompleteSyncBridge* bridge() { return bridge_.get(); }
+
+  syncer::RecordingModelTypeChangeProcessor* processor() { return processor_; }
+
+  AutofillTable* table() { return &table_; }
+
  private:
   ScopedTempDir temp_dir_;
   base::MessageLoop message_loop_;
@@ -224,6 +248,18 @@ class AutocompleteSyncBridgeTest : public testing::Test {
   AutofillTable table_;
   WebDatabase db_;
   std::unique_ptr<AutocompleteSyncBridge> bridge_;
+  // A non-owning pointer to the processor given to the bridge. Will be null
+  // before being given to the bridge, to make ownership easier.
+  syncer::RecordingModelTypeChangeProcessor* processor_ = nullptr;
+
+  std::unique_ptr<syncer::ModelTypeChangeProcessor>
+  CreateModelTypeChangeProcessor(syncer::ModelType type,

[maxbogue, 2017/01/18 00:01:42]

Functions go above member variables:
https://google.github.io/styleguide/cppguide.html#Declaration_Order

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

Done.

+                                 syncer::ModelTypeSyncBridge* bridge) {
+    auto processor =
+        base::MakeUnique<syncer::RecordingModelTypeChangeProcessor>();
+    processor_ = processor.get();
+    return std::move(processor);
+  }
 
   DISALLOW_COPY_AND_ASSIGN(AutocompleteSyncBridgeTest);
 };
@@ -426,4 +462,63 @@ TEST_F(AutocompleteSyncBridgeTest, ApplySyncChangesDatabaseFailure) {
   // making db calls and verify the errors are propagated up.
 }
 
+TEST_F(AutocompleteSyncBridgeTest, LocalEntriesAdded) {
+  const AutofillSpecifics added_specifics1 = CreateSpecifics(1, {2, 3});
+  const AutofillSpecifics added_specifics2 = CreateSpecifics(2, {2, 3});
+
+  const AutofillEntry added_entry1 = CreateAutofillEntry(added_specifics1);
+  const AutofillEntry added_entry2 = CreateAutofillEntry(added_specifics2);
+
+  table()->UpdateAutofillEntries({added_entry1, added_entry2});
+
+  bridge()->AutofillEntriesChanged(
+      {AutofillChange(AutofillChange::ADD, added_entry1.key()),
+       AutofillChange(AutofillChange::ADD, added_entry2.key())});
+
+  EXPECT_EQ(2u, processor()->put_multimap().size());
+
+  VerifyProcessorRecordedPut(added_specifics1);
+  VerifyProcessorRecordedPut(added_specifics2);
+}
+
+TEST_F(AutocompleteSyncBridgeTest, LocalEntryAddedThenUpdated) {
+  const AutofillSpecifics added_specifics = CreateSpecifics(1, {2, 3});
+  const AutofillEntry added_entry = CreateAutofillEntry(added_specifics);
+  table()->UpdateAutofillEntries({added_entry});
+
+  bridge()->AutofillEntriesChanged(
+      {AutofillChange(AutofillChange::ADD, added_entry.key())});
+
+  EXPECT_EQ(1u, processor()->put_multimap().size());
+
+  VerifyProcessorRecordedPut(added_specifics);
+
+  const AutofillSpecifics updated_specifics = CreateSpecifics(1, {2, 4});
+  const AutofillEntry updated_entry = CreateAutofillEntry(updated_specifics);
+  table()->UpdateAutofillEntries({updated_entry});
+
+  bridge()->AutofillEntriesChanged(
+      {AutofillChange(AutofillChange::UPDATE, updated_entry.key())});
+
+  VerifyProcessorRecordedPut(updated_specifics, 1);
+}
+
+TEST_F(AutocompleteSyncBridgeTest, LocalEntryDeleted) {
+  const AutofillSpecifics deleted_specifics = CreateSpecifics(1, {2, 3});
+  const AutofillEntry deleted_entry = CreateAutofillEntry(deleted_specifics);
+  const std::string storage_key = GetStorageKey(deleted_specifics);
+
+  bridge()->AutofillEntriesChanged(
+      {AutofillChange(AutofillChange::REMOVE, deleted_entry.key())});
+
+  EXPECT_EQ(1u, processor()->delete_set().size());
+  const auto& deleted_entries = processor()->delete_set().find(storage_key);

[maxbogue, 2017/01/18 00:01:42]

This would read more clearly if it had iter in the name, like deleted_iter or
something. Also plain auto is probably fine; it has to copy it anyways doesn't
it?

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

I think I'll just inline it actually, since it's only used in the line below.

+  EXPECT_NE(deleted_entries, processor()->delete_set().end());

[maxbogue, 2017/01/18 00:01:42]

switch order; (not) expected thing comes first.

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

Done.

+}
+
+TEST_F(AutocompleteSyncBridgeTest, LoadMetadataCalled) {
+  EXPECT_NE(processor()->metadata(), nullptr);
+  EXPECT_TRUE(processor()->metadata()->GetModelTypeState().initial_sync_done());
+}
+
 }  // namespace autofill