In the sync database, use protobuf-based storage. Drop the old

bookmark-only columns.  Add getters and setters for BookmarkSpecifics to
syncapi as well as syncable entries.  Make the datatype be a required
property when creating a syncapi node.  Add a datatype for the 'google
chrome' top level folder.  Add database migrations from version 67 to
the new schema.  Add infrastructure to support migrations generically.
Add unit tests for the migrations.

Pull a new version of the protobuf library to pick up a fix for a
bug that this change exposed (I upstreamed the fix).

Fix some example code in the sql helpers so that it would actually
compile.

BUG=29899,30041

TEST=New unit tests for migrations: unit tests are based on actual
database dumps.  Additionally, I manually tested 2-client sync using
combos of old-protocol servers, new-protocol servers, and initial
database versions v67, v68, and v0 (new client).  I manually verified
that add/edit/delete works in these combination cases.  Afterwards I
verified (by inspecting the sync databases) that the ModelTypes are
consistent across the various migration/protocol paths.

Review URL: http://codereview.chromium.org/554066

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

12 files changed, 438 insertions, 197 deletions

diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index a9bd051..49ec3ce 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -14,7 +14,6 @@
 #include "chrome/browser/sync/engine/net/server_connection_manager.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_thread.h"
-#include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/notifier/listener/talk_mediator.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
 #include "chrome/browser/sync/sessions/session_state.h"
diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
index ed6811f..546adef 100755
--- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc
+++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/engine/apply_updates_command.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
@@ -78,6 +79,9 @@ class ApplyUpdatesCommandTest : public testing::Test,
     entry.Put(syncable::SERVER_NON_UNIQUE_NAME, item_id);
     entry.Put(syncable::SERVER_PARENT_ID, Id::CreateFromServerId(parent_id));
     entry.Put(syncable::SERVER_IS_DIR, true);
+    sync_pb::EntitySpecifics default_bookmark_specifics;
+    default_bookmark_specifics.MutableExtension(sync_pb::bookmark);
+    entry.Put(syncable::SERVER_SPECIFICS, default_bookmark_specifics);
   }
 
   TestDirectorySetterUpper syncdb_;
diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc
index 9544405..b945fd7 100755
--- a/chrome/browser/sync/engine/build_commit_command.cc
+++ b/chrome/browser/sync/engine/build_commit_command.cc
@@ -22,6 +22,7 @@ using std::vector;
 using syncable::ExtendedAttribute;
 using syncable::Id;
 using syncable::MutableEntry;
+using syncable::SPECIFICS;
 
 namespace browser_sync {
 
@@ -45,38 +46,26 @@ void BuildCommitCommand::AddExtensionsActivityToMessage(
 }
 
 namespace {
-void SetNewStyleBookmarkData(MutableEntry* meta_entry, SyncEntity* sync_entry) {
+void SetEntrySpecifics(MutableEntry* meta_entry, SyncEntity* sync_entry) {
+  // Add the new style extension and the folder bit.
+  sync_entry->mutable_specifics()->CopyFrom(meta_entry->Get(SPECIFICS));
+  sync_entry->set_folder(meta_entry->Get(syncable::IS_DIR));
 
-  // Add the new style extension. Needed for folders as well as bookmarks.
-  // It'll identify the folder type on the server.
-  sync_pb::BookmarkSpecifics* bookmark_specifics = sync_entry->
-      mutable_specifics()->MutableExtension(sync_pb::bookmark);
-
-  if (!meta_entry->Get(syncable::IS_DIR)) {
-    // Set new-style extended bookmark data.
-    string bookmark_url = meta_entry->Get(syncable::BOOKMARK_URL);
-    bookmark_specifics->set_url(bookmark_url);
-    SyncerProtoUtil::CopyBlobIntoProtoBytes(
-        meta_entry->Get(syncable::BOOKMARK_FAVICON),
-        bookmark_specifics->mutable_favicon());
-  } else {
-    sync_entry->set_folder(true);
-  }
+  DCHECK(meta_entry->GetModelType() == sync_entry->GetModelType());
 }
 
 void SetOldStyleBookmarkData(MutableEntry* meta_entry, SyncEntity* sync_entry) {
+  DCHECK(meta_entry->Get(SPECIFICS).HasExtension(sync_pb::bookmark));
 
   // Old-style inlined bookmark data.
   sync_pb::SyncEntity_BookmarkData* bookmark =
       sync_entry->mutable_bookmarkdata();
 
   if (!meta_entry->Get(syncable::IS_DIR)) {
-    string bookmark_url = meta_entry->Get(syncable::BOOKMARK_URL);
-    bookmark->set_bookmark_url(bookmark_url);
-    SyncerProtoUtil::CopyBlobIntoProtoBytes(
-        meta_entry->Get(syncable::BOOKMARK_FAVICON),
-        bookmark->mutable_bookmark_favicon());
-
+    const sync_pb::BookmarkSpecifics& bookmark_specifics =
+        meta_entry->Get(SPECIFICS).GetExtension(sync_pb::bookmark);
+    bookmark->set_bookmark_url(bookmark_specifics.url());
+    bookmark->set_bookmark_favicon(bookmark_specifics.favicon());
     bookmark->set_bookmark_folder(false);
   } else {
     bookmark->set_bookmark_folder(true);
@@ -172,8 +161,7 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
     // Deletion is final on the server, let's move things and then delete them.
     if (meta_entry.Get(syncable::IS_DEL)) {
       sync_entry->set_deleted(true);
-    } else if (meta_entry.Get(syncable::IS_BOOKMARK_OBJECT)) {
-
+    } else if (meta_entry.Get(SPECIFICS).HasExtension(sync_pb::bookmark)) {
       // Common data in both new and old protocol.
       const Id& prev_id = meta_entry.Get(syncable::PREV_ID);
       string prev_string = prev_id.IsRoot() ? string() : prev_id.GetServerId();
@@ -183,7 +171,7 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
       // over the wire; instead, when deployed servers are able to accept
       // the new-style scheme, we should abandon the old way.
       SetOldStyleBookmarkData(&meta_entry, sync_entry);
-      SetNewStyleBookmarkData(&meta_entry, sync_entry);
+      SetEntrySpecifics(&meta_entry, sync_entry);
     }
   }
   session->status_controller()->mutable_commit_message()->CopyFrom(message);
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index eee2565..a54481f 100755
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -121,15 +121,14 @@ bool ReverifyEntry(syncable::WriteTransaction* trans, const SyncEntity& entry,
 
   const bool deleted = entry.has_deleted() && entry.deleted();
   const bool is_directory = entry.IsFolder();
-  const bool is_bookmark =
-      SyncerUtil::GetModelType(entry) == syncable::BOOKMARKS;
+  const syncable::ModelType model_type = entry.GetModelType();
 
   return VERIFY_SUCCESS == SyncerUtil::VerifyUpdateConsistency(trans,
                                                                entry,
                                                                same_id,
                                                                deleted,
                                                                is_directory,
-                                                               is_bookmark);
+                                                               model_type);
 }
 }  // namespace
 
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 3f0ad2d..8facc52 100755
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -47,6 +47,7 @@
 #include "chrome/browser/sync/engine/syncer_thread.h"
 #include "chrome/browser/sync/notifier/listener/talk_mediator.h"
 #include "chrome/browser/sync/notifier/listener/talk_mediator_impl.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
 #include "chrome/browser/sync/sessions/sync_session_context.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ -82,6 +83,7 @@ using std::string;
 using std::vector;
 using syncable::Directory;
 using syncable::DirectoryManager;
+using syncable::SPECIFICS;
 
 typedef GoogleServiceAuthError AuthError;
 
@@ -400,20 +402,9 @@ struct UserShare {
 ////////////////////////////////////
 // BaseNode member definitions.
 
-// BaseNode::BaseNodeInternal provides storage for member Get() functions that
-// need to return pointers (e.g. strings).
-struct BaseNode::BaseNodeInternal {
-  GURL url;
-  std::wstring title;
-  Directory::ChildHandles child_handles;
-  syncable::Blob favicon;
-};
+BaseNode::BaseNode() {}
 
-BaseNode::BaseNode() : data_(new BaseNode::BaseNodeInternal) {}
-
-BaseNode::~BaseNode() {
-  delete data_;
-}
+BaseNode::~BaseNode() {}
 
 int64 BaseNode::GetParentId() const {
   return IdToMetahandle(GetTransaction()->GetWrappedTrans(),
@@ -428,26 +419,14 @@ bool BaseNode::GetIsFolder() const {
   return GetEntry()->Get(syncable::IS_DIR);
 }
 
-const std::wstring& BaseNode::GetTitle() const {
-  ServerNameToSyncAPIName(GetEntry()->Get(syncable::NON_UNIQUE_NAME),
-                          &data_->title);
-  return data_->title;
+std::wstring BaseNode::GetTitle() const {
+  std::wstring result;
+  ServerNameToSyncAPIName(GetEntry()->Get(syncable::NON_UNIQUE_NAME), &result);
+  return result;
 }
 
-const GURL& BaseNode::GetURL() const {
-  GURL url(GetEntry()->Get(syncable::BOOKMARK_URL));
-  url.Swap(&data_->url);
-  return data_->url;
-}
-
-const int64* BaseNode::GetChildIds(size_t* child_count) const {
-  DCHECK(child_count);
-  Directory* dir = GetTransaction()->GetLookup();
-  dir->GetChildHandles(GetTransaction()->GetWrappedTrans(),
-                       GetEntry()->Get(syncable::ID), &data_->child_handles);
-
-  *child_count = data_->child_handles.size();
-  return (data_->child_handles.empty()) ? NULL : &data_->child_handles[0];
+GURL BaseNode::GetURL() const {
+  return GURL(GetBookmarkSpecifics().url());
 }
 
 int64 BaseNode::GetPredecessorId() const {
@@ -474,19 +453,28 @@ int64 BaseNode::GetFirstChildId() const {
   return IdToMetahandle(GetTransaction()->GetWrappedTrans(), id_string);
 }
 
-const unsigned char* BaseNode::GetFaviconBytes(size_t* size_in_bytes) {
-  data_->favicon = GetEntry()->Get(syncable::BOOKMARK_FAVICON);
-  *size_in_bytes = data_->favicon.size();
-  if (*size_in_bytes)
-    return &(data_->favicon[0]);
-  else
-    return NULL;
+void BaseNode::GetFaviconBytes(std::vector<unsigned char>* output) const {
+  if (!output)
+    return;
+  const std::string& favicon = GetBookmarkSpecifics().favicon();
+  output->assign(reinterpret_cast<const unsigned char*>(favicon.data()),
+      reinterpret_cast<const unsigned char*>(favicon.data() +
+                                             favicon.length()));
 }
 
 int64 BaseNode::GetExternalId() const {
   return GetEntry()->Get(syncable::LOCAL_EXTERNAL_ID);
 }
 
+const sync_pb::BookmarkSpecifics& BaseNode::GetBookmarkSpecifics() const {
+  DCHECK(GetModelType() == syncable::BOOKMARKS);
+  return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::bookmark);
+}
+
+syncable::ModelType BaseNode::GetModelType() const {
+  return GetEntry()->GetModelType();
+}
+
 ////////////////////////////////////
 // WriteNode member definitions
 void WriteNode::SetIsFolder(bool folder) {
@@ -511,11 +499,32 @@ void WriteNode::SetTitle(const std::wstring& title) {
 }
 
 void WriteNode::SetURL(const GURL& url) {
-  const std::string& url_string = url.spec();
-  if (url_string == entry_->Get(syncable::BOOKMARK_URL))
-    return;  // Skip redundant changes.
+  sync_pb::BookmarkSpecifics new_value = GetBookmarkSpecifics();
+  new_value.set_url(url.spec());
+  SetBookmarkSpecifics(new_value);
+}
+
+void WriteNode::SetBookmarkSpecifics(
+    const sync_pb::BookmarkSpecifics& new_value) {
+  DCHECK(GetModelType() == syncable::BOOKMARKS);
+  PutBookmarkSpecificsAndMarkForSyncing(new_value);
+}
+
+void WriteNode::PutBookmarkSpecificsAndMarkForSyncing(
+    const sync_pb::BookmarkSpecifics& new_value) {
+  sync_pb::EntitySpecifics entity_specifics;
+  entity_specifics.MutableExtension(sync_pb::bookmark)->CopyFrom(new_value);
+  PutSpecificsAndMarkForSyncing(entity_specifics);
+}
 
-  entry_->Put(syncable::BOOKMARK_URL, url_string);
+void WriteNode::PutSpecificsAndMarkForSyncing(
+    const sync_pb::EntitySpecifics& specifics) {
+  // Skip redundant changes.
+  if (specifics.SerializeAsString() ==
+      entry_->Get(SPECIFICS).SerializeAsString()) {
+    return;
+  }
+  entry_->Put(SPECIFICS, specifics);
   MarkForSyncing();
 }
 
@@ -545,7 +554,8 @@ bool WriteNode::InitByIdLookup(int64 id) {
 
 // Create a new node with default properties, and bind this WriteNode to it.
 // Return true on success.
-bool WriteNode::InitByCreation(const BaseNode& parent,
+bool WriteNode::InitByCreation(syncable::ModelType model_type,
+                               const BaseNode& parent,
                                const BaseNode* predecessor) {
   DCHECK(!entry_) << "Init called twice";
   // |predecessor| must be a child of |parent| or NULL.
@@ -568,9 +578,18 @@ bool WriteNode::InitByCreation(const BaseNode& parent,
 
   // Entries are untitled folders by default.
   entry_->Put(syncable::IS_DIR, true);
-  // TODO(ncarter): Naming this bit IS_BOOKMARK_OBJECT is a bit unfortunate,
-  // since the rest of SyncAPI is essentially bookmark-agnostic.
-  entry_->Put(syncable::IS_BOOKMARK_OBJECT, true);
+
+  // Set an empty specifics of the appropriate datatype.  The presence
+  // of the specific extension will identify the model type.
+  switch (model_type) {
+    case syncable::BOOKMARKS:
+      PutBookmarkSpecificsAndMarkForSyncing(
+          sync_pb::BookmarkSpecifics::default_instance());
+      break;
+    default:
+      NOTREACHED();
+  }
+  DCHECK(GetModelType() == model_type);
 
   // Now set the predecessor, which sets IS_UNSYNCED as necessary.
   PutPredecessor(predecessor);
@@ -629,14 +648,10 @@ void WriteNode::PutPredecessor(const BaseNode* predecessor) {
   MarkForSyncing();
 }
 
-void WriteNode::SetFaviconBytes(const unsigned char* bytes,
-                                size_t size_in_bytes) {
-  syncable::Blob new_favicon(bytes, bytes + size_in_bytes);
-  if (new_favicon == entry_->Get(syncable::BOOKMARK_FAVICON))
-    return;  // Skip redundant changes.
-
-  entry_->Put(syncable::BOOKMARK_FAVICON, new_favicon);
-  MarkForSyncing();
+void WriteNode::SetFaviconBytes(const vector<unsigned char>& bytes) {
+  sync_pb::BookmarkSpecifics new_value = GetBookmarkSpecifics();
+  new_value.set_favicon(bytes.empty() ? NULL : &bytes[0], bytes.size());
+  SetBookmarkSpecifics(new_value);
 }
 
 void WriteNode::MarkForSyncing() {
@@ -671,8 +686,10 @@ bool ReadNode::InitByIdLookup(int64 id) {
     return false;
   if (entry_->Get(syncable::IS_DEL))
     return false;
-  LOG_IF(WARNING, !entry_->Get(syncable::IS_BOOKMARK_OBJECT))
-      << "SyncAPI InitByIdLookup referencing non-bookmark object.";
+  syncable::ModelType model_type = GetModelType();
+  LOG_IF(WARNING, model_type == syncable::UNSPECIFIED ||
+                  model_type == syncable::TOP_LEVEL_FOLDER)
+      << "SyncAPI InitByIdLookup referencing unusual object.";
   return true;
 }
 
@@ -694,12 +711,13 @@ bool ReadNode::InitByTagLookup(const std::string& tag) {
     return false;
   if (entry_->Get(syncable::IS_DEL))
     return false;
-  LOG_IF(WARNING, !entry_->Get(syncable::IS_BOOKMARK_OBJECT))
-      << "SyncAPI InitByTagLookup referencing non-bookmark object.";
+  syncable::ModelType model_type = GetModelType();
+  LOG_IF(WARNING, model_type == syncable::UNSPECIFIED ||
+                  model_type == syncable::TOP_LEVEL_FOLDER)
+      << "SyncAPI InitByTagLookup referencing unusually typed object.";
   return true;
 }
 
-
 //////////////////////////////////////////////////////////////////////////
 // ReadTransaction member definitions
 ReadTransaction::ReadTransaction(UserShare* share)
@@ -917,8 +935,12 @@ class SyncManager::SyncInternal {
   // the relative order is unchanged).  To handle such cases, we rely on the
   // caller to treat a position update on any sibling as updating the positions
   // of all siblings.
-  static bool BookmarkPositionsDiffer(const syncable::EntryKernel& a,
-                                      const syncable::Entry& b) {
+  static bool VisiblePositionsDiffer(const syncable::EntryKernel& a,
+                                     const syncable::Entry& b) {
+    // If the datatype isn't one where the browser model cares about position,
+    // don't bother notifying that data model of position-only changes.
+    if (!b.ShouldMaintainPosition())
+      return false;
     if (a.ref(syncable::NEXT_ID) != b.Get(syncable::NEXT_ID))
       return true;
     if (a.ref(syncable::PARENT_ID) != b.Get(syncable::PARENT_ID))
@@ -929,17 +951,23 @@ class SyncManager::SyncInternal {
   // Determine if any of the fields made visible to clients of the Sync API
   // differ between the versions of an entry stored in |a| and |b|. A return
   // value of false means that it should be OK to ignore this change.
-  static bool BookmarkPropertiesDiffer(const syncable::EntryKernel& a,
-                                       const syncable::Entry& b) {
+  static bool VisiblePropertiesDiffer(const syncable::EntryKernel& a,
+                                      const syncable::Entry& b) {
+    syncable::ModelType model_type = b.GetModelType();
+    // Suppress updates to items that aren't tracked by any browser model.
+    if (model_type == syncable::UNSPECIFIED ||
+        model_type == syncable::TOP_LEVEL_FOLDER) {
+      return false;
+    }
     if (a.ref(syncable::NON_UNIQUE_NAME) != b.Get(syncable::NON_UNIQUE_NAME))
       return true;
     if (a.ref(syncable::IS_DIR) != b.Get(syncable::IS_DIR))
       return true;
-    if (a.ref(syncable::BOOKMARK_URL) != b.Get(syncable::BOOKMARK_URL))
+    if (a.ref(SPECIFICS).SerializeAsString() !=
+        b.Get(SPECIFICS).SerializeAsString()) {
       return true;
-    if (a.ref(syncable::BOOKMARK_FAVICON) != b.Get(syncable::BOOKMARK_FAVICON))
-      return true;
-    if (BookmarkPositionsDiffer(a, b))
+    }
+    if (VisiblePositionsDiffer(a, b))
       return true;
     return false;
   }
@@ -1398,7 +1426,7 @@ void SyncManager::SyncInternal::HandleCalculateChangesChangeEventFromSyncApi(
     if (e.IsRoot()) {
       // Ignore root object, should it ever change.
       continue;
-    } else if (!e.Get(syncable::IS_BOOKMARK_OBJECT)) {
+    } else if (e.GetModelType() != syncable::BOOKMARKS) {
       // Ignore non-bookmark objects.
       continue;
     } else if (e.Get(syncable::IS_UNSYNCED)) {
@@ -1432,15 +1460,15 @@ void SyncManager::SyncInternal::HandleCalculateChangesChangeEventFromSyncer(
     if (e.IsRoot())
       continue;
     // Ignore non-bookmark objects.
-    if (!e.Get(syncable::IS_BOOKMARK_OBJECT))
+    if (e.GetModelType() != syncable::BOOKMARKS)
       continue;
 
     if (exists_now && !existed_before)
       change_buffer_.PushAddedItem(id);
     else if (!exists_now && existed_before)
       change_buffer_.PushDeletedItem(id);
-    else if (exists_now && existed_before && BookmarkPropertiesDiffer(*i, e))
-      change_buffer_.PushUpdatedItem(id, BookmarkPositionsDiffer(*i, e));
+    else if (exists_now && existed_before && VisiblePropertiesDiffer(*i, e))
+      change_buffer_.PushUpdatedItem(id, VisiblePositionsDiffer(*i, e));
   }
 }
 
diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h
index 63ce5f5..b49d97a 100644
--- a/chrome/browser/sync/engine/syncapi.h
+++ b/chrome/browser/sync/engine/syncapi.h
@@ -39,11 +39,13 @@
 #define CHROME_BROWSER_SYNC_ENGINE_SYNCAPI_H_
 
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "build/build_config.h"
 #include "chrome/browser/google_service_auth_error.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 #include "googleurl/src/gurl.h"
 
 namespace browser_sync {
@@ -62,6 +64,11 @@ class ScopedDirLookup;
 class WriteTransaction;
 }
 
+namespace sync_pb {
+class BookmarkSpecifics;
+class EntitySpecifics;
+}
+
 namespace sync_api {
 
 // Forward declarations of classes to be defined later in this file.
@@ -104,18 +111,29 @@ class BaseNode {
   // Returns the title of the object.
   // Uniqueness of the title is not enforced on siblings -- it is not an error
   // for two children to share a title.
-  const std::wstring& GetTitle() const;
+  std::wstring GetTitle() const;
+
+  // Returns the model type of this object.  The model type is set at node
+  // creation time and is expected never to change.
+  syncable::ModelType GetModelType() const;
+
+  // Getter specific to the BOOKMARK datatype.  Returns protobuf
+  // data.  Can only be called if GetModelType() == BOOKMARK.
+  const sync_pb::BookmarkSpecifics& GetBookmarkSpecifics() const;
 
+  // Legacy, bookmark-specific getter that wraps GetBookmarkSpecifics() above.
   // Returns the URL of a bookmark object.
-  const GURL& GetURL() const;
+  // TODO(ncarter): Remove this datatype-specific accessor.
+  GURL GetURL() const;
 
-  // Return a pointer to the byte data of the favicon image for this node.
-  // Will return NULL if there is no favicon data associated with this node.
-  // The length of the array is returned to the caller via |size_in_bytes|.
+  // Legacy, bookmark-specific getter that wraps GetBookmarkSpecifics() above.
+  // Fill in a vector with the byte data of this node's favicon.  Assumes
+  // that the node is a bookmark.
   // Favicons are expected to be PNG images, and though no verification is
   // done on the syncapi client of this, the server may reject favicon updates
   // that are invalid for whatever reason.
-  const unsigned char* GetFaviconBytes(size_t* size_in_bytes);
+  // TODO(ncarter): Remove this datatype-specific accessor.
+  void GetFaviconBytes(std::vector<unsigned char>* output) const;
 
   // Returns the local external ID associated with the node.
   int64 GetExternalId() const;
@@ -132,12 +150,6 @@ class BaseNode {
   // children, return 0.
   int64 GetFirstChildId() const;
 
-  // Get an array containing the IDs of this node's children.  The memory is
-  // owned by BaseNode and becomes invalid if GetChildIds() is called a second
-  // time on this node, or when the node is destroyed.  Return the array size
-  // in the child_count parameter.
-  const int64* GetChildIds(size_t* child_count) const;
-
   // These virtual accessors provide access to data members of derived classes.
   virtual const syncable::Entry* GetEntry() const = 0;
   virtual const BaseTransaction* GetTransaction() const = 0;
@@ -147,15 +159,9 @@ class BaseNode {
   virtual ~BaseNode();
 
  private:
-  struct BaseNodeInternal;
-
   // Node is meant for stack use only.
   void* operator new(size_t size);
 
-  // Provides storage for member functions that return pointers to class
-  // memory, e.g. C strings returned by GetTitle().
-  BaseNodeInternal* data_;
-
   DISALLOW_COPY_AND_ASSIGN(BaseNode);
 };
 
@@ -173,17 +179,20 @@ class WriteNode : public BaseNode {
   // BaseNode implementation.
   virtual bool InitByIdLookup(int64 id);
 
-  // Create a new node with the specified parent and predecessor.  Use a NULL
-  // |predecessor| to indicate that this is to be the first child.
+  // Create a new node with the specified parent and predecessor.  |model_type|
+  // dictates the type of the item, and controls which EntitySpecifics proto
+  // extension can be used with this item.  Use a NULL |predecessor|
+  // to indicate that this is to be the first child.
   // |predecessor| must be a child of |new_parent| or NULL. Returns false on
   // failure.
-  bool InitByCreation(const BaseNode& parent, const BaseNode* predecessor);
+  bool InitByCreation(syncable::ModelType model_type,
+                      const BaseNode& parent,
+                      const BaseNode* predecessor);
 
   // These Set() functions correspond to the Get() functions of BaseNode.
   void SetIsFolder(bool folder);
   void SetTitle(const std::wstring& title);
-  void SetURL(const GURL& url);
-  void SetFaviconBytes(const unsigned char* bytes, size_t size_in_bytes);
+
   // External ID is a client-only field, so setting it doesn't cause the item to
   // be synced again.
   void SetExternalId(int64 external_id);
@@ -196,6 +205,16 @@ class WriteNode : public BaseNode {
   // be a child of |new_parent| or NULL.  Returns false on failure..
   bool SetPosition(const BaseNode& new_parent, const BaseNode* predecessor);
 
+  // Set the bookmark specifics (url and favicon).
+  // Should only be called if GetModelType() == BOOKMARK.
+  void SetBookmarkSpecifics(const sync_pb::BookmarkSpecifics& specifics);
+
+  // Legacy, bookmark-specific setters that wrap SetBookmarkSpecifics() above.
+  // Should only be called if GetModelType() == BOOKMARK.
+  // TODO(ncarter): Remove these two datatype-specific accessors.
+  void SetURL(const GURL& url);
+  void SetFaviconBytes(const std::vector<unsigned char>& bytes);
+
   // Implementation of BaseNode's abstract virtual accessors.
   virtual const syncable::Entry* GetEntry() const;
 
@@ -207,6 +226,16 @@ class WriteNode : public BaseNode {
   // Helper to set the previous node.
   void PutPredecessor(const BaseNode* predecessor);
 
+  // Private helpers to set type-specific protobuf data.  These don't
+  // do any checking on the previous modeltype, so they can be used
+  // for internal initialization (you can use them to set the modeltype).
+  // Additionally, they will mark for syncing if the underlying value
+  // changes.
+  void PutBookmarkSpecificsAndMarkForSyncing(
+      const sync_pb::BookmarkSpecifics& new_value);
+  void PutSpecificsAndMarkForSyncing(
+      const sync_pb::EntitySpecifics& specifics);
+
   // Sets IS_UNSYNCED and SYNCING to ensure this entry is considered in an
   // upcoming commit pass.
   void MarkForSyncing();
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index 01c2fdd..8316390 100755
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -33,16 +33,14 @@ using base::TimeDelta;
 using sync_pb::ClientCommand;
 using syncable::Blob;
 using syncable::IS_UNAPPLIED_UPDATE;
-using syncable::SERVER_BOOKMARK_FAVICON;
-using syncable::SERVER_BOOKMARK_URL;
 using syncable::SERVER_CTIME;
-using syncable::SERVER_IS_BOOKMARK_OBJECT;
 using syncable::SERVER_IS_DEL;
 using syncable::SERVER_IS_DIR;
 using syncable::SERVER_MTIME;
 using syncable::SERVER_NON_UNIQUE_NAME;
 using syncable::SERVER_PARENT_ID;
 using syncable::SERVER_POSITION_IN_PARENT;
+using syncable::SERVER_SPECIFICS;
 using syncable::SERVER_VERSION;
 using syncable::SYNCER;
 using syncable::ScopedDirLookup;
@@ -294,10 +292,8 @@ void CopyServerFields(syncable::Entry* src, syncable::MutableEntry* dest) {
   dest->Put(SERVER_VERSION, src->Get(SERVER_VERSION));
   dest->Put(SERVER_IS_DIR, src->Get(SERVER_IS_DIR));
   dest->Put(SERVER_IS_DEL, src->Get(SERVER_IS_DEL));
-  dest->Put(SERVER_IS_BOOKMARK_OBJECT, src->Get(SERVER_IS_BOOKMARK_OBJECT));
   dest->Put(IS_UNAPPLIED_UPDATE, src->Get(IS_UNAPPLIED_UPDATE));
-  dest->Put(SERVER_BOOKMARK_URL, src->Get(SERVER_BOOKMARK_URL));
-  dest->Put(SERVER_BOOKMARK_FAVICON, src->Get(SERVER_BOOKMARK_FAVICON));
+  dest->Put(SERVER_SPECIFICS, src->Get(SERVER_SPECIFICS));
   dest->Put(SERVER_POSITION_IN_PARENT, src->Get(SERVER_POSITION_IN_PARENT));
 }
 
@@ -309,10 +305,8 @@ void ClearServerData(syncable::MutableEntry* entry) {
   entry->Put(SERVER_VERSION, 0);
   entry->Put(SERVER_IS_DIR, false);
   entry->Put(SERVER_IS_DEL, false);
-  entry->Put(SERVER_IS_BOOKMARK_OBJECT, false);
   entry->Put(IS_UNAPPLIED_UPDATE, false);
-  entry->Put(SERVER_BOOKMARK_URL, "");
-  entry->Put(SERVER_BOOKMARK_FAVICON, Blob());
+  entry->Put(SERVER_SPECIFICS, sync_pb::EntitySpecifics::default_instance());
   entry->Put(SERVER_POSITION_IN_PARENT, 0);
 }
 
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 17ed151..b9729eb 100755
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -20,8 +20,9 @@
 #include "chrome/browser/sync/engine/net/server_connection_manager.h"
 #include "chrome/browser/sync/engine/process_updates_command.h"
 #include "chrome/browser/sync/engine/syncer.h"
-#include "chrome/browser/sync/engine/syncer_util.h"
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
+#include "chrome/browser/sync/engine/syncer_util.h"
+#include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/protocol/sync.pb.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
@@ -65,7 +66,6 @@ using syncable::GET_BY_HANDLE;
 using syncable::GET_BY_ID;
 using syncable::GET_BY_TAG;
 using syncable::ID;
-using syncable::IS_BOOKMARK_OBJECT;
 using syncable::IS_DEL;
 using syncable::IS_DIR;
 using syncable::IS_UNAPPLIED_UPDATE;
@@ -80,8 +80,10 @@ using syncable::SERVER_IS_DEL;
 using syncable::SERVER_NON_UNIQUE_NAME;
 using syncable::SERVER_PARENT_ID;
 using syncable::SERVER_POSITION_IN_PARENT;
+using syncable::SERVER_SPECIFICS;
 using syncable::SERVER_VERSION;
 using syncable::SINGLETON_TAG;
+using syncable::SPECIFICS;
 using syncable::UNITTEST;
 
 using sessions::ConflictProgress;
@@ -205,6 +207,10 @@ class SyncerTest : public testing::Test,
     ExtendedAttributeKey key(entry->Get(META_HANDLE), "DATA");
     MutableExtendedAttribute attr(trans, CREATE, key);
     attr.mutable_value()->swap(test_value);
+    sync_pb::EntitySpecifics specifics;
+    specifics.MutableExtension(sync_pb::bookmark)->set_url("http://demo/");
+    specifics.MutableExtension(sync_pb::bookmark)->set_favicon("PNG");
+    entry->Put(syncable::SPECIFICS, specifics);
     entry->Put(syncable::IS_UNSYNCED, true);
   }
   void VerifyTestDataInEntry(BaseTransaction* trans, Entry* entry) {
@@ -215,7 +221,15 @@ class SyncerTest : public testing::Test,
     ExtendedAttribute attr(trans, GET_BY_HANDLE, key);
     EXPECT_FALSE(attr.is_deleted());
     EXPECT_TRUE(test_value == attr.value());
+    VerifyTestBookmarkDataInEntry(entry);
   }
+  void VerifyTestBookmarkDataInEntry(Entry* entry) {
+    const sync_pb::EntitySpecifics& specifics = entry->Get(syncable::SPECIFICS);
+    EXPECT_TRUE(specifics.HasExtension(sync_pb::bookmark));
+    EXPECT_EQ("PNG", specifics.GetExtension(sync_pb::bookmark).favicon());
+    EXPECT_EQ("http://demo/", specifics.GetExtension(sync_pb::bookmark).url());
+  }
+
   void SyncRepeatedlyToTriggerConflictResolution(SyncSession* session) {
     // We should trigger after less than 6 syncs, but extra does no harm.
     for (int i = 0 ; i < 6 ; ++i)
@@ -227,7 +241,11 @@ class SyncerTest : public testing::Test,
     for (int i = 0 ; i < 12 ; ++i)
       syncer_->SyncShare(session);
   }
-
+  sync_pb::EntitySpecifics DefaultBookmarkSpecifics() {
+    sync_pb::EntitySpecifics result;
+    result.MutableExtension(sync_pb::bookmark);
+    return result;
+  }
   // Enumeration of alterations to entries for commit ordering tests.
   enum EntryFeature {
     LIST_END = 0,  // Denotes the end of the list of features from below.
@@ -251,6 +269,7 @@ class SyncerTest : public testing::Test,
   void RunCommitOrderingTest(CommitOrderingTest* test) {
     ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
     ASSERT_TRUE(dir.good());
+
     map<int, syncable::Id> expected_positions;
     {  // Transaction scope.
       WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__);
@@ -264,6 +283,7 @@ class SyncerTest : public testing::Test,
         string utf8_name = test->id.GetServerId();
         string name(utf8_name.begin(), utf8_name.end());
         MutableEntry entry(&trans, CREATE, test->parent_id, name);
+
         entry.Put(syncable::ID, test->id);
         if (test->id.ServerKnows()) {
           entry.Put(BASE_VERSION, 5);
@@ -272,6 +292,7 @@ class SyncerTest : public testing::Test,
         }
         entry.Put(syncable::IS_DIR, true);
         entry.Put(syncable::IS_UNSYNCED, true);
+        entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
         // Set the time to 30 seconds in the future to reduce the chance of
         // flaky tests.
         int64 now_server_time = ClientTimeToServerTime(syncable::Now());
@@ -351,6 +372,7 @@ class SyncerTest : public testing::Test,
     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);
@@ -413,11 +435,11 @@ TEST_F(SyncerTest, GetCommitIdsCommandTruncates) {
     MutableEntry entry_c(&wtrans, GET_BY_HANDLE, handle_c);
     MutableEntry entry_d(&wtrans, GET_BY_HANDLE, handle_d);
     MutableEntry entry_e(&wtrans, GET_BY_HANDLE, handle_e);
-    entry_x.Put(IS_BOOKMARK_OBJECT, true);
-    entry_b.Put(IS_BOOKMARK_OBJECT, true);
-    entry_c.Put(IS_BOOKMARK_OBJECT, true);
-    entry_d.Put(IS_BOOKMARK_OBJECT, true);
-    entry_e.Put(IS_BOOKMARK_OBJECT, true);
+    entry_x.Put(SPECIFICS, DefaultBookmarkSpecifics());
+    entry_b.Put(SPECIFICS, DefaultBookmarkSpecifics());
+    entry_c.Put(SPECIFICS, DefaultBookmarkSpecifics());
+    entry_d.Put(SPECIFICS, DefaultBookmarkSpecifics());
+    entry_e.Put(SPECIFICS, DefaultBookmarkSpecifics());
     entry_b.Put(PARENT_ID, entry_x.Get(ID));
     entry_c.Put(PARENT_ID, entry_x.Get(ID));
     entry_c.PutPredecessor(entry_b.Get(ID));
@@ -495,6 +517,7 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
     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");
@@ -661,6 +684,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       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),
@@ -668,6 +692,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       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),
@@ -675,6 +700,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       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);
     }
     {
@@ -736,11 +762,13 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
     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");
     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_);
     parent.Put(syncable::BASE_VERSION, 1);
     child.Put(syncable::BASE_VERSION, 1);
@@ -751,11 +779,13 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
     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");
     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);
   }
@@ -765,11 +795,13 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
     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");
     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);
   }
@@ -798,6 +830,7 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
     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");
     ASSERT_TRUE(child1.good());
@@ -836,6 +869,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
     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);
   }
@@ -848,12 +882,14 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
     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);
     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);
   }
@@ -906,6 +942,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
     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);
   }
@@ -919,6 +956,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
     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);
     meta_handle_a = parent2.Get(syncable::META_HANDLE);
@@ -926,6 +964,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
     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);
   }
@@ -1197,6 +1236,7 @@ TEST_F(SyncerTest, CommitTimeRename) {
     MutableEntry parent(&trans, CREATE, 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);
 
@@ -1246,6 +1286,7 @@ TEST_F(SyncerTest, CommitTimeRenameI18N) {
     MutableEntry parent(&trans, CREATE, 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);
   }
@@ -1279,6 +1320,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
     MutableEntry entry(&trans, CREATE, 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);
   }
@@ -1604,6 +1646,7 @@ class EntryCreatedInNewFolderTest : public SyncerTest {
     CHECK(entry2.good());
     entry2.Put(syncable::IS_DIR, true);
     entry2.Put(syncable::IS_UNSYNCED, true);
+    entry2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
   }
 };
 
@@ -1617,6 +1660,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
     ASSERT_TRUE(entry.good());
     entry.Put(syncable::IS_DIR, true);
     entry.Put(syncable::IS_UNSYNCED, true);
+    entry.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   mock_server_->SetMidCommitCallback(
@@ -1697,24 +1741,28 @@ TEST_F(SyncerTest, DoublyChangedWithResolver) {
     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");
     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_, "Pete.htm", 11, 10);
+  mock_server_->AddUpdateBookmark(child_id_, parent_id_, "Pete2.htm", 11, 10);
   mock_server_->set_conflict_all_commits(true);
   LoopSyncShare(syncer_);
   syncable::Directory::ChildHandles children;
   {
     ReadTransaction trans(dir, __FILE__, __LINE__);
     dir->GetChildHandles(&trans, parent_id_, &children);
-    // We expect the conflict resolver to just clobber the entry.
+    // We expect the conflict resolver to preserve the local entry.
     Entry child(&trans, syncable::GET_BY_ID, child_id_);
     ASSERT_TRUE(child.good());
     EXPECT_TRUE(child.Get(syncable::IS_UNSYNCED));
     EXPECT_FALSE(child.Get(syncable::IS_UNAPPLIED_UPDATE));
+    EXPECT_TRUE(child.Get(SPECIFICS).HasExtension(sync_pb::bookmark));
+    EXPECT_EQ("Pete.htm", child.Get(NON_UNIQUE_NAME));
+    VerifyTestBookmarkDataInEntry(&child);
   }
 
   // Only one entry, since we just overwrite one.
@@ -1735,6 +1783,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
     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);
@@ -1777,13 +1826,13 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
     parent.Put(IS_UNSYNCED, true);
     parent.Put(ID, parent_id);
     parent.Put(BASE_VERSION, 1);
-    parent.Put(IS_BOOKMARK_OBJECT, true);
+    parent.Put(SPECIFICS, DefaultBookmarkSpecifics());
 
     MutableEntry child(&wtrans, CREATE, parent.Get(ID), "test.htm");
     ASSERT_TRUE(child.good());
     child.Put(ID, child_id);
     child.Put(BASE_VERSION, 1);
-    child.Put(IS_BOOKMARK_OBJECT, true);
+    child.Put(SPECIFICS, DefaultBookmarkSpecifics());
     WriteTestDataToEntry(&wtrans, &child);
   }
   mock_server_->AddUpdateDirectory(parent_id, root_id_, "Folder", 10, 10);
@@ -1844,6 +1893,8 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) {
     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);
   }
   syncer_->SyncShare(session_.get());
@@ -1875,6 +1926,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
     MutableEntry entry(&trans, CREATE, 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);
   }
@@ -1884,6 +1936,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
     MutableEntry newfolder(&trans, CREATE, 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);
@@ -1912,6 +1965,8 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
     MutableEntry newfolder(&trans, CREATE, 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);
@@ -2004,6 +2059,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo) {
       MutableEntry e(&trans, CREATE, trans.root_id(), name);
       e.Put(IS_UNSYNCED, true);
       e.Put(IS_DIR, true);
+      e.Put(SPECIFICS, DefaultBookmarkSpecifics());
     }
   }
   uint32 num_loops = 0;
@@ -2107,6 +2163,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
     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);
@@ -2115,8 +2172,118 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   mock_server_->set_conflict_all_commits(true);
   syncer_->SyncShare(this);
   syncer_events_.clear();
+  {
+    // Update #20 should have been dropped in favor of the local version.
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1));
+    MutableEntry local(&wtrans, GET_BY_HANDLE, local_folder_handle);
+    ASSERT_TRUE(server.good());
+    ASSERT_TRUE(local.good());
+    EXPECT_TRUE(local.Get(META_HANDLE) != server.Get(META_HANDLE));
+    EXPECT_FALSE(server.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(local.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_TRUE(server.Get(IS_UNSYNCED));
+    EXPECT_TRUE(local.Get(IS_UNSYNCED));
+    EXPECT_EQ("Foo.htm", server.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("Bar.htm", local.Get(NON_UNIQUE_NAME));
+  }
+  // Allow local changes to commit.
+  mock_server_->set_conflict_all_commits(false);
+  syncer_->SyncShare(this);
+  syncer_events_.clear();
+
+  // Now add a server change to make the two names equal.  There should
+  // be no conflict with that, since names are not unique.
+  mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30);
+  syncer_->SyncShare(this);
+  syncer_events_.clear();
+  {
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1));
+    MutableEntry local(&wtrans, GET_BY_HANDLE, local_folder_handle);
+    ASSERT_TRUE(server.good());
+    ASSERT_TRUE(local.good());
+    EXPECT_TRUE(local.Get(META_HANDLE) != server.Get(META_HANDLE));
+    EXPECT_FALSE(server.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(local.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(server.Get(IS_UNSYNCED));
+    EXPECT_FALSE(local.Get(IS_UNSYNCED));
+    EXPECT_EQ("Bar.htm", server.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("Bar.htm", local.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("http://google.com",  // Default from AddUpdateBookmark.
+        server.Get(SPECIFICS).GetExtension(sync_pb::bookmark).url());
+  }
 }
 
+// Same as NewEntryAnddServerEntrySharePath, but using the old-style protocol.
+TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
+  mock_server_->set_use_legacy_bookmarks_protocol(true);
+  ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
+  CHECK(dir.good());
+  mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10);
+  syncer_->SyncShare(this);
+  int64 local_folder_handle;
+  syncable::Id local_folder_id;
+  {
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry new_entry(&wtrans, CREATE, 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);
+  mock_server_->set_conflict_all_commits(true);
+  syncer_->SyncShare(this);
+  syncer_events_.clear();
+  {
+    // Update #20 should have been dropped in favor of the local version.
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1));
+    MutableEntry local(&wtrans, GET_BY_HANDLE, local_folder_handle);
+    ASSERT_TRUE(server.good());
+    ASSERT_TRUE(local.good());
+    EXPECT_TRUE(local.Get(META_HANDLE) != server.Get(META_HANDLE));
+    EXPECT_FALSE(server.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(local.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_TRUE(server.Get(IS_UNSYNCED));
+    EXPECT_TRUE(local.Get(IS_UNSYNCED));
+    EXPECT_EQ("Foo.htm", server.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("Bar.htm", local.Get(NON_UNIQUE_NAME));
+  }
+  // Allow local changes to commit.
+  mock_server_->set_conflict_all_commits(false);
+  syncer_->SyncShare(this);
+  syncer_events_.clear();
+
+  // Now add a server change to make the two names equal.  There should
+  // be no conflict with that, since names are not unique.
+  mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30);
+  syncer_->SyncShare(this);
+  syncer_events_.clear();
+  {
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry server(&wtrans, GET_BY_ID, ids_.FromNumber(1));
+    MutableEntry local(&wtrans, GET_BY_HANDLE, local_folder_handle);
+    ASSERT_TRUE(server.good());
+    ASSERT_TRUE(local.good());
+    EXPECT_TRUE(local.Get(META_HANDLE) != server.Get(META_HANDLE));
+    EXPECT_FALSE(server.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(local.Get(IS_UNAPPLIED_UPDATE));
+    EXPECT_FALSE(server.Get(IS_UNSYNCED));
+    EXPECT_FALSE(local.Get(IS_UNSYNCED));
+    EXPECT_EQ("Bar.htm", server.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("Bar.htm", local.Get(NON_UNIQUE_NAME));
+    EXPECT_EQ("http://google.com",  // Default from AddUpdateBookmark.
+        server.Get(SPECIFICS).GetExtension(sync_pb::bookmark).url());
+  }
+}
+
+
 // Circular links should be resolved by the server.
 TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
   // we don't currently resolve this. This test ensures we don't.
@@ -3075,10 +3242,12 @@ TEST_F(SyncerTest, DuplicateIDReturn) {
     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");
     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"));
   }
@@ -3158,7 +3327,7 @@ TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
     local_deleted.Put(IS_DEL, true);
     local_deleted.Put(IS_DIR, false);
     local_deleted.Put(IS_UNSYNCED, true);
-    local_deleted.Put(IS_BOOKMARK_OBJECT, true);
+    local_deleted.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   mock_server_->AddUpdateBookmark(ids_.FromNumber(1), root_id_, "name", 10, 10);
@@ -3476,6 +3645,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
     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);
 
@@ -3483,6 +3653,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
     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);
   }
@@ -3571,15 +3742,16 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
       "folder_two", 1, 1);
   syncer_->SyncShare(this);
   {
-    // A moved entry should send an old parent.
+    // A moved entry should send an "old parent."
     WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__);
     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 parent.
+    // A new entry should send no "old parent."
     MutableEntry create(&trans, CREATE, trans.root_id(), "new_folder");
     create.Put(IS_UNSYNCED, true);
+    create.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
   syncer_->SyncShare(this);
   const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit();
diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc
index da451a5..c784b93 100755
--- a/chrome/browser/sync/engine/syncer_util.cc
+++ b/chrome/browser/sync/engine/syncer_util.cc
@@ -14,13 +14,12 @@
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 #include "chrome/browser/sync/syncable/syncable_changes_version.h"
 #include "chrome/browser/sync/util/sync_types.h"
 
 using syncable::BASE_VERSION;
-using syncable::BOOKMARK_FAVICON;
-using syncable::BOOKMARK_URL;
 using syncable::Blob;
 using syncable::CHANGES_VERSION;
 using syncable::CREATE;
@@ -33,7 +32,6 @@ using syncable::ExtendedAttributeKey;
 using syncable::GET_BY_HANDLE;
 using syncable::GET_BY_ID;
 using syncable::ID;
-using syncable::IS_BOOKMARK_OBJECT;
 using syncable::IS_DEL;
 using syncable::IS_DIR;
 using syncable::IS_UNAPPLIED_UPDATE;
@@ -48,18 +46,17 @@ using syncable::NON_UNIQUE_NAME;
 using syncable::PARENT_ID;
 using syncable::PREV_ID;
 using syncable::ReadTransaction;
-using syncable::SERVER_BOOKMARK_FAVICON;
-using syncable::SERVER_BOOKMARK_URL;
 using syncable::SERVER_CTIME;
-using syncable::SERVER_IS_BOOKMARK_OBJECT;
 using syncable::SERVER_IS_DEL;
 using syncable::SERVER_IS_DIR;
 using syncable::SERVER_MTIME;
 using syncable::SERVER_NON_UNIQUE_NAME;
 using syncable::SERVER_PARENT_ID;
 using syncable::SERVER_POSITION_IN_PARENT;
+using syncable::SERVER_SPECIFICS;
 using syncable::SERVER_VERSION;
 using syncable::SINGLETON_TAG;
+using syncable::SPECIFICS;
 using syncable::SYNCER;
 using syncable::WriteTransaction;
 
@@ -237,14 +234,24 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
 }
 
 namespace {
-void UpdateLocalBookmarkSpecifics(const string& url,
-                                  const string& favicon_bytes,
-                                  MutableEntry* local_entry) {
-  local_entry->Put(SERVER_BOOKMARK_URL, url);
-  Blob favicon_blob;
-  SyncerProtoUtil::CopyProtoBytesIntoBlob(favicon_bytes,
-                                          &favicon_blob);
-  local_entry->Put(SERVER_BOOKMARK_FAVICON, favicon_blob);
+// Helper to synthesize a new-style sync_pb::EntitySpecifics for use locally,
+// when the server speaks only the old sync_pb::SyncEntity_BookmarkData-based
+// protocol.
+void UpdateBookmarkSpecifics(const string& singleton_tag,
+                             const string& url,
+                             const string& favicon_bytes,
+                             MutableEntry* local_entry) {
+  // In the new-style protocol, the server no longer sends bookmark info for
+  // the "google_chrome" folder.  Mimic that here.
+  if (singleton_tag == "google_chrome")
+    return;
+  sync_pb::EntitySpecifics pb;
+  sync_pb::BookmarkSpecifics* bookmark = pb.MutableExtension(sync_pb::bookmark);
+  if (!url.empty())
+    bookmark->set_url(url);
+  if (!favicon_bytes.empty())
+    bookmark->set_favicon(favicon_bytes);
+  local_entry->Put(SERVER_SPECIFICS, pb);
 }
 
 }  // namespace
@@ -275,27 +282,23 @@ void SyncerUtil::UpdateServerFieldsFromUpdate(
       ServerTimeToClientTime(server_entry.ctime()));
   local_entry->Put(SERVER_MTIME,
       ServerTimeToClientTime(server_entry.mtime()));
-  local_entry->Put(SERVER_IS_BOOKMARK_OBJECT,
-      GetModelType(server_entry) == syncable::BOOKMARKS);
   local_entry->Put(SERVER_IS_DIR, server_entry.IsFolder());
   if (server_entry.has_singleton_tag()) {
     const string& tag = server_entry.singleton_tag();
     local_entry->Put(SINGLETON_TAG, tag);
   }
-  if (!server_entry.deleted()) {
-    if (server_entry.specifics().HasExtension(sync_pb::bookmark)) {
-      // New style bookmark data.
-      const sync_pb::BookmarkSpecifics& bookmark =
-          server_entry.specifics().GetExtension(sync_pb::bookmark);
-      UpdateLocalBookmarkSpecifics(bookmark.url(),
-          bookmark.favicon(), local_entry);
-    } else if (server_entry.has_bookmarkdata()) {
-      // Old style bookmark data.
-      const SyncEntity::BookmarkData& bookmark = server_entry.bookmarkdata();
-      UpdateLocalBookmarkSpecifics(bookmark.bookmark_url(),
-                                   bookmark.bookmark_favicon(),
-                                   local_entry);
-    }
+  // Store the datatype-specific part as a protobuf.
+  if (server_entry.has_specifics()) {
+    DCHECK(server_entry.GetModelType() != syncable::UNSPECIFIED)
+        << "Storing unrecognized datatype in sync database.";
+    local_entry->Put(SERVER_SPECIFICS, server_entry.specifics());
+  } else if (server_entry.has_bookmarkdata()) {
+    // Legacy protocol response for bookmark data.
+    const SyncEntity::BookmarkData& bookmark = server_entry.bookmarkdata();
+    UpdateBookmarkSpecifics(server_entry.singleton_tag(),
+                            bookmark.bookmark_url(),
+                            bookmark.bookmark_favicon(),
+                            local_entry);
   }
   if (server_entry.has_position_in_parent()) {
     local_entry->Put(SERVER_POSITION_IN_PARENT,
@@ -391,13 +394,11 @@ bool SyncerUtil::ServerAndLocalEntriesMatch(syncable::Entry* entry) {
     return false;
   }
 
-  if (entry->Get(IS_BOOKMARK_OBJECT)) {
-    if (!entry->Get(IS_DIR)) {
-      if (entry->Get(BOOKMARK_URL) != entry->Get(SERVER_BOOKMARK_URL)) {
-        LOG(WARNING) << "Bookmark URL mismatch";
-        return false;
-      }
-    }
+  // TODO(ncarter): This is unfortunately heavyweight.  Can we do better?
+  if (entry->Get(SPECIFICS).SerializeAsString() !=
+      entry->Get(SERVER_SPECIFICS).SerializeAsString()) {
+    LOG(WARNING) << "Specifics mismatch";
+    return false;
   }
   if (entry->Get(IS_DIR))
     return true;
@@ -435,8 +436,11 @@ void SyncerUtil::UpdateLocalDataFromServerData(
     syncable::MutableEntry* entry) {
   CHECK(!entry->Get(IS_UNSYNCED));
   CHECK(entry->Get(IS_UNAPPLIED_UPDATE));
+
   LOG(INFO) << "Updating entry : " << *entry;
-  entry->Put(IS_BOOKMARK_OBJECT, entry->Get(SERVER_IS_BOOKMARK_OBJECT));
+  // Start by setting the properties that determine the model_type.
+  entry->Put(SPECIFICS, entry->Get(SERVER_SPECIFICS));
+  entry->Put(IS_DIR, entry->Get(SERVER_IS_DIR));
   // This strange dance around the IS_DEL flag avoids problems when setting
   // the name.
   // TODO(chron): Is this still an issue? Unit test this codepath.
@@ -455,10 +459,7 @@ void SyncerUtil::UpdateLocalDataFromServerData(
   entry->Put(CTIME, entry->Get(SERVER_CTIME));
   entry->Put(MTIME, entry->Get(SERVER_MTIME));
   entry->Put(BASE_VERSION, entry->Get(SERVER_VERSION));
-  entry->Put(IS_DIR, entry->Get(SERVER_IS_DIR));
   entry->Put(IS_DEL, entry->Get(SERVER_IS_DEL));
-  entry->Put(BOOKMARK_URL, entry->Get(SERVER_BOOKMARK_URL));
-  entry->Put(BOOKMARK_FAVICON, entry->Get(SERVER_BOOKMARK_FAVICON));
   entry->Put(IS_UNAPPLIED_UPDATE, false);
 }
 
@@ -616,7 +617,7 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency(
     syncable::MutableEntry* same_id,
     const bool deleted,
     const bool is_directory,
-    const bool has_bookmark_data) {
+    syncable::ModelType model_type) {
 
   CHECK(same_id->good());
 
@@ -624,11 +625,16 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency(
   if (deleted)
     return VERIFY_SUCCESS;
 
+  if (model_type == syncable::UNSPECIFIED) {
+    // This update is to an item of a datatype we don't recognize. The server
+    // shouldn't have sent it to us.  Throw it on the ground.
+    return VERIFY_SKIP;
+  }
+
   if (same_id->Get(SERVER_VERSION) > 0) {
     // Then we've had an update for this entry before.
     if (is_directory != same_id->Get(SERVER_IS_DIR) ||
-        (!is_directory &&
-         has_bookmark_data != same_id->Get(SERVER_IS_BOOKMARK_OBJECT))) {
+        model_type != same_id->GetServerModelType()) {
       if (same_id->Get(IS_DEL)) {  // If we've deleted the item, we don't care.
         return VERIFY_SKIP;
       } else {
@@ -657,8 +663,7 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency(
   if (same_id->Get(BASE_VERSION) > 0) {
     // We've committed this entry in the past.
     if (is_directory != same_id->Get(IS_DIR) ||
-        (!is_directory &&
-         has_bookmark_data != same_id->Get(IS_BOOKMARK_OBJECT))) {
+        model_type != same_id->GetModelType()) {
       LOG(ERROR) << "Server update doesn't agree with committed item. ";
       LOG(ERROR) << " Entry: " << *same_id;
       LOG(ERROR) << " Update: " << SyncEntityDebugString(entry);
diff --git a/chrome/browser/sync/engine/syncer_util.h b/chrome/browser/sync/engine/syncer_util.h
index f42bbe3..3f8d371 100755
--- a/chrome/browser/sync/engine/syncer_util.h
+++ b/chrome/browser/sync/engine/syncer_util.h
@@ -88,7 +88,7 @@ class SyncerUtil {
                                               syncable::MutableEntry* same_id,
                                               const bool deleted,
                                               const bool is_directory,
-                                              const bool is_bookmark);
+                                              syncable::ModelType model_type);
 
   // Assumes we have an existing entry; verify an update that seems to be
   // expressing an 'undelete'
diff --git a/chrome/browser/sync/engine/syncproto.h b/chrome/browser/sync/engine/syncproto.h
index cbe56a1..1d5f428 100644
--- a/chrome/browser/sync/engine/syncproto.h
+++ b/chrome/browser/sync/engine/syncproto.h
@@ -7,7 +7,9 @@
 #ifndef CHROME_BROWSER_SYNC_ENGINE_SYNCPROTO_H_
 #define CHROME_BROWSER_SYNC_ENGINE_SYNCPROTO_H_
 
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/protocol/sync.pb.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 #include "chrome/browser/sync/syncable/syncable_id.h"
 
 namespace browser_sync {
@@ -49,6 +51,28 @@ class SyncEntity : public IdWrapper<sync_pb::SyncEntity> {
     return ((has_folder() && folder()) ||
             (has_bookmarkdata() && bookmarkdata().bookmark_folder()));
   }
+
+  // Note: keep this consistent with GetModelType in syncable.cc!
+  syncable::ModelType GetModelType() const {
+    DCHECK(!id().IsRoot());  // Root shouldn't ever go over the wire.
+
+    if (deleted())
+      return syncable::UNSPECIFIED;
+
+    if (specifics().HasExtension(sync_pb::bookmark) || has_bookmarkdata())
+      return syncable::BOOKMARKS;
+
+    // Loose check for server-created top-level folders that aren't
+    // bound to a particular model type.
+    if (!singleton_tag().empty() && IsFolder())
+      return syncable::TOP_LEVEL_FOLDER;
+
+    // This is an item of a datatype we can't understand. Maybe it's
+    // from the future?  Either we mis-encoded the object, or the
+    // server sent us entries it shouldn't have.
+    NOTREACHED() << "Unknown datatype in sync proto.";
+    return syncable::UNSPECIFIED;
+  }
 };
 
 class CommitResponse_EntryResponse
diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc
index d357c7c..76b3851 100755
--- a/chrome/browser/sync/engine/verify_updates_command.cc
+++ b/chrome/browser/sync/engine/verify_updates_command.cc
@@ -59,8 +59,7 @@ VerifyResult VerifyUpdatesCommand::VerifyUpdate(
 
   const bool deleted = entry.has_deleted() && entry.deleted();
   const bool is_directory = entry.IsFolder();
-  const bool is_bookmark =
-      SyncerUtil::GetModelType(entry) == syncable::BOOKMARKS;
+  const syncable::ModelType model_type = entry.GetModelType();
 
   if (!id.ServerKnows()) {
     LOG(ERROR) << "Illegal negative id in received updates";
@@ -91,7 +90,7 @@ VerifyResult VerifyUpdatesCommand::VerifyUpdate(
   // consistency rules.
   if (VERIFY_UNDECIDED == result) {
     result = SyncerUtil::VerifyUpdateConsistency(trans, entry, &same_id,
-      deleted, is_directory, is_bookmark);
+        deleted, is_directory, model_type);
   }
 
   if (VERIFY_UNDECIDED == result)