sync: Remove the remaining conflict sets code

chrome/browser/sync/engine/syncer_util.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/engine/syncer_util.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 315 matching lines @@
     // Passwords use their own legacy encryption scheme.
     const sync_pb::PasswordSpecifics& password =
         specifics.GetExtension(sync_pb::password);
     if (!cryptographer->CanDecrypt(password.encrypted())) {
       DVLOG(1) << "Received an undecryptable password update, returning "
                << "encryption_conflict.";
       return CONFLICT_ENCRYPTION;
     }
   }
 
-  if (entry->Get(IS_UNSYNCED)) {
-    DVLOG(1) << "Skipping update, returning conflict for: " << id
-             << " ; it's unsynced.";
-    return CONFLICT;
-  }
   if (!entry->Get(SERVER_IS_DEL)) {
     syncable::Id new_parent = entry->Get(SERVER_PARENT_ID);
     Entry parent(trans, GET_BY_ID,  new_parent);
     // A note on non-directory parents:
     // We catch most unfixable tree invariant errors at update receipt time,
     // however we deal with this case here because we may receive the child
     // first then the illegal parent. Instead of dealing with it twice in
     // different ways we deal with it once here to reduce the amount of code and
     // potential errors.
     if (!parent.good() || parent.Get(IS_DEL) || !parent.Get(IS_DIR)) {
-      return CONFLICT;
+      return HIERARCHY_CONFLICT;

[rlarocque, 2012/01/31 19:12:17]

There is a subtle side-effect to this change, which affects mostly unit tests.  

The old logic would have treated !parent.good() as though it were a simple
conflict.  This commonly happens in unit tests, so I had to update a bunch of
assertions accordingly.  

In the real world, this could happen if we received updates out of order.  Under
the old system, we would have classified this as a simple conflict, but it would
not have been put into a conflict set.  We would have tried to resolve this as
though it were a simple conflict, wasting CPU cycles and possibly throwing away
a valid server changes if we decided for some reason that this was a "client
wins" case.  

Under the new system, we will treat it as a HIERARCHY conflict, which means will
ignore the update and hope the server sends us a better update soon.

[Nicolas Zea, 2012/02/01 19:16:19]

So I'm clear, the only side effect here is that we no longer invoke the conflict
resolver on this item, right? We'll still attempt to apply this update next time
around (and if the parent has arrived by then the update will apply fine).

Effectively, hierarchy conflict is treated the same as encryption conflict.

[rlarocque, 2012/02/02 00:32:56]

Yes, that is correct.

     }
     if (entry->Get(PARENT_ID) != new_parent) {
       if (!entry->Get(IS_DEL) && !IsLegalNewParent(trans, id, new_parent)) {
         DVLOG(1) << "Not updating item " << id
                  << ", illegal new parent (would cause loop).";
-        return CONFLICT;
+        return HIERARCHY_CONFLICT;
       }
     }
   } else if (entry->Get(IS_DIR)) {
     Directory::ChildHandles handles;
     trans->directory()->GetChildHandlesById(trans, id, &handles);
     if (!handles.empty()) {
       // If we have still-existing children, then we need to deal with
       // them before we can process this change.
       DVLOG(1) << "Not deleting directory; it's not empty " << *entry;
-      return CONFLICT;
+      return HIERARCHY_CONFLICT;
     }
   }
 
+  if (entry->Get(IS_UNSYNCED)) {

[rlarocque, 2012/01/31 19:12:17]

This was moved down here because it's important that HIERARCHY_CONFLICT take
precedence over CONFLICT.

+    DVLOG(1) << "Skipping update, returning conflict for: " << id
+             << " ; it's unsynced.";
+    return CONFLICT;
+  }
+
   if (specifics.has_encrypted()) {
     DVLOG(2) << "Received a decryptable "
              << syncable::ModelTypeToString(entry->GetServerModelType())
              << " update, applying normally.";
   } else {
     DVLOG(2) << "Received an unencrypted "
              << syncable::ModelTypeToString(entry->GetServerModelType())
              << " update, applying normally.";
   }
 
@@ skipping 390 matching lines @@
   if (update.version() < target->Get(SERVER_VERSION)) {
     LOG(WARNING) << "Update older than current server version for "
                  << *target << " Update:"
                  << SyncerProtoUtil::SyncEntityDebugString(update);
     return VERIFY_SUCCESS;  // Expected in new sync protocol.
   }
   return VERIFY_UNDECIDED;
 }
 
 }  // namespace browser_sync