WIP: The Bookmark Position Megapatch

sync/engine/process_commit_response_command.cc

 // Copyright (c) 2012 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 "sync/engine/process_commit_response_command.h"
 
 #include <cstddef>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/location.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/engine/syncer_util.h"
+#include "sync/internal_api/public/base/unique_position.h"
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/read_transaction.h"
 #include "sync/syncable/syncable_proto_util.h"
 #include "sync/syncable/syncable_util.h"
 #include "sync/syncable/write_transaction.h"
 #include "sync/util/time.h"
 
-// TODO(vishwath): Remove this include after node positions have
-// shifted to completely using Ordinals.
-// See http://crbug.com/145412 .
-#include "sync/internal_api/public/base/node_ordinal.h"
-
 using std::set;
 using std::string;
 using std::vector;
 using sync_pb::CommitResponse;
 
 namespace syncer {
 
 using sessions::OrderedCommitSet;
 using sessions::StatusController;
 using sessions::SyncSession;
 using syncable::WriteTransaction;
 using syncable::MutableEntry;
 using syncable::Entry;
 using syncable::BASE_VERSION;
 using syncable::GET_BY_ID;
 using syncable::ID;
 using syncable::IS_DEL;
 using syncable::IS_DIR;
 using syncable::IS_UNAPPLIED_UPDATE;
 using syncable::IS_UNSYNCED;
 using syncable::PARENT_ID;
 using syncable::SERVER_IS_DEL;
 using syncable::SERVER_PARENT_ID;
-using syncable::SERVER_ORDINAL_IN_PARENT;
 using syncable::SERVER_VERSION;
 using syncable::SYNCER;
 using syncable::SYNCING;
 
 ProcessCommitResponseCommand::ProcessCommitResponseCommand(
       const sessions::OrderedCommitSet& commit_set,
       const sync_pb::ClientToServerMessage& commit_message,
       const sync_pb::ClientToServerResponse& commit_response)
   : commit_set_(commit_set),
     commit_message_(commit_message),
@@ skipping 290 matching lines @@
 
   local_entry->Put(syncable::SERVER_IS_DIR,
       (committed_entry.folder() ||
        committed_entry.bookmarkdata().bookmark_folder()));
   local_entry->Put(syncable::SERVER_SPECIFICS,
       committed_entry.specifics());
   local_entry->Put(syncable::SERVER_MTIME,
                    ProtoTimeToTime(committed_entry.mtime()));
   local_entry->Put(syncable::SERVER_CTIME,
                    ProtoTimeToTime(committed_entry.ctime()));
-  local_entry->Put(syncable::SERVER_ORDINAL_IN_PARENT,
-                   Int64ToNodeOrdinal(entry_response.position_in_parent()));
+  if (committed_entry.has_unique_position()) {
+    local_entry->Put(syncable::SERVER_UNIQUE_POSITION,
+                     UniquePosition::FromBytes(
+                         committed_entry.unique_position()));
+  }
 
   // TODO(nick): The server doesn't set entry_response.server_parent_id in
   // practice; to update SERVER_PARENT_ID appropriately here we'd need to
   // get the post-commit ID of the parent indicated by
   // committed_entry.parent_id_string(). That should be inferrable from the
   // information we have, but it's a bit convoluted to pull it out directly.
   // Getting this right is important: SERVER_PARENT_ID gets fed back into
   // old_parent_id during the next commit.
   local_entry->Put(syncable::SERVER_PARENT_ID,
       local_entry->Get(syncable::PARENT_ID));
@@ skipping 22 matching lines @@
   const string& server_name =
       GetResultingPostCommitName(committed_entry, entry_response);
   const string& old_name =
       local_entry->Get(syncable::NON_UNIQUE_NAME);
 
   if (!server_name.empty() && old_name != server_name) {
     DVLOG(1) << "During commit, server changed name: " << old_name
              << " to new name: " << server_name;
     local_entry->Put(syncable::NON_UNIQUE_NAME, server_name);
   }
-
-  // The server has the final say on positioning, so apply the absolute
-  // position that it returns.
-  if (entry_response.has_position_in_parent()) {
-    // The SERVER_ field should already have been written.
-    DCHECK_EQ(entry_response.position_in_parent(),
-              NodeOrdinalToInt64(local_entry->Get(SERVER_ORDINAL_IN_PARENT)));
-
-    // We just committed successfully, so we assume that the position
-    // value we got applies to the PARENT_ID we submitted.
-    syncable::Id new_prev = local_entry->ComputePrevIdFromServerPosition(
-        local_entry->Get(PARENT_ID));
-    if (!local_entry->PutPredecessor(new_prev)) {
-      // TODO(lipalani) : Propagate the error to caller. crbug.com/100444.
-      NOTREACHED();
-    }
-  }
 }
 
 void ProcessCommitResponseCommand::ProcessSuccessfulCommitResponse(
     const sync_pb::SyncEntity& committed_entry,
     const sync_pb::CommitResponse_EntryResponse& entry_response,
     const syncable::Id& pre_commit_id, syncable::MutableEntry* local_entry,
     bool syncing_was_set, set<syncable::Id>* deleted_folders) {
   DCHECK(local_entry->Get(IS_UNSYNCED));
 
   // Update SERVER_VERSION and BASE_VERSION.
@@ skipping 28 matching lines @@
   // been recursively deleted.
   // TODO(nick): Here, commit_message.deleted() would be more correct than
   // local_entry->Get(IS_DEL).  For example, an item could be renamed, and then
   // deleted during the commit of the rename.  Unit test & fix.
   if (local_entry->Get(IS_DIR) && local_entry->Get(IS_DEL)) {
     deleted_folders->insert(local_entry->Get(ID));
   }
 }
 
 }  // namespace syncer