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

Unified Diff: chrome/browser/sync/glue/password_change_processor.cc

Issue 8862005: Handle duplicate password sync nodes (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Use dictionary words in logs Created 9 years 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sync/glue/password_change_processor.cc
diff --git a/chrome/browser/sync/glue/password_change_processor.cc b/chrome/browser/sync/glue/password_change_processor.cc
index 180a79610e9452eac8330bb3215faa9fa2b3ce48..432b293d1381237b5f40f47a850daa7f86d17996 100644
--- a/chrome/browser/sync/glue/password_change_processor.cc
+++ b/chrome/browser/sync/glue/password_change_processor.cc
@@ -81,16 +81,36 @@ void PasswordChangeProcessor::Observe(
switch (change->type()) {
case PasswordStoreChange::ADD: {
sync_api::WriteNode sync_node(&trans);
- if (!sync_node.InitUniqueByCreation(syncable::PASSWORDS,
- password_root, tag)) {
- error_handler()->OnUnrecoverableError(FROM_HERE,
- "Failed to create password sync node.");
- return;
+ if (sync_node.InitUniqueByCreation(syncable::PASSWORDS,
+ password_root, tag)) {
+ PasswordModelAssociator::WriteToSyncNode(change->form(), &sync_node);
+ model_associator_->Associate(&tag, sync_node.GetId());
+ break;
+ } else {
+ // Maybe this node already exists and we should update it.
+ //
+ // If the PasswordStore is told to add an entry but an entry with the
+ // same name already exists, it will overwrite it. It will report
+ // this change as an ADD rather than an UPDATE. Ideally, it would be
+ // able to tell us what action was actually taken, rather than what
+ // action was requested. If it did so, we wouldn't need to fall back
+ // to trying to update an existing password node here.
+ //
+ // TODO: Remove this. See crbug.com/87855.
+ int64 sync_id = model_associator_->GetSyncIdFromChromeId(tag);
+ if (sync_api::kInvalidId == sync_id) {
+ error_handler()->OnUnrecoverableError(FROM_HERE,
+ "Unable to create or retrieve password node");
+ return;
+ }
+ if (!sync_node.InitByIdLookup(sync_id)) {
+ error_handler()->OnUnrecoverableError(FROM_HERE,
+ "Unable to create or retrieve password node");
+ return;
+ }
+ PasswordModelAssociator::WriteToSyncNode(change->form(), &sync_node);
+ break;
}
-
- PasswordModelAssociator::WriteToSyncNode(change->form(), &sync_node);
- model_associator_->Associate(&tag, sync_node.GetId());
- break;
}
case PasswordStoreChange::UPDATE: {
sync_api::WriteNode sync_node(&trans);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698