[Sync] Fix undelete conflict resolution

Previously we were recreating bookmarks when undeleting, and resetting the
version of non-bookmarks. Neither of these are necessary. We now reuse the
current version (so the server knows we are the most up to date versin)
and no longer re-creae bookmarks (because we reuse the existing id, the
write is treated as a modify, not a create).

BUG=154884
TEST=with three signed in clients A, B, C, delete a bookmark on A. On B and C,
before they recieve the notification, modify that same bookmark. Ensure no
duplicates are created.


Review URL: https://chromiumcodereview.appspot.com/11204002

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

2 files changed, 15 insertions, 30 deletions

diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index eec418a..c6806be 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -244,36 +244,16 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
     }
 
     // The entry is deleted on the server but still exists locally.
-    if (!entry.Get(syncable::UNIQUE_CLIENT_TAG).empty()) {
-      // If we've got a client-unique tag, we can undelete while retaining
-      // our present ID.
-      DCHECK_EQ(entry.Get(syncable::SERVER_VERSION), 0) << "For the server to "
-          "know to re-create, client-tagged items should revert to version 0 "
-          "when server-deleted.";
-      OverwriteServerChanges(&entry);
-      status->increment_num_server_overwrites();
-      DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
-               << entry;
-      UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
-                                OVERWRITE_SERVER,
-                                CONFLICT_RESOLUTION_SIZE);
-      // Clobber the versions, just in case the above DCHECK is violated.
-      entry.Put(syncable::SERVER_VERSION, 0);
-      entry.Put(syncable::BASE_VERSION, 0);
-    } else {
-      // Otherwise, we've got to undelete by creating a new locally
-      // uncommitted entry.
-      SplitServerInformationIntoNewEntry(trans, &entry);
+    // We undelete it by overwriting the server's tombstone with the local
+    // data.
+    OverwriteServerChanges(&entry);
+    status->increment_num_server_overwrites();
+    DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
+             << entry;
+    UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
+                              UNDELETE,
+                              CONFLICT_RESOLUTION_SIZE);
 
-      MutableEntry server_update(trans, syncable::GET_BY_ID, id);
-      CHECK(server_update.good());
-      CHECK(server_update.Get(syncable::META_HANDLE) !=
-            entry.Get(syncable::META_HANDLE))
-          << server_update << entry;
-      UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
-                                UNDELETE,
-                                CONFLICT_RESOLUTION_SIZE);
-    }
     return SYNC_PROGRESS;
   }
 }
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 0de57ca..feef945 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -2864,6 +2864,9 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) {
   saw_syncer_event_ = false;
 }
 
+// When we undelete an entity as a result of conflict resolution, we reuse the
+// existing server id and preserve the old version, simply updating the server
+// version with the new non-deleted entity.
 TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
   int64 bob_metahandle;
 
@@ -2886,9 +2889,11 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
     Entry bob(&trans, GET_BY_HANDLE, bob_metahandle);
     ASSERT_TRUE(bob.good());
     EXPECT_TRUE(bob.Get(IS_UNSYNCED));
-    EXPECT_FALSE(bob.Get(ID).ServerKnows());
+    EXPECT_TRUE(bob.Get(ID).ServerKnows());
     EXPECT_FALSE(bob.Get(IS_UNAPPLIED_UPDATE));
     EXPECT_FALSE(bob.Get(IS_DEL));
+    EXPECT_EQ(2, bob.Get(SERVER_VERSION));
+    EXPECT_EQ(2, bob.Get(BASE_VERSION));
   }
   saw_syncer_event_ = false;
 }