Merge protocol buffer changes from server back into client.

Use new style protocol in addition to old style protocol when talking to the server in preparation for new data type synchronization.

original patch from issue 487011

TEST=manually tested old client new client new server mixes
BUG=30032

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

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

17 files changed, 213 insertions, 61 deletions

diff --git a/chrome/browser/sync/engine/authenticator.cc b/chrome/browser/sync/engine/authenticator.cc
index 6bffd9f..455e153 100644
--- a/chrome/browser/sync/engine/authenticator.cc
+++ b/chrome/browser/sync/engine/authenticator.cc
@@ -46,7 +46,7 @@ Authenticator::AuthenticationResult Authenticator::Authenticate(
   return AuthenticateToken(auth_service.auth_token());
 }
 
-COMPILE_ASSERT(sync_pb::ClientToServerResponse::ERROR_TYPE_MAX == 6,
+COMPILE_ASSERT(sync_pb::ClientToServerResponse::ErrorType_MAX == 6,
                client_to_server_response_errors_changed);
 
 Authenticator::AuthenticationResult Authenticator::HandleSuccessfulTokenRequest(
diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc
index 2381436..9544405 100644..100755
--- a/chrome/browser/sync/engine/build_commit_command.cc
+++ b/chrome/browser/sync/engine/build_commit_command.cc
@@ -10,6 +10,7 @@
 
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 #include "chrome/browser/sync/syncable/syncable_changes_version.h"
@@ -43,6 +44,46 @@ void BuildCommitCommand::AddExtensionsActivityToMessage(
   }
 }
 
+namespace {
+void SetNewStyleBookmarkData(MutableEntry* meta_entry, SyncEntity* sync_entry) {
+
+  // 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);
+  }
+}
+
+void SetOldStyleBookmarkData(MutableEntry* meta_entry, SyncEntity* sync_entry) {
+
+  // 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());
+
+    bookmark->set_bookmark_folder(false);
+  } else {
+    bookmark->set_bookmark_folder(true);
+  }
+}
+}  // namespace
+
 void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
   ClientToServerMessage message;
   message.set_share(session->context()->account_name());
@@ -132,20 +173,17 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
     if (meta_entry.Get(syncable::IS_DEL)) {
       sync_entry->set_deleted(true);
     } else if (meta_entry.Get(syncable::IS_BOOKMARK_OBJECT)) {
-      sync_pb::SyncEntity_BookmarkData* bookmark =
-          sync_entry->mutable_bookmarkdata();
-      bookmark->set_bookmark_folder(meta_entry.Get(syncable::IS_DIR));
+
+      // 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();
       sync_entry->set_insert_after_item_id(prev_string);
 
-      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());
-      }
+      // TODO(ncarter): In practice we won't want to send this data twice
+      // 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);
     }
   }
   session->status_controller()->mutable_commit_message()->CopyFrom(message);
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index ac05cfa..19cd9bd 100755
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -116,7 +116,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
   { // Scope for WriteTransaction.
     WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__);
     for (int i = 0; i < cr.entryresponse_size(); i++) {
-      CommitResponse::RESPONSE_TYPE response_type =
+      CommitResponse::ResponseType response_type =
           ProcessSingleCommitResponse(&trans, cr.entryresponse(i),
                                       commit_ids[i],
                                       &conflicting_new_folder_ids,
@@ -179,7 +179,7 @@ void LogServerError(const CommitResponse_EntryResponse& res) {
     LOG(ERROR) << "  No detailed error message returned from server";
 }
 
-CommitResponse::RESPONSE_TYPE
+CommitResponse::ResponseType
 ProcessCommitResponseCommand::ProcessSingleCommitResponse(
     syncable::WriteTransaction* trans,
     const sync_pb::CommitResponse_EntryResponse& pb_server_entry,
@@ -194,9 +194,9 @@ ProcessCommitResponseCommand::ProcessSingleCommitResponse(
   bool syncing_was_set = local_entry.Get(SYNCING);
   local_entry.Put(SYNCING, false);
 
-  CommitResponse::RESPONSE_TYPE response = (CommitResponse::RESPONSE_TYPE)
+  CommitResponse::ResponseType response = (CommitResponse::ResponseType)
       server_entry.response_type();
-  if (!CommitResponse::RESPONSE_TYPE_IsValid(response)) {
+  if (!CommitResponse::ResponseType_IsValid(response)) {
     LOG(ERROR) << "Commit response has unknown response type! Possibly out "
                "of date client?";
     return CommitResponse::INVALID_MESSAGE;
diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h
index 1eae175..b1ba659 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.h
+++ b/chrome/browser/sync/engine/process_commit_response_command.h
@@ -29,7 +29,7 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand {
   virtual void ModelChangingExecuteImpl(sessions::SyncSession* session);
 
  private:
-  CommitResponse::RESPONSE_TYPE ProcessSingleCommitResponse(
+  CommitResponse::ResponseType ProcessSingleCommitResponse(
       syncable::WriteTransaction* trans,
       const sync_pb::CommitResponse_EntryResponse& pb_server_entry,
       const syncable::Id& pre_commit_id, std::set<syncable::Id>*
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index 0f89442..8a337f5 100755
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -121,7 +121,8 @@ 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 = entry.has_bookmarkdata();
+  const bool is_bookmark =
+      SyncerUtil::GetSyncDataType(entry) == SYNC_TYPE_BOOKMARK;
 
   return VERIFY_SUCCESS == SyncerUtil::VerifyUpdateConsistency(trans,
                                                                entry,
diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h
index d1bbe9d..d5da671 100755
--- a/chrome/browser/sync/engine/syncer.h
+++ b/chrome/browser/sync/engine/syncer.h
@@ -110,15 +110,15 @@ class Syncer {
   // Volatile reader for the source member of the syncer session object.  The
   // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has
   // been read.
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE TestAndSetUpdatesSource() {
-    sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE old_source =
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource TestAndSetUpdatesSource() {
+    sync_pb::GetUpdatesCallerInfo::GetUpdatesSource old_source =
         updates_source_;
     set_updates_source(sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION);
     return old_source;
   }
 
   void set_updates_source(
-      sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source) {
+      sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source) {
     updates_source_ = source;
   }
 
@@ -150,7 +150,7 @@ class Syncer {
   scoped_ptr<ShutdownChannel> shutdown_channel_;
 
   // The source of the last nudge.
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE updates_source_;
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source_;
 
   // A callback hook used in unittests to simulate changes between conflict set
   // building and conflict resolution.
diff --git a/chrome/browser/sync/engine/syncer_thread.cc b/chrome/browser/sync/engine/syncer_thread.cc
index d8271a2..f80ba16 100644
--- a/chrome/browser/sync/engine/syncer_thread.cc
+++ b/chrome/browser/sync/engine/syncer_thread.cc
@@ -463,7 +463,7 @@ bool SyncerThread::UpdateNudgeSource(bool was_throttled,
 
 void SyncerThread::SetUpdatesSource(bool nudged, NudgeSource nudge_source,
                                     bool* initial_sync) {
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE updates_source =
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source =
       sync_pb::GetUpdatesCallerInfo::UNKNOWN;
   if (*initial_sync) {
     updates_source = sync_pb::GetUpdatesCallerInfo::FIRST_UPDATE;
diff --git a/chrome/browser/sync/engine/syncer_types.h b/chrome/browser/sync/engine/syncer_types.h
index 79c53f9..925b109 100755
--- a/chrome/browser/sync/engine/syncer_types.h
+++ b/chrome/browser/sync/engine/syncer_types.h
@@ -69,6 +69,11 @@ enum VerifyCommitResult {
   VERIFY_OK,
 };
 
+enum SyncDataType {
+  SYNC_TYPE_BOOKMARK,
+  SYNC_TYPE_UNKNOWN,
+};
+
 struct SyncerEvent {
   typedef SyncerEvent EventType;
 
diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc
index d0e9cb3..21656be 100755
--- a/chrome/browser/sync/engine/syncer_util.cc
+++ b/chrome/browser/sync/engine/syncer_util.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
 #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/syncable.h"
 #include "chrome/browser/sync/syncable/syncable_changes_version.h"
@@ -235,6 +236,19 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
   return SUCCESS;
 }
 
+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);
+}
+
+}  // namespace
+
 // Pass in name and checksum because of UTF8 conversion.
 // static
 void SyncerUtil::UpdateServerFieldsFromUpdate(
@@ -261,23 +275,26 @@ void SyncerUtil::UpdateServerFieldsFromUpdate(
       ServerTimeToClientTime(server_entry.ctime()));
   local_entry->Put(SERVER_MTIME,
       ServerTimeToClientTime(server_entry.mtime()));
-  local_entry->Put(SERVER_IS_BOOKMARK_OBJECT, server_entry.has_bookmarkdata());
+  local_entry->Put(SERVER_IS_BOOKMARK_OBJECT,
+      GetSyncDataType(server_entry) == SYNC_TYPE_BOOKMARK);
   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.has_bookmarkdata() && !server_entry.deleted()) {
-    const SyncEntity::BookmarkData& bookmark = server_entry.bookmarkdata();
-    if (bookmark.has_bookmark_url()) {
-      const string& url = bookmark.bookmark_url();
-      local_entry->Put(SERVER_BOOKMARK_URL, url);
-    }
-    if (bookmark.has_bookmark_favicon()) {
-      Blob favicon_blob;
-      SyncerProtoUtil::CopyProtoBytesIntoBlob(bookmark.bookmark_favicon(),
-                                              &favicon_blob);
-      local_entry->Put(SERVER_BOOKMARK_FAVICON, favicon_blob);
+  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);
     }
   }
   if (server_entry.has_position_in_parent()) {
@@ -610,7 +627,8 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency(
   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) ||
-        has_bookmark_data != same_id->Get(SERVER_IS_BOOKMARK_OBJECT)) {
+        (!is_directory &&
+         has_bookmark_data != same_id->Get(SERVER_IS_BOOKMARK_OBJECT))) {
       if (same_id->Get(IS_DEL)) {  // If we've deleted the item, we don't care.
         return VERIFY_SKIP;
       } else {
@@ -639,7 +657,8 @@ 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) ||
-        has_bookmark_data != same_id->Get(IS_BOOKMARK_OBJECT)) {
+        (!is_directory &&
+         has_bookmark_data != same_id->Get(IS_BOOKMARK_OBJECT))) {
       LOG(ERROR) << "Server update doesn't agree with committed item. ";
       LOG(ERROR) << " Entry: " << *same_id;
       LOG(ERROR) << " Update: " << SyncEntityDebugString(entry);
@@ -748,4 +767,19 @@ syncable::Id SyncerUtil::ComputePrevIdFromServerPosition(
   return closest_sibling;
 }
 
+browser_sync::SyncDataType SyncerUtil::GetSyncDataType(
+    const SyncEntity& entry) {
+
+  const bool is_bookmark =
+      (entry.has_specifics() &&
+       entry.specifics().HasExtension(sync_pb::bookmark)) ||
+       entry.has_bookmarkdata();
+
+  if (is_bookmark) {
+    return SYNC_TYPE_BOOKMARK;
+  }
+
+  return SYNC_TYPE_UNKNOWN;
+}
+
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/syncer_util.h b/chrome/browser/sync/engine/syncer_util.h
index 015c0b0..2396706 100755
--- a/chrome/browser/sync/engine/syncer_util.h
+++ b/chrome/browser/sync/engine/syncer_util.h
@@ -146,6 +146,8 @@ class SyncerUtil {
   // if they match.  For an up-to-date item, this should be the case.
   static bool ServerAndLocalOrdersMatch(syncable::Entry* entry);
 
+  static browser_sync::SyncDataType GetSyncDataType(const SyncEntity& entry);
+
  private:
   // Private ctor/dtor since this class shouldn't be instantiated.
   SyncerUtil() {}
diff --git a/chrome/browser/sync/engine/syncproto.h b/chrome/browser/sync/engine/syncproto.h
index 25901dc..cbe56a1 100644
--- a/chrome/browser/sync/engine/syncproto.h
+++ b/chrome/browser/sync/engine/syncproto.h
@@ -46,7 +46,8 @@ class SyncEntity : public IdWrapper<sync_pb::SyncEntity> {
   // directly, because the addition of bookmarks to the protobuf schema
   // makes the check slightly more tricky.
   bool IsFolder() const {
-    return (!has_bookmarkdata() || bookmarkdata().bookmark_folder());
+    return ((has_folder() && folder()) ||
+            (has_bookmarkdata() && bookmarkdata().bookmark_folder()));
   }
 };
 
diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc
index 8a1a508..86cd71c 100644..100755
--- a/chrome/browser/sync/engine/verify_updates_command.cc
+++ b/chrome/browser/sync/engine/verify_updates_command.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
 #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/syncable.h"
 
@@ -58,7 +59,8 @@ VerifyResult VerifyUpdatesCommand::VerifyUpdate(
 
   const bool deleted = entry.has_deleted() && entry.deleted();
   const bool is_directory = entry.IsFolder();
-  const bool is_bookmark = entry.has_bookmarkdata();
+  const bool is_bookmark =
+      SyncerUtil::GetSyncDataType(entry) == SYNC_TYPE_BOOKMARK;
 
   if (!id.ServerKnows()) {
     LOG(ERROR) << "Illegal negative id in received updates";
diff --git a/chrome/browser/sync/protocol/bookmark_specifics.proto b/chrome/browser/sync/protocol/bookmark_specifics.proto
new file mode 100644
index 0000000..976cedc
--- /dev/null
+++ b/chrome/browser/sync/protocol/bookmark_specifics.proto
@@ -0,0 +1,23 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Sync protocol datatype extension for bookmarks.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package sync_pb;
+
+import "sync.proto";
+
+// Properties of bookmark sync objects.
+message BookmarkSpecifics {
+  optional string url = 1;
+  optional bytes favicon = 2;
+}
+
+extend EntitySpecifics {
+  optional BookmarkSpecifics bookmark = 32904;
+}
diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto
index ad7f61f..ffc69a7 100644
--- a/chrome/browser/sync/protocol/sync.proto
+++ b/chrome/browser/sync/protocol/sync.proto
@@ -30,6 +30,32 @@ message ProfilingData {
   optional int64 total_request_time = 6;
 }
 
+message EntitySpecifics {
+  // To add new datatype-specific fields to the protocol, extend
+  // EntitySpecifics.  First, pick a non-colliding tag number by
+  // picking a revision number of one of your past commits
+  // to src.chromium.org.  Then, in a different protocol buffer
+  // definition that includes this, do the following:
+  //
+  //   extend EntitySpecifics {
+  //     MyDatatypeSpecifics my_datatype = 32222;
+  //   }
+  //
+  // where:
+  //   - 32222 is the non-colliding tag number you picked earlier.
+  //   - MyDatatypeSpecifics is the type (probably a message type defined
+  //     in your new .proto file) that you want to associate with each
+  //     object of the new datatype.
+  //   - my_datatype is the field identifier you'll use to access the
+  //     datatype specifics from the code.
+  //
+  // Server implementations are obligated to preserve the contents of
+  // EntitySpecifics when it contains unrecognized extensions.  In this
+  // way, it is possible to add new datatype fields without having
+  // to update the server.
+  extensions 30000 to max;
+}
+
 message SyncEntity {
   // This item's identifier.  In a commit of a new item, this will be a
   // client-generated ID.  If the commit succeeds, the server will generate
@@ -164,6 +190,13 @@ message SyncEntity {
   // committed this entity. Typically a negative integer.
   // Present only in GetUpdatesResponse.
   optional string originator_client_item_id = 20;
+
+  // Extensible container for datatype-specific data.
+  // This became available in version 23 of the protocol.
+  optional EntitySpecifics specifics = 21;
+
+  // Indicate whether this is a folder or not. Available in version 23+.
+  optional bool folder = 22 [default = false];
 };
 
 message CommitMessage {
@@ -181,7 +214,7 @@ message CommitMessage {
     // The human-readable ID identifying the extension responsible
     // for the traffic reported in this ChromiumExtensionsActivity.
     optional string extension_id = 1;
-    
+
     // How many times the extension successfully invoked a write
     // operation through the bookmarks API since the last CommitMessage.
     optional uint32 bookmark_writes_since_last_commit = 2;
@@ -191,7 +224,7 @@ message CommitMessage {
 };
 
 message GetUpdatesCallerInfo {
-  enum GET_UPDATES_SOURCE {
+  enum GetUpdatesSource {
     UNKNOWN = 0;  // The source was not set by the caller.
     FIRST_UPDATE = 1;  // First update from an instance of Chrome.
     LOCAL = 2;  // The source of the update was a local change.
@@ -200,7 +233,7 @@ message GetUpdatesCallerInfo {
     SYNC_CYCLE_CONTINUATION = 5;  // The source of the update was a
   }                               // continuation of a previous update.
 
-  required GET_UPDATES_SOURCE source = 1;
+  required GetUpdatesSource source = 1;
 
   // True only if notifications were enabled for this GetUpdateMessage.
   optional bool notifications_enabled = 2;
@@ -219,14 +252,14 @@ message AuthenticateMessage {
 
 message ClientToServerMessage {
   required string share = 1;
-  optional int32 protocol_version = 2 [default = 22];
-  enum CONTENTS {
+  optional int32 protocol_version = 2 [default = 23];
+  enum Contents {
     COMMIT = 1;
     GET_UPDATES = 2;
     AUTHENTICATE = 3;
   }
 
-  required CONTENTS message_contents = 3;
+  required Contents message_contents = 3;
   optional CommitMessage commit = 4;
   optional GetUpdatesMessage get_updates = 5;
   optional AuthenticateMessage authenticate = 6;
@@ -238,7 +271,7 @@ message ClientToServerMessage {
 };
 
 message CommitResponse {
-  enum RESPONSE_TYPE {
+  enum ResponseType {
     SUCCESS = 1;
     CONFLICT = 2; // You're out of date; update and check your data
     // TODO(ncarter): What's the difference between RETRY and TRANSIENT_ERROR?
@@ -249,7 +282,7 @@ message CommitResponse {
     TRANSIENT_ERROR = 6; // Something went wrong; try again in a bit
   }
   repeated group EntryResponse = 1 {
-    required RESPONSE_TYPE response_type = 2;
+    required ResponseType response_type = 2;
 
     // Sync servers may also return a new ID for an existing item, indicating
     // a new entry's been created to hold the data the client's sending up.
@@ -330,7 +363,7 @@ message ClientToServerResponse {
   optional GetUpdatesResponse get_updates = 2;
   optional AuthenticateResponse authenticate = 3;
 
-  enum ERROR_TYPE {
+  enum ErrorType {
     SUCCESS            = 0;
     ACCESS_DENIED      = 1; // Returned when the user doesn't have access to
                             // store (instead of HTTP 401).
@@ -343,7 +376,7 @@ message ClientToServerResponse {
                             // Google Account.
     AUTH_INVALID       = 6; // Auth token or cookie is otherwise invalid.
   }
-  optional ERROR_TYPE error_code = 4 [default = SUCCESS];
+  optional ErrorType error_code = 4 [default = SUCCESS];
   optional string error_message = 5;
 
   // Opaque store ID; if it changes, the contents of the client's cache
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index 963e0e0..296c532 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -35,9 +35,9 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
       status_controller_.did_commit_items());
 }
 
-sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE
+sync_pb::GetUpdatesCallerInfo::GetUpdatesSource
     SyncSession::TestAndSetSource() {
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE old_source = source_;
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource old_source = source_;
   set_source(sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION);
   return old_source;
 }
diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h
index 2d6d1dd5..4f44596 100644
--- a/chrome/browser/sync/sessions/sync_session.h
+++ b/chrome/browser/sync/sessions/sync_session.h
@@ -94,8 +94,8 @@ class SyncSession {
   // Volatile reader for the source member of the sync session object.  The
   // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has
   // been read.
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE TestAndSetSource();
-  void set_source(sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source) {
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource TestAndSetSource();
+  void set_source(sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source) {
     source_ = source;
   }
 
@@ -109,7 +109,7 @@ class SyncSession {
   SyncSessionContext* const context_;
 
   // The source for initiating this sync session.
-  sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source_;
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source_;
 
   // Information about extensions activity since the last successful commit.
   ExtensionsActivityMonitor::Records extensions_activity_;
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 3e280a9..286c16e 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -608,23 +608,34 @@
       # Protobuf compiler / generate rule for sync.proto
       'target_name': 'sync_proto',
       'type': 'none',
-      'actions': [
+      'sources': [
+        'browser/sync/protocol/sync.proto',
+        'browser/sync/protocol/bookmark_specifics.proto',
+      ],
+      'rules': [
         {
-          'action_name': 'compiling sync.proto',
+          'rule_name': 'genproto',
+          'extension': 'proto',
           'inputs': [
             '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
-            'browser/sync/protocol/sync.proto',
           ],
+          'variables': {
+            # The protoc compiler requires a proto_path argument with the
+            # directory containing the .proto file.
+            # There's no generator variable that corresponds to this, so fake it.
+            'rule_input_relpath': 'browser/sync/protocol',
+          },
           'outputs': [
-            '<(protoc_out_dir)/chrome/browser/sync/protocol/sync.pb.cc',
-            '<(protoc_out_dir)/chrome/browser/sync/protocol/sync.pb.h',
+            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.h',
+            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.cc',
           ],
           'action': [
             '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
-            '--proto_path=browser/sync/protocol',
-            'browser/sync/protocol/sync.proto',
-            '--cpp_out=<(protoc_out_dir)/chrome/browser/sync/protocol',
+            '--proto_path=./<(rule_input_relpath)',
+            './<(rule_input_relpath)/<(RULE_INPUT_ROOT)<(RULE_INPUT_EXT)',
+            '--cpp_out=<(protoc_out_dir)/chrome/<(rule_input_relpath)',
           ],
+          'message': 'Generating C++ code from <(RULE_INPUT_PATH)',
         },
       ],
       'dependencies': [
@@ -747,6 +758,8 @@
       'sources': [
         '<(protoc_out_dir)/chrome/browser/sync/protocol/sync.pb.cc',
         '<(protoc_out_dir)/chrome/browser/sync/protocol/sync.pb.h',
+        '<(protoc_out_dir)/chrome/browser/sync/protocol/bookmark_specifics.pb.cc',
+        '<(protoc_out_dir)/chrome/browser/sync/protocol/bookmark_specifics.pb.h',
         'browser/sync/engine/all_status.cc',
         'browser/sync/engine/all_status.h',
         'browser/sync/engine/apply_updates_command.cc',