Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1545)

Unified Diff: components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc

Issue 2624883002: [Sync] ApplySyncChanges autofill implementation. (Closed)
Patch Set: More updates for Max's comments. Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 5b649892599217e67fadfbfac4a04e9729ecfce9..12702868ce18b6f1d878ccbdccc6db9bb03d10ec 100644
--- a/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
+++ b/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
@@ -4,6 +4,8 @@
#include "components/autofill/core/browser/webdata/autocomplete_sync_bridge.h"
+#include <algorithm>
+#include <set>
#include <unordered_set>
#include <utility>
#include <vector>
@@ -21,6 +23,18 @@
#include "components/sync/model/mutable_data_batch.h"
#include "net/base/escape.h"
+using base::Optional;
+using base::Time;
+using sync_pb::AutofillSpecifics;
+using syncer::EntityChange;
+using syncer::EntityChangeList;
+using syncer::EntityData;
+using syncer::EntityDataMap;
+using syncer::MetadataChangeList;
+using syncer::ModelError;
+using syncer::ModelTypeChangeProcessor;
+using syncer::MutableDataBatch;
+
namespace autofill {
namespace {
@@ -28,6 +42,12 @@ namespace {
const char kAutocompleteEntryNamespaceTag[] = "autofill_entry|";
const char kAutocompleteTagDelimiter[] = "|";
+// Simplify checking for optional errors and returning only when present.
+#define RETURN_IF_ERROR(x) \
+ if (Optional<ModelError> ret_val = x) { \
+ return ret_val; \
+ }
+
void* UserDataKey() {
// Use the address of a static that COMDAT folding won't ever collide
// with something else.
@@ -35,12 +55,10 @@ void* UserDataKey() {
return reinterpret_cast<void*>(&user_data_key);
}
-std::unique_ptr<syncer::EntityData> CreateEntityData(
- const AutofillEntry& entry) {
- auto entity_data = base::MakeUnique<syncer::EntityData>();
+std::unique_ptr<EntityData> CreateEntityData(const AutofillEntry& entry) {
+ auto entity_data = base::MakeUnique<EntityData>();
entity_data->non_unique_name = base::UTF16ToUTF8(entry.key().name());
- sync_pb::AutofillSpecifics* autofill =
- entity_data->specifics.mutable_autofill();
+ AutofillSpecifics* autofill = entity_data->specifics.mutable_autofill();
autofill->set_name(base::UTF16ToUTF8(entry.key().name()));
autofill->set_value(base::UTF16ToUTF8(entry.key().value()));
autofill->add_usage_timestamp(entry.date_created().ToInternalValue());
@@ -62,6 +80,191 @@ std::string GetStorageKeyFromModel(const AutofillKey& key) {
base::UTF16ToUTF8(key.value()));
}
+AutofillEntry MergeEntryDates(const AutofillEntry& entry1,
+ const AutofillEntry& entry2) {
+ DCHECK(entry1.key() == entry2.key());
+ return AutofillEntry(
+ entry1.key(), std::min(entry1.date_created(), entry2.date_created()),
+ std::max(entry1.date_last_used(), entry2.date_last_used()));
+}
+
+bool ParseStorageKey(const std::string& storage_key, AutofillKey* out_key) {
+ AutofillSyncStorageKey proto;
+ if (proto.ParseFromString(storage_key)) {
+ *out_key = AutofillKey(base::UTF8ToUTF16(proto.name()),
+ base::UTF8ToUTF16((proto.value())));
+ return true;
+ } else {
+ return false;
+ }
+}
+
+AutofillEntry CreateAutofillEntry(const AutofillSpecifics& autofill_specifics) {
+ AutofillKey key(base::UTF8ToUTF16(autofill_specifics.name()),
+ base::UTF8ToUTF16(autofill_specifics.value()));
+ Time date_created, date_last_used;
+ const google::protobuf::RepeatedField<int64_t>& timestamps =
+ autofill_specifics.usage_timestamp();
+ if (!timestamps.empty()) {
+ auto iter_pair = std::minmax_element(timestamps.begin(), timestamps.end());
+ date_created = Time::FromInternalValue(*iter_pair.first);
+ date_last_used = Time::FromInternalValue(*iter_pair.second);
+ }
+ return AutofillEntry(key, date_created, date_last_used);
+}
+
+// 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 SyncDifferenceTracker {
+ public:
+ explicit SyncDifferenceTracker(AutofillTable* table) : table_(table) {}
+
+ Optional<ModelError> IncorporateRemoteSpecifics(
+ const std::string& storage_key,
+ const AutofillSpecifics& specifics) {
+ if (!specifics.has_value()) {
+ // A long time ago autofill had a different format, and it's possible we
+ // could encounter some of that legacy data. It is not useful to us,
+ // because an autofill entry with no value will not place any text in a
+ // form for the user. So drop all of these on the floor.
+ DVLOG(1) << "Dropping old-style autofill profile change.";
+ return {};
+ }
+
+ const AutofillEntry remote = CreateAutofillEntry(specifics);
+ DCHECK_EQ(storage_key, GetStorageKeyFromModel(remote.key()));
+
+ Optional<AutofillEntry> local;
+ if (!ReadEntry(remote.key(), &local)) {
+ return ModelError(FROM_HERE, "Failed reading from WebDatabase.");
+ } else if (!local) {
+ save_to_local_.push_back(remote);
+ } else if (remote != local.value()) {
+ if (specifics.usage_timestamp().empty()) {
+ // Skip merging if there are no timestamps. We don't want to wipe out
+ // a local value of |date_created| if the remote copy is oddly formed.
+ save_to_sync_.push_back(local.value());
+ } else {
+ const AutofillEntry merged = MergeEntryDates(local.value(), remote);
+ save_to_local_.push_back(merged);
+ save_to_sync_.push_back(merged);
+ }
+ unique_to_local_.erase(local.value());
+ }
+ return {};
+ }
+
+ Optional<ModelError> IncorporateRemoteDelete(const std::string& storage_key) {
+ AutofillKey key;
+ if (!ParseStorageKey(storage_key, &key)) {
+ return ModelError(FROM_HERE, "Failed parsing storage key.");
+ }
+ delete_from_local_.insert(key);
+ return {};
+ }
+
+ Optional<ModelError> FlushToLocal(AutofillWebDataBackend* web_data_backend) {
+ for (const AutofillKey& key : delete_from_local_) {
+ if (!table_->RemoveFormElement(key.name(), key.value())) {
+ return ModelError(FROM_HERE, "Failed deleting from WebDatabase");
+ }
+ }
+ if (!table_->UpdateAutofillEntries(save_to_local_)) {
+ return ModelError(FROM_HERE, "Failed updating WebDatabase");
+ }
+ if (!delete_from_local_.empty() || !save_to_local_.empty()) {
+ web_data_backend->NotifyOfMultipleAutofillChanges();
+ }
+ return {};
+ }
+
+ Optional<ModelError> FlushToSync(
+ bool include_local_only,
+ std::unique_ptr<MetadataChangeList> metadata_change_list,
+ ModelTypeChangeProcessor* change_processor) {
+ for (const AutofillEntry& entry : save_to_sync_) {
+ change_processor->Put(GetStorageKeyFromModel(entry.key()),
+ CreateEntityData(entry),
+ metadata_change_list.get());
+ }
+ if (include_local_only) {
+ if (!InitializeIfNeeded()) {
+ return ModelError(FROM_HERE, "Failed reading from WebDatabase.");
+ }
+ for (const AutofillEntry& entry : unique_to_local_) {
+ // This should never be true because only ApplySyncChanges should be
+ // calling IncorporateRemoteDelete, while only MergeSyncData should be
+ // passing in true for |include_local_only|. If this requirement
+ // changes, this DCHECK can change to act as a filter.
+ DCHECK(delete_from_local_.find(entry.key()) ==
+ delete_from_local_.end());
+ change_processor->Put(GetStorageKeyFromModel(entry.key()),
+ CreateEntityData(entry),
+ metadata_change_list.get());
+ }
+ }
+ return static_cast<AutofillMetadataChangeList*>(metadata_change_list.get())
+ ->TakeError();
+ }
+
+ private:
+ // There are three major outcomes of this method.
+ // 1. An error is encountered reading from the db, false is returned.
+ // 2. The entry is not found, |entry| will not be touched.
+ // 3. The entry is found, |entry| will be set.
+ bool ReadEntry(const AutofillKey& key, Optional<AutofillEntry>* entry) {
+ if (!InitializeIfNeeded()) {
+ return false;
+ }
+ auto iter = unique_to_local_.find(AutofillEntry(key, Time(), Time()));
+ if (iter != unique_to_local_.end()) {
+ *entry = *iter;
+ }
+ return true;
+ }
+
+ bool InitializeIfNeeded() {
+ if (initialized_) {
+ return true;
+ }
+
+ std::vector<AutofillEntry> vector;
+ if (!table_->GetAllAutofillEntries(&vector)) {
+ return false;
+ }
+
+ unique_to_local_ = std::set<AutofillEntry>(vector.begin(), vector.end());
+ initialized_ = true;
+ return true;
+ }
+
+ AutofillTable* table_;
+
+ // This class attempts to lazily load data from |table_|. This field tracks
+ // if that has happened or not yet. To facilitate this, the first usage of
+ // |unique_to_local_| should typically be done through ReadEntry().
+ bool initialized_ = false;
+
+ // Important to note that because AutofillEntry's operator < simply compares
+ // contained AutofillKeys, this acts as a map<AutofillKey, AutofillEntry>.
+ // Shouldn't be accessed until either ReadEntry() or InitializeIfNeeded() is
+ // called, afterward it will start with all the local data. As sync data is
+ // encountered entries are removed from here, leaving only entries that exist
+ // solely on the local client.
+ std::set<AutofillEntry> unique_to_local_;
+
+ std::set<AutofillKey> delete_from_local_;
+ std::vector<AutofillEntry> save_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<AutofillEntry> save_to_sync_;
+
+ DISALLOW_COPY_AND_ASSIGN(SyncDifferenceTracker);
+};
+
} // namespace
// static
@@ -71,8 +274,7 @@ void AutocompleteSyncBridge::CreateForWebDataServiceAndBackend(
web_data_service->GetDBUserData()->SetUserData(
UserDataKey(),
new AutocompleteSyncBridge(
- web_data_backend,
- base::Bind(&syncer::ModelTypeChangeProcessor::Create)));
+ web_data_backend, base::Bind(&ModelTypeChangeProcessor::Create)));
}
// static
@@ -97,26 +299,54 @@ AutocompleteSyncBridge::~AutocompleteSyncBridge() {
DCHECK(thread_checker_.CalledOnValidThread());
}
-std::unique_ptr<syncer::MetadataChangeList>
+std::unique_ptr<MetadataChangeList>
AutocompleteSyncBridge::CreateMetadataChangeList() {
DCHECK(thread_checker_.CalledOnValidThread());
return base::MakeUnique<AutofillMetadataChangeList>(GetAutofillTable(),
syncer::AUTOFILL);
}
-base::Optional<syncer::ModelError> AutocompleteSyncBridge::MergeSyncData(
- std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
- syncer::EntityDataMap entity_data_map) {
+Optional<syncer::ModelError> AutocompleteSyncBridge::MergeSyncData(
+ std::unique_ptr<MetadataChangeList> metadata_change_list,
+ EntityDataMap entity_data_map) {
DCHECK(thread_checker_.CalledOnValidThread());
- NOTIMPLEMENTED();
+
+ // TODO(skym, crbug.com/680218): Uncomment and add unit tests.
+ /*SyncDifferenceTracker tracker(GetAutofillTable());
+ for (auto kv : entity_data_map) {
+ DCHECK(kv.second->specifics.has_autofill());
+ RETURN_IF_ERROR(tracker.IncorporateRemoteSpecifics(
+ kv.first, kv.second->specifics.autofill()));
+ }
+
+ RETURN_IF_ERROR(tracker.FlushToLocal(web_data_backend_));
+ RETURN_IF_ERROR(tracker.FlushToSync(true, std::move(metadata_change_list),
+ change_processor()));
+ web_data_backend_->RemoveExpiredFormElements();
+ web_data_backend_->NotifyThatSyncHasStarted(syncer::AUTOFILL);*/
return {};
}
-base::Optional<syncer::ModelError> AutocompleteSyncBridge::ApplySyncChanges(
- std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
- syncer::EntityChangeList entity_changes) {
+Optional<ModelError> AutocompleteSyncBridge::ApplySyncChanges(
+ std::unique_ptr<MetadataChangeList> metadata_change_list,
+ EntityChangeList entity_changes) {
DCHECK(thread_checker_.CalledOnValidThread());
- NOTIMPLEMENTED();
+
+ SyncDifferenceTracker tracker(GetAutofillTable());
+ for (const EntityChange& change : entity_changes) {
+ if (change.type() == EntityChange::ACTION_DELETE) {
+ RETURN_IF_ERROR(tracker.IncorporateRemoteDelete(change.storage_key()));
+ } else {
+ DCHECK(change.data().specifics.has_autofill());
+ RETURN_IF_ERROR(tracker.IncorporateRemoteSpecifics(
+ change.storage_key(), change.data().specifics.autofill()));
+ }
+ }
+
+ RETURN_IF_ERROR(tracker.FlushToLocal(web_data_backend_));
+ RETURN_IF_ERROR(tracker.FlushToSync(false, std::move(metadata_change_list),
+ change_processor()));
+ web_data_backend_->RemoveExpiredFormElements();
return {};
}
@@ -131,12 +361,9 @@ void AutocompleteSyncBridge::AutocompleteSyncBridge::GetData(
return;
}
- std::unordered_set<std::string> keys_set;
- for (const auto& key : storage_keys) {
- keys_set.insert(key);
- }
-
- auto batch = base::MakeUnique<syncer::MutableDataBatch>();
+ std::unordered_set<std::string> keys_set(storage_keys.begin(),
+ storage_keys.end());
+ auto batch = base::MakeUnique<MutableDataBatch>();
for (const AutofillEntry& entry : entries) {
std::string key = GetStorageKeyFromModel(entry.key());
if (keys_set.find(key) != keys_set.end()) {
@@ -156,7 +383,7 @@ void AutocompleteSyncBridge::GetAllData(DataCallback callback) {
return;
}
- auto batch = base::MakeUnique<syncer::MutableDataBatch>();
+ auto batch = base::MakeUnique<MutableDataBatch>();
for (const AutofillEntry& entry : entries) {
batch->Put(GetStorageKeyFromModel(entry.key()), CreateEntityData(entry));
}
@@ -164,9 +391,9 @@ void AutocompleteSyncBridge::GetAllData(DataCallback callback) {
}
std::string AutocompleteSyncBridge::GetClientTag(
- const syncer::EntityData& entity_data) {
+ const EntityData& entity_data) {
DCHECK(entity_data.specifics.has_autofill());
- const sync_pb::AutofillSpecifics specifics = entity_data.specifics.autofill();
+ const AutofillSpecifics specifics = entity_data.specifics.autofill();
return std::string(kAutocompleteEntryNamespaceTag) +
net::EscapePath(specifics.name()) +
std::string(kAutocompleteTagDelimiter) +
@@ -174,9 +401,9 @@ std::string AutocompleteSyncBridge::GetClientTag(
}
std::string AutocompleteSyncBridge::GetStorageKey(
- const syncer::EntityData& entity_data) {
+ const EntityData& entity_data) {
DCHECK(entity_data.specifics.has_autofill());
- const sync_pb::AutofillSpecifics specifics = entity_data.specifics.autofill();
+ const AutofillSpecifics specifics = entity_data.specifics.autofill();
return BuildSerializedStorageKey(specifics.name(), specifics.value());
}
@@ -186,21 +413,6 @@ void AutocompleteSyncBridge::AutofillEntriesChanged(
DCHECK(thread_checker_.CalledOnValidThread());
}
-// static
-AutofillEntry AutocompleteSyncBridge::CreateAutofillEntry(
- const sync_pb::AutofillSpecifics& autofill_specifics) {
- AutofillKey key(base::UTF8ToUTF16(autofill_specifics.name()),
- base::UTF8ToUTF16(autofill_specifics.value()));
- base::Time date_created, date_last_used;
- const google::protobuf::RepeatedField<int64_t>& timestamps =
- autofill_specifics.usage_timestamp();
- if (!timestamps.empty()) {
- date_created = base::Time::FromInternalValue(*timestamps.begin());
- date_last_used = base::Time::FromInternalValue(*timestamps.rbegin());
- }
- return AutofillEntry(key, date_created, date_last_used);
-}
-
AutofillTable* AutocompleteSyncBridge::GetAutofillTable() const {
return AutofillTable::FromWebDatabase(web_data_backend_->GetDatabase());
}

Powered by Google App Engine
This is Rietveld 408576698