sync: Start moving away from PREV_ID and NEXT_ID

This change replaces serveral instances of Get(PREV_ID) and Get(NEXT_ID)
with calls to the newly introduced GetPredecessorId() and
GetSuccessorId().

This is done in anticipation of the change that will remove PREV_ID and
NEXT_ID entirely.  By making this change in advance, that patch will be
smaller and easier to review.

BUG=145412,126505


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

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

14 files changed, 46 insertions, 38 deletions

diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc
index 7ecf152..9d1a4b9 100644
--- a/sync/engine/build_commit_command.cc
+++ b/sync/engine/build_commit_command.cc
@@ -197,7 +197,7 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
     } else {
       if (meta_entry.Get(SPECIFICS).has_bookmark()) {
         // Common data in both new and old protocol.
-        const Id& prev_id = meta_entry.Get(syncable::PREV_ID);
+        const Id& prev_id = meta_entry.GetPredecessorId();
         string prev_id_string =
             prev_id.IsRoot() ? string() : prev_id.GetServerId();
         sync_entry->set_insert_after_item_id(prev_id_string);
diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 6e2eddd..5b485a1 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -140,10 +140,10 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
     syncable::Id server_prev_id = entry.ComputePrevIdFromServerPosition(
         entry.Get(syncable::SERVER_PARENT_ID));
     bool needs_reinsertion = !parent_matches ||
-         server_prev_id != entry.Get(syncable::PREV_ID);
+         server_prev_id != entry.GetPredecessorId();
     DVLOG_IF(1, needs_reinsertion) << "Insertion needed, server prev id "
         << " is " << server_prev_id << ", local prev id is "
-        << entry.Get(syncable::PREV_ID);
+        << entry.GetPredecessorId();
     const sync_pb::EntitySpecifics& specifics =
         entry.Get(syncable::SPECIFICS);
     const sync_pb::EntitySpecifics& server_specifics =
@@ -288,7 +288,7 @@ void ConflictResolver::ResolveConflicts(
       Entry entry(trans, syncable::GET_BY_ID, prev_id);
       // Any entry in conflict must be valid.
       CHECK(entry.good());
-      Id new_prev_id = entry.Get(syncable::PREV_ID);
+      Id new_prev_id = entry.GetPredecessorId();
       if (new_prev_id == prev_id)
         break;
       prev_id = new_prev_id;
diff --git a/sync/engine/get_commit_ids_command.cc b/sync/engine/get_commit_ids_command.cc
index 4997239..b701ea5 100644
--- a/sync/engine/get_commit_ids_command.cc
+++ b/sync/engine/get_commit_ids_command.cc
@@ -248,7 +248,7 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
   if (item.Get(syncable::IS_DEL))
     return true;  // Deleted items have no predecessors.
 
-  syncable::Id prev_id = item.Get(syncable::PREV_ID);
+  syncable::Id prev_id = item.GetPredecessorId();
   while (!prev_id.IsRoot()) {
     syncable::Entry prev(trans, syncable::GET_BY_ID, prev_id);
     CHECK(prev.good()) << "Bad id when walking predecessors.";
@@ -262,7 +262,7 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
     }
     if (!AddItem(ready_unsynced_set, prev, result))
       return false;  // Item is in conflict.
-    prev_id = prev.Get(syncable::PREV_ID);
+    prev_id = prev.GetPredecessorId();
   }
   return true;
 }
diff --git a/sync/engine/process_commit_response_command_unittest.cc b/sync/engine/process_commit_response_command_unittest.cc
index f20f86c..f96ea5e 100644
--- a/sync/engine/process_commit_response_command_unittest.cc
+++ b/sync/engine/process_commit_response_command_unittest.cc
@@ -247,22 +247,22 @@ TEST_F(ProcessCommitResponseCommandTest, MultipleCommitIdProjections) {
   Id cid;
   ASSERT_TRUE(directory()->GetFirstChildId(&trans, new_fid, &cid));
   Entry b1(&trans, syncable::GET_BY_ID, cid);
-  Entry b2(&trans, syncable::GET_BY_ID, b1.Get(syncable::NEXT_ID));
+  Entry b2(&trans, syncable::GET_BY_ID, b1.GetSuccessorId());
   CheckEntry(&b1, "bookmark 1", BOOKMARKS, new_fid);
   CheckEntry(&b2, "bookmark 2", BOOKMARKS, new_fid);
-  ASSERT_TRUE(b2.Get(syncable::NEXT_ID).IsRoot());
+  ASSERT_TRUE(b2.GetSuccessorId().IsRoot());
 
   // Look at the prefs and autofill items.
-  Entry p1(&trans, syncable::GET_BY_ID, b_folder.Get(syncable::NEXT_ID));
-  Entry p2(&trans, syncable::GET_BY_ID, p1.Get(syncable::NEXT_ID));
+  Entry p1(&trans, syncable::GET_BY_ID, b_folder.GetSuccessorId());
+  Entry p2(&trans, syncable::GET_BY_ID, p1.GetSuccessorId());
   CheckEntry(&p1, "Pref 1", PREFERENCES, id_factory_.root());
   CheckEntry(&p2, "Pref 2", PREFERENCES, id_factory_.root());
 
-  Entry a1(&trans, syncable::GET_BY_ID, p2.Get(syncable::NEXT_ID));
-  Entry a2(&trans, syncable::GET_BY_ID, a1.Get(syncable::NEXT_ID));
+  Entry a1(&trans, syncable::GET_BY_ID, p2.GetSuccessorId());
+  Entry a2(&trans, syncable::GET_BY_ID, a1.GetSuccessorId());
   CheckEntry(&a1, "Autofill 1", AUTOFILL, id_factory_.root());
   CheckEntry(&a2, "Autofill 2", AUTOFILL, id_factory_.root());
-  ASSERT_TRUE(a2.Get(syncable::NEXT_ID).IsRoot());
+  ASSERT_TRUE(a2.GetSuccessorId().IsRoot());
 }
 
 // In this test, we test processing a commit response for a commit batch that
@@ -371,7 +371,7 @@ TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
         ASSERT_LT(0, c.Get(BASE_VERSION));
       }
     }
-    cid = c.Get(syncable::NEXT_ID);
+    cid = c.GetSuccessorId();
     child_count++;
   }
   ASSERT_EQ(batch_size*2, child_count)
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 2bfcdf8..7fef4f4 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -90,10 +90,8 @@ using syncable::IS_UNAPPLIED_UPDATE;
 using syncable::IS_UNSYNCED;
 using syncable::META_HANDLE;
 using syncable::MTIME;
-using syncable::NEXT_ID;
 using syncable::NON_UNIQUE_NAME;
 using syncable::PARENT_ID;
-using syncable::PREV_ID;
 using syncable::BASE_SERVER_SPECIFICS;
 using syncable::SERVER_IS_DEL;
 using syncable::SERVER_PARENT_ID;
@@ -4509,14 +4507,14 @@ class SyncerPositionUpdateTest : public SyncerTest {
       Id id = i->second;
       Entry entry_with_id(&trans, GET_BY_ID, id);
       EXPECT_TRUE(entry_with_id.good());
-      EXPECT_EQ(prev_id, entry_with_id.Get(PREV_ID));
+      EXPECT_EQ(prev_id, entry_with_id.GetPredecessorId());
       EXPECT_EQ(
           i->first,
           NodeOrdinalToInt64(entry_with_id.Get(SERVER_ORDINAL_IN_PARENT)));
       if (next == position_map_.end()) {
-        EXPECT_EQ(Id(), entry_with_id.Get(NEXT_ID));
+        EXPECT_EQ(Id(), entry_with_id.GetSuccessorId());
       } else {
-        EXPECT_EQ(next->second, entry_with_id.Get(NEXT_ID));
+        EXPECT_EQ(next->second, entry_with_id.GetSuccessorId());
         next++;
       }
       prev_id = id;
@@ -4636,12 +4634,12 @@ class SyncerPositionTiebreakingTest : public SyncerTest {
     EXPECT_TRUE(low.good());
     EXPECT_TRUE(mid.good());
     EXPECT_TRUE(high.good());
-    EXPECT_TRUE(low.Get(PREV_ID) == null_id);
-    EXPECT_TRUE(mid.Get(PREV_ID) == low_id_);
-    EXPECT_TRUE(high.Get(PREV_ID) == mid_id_);
-    EXPECT_TRUE(high.Get(NEXT_ID) == null_id);
-    EXPECT_TRUE(mid.Get(NEXT_ID) == high_id_);
-    EXPECT_TRUE(low.Get(NEXT_ID) == mid_id_);
+    EXPECT_TRUE(low.GetPredecessorId() == null_id);
+    EXPECT_TRUE(mid.GetPredecessorId() == low_id_);
+    EXPECT_TRUE(high.GetPredecessorId() == mid_id_);
+    EXPECT_TRUE(high.GetSuccessorId() == null_id);
+    EXPECT_TRUE(mid.GetSuccessorId() == high_id_);
+    EXPECT_TRUE(low.GetSuccessorId() == mid_id_);
   }
 
  protected:
diff --git a/sync/engine/syncer_util.cc b/sync/engine/syncer_util.cc
index afbea45..3548ba1 100644
--- a/sync/engine/syncer_util.cc
+++ b/sync/engine/syncer_util.cc
@@ -56,7 +56,6 @@ using syncable::MutableEntry;
 using syncable::NON_UNIQUE_NAME;
 using syncable::BASE_SERVER_SPECIFICS;
 using syncable::PARENT_ID;
-using syncable::PREV_ID;
 using syncable::SERVER_CTIME;
 using syncable::SERVER_IS_DEL;
 using syncable::SERVER_IS_DIR;
@@ -462,7 +461,7 @@ bool AddItemThenPredecessors(
   if (item->Get(IS_DEL))
     return true;  // Deleted items have no predecessors.
 
-  Id prev_id = item->Get(PREV_ID);
+  Id prev_id = item->GetPredecessorId();
   while (!prev_id.IsRoot()) {
     Entry prev(trans, GET_BY_ID, prev_id);
     CHECK(prev.good()) << "Bad id when walking predecessors.";
@@ -471,7 +470,7 @@ bool AddItemThenPredecessors(
     if (!inserted_items->insert(prev.Get(META_HANDLE)).second)
       break;
     commit_ids->push_back(prev_id);
-    prev_id = prev.Get(PREV_ID);
+    prev_id = prev.GetPredecessorId();
   }
   return true;
 }
diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc
index f640532..f30c36e 100644
--- a/sync/internal_api/base_node.cc
+++ b/sync/internal_api/base_node.cc
@@ -195,14 +195,14 @@ bool BaseNode::HasChildren() const {
 }
 
 int64 BaseNode::GetPredecessorId() const {
-  syncable::Id id_string = GetEntry()->Get(syncable::PREV_ID);
+  syncable::Id id_string = GetEntry()->GetPredecessorId();
   if (id_string.IsRoot())
     return kInvalidId;
   return IdToMetahandle(GetTransaction()->GetWrappedTrans(), id_string);
 }
 
 int64 BaseNode::GetSuccessorId() const {
-  syncable::Id id_string = GetEntry()->Get(syncable::NEXT_ID);
+  syncable::Id id_string = GetEntry()->GetSuccessorId();
   if (id_string.IsRoot())
     return kInvalidId;
   return IdToMetahandle(GetTransaction()->GetWrappedTrans(), id_string);
@@ -242,7 +242,7 @@ int BaseNode::GetTotalNodeCount() const {
     syncable::Id child_id;
     if (dir->GetFirstChildId(trans, id, &child_id) && !child_id.IsRoot())
       stack.push(IdToMetahandle(trans, child_id));
-    syncable::Id successor_id = entry.Get(syncable::NEXT_ID);
+    syncable::Id successor_id = entry.GetSuccessorId();
     if (!successor_id.IsRoot())
       stack.push(IdToMetahandle(trans, successor_id));
   }
diff --git a/sync/internal_api/change_reorder_buffer.cc b/sync/internal_api/change_reorder_buffer.cc
index e6e4e2f..5358fa2 100644
--- a/sync/internal_api/change_reorder_buffer.cc
+++ b/sync/internal_api/change_reorder_buffer.cc
@@ -216,7 +216,7 @@ bool ChangeReorderBuffer::GetAllChangesInTreeOrder(
         // ordering.
         if (operations_.find(handle) == operations_.end())
           operations_[handle] = OP_UPDATE_POSITION_AND_PROPERTIES;
-        id = child.Get(syncable::NEXT_ID);
+        id = child.GetSuccessorId();
       }
     }
   }
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index 0c1c2ad..2057c25 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -461,7 +461,7 @@ bool WriteNode::SetPosition(const BaseNode& new_parent,
 
   // Filter out redundant changes if both the parent and the predecessor match.
   if (new_parent_id == entry_->Get(syncable::PARENT_ID)) {
-    const syncable::Id& old = entry_->Get(syncable::PREV_ID);
+    const syncable::Id& old = entry_->GetPredecessorId();
     if ((!predecessor && old.IsRoot()) ||
         (predecessor && (old == predecessor->GetEntry()->Get(syncable::ID)))) {
       return true;
diff --git a/sync/syncable/entry.cc b/sync/syncable/entry.cc
index 07ec19a..c6cc4f0 100644
--- a/sync/syncable/entry.cc
+++ b/sync/syncable/entry.cc
@@ -95,6 +95,14 @@ ModelType Entry::GetModelType() const {
   return UNSPECIFIED;
 }
 
+Id Entry::GetPredecessorId() const {
+  return kernel_->ref(PREV_ID);
+}
+
+Id Entry::GetSuccessorId() const {
+  return kernel_->ref(NEXT_ID);
+}
+
 std::ostream& operator<<(std::ostream& s, const Blob& blob) {
   for (Blob::const_iterator i = blob.begin(); i != blob.end(); ++i)
     s << std::hex << std::setw(2)
diff --git a/sync/syncable/entry.h b/sync/syncable/entry.h
index 6714696..e1c8717 100644
--- a/sync/syncable/entry.h
+++ b/sync/syncable/entry.h
@@ -105,6 +105,9 @@ class SYNC_EXPORT Entry {
   ModelType GetServerModelType() const;
   ModelType GetModelType() const;
 
+  Id GetPredecessorId() const;
+  Id GetSuccessorId() const;
+
   inline bool ExistsOnClientBecauseNameIsNonEmpty() const {
     DCHECK(kernel_);
     return !kernel_->ref(NON_UNIQUE_NAME).empty();
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 75e51f1..64a23ce 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -400,7 +400,7 @@ bool MutableEntry::PutPredecessor(const Id& predecessor_id) {
     }
     if (predecessor.Get(PARENT_ID) != Get(PARENT_ID))
       return false;
-    successor_id = predecessor.Get(NEXT_ID);
+    successor_id = predecessor.GetSuccessorId();
     predecessor.Put(NEXT_ID, Get(ID));
   } else {
     syncable::Directory* dir = trans()->directory();
diff --git a/sync/syncable/nigori_util.cc b/sync/syncable/nigori_util.cc
index b0fee50..584adff 100644
--- a/sync/syncable/nigori_util.cc
+++ b/sync/syncable/nigori_util.cc
@@ -151,7 +151,7 @@ bool VerifyDataTypeEncryptionForTest(
       }
     }
     // Push the successor.
-    to_visit.push(child.Get(NEXT_ID));
+    to_visit.push(child.GetSuccessorId());
   }
   return true;
 }
diff --git a/sync/syncable/syncable_util.cc b/sync/syncable/syncable_util.cc
index 0597109..857fc85 100644
--- a/sync/syncable/syncable_util.cc
+++ b/sync/syncable/syncable_util.cc
@@ -78,14 +78,14 @@ void ChangeEntryIDAndUpdateChildren(
   // order.  Do this by reinserting into the linked list; the first
   // step in PutPredecessor is to Unlink from the existing order, which
   // will overwrite the stale Id value from the adjacent nodes.
-  if (entry->Get(PREV_ID) == entry->Get(NEXT_ID) &&
-      entry->Get(PREV_ID) == old_id) {
+  if (entry->GetPredecessorId() == entry->GetSuccessorId() &&
+      entry->GetPredecessorId() == old_id) {
     // We just need a shallow update to |entry|'s fields since it is already
     // self looped.
     entry->Put(NEXT_ID, new_id);
     entry->Put(PREV_ID, new_id);
   } else {
-    entry->PutPredecessor(entry->Get(PREV_ID));
+    entry->PutPredecessor(entry->GetPredecessorId());
   }
 }