Fix syncing deleted autofills and properly associate when an autofill is added.

BUG=39957
TEST=Manually test.

Review URL: http://codereview.chromium.org/1546009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43445 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 37 insertions, 23 deletions

diff --git a/chrome/browser/sync/glue/autofill_change_processor.cc b/chrome/browser/sync/glue/autofill_change_processor.cc
index a8ffd9b..d8e9345 100644
--- a/chrome/browser/sync/glue/autofill_change_processor.cc
+++ b/chrome/browser/sync/glue/autofill_change_processor.cc
@@ -167,39 +167,46 @@ void AutofillChangeProcessor::ApplyChangesFromSyncModel(
 
   std::vector<AutofillEntry> new_entries;
   for (int i = 0; i < change_count; ++i) {
-    sync_api::ReadNode sync_node(trans);
-    if (!sync_node.InitByIdLookup(changes[i].id)) {
-      LOG(ERROR) << "Autofill node lookup failed.";
-      error_handler()->OnUnrecoverableError();
-      return;
-    }
-
-    // Check that the changed node is a child of the autofills folder.
-    DCHECK(autofill_root.GetId() == sync_node.GetParentId());
-    DCHECK(syncable::AUTOFILL == sync_node.GetModelType());
-
-    const sync_pb::AutofillSpecifics& autofill(
-        sync_node.GetAutofillSpecifics());
     if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE ==
         changes[i].action) {
-      if (!web_database_->RemoveFormElement(
-              UTF8ToUTF16(autofill.name()), UTF8ToUTF16(autofill.value()))) {
+      const AutofillKey* key =
+        model_associator_->GetChromeNodeFromSyncId(changes[i].id);
+      if (!key || !web_database_->RemoveFormElement(key->name(),
+                                                    key->value())) {
         LOG(ERROR) << "Could not remove autofill node.";
         error_handler()->OnUnrecoverableError();
         return;
       }
+      model_associator_->Disassociate(changes[i].id);
     } else {
+      sync_api::ReadNode sync_node(trans);
+      if (!sync_node.InitByIdLookup(changes[i].id)) {
+        LOG(ERROR) << "Autofill node lookup failed.";
+        error_handler()->OnUnrecoverableError();
+        return;
+      }
+
+      // Check that the changed node is a child of the autofills folder.
+      DCHECK(autofill_root.GetId() == sync_node.GetParentId());
+      DCHECK(syncable::AUTOFILL == sync_node.GetModelType());
+
+      const sync_pb::AutofillSpecifics& autofill(
+          sync_node.GetAutofillSpecifics());
       std::vector<base::Time> timestamps;
       size_t timestamps_size = autofill.usage_timestamp_size();
       for (size_t c = 0; c < timestamps_size; ++c) {
         timestamps.push_back(
             base::Time::FromInternalValue(autofill.usage_timestamp(c)));
       }
-      AutofillEntry new_entry(AutofillKey(UTF8ToUTF16(autofill.name()),
-                                          UTF8ToUTF16(autofill.value())),
-                              timestamps);
+      AutofillKey key(UTF8ToUTF16(autofill.name()),
+                      UTF8ToUTF16(autofill.value()));
 
-      new_entries.push_back(new_entry);
+      new_entries.push_back(AutofillEntry(key, timestamps));
+
+      if (sync_api::SyncManager::ChangeRecord::ACTION_ADD ==
+          changes[i].action) {
+        model_associator_->Associate(&key, sync_node.GetId());
+      }
     }
   }
   if (!web_database_->UpdateAutofillEntries(new_entries)) {
diff --git a/chrome/browser/sync/glue/autofill_model_associator.cc b/chrome/browser/sync/glue/autofill_model_associator.cc
index 25175e7..54f254b 100644
--- a/chrome/browser/sync/glue/autofill_model_associator.cc
+++ b/chrome/browser/sync/glue/autofill_model_associator.cc
@@ -207,6 +207,16 @@ void AutofillModelAssociator::Associate(
   id_map_inverse_[sync_id] = *autofill;
 }
 
+const AutofillKey* AutofillModelAssociator::GetChromeNodeFromSyncId(
+                       int64 sync_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
+  SyncIdToAutofillMap::iterator iter = id_map_inverse_.find(sync_id);
+  if (iter == id_map_inverse_.end())
+    return NULL;
+
+  return &(iter->second);
+}
+
 void AutofillModelAssociator::Disassociate(int64 sync_id) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
   SyncIdToAutofillMap::iterator iter = id_map_inverse_.find(sync_id);
diff --git a/chrome/browser/sync/glue/autofill_model_associator.h b/chrome/browser/sync/glue/autofill_model_associator.h
index b9031ed..eb279ed 100644
--- a/chrome/browser/sync/glue/autofill_model_associator.h
+++ b/chrome/browser/sync/glue/autofill_model_associator.h
@@ -56,10 +56,7 @@ class AutofillModelAssociator
   // user-defined autofill entries.
   virtual bool ChromeModelHasUserCreatedNodes(bool* has_nodes);
 
-  // Not implemented.
-  virtual const AutofillKey* GetChromeNodeFromSyncId(int64 sync_id) {
-    return NULL;
-  }
+  virtual const AutofillKey* GetChromeNodeFromSyncId(int64 sync_id);
 
   // Not implemented.
   virtual bool InitSyncNodeFromChromeId(AutofillKey node_id,