[Autofill profile] Implement remote changes for the USS bridge

This CL completes full functionality for the sync bridge for
autofill_profile model type. The CL only converts applicable
unit-tests from AutofillProfileSyncableServiceTest. Tests corresponding
to ProfileSyncServiceAutofillTest will come in a later CL.

Bug: 836718
Change-Id: I5cf596e117d74c85d2ac40a356b0beaab98e1a1f
Reviewed-on: https://chromium-review.googlesource.com/1100762Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572552}

components/autofill/core/browser/BUILD.gn

@@ -170,6 +170,8 @@ static_library("browser") {
     "webdata/autofill_profile_data_type_controller.h",
     "webdata/autofill_profile_sync_bridge.cc",
     "webdata/autofill_profile_sync_bridge.h",
+    "webdata/autofill_profile_sync_difference_tracker.cc",
+    "webdata/autofill_profile_sync_difference_tracker.h",
     "webdata/autofill_profile_syncable_service.cc",
     "webdata/autofill_profile_syncable_service.h",
     "webdata/autofill_table.cc",
@@ -444,6 +446,7 @@ source_set("unit_tests") {
     "validation_unittest.cc",
     "webdata/autocomplete_sync_bridge_unittest.cc",
     "webdata/autofill_profile_sync_bridge_unittest.cc",
+    "webdata/autofill_profile_sync_difference_tracker_unittest.cc",
     "webdata/autofill_profile_syncable_service_unittest.cc",
     "webdata/autofill_table_unittest.cc",
     "webdata/autofill_wallet_metadata_syncable_service_unittest.cc",

components/autofill/core/browser/autofill_profile.cc

@@ -254,14 +254,14 @@ AutofillProfile::~AutofillProfile() {
 }
 
 AutofillProfile& AutofillProfile::operator=(const AutofillProfile& profile) {
+  if (this == &profile)
+    return *this;
+
   set_use_count(profile.use_count());
   set_use_date(profile.use_date());
   set_previous_use_date(profile.previous_use_date());
   set_modification_date(profile.modification_date());
 
-  if (this == &profile)
-    return *this;
-
   set_guid(profile.guid());
   set_origin(profile.origin());
 
@@ -465,6 +465,30 @@ bool AutofillProfile::IsSubsetOfForFieldSet(
   return true;
 }
 
+void AutofillProfile::OverwriteDataFrom(const AutofillProfile& profile) {
+  // Verified profiles should never be overwritten with unverified data.
+  DCHECK(!IsVerified() || profile.IsVerified());
+  DCHECK_EQ(guid(), profile.guid());
+
+  // Some fields should not got overwritten by empty values; back-up the
+  // values.
+  std::string language_code_value = language_code();
+  std::string origin_value = origin();
+  int validity_bitfield_value = GetValidityBitfieldValue();
+  base::string16 name_full_value = GetRawInfo(NAME_FULL);
+
+  *this = profile;
+
+  if (origin().empty())
+    set_origin(origin_value);
+  if (language_code().empty())
+    set_language_code(language_code_value);
+  if (GetValidityBitfieldValue() == 0)
+    SetValidityFromBitfieldValue(validity_bitfield_value);
+  if (!HasRawInfo(NAME_FULL))
+    SetRawInfo(NAME_FULL, name_full_value);
+}
+
 bool AutofillProfile::MergeDataFrom(const AutofillProfile& profile,
                                     const std::string& app_locale) {
   // Verified profiles should never be overwritten with unverified data.

components/autofill/core/browser/autofill_profile.h

@@ -122,6 +122,10 @@ class AutofillProfile : public AutofillDataModel,
                              const std::string& app_locale,
                              const ServerFieldTypeSet& types) const;
 
+  // Overwrites the data of |this| profile with data from the given |profile|.
+  // Expects that the profiles have the same guid.
+  void OverwriteDataFrom(const AutofillProfile& profile);
+
   // Merges the data from |this| profile and the given |profile| into |this|
   // profile. Expects that |this| and |profile| have already been deemed
   // mergeable by an AutofillProfileComparator.

components/autofill/core/browser/autofill_profile_sync_util.cc

@@ -29,13 +29,17 @@ std::string TruncateUTF8(const std::string& data) {
   return trimmed_value;
 }
 
+bool IsAutofillProfileSpecificsValid(
+    const AutofillProfileSpecifics& specifics) {
+  return base::IsValidGUID(specifics.guid());
+}
+
 }  // namespace
 
 std::unique_ptr<EntityData> CreateEntityDataFromAutofillProfile(
     const AutofillProfile& entry) {
-  if (!base::IsValidGUID(entry.guid())) {
-    return nullptr;
-  }
+  // Validity of the guid is guaranteed by the database layer.
+  DCHECK(base::IsValidGUID(entry.guid()));
 
   auto entity_data = std::make_unique<EntityData>();
   entity_data->non_unique_name = entry.guid();
@@ -124,7 +128,7 @@ std::unique_ptr<EntityData> CreateEntityDataFromAutofillProfile(
 
 std::unique_ptr<AutofillProfile> CreateAutofillProfileFromSpecifics(
     const AutofillProfileSpecifics& specifics) {
-  if (!base::IsValidGUID(specifics.guid())) {
+  if (!IsAutofillProfileSpecificsValid(specifics)) {
     return nullptr;
   }
   std::unique_ptr<AutofillProfile> profile =
@@ -215,15 +219,14 @@ std::unique_ptr<AutofillProfile> CreateAutofillProfileFromSpecifics(
 }
 
 std::string GetStorageKeyFromAutofillProfile(const AutofillProfile& entry) {
-  if (!base::IsValidGUID(entry.guid())) {
-    return std::string();
-  }
+  // Validity of the guid is guaranteed by the database layer.
+  DCHECK(base::IsValidGUID(entry.guid()));
   return entry.guid();
 }
 
 std::string GetStorageKeyFromAutofillProfileSpecifics(
     const AutofillProfileSpecifics& specifics) {
-  if (!base::IsValidGUID(specifics.guid())) {
+  if (!IsAutofillProfileSpecificsValid(specifics)) {
     return std::string();
   }
   return specifics.guid();

components/autofill/core/browser/autofill_profile_sync_util.h

@@ -21,13 +21,13 @@ namespace autofill {
 class AutofillProfile;
 
 // Converts the given |entry| into a syncer EntityData with equivalent
-// autofill profile specifics. Returns nullptr if |entry| has invalid guid.
+// autofill profile specifics. Returns nullptr if |entry| is invalid.
 // Shortens all string fields to AutofillTable::kMaxDataLength.
 std::unique_ptr<syncer::EntityData> CreateEntityDataFromAutofillProfile(
     const AutofillProfile& entry);
 
 // Converts the given autofill profile |specifics| into an equivalent
-// AutofillProfile. Returns nullptr if |specifics| has invalid guid.
+// AutofillProfile. Returns nullptr if |specifics| is invalid.
 std::unique_ptr<AutofillProfile> CreateAutofillProfileFromSpecifics(
     const sync_pb::AutofillProfileSpecifics& specifics);
 

components/autofill/core/browser/autofill_profile_sync_util_unittest.cc

@@ -142,14 +142,6 @@ TEST_F(AutofillProfileSyncUtilTest, CreateEntityDataFromAutofillProfile_Empty) {
   EXPECT_FALSE(entity_data->specifics.autofill_profile().has_company_name());
 }
 
-// Test that nullptr is produced if the input guid is invalid.
-TEST_F(AutofillProfileSyncUtilTest,
-       CreateEntityDataFromAutofillProfile_Invalid) {
-  AutofillProfile profile(kGuidInvalid, std::string());
-
-  EXPECT_EQ(nullptr, CreateEntityDataFromAutofillProfile(profile));
-}
-
 // Test that long fields get trimmed.
 TEST_F(AutofillProfileSyncUtilTest,
        CreateEntityDataFromAutofillProfile_Trimmed) {
@@ -275,13 +267,6 @@ TEST_F(AutofillProfileSyncUtilTest, GetStorageKeyFromAutofillProfile) {
   EXPECT_EQ(kGuid, GetStorageKeyFromAutofillProfile(profile));
 }
 
-// Tests that empty string is returned for entry with invalid guid.
-TEST_F(AutofillProfileSyncUtilTest, GetStorageKeyFromAutofillProfile_Invalid) {
-  AutofillProfile profile(kGuidInvalid, std::string());
-
-  EXPECT_EQ(std::string(), GetStorageKeyFromAutofillProfile(profile));
-}
-
 // Tests that guid is returned as storage key.
 TEST_F(AutofillProfileSyncUtilTest, GetStorageKeyFromAutofillProfileSpecifics) {
   AutofillProfileSpecifics specifics;

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

@@ -17,6 +17,7 @@
 #include "components/autofill/core/browser/country_names.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/browser/proto/autofill_sync.pb.h"
+#include "components/autofill/core/browser/webdata/autofill_profile_sync_difference_tracker.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"
@@ -105,8 +106,30 @@ Optional<syncer::ModelError> AutofillProfileSyncBridge::MergeSyncData(
     syncer::EntityChangeList entity_data) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  DCHECK(entity_data.empty());
-  // TODO(jkrcal): Implement non-empty initial merge.
+  AutofillProfileInitialSyncDifferenceTracker initial_sync_tracker(
+      GetAutofillTable());
+
+  for (const auto& change : entity_data) {
+    DCHECK(change.data().specifics.has_autofill_profile());
+    std::unique_ptr<AutofillProfile> remote =
+        CreateAutofillProfileFromSpecifics(
+            change.data().specifics.autofill_profile());
+    if (!remote) {
+      DVLOG(2) << "[AUTOFILL SYNC] Invalid remote specifics "
+               << change.data().specifics.autofill_profile().SerializeAsString()
+               << " received from the server in an initial sync.";
+      continue;
+    }
+    RETURN_IF_ERROR(
+        initial_sync_tracker.IncorporateRemoteProfile(std::move(remote)));
+  }
+
+  RETURN_IF_ERROR(
+      initial_sync_tracker.MergeSimilarEntriesForInitialSync(app_locale_));
+  RETURN_IF_ERROR(
+      FlushSyncTracker(std::move(metadata_change_list), &initial_sync_tracker));
+
+  web_data_backend_->NotifyThatSyncHasStarted(syncer::AUTOFILL_PROFILE);
   return base::nullopt;
 }
 
@@ -115,9 +138,27 @@ Optional<ModelError> AutofillProfileSyncBridge::ApplySyncChanges(
     syncer::EntityChangeList entity_changes) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  DCHECK(entity_changes.empty());
-  // TODO(jkrcal): Implement non-empty sync changes.
-  return base::nullopt;
+  AutofillProfileSyncDifferenceTracker tracker(GetAutofillTable());
+  for (const syncer::EntityChange& change : entity_changes) {
+    if (change.type() == syncer::EntityChange::ACTION_DELETE) {
+      RETURN_IF_ERROR(tracker.IncorporateRemoteDelete(change.storage_key()));
+    } else {
+      DCHECK(change.data().specifics.has_autofill_profile());
+      std::unique_ptr<AutofillProfile> remote =
+          CreateAutofillProfileFromSpecifics(
+              change.data().specifics.autofill_profile());
+      if (!remote) {
+        DVLOG(2)
+            << "[AUTOFILL SYNC] Invalid remote specifics "
+            << change.data().specifics.autofill_profile().SerializeAsString()
+            << " received from the server in an initial sync.";
+        continue;
+      }
+      RETURN_IF_ERROR(tracker.IncorporateRemoteProfile(std::move(remote)));
+    }
+  }
+
+  return FlushSyncTracker(std::move(metadata_change_list), &tracker);
 }
 
 void AutofillProfileSyncBridge::GetData(StorageKeyList storage_keys,
@@ -194,6 +235,29 @@ void AutofillProfileSyncBridge::ActOnLocalChange(
   }
 }
 
+base::Optional<syncer::ModelError> AutofillProfileSyncBridge::FlushSyncTracker(
+    std::unique_ptr<MetadataChangeList> metadata_change_list,
+    AutofillProfileSyncDifferenceTracker* tracker) {
+  DCHECK(tracker);
+
+  RETURN_IF_ERROR(tracker->FlushToLocal(
+      base::BindOnce(&AutofillWebDataBackend::NotifyOfMultipleAutofillChanges,
+                     base::Unretained(web_data_backend_))));
+
+  std::vector<std::unique_ptr<AutofillProfile>> profiles_to_upload_to_sync;
+  RETURN_IF_ERROR(tracker->FlushToSync(&profiles_to_upload_to_sync));
+  for (const std::unique_ptr<AutofillProfile>& entry :
+       profiles_to_upload_to_sync) {
+    change_processor()->Put(GetStorageKeyFromAutofillProfile(*entry),
+                            CreateEntityDataFromAutofillProfile(*entry),
+                            metadata_change_list.get());
+  }
+
+  return static_cast<syncer::SyncMetadataStoreChangeList*>(
+             metadata_change_list.get())
+      ->TakeError();
+}
+
 void AutofillProfileSyncBridge::LoadMetadata() {
   if (!web_data_backend_ || !web_data_backend_->GetDatabase() ||
       !GetAutofillTable()) {

components/autofill/core/browser/webdata/autofill_profile_sync_bridge.h

@@ -26,6 +26,7 @@ class ModelTypeChangeProcessor;
 
 namespace autofill {
 
+class AutofillProfileSyncDifferenceTracker;
 class AutofillTable;
 class AutofillWebDataBackend;
 class AutofillWebDataService;
@@ -87,6 +88,11 @@ class AutofillProfileSyncBridge
   // changes.
   void ActOnLocalChange(const AutofillProfileChange& change);
 
+  // Flushes changes accumulated within |tracker| both to local and to sync.
+  base::Optional<syncer::ModelError> FlushSyncTracker(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      AutofillProfileSyncDifferenceTracker* tracker);
+
   // Synchronously load sync metadata from the autofill table and pass it to the
   // processor so that it can start tracking changes.
   void LoadMetadata();

components/autofill/core/browser/webdata/autofill_profile_sync_difference_tracker.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNC_DIFFERENCE_TRACKER_H_
+#define COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNC_DIFFERENCE_TRACKER_H_
+
+#include <map>
+#include <memory>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/optional.h"
+
+namespace syncer {
+class ModelError;
+}  // namespace syncer
+
+namespace autofill {
+
+class AutofillProfile;
+class AutofillProfileComparator;
+class AutofillTable;
+
+// This is used to respond to ApplySyncChanges() and MergeSyncData(). Attempts
+// to lazily load local data, and then react to sync data by maintaining
+// internal state until flush calls are made, at which point the applicable
+// modification should be sent towards local and sync directions.
+class AutofillProfileSyncDifferenceTracker {
+ public:
+  explicit AutofillProfileSyncDifferenceTracker(AutofillTable* table);
+  virtual ~AutofillProfileSyncDifferenceTracker();
+
+  // Adds a new |remote| entry to the diff tracker, originating from the sync
+  // server. The provided |remote| entry must be valid.
+  base::Optional<syncer::ModelError> IncorporateRemoteProfile(
+      std::unique_ptr<AutofillProfile> remote);
+
+  // Informs the diff tracker that the entry with |storage_key| has been deleted
+  // from the sync server. |storage_key| must be non-empty.
+  virtual base::Optional<syncer::ModelError> IncorporateRemoteDelete(
+      const std::string& storage_key);
+
+  // Writes all local changes to the provided autofill |table_|. After flushing,
+  // not further remote changes should get incorporated.
+  base::Optional<syncer::ModelError> FlushToLocal(
+      base::OnceClosure autofill_changes_callback);
+
+  // Writes into |profiles_to_upload_to_sync| all autofill profiles to be sent
+  // to the sync server. After flushing, not further remote changes should get
+  // incorporated.
+  virtual base::Optional<syncer::ModelError> FlushToSync(
+      std::vector<std::unique_ptr<AutofillProfile>>*
+          profiles_to_upload_to_sync);
+
+ protected:
+  // If the entry is found, |entry| will be return, otherwise base::nullopt is
+  // returned.
+  base::Optional<AutofillProfile> ReadEntry(const std::string& storage_key);
+
+  // Tries to find a local entry that is mergeable with |remote| (according to
+  // |comparator|). If such an entry is found, it is returned. Otherwise,
+  // base::nullopt is returned.
+  base::Optional<AutofillProfile> FindMergeableLocalEntry(
+      const AutofillProfile& remote,
+      const AutofillProfileComparator& comparator);
+
+  // Informs the tracker that a local entry with |storage_key| should get
+  // deleted.
+  void DeleteFromLocal(const std::string& storage_key);
+
+  // Accessor for data that is only stored local. Initializes the data if
+  // needed. Returns nullptr if initialization failed.
+  std::map<std::string, std::unique_ptr<AutofillProfile>>*
+  GetLocalOnlyEntries();
+
+  // Helper function called by GetLocalOnlyEntries().
+  bool InitializeLocalOnlyEntriesIfNeeded();
+
+  // The table for reading local data.
+  AutofillTable* const table_;
+
+  // This class loads local data from |table_| lazily. This field tracks if that
+  // has happened or not yet.
+  bool local_only_entries_initialized_ = false;
+
+  // We use unique_ptrs for storing AutofillProfile to avoid unnecessary copies.
+
+  // Local data, mapped by storage key. Use unique_to_local() to access it.
+  std::map<std::string, std::unique_ptr<AutofillProfile>> local_only_entries_;
+
+  // Contain changes (originating from sync) that need to be saved to the local
+  // store.
+  std::set<std::string> delete_from_local_;
+  std::vector<std::unique_ptr<AutofillProfile>> add_to_local_;
+  std::vector<std::unique_ptr<AutofillProfile>> update_to_local_;
+
+  // Contains merged data for entries that existed on both sync and local sides
+  // and need to be saved back to sync.
+  std::vector<std::unique_ptr<AutofillProfile>> save_to_sync_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AutofillProfileSyncDifferenceTracker);
+};
+
+class AutofillProfileInitialSyncDifferenceTracker
+    : public AutofillProfileSyncDifferenceTracker {
+ public:
+  explicit AutofillProfileInitialSyncDifferenceTracker(AutofillTable* table);
+  ~AutofillProfileInitialSyncDifferenceTracker() override;
+
+  base::Optional<syncer::ModelError> IncorporateRemoteDelete(
+      const std::string& storage_key) override;
+
+  base::Optional<syncer::ModelError> FlushToSync(
+      std::vector<std::unique_ptr<AutofillProfile>>* profiles_to_upload_to_sync)
+      override;
+
+  // Performs an additional pass through remote entries incorporated from sync
+  // to find any similarities with local entries. Should be run after all
+  // entries get incorporated but before flushing results to local/sync.
+  base::Optional<syncer::ModelError> MergeSimilarEntriesForInitialSync(
+      const std::string& app_locale);
+
+ private:
+  // Returns a local entry that is mergeable with |remote| if it exists.
+  // Otherwise, returns base::nullopt.
+  base::Optional<AutofillProfile> FindMergeableLocalEntry(
+      const AutofillProfile& remote,
+      const AutofillProfileComparator& comparator);
+
+  DISALLOW_COPY_AND_ASSIGN(AutofillProfileInitialSyncDifferenceTracker);
+};
+
+}  // namespace autofill
+
+#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNC_DIFFERENCE_TRACKER_H_