Handle duplicate password sync nodes

chrome/browser/sync/glue/password_change_processor.cc

 // Copyright (c) 2011 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.
 
 #include "chrome/browser/sync/glue/password_change_processor.h"
 
 #include <string>
 
 #include "base/location.h"
 #include "base/string_util.h"
@@ skipping 63 matching lines @@
   }
 
   PasswordStoreChangeList* changes =
       content::Details<PasswordStoreChangeList>(details).ptr();
   for (PasswordStoreChangeList::iterator change = changes->begin();
        change != changes->end(); ++change) {
     std::string tag = PasswordModelAssociator::MakeTag(change->form());
     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);
         int64 sync_id = model_associator_->GetSyncIdFromChromeId(tag);
         if (sync_api::kInvalidId == sync_id) {
           error_handler()->OnUnrecoverableError(FROM_HERE,
               "Unexpected notification for: ");
           return;
         } else {
           if (!sync_node.InitByIdLookup(sync_id)) {
@@ skipping 129 matching lines @@
 
 void PasswordChangeProcessor::StopObserving() {
   DCHECK(expected_loop_ == MessageLoop::current());
   notification_registrar_.Remove(
       this,
       chrome::NOTIFICATION_LOGINS_CHANGED,
       content::Source<PasswordStore>(password_store_));
 }
 
 }  // namespace browser_sync