sync: Specialize functions that fetch type root

Adds functions to look up the root node of a give type.  This replaces
most of the use cases for the functions that look up a node by unique
server tag, so those functions have been renamed to indicate their
near-deprecation.

This is part of the effort to remove the client's dependence on the
server for the creation of type root nodes.  Although it's unlikely that
we can make this work for existing types, new types should be prepared
to work properly in the absence of tyep root nodes.  The first step on
that path is to abstract away the mechanism by which we look up the type
roots.

Renaming the old lookup by tag functions allows us to ensure that there
no remaining cases where the code base looks up type roots by server
tag.  This CL also adds a few DCHECKs to ensure type roots are never
looked up using the old lookup by server tag functions.

BUG=373869

Review URL: https://codereview.chromium.org/302173004

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

24 files changed, 121 insertions, 102 deletions

diff --git a/sync/engine/apply_control_data_updates.cc b/sync/engine/apply_control_data_updates.cc
index e97741e..9b4b837 100644
--- a/sync/engine/apply_control_data_updates.cc
+++ b/sync/engine/apply_control_data_updates.cc
@@ -17,7 +17,7 @@
 
 namespace syncer {
 
-using syncable::GET_BY_SERVER_TAG;
+using syncable::GET_TYPE_ROOT;
 using syncable::IS_UNAPPLIED_UPDATE;
 using syncable::IS_UNSYNCED;
 using syncable::SERVER_SPECIFICS;
@@ -39,9 +39,7 @@ void ApplyControlDataUpdates(syncable::Directory* dir) {
   ModelTypeSet control_types = ControlTypes();
   for (ModelTypeSet::Iterator iter = control_types.First(); iter.Good();
        iter.Inc()) {
-    syncable::MutableEntry entry(&trans,
-                                 syncable::GET_BY_SERVER_TAG,
-                                 ModelTypeToRootTag(iter.Get()));
+    syncable::MutableEntry entry(&trans, syncable::GET_TYPE_ROOT, iter.Get());
     if (!entry.good())
       continue;
     if (!entry.GetIsUnappliedUpdate())
diff --git a/sync/engine/apply_control_data_updates_unittest.cc b/sync/engine/apply_control_data_updates_unittest.cc
index caacfbc..7248d14 100644
--- a/sync/engine/apply_control_data_updates_unittest.cc
+++ b/sync/engine/apply_control_data_updates_unittest.cc
@@ -188,8 +188,7 @@ TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) {
   // Simulate another nigori update that doesn't change anything.
   {
     syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::GET_BY_SERVER_TAG,
-                       ModelTypeToRootTag(NIGORI));
+    MutableEntry entry(&trans, syncable::GET_TYPE_ROOT, NIGORI);
     ASSERT_TRUE(entry.good());
     entry.PutServerVersion(entry_factory_->GetNextRevision());
     entry.PutIsUnappliedUpdate(true);
diff --git a/sync/engine/directory_commit_contribution_unittest.cc b/sync/engine/directory_commit_contribution_unittest.cc
index 475985c..b342b54 100644
--- a/sync/engine/directory_commit_contribution_unittest.cc
+++ b/sync/engine/directory_commit_contribution_unittest.cc
@@ -36,10 +36,7 @@ class DirectoryCommitContributionTest : public ::testing::Test {
   int64 CreateUnsyncedItem(syncable::WriteTransaction* trans,
                            ModelType type,
                            const std::string& tag) {
-    syncable::Entry parent_entry(
-        trans,
-        syncable::GET_BY_SERVER_TAG,
-        ModelTypeToRootTag(type));
+    syncable::Entry parent_entry(trans, syncable::GET_TYPE_ROOT, type);
     syncable::MutableEntry entry(
         trans,
         syncable::CREATE,
@@ -53,10 +50,7 @@ class DirectoryCommitContributionTest : public ::testing::Test {
   int64 CreateSyncedItem(syncable::WriteTransaction* trans,
                          ModelType type,
                          const std::string& tag) {
-    syncable::Entry parent_entry(
-        trans,
-        syncable::GET_BY_SERVER_TAG,
-        ModelTypeToRootTag(type));
+    syncable::Entry parent_entry(trans, syncable::GET_TYPE_ROOT, type);
     syncable::MutableEntry entry(
         trans,
         syncable::CREATE,
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index b88feb3..98cde3b 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -86,6 +86,7 @@ using syncable::GET_BY_HANDLE;
 using syncable::GET_BY_ID;
 using syncable::GET_BY_CLIENT_TAG;
 using syncable::GET_BY_SERVER_TAG;
+using syncable::GET_TYPE_ROOT;
 using syncable::UNITTEST;
 
 using sessions::MockDebugInfoGetter;
diff --git a/sync/internal_api/public/read_node.h b/sync/internal_api/public/read_node.h
index 50e7639..e3d37b5 100644
--- a/sync/internal_api/public/read_node.h
+++ b/sync/internal_api/public/read_node.h
@@ -37,10 +37,17 @@ class SYNC_EXPORT ReadNode : public BaseNode {
   // never mutable, so root lookup is only possible on a ReadNode.
   void InitByRootLookup();
 
-  // Each server-created permanent node is tagged with a unique string.
-  // Look up the node with the particular tag.  If it does not exist,
-  // return false.
-  InitByLookupResult InitByTagLookup(const std::string& tag);
+  // Returns the type root node, if it exists.  This is usually created by the
+  // server during first sync.  Eventually, we plan to remove support for it
+  // from the protocol and have the client create the node instead.
+  InitByLookupResult InitTypeRoot(ModelType type);
+
+  // Returns a server-created and unique-server-tagged item.
+  //
+  // This functionality is only useful for bookmarks because only bookmarks
+  // have server-tagged items.  All other server-tagged items are type root
+  // nodes, which should be looked up with InitTypeRoot().
+  InitByLookupResult InitByTagLookupForBookmarks(const std::string& tag);
 
   // Implementation of BaseNode's abstract virtual accessors.
   virtual const syncable::Entry* GetEntry() const OVERRIDE;
diff --git a/sync/internal_api/public/write_node.h b/sync/internal_api/public/write_node.h
index 5b47bd6..c46e376 100644
--- a/sync/internal_api/public/write_node.h
+++ b/sync/internal_api/public/write_node.h
@@ -85,10 +85,10 @@ class SYNC_EXPORT WriteNode : public BaseNode {
       const BaseNode& parent,
       const std::string& client_tag);
 
-  // Each server-created permanent node is tagged with a unique string.
-  // Look up the node with the particular tag.  If it does not exist,
-  // return false.
-  InitByLookupResult InitByTagLookup(const std::string& tag);
+  // Looks up the type's root folder.  This is usually created by the sync
+  // server during initial sync, though we do eventually wish to remove it from
+  // the protocol and have the client "fake it" instead.
+  InitByLookupResult InitTypeRoot(ModelType type);
 
   // These Set() functions correspond to the Get() functions of BaseNode.
   void SetIsFolder(bool folder);
diff --git a/sync/internal_api/read_node.cc b/sync/internal_api/read_node.cc
index ec85af6..c162800 100644
--- a/sync/internal_api/read_node.cc
+++ b/sync/internal_api/read_node.cc
@@ -77,7 +77,7 @@ const BaseTransaction* ReadNode::GetTransaction() const {
   return transaction_;
 }
 
-BaseNode::InitByLookupResult ReadNode::InitByTagLookup(
+BaseNode::InitByLookupResult ReadNode::InitByTagLookupForBookmarks(
     const std::string& tag) {
   DCHECK(!entry_) << "Init called twice";
   if (tag.empty())
@@ -89,8 +89,25 @@ BaseNode::InitByLookupResult ReadNode::InitByTagLookup(
   if (entry_->GetIsDel())
     return INIT_FAILED_ENTRY_IS_DEL;
   ModelType model_type = GetModelType();
-  LOG_IF(WARNING, model_type == UNSPECIFIED || model_type == TOP_LEVEL_FOLDER)
-      << "SyncAPI InitByTagLookup referencing unusually typed object.";
+  DCHECK_EQ(model_type, BOOKMARKS)
+      << "InitByTagLookup deprecated for all types except bookmarks.";
+  return DecryptIfNecessary() ? INIT_OK : INIT_FAILED_DECRYPT_IF_NECESSARY;
+}
+
+BaseNode::InitByLookupResult ReadNode::InitTypeRoot(ModelType type) {
+  DCHECK(!entry_) << "Init called twice";
+  if (!IsRealDataType(type))
+    return INIT_FAILED_PRECONDITION;
+  syncable::BaseTransaction* trans = transaction_->GetWrappedTrans();
+  entry_ = new syncable::Entry(trans, syncable::GET_TYPE_ROOT, type);
+  if (!entry_->good())
+    return INIT_FAILED_ENTRY_NOT_GOOD;
+  if (entry_->GetIsDel())
+    return INIT_FAILED_ENTRY_IS_DEL;
+  ModelType found_model_type = GetModelType();
+  LOG_IF(WARNING, found_model_type == UNSPECIFIED ||
+                  found_model_type == TOP_LEVEL_FOLDER)
+      << "SyncAPI InitTypeRoot referencing unusually typed object.";
   return DecryptIfNecessary() ? INIT_OK : INIT_FAILED_DECRYPT_IF_NECESSARY;
 }
 
diff --git a/sync/internal_api/sync_backup_manager_unittest.cc b/sync/internal_api/sync_backup_manager_unittest.cc
index 5de790b..5472c02 100644
--- a/sync/internal_api/sync_backup_manager_unittest.cc
+++ b/sync/internal_api/sync_backup_manager_unittest.cc
@@ -52,8 +52,7 @@ class SyncBackupManagerTest : public testing::Test {
                    const std::string& client_tag) {
     WriteTransaction trans(FROM_HERE, user_share);
     ReadNode type_root(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK,
-              type_root.InitByTagLookup(ModelTypeToRootTag(type)));
+    EXPECT_EQ(BaseNode::INIT_OK, type_root.InitTypeRoot(type));
 
     WriteNode node(&trans);
     EXPECT_EQ(WriteNode::INIT_SUCCESS,
diff --git a/sync/internal_api/sync_encryption_handler_impl.cc b/sync/internal_api/sync_encryption_handler_impl.cc
index d3dd33f..3a88f53 100644
--- a/sync/internal_api/sync_encryption_handler_impl.cc
+++ b/sync/internal_api/sync_encryption_handler_impl.cc
@@ -248,7 +248,7 @@ void SyncEncryptionHandlerImpl::Init() {
   WriteTransaction trans(FROM_HERE, user_share_);
   WriteNode node(&trans);
 
-  if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK)
+  if (node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK)
     return;
   if (!ApplyNigoriUpdateImpl(node.GetNigoriSpecifics(),
                              trans.GetWrappedTrans())) {
@@ -332,7 +332,7 @@ void SyncEncryptionHandlerImpl::SetEncryptionPassphrase(
   WriteTransaction trans(FROM_HERE, user_share_);
   KeyParams key_params = {"localhost", "dummy", passphrase};
   WriteNode node(&trans);
-  if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK) {
+  if (node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK) {
     NOTREACHED();
     return;
   }
@@ -484,7 +484,7 @@ void SyncEncryptionHandlerImpl::SetDecryptionPassphrase(
   WriteTransaction trans(FROM_HERE, user_share_);
   KeyParams key_params = {"localhost", "dummy", passphrase};
   WriteNode node(&trans);
-  if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK) {
+  if (node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK) {
     NOTREACHED();
     return;
   }
@@ -717,7 +717,7 @@ bool SyncEncryptionHandlerImpl::SetKeystoreKeys(
   // If this is a first time sync, we get the encryption keys before we process
   // the nigori node. Just return for now, ApplyNigoriUpdate will be invoked
   // once we have the nigori node.
-  syncable::Entry entry(trans, syncable::GET_BY_SERVER_TAG, kNigoriTag);
+  syncable::Entry entry(trans, syncable::GET_TYPE_ROOT, NIGORI);
   if (!entry.good())
     return true;
 
@@ -765,7 +765,7 @@ bool SyncEncryptionHandlerImpl::MigratedToKeystore() {
   DCHECK(thread_checker_.CalledOnValidThread());
   ReadTransaction trans(FROM_HERE, user_share_);
   ReadNode nigori_node(&trans);
-  if (nigori_node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK)
+  if (nigori_node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK)
     return false;
   return IsNigoriMigratedToKeystore(nigori_node.GetNigoriSpecifics());
 }
@@ -793,8 +793,7 @@ void SyncEncryptionHandlerImpl::ReEncryptEverything(
       continue; // These types handle encryption differently.
 
     ReadNode type_root(trans);
-    std::string tag = ModelTypeToRootTag(iter.Get());
-    if (type_root.InitByTagLookup(tag) != BaseNode::INIT_OK)
+    if (type_root.InitTypeRoot(iter.Get()) != BaseNode::INIT_OK)
       continue; // Don't try to reencrypt if the type's data is unavailable.
 
     // Iterate through all children of this datatype.
@@ -825,9 +824,7 @@ void SyncEncryptionHandlerImpl::ReEncryptEverything(
   // Passwords are encrypted with their own legacy scheme.  Passwords are always
   // encrypted so we don't need to check GetEncryptedTypes() here.
   ReadNode passwords_root(trans);
-  std::string passwords_tag = ModelTypeToRootTag(PASSWORDS);
-  if (passwords_root.InitByTagLookup(passwords_tag) ==
-          BaseNode::INIT_OK) {
+  if (passwords_root.InitTypeRoot(PASSWORDS) == BaseNode::INIT_OK) {
     int64 child_id = passwords_root.GetFirstChildId();
     while (child_id != kInvalidId) {
       WriteNode child(trans);
@@ -1012,7 +1009,7 @@ void SyncEncryptionHandlerImpl::WriteEncryptionStateToNigori(
   DCHECK(thread_checker_.CalledOnValidThread());
   WriteNode nigori_node(trans);
   // This can happen in tests that don't have nigori nodes.
-  if (nigori_node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK)
+  if (nigori_node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK)
     return;
 
   sync_pb::NigoriSpecifics nigori = nigori_node.GetNigoriSpecifics();
diff --git a/sync/internal_api/sync_encryption_handler_impl_unittest.cc b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
index 2f09cf9..5aad1b0 100644
--- a/sync/internal_api/sync_encryption_handler_impl_unittest.cc
+++ b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
@@ -146,7 +146,7 @@ class SyncEncryptionHandlerImplTest : public ::testing::Test {
       const std::string& passphrase) {
     ReadTransaction trans(FROM_HERE, user_share());
     ReadNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     const sync_pb::NigoriSpecifics& nigori = nigori_node.GetNigoriSpecifics();
     if (migration_time > 0)
       EXPECT_EQ(migration_time, nigori.keystore_migration_time());
@@ -239,7 +239,7 @@ class SyncEncryptionHandlerImplTest : public ::testing::Test {
     {
       WriteTransaction trans(FROM_HERE, user_share());
       WriteNode nigori_node(&trans);
-      ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+      ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
       sync_pb::NigoriSpecifics nigori = BuildMigratedNigori(
           KEYSTORE_PASSPHRASE,
           migration_time,
@@ -272,7 +272,7 @@ class SyncEncryptionHandlerImplTest : public ::testing::Test {
     {
       WriteTransaction trans(FROM_HERE, user_share());
       WriteNode nigori_node(&trans);
-      ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+      ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
       sync_pb::NigoriSpecifics nigori = BuildMigratedNigori(
           CUSTOM_PASSPHRASE,
           migration_time,
@@ -309,7 +309,7 @@ class SyncEncryptionHandlerImplTest : public ::testing::Test {
     {
       WriteTransaction trans(FROM_HERE, user_share());
       WriteNode nigori_node(&trans);
-      ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+      ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
       sync_pb::NigoriSpecifics nigori;
       other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
       nigori.set_keybag_is_frozen(passphrase_type == CUSTOM_PASSPHRASE);
@@ -587,7 +587,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldNigori) {
     // In addition, the nigori node should match the current encryption state.
     ReadTransaction trans(FROM_HERE, user_share());
     ReadNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     const sync_pb::NigoriSpecifics& nigori = nigori_node.GetNigoriSpecifics();
     EXPECT_TRUE(GetCryptographer()->CanDecryptUsingDefaultKey(
         our_encrypted_specifics.encrypted()));
@@ -730,7 +730,7 @@ TEST_F(SyncEncryptionHandlerImplTest, MigrateOnDecryptImplicitPass) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     Cryptographer other_cryptographer(GetCryptographer()->encryptor());
     KeyParams other_key = {"localhost", "dummy", kOtherKey};
     other_cryptographer.AddKey(other_key);
@@ -787,7 +787,7 @@ TEST_F(SyncEncryptionHandlerImplTest, MigrateOnDecryptCustomPass) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     Cryptographer other_cryptographer(GetCryptographer()->encryptor());
     KeyParams other_key = {"localhost", "dummy", kOtherKey};
     other_cryptographer.AddKey(other_key);
@@ -1035,7 +1035,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveMigratedNigoriKeystorePass) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1115,7 +1115,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveMigratedNigoriFrozenImplicitPass) {
                 OnEncryptedTypesChanged(_, true));
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.set_keybag_is_frozen(true);
     nigori.set_passphrase_type(
@@ -1195,7 +1195,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveMigratedNigoriCustomPass) {
                 OnEncryptedTypesChanged(_, true));
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.set_keybag_is_frozen(true);
     nigori.set_passphrase_type(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE);
@@ -1257,7 +1257,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveUnmigratedNigoriAfterMigration) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     GetCryptographer()->GetKeys(nigori.mutable_encryption_keybag());
     nigori.set_keybag_is_frozen(true);
@@ -1302,7 +1302,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveUnmigratedNigoriAfterMigration) {
     other_cryptographer.AddKey(old_key);
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
     nigori.set_keybag_is_frozen(false);
@@ -1336,7 +1336,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldMigratedNigori) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     GetCryptographer()->GetKeys(nigori.mutable_encryption_keybag());
     nigori.set_keybag_is_frozen(true);
@@ -1379,7 +1379,7 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldMigratedNigori) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     Cryptographer other_cryptographer(GetCryptographer()->encryptor());
     other_cryptographer.AddKey(old_key);
@@ -1428,7 +1428,7 @@ TEST_F(SyncEncryptionHandlerImplTest, SetKeystoreAfterReceivingMigratedNigori) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1509,7 +1509,7 @@ TEST_F(SyncEncryptionHandlerImplTest, SetCustomPassAfterMigration) {
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1611,7 +1611,7 @@ TEST_F(SyncEncryptionHandlerImplTest,
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1718,7 +1718,7 @@ TEST_F(SyncEncryptionHandlerImplTest,
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1813,7 +1813,7 @@ TEST_F(SyncEncryptionHandlerImplTest,
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     nigori.mutable_keystore_decryptor_token()->CopyFrom(
         keystore_decryptor_token);
@@ -1931,7 +1931,7 @@ TEST_F(SyncEncryptionHandlerImplTest,
   {
     WriteTransaction trans(FROM_HERE, user_share());
     WriteNode nigori_node(&trans);
-    ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK);
+    ASSERT_EQ(nigori_node.InitTypeRoot(NIGORI), BaseNode::INIT_OK);
     sync_pb::NigoriSpecifics nigori;
     Cryptographer other_cryptographer(GetCryptographer()->encryptor());
     other_cryptographer.AddKey(old_key);
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 35a1caa..a5f2de0 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -1035,7 +1035,7 @@ const std::string SyncManagerImpl::cache_guid() {
 bool SyncManagerImpl::ReceivedExperiment(Experiments* experiments) {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   ReadNode nigori_node(&trans);
-  if (nigori_node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK) {
+  if (nigori_node.InitTypeRoot(NIGORI) != BaseNode::INIT_OK) {
     DVLOG(1) << "Couldn't find Nigori node.";
     return false;
   }
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 20b7685..7c0d3d5 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -174,8 +174,7 @@ int64 MakeServerNode(UserShare* share, ModelType model_type,
                      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));
+  syncable::Entry root_entry(&trans, syncable::GET_TYPE_ROOT, model_type);
   EXPECT_TRUE(root_entry.good());
   syncable::Id root_id = root_entry.GetId();
   syncable::Id node_id = syncable::Id::CreateFromServerId(client_tag);
@@ -583,7 +582,7 @@ TEST_F(SyncApiTest, EmptyTags) {
       node.InitUniqueByCreation(TYPED_URLS, root_node, empty_tag);
   EXPECT_NE(WriteNode::INIT_SUCCESS, result);
   EXPECT_EQ(BaseNode::INIT_FAILED_PRECONDITION,
-            node.InitByTagLookup(empty_tag));
+            node.InitByClientTagLookup(TYPED_URLS, empty_tag));
 }
 
 // Test counting nodes when the type's root node has no children.
@@ -1248,7 +1247,7 @@ TEST_F(SyncManagerTest, SetInitialGaiaPass) {
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     ReadNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByTagLookup(kNigoriTag));
+    EXPECT_EQ(BaseNode::INIT_OK, node.InitTypeRoot(NIGORI));
     sync_pb::NigoriSpecifics nigori = node.GetNigoriSpecifics();
     Cryptographer* cryptographer = trans.GetCryptographer();
     EXPECT_TRUE(cryptographer->is_ready());
@@ -1368,7 +1367,7 @@ TEST_F(SyncManagerTest, SupplyPendingGAIAPass) {
     KeyParams params = {"localhost", "dummy", "passphrase2"};
     other_cryptographer.AddKey(params);
     WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByTagLookup(kNigoriTag));
+    EXPECT_EQ(BaseNode::INIT_OK, node.InitTypeRoot(NIGORI));
     sync_pb::NigoriSpecifics nigori;
     other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
     cryptographer->SetPendingKeys(nigori.encryption_keybag());
@@ -1416,7 +1415,7 @@ TEST_F(SyncManagerTest, SupplyPendingOldGAIAPass) {
     KeyParams params = {"localhost", "dummy", "old_gaia"};
     other_cryptographer.AddKey(params);
     WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByTagLookup(kNigoriTag));
+    EXPECT_EQ(BaseNode::INIT_OK, node.InitTypeRoot(NIGORI));
     sync_pb::NigoriSpecifics nigori;
     other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
     node.SetNigoriSpecifics(nigori);
@@ -1499,7 +1498,7 @@ TEST_F(SyncManagerTest, SupplyPendingExplicitPass) {
     KeyParams params = {"localhost", "dummy", "explicit"};
     other_cryptographer.AddKey(params);
     WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByTagLookup(kNigoriTag));
+    EXPECT_EQ(BaseNode::INIT_OK, node.InitTypeRoot(NIGORI));
     sync_pb::NigoriSpecifics nigori;
     other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
     cryptographer->SetPendingKeys(nigori.encryption_keybag());
@@ -1548,7 +1547,7 @@ TEST_F(SyncManagerTest, SupplyPendingGAIAPassUserProvided) {
     KeyParams params = {"localhost", "dummy", "passphrase"};
     other_cryptographer.AddKey(params);
     WriteNode node(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK, node.InitByTagLookup(kNigoriTag));
+    EXPECT_EQ(BaseNode::INIT_OK, node.InitTypeRoot(NIGORI));
     sync_pb::NigoriSpecifics nigori;
     other_cryptographer.GetKeys(nigori.mutable_encryption_keybag());
     node.SetNigoriSpecifics(nigori);
@@ -1743,8 +1742,7 @@ TEST_F(SyncManagerTest, CreateLocalBookmark) {
   {
     WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     ReadNode bookmark_root(&trans);
-    ASSERT_EQ(BaseNode::INIT_OK,
-              bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
+    ASSERT_EQ(BaseNode::INIT_OK, bookmark_root.InitTypeRoot(BOOKMARKS));
     WriteNode node(&trans);
     ASSERT_TRUE(node.InitBookmarkByCreation(bookmark_root, NULL));
     node.SetIsFolder(false);
@@ -1757,8 +1755,7 @@ TEST_F(SyncManagerTest, CreateLocalBookmark) {
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     ReadNode bookmark_root(&trans);
-    ASSERT_EQ(BaseNode::INIT_OK,
-              bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
+    ASSERT_EQ(BaseNode::INIT_OK, bookmark_root.InitTypeRoot(BOOKMARKS));
     int64 child_id = bookmark_root.GetFirstChildId();
 
     ReadNode node(&trans);
@@ -2583,8 +2580,9 @@ TEST_F(SyncManagerTest, PurgePartiallySyncedTypes) {
   // Further ensure that the test harness did not create its root node.
   {
     syncable::ReadTransaction trans(FROM_HERE, share->directory.get());
-    syncable::Entry autofill_root_node(&trans, syncable::GET_BY_SERVER_TAG,
-                                       ModelTypeToRootTag(AUTOFILL));
+    syncable::Entry autofill_root_node(&trans,
+                                       syncable::GET_TYPE_ROOT,
+                                       AUTOFILL);
     ASSERT_FALSE(autofill_root_node.good());
   }
 
diff --git a/sync/internal_api/sync_rollback_manager_base.cc b/sync/internal_api/sync_rollback_manager_base.cc
index e3711bc0..ade0422 100644
--- a/sync/internal_api/sync_rollback_manager_base.cc
+++ b/sync/internal_api/sync_rollback_manager_base.cc
@@ -278,7 +278,7 @@ bool SyncRollbackManagerBase::InitBackupDB(
 bool SyncRollbackManagerBase::InitTypeRootNode(ModelType type) {
   WriteTransaction trans(FROM_HERE, &share_);
   ReadNode root(&trans);
-  if (BaseNode::INIT_OK == root.InitByTagLookup(ModelTypeToRootTag(type)))
+  if (BaseNode::INIT_OK == root.InitTypeRoot(type))
     return true;
 
   syncable::MutableEntry entry(trans.GetWrappedWriteTrans(),
@@ -305,8 +305,8 @@ bool SyncRollbackManagerBase::InitTypeRootNode(ModelType type) {
 void SyncRollbackManagerBase::InitBookmarkFolder(const std::string& folder) {
   WriteTransaction trans(FROM_HERE, &share_);
   syncable::Entry bookmark_root(trans.GetWrappedTrans(),
-                                syncable::GET_BY_SERVER_TAG,
-                                ModelTypeToRootTag(BOOKMARKS));
+                                syncable::GET_TYPE_ROOT,
+                                BOOKMARKS);
   if (!bookmark_root.good())
     return;
 
diff --git a/sync/internal_api/sync_rollback_manager_base_unittest.cc b/sync/internal_api/sync_rollback_manager_base_unittest.cc
index 5bb1548..79075d2 100644
--- a/sync/internal_api/sync_rollback_manager_base_unittest.cc
+++ b/sync/internal_api/sync_rollback_manager_base_unittest.cc
@@ -49,20 +49,20 @@ TEST_F(SyncRollbackManagerBaseTest, InitTypeOnConfiguration) {
   ReadTransaction trans(FROM_HERE, manager_.GetUserShare());
   ReadNode pref_root(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,
-            pref_root.InitByTagLookup(ModelTypeToRootTag(PREFERENCES)));
+            pref_root.InitTypeRoot(PREFERENCES));
 
   ReadNode bookmark_root(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,
-            bookmark_root.InitByTagLookup(ModelTypeToRootTag(BOOKMARKS)));
+            bookmark_root.InitTypeRoot(BOOKMARKS));
   ReadNode bookmark_bar(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,
-            bookmark_bar.InitByTagLookup("bookmark_bar"));
+            bookmark_bar.InitByTagLookupForBookmarks("bookmark_bar"));
   ReadNode bookmark_mobile(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,
-            bookmark_mobile.InitByTagLookup("synced_bookmarks"));
+            bookmark_mobile.InitByTagLookupForBookmarks("synced_bookmarks"));
   ReadNode bookmark_other(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,
-            bookmark_other.InitByTagLookup("other_bookmarks"));
+            bookmark_other.InitByTagLookupForBookmarks("other_bookmarks"));
 }
 
 }  // anonymous namespace
diff --git a/sync/internal_api/sync_rollback_manager_unittest.cc b/sync/internal_api/sync_rollback_manager_unittest.cc
index 925d635..d66ee06 100644
--- a/sync/internal_api/sync_rollback_manager_unittest.cc
+++ b/sync/internal_api/sync_rollback_manager_unittest.cc
@@ -91,8 +91,7 @@ class SyncRollbackManagerTest : public testing::Test,
                     const std::string& client_tag) {
     WriteTransaction trans(FROM_HERE, user_share);
     ReadNode type_root(&trans);
-    EXPECT_EQ(BaseNode::INIT_OK,
-              type_root.InitByTagLookup(ModelTypeToRootTag(type)));
+    EXPECT_EQ(BaseNode::INIT_OK, type_root.InitTypeRoot(type));
 
     WriteNode node(&trans);
     EXPECT_EQ(WriteNode::INIT_SUCCESS,
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index cefd6df..1fbe029 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -310,13 +310,12 @@ BaseNode::InitByLookupResult WriteNode::InitByClientTagLookup(
   return DecryptIfNecessary() ? INIT_OK : INIT_FAILED_DECRYPT_IF_NECESSARY;
 }
 
-BaseNode::InitByLookupResult WriteNode::InitByTagLookup(
-    const std::string& tag) {
+BaseNode::InitByLookupResult WriteNode::InitTypeRoot(ModelType type) {
   DCHECK(!entry_) << "Init called twice";
-  if (tag.empty())
+  if (!IsRealDataType(type))
     return INIT_FAILED_PRECONDITION;
   entry_ = new syncable::MutableEntry(transaction_->GetWrappedWriteTrans(),
-                                      syncable::GET_BY_SERVER_TAG, tag);
+                                      syncable::GET_TYPE_ROOT, type);
   if (!entry_->good())
     return INIT_FAILED_ENTRY_NOT_GOOD;
   if (entry_->GetIsDel())
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 39233fe..ad107b1 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -952,9 +952,7 @@ bool Directory::InitialSyncEndedForType(ModelType type) {
 bool Directory::InitialSyncEndedForType(
     BaseTransaction* trans, ModelType type) {
   // True iff the type's root node has been received and applied.
-  syncable::Entry entry(trans,
-                        syncable::GET_BY_SERVER_TAG,
-                        ModelTypeToRootTag(type));
+  syncable::Entry entry(trans, syncable::GET_TYPE_ROOT, type);
   return entry.good() && entry.GetBaseVersion() != CHANGES_VERSION;
 }
 
diff --git a/sync/syncable/entry.cc b/sync/syncable/entry.cc
index 2be2aad..0a4a44f 100644
--- a/sync/syncable/entry.cc
+++ b/sync/syncable/entry.cc
@@ -28,8 +28,9 @@ Entry::Entry(BaseTransaction* trans, GetByClientTag, const string& tag)
   kernel_ = trans->directory()->GetEntryByClientTag(tag);
 }
 
-Entry::Entry(BaseTransaction* trans, GetByServerTag, const string& tag)
+Entry::Entry(BaseTransaction* trans, GetTypeRoot, ModelType type)
     : basetrans_(trans) {
+  const std::string& tag = ModelTypeToRootTag(type);
   kernel_ = trans->directory()->GetEntryByServerTag(tag);
 }
 
@@ -38,6 +39,11 @@ Entry::Entry(BaseTransaction* trans, GetByHandle, int64 metahandle)
   kernel_ = trans->directory()->GetEntryByHandle(metahandle);
 }
 
+Entry::Entry(BaseTransaction* trans, GetByServerTag, const string& tag)
+    : basetrans_(trans) {
+  kernel_ = trans->directory()->GetEntryByServerTag(tag);
+}
+
 Directory* Entry::dir() const {
   return basetrans_->directory();
 }
diff --git a/sync/syncable/entry.h b/sync/syncable/entry.h
index 47b9c59..619c84b 100644
--- a/sync/syncable/entry.h
+++ b/sync/syncable/entry.h
@@ -35,9 +35,14 @@ enum GetByClientTag {
 };
 
 enum GetByServerTag {
+  // Server tagged items are deprecated for all types but bookmarks.
   GET_BY_SERVER_TAG
 };
 
+enum GetTypeRoot {
+  GET_TYPE_ROOT
+};
+
 enum GetByHandle {
   GET_BY_HANDLE
 };
@@ -48,9 +53,13 @@ class SYNC_EXPORT Entry {
   // succeeded.
   Entry(BaseTransaction* trans, GetByHandle, int64 handle);
   Entry(BaseTransaction* trans, GetById, const Id& id);
-  Entry(BaseTransaction* trans, GetByServerTag, const std::string& tag);
+  Entry(BaseTransaction* trans, GetTypeRoot, ModelType type);
   Entry(BaseTransaction* trans, GetByClientTag, const std::string& tag);
 
+  // This lookup function is deprecated.  All types except bookmarks can use
+  // the GetTypeRoot variant instead.
+  Entry(BaseTransaction* trans, GetByServerTag, const std::string& tag);
+
   bool good() const { return 0 != kernel_; }
 
   BaseTransaction* trans() const { return basetrans_; }
diff --git a/sync/syncable/model_neutral_mutable_entry.cc b/sync/syncable/model_neutral_mutable_entry.cc
index d778aba..2ae5450 100644
--- a/sync/syncable/model_neutral_mutable_entry.cc
+++ b/sync/syncable/model_neutral_mutable_entry.cc
@@ -60,8 +60,8 @@ ModelNeutralMutableEntry::ModelNeutralMutableEntry(
 }
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(
-    BaseWriteTransaction* trans, GetByServerTag, const string& tag)
-    : Entry(trans, GET_BY_SERVER_TAG, tag), base_write_transaction_(trans) {
+    BaseWriteTransaction* trans, GetTypeRoot, ModelType type)
+    : Entry(trans, GET_TYPE_ROOT, type), base_write_transaction_(trans) {
 }
 
 void ModelNeutralMutableEntry::PutBaseVersion(int64 value) {
diff --git a/sync/syncable/model_neutral_mutable_entry.h b/sync/syncable/model_neutral_mutable_entry.h
index e2292e7..4010dff 100644
--- a/sync/syncable/model_neutral_mutable_entry.h
+++ b/sync/syncable/model_neutral_mutable_entry.h
@@ -38,8 +38,8 @@ class SYNC_EXPORT_PRIVATE ModelNeutralMutableEntry : public Entry {
       const std::string& tag);
   ModelNeutralMutableEntry(
       BaseWriteTransaction* trans,
-      GetByServerTag,
-      const std::string& tag);
+      GetTypeRoot,
+      ModelType type);
 
   inline BaseWriteTransaction* base_write_transaction() const {
     return base_write_transaction_;
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 98af1cc..a506e4f 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -98,9 +98,8 @@ MutableEntry::MutableEntry(WriteTransaction* trans, GetByClientTag,
       write_transaction_(trans) {
 }
 
-MutableEntry::MutableEntry(WriteTransaction* trans, GetByServerTag,
-                           const string& tag)
-    : ModelNeutralMutableEntry(trans, GET_BY_SERVER_TAG, tag),
+MutableEntry::MutableEntry(WriteTransaction* trans, GetTypeRoot, ModelType type)
+    : ModelNeutralMutableEntry(trans, GET_TYPE_ROOT, type),
       write_transaction_(trans) {
 }
 
diff --git a/sync/syncable/mutable_entry.h b/sync/syncable/mutable_entry.h
index 9b91113..1709475 100644
--- a/sync/syncable/mutable_entry.h
+++ b/sync/syncable/mutable_entry.h
@@ -35,7 +35,7 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public ModelNeutralMutableEntry {
   MutableEntry(WriteTransaction* trans, GetByHandle, int64);
   MutableEntry(WriteTransaction* trans, GetById, const Id&);
   MutableEntry(WriteTransaction* trans, GetByClientTag, const std::string& tag);
-  MutableEntry(WriteTransaction* trans, GetByServerTag, const std::string& tag);
+  MutableEntry(WriteTransaction* trans, GetTypeRoot, ModelType type);
 
   inline WriteTransaction* write_transaction() const {
     return write_transaction_;
diff --git a/sync/syncable/nigori_util.cc b/sync/syncable/nigori_util.cc
index 5463140..63dbcac 100644
--- a/sync/syncable/nigori_util.cc
+++ b/sync/syncable/nigori_util.cc
@@ -100,8 +100,7 @@ bool VerifyDataTypeEncryptionForTest(
     NOTREACHED();
     return true;
   }
-  std::string type_tag = ModelTypeToRootTag(type);
-  Entry type_root(trans, GET_BY_SERVER_TAG, type_tag);
+  Entry type_root(trans, GET_TYPE_ROOT, type);
   if (!type_root.good()) {
     NOTREACHED();
     return false;