[Sync] DeviceInfo implementation of MergeSyncData.

BUG=543404, 543405, 543406

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

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

3 files changed, 185 insertions, 19 deletions

diff --git a/components/sync_driver/device_info_service.cc b/components/sync_driver/device_info_service.cc
index 8702864..6a63dcb 100644
--- a/components/sync_driver/device_info_service.cc
+++ b/components/sync_driver/device_info_service.cc
@@ -4,6 +4,7 @@
 
 #include "components/sync_driver/device_info_service.h"
 
+#include <set>
 #include <utility>
 #include <vector>
 
@@ -69,8 +70,59 @@ scoped_ptr<MetadataChangeList> DeviceInfoService::CreateMetadataChangeList() {
 SyncError DeviceInfoService::MergeSyncData(
     scoped_ptr<MetadataChangeList> metadata_change_list,
     EntityDataMap entity_data_map) {
-  // TODO(skym): crbug.com/543406: Implementation.
-  return SyncError();
+  if (!has_provider_initialized_ || !has_data_loaded_ || !change_processor()) {
+    return SyncError(
+        FROM_HERE, SyncError::DATATYPE_ERROR,
+        "Cannot call MergeSyncData without provider, data, and processor.",
+        syncer::DEVICE_INFO);
+  }
+
+  // Local data should typically be near empty, with the only possible value
+  // corresponding to this device. This is because on signout all device info
+  // data is blown away. However, this simplification is being ignored here and
+  // a full difference is going to be calculated to explore what other service
+  // implementations may look like.
+  std::set<std::string> local_only_tags;
+  for (const auto& kv : all_data_) {
+    local_only_tags.insert(kv.first);
+  }
+
+  bool has_changes = false;
+  std::string local_tag =
+      local_device_info_provider_->GetLocalDeviceInfo()->guid();
+  scoped_ptr<WriteBatch> batch = store_->CreateWriteBatch();
+  for (const auto& kv : entity_data_map) {
+    const std::string tag = GetClientTag(kv.second.value());
+    const DeviceInfoSpecifics& specifics =
+        kv.second.value().specifics.device_info();
+
+    // Ignore any remote changes that have our local cache guid.
+    if (tag == local_tag) {
+      continue;
+    }
+
+    // Remote data wins conflicts.
+    local_only_tags.erase(tag);
+    StoreSpecifics(make_scoped_ptr(new DeviceInfoSpecifics(specifics)),
+                   batch.get());
+    has_changes = true;
+  }
+
+  for (const std::string& tag : local_only_tags) {
+    change_processor()->Put(tag, CopyIntoNewEntityData(*all_data_[tag]),
+                            metadata_change_list.get());
+  }
+
+  // Transfer at the end because processor Put calls may update metadata.
+  static_cast<SimpleMetadataChangeList*>(metadata_change_list.get())
+      ->TransferChanges(store_.get(), batch.get());
+  store_->CommitWriteBatch(
+      std::move(batch),
+      base::Bind(&DeviceInfoService::OnCommit, weak_factory_.GetWeakPtr()));
+  if (has_changes) {
+    NotifyObservers();
+  }
+  return syncer::SyncError();
 }
 
 SyncError DeviceInfoService::ApplySyncChanges(
@@ -128,8 +180,8 @@ void DeviceInfoService::GetData(ClientTagList client_tags,
 
   syncer::SyncError error;
   scoped_ptr<DataBatchImpl> batch(new DataBatchImpl());
-  for (auto& tag : client_tags) {
-    auto iter = all_data_.find(tag);
+  for (const auto& tag : client_tags) {
+    const auto iter = all_data_.find(tag);
     if (iter != all_data_.end()) {
       batch->Put(tag, CopyIntoNewEntityData(*iter->second));
     }
@@ -148,7 +200,7 @@ void DeviceInfoService::GetAllData(DataCallback callback) {
 
   syncer::SyncError error;
   scoped_ptr<DataBatchImpl> batch(new DataBatchImpl());
-  for (auto& kv : all_data_) {
+  for (const auto& kv : all_data_) {
     batch->Put(kv.first, CopyIntoNewEntityData(*kv.second));
   }
   callback.Run(error, std::move(batch));
@@ -169,7 +221,7 @@ bool DeviceInfoService::IsSyncing() const {
 
 scoped_ptr<DeviceInfo> DeviceInfoService::GetDeviceInfo(
     const std::string& client_id) const {
-  ClientIdToSpecifics::const_iterator iter = all_data_.find(client_id);
+  const ClientIdToSpecifics::const_iterator iter = all_data_.find(client_id);
   if (iter == all_data_.end()) {
     return scoped_ptr<DeviceInfo>();
   }
diff --git a/components/sync_driver/device_info_service_unittest.cc b/components/sync_driver/device_info_service_unittest.cc
index ccaa692..4cd69c6 100644
--- a/components/sync_driver/device_info_service_unittest.cc
+++ b/components/sync_driver/device_info_service_unittest.cc
@@ -16,6 +16,7 @@
 #include "base/strings/stringprintf.h"
 #include "components/sync_driver/local_device_info_provider_mock.h"
 #include "sync/api/data_batch.h"
+#include "sync/api/entity_data.h"
 #include "sync/api/metadata_batch.h"
 #include "sync/api/model_type_store.h"
 #include "sync/internal_api/public/test/model_type_store_test_util.h"
@@ -29,6 +30,7 @@ using syncer_v2::DataBatch;
 using syncer_v2::EntityChange;
 using syncer_v2::EntityChangeList;
 using syncer_v2::EntityData;
+using syncer_v2::EntityDataMap;
 using syncer_v2::EntityDataPtr;
 using syncer_v2::MetadataBatch;
 using syncer_v2::MetadataChangeList;
@@ -97,6 +99,18 @@ void AssertExpectedFromDataBatch(
   ASSERT_TRUE(expected.empty());
 }
 
+// Creats an EntityData/EntityDataPtr around a copy of the given specifics.
+EntityDataPtr SpecificsToEntity(const DeviceInfoSpecifics& specifics) {
+  EntityData data;
+  // These tests do not care about the tag hash, but EntityData and friends
+  // cannot differentiate between the default EntityData object if the hash
+  // is unset, which causes pass/copy operations to no-op and things start to
+  // break, so we throw in a junk value and forget about it.
+  data.client_tag_hash = "junk";
+  *data.specifics.mutable_device_info() = specifics;
+  return data.PassToPtr();
+}
+
 // Instead of actually processing anything, simply accumulates all instructions
 // in members that can then be accessed. TODO(skym): If this ends up being
 // useful for other model type unittests it should be moved out to a shared
@@ -206,6 +220,13 @@ class DeviceInfoServiceTest : public testing::Test,
     return specifics;
   }
 
+  // Override to allow specific cache guids.
+  DeviceInfoSpecifics GenerateTestSpecifics(const std::string& guid) {
+    DeviceInfoSpecifics specifics(GenerateTestSpecifics());
+    specifics.set_cache_guid(guid);
+    return specifics;
+  }
+
   // Helper method to reduce duplicated code between tests. Wraps the given
   // specifics object in an EntityData and EntityChange of type ACTION_ADD, and
   // pushes them onto the given change list. The corresponding client tag the
@@ -213,15 +234,9 @@ class DeviceInfoServiceTest : public testing::Test,
   // service to generate the tag.
   std::string PushBackEntityChangeAdd(const DeviceInfoSpecifics& specifics,
                                       EntityChangeList* changes) {
-    EntityData data;
-    // These tests do not care about the tag hash, but EntityData and friends
-    // cannot differentiate between the default EntityData object if the hash
-    // is unset, which causes pass/copy operations to no-op and things start to
-    // break, so we throw in a junk value and forget about it.
-    data.client_tag_hash = "junk";
-    *data.specifics.mutable_device_info() = specifics;
-    const std::string tag = service()->GetClientTag(data);
-    changes->push_back(EntityChange::CreateAdd(tag, data.PassToPtr()));
+    EntityDataPtr ptr = SpecificsToEntity(specifics);
+    const std::string tag = service()->GetClientTag(ptr.value());
+    changes->push_back(EntityChange::CreateAdd(tag, ptr));
     return tag;
   }
 
@@ -537,8 +552,8 @@ TEST_F(DeviceInfoServiceTest, ApplySyncChangesWithLocalGuid) {
   // The point of this test is to try to apply remote changes that have the same
   // cache guid as the local device. The service should ignore these changes
   // since only it should be performing writes on its data.
-  DeviceInfoSpecifics specifics = GenerateTestSpecifics();
-  specifics.set_cache_guid(local_device()->GetLocalDeviceInfo()->guid());
+  DeviceInfoSpecifics specifics =
+      GenerateTestSpecifics(local_device()->GetLocalDeviceInfo()->guid());
 
   EntityChangeList data_changes;
   const std::string tag = PushBackEntityChangeAdd(specifics, &data_changes);
@@ -560,6 +575,94 @@ TEST_F(DeviceInfoServiceTest, ApplyDeleteNonexistent) {
   EXPECT_EQ(0, change_count());
 }
 
+TEST_F(DeviceInfoServiceTest, MergeBeforeInit) {
+  InitializeService();
+  const SyncError error = service()->MergeSyncData(
+      service()->CreateMetadataChangeList(), EntityDataMap());
+  EXPECT_TRUE(error.IsSet());
+  EXPECT_EQ(0, change_count());
+}
+
+TEST_F(DeviceInfoServiceTest, MergeEmpty) {
+  InitializeAndPump();
+  SetProcessorAndPump();
+  const SyncError error = service()->MergeSyncData(
+      service()->CreateMetadataChangeList(), EntityDataMap());
+  EXPECT_FALSE(error.IsSet());
+  EXPECT_EQ(0, change_count());
+}
+
+TEST_F(DeviceInfoServiceTest, MergeWithData) {
+  const DeviceInfoSpecifics unique_local(GenerateTestSpecifics("unique_local"));
+  const DeviceInfoSpecifics conflict_local(GenerateTestSpecifics("conflict"));
+  DeviceInfoSpecifics conflict_remote(GenerateTestSpecifics("conflict"));
+  DeviceInfoSpecifics unique_remote(GenerateTestSpecifics("unique_remote"));
+
+  scoped_ptr<WriteBatch> batch = store()->CreateWriteBatch();
+  store()->WriteData(batch.get(), unique_local.cache_guid(),
+                     unique_local.SerializeAsString());
+  store()->WriteData(batch.get(), conflict_local.cache_guid(),
+                     conflict_local.SerializeAsString());
+  store()->CommitWriteBatch(std::move(batch),
+                            base::Bind(&AssertResultIsSuccess));
+
+  InitializeAndPump();
+  SetProcessorAndPump();
+
+  EntityDataMap remote_input;
+  remote_input[conflict_remote.cache_guid()] =
+      SpecificsToEntity(conflict_remote);
+  remote_input[unique_remote.cache_guid()] = SpecificsToEntity(unique_remote);
+
+  DataTypeState state;
+  state.set_encryption_key_name("ekn");
+  scoped_ptr<MetadataChangeList> metadata_changes(
+      service()->CreateMetadataChangeList());
+  metadata_changes->UpdateDataTypeState(state);
+
+  const SyncError error =
+      service()->MergeSyncData(std::move(metadata_changes), remote_input);
+  EXPECT_FALSE(error.IsSet());
+  EXPECT_EQ(1, change_count());
+
+  // The remote should beat the local in conflict.
+  EXPECT_EQ(3u, service()->GetAllDeviceInfo().size());
+  AssertEqual(unique_local, *service()->GetDeviceInfo("unique_local").get());
+  AssertEqual(unique_remote, *service()->GetDeviceInfo("unique_remote").get());
+  AssertEqual(conflict_remote, *service()->GetDeviceInfo("conflict").get());
+
+  // Service should have told the processor about the existance of unique_local.
+  EXPECT_TRUE(processor()->delete_set().empty());
+  EXPECT_EQ(1u, processor()->put_map().size());
+  const auto& it = processor()->put_map().find("unique_local");
+  ASSERT_NE(processor()->put_map().end(), it);
+  AssertEqual(unique_local, it->second->specifics.device_info());
+
+  // TODO(skym): Uncomment once SimpleMetadataChangeList::TransferChanges is
+  // implemented.
+  // ASSERT_EQ(state.encryption_key_name(),
+  // processor()->metadata()->GetDataTypeState().encryption_key_name());
+}
+
+TEST_F(DeviceInfoServiceTest, MergeLocalGuid) {
+  InitializeAndPump();
+  SetProcessorAndPump();
+
+  // Service should ignore this because it uses the local device's guid.
+  DeviceInfoSpecifics specifics(
+      GenerateTestSpecifics(local_device()->GetLocalDeviceInfo()->guid()));
+  EntityDataMap remote_input;
+  remote_input[specifics.cache_guid()] = SpecificsToEntity(specifics);
+
+  const SyncError error = service()->MergeSyncData(
+      service()->CreateMetadataChangeList(), remote_input);
+  EXPECT_FALSE(error.IsSet());
+  EXPECT_EQ(0, change_count());
+  EXPECT_TRUE(service()->GetAllDeviceInfo().empty());
+  EXPECT_TRUE(processor()->delete_set().empty());
+  EXPECT_TRUE(processor()->put_map().empty());
+}
+
 }  // namespace
 
 }  // namespace sync_driver_v2
diff --git a/sync/api/model_type_service.h b/sync/api/model_type_service.h
index 950060e..bba4211 100644
--- a/sync/api/model_type_service.h
+++ b/sync/api/model_type_service.h
@@ -38,8 +38,19 @@ class SYNC_EXPORT ModelTypeService {
   // model type store.
   virtual scoped_ptr<MetadataChangeList> CreateMetadataChangeList() = 0;
 
-  // Perform the initial merge of data from the sync server. Should only need
-  // to be called when sync is first turned on, not on every restart.
+  // Perform the initial merge between local and sync data. This should only be
+  // called when a data type is first enabled to start syncing, and there is no
+  // sync metadata. Best effort should be made to match local and sync data. The
+  // keys in the |entity_data_map| will have been created via GetClientTag(...),
+  // and if a local and sync data should match/merge but disagree on tags, the
+  // service should use the sync data's tag. Any local pieces of data that are
+  // not present in sync should immediately be Put(...) to the processor before
+  // returning. The same MetadataChangeList that was passed into this function
+  // can be passed to Put(...) calls. Delete(...) can also be called but should
+  // not be needed for most model types. Durable storage writes, if not able to
+  // combine all change atomically, should save the metadata after the data
+  // changes, so that this merge will be re-driven by sync if is not completely
+  // saved during the current run.
   virtual syncer::SyncError MergeSyncData(
       scoped_ptr<MetadataChangeList> metadata_change_list,
       EntityDataMap entity_data_map) = 0;