WIP: The Bookmark Position Megapatch

sync/engine/syncer_unittest.cc

Index: sync/engine/syncer_unittest.cc
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index df4196022489bbd6f553bcdc2beb5a524d0ea49c..15564571ad4a46e1e0d420d58068fc76b1af4519 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -33,7 +33,6 @@
 #include "sync/engine/traffic_recorder.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
-#include "sync/internal_api/public/base/node_ordinal.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/nigori_specifics.pb.h"
 #include "sync/protocol/preference_specifics.pb.h"
@@ -58,10 +57,12 @@
 
 using base::TimeDelta;
 
+using std::count;
 using std::map;
 using std::multimap;
 using std::set;
 using std::string;
+using std::vector;
 
 namespace syncer {
 
@@ -78,7 +79,8 @@ using syncable::MutableEntry;
 using syncable::WriteTransaction;
 
 using syncable::BASE_VERSION;
-using syncable::CREATE;
+using syncable::CREATE_BOOKMARK;
+using syncable::CREATE_UNIQUE;
 using syncable::GET_BY_HANDLE;
 using syncable::GET_BY_ID;
 using syncable::GET_BY_CLIENT_TAG;
@@ -90,14 +92,11 @@ 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;
-using syncable::SERVER_ORDINAL_IN_PARENT;
 using syncable::SERVER_SPECIFICS;
 using syncable::SERVER_VERSION;
 using syncable::UNIQUE_CLIENT_TAG;
@@ -357,7 +356,7 @@ class SyncerTest : public testing::Test,
         }
         string utf8_name = test->id.GetServerId();
         string name(utf8_name.begin(), utf8_name.end());
-        MutableEntry entry(&trans, CREATE, test->parent_id, name);
+        MutableEntry entry(&trans, CREATE_BOOKMARK, test->parent_id, name);
 
         entry.Put(syncable::ID, test->id);
         if (test->id.ServerKnows()) {
@@ -460,7 +459,7 @@ class SyncerTest : public testing::Test,
   int64 CreateUnsyncedDirectory(const string& entry_name,
       const syncable::Id& id) {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&wtrans, syncable::CREATE, wtrans.root_id(),
+    MutableEntry entry(&wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(),
                        entry_name);
     EXPECT_TRUE(entry.good());
     entry.Put(syncable::IS_UNSYNCED, true);
@@ -850,9 +849,18 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     EXPECT_TRUE(GetCryptographer(&wtrans)->has_pending_keys());
   }
 
-  mock_server_->AddUpdateSpecifics(1, 0, "A", 10, 10, true, 0, bookmark);
-  mock_server_->AddUpdateSpecifics(2, 1, "B", 10, 10, false, 2, bookmark);
-  mock_server_->AddUpdateSpecifics(3, 1, "C", 10, 10, false, 1, bookmark);
+  // We need to remember the exact position of our local items, so we can
+  // make updates that do not modify those positions.
+  UniquePosition pos1;
+  UniquePosition pos2;
+  UniquePosition pos3;
+
+  mock_server_->AddBookmarkUpdateSpecifics(
+      1, 0, "A", 10, 10, true, 0, bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      2, 1, "B", 10, 10, false, 2, bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      3, 1, "C", 10, 10, false, 1, bookmark);
   mock_server_->AddUpdateSpecifics(4, 0, "D", 10, 10, false, 0, pref);
   SyncShareNudge();
   {
@@ -862,16 +870,25 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     VERIFY_ENTRY(2, false, false, false, 1, 10, 10, ids_, &rtrans);
     VERIFY_ENTRY(3, false, false, false, 1, 10, 10, ids_, &rtrans);
     VERIFY_ENTRY(4, false, false, false, 0, 10, 10, ids_, &rtrans);
+
+    Entry entry1(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1));
+    ASSERT_TRUE(entry1.Get(syncable::UNIQUE_POSITION).Equals(
+        entry1.Get(syncable::SERVER_UNIQUE_POSITION)));
+    pos1 = entry1.Get(syncable::UNIQUE_POSITION);
+    Entry entry2(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(2));
+    pos2 = entry2.Get(syncable::UNIQUE_POSITION);
+    Entry entry3(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(3));
+    pos3 = entry3.Get(syncable::UNIQUE_POSITION);
   }
 
   // Server side encryption will not be applied due to undecryptable data.
   // At this point, BASE_SERVER_SPECIFICS should be filled for all four items.
-  mock_server_->AddUpdateSpecifics(1, 0, kEncryptedString, 20, 20, true, 0,
-                                   encrypted_bookmark);
-  mock_server_->AddUpdateSpecifics(2, 1, kEncryptedString, 20, 20, false, 2,
-                                   encrypted_bookmark);
-  mock_server_->AddUpdateSpecifics(3, 1, kEncryptedString, 20, 20, false, 1,
-                                   encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      1, 0, kEncryptedString, 20, 20, true, pos1, encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      2, 1, kEncryptedString, 20, 20, false, pos2, encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      3, 1, kEncryptedString, 20, 20, false, pos3, encrypted_bookmark);
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 20, 20, false, 0,
                                    encrypted_pref);
   SyncShareNudge();
@@ -887,10 +904,10 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
 
   // Server side change that don't modify anything should not affect
   // BASE_SERVER_SPECIFICS (such as name changes and mtime changes).
-  mock_server_->AddUpdateSpecifics(1, 0, kEncryptedString, 30, 30, true, 0,
-                                   encrypted_bookmark);
-  mock_server_->AddUpdateSpecifics(2, 1, kEncryptedString, 30, 30, false, 2,
-                                   encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      1, 0, kEncryptedString, 30, 30, true, pos1, encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      2, 1, kEncryptedString, 30, 30, false, pos2, encrypted_bookmark);
   // Item 3 doesn't change.
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 30, 30, false, 0,
                                    encrypted_pref);
@@ -908,10 +925,11 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   // Positional changes, parent changes, and specifics changes should reset
   // BASE_SERVER_SPECIFICS.
   // Became unencrypted.
-  mock_server_->AddUpdateSpecifics(1, 0, "A", 40, 40, true, 0, bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      1, 0, "A", 40, 40, true, 0, bookmark);
   // Reordered to after item 2.
-  mock_server_->AddUpdateSpecifics(3, 1, kEncryptedString, 30, 30, false, 3,
-                                   encrypted_bookmark);
+  mock_server_->AddBookmarkUpdateSpecifics(
+      3, 1, kEncryptedString, 30, 30, false, 3, encrypted_bookmark);
   SyncShareNudge();
   {
     // Items 2 and 4 should be the only ones with BASE_SERVER_SPECIFICS set.
@@ -989,7 +1007,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
 TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(),
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(),
                         "Pete");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
@@ -997,7 +1015,7 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::BASE_VERSION, 1);
     parent.Put(syncable::ID, parent_id_);
-    MutableEntry child(&wtrans, syncable::CREATE, parent_id_, "Pete");
+    MutableEntry child(&wtrans, syncable::CREATE_BOOKMARK, parent_id_, "Pete");
     ASSERT_TRUE(child.good());
     child.Put(syncable::ID, child_id_);
     child.Put(syncable::BASE_VERSION, 1);
@@ -1022,20 +1040,22 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
   syncable::Id pref_node_id = TestIdFactory::MakeServer("Tim");
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(), "Pete");
+    MutableEntry parent(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "Pete");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::BASE_VERSION, 1);
     parent.Put(syncable::ID, parent_id_);
-    MutableEntry child(&wtrans, syncable::CREATE, parent_id_, "Pete");
+    MutableEntry child(&wtrans, syncable::CREATE_BOOKMARK, parent_id_, "Pete");
     ASSERT_TRUE(child.good());
     child.Put(syncable::ID, child_id_);
     child.Put(syncable::BASE_VERSION, 1);
     WriteTestDataToEntry(&wtrans, &child);
 
-    MutableEntry parent2(&wtrans, syncable::CREATE, wtrans.root_id(), "Tim");
+    MutableEntry parent2(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "Tim");
     ASSERT_TRUE(parent2.good());
     parent2.Put(syncable::IS_UNSYNCED, true);
     parent2.Put(syncable::IS_DIR, true);
@@ -1069,7 +1089,8 @@ TEST_F(SyncerTest, TestPurgeWhileUnapplied) {
   // Similar to above, but for unapplied items. Bug 49278.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(), "Pete");
+    MutableEntry parent(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "Pete");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNAPPLIED_UPDATE, true);
     parent.Put(syncable::IS_DIR, true);
@@ -1224,7 +1245,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     {
-      MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(),
+      MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(),
                           "Bob");
       ASSERT_TRUE(parent.good());
       parent.Put(syncable::IS_UNSYNCED, true);
@@ -1232,16 +1253,16 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       parent.Put(syncable::ID, ids_.FromNumber(100));
       parent.Put(syncable::BASE_VERSION, 1);
-      MutableEntry child(&wtrans, syncable::CREATE, ids_.FromNumber(100),
-                         "Bob");
+      MutableEntry child(
+          &wtrans, syncable::CREATE_BOOKMARK, ids_.FromNumber(100), "Bob");
       ASSERT_TRUE(child.good());
       child.Put(syncable::IS_UNSYNCED, true);
       child.Put(syncable::IS_DIR, true);
       child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       child.Put(syncable::ID, ids_.FromNumber(101));
       child.Put(syncable::BASE_VERSION, 1);
-      MutableEntry grandchild(&wtrans, syncable::CREATE, ids_.FromNumber(101),
-                              "Bob");
+      MutableEntry grandchild(
+          &wtrans, syncable::CREATE_BOOKMARK, ids_.FromNumber(101), "Bob");
       ASSERT_TRUE(grandchild.good());
       grandchild.Put(syncable::ID, ids_.FromNumber(102));
       grandchild.Put(syncable::IS_UNSYNCED, true);
@@ -1251,7 +1272,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
     {
       // Create three deleted items which deletions we expect to be sent to the
       // server.
-      MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(),
+      MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(),
                           "Pete");
       ASSERT_TRUE(parent.good());
       parent.Put(syncable::ID, ids_.FromNumber(103));
@@ -1261,8 +1282,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       parent.Put(syncable::IS_DEL, true);
       parent.Put(syncable::BASE_VERSION, 1);
       parent.Put(syncable::MTIME, now_minus_2h);
-      MutableEntry child(&wtrans, syncable::CREATE, ids_.FromNumber(103),
-                         "Pete");
+      MutableEntry child(
+          &wtrans, syncable::CREATE_BOOKMARK, ids_.FromNumber(103), "Pete");
       ASSERT_TRUE(child.good());
       child.Put(syncable::ID, ids_.FromNumber(104));
       child.Put(syncable::IS_UNSYNCED, true);
@@ -1271,8 +1292,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       child.Put(syncable::IS_DEL, true);
       child.Put(syncable::BASE_VERSION, 1);
       child.Put(syncable::MTIME, now_minus_2h);
-      MutableEntry grandchild(&wtrans, syncable::CREATE, ids_.FromNumber(104),
-                              "Pete");
+      MutableEntry grandchild(
+          &wtrans, syncable::CREATE_BOOKMARK, ids_.FromNumber(104), "Pete");
       ASSERT_TRUE(grandchild.good());
       grandchild.Put(syncable::ID, ids_.FromNumber(105));
       grandchild.Put(syncable::IS_UNSYNCED, true);
@@ -1301,32 +1322,37 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
 }
 
 TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
+  syncable::Id parent1_id = ids_.MakeServer("p1");
+  syncable::Id parent2_id = ids_.MakeServer("p2");
+
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(), "1");
+    MutableEntry parent(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "1");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
-    parent.Put(syncable::ID, parent_id_);
-    MutableEntry child(&wtrans, syncable::CREATE, wtrans.root_id(), "2");
+    parent.Put(syncable::ID, parent1_id);
+    MutableEntry child(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "2");
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
-    child.Put(syncable::ID, child_id_);
+    child.Put(syncable::ID, parent2_id);
     parent.Put(syncable::BASE_VERSION, 1);
     child.Put(syncable::BASE_VERSION, 1);
   }
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, parent_id_, "A");
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, parent1_id, "A");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::ID, ids_.FromNumber(102));
-    MutableEntry child(&wtrans, syncable::CREATE, parent_id_, "B");
+    MutableEntry child(&wtrans, syncable::CREATE_BOOKMARK, parent1_id, "B");
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
@@ -1336,13 +1362,13 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
   }
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, child_id_, "A");
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, parent2_id, "A");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::ID, ids_.FromNumber(-104));
-    MutableEntry child(&wtrans, syncable::CREATE, child_id_, "B");
+    MutableEntry child(&wtrans, syncable::CREATE_BOOKMARK, parent2_id, "B");
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
@@ -1353,13 +1379,29 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
 
   SyncShareNudge();
   ASSERT_EQ(6u, mock_server_->committed_ids().size());
-  // If this test starts failing, be aware other sort orders could be valid.
-  EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
-  EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]);
-  EXPECT_TRUE(ids_.FromNumber(102) == mock_server_->committed_ids()[2]);
-  EXPECT_TRUE(ids_.FromNumber(-103) == mock_server_->committed_ids()[3]);
-  EXPECT_TRUE(ids_.FromNumber(-104) == mock_server_->committed_ids()[4]);
-  EXPECT_TRUE(ids_.FromNumber(105) == mock_server_->committed_ids()[5]);
+
+  // This strange iteration and std::count() usage is to allow the order to
+  // vary.  All we really care about is that parent1_id and parent2_id are the
+  // first two IDs, and that the children make up the next four.  Other than
+  // that, ordering doesn't matter.
+
+  vector<syncable::Id>::const_iterator i =
+      mock_server_->committed_ids().begin();
+  vector<syncable::Id>::const_iterator parents_begin = i;
+  i++;
+  i++;
+  vector<syncable::Id>::const_iterator parents_end = i;
+  vector<syncable::Id>::const_iterator children_begin = i;
+  vector<syncable::Id>::const_iterator children_end =
+      mock_server_->committed_ids().end();
+
+  EXPECT_EQ(1, count(parents_begin, parents_end, parent1_id));
+  EXPECT_EQ(1, count(parents_begin, parents_end, parent2_id));
+
+  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(-103)));
+  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(102)));
+  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(105)));
+  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(-104)));
 }
 
 TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
@@ -1367,18 +1409,19 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(), "P");
+    MutableEntry parent(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), "P");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::ID, parent_id_);
-    MutableEntry child1(&wtrans, syncable::CREATE, parent_id_, "1");
+    MutableEntry child1(&wtrans, syncable::CREATE_BOOKMARK, parent_id_, "1");
     ASSERT_TRUE(child1.good());
     child1.Put(syncable::IS_UNSYNCED, true);
     child1.Put(syncable::ID, child_id_);
     child1.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
-    MutableEntry child2(&wtrans, syncable::CREATE, parent_id_, "2");
+    MutableEntry child2(&wtrans, syncable::CREATE_BOOKMARK, parent_id_, "2");
     ASSERT_TRUE(child2.good());
     child2.Put(syncable::IS_UNSYNCED, true);
     child2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
@@ -1391,10 +1434,15 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
 
   SyncShareNudge();
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
-  // If this test starts failing, be aware other sort orders could be valid.
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
-  EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]);
-  EXPECT_TRUE(child2_id == mock_server_->committed_ids()[2]);
+  // There are two possible valid orderings.
+  if (child2_id == mock_server_->committed_ids()[1]) {
+    EXPECT_TRUE(child2_id == mock_server_->committed_ids()[1]);
+    EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[2]);
+  } else {
+    EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]);
+    EXPECT_TRUE(child2_id == mock_server_->committed_ids()[2]);
+  }
 }
 
 TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
@@ -1404,7 +1452,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, wtrans.root_id(),
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(),
                         parent1_name);
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
@@ -1418,14 +1466,16 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
   syncable::Id child_id = ids_.NewServerId();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent2(&wtrans, syncable::CREATE, parent_id_, parent2_name);
+    MutableEntry parent2(
+        &wtrans, syncable::CREATE_BOOKMARK, parent_id_, parent2_name);
     ASSERT_TRUE(parent2.good());
     parent2.Put(syncable::IS_UNSYNCED, true);
     parent2.Put(syncable::IS_DIR, true);
     parent2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent2.Put(syncable::ID, parent2_id);
 
-    MutableEntry child(&wtrans, syncable::CREATE, parent2_id, child_name);
+    MutableEntry child(
+        &wtrans, syncable::CREATE_BOOKMARK, parent2_id, child_name);
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
@@ -1472,7 +1522,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&wtrans,
-                        syncable::CREATE,
+                        syncable::CREATE_BOOKMARK,
                         wtrans.root_id(),
                         parent_name);
     ASSERT_TRUE(parent.good());
@@ -1488,14 +1538,16 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
   const Id child_local_id = ids_.NewLocalId();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent2(&wtrans, syncable::CREATE, parent_id_, parent2_name);
+    MutableEntry parent2(
+        &wtrans, syncable::CREATE_BOOKMARK, parent_id_, parent2_name);
     ASSERT_TRUE(parent2.good());
     parent2.Put(syncable::IS_UNSYNCED, true);
     parent2.Put(syncable::IS_DIR, true);
     parent2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
 
     parent2.Put(syncable::ID, parent2_local_id);
-    MutableEntry child(&wtrans, syncable::CREATE, parent2_local_id, child_name);
+    MutableEntry child(
+        &wtrans, syncable::CREATE_BOOKMARK, parent2_local_id, child_name);
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
@@ -1713,14 +1765,14 @@ TEST_F(SyncerTest, CommitTimeRename) {
   // Create a folder and an entry.
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&trans, CREATE, root_id_, "Folder");
+    MutableEntry parent(&trans, CREATE_BOOKMARK, root_id_, "Folder");
     ASSERT_TRUE(parent.good());
     parent.Put(IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(IS_UNSYNCED, true);
     metahandle_folder = parent.Get(META_HANDLE);
 
-    MutableEntry entry(&trans, CREATE, parent.Get(ID), "new_entry");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, parent.Get(ID), "new_entry");
     ASSERT_TRUE(entry.good());
     metahandle_new_entry = entry.Get(META_HANDLE);
     WriteTestDataToEntry(&trans, &entry);
@@ -1761,7 +1813,7 @@ TEST_F(SyncerTest, CommitTimeRenameI18N) {
   // Create a folder, expect a commit time rename.
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&trans, CREATE, root_id_, "Folder");
+    MutableEntry parent(&trans, CREATE_BOOKMARK, root_id_, "Folder");
     ASSERT_TRUE(parent.good());
     parent.Put(IS_DIR, true);
     parent.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -1792,7 +1844,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
   int64 metahandle_folder;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "new_folder");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "new_folder");
     ASSERT_TRUE(entry.good());
     entry.Put(IS_DIR, true);
     entry.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -1814,7 +1866,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
   // Create an entry in the newly created folder.
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, folder_id, "new_entry");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, folder_id, "new_entry");
     ASSERT_TRUE(entry.good());
     metahandle_entry = entry.Get(META_HANDLE);
     WriteTestDataToEntry(&trans, &entry);
@@ -1883,7 +1935,7 @@ TEST_F(SyncerTest, CommitReuniteUpdate) {
   int64 entry_metahandle;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "new_entry");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "new_entry");
     ASSERT_TRUE(entry.good());
     entry_metahandle = entry.Get(META_HANDLE);
     WriteTestDataToEntry(&trans, &entry);
@@ -1937,7 +1989,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateDoesNotChokeOnDeletedLocalEntry) {
   int64 entry_metahandle;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "new_entry");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "new_entry");
     ASSERT_TRUE(entry.good());
     entry_metahandle = entry.Get(META_HANDLE);
     WriteTestDataToEntry(&trans, &entry);
@@ -2101,8 +2153,8 @@ class EntryCreatedInNewFolderTest : public SyncerTest {
                                           "bob"));
     CHECK(bob.good());
 
-    MutableEntry entry2(&trans, syncable::CREATE, bob.Get(syncable::ID),
-                        "bob");
+    MutableEntry entry2(
+        &trans, syncable::CREATE_BOOKMARK, bob.Get(syncable::ID), "bob");
     CHECK(entry2.good());
     entry2.Put(syncable::IS_DIR, true);
     entry2.Put(syncable::IS_UNSYNCED, true);
@@ -2113,7 +2165,7 @@ class EntryCreatedInNewFolderTest : public SyncerTest {
 TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::CREATE, trans.root_id(),
+    MutableEntry entry(&trans, syncable::CREATE_BOOKMARK, trans.root_id(),
                        "bob");
     ASSERT_TRUE(entry.good());
     entry.Put(syncable::IS_DIR, true);
@@ -2152,7 +2204,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
   {
     // Create an item.
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry fred_match(&trans, CREATE, trans.root_id(),
+    MutableEntry fred_match(&trans, CREATE_BOOKMARK, trans.root_id(),
                             "fred_match");
     ASSERT_TRUE(fred_match.good());
     metahandle_fred = fred_match.Get(META_HANDLE);
@@ -2188,13 +2240,14 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
 TEST_F(SyncerTest, DoublyChangedWithResolver) {
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, root_id_, "Folder");
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, root_id_, "Folder");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::ID, parent_id_);
     parent.Put(syncable::BASE_VERSION, 5);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
-    MutableEntry child(&wtrans, syncable::CREATE, parent_id_, "Pete.htm");
+    MutableEntry child(
+        &wtrans, syncable::CREATE_BOOKMARK, parent_id_, "Pete.htm");
     ASSERT_TRUE(child.good());
     child.Put(syncable::ID, child_id_);
     child.Put(syncable::BASE_VERSION, 10);
@@ -2229,7 +2282,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
   int64 entry_metahandle;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&wtrans, syncable::CREATE, root_id_, "Pete");
+    MutableEntry entry(&wtrans, syncable::CREATE_BOOKMARK, root_id_, "Pete");
     ASSERT_TRUE(entry.good());
     EXPECT_FALSE(entry.Get(ID).ServerKnows());
     entry.Put(syncable::IS_DIR, true);
@@ -2241,7 +2294,6 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
   SyncShareNudge();
   syncable::Id id;
   int64 version;
-  NodeOrdinal server_ordinal_in_parent;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, syncable::GET_BY_HANDLE, entry_metahandle);
@@ -2249,16 +2301,12 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
     id = entry.Get(ID);
     EXPECT_TRUE(id.ServerKnows());
     version = entry.Get(BASE_VERSION);
-    server_ordinal_in_parent = entry.Get(SERVER_ORDINAL_IN_PARENT);
   }
   sync_pb::SyncEntity* update = mock_server_->AddUpdateFromLastCommit();
   EXPECT_EQ("Pete", update->name());
   EXPECT_EQ(id.GetServerId(), update->id_string());
   EXPECT_EQ(root_id_.GetServerId(), update->parent_id_string());
   EXPECT_EQ(version, update->version());
-  EXPECT_EQ(
-      NodeOrdinalToInt64(server_ordinal_in_parent),
-      update->position_in_parent());
   SyncShareNudge();
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -2275,7 +2323,7 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, root_id_, "Folder");
+    MutableEntry parent(&wtrans, CREATE_BOOKMARK, root_id_, "Folder");
     ASSERT_TRUE(parent.good());
     parent.Put(IS_DIR, true);
     parent.Put(IS_UNSYNCED, true);
@@ -2283,7 +2331,7 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
     parent.Put(BASE_VERSION, 1);
     parent.Put(SPECIFICS, DefaultBookmarkSpecifics());
 
-    MutableEntry child(&wtrans, CREATE, parent.Get(ID), "test.htm");
+    MutableEntry child(&wtrans, CREATE_BOOKMARK, parent.Get(ID), "test.htm");
     ASSERT_TRUE(child.good());
     child.Put(ID, child_id);
     child.Put(BASE_VERSION, 1);
@@ -2316,7 +2364,7 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
 TEST_F(SyncerTest, CommittingNewDeleted) {
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "bob");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "bob");
     entry.Put(IS_UNSYNCED, true);
     entry.Put(IS_DEL, true);
   }
@@ -2337,7 +2385,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) {
   // This test is a little fake.
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "bob");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "bob");
     entry.Put(ID, ids_.FromNumber(20));
     entry.Put(BASE_VERSION, 1);
     entry.Put(SERVER_VERSION, 1);
@@ -2369,7 +2417,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
   int64 existing_metahandle;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "existing");
+    MutableEntry entry(&trans, CREATE_BOOKMARK, trans.root_id(), "existing");
     ASSERT_TRUE(entry.good());
     entry.Put(IS_DIR, true);
     entry.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -2379,7 +2427,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry newfolder(&trans, CREATE, trans.root_id(), "new");
+    MutableEntry newfolder(&trans, CREATE_BOOKMARK, trans.root_id(), "new");
     ASSERT_TRUE(newfolder.good());
     newfolder.Put(IS_DIR, true);
     newfolder.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -2405,7 +2453,8 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
   SyncShareNudge();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry newfolder(&trans, CREATE, ids_.FromNumber(1), "local");
+    MutableEntry newfolder(
+        &trans, CREATE_BOOKMARK, ids_.FromNumber(1), "local");
     ASSERT_TRUE(newfolder.good());
     newfolder.Put(IS_UNSYNCED, true);
     newfolder.Put(IS_DIR, true);
@@ -2496,7 +2545,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_Success) {
     for (uint32 i = 0; i < items_to_commit; i++) {
       string nameutf8 = base::StringPrintf("%d", i);
       string name(nameutf8.begin(), nameutf8.end());
-      MutableEntry e(&trans, CREATE, trans.root_id(), name);
+      MutableEntry e(&trans, CREATE_BOOKMARK, trans.root_id(), name);
       e.Put(IS_UNSYNCED, true);
       e.Put(IS_DIR, true);
       e.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -2519,7 +2568,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_PostBufferFail) {
     for (uint32 i = 0; i < items_to_commit; i++) {
       string nameutf8 = base::StringPrintf("%d", i);
       string name(nameutf8.begin(), nameutf8.end());
-      MutableEntry e(&trans, CREATE, trans.root_id(), name);
+      MutableEntry e(&trans, CREATE_BOOKMARK, trans.root_id(), name);
       e.Put(IS_UNSYNCED, true);
       e.Put(IS_DIR, true);
       e.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -2549,7 +2598,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_CommitConflict) {
     for (uint32 i = 0; i < items_to_commit; i++) {
       string nameutf8 = base::StringPrintf("%d", i);
       string name(nameutf8.begin(), nameutf8.end());
-      MutableEntry e(&trans, CREATE, trans.root_id(), name);
+      MutableEntry e(&trans, CREATE_BOOKMARK, trans.root_id(), name);
       e.Put(IS_UNSYNCED, true);
       e.Put(IS_DIR, true);
       e.Put(SPECIFICS, DefaultBookmarkSpecifics());
@@ -2647,7 +2696,8 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   syncable::Id local_folder_id;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry new_entry(&wtrans, CREATE, wtrans.root_id(), "Bar.htm");
+    MutableEntry new_entry(
+        &wtrans, CREATE_BOOKMARK, wtrans.root_id(), "Bar.htm");
     ASSERT_TRUE(new_entry.good());
     local_folder_id = new_entry.Get(ID);
     local_folder_handle = new_entry.Get(META_HANDLE);
@@ -2713,7 +2763,8 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   syncable::Id local_folder_id;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry new_entry(&wtrans, CREATE, wtrans.root_id(), "Bar.htm");
+    MutableEntry new_entry(
+        &wtrans, CREATE_BOOKMARK, wtrans.root_id(), "Bar.htm");
     ASSERT_TRUE(new_entry.good());
     local_folder_id = new_entry.Get(ID);
     local_folder_handle = new_entry.Get(META_HANDLE);
@@ -2885,12 +2936,12 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
 TEST_F(SyncerTest, DuplicateIDReturn) {
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry folder(&trans, CREATE, trans.root_id(), "bob");
+    MutableEntry folder(&trans, CREATE_BOOKMARK, trans.root_id(), "bob");
     ASSERT_TRUE(folder.good());
     folder.Put(IS_UNSYNCED, true);
     folder.Put(IS_DIR, true);
     folder.Put(SPECIFICS, DefaultBookmarkSpecifics());
-    MutableEntry folder2(&trans, CREATE, trans.root_id(), "fred");
+    MutableEntry folder2(&trans, CREATE_BOOKMARK, trans.root_id(), "fred");
     ASSERT_TRUE(folder2.good());
     folder2.Put(IS_UNSYNCED, false);
     folder2.Put(IS_DIR, true);
@@ -2929,7 +2980,8 @@ TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
 
-    MutableEntry local_deleted(&trans, CREATE, trans.root_id(), "name");
+    MutableEntry local_deleted(
+        &trans, CREATE_BOOKMARK, trans.root_id(), "name");
     local_deleted.Put(ID, ids_.FromNumber(1));
     local_deleted.Put(BASE_VERSION, 1);
     local_deleted.Put(IS_DEL, true);
@@ -2962,7 +3014,8 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
 
-    MutableEntry local_deleted(&trans, CREATE, trans.root_id(), "name");
+    MutableEntry local_deleted(
+        &trans, CREATE_BOOKMARK, trans.root_id(), "name");
     local_deleted.Put(ID, ids_.FromNumber(1));
     local_deleted.Put(BASE_VERSION, 1);
     local_deleted.Put(IS_DEL, true);
@@ -3000,7 +3053,8 @@ TEST_F(SyncerTest, MergingExistingItems) {
   SyncShareNudge();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "Copy of base");
+    MutableEntry entry(
+        &trans, CREATE_BOOKMARK, trans.root_id(), "Copy of base");
     WriteTestDataToEntry(&trans, &entry);
   }
   mock_server_->AddUpdateBookmark(1, 0, "Copy of base", 50, 50);
@@ -3190,7 +3244,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, syncable::CREATE, root_id_, "foo");
+    MutableEntry parent(&wtrans, syncable::CREATE_BOOKMARK, root_id_, "foo");
     ASSERT_TRUE(parent.good());
     parent.Put(syncable::IS_UNSYNCED, true);
     parent.Put(syncable::IS_DIR, true);
@@ -3198,7 +3252,8 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
     in_root_id = parent.Get(syncable::ID);
     foo_metahandle = parent.Get(META_HANDLE);
 
-    MutableEntry child(&wtrans, syncable::CREATE, parent.Get(ID), "bar");
+    MutableEntry child(
+        &wtrans, syncable::CREATE_BOOKMARK, parent.Get(ID), "bar");
     ASSERT_TRUE(child.good());
     child.Put(syncable::IS_UNSYNCED, true);
     child.Put(syncable::IS_DIR, true);
@@ -3310,7 +3365,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
     entry.Put(PARENT_ID, folder_two_id);
     entry.Put(IS_UNSYNCED, true);
     // A new entry should send no "old parent."
-    MutableEntry create(&trans, CREATE, trans.root_id(), "new_folder");
+    MutableEntry create(&trans, CREATE_BOOKMARK, trans.root_id(), "new_folder");
     create.Put(IS_UNSYNCED, true);
     create.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
@@ -3330,7 +3385,8 @@ TEST_F(SyncerTest, Test64BitVersionSupport) {
   // Try writing max int64 to the version fields of a meta entry.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&wtrans, syncable::CREATE, wtrans.root_id(), name);
+    MutableEntry entry(
+        &wtrans, syncable::CREATE_BOOKMARK, wtrans.root_id(), name);
     ASSERT_TRUE(entry.good());
     entry.Put(syncable::BASE_VERSION, really_big_int);
     entry.Put(syncable::SERVER_VERSION, really_big_int);
@@ -3541,7 +3597,8 @@ TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
 
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry perm_folder(&trans, CREATE, ids_.root(), "clientname");
+    MutableEntry perm_folder(
+        &trans, CREATE_BOOKMARK, ids_.root(), "clientname");
     ASSERT_TRUE(perm_folder.good());
     perm_folder.Put(UNIQUE_CLIENT_TAG, "clientperm");
     perm_folder.Put(SPECIFICS, local_bookmark);
@@ -3606,7 +3663,8 @@ TEST_F(SyncerTest, ClientTagConflictWithDeletedLocalEntry) {
   {
     // Create a deleted local entry with a unique client tag.
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry perm_folder(&trans, CREATE, ids_.root(), "clientname");
+    MutableEntry perm_folder(
+        &trans, CREATE_BOOKMARK, ids_.root(), "clientname");
     ASSERT_TRUE(perm_folder.good());
     ASSERT_FALSE(perm_folder.Get(ID).ServerKnows());
     perm_folder.Put(UNIQUE_CLIENT_TAG, "clientperm");
@@ -4068,7 +4126,8 @@ class SyncerUndeletionTest : public SyncerTest {
 
   void Create() {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry perm_folder(&trans, CREATE, ids_.root(), "clientname");
+    MutableEntry perm_folder(
+        &trans, CREATE_BOOKMARK, ids_.root(), "clientname");
     ASSERT_TRUE(perm_folder.good());
     perm_folder.Put(UNIQUE_CLIENT_TAG, client_tag_);
     perm_folder.Put(IS_UNSYNCED, true);
@@ -4489,220 +4548,4 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
   EXPECT_EQ("Thadeusz", Get(metahandle_, NON_UNIQUE_NAME));
 }
 
-// A group of tests exercising the syncer's handling of sibling ordering, as
-// represented in the sync protocol.
-class SyncerPositionUpdateTest : public SyncerTest {
- public:
-  SyncerPositionUpdateTest() : next_update_id_(1), next_revision_(1) {}
-
- protected:
-  void ExpectLocalItemsInServerOrder() {
-    if (position_map_.empty())
-      return;
-
-    syncable::ReadTransaction trans(FROM_HERE, directory());
-
-    Id prev_id;
-    DCHECK(prev_id.IsRoot());
-    PosMap::iterator next = position_map_.begin();
-    for (PosMap::iterator i = next++; i != position_map_.end(); ++i) {
-      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(
-          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));
-      } else {
-        EXPECT_EQ(next->second, entry_with_id.Get(NEXT_ID));
-        next++;
-      }
-      prev_id = id;
-    }
-  }
-
-  void AddRootItemWithPosition(int64 position) {
-    string id = string("ServerId") + base::Int64ToString(next_update_id_++);
-    string name = "my name is my id -- " + id;
-    int revision = next_revision_++;
-    mock_server_->AddUpdateDirectory(id, kRootId, name, revision, revision);
-    mock_server_->SetLastUpdatePosition(position);
-    position_map_.insert(
-        PosMap::value_type(position, Id::CreateFromServerId(id)));
-  }
- private:
-  typedef multimap<int64, Id> PosMap;
-  PosMap position_map_;
-  int next_update_id_;
-  int next_revision_;
-  DISALLOW_COPY_AND_ASSIGN(SyncerPositionUpdateTest);
-};
-
-TEST_F(SyncerPositionUpdateTest, InOrderPositive) {
-  // Add a bunch of items in increasing order, starting with just positive
-  // position values.
-  AddRootItemWithPosition(100);
-  AddRootItemWithPosition(199);
-  AddRootItemWithPosition(200);
-  AddRootItemWithPosition(201);
-  AddRootItemWithPosition(400);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-}
-
-TEST_F(SyncerPositionUpdateTest, InOrderNegative) {
-  // Test negative position values, but in increasing order.
-  AddRootItemWithPosition(-400);
-  AddRootItemWithPosition(-201);
-  AddRootItemWithPosition(-200);
-  AddRootItemWithPosition(-150);
-  AddRootItemWithPosition(100);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-}
-
-TEST_F(SyncerPositionUpdateTest, ReverseOrder) {
-  // Test when items are sent in the reverse order.
-  AddRootItemWithPosition(400);
-  AddRootItemWithPosition(201);
-  AddRootItemWithPosition(200);
-  AddRootItemWithPosition(100);
-  AddRootItemWithPosition(-150);
-  AddRootItemWithPosition(-201);
-  AddRootItemWithPosition(-200);
-  AddRootItemWithPosition(-400);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-}
-
-TEST_F(SyncerPositionUpdateTest, RandomOrderInBatches) {
-  // Mix it all up, interleaving position values, and try multiple batches of
-  // updates.
-  AddRootItemWithPosition(400);
-  AddRootItemWithPosition(201);
-  AddRootItemWithPosition(-400);
-  AddRootItemWithPosition(100);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-
-  AddRootItemWithPosition(-150);
-  AddRootItemWithPosition(-200);
-  AddRootItemWithPosition(200);
-  AddRootItemWithPosition(-201);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-
-  AddRootItemWithPosition(-144);
-
-  SyncShareNudge();
-  ExpectLocalItemsInServerOrder();
-}
-
-class SyncerPositionTiebreakingTest : public SyncerTest {
- public:
-  SyncerPositionTiebreakingTest()
-      : low_id_(Id::CreateFromServerId("A")),
-        mid_id_(Id::CreateFromServerId("M")),
-        high_id_(Id::CreateFromServerId("Z")),
-        next_revision_(1) {
-    DCHECK(low_id_ < mid_id_);
-    DCHECK(mid_id_ < high_id_);
-    DCHECK(low_id_ < high_id_);
-  }
-
-  // Adds the item by its Id, using a constant value for the position
-  // so that the syncer has to resolve the order some other way.
-  void Add(const Id& id) {
-    int revision = next_revision_++;
-    mock_server_->AddUpdateDirectory(id.GetServerId(), kRootId,
-        id.GetServerId(), revision, revision);
-    // The update position doesn't vary.
-    mock_server_->SetLastUpdatePosition(90210);
-  }
-
-  void ExpectLocalOrderIsByServerId() {
-    syncable::ReadTransaction trans(FROM_HERE, directory());
-    Id null_id;
-    Entry low(&trans, GET_BY_ID, low_id_);
-    Entry mid(&trans, GET_BY_ID, mid_id_);
-    Entry high(&trans, GET_BY_ID, high_id_);
-    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_);
-  }
-
- protected:
-  // When there's a tiebreak on the numeric position, it's supposed to be
-  // broken by string comparison of the ids.  These ids are in increasing
-  // order.
-  const Id low_id_;
-  const Id mid_id_;
-  const Id high_id_;
-
- private:
-  int next_revision_;
-  DISALLOW_COPY_AND_ASSIGN(SyncerPositionTiebreakingTest);
-};
-
-TEST_F(SyncerPositionTiebreakingTest, LowMidHigh) {
-  Add(low_id_);
-  Add(mid_id_);
-  Add(high_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
-TEST_F(SyncerPositionTiebreakingTest, LowHighMid) {
-  Add(low_id_);
-  Add(high_id_);
-  Add(mid_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
-TEST_F(SyncerPositionTiebreakingTest, HighMidLow) {
-  Add(high_id_);
-  Add(mid_id_);
-  Add(low_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
-TEST_F(SyncerPositionTiebreakingTest, HighLowMid) {
-  Add(high_id_);
-  Add(low_id_);
-  Add(mid_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
-TEST_F(SyncerPositionTiebreakingTest, MidHighLow) {
-  Add(mid_id_);
-  Add(high_id_);
-  Add(low_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
-TEST_F(SyncerPositionTiebreakingTest, MidLowHigh) {
-  Add(mid_id_);
-  Add(low_id_);
-  Add(high_id_);
-  SyncShareNudge();
-  ExpectLocalOrderIsByServerId();
-}
-
 }  // namespace syncer