WIP: The Bookmark Position Megapatch

sync/internal_api/sync_manager_impl_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.
 
 // Unit tests for the SyncApi. Note that a lot of the underlying
 // functionality is provided by the Syncable layer, which has its own
 // unit tests. We'll test SyncApi specific things in this harness.
 
 #include <cstddef>
 #include <map>
@@ skipping 45 matching lines @@
 #include "sync/protocol/preference_specifics.pb.h"
 #include "sync/protocol/proto_value_conversions.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/nigori_util.h"
 #include "sync/syncable/read_transaction.h"
 #include "sync/syncable/syncable_id.h"
+#include "sync/syncable/syncable_util.h"
 #include "sync/syncable/write_transaction.h"
 #include "sync/test/callback_counter.h"
 #include "sync/test/engine/fake_sync_scheduler.h"
 #include "sync/test/fake_encryptor.h"
 #include "sync/test/fake_extensions_activity_monitor.h"
 #include "sync/util/cryptographer.h"
 #include "sync/util/extensions_activity_monitor.h"
 #include "sync/util/test_unrecoverable_error_handler.h"
 #include "sync/util/time.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ skipping 41 matching lines @@
 // newly-created node.
 int64 MakeNode(UserShare* share,
                ModelType model_type,
                const std::string& client_tag) {
   WriteTransaction trans(FROM_HERE, share);
   ReadNode root_node(&trans);
   root_node.InitByRootLookup();
   WriteNode node(&trans);
   WriteNode::InitUniqueByCreationResult result =
       node.InitUniqueByCreation(model_type, root_node, client_tag);
-  EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
+  DCHECK_EQ(WriteNode::INIT_SUCCESS, result);
   node.SetIsFolder(false);
   return node.GetId();
 }
 
 // Makes a non-folder child of a non-root node. Returns the id of the
 // newly-created node.
 int64 MakeNodeWithParent(UserShare* share,
                          ModelType model_type,
-                         const std::string& client_tag,
                          int64 parent_id) {
+  DCHECK_EQ(model_type, BOOKMARKS); // Only bookmarks have hierarchy.
   WriteTransaction trans(FROM_HERE, share);
   ReadNode parent_node(&trans);
-  EXPECT_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
+  DCHECK_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
   WriteNode node(&trans);
-  WriteNode::InitUniqueByCreationResult result =
-      node.InitUniqueByCreation(model_type, parent_node, client_tag);
-  EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
+  DCHECK(node.InitBookmarkByCreation(parent_node, NULL));
   node.SetIsFolder(false);
   return node.GetId();
 }
 
 // Makes a folder child of a non-root node. Returns the id of the
 // newly-created node.
 int64 MakeFolderWithParent(UserShare* share,
                            ModelType model_type,
                            int64 parent_id,
                            BaseNode* predecessor) {
   WriteTransaction trans(FROM_HERE, share);
   ReadNode parent_node(&trans);
-  EXPECT_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
+  DCHECK_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
   WriteNode node(&trans);
-  EXPECT_TRUE(node.InitByCreation(model_type, parent_node, predecessor));
+  DCHECK(node.InitBookmarkByCreation(parent_node, predecessor));
   node.SetIsFolder(true);
   return node.GetId();
 }
 
+int64 MakeBookmarkWithParent(UserShare* share,
+                             int64 parent_id,
+                             BaseNode* predecessor) {
+  WriteTransaction trans(FROM_HERE, share);
+  ReadNode parent_node(&trans);
+  DCHECK_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
+  WriteNode node(&trans);
+  DCHECK(node.InitBookmarkByCreation(parent_node, predecessor));
+  return node.GetId();
+}
+
 // Creates the "synced" root node for a particular datatype. We use the syncable
 // methods here so that the syncer treats these nodes as if they were already
 // received from the server.
 int64 MakeServerNodeForType(UserShare* share,
                             ModelType model_type) {
   sync_pb::EntitySpecifics specifics;
   AddDefaultFieldValue(model_type, &specifics);
   syncable::WriteTransaction trans(
       FROM_HERE, syncable::UNITTEST, share->directory.get());
   // Attempt to lookup by nigori tag.
   std::string type_tag = ModelTypeToRootTag(model_type);
   syncable::Id node_id = syncable::Id::CreateFromServerId(type_tag);
   syncable::MutableEntry entry(&trans, syncable::CREATE_NEW_UPDATE_ITEM,
                                node_id);
   EXPECT_TRUE(entry.good());
   entry.Put(syncable::BASE_VERSION, 1);
   entry.Put(syncable::SERVER_VERSION, 1);
   entry.Put(syncable::IS_UNAPPLIED_UPDATE, false);
   entry.Put(syncable::SERVER_PARENT_ID, syncable::GetNullId());
   entry.Put(syncable::SERVER_IS_DIR, true);
   entry.Put(syncable::IS_DIR, true);
   entry.Put(syncable::SERVER_SPECIFICS, specifics);
   entry.Put(syncable::UNIQUE_SERVER_TAG, type_tag);
   entry.Put(syncable::NON_UNIQUE_NAME, type_tag);
   entry.Put(syncable::IS_DEL, false);
   entry.Put(syncable::SPECIFICS, specifics);
   return entry.Get(syncable::META_HANDLE);
 }
 
+int64 GetServerNodeForType(UserShare* share, ModelType type) {
+  ReadTransaction trans(FROM_HERE, share);
+  ReadNode bookmark_root(&trans);
+  EXPECT_EQ(BaseNode::INIT_OK,
+            bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
+  return bookmark_root.GetId();
+}
+
 // Simulates creating a "synced" node as a child of the root datatype node.
 int64 MakeServerNode(UserShare* share, ModelType model_type,
                      const std::string& client_tag,
                      const std::string& hashed_tag,
                      const sync_pb::EntitySpecifics& specifics) {
   syncable::WriteTransaction trans(
       FROM_HERE, syncable::UNITTEST, share->directory.get());
   syncable::Entry root_entry(&trans, syncable::GET_BY_SERVER_TAG,
                              ModelTypeToRootTag(model_type));
   EXPECT_TRUE(root_entry.good());
@@ skipping 54 matching lines @@
 
 TEST_F(SyncApiTest, BasicTagWrite) {
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
   ignore_result(MakeNode(test_user_share_.user_share(),
-                         BOOKMARKS, "testtag"));
+                         AUTOFILL, "testtag"));
 
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              node.InitByClientTagLookup(BOOKMARKS, "testtag"));
+              node.InitByClientTagLookup(AUTOFILL, "testtag"));
 
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_NE(node.GetId(), 0);
     EXPECT_EQ(node.GetId(), root_node.GetFirstChildId());
   }
 }
 
 TEST_F(SyncApiTest, GenerateSyncableHash) {
-  EXPECT_EQ("OyaXV5mEzrPS4wbogmtKvRfekAI=",
-      BaseNode::GenerateSyncableHash(BOOKMARKS, "tag1"));
+  EXPECT_EQ("TrJxl8908A2qDbaLzjxjyUzskpY=",
+            syncable::GenerateSyncableHash(THEMES, "tag1"));
   EXPECT_EQ("iNFQtRFQb+IZcn1kKUJEZDDkLs4=",
-      BaseNode::GenerateSyncableHash(PREFERENCES, "tag1"));
+            syncable::GenerateSyncableHash(PREFERENCES, "tag1"));
   EXPECT_EQ("gO1cPZQXaM73sHOvSA+tKCKFs58=",
-      BaseNode::GenerateSyncableHash(AUTOFILL, "tag1"));
+            syncable::GenerateSyncableHash(AUTOFILL, "tag1"));
 
-  EXPECT_EQ("A0eYIHXM1/jVwKDDp12Up20IkKY=",
-      BaseNode::GenerateSyncableHash(BOOKMARKS, "tag2"));
+  EXPECT_EQ("YCbRusldaxqFnrd36Y7cJagN8TA=",
+            syncable::GenerateSyncableHash(THEMES, "tag2"));
   EXPECT_EQ("XYxkF7bhS4eItStFgiOIAU23swI=",
-      BaseNode::GenerateSyncableHash(PREFERENCES, "tag2"));
+            syncable::GenerateSyncableHash(PREFERENCES, "tag2"));
   EXPECT_EQ("GFiWzo5NGhjLlN+OyCfhy28DJTQ=",
-      BaseNode::GenerateSyncableHash(AUTOFILL, "tag2"));
+            syncable::GenerateSyncableHash(AUTOFILL, "tag2"));
 }
 
 TEST_F(SyncApiTest, ModelTypesSiloed) {
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
   ignore_result(MakeNode(test_user_share_.user_share(),
-                         BOOKMARKS, "collideme"));
+                         THEMES, "collideme"));
   ignore_result(MakeNode(test_user_share_.user_share(),
                          PREFERENCES, "collideme"));
   ignore_result(MakeNode(test_user_share_.user_share(),
                          AUTOFILL, "collideme"));
 
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
 
     ReadNode bookmarknode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              bookmarknode.InitByClientTagLookup(BOOKMARKS,
+              bookmarknode.InitByClientTagLookup(THEMES,
                   "collideme"));
 
     ReadNode prefnode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               prefnode.InitByClientTagLookup(PREFERENCES,
                   "collideme"));
 
     ReadNode autofillnode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               autofillnode.InitByClientTagLookup(AUTOFILL,
                   "collideme"));
 
     EXPECT_NE(bookmarknode.GetId(), prefnode.GetId());
     EXPECT_NE(autofillnode.GetId(), prefnode.GetId());
     EXPECT_NE(bookmarknode.GetId(), autofillnode.GetId());
   }
 }
 
 TEST_F(SyncApiTest, ReadMissingTagsFails) {
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
-              node.InitByClientTagLookup(BOOKMARKS,
+              node.InitByClientTagLookup(THEMES,
                   "testtag"));
   }
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
-              node.InitByClientTagLookup(BOOKMARKS,
+              node.InitByClientTagLookup(THEMES,
                   "testtag"));
   }
 }
 
 // TODO(chron): Hook this all up to the server and write full integration tests
 //              for update->undelete behavior.
 TEST_F(SyncApiTest, TestDeleteBehavior) {
   int64 node_id;
   int64 folder_id;
   std::string test_title("test1");
 
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
     // we'll use this spare folder later
     WriteNode folder_node(&trans);
-    EXPECT_TRUE(folder_node.InitByCreation(BOOKMARKS,
-        root_node, NULL));
+    EXPECT_TRUE(folder_node.InitBookmarkByCreation(root_node, NULL));
     folder_id = folder_node.GetId();
 
     WriteNode wnode(&trans);
     WriteNode::InitUniqueByCreationResult result =
-        wnode.InitUniqueByCreation(BOOKMARKS, root_node, "testtag");
+        wnode.InitUniqueByCreation(AUTOFILL, root_node, "testtag");
     EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
     wnode.SetIsFolder(false);
     wnode.SetTitle(UTF8ToWide(test_title));
 
     node_id = wnode.GetId();
   }
 
   // Ensure we can delete something with a tag.
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     WriteNode wnode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              wnode.InitByClientTagLookup(BOOKMARKS,
-                  "testtag"));
+              wnode.InitByClientTagLookup(AUTOFILL, "testtag"));
     EXPECT_FALSE(wnode.GetIsFolder());
     EXPECT_EQ(wnode.GetTitle(), test_title);
 
     wnode.Remove();
   }
 
   // Lookup of a node which was deleted should return failure,
   // but have found some data about the node.
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_IS_DEL,
-              node.InitByClientTagLookup(BOOKMARKS,
-                  "testtag"));
+              node.InitByClientTagLookup(AUTOFILL, "testtag"));
     // Note that for proper function of this API this doesn't need to be
     // filled, we're checking just to make sure the DB worked in this test.
     EXPECT_EQ(node.GetTitle(), test_title);
   }
 
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode folder_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK, folder_node.InitByIdLookup(folder_id));
 
     WriteNode wnode(&trans);
     // This will undelete the tag.
     WriteNode::InitUniqueByCreationResult result =
-        wnode.InitUniqueByCreation(BOOKMARKS, folder_node, "testtag");
+        wnode.InitUniqueByCreation(AUTOFILL, folder_node, "testtag");
     EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
     EXPECT_EQ(wnode.GetIsFolder(), false);
     EXPECT_EQ(wnode.GetParentId(), folder_node.GetId());
     EXPECT_EQ(wnode.GetId(), node_id);
     EXPECT_NE(wnode.GetTitle(), test_title);  // Title should be cleared
     wnode.SetTitle(UTF8ToWide(test_title));
   }
 
   // Now look up should work.
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              node.InitByClientTagLookup(BOOKMARKS,
+              node.InitByClientTagLookup(AUTOFILL,
                     "testtag"));
     EXPECT_EQ(node.GetTitle(), test_title);
-    EXPECT_EQ(node.GetModelType(), BOOKMARKS);
+    EXPECT_EQ(node.GetModelType(), AUTOFILL);
   }
 }
 
 TEST_F(SyncApiTest, WriteAndReadPassword) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   {
@@ skipping 24 matching lines @@
   }
 }
 
 TEST_F(SyncApiTest, WriteEncryptedTitle) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   test_user_share_.encryption_handler()->EnableEncryptEverything();
+  int bookmark_id;
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
     WriteNode bookmark_node(&trans);
-    WriteNode::InitUniqueByCreationResult result =
-        bookmark_node.InitUniqueByCreation(BOOKMARKS,
-                                           root_node, "foo");
-    EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
+    EXPECT_TRUE(bookmark_node.InitBookmarkByCreation(root_node, NULL));
+    bookmark_id = bookmark_node.GetId();
     bookmark_node.SetTitle(UTF8ToWide("foo"));
 
     WriteNode pref_node(&trans);
-    result =
+    WriteNode::InitUniqueByCreationResult result =
         pref_node.InitUniqueByCreation(PREFERENCES, root_node, "bar");
     EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
     pref_node.SetTitle(UTF8ToWide("bar"));
   }
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
     ReadNode bookmark_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              bookmark_node.InitByClientTagLookup(BOOKMARKS,
-                                                  "foo"));
+              bookmark_node.InitByIdLookup(bookmark_id));
     EXPECT_EQ("foo", bookmark_node.GetTitle());
     EXPECT_EQ(kEncryptedString,
               bookmark_node.GetEntry()->Get(syncable::NON_UNIQUE_NAME));
 
     ReadNode pref_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               pref_node.InitByClientTagLookup(PREFERENCES,
                                               "bar"));
     EXPECT_EQ(kEncryptedString, pref_node.GetTitle());
   }
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecifics) {
   int64 child_id = MakeNode(test_user_share_.user_share(),
-                            BOOKMARKS, "testtag");
+                            THEMES, "testtag");
   WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
 
+
   sync_pb::EntitySpecifics entity_specifics;
-  entity_specifics.mutable_bookmark()->set_url("http://www.google.com");
+  entity_specifics.mutable_theme()->set_custom_theme_name("ThemeName");
 
   EXPECT_NE(entity_specifics.SerializeAsString(),
             node.GetEntitySpecifics().SerializeAsString());
   node.SetEntitySpecifics(entity_specifics);
   EXPECT_EQ(entity_specifics.SerializeAsString(),
             node.GetEntitySpecifics().SerializeAsString());
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) {
   int64 child_id = MakeNode(test_user_share_.user_share(),
-                            BOOKMARKS, "testtag");
+                            THEMES, "testtag");
   WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
   EXPECT_TRUE(node.GetEntitySpecifics().unknown_fields().empty());
 
   sync_pb::EntitySpecifics entity_specifics;
-  entity_specifics.mutable_bookmark()->set_url("http://www.google.com");
+  entity_specifics.mutable_theme()->set_custom_theme_name("ThemeName");
   entity_specifics.mutable_unknown_fields()->AddFixed32(5, 100);
   node.SetEntitySpecifics(entity_specifics);
   EXPECT_FALSE(node.GetEntitySpecifics().unknown_fields().empty());
 
   entity_specifics.mutable_unknown_fields()->Clear();
   node.SetEntitySpecifics(entity_specifics);
   EXPECT_FALSE(node.GetEntitySpecifics().unknown_fields().empty());
 }
 
 namespace {
@@ skipping 19 matching lines @@
     } else if (expected_model_type == UNSPECIFIED) {
       EXPECT_EQ("Unspecified", type_str);
     } else {
       ADD_FAILURE();
     }
   }
   if (is_detailed) {
     ExpectInt64Value(node.GetParentId(), value, "parentId");
     ExpectTimeValue(node.GetModificationTime(), value, "modificationTime");
     ExpectInt64Value(node.GetExternalId(), value, "externalId");
-    ExpectInt64Value(node.GetPredecessorId(), value, "predecessorId");
-    ExpectInt64Value(node.GetSuccessorId(), value, "successorId");
-    ExpectInt64Value(node.GetFirstChildId(), value, "firstChildId");
     {
       scoped_ptr<DictionaryValue> expected_entry(
           node.GetEntry()->ToValue(NULL));
       const Value* entry = NULL;
       EXPECT_TRUE(value.Get("entry", &entry));
       EXPECT_TRUE(Value::Equals(entry, expected_entry.get()));
     }
-    EXPECT_EQ(11u, value.size());
+    EXPECT_EQ(8u, value.size());
   } else {
     EXPECT_EQ(4u, value.size());
   }
 }
 
 }  // namespace
 
 TEST_F(SyncApiTest, BaseNodeGetSummaryAsValue) {
   ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
   ReadNode node(&trans);
@@ skipping 76 matching lines @@
                                       NULL);
   ignore_result(MakeFolderWithParent(test_user_share_.user_share(),
                                      BOOKMARKS,
                                      type_root,
                                      NULL));
   int64 child1 = MakeFolderWithParent(
       test_user_share_.user_share(),
       BOOKMARKS,
       parent,
       NULL);
-  ignore_result(MakeNodeWithParent(
+  ignore_result(MakeBookmarkWithParent(
       test_user_share_.user_share(),
-      BOOKMARKS,
-      "c2",
-      parent));
-  ignore_result(MakeNodeWithParent(
+      parent,
+      NULL));
+  ignore_result(MakeBookmarkWithParent(
       test_user_share_.user_share(),
-      BOOKMARKS,
-      "c1c1",
-      child1));
+      child1,
+      NULL));
 
   {
     ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
     EXPECT_EQ(6, type_root_node.GetTotalNodeCount());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByIdLookup(parent));
@@ skipping 239 matching lines @@
   }
 
   // Looks up an entry by client tag and resets IS_UNSYNCED value to false.
   // Returns true if entry was previously unsynced, false if IS_UNSYNCED was
   // already false.
   bool ResetUnsyncedEntry(ModelType type,
                           const std::string& client_tag) {
     UserShare* share = sync_manager_.GetUserShare();
     syncable::WriteTransaction trans(
         FROM_HERE, syncable::UNITTEST, share->directory.get());
-    const std::string hash = BaseNode::GenerateSyncableHash(type, client_tag);
+    const std::string hash = syncable::GenerateSyncableHash(type, client_tag);
     syncable::MutableEntry entry(&trans, syncable::GET_BY_CLIENT_TAG,
                                  hash);
     EXPECT_TRUE(entry.good());
     if (!entry.Get(IS_UNSYNCED))
       return false;
     entry.Put(IS_UNSYNCED, false);
     return true;
   }
 
   virtual InternalComponentsFactory* GetFactory() {
@@ skipping 162 matching lines @@
   void RunGetNodesByIdTest(const char* message_name, bool is_detailed) {
     int64 root_id = kInvalidId;
     {
       ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
       ReadNode root_node(&trans);
       root_node.InitByRootLookup();
       root_id = root_node.GetId();
     }
 
     int64 child_id =
-        MakeNode(sync_manager_.GetUserShare(), BOOKMARKS, "testtag");
+        MakeNode(sync_manager_.GetUserShare(), THEMES, "testtag");
 
     StrictMock<MockJsReplyHandler> reply_handler;
 
     JsArgList return_args;
 
     const int64 ids[] = { root_id, child_id };
 
     EXPECT_CALL(reply_handler,
                 HandleJsReply(message_name, _))
         .Times(arraysize(ids)).WillRepeatedly(SaveArg<1>(&return_args));
@@ skipping 394 matching lines @@
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
 
   // Create some unencrypted unsynced data.
   int64 folder = MakeFolderWithParent(sync_manager_.GetUserShare(),
                                       BOOKMARKS,
                                       GetIdForDataType(BOOKMARKS),
                                       NULL);
   // First batch_size nodes are children of folder.
   size_t i;
   for (i = 0; i < batch_size; ++i) {
-    MakeNodeWithParent(sync_manager_.GetUserShare(), BOOKMARKS,
-                       base::StringPrintf("%"PRIuS"", i), folder);
+    MakeNodeWithParent(sync_manager_.GetUserShare(), BOOKMARKS, folder);
   }
   // Next batch_size nodes are a different type and on their own.
   for (; i < 2*batch_size; ++i) {
-    MakeNodeWithParent(sync_manager_.GetUserShare(), SESSIONS,
-                       base::StringPrintf("%"PRIuS"", i),
-                       GetIdForDataType(SESSIONS));
+    MakeNode(sync_manager_.GetUserShare(), SESSIONS,
+             base::StringPrintf("%"PRIuS"", i));
   }
   // Last batch_size nodes are a third type that will not need encryption.
   for (; i < 3*batch_size; ++i) {
-    MakeNodeWithParent(sync_manager_.GetUserShare(), THEMES,
-                       base::StringPrintf("%"PRIuS"", i),
-                       GetIdForDataType(THEMES));
+    MakeNode(sync_manager_.GetUserShare(), THEMES,
+             base::StringPrintf("%"PRIuS"", i));
   }
 
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     EXPECT_TRUE(GetEncryptedTypesWithTrans(&trans).Equals(
         SyncEncryptionHandler::SensitiveTypes()));
     EXPECT_TRUE(syncable::VerifyDataTypeEncryptionForTest(
         trans.GetWrappedTrans(),
         BOOKMARKS,
         false /* not encrypted */));
@@ skipping 475 matching lines @@
 TEST_F(SyncManagerTest, EncryptBookmarksWithLegacyData) {
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
   std::string title;
   SyncAPINameToServerName("Google", &title);
   std::string url = "http://www.google.com";
   std::string raw_title2 = "..";  // An invalid cosmo title.
   std::string title2;
   SyncAPINameToServerName(raw_title2, &title2);
   std::string url2 = "http://www.bla.com";
 
+  int64 parent = GetServerNodeForType(sync_manager_.GetUserShare(), BOOKMARKS);
+
   // Create a bookmark using the legacy format.
-  int64 node_id1 = MakeNode(sync_manager_.GetUserShare(),
-      BOOKMARKS,
-      "testtag");
-  int64 node_id2 = MakeNode(sync_manager_.GetUserShare(),
-      BOOKMARKS,
-      "testtag2");
+  int64 node_id1 = MakeBookmarkWithParent(sync_manager_.GetUserShare(),
+                                          parent, NULL);
+  int64 node_id2 = MakeBookmarkWithParent(sync_manager_.GetUserShare(),
+                                          parent, NULL);
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(node_id1));
 
     sync_pb::EntitySpecifics entity_specifics;
     entity_specifics.mutable_bookmark()->set_url(url);
     node.SetEntitySpecifics(entity_specifics);
 
     // Set the old style title.
@@ skipping 74 matching lines @@
 }
 
 // Create a bookmark and set the title/url, then verify the data was properly
 // set. This replicates the unique way bookmarks have of creating sync nodes.
 // See BookmarkChangeProcessor::PlaceSyncNode(..).
 TEST_F(SyncManagerTest, CreateLocalBookmark) {
   std::string title = "title";
   std::string url = "url";
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
-    ReadNode root_node(&trans);
-    root_node.InitByRootLookup();
+    ReadNode bookmark_root(&trans);
+    ASSERT_EQ(BaseNode::INIT_OK,
+              bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
     WriteNode node(&trans);
-    ASSERT_TRUE(node.InitByCreation(BOOKMARKS, root_node, NULL));
+    ASSERT_TRUE(node.InitBookmarkByCreation(bookmark_root, NULL));
     node.SetIsFolder(false);
     node.SetTitle(UTF8ToWide(title));
 
     sync_pb::BookmarkSpecifics bookmark_specifics(node.GetBookmarkSpecifics());
     bookmark_specifics.set_url(url);
     node.SetBookmarkSpecifics(bookmark_specifics);
   }
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
-    ReadNode root_node(&trans);
-    root_node.InitByRootLookup();
-    int64 child_id = root_node.GetFirstChildId();
+    ReadNode bookmark_root(&trans);
+    ASSERT_EQ(BaseNode::INIT_OK,
+              bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
+    int64 child_id = bookmark_root.GetFirstChildId();
 
     ReadNode node(&trans);
     ASSERT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
     EXPECT_FALSE(node.GetIsFolder());
     EXPECT_EQ(title, node.GetTitle());
     EXPECT_EQ(url, node.GetBookmarkSpecifics().url());
   }
 }
 
 // Verifies WriteNode::UpdateEntryWithEncryption does not make unnecessary
 // changes.
 TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(BOOKMARKS, client_tag));
   // Manually change to the same data. Should not set is_unsynced.
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS, client_tag));
@@ skipping 128 matching lines @@
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     Cryptographer* cryptographer = trans.GetCryptographer();
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     cryptographer->Encrypt(
         data,
         entity_specifics.mutable_password()->
             mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 
   // Manually change to the same data via SetEntitySpecifics. Should not set
   // is_unsynced.
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
@@ skipping 14 matching lines @@
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     Cryptographer* cryptographer = trans.GetCryptographer();
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     cryptographer->Encrypt(
         data,
         entity_specifics.mutable_password()->
             mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 
   // Manually change to the same data via SetPasswordSpecifics. Should not set
   // is_unsynced.
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
@@ skipping 30 matching lines @@
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     Cryptographer* cryptographer = trans.GetCryptographer();
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     cryptographer->Encrypt(
         data,
         entity_specifics.mutable_password()->mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 
   // Set a new passphrase. Should set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
   EXPECT_CALL(encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   EXPECT_CALL(encryption_observer_, OnPassphraseAccepted());
@@ skipping 18 matching lines @@
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     Cryptographer* cryptographer = trans.GetCryptographer();
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     cryptographer->Encrypt(
         data,
         entity_specifics.mutable_password()->mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 
   // Force a re-encrypt everything. Should not set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
   EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
   EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
   EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 }
 
 // Verify SetTitle(..) doesn't unnecessarily set IS_UNSYNCED for bookmarks
 // when we write the same data, but does set it when we write new data.
 TEST_F(SyncManagerTest, SetBookmarkTitle) {
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(BOOKMARKS, client_tag));
 
   // Manually change to the same title. Should not set is_unsynced.
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
@@ skipping 15 matching lines @@
 
 // Verify SetTitle(..) doesn't unnecessarily set IS_UNSYNCED for encrypted
 // bookmarks when we write the same data, but does set it when we write new
 // data.
 TEST_F(SyncManagerTest, SetBookmarkTitleWithEncryption) {
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(BOOKMARKS, client_tag));
 
   // Encrypt the datatatype, should set is_unsynced.
   EXPECT_CALL(encryption_observer_,
               OnEncryptedTypesChanged(
                   HasModelTypes(EncryptableUserTypes()), true));
   EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
@@ skipping 38 matching lines @@
 // Verify SetTitle(..) doesn't unnecessarily set IS_UNSYNCED for non-bookmarks
 // when we write the same data, but does set it when we write new data.
 TEST_F(SyncManagerTest, SetNonBookmarkTitle) {
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_preference()->set_name("name");
   entity_specifics.mutable_preference()->set_value("value");
   MakeServerNode(sync_manager_.GetUserShare(),
                  PREFERENCES,
                  client_tag,
-                 BaseNode::GenerateSyncableHash(PREFERENCES,
+                 syncable::GenerateSyncableHash(PREFERENCES,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PREFERENCES, client_tag));
 
   // Manually change to the same title. Should not set is_unsynced.
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
@@ skipping 17 matching lines @@
 // non-bookmarks when we write the same data or when we write new data
 // data (should remained kEncryptedString).
 TEST_F(SyncManagerTest, SetNonBookmarkTitleWithEncryption) {
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_preference()->set_name("name");
   entity_specifics.mutable_preference()->set_value("value");
   MakeServerNode(sync_manager_.GetUserShare(),
                  PREFERENCES,
                  client_tag,
-                 BaseNode::GenerateSyncableHash(PREFERENCES,
+                 syncable::GenerateSyncableHash(PREFERENCES,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
   EXPECT_FALSE(ResetUnsyncedEntry(PREFERENCES, client_tag));
 
   // Encrypt the datatatype, should set is_unsynced.
   EXPECT_CALL(encryption_observer_,
               OnEncryptedTypesChanged(
                   HasModelTypes(EncryptableUserTypes()), true));
   EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
@@ skipping 50 matching lines @@
     Cryptographer* crypto = trans.GetCryptographer();
     sync_pb::EntitySpecifics bm_specifics;
     bm_specifics.mutable_bookmark()->set_title("title");
     bm_specifics.mutable_bookmark()->set_url("url");
     sync_pb::EncryptedData encrypted;
     crypto->Encrypt(bm_specifics, &encrypted);
     entity_specifics.mutable_encrypted()->CopyFrom(encrypted);
     AddDefaultFieldValue(BOOKMARKS, &entity_specifics);
   }
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
 
   {
     // Verify the data.
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS, client_tag));
     EXPECT_EQ(title, node.GetTitle());
@@ skipping 44 matching lines @@
                     GetTransactionVersion(i->first));
     }
   }
 
   // Create bookmark node to increment transaction version of bookmark model.
   std::string client_tag = "title";
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
 
   {
     ReadTransaction read_trans(FROM_HERE, sync_manager_.GetUserShare());
     for (ModelSafeRoutingInfo::iterator i = routing_info.begin();
          i != routing_info.end(); ++i) {
       EXPECT_EQ(i->first == BOOKMARKS ? 2 : 1,
                 sync_manager_.GetUserShare()->directory->
                     GetTransactionVersion(i->first));
@@ skipping 294 matching lines @@
       Difference(ModelTypeSet::All(), disabled_types);
 
   // Verify only the non-disabled types remain after cleanup.
   sync_manager_.PurgeDisabledTypes(enabled_types, new_enabled_types);
   EXPECT_TRUE(new_enabled_types.Equals(sync_manager_.InitialSyncEndedTypes()));
   EXPECT_TRUE(disabled_types.Equals(
       sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
 }
 
 }  // namespace