WIP: The Bookmark Position Megapatch

sync/internal_api/sync_manager_impl.cc

Index: sync/internal_api/sync_manager_impl.cc
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 874444ba1b552a8ddff4779b8f9cb8734e4c1799..b75ea50651a8221f1a6698001df5b4620010a393 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -56,6 +56,7 @@ namespace syncer {
 using sessions::SyncSessionContext;
 using syncable::ImmutableWriteTransactionInfo;
 using syncable::SPECIFICS;
+using syncable::UNIQUE_POSITION;
 
 namespace {
 
@@ -221,22 +222,6 @@ DictionaryValue* SyncManagerImpl::NotificationInfo::ToValue() const {
   return value;
 }
 
-bool SyncManagerImpl::VisiblePositionsDiffer(
-    const syncable::EntryKernelMutation& mutation) const {
-  const syncable::EntryKernel& a = mutation.original;
-  const syncable::EntryKernel& b = mutation.mutated;
-  // If the datatype isn't one where the browser model cares about position,
-  // don't bother notifying that data model of position-only changes.
-  if (!ShouldMaintainPosition(GetModelTypeFromSpecifics(b.ref(SPECIFICS)))) {
-    return false;
-  }
-  if (a.ref(syncable::NEXT_ID) != b.ref(syncable::NEXT_ID))
-    return true;
-  if (a.ref(syncable::PARENT_ID) != b.ref(syncable::PARENT_ID))
-    return true;
-  return false;
-}
-
 bool SyncManagerImpl::VisiblePropertiesDiffer(
     const syncable::EntryKernelMutation& mutation,
     Cryptographer* cryptographer) const {
@@ -264,7 +249,7 @@ bool SyncManagerImpl::VisiblePropertiesDiffer(
   if (!a_specifics.has_encrypted() && !b_specifics.has_encrypted() &&
       a.ref(syncable::NON_UNIQUE_NAME) != b.ref(syncable::NON_UNIQUE_NAME))
     return true;
-  if (VisiblePositionsDiffer(mutation))
+  if (!a.ref(UNIQUE_POSITION).Equals(b.ref(UNIQUE_POSITION)))
     return true;
   return false;
 }
@@ -902,6 +887,8 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer(
            mutations.Get().begin(); it != mutations.Get().end(); ++it) {
     bool existed_before = !it->second.original.ref(syncable::IS_DEL);
     bool exists_now = !it->second.mutated.ref(syncable::IS_DEL);
+    bool positions_differ = !it->second.original.ref(UNIQUE_POSITION).Equals(
+        it->second.mutated.ref(UNIQUE_POSITION));
 
     // Omit items that aren't associated with a model.
     ModelType type =
@@ -917,7 +904,7 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer(
     else if (exists_now && existed_before &&
              VisiblePropertiesDiffer(it->second, crypto)) {
       change_buffers[type].PushUpdatedItem(
-          handle, VisiblePositionsDiffer(it->second));
+          handle, positions_differ);
     }
 
     SetExtraChangeRecordData(handle, type, &change_buffers[type], crypto,