Commit 7b5f809e authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Sync::USS] Fix DCHECK failure crash in PasswordStoreSync::MergeSyncData

Recent DCHECK failure in DCHECK enabled builds suggest that unexpected
behavior takes place at initial merger where the bridge isn't aware of
all the passwords in the login database.

This CL remove the problematic DCHECK until the underlying cause is
addressed.

More details are in crbug.com/936823

Bug: 936823,933711
Change-Id: I5aa7340e0ebc144d845115764453407495d7249d
Reviewed-on: https://chromium-review.googlesource.com/c/1494878
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636456}
parent dad52a60
......@@ -256,7 +256,11 @@ base::Optional<syncer::ModelError> PasswordSyncBridge::MergeSyncData(
0) {
changes = password_store_sync_->AddLoginSync(
PasswordFromEntityChange(entity_change, /*sync_time=*/time_now));
DCHECK_LE(1U, changes.size());
// TODO(crbug.com/936823): The DCHECK_LE below is legit. However, recent
// crashes suggest that 2 changes are returned in the above call
// (details are in the bug). It should be uncommentted once the
// underlying cause is discovered.
// DCHECK_LE(1U, changes.size());
} else {
changes = password_store_sync_->UpdateLoginSync(
PasswordFromEntityChange(entity_change, /*sync_time=*/time_now));
......@@ -266,6 +270,25 @@ base::Optional<syncer::ModelError> PasswordSyncBridge::MergeSyncData(
return syncer::ModelError(
FROM_HERE, "Failed to add/update an entry in the password store.");
}
// TODO(crbug.com/936823): |changes| should never contain more than one
// change. This code path has been added to address the issue explained in
// the bug. It should be removed once the underlying cause of the issue is
// discovered.
if (changes.size() == 2) {
// In that case the first change indicates a removal of an existing
// password and we aren't interested in it.
DCHECK(changes[0].type() == PasswordStoreChange::REMOVE);
DCHECK(changes[1].type() == PasswordStoreChange::ADD);
change_processor()->UpdateStorageKey(
entity_change.data(),
/*storage_key=*/
base::NumberToString(changes[1].primary_key()),
metadata_change_list.get());
password_store_changes.push_back(changes[1]);
continue;
}
change_processor()->UpdateStorageKey(
entity_change.data(),
/*storage_key=*/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment