WIP: The Bookmark Position Megapatch

sync/engine/syncer_unittest.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.
 //
 // Syncer unit tests. Unfortunately a lot of these tests
 // are outdated and need to be reworked and updated.
 
 #include <algorithm>
 #include <limits>
 #include <list>
@@ skipping 15 matching lines @@
 #include "sync/engine/get_commit_ids_command.h"
 #include "sync/engine/net/server_connection_manager.h"
 #include "sync/engine/process_updates_command.h"
 #include "sync/engine/sync_scheduler_impl.h"
 #include "sync/engine/syncer.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/engine/throttled_data_type_tracker.h"
 #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"
 #include "sync/protocol/sync.pb.h"
 #include "sync/sessions/sync_session_context.h"
 #include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/nigori_util.h"
 #include "sync/syncable/read_transaction.h"
 #include "sync/syncable/syncable_util.h"
 #include "sync/syncable/write_transaction.h"
 #include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/mock_connection_manager.h"
 #include "sync/test/engine/test_directory_setter_upper.h"
 #include "sync/test/engine/test_id_factory.h"
 #include "sync/test/engine/test_syncable_utils.h"
 #include "sync/test/fake_encryptor.h"
 #include "sync/test/fake_extensions_activity_monitor.h"
 #include "sync/test/fake_sync_encryption_handler.h"
 #include "sync/util/cryptographer.h"
 #include "sync/util/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::TimeDelta;
 
+using std::count;
 using std::map;
 using std::multimap;
 using std::set;
 using std::string;
+using std::vector;
 
 namespace syncer {
 
 using syncable::BaseTransaction;
 using syncable::Blob;
 using syncable::CountEntriesWithName;
 using syncable::Directory;
 using syncable::Entry;
 using syncable::GetFirstEntryWithName;
 using syncable::GetOnlyEntryWithName;
 using syncable::Id;
 using syncable::kEncryptedString;
 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;
 using syncable::GET_BY_SERVER_TAG;
 using syncable::ID;
 using syncable::IS_DEL;
 using syncable::IS_DIR;
 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;
 using syncable::UNIQUE_SERVER_TAG;
 using syncable::SPECIFICS;
 using syncable::SYNCING;
 using syncable::UNITTEST;
 
 using sessions::ScopedSetSessionWriteTransaction;
 using sessions::StatusController;
@@ skipping 239 matching lines @@
       WriteTransaction trans(FROM_HERE, UNITTEST, directory());
       while (!test->id.IsRoot()) {
         if (test->commit_index >= 0) {
           map<int, syncable::Id>::value_type entry(test->commit_index,
                                                    test->id);
           bool double_position = !expected_positions.insert(entry).second;
           ASSERT_FALSE(double_position) << "Two id's expected at one position";
         }
         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()) {
           entry.Put(BASE_VERSION, 5);
           entry.Put(SERVER_VERSION, 5);
           entry.Put(SERVER_PARENT_ID, test->parent_id);
         }
         entry.Put(syncable::IS_DIR, true);
         entry.Put(syncable::IS_UNSYNCED, true);
         entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
@@ skipping 82 matching lines @@
 
   int64 CreateUnsyncedDirectory(const string& entry_name,
       const string& idstring) {
     return CreateUnsyncedDirectory(entry_name,
         syncable::Id::CreateFromServerId(idstring));
   }
 
   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);
     entry.Put(syncable::IS_DIR, true);
     entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     entry.Put(syncable::BASE_VERSION, id.ServerKnows() ? 1 : 0);
     entry.Put(syncable::ID, id);
     return entry.Get(META_HANDLE);
   }
 
@@ skipping 369 matching lines @@
     // have pending keys.
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     sync_pb::EntitySpecifics specifics;
     sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
     other_cryptographer.GetKeys(nigori->mutable_encryption_keybag());
     dir_maker_.encryption_handler()->EnableEncryptEverything();
     GetCryptographer(&wtrans)->SetPendingKeys(nigori->encryption_keybag());
     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();
   {
     // Initial state. Everything is normal.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     VERIFY_ENTRY(1, false, false, false, 0, 10, 10, ids_, &rtrans);
     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();
   {
     // All should be unapplied due to being undecryptable and have a valid
     // BASE_SERVER_SPECIFICS.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     VERIFY_ENTRY(1, true, false, true, 0, 10, 20, ids_, &rtrans);
     VERIFY_ENTRY(2, true, false, true, 1, 10, 20, ids_, &rtrans);
     VERIFY_ENTRY(3, true, false, true, 1, 10, 20, ids_, &rtrans);
     VERIFY_ENTRY(4, true, false, true, 0, 10, 20, ids_, &rtrans);
   }
 
   // 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);
   SyncShareNudge();
   {
     // Items 1, 2, and 4 should have newer server versions, 3 remains the same.
     // All should remain unapplied due to be undecryptable.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     VERIFY_ENTRY(1, true, false, true, 0, 10, 30, ids_, &rtrans);
     VERIFY_ENTRY(2, true, false, true, 1, 10, 30, ids_, &rtrans);
     VERIFY_ENTRY(3, true, false, true, 1, 10, 20, ids_, &rtrans);
     VERIFY_ENTRY(4, true, false, true, 0, 10, 30, ids_, &rtrans);
   }
 
   // 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.
     // Items 1 is now unencrypted, so should have applied normally.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     VERIFY_ENTRY(1, false, false, false, 0, 40, 40, ids_, &rtrans);
     VERIFY_ENTRY(2, true, false, true, 1, 10, 30, ids_, &rtrans);
     VERIFY_ENTRY(3, true, false, false, 1, 10, 30, ids_, &rtrans);
     VERIFY_ENTRY(4, true, false, true, 0, 10, 30, ids_, &rtrans);
   }
@@ skipping 57 matching lines @@
   VERIFY_ENTRY(2, false, false, false, 1, 31, 31, ids_, &rtrans);
   VERIFY_ENTRY(3, false, false, false, 1, 30, 30, ids_, &rtrans);
   VERIFY_ENTRY(4, false, false, false, 0, 31, 31, ids_, &rtrans);
 }
 
 #undef VERIFY_ENTRY
 
 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);
     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);
   }
 
   SyncShareNudge();
   ASSERT_EQ(2u, 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]);
   {
     syncable::ReadTransaction rt(FROM_HERE, directory());
     Entry entry(&rt, syncable::GET_BY_ID, child_id_);
     ASSERT_TRUE(entry.good());
     VerifyTestDataInEntry(&rt, &entry);
   }
 }
 
 TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
   // Similar to above, but throw a purge operation into the mix. Bug 49278.
   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);
     parent2.Put(syncable::SPECIFICS, DefaultPreferencesSpecifics());
     parent2.Put(syncable::BASE_VERSION, 1);
     parent2.Put(syncable::ID, pref_node_id);
   }
 
   directory()->PurgeEntriesWithTypeIn(ModelTypeSet(PREFERENCES));
 
@@ skipping 13 matching lines @@
     syncable::ReadTransaction rt(FROM_HERE, directory());
     Entry entry(&rt, syncable::GET_BY_ID, pref_node_id);
     ASSERT_FALSE(entry.good());
   }
 }
 
 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);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::BASE_VERSION, 1);
     parent.Put(syncable::ID, parent_id_);
   }
 
   directory()->PurgeEntriesWithTypeIn(
       ModelTypeSet(BOOKMARKS));
@@ skipping 134 matching lines @@
   };
   RunCommitOrderingTest(items);
 }
 
 TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
   const base::Time& now_minus_2h =
       base::Time::Now() - base::TimeDelta::FromHours(2);
   {
     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);
       parent.Put(syncable::IS_DIR, true);
       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);
       grandchild.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       grandchild.Put(syncable::BASE_VERSION, 1);
     }
     {
       // 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));
       parent.Put(syncable::IS_UNSYNCED, true);
       parent.Put(syncable::IS_DIR, true);
       parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       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);
       child.Put(syncable::IS_DIR, true);
       child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       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);
       grandchild.Put(syncable::IS_DEL, true);
       grandchild.Put(syncable::IS_DIR, false);
       grandchild.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       grandchild.Put(syncable::BASE_VERSION, 1);
       grandchild.Put(syncable::MTIME, now_minus_2h);
     }
   }
 
   SyncShareNudge();
   ASSERT_EQ(6u, mock_server_->committed_ids().size());
   // This test will NOT unroll deletes because SERVER_PARENT_ID is not set.
   // It will treat these like moves.
   vector<syncable::Id> commit_ids(mock_server_->committed_ids());
   EXPECT_TRUE(ids_.FromNumber(100) == commit_ids[0]);
   EXPECT_TRUE(ids_.FromNumber(101) == commit_ids[1]);
   EXPECT_TRUE(ids_.FromNumber(102) == commit_ids[2]);
   // We don't guarantee the delete orders in this test, only that they occur
   // at the end.
   std::sort(commit_ids.begin() + 3, commit_ids.end());
   EXPECT_TRUE(ids_.FromNumber(103) == commit_ids[3]);
   EXPECT_TRUE(ids_.FromNumber(104) == commit_ids[4]);
   EXPECT_TRUE(ids_.FromNumber(105) == commit_ids[5]);
 }
 
 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);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     child.Put(syncable::ID, ids_.FromNumber(-103));
     parent.Put(syncable::BASE_VERSION, 1);
   }
   {
     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);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     child.Put(syncable::ID, ids_.FromNumber(105));
     child.Put(syncable::BASE_VERSION, 1);
   }
 
   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) {
   syncable::Id child2_id = ids_.NewServerId();
 
   {
     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());
     child2.Put(syncable::ID, child2_id);
 
     parent.Put(syncable::BASE_VERSION, 1);
     child1.Put(syncable::BASE_VERSION, 1);
     child2.Put(syncable::BASE_VERSION, 1);
   }
 
   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) {
   string parent1_name = "1";
   string parent2_name = "A";
   string child_name = "B";
 
   {
     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);
     parent.Put(syncable::IS_DIR, true);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     parent.Put(syncable::ID, parent_id_);
     parent.Put(syncable::BASE_VERSION, 1);
   }
 
   syncable::Id parent2_id = ids_.NewLocalId();
   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);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     child.Put(syncable::ID, child_id);
     child.Put(syncable::BASE_VERSION, 1);
   }
 
   SyncShareNudge();
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
@@ skipping 26 matching lines @@
 }
 
 TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
   string parent_name = "1";
   string parent2_name = "A";
   string child_name = "B";
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&wtrans,
-                        syncable::CREATE,
+                        syncable::CREATE_BOOKMARK,
                         wtrans.root_id(),
                         parent_name);
     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_);
     parent.Put(syncable::BASE_VERSION, 1);
   }
 
   int64 meta_handle_b;
   const Id parent2_local_id = ids_.NewLocalId();
   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);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     child.Put(syncable::ID, child_local_id);
     meta_handle_b = child.Get(syncable::META_HANDLE);
   }
 
   SyncShareNudge();
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
@@ skipping 197 matching lines @@
   EXPECT_EQ(4, status().num_hierarchy_conflicts());
 }
 
 TEST_F(SyncerTest, CommitTimeRename) {
   int64 metahandle_folder;
   int64 metahandle_new_entry;
 
   // 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);
   }
 
   // Mix in a directory creation too for later.
   mock_server_->AddUpdateDirectory(2, 0, "dir_in_root", 10, 10);
   mock_server_->SetCommitTimeRename("renamed_");
   SyncShareNudge();
 
@@ skipping 20 matching lines @@
 TEST_F(SyncerTest, CommitTimeRenameI18N) {
   // This is utf-8 for the diacritized Internationalization.
   const char* i18nString = "\xc3\x8e\xc3\xb1\x74\xc3\xa9\x72\xc3\xb1"
       "\xc3\xa5\x74\xc3\xae\xc3\xb6\xc3\xb1\xc3\xa5\x6c\xc3\xae"
       "\xc2\x9e\xc3\xa5\x74\xc3\xae\xc3\xb6\xc3\xb1";
 
   int64 metahandle;
   // 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());
     parent.Put(IS_UNSYNCED, true);
     metahandle = parent.Get(META_HANDLE);
   }
 
   mock_server_->SetCommitTimeRename(i18nString);
   SyncShareNudge();
 
@@ skipping 10 matching lines @@
   }
 }
 
 // A commit with a lost response produces an update that has to be reunited with
 // its parent.
 TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
   // Create a folder in the root.
   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());
     entry.Put(IS_UNSYNCED, true);
     metahandle_folder = entry.Get(META_HANDLE);
   }
 
   // Verify it and pull the ID out of the folder.
   syncable::Id folder_id;
   int64 metahandle_entry;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_HANDLE, metahandle_folder);
     ASSERT_TRUE(entry.good());
     folder_id = entry.Get(ID);
     ASSERT_TRUE(!folder_id.ServerKnows());
   }
 
   // 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);
   }
 
   // Verify it and pull the ID out of the entry.
   syncable::Id entry_id;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, syncable::GET_BY_HANDLE, metahandle_entry);
@@ skipping 48 matching lines @@
   }
 }
 
 // A commit with a lost response produces an update that has to be reunited with
 // its parent.
 TEST_F(SyncerTest, CommitReuniteUpdate) {
   // Create an entry in the root.
   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);
   }
 
   // Verify it and pull the ID out.
   syncable::Id entry_id;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
 
@@ skipping 33 matching lines @@
 // A commit with a lost response must work even if the local entry was deleted
 // before the update is applied. We should not duplicate the local entry in
 // this case, but just create another one alongside. We may wish to examine
 // this behavior in the future as it can create hanging uploads that never
 // finish, that must be cleaned up on the server side after some time.
 TEST_F(SyncerTest, CommitReuniteUpdateDoesNotChokeOnDeletedLocalEntry) {
   // Create a entry in the root.
   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);
   }
   // Verify it and pull the ID out.
   syncable::Id entry_id;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_HANDLE, entry_metahandle);
     ASSERT_TRUE(entry.good());
@@ skipping 143 matching lines @@
  public:
   void CreateFolderInBob() {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry bob(&trans,
                      syncable::GET_BY_ID,
                      GetOnlyEntryWithName(&trans,
                                           TestIdFactory::root(),
                                           "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);
     entry2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
   }
 };
 
 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);
     entry.Put(syncable::IS_UNSYNCED, true);
     entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   mock_server_->SetMidCommitCallback(
       base::Bind(&EntryCreatedInNewFolderTest::CreateFolderInBob,
                  base::Unretained(this)));
@@ skipping 18 matching lines @@
   mock_server_->AddUpdateBookmark(-10, 0, "bad", 40, 40);
   SyncShareNudge();
   // The negative id would make us CHECK!
 }
 
 TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
   int64 metahandle_fred;
   {
     // 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);
     WriteTestDataToEntry(&trans, &fred_match);
   }
   // Commit it.
   SyncShareNudge();
   EXPECT_EQ(1u, mock_server_->committed_ids().size());
   mock_server_->set_conflict_all_commits(true);
   syncable::Id fred_match_id;
@@ skipping 15 matching lines @@
 
 /**
  * In the event that we have a double changed entry, that is changed on both
  * the client and the server, the conflict resolver should just drop one of
  * them and accept the other.
  */
 
 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);
     WriteTestDataToEntry(&wtrans, &child);
   }
   mock_server_->AddUpdateBookmark(child_id_, parent_id_, "Pete2.htm", 11, 10);
   mock_server_->set_conflict_all_commits(true);
   SyncShareNudge();
   syncable::Directory::ChildHandles children;
   {
@@ skipping 14 matching lines @@
   saw_syncer_event_ = false;
 }
 
 // We got this repro case when someone was editing bookmarks while sync was
 // occuring. The entry had changed out underneath the user.
 TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
   const base::Time& test_time = ProtoTimeToTime(123456);
   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);
     entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     entry.Put(syncable::IS_UNSYNCED, true);
     entry.Put(syncable::MTIME, test_time);
     entry_metahandle = entry.Get(META_HANDLE);
   }
   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);
     ASSERT_TRUE(entry.good());
     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());
     Entry entry(&trans, syncable::GET_BY_ID, id);
     ASSERT_TRUE(entry.good());
     EXPECT_TRUE(entry.Get(MTIME) == test_time);
   }
 }
 
 TEST_F(SyncerTest, ParentAndChildBothMatch) {
   const FullModelTypeSet all_types = FullModelTypeSet::All();
   syncable::Id parent_id = ids_.NewServerId();
   syncable::Id child_id = ids_.NewServerId();
 
   {
     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);
     parent.Put(ID, parent_id);
     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);
     child.Put(SPECIFICS, DefaultBookmarkSpecifics());
     WriteTestDataToEntry(&wtrans, &child);
   }
   mock_server_->AddUpdateDirectory(parent_id, root_id_, "Folder", 10, 10);
   mock_server_->AddUpdateBookmark(child_id, parent_id, "test.htm", 10, 10);
   mock_server_->set_conflict_all_commits(true);
   SyncShareNudge();
@@ skipping 12 matching lines @@
     syncable::Directory::UnsyncedMetaHandles unsynced;
     directory()->GetUnsyncedMetaHandles(&trans, &unsynced);
     EXPECT_EQ(0u, unsynced.size());
     saw_syncer_event_ = false;
   }
 }
 
 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);
   }
   SyncShareNudge();
   EXPECT_EQ(0u, mock_server_->committed_ids().size());
 }
 
 // Original problem synopsis:
 // Check failed: entry->Get(BASE_VERSION) <= entry->Get(SERVER_VERSION)
 // Client creates entry, client finishes committing entry. Between
 // commit and getting update back, we delete the entry.
 // We get the update for the entry, but the local one was modified
 // so we store the entry but don't apply it. IS_UNAPPLIED_UPDATE is set.
 // We commit deletion and get a new version number.
 // We apply unapplied updates again before we get the update about the deletion.
 // This means we have an unapplied update where server_version < base_version.
 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);
     entry.Put(SERVER_PARENT_ID, ids_.FromNumber(9999));  // Bad parent.
     entry.Put(IS_UNSYNCED, true);
     entry.Put(IS_UNAPPLIED_UPDATE, true);
     entry.Put(SPECIFICS, DefaultBookmarkSpecifics());
     entry.Put(SERVER_SPECIFICS, DefaultBookmarkSpecifics());
     entry.Put(IS_DEL, false);
   }
@@ skipping 11 matching lines @@
 //   make a new folder fred
 //   move bob into fred
 //   remove bob
 //   remove fred
 // if no syncing occured midway, bob will have an illegal parent
 TEST_F(SyncerTest, DeletingEntryInFolder) {
   // This test is a little fake.
   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());
     entry.Put(IS_UNSYNCED, true);
     existing_metahandle = entry.Get(META_HANDLE);
   }
   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());
     newfolder.Put(IS_UNSYNCED, true);
 
     MutableEntry existing(&trans, GET_BY_HANDLE, existing_metahandle);
     ASSERT_TRUE(existing.good());
     existing.Put(PARENT_ID, newfolder.Get(ID));
     existing.Put(IS_UNSYNCED, true);
     EXPECT_TRUE(existing.Get(ID).ServerKnows());
 
     newfolder.Put(IS_DEL, true);
     existing.Put(IS_DEL, true);
   }
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
   EXPECT_EQ(0, status().num_server_conflicts());
 }
 
 TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
   int64 newfolder_metahandle;
 
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
   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);
     newfolder.Put(SPECIFICS, DefaultBookmarkSpecifics());
     newfolder_metahandle = newfolder.Get(META_HANDLE);
   }
   mock_server_->AddUpdateDirectory(1, 0, "bob", 2, 20);
   mock_server_->SetLastUpdateDeleted();
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, APPLY_UPDATES);
   {
@@ skipping 70 matching lines @@
 // we post more than one commit command to the server.  This test makes
 // sure that scenario works as expected.
 TEST_F(SyncerTest, CommitManyItemsInOneGo_Success) {
   uint32 num_batches = 3;
   uint32 items_to_commit = kDefaultMaxCommitBatchSize * num_batches;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     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());
     }
   }
   ASSERT_EQ(items_to_commit, directory()->unsynced_entity_count());
 
   SyncShareNudge();
   EXPECT_EQ(num_batches, mock_server_->commit_messages().size());
   EXPECT_EQ(0, directory()->unsynced_entity_count());
 }
 
 // Test that a single failure to contact the server will cause us to exit the
 // commit loop immediately.
 TEST_F(SyncerTest, CommitManyItemsInOneGo_PostBufferFail) {
   uint32 num_batches = 3;
   uint32 items_to_commit = kDefaultMaxCommitBatchSize * num_batches;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     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());
     }
   }
   ASSERT_EQ(items_to_commit, directory()->unsynced_entity_count());
 
   // The second commit should fail.  It will be preceded by one successful
   // GetUpdate and one succesful commit.
   mock_server_->FailNthPostBufferToPathCall(3);
   SyncShareNudge();
 
   EXPECT_EQ(1U, mock_server_->commit_messages().size());
   EXPECT_EQ(SYNC_SERVER_ERROR,
             session_->status_controller().model_neutral_state().commit_result);
   EXPECT_EQ(items_to_commit - kDefaultMaxCommitBatchSize,
             directory()->unsynced_entity_count());
 }
 
 // Test that a single conflict response from the server will cause us to exit
 // the commit loop immediately.
 TEST_F(SyncerTest, CommitManyItemsInOneGo_CommitConflict) {
   uint32 num_batches = 2;
   uint32 items_to_commit = kDefaultMaxCommitBatchSize * num_batches;
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     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());
     }
   }
   ASSERT_EQ(items_to_commit, directory()->unsynced_entity_count());
 
   // Return a CONFLICT response for the first item.
   mock_server_->set_conflict_n_commits(1);
   SyncShareNudge();
@@ skipping 77 matching lines @@
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10);
   SyncShareNudge();
   int64 local_folder_handle;
   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);
     new_entry.Put(IS_UNSYNCED, true);
     new_entry.Put(SPECIFICS, DefaultBookmarkSpecifics());
     MutableEntry old(&wtrans, GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(old.good());
     WriteTestDataToEntry(&wtrans, &old);
   }
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20);
@@ skipping 45 matching lines @@
 
 // Same as NewEntryAnddServerEntrySharePath, but using the old-style protocol.
 TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   mock_server_->set_use_legacy_bookmarks_protocol(true);
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10);
   SyncShareNudge();
   int64 local_folder_handle;
   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);
     new_entry.Put(IS_UNSYNCED, true);
     new_entry.Put(SPECIFICS, DefaultBookmarkSpecifics());
     MutableEntry old(&wtrans, GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(old.good());
     WriteTestDataToEntry(&wtrans, &old);
   }
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20);
@@ skipping 151 matching lines @@
     EXPECT_EQ(2, bob.Get(BASE_VERSION));
   }
   saw_syncer_event_ = false;
 }
 
 // This test is to reproduce a check failure. Sometimes we would get a bad ID
 // back when creating an entry.
 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);
     folder2.Put(SPECIFICS, DefaultBookmarkSpecifics());
     folder2.Put(BASE_VERSION, 3);
     folder2.Put(ID, syncable::Id::CreateFromServerId("mock_server:10000"));
   }
   mock_server_->set_next_new_id(10000);
   EXPECT_EQ(1u, directory()->unsynced_entity_count());
   // we get back a bad id in here (should never happen).
@@ skipping 18 matching lines @@
   }
   mock_server_->AddUpdateDirectory(2, 1, "fred", 1, 10);
   SyncShareNudge();
   SyncShareNudge();
 }
 
 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);
     local_deleted.Put(IS_DIR, false);
     local_deleted.Put(IS_UNSYNCED, true);
     local_deleted.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   mock_server_->AddUpdateBookmark(ids_.FromNumber(1), root_id_, "name", 10, 10);
 
@@ skipping 12 matching lines @@
   }
 }
 
 // See what happens if the IS_DIR bit gets flipped.  This can cause us
 // all kinds of disasters.
 TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
   // Local object: a deleted directory (container), revision 1, unsynced.
   {
     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);
     local_deleted.Put(IS_DIR, true);
     local_deleted.Put(IS_UNSYNCED, true);
     local_deleted.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   // Server update: entry-type object (not a container), revision 10.
   mock_server_->AddUpdateBookmark(ids_.FromNumber(1), root_id_, "name", 10, 10);
@@ skipping 17 matching lines @@
 
 // Bug Synopsis:
 // Merge conflict resolution will merge a new local entry with another entry
 // that needs updates, resulting in CHECK.
 TEST_F(SyncerTest, MergingExistingItems) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10);
   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);
   SyncRepeatedlyToTriggerConflictResolution(session_.get());
 }
 
 // In this test a long changelog contains a child at the start of the changelog
 // and a parent at the end. While these updates are in progress the client would
 // appear stuck.
 TEST_F(SyncerTest, LongChangelistWithApplicationConflict) {
@@ skipping 169 matching lines @@
   }
 }
 
 TEST_F(SyncerTest, DirectoryCommitTest) {
   syncable::Id in_root_id, in_dir_id;
   int64 foo_metahandle;
   int64 bar_metahandle;
 
   {
     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);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     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);
     child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     bar_metahandle = child.Get(META_HANDLE);
     in_dir_id = parent.Get(syncable::ID);
   }
   SyncShareNudge();
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ skipping 91 matching lines @@
       "folder_two", 1, 1);
   SyncShareNudge();
   {
     // A moved entry should send an "old parent."
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, GET_BY_ID, folder_one_id);
     ASSERT_TRUE(entry.good());
     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());
   }
   SyncShareNudge();
   const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit();
   ASSERT_EQ(2, commit.entries_size());
   EXPECT_TRUE(commit.entries(0).parent_id_string() == "2");
   EXPECT_TRUE(commit.entries(0).old_parent_id() == "0");
   EXPECT_FALSE(commit.entries(1).has_old_parent_id());
 }
 
 TEST_F(SyncerTest, Test64BitVersionSupport) {
   int64 really_big_int = std::numeric_limits<int64>::max() - 12;
   const string name("ringo's dang orang ran rings around my o-ring");
   int64 item_metahandle;
 
   // 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);
     entry.Put(syncable::ID, ids_.NewServerId());
     item_metahandle = entry.Get(META_HANDLE);
   }
   // Now read it back out and make sure the value is max int64.
   syncable::ReadTransaction rtrans(FROM_HERE, directory());
   Entry entry(&rtrans, syncable::GET_BY_HANDLE, item_metahandle);
   ASSERT_TRUE(entry.good());
@@ skipping 190 matching lines @@
 TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
   int64 original_metahandle = 0;
 
   sync_pb::EntitySpecifics local_bookmark(DefaultBookmarkSpecifics());
   local_bookmark.mutable_bookmark()->set_url("http://foo/localsite");
   sync_pb::EntitySpecifics server_bookmark(DefaultBookmarkSpecifics());
   server_bookmark.mutable_bookmark()->set_url("http://bar/serversite");
 
   {
     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);
     perm_folder.Put(IS_UNSYNCED, true);
     EXPECT_FALSE(perm_folder.Get(IS_UNAPPLIED_UPDATE));
     EXPECT_FALSE(perm_folder.Get(ID).ServerKnows());
     original_metahandle = perm_folder.Get(META_HANDLE);
   }
 
   mock_server_->AddUpdateBookmark(1, 0, "permitem_renamed", 10, 100);
@@ skipping 44 matching lines @@
     EXPECT_EQ(local_bookmark.SerializeAsString(),
               perm_folder.Get(SPECIFICS).SerializeAsString());
     EXPECT_TRUE(perm_folder.Get(ID).ServerKnows());
   }
 }
 
 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");
     perm_folder.Put(SPECIFICS, DefaultBookmarkSpecifics());
     perm_folder.Put(IS_UNSYNCED, true);
 
     // Note: IS_DEL && !ServerKnows() will clear the UNSYNCED bit.
     // (We never attempt to commit server-unknown deleted items, so this
     // helps us clean up those entries).
     perm_folder.Put(IS_DEL, true);
@@ skipping 441 matching lines @@
 //        - commitresponse - getupdates.
 class SyncerUndeletionTest : public SyncerTest {
  public:
   SyncerUndeletionTest()
       : client_tag_("foobar"),
         metahandle_(syncable::kInvalidMetaHandle) {
   }
 
   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);
     perm_folder.Put(SYNCING, false);
     perm_folder.Put(SPECIFICS, DefaultBookmarkSpecifics());
     EXPECT_FALSE(perm_folder.Get(IS_UNAPPLIED_UPDATE));
     EXPECT_FALSE(perm_folder.Get(ID).ServerKnows());
     metahandle_ = perm_folder.Get(META_HANDLE);
   }
 
@@ skipping 400 matching lines @@
                                   Get(metahandle_, PARENT_ID),
                                   "Thadeusz", 100, 1000);
   mock_server_->SetLastUpdateClientTag(client_tag_);
   SyncShareNudge();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
   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