Handle duplicate password sync nodes

chrome/browser/sync/glue/password_change_processor.cc

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..c5a1c351bc6e8743ec23e3f0d47f3d223a7a48c6 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 lookup password node");
+            return;
+          }
+          if (!sync_node.InitByIdLookup(sync_id)) {
+            error_handler()->OnUnrecoverableError(FROM_HERE,
+                "Unable to create or lookup password node");

[Mike Mammarella, 2011/12/08 04:32:04]

Grammar nit: "look up" is two words as a verb.

[rlarocque, 2011/12/09 18:53:42]

Done.

+            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);