sync: Implement per-type commit interface

sync/engine/build_commit_command.cc

Index: sync/engine/build_commit_command.cc
diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc
index 65f1cdfb9cf7616692547406ae3f13a5a07579a6..78249fdf74dfa54e60bf5da67407c5db5c09b26f 100644
--- a/sync/engine/build_commit_command.cc
+++ b/sync/engine/build_commit_command.cc
@@ -14,7 +14,6 @@
 #include "sync/internal_api/public/base/unique_position.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/sync.pb.h"
-#include "sync/sessions/ordered_commit_set.h"
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
@@ -38,63 +37,44 @@ using syncable::Id;
 using syncable::SPECIFICS;
 using syncable::UNIQUE_POSITION;
 
-BuildCommitCommand::BuildCommitCommand(
-    syncable::BaseTransaction* trans,
-    const sessions::OrderedCommitSet& batch_commit_set,
-    sync_pb::ClientToServerMessage* commit_message,
-    ExtensionsActivity::Records* extensions_activity_buffer)
-  : trans_(trans),
-    batch_commit_set_(batch_commit_set),
-    commit_message_(commit_message),
-    extensions_activity_buffer_(extensions_activity_buffer) {
-}
-
-BuildCommitCommand::~BuildCommitCommand() {}
-
 void BuildCommitCommand::AddExtensionsActivityToMessage(
-    SyncSession* session, sync_pb::CommitMessage* message) {
-  // We only send ExtensionsActivity to the server if bookmarks are being
-  // committed.
-  ExtensionsActivity* activity = session->context()->extensions_activity();
-  if (batch_commit_set_.HasBookmarkCommitId()) {
-    // This isn't perfect, since the set of extensions activity may not
-    // correlate exactly with the items being committed.  That's OK as
-    // long as we're looking for a rough estimate of extensions activity,
-    // not an precise mapping of which commits were triggered by which
-    // extension.
-    //
-    // We will push this list of extensions activity back into the
-    // ExtensionsActivityMonitor if this commit fails.  That's why we must keep
-    // a copy of these records in the session.
-    activity->GetAndClearRecords(extensions_activity_buffer_);
-
-    const ExtensionsActivity::Records& records =
-        *extensions_activity_buffer_;
-    for (ExtensionsActivity::Records::const_iterator it =
-         records.begin();
-         it != records.end(); ++it) {
-      sync_pb::ChromiumExtensionsActivity* activity_message =
-          message->add_extensions_activity();
-      activity_message->set_extension_id(it->second.extension_id);
-      activity_message->set_bookmark_writes_since_last_commit(
-          it->second.bookmark_write_count);
-    }
+    ExtensionsActivity* activity,
+    ExtensionsActivity::Records* extensions_activity_buffer,
+    sync_pb::CommitMessage* message) {
+  // This isn't perfect, since the set of extensions activity may not correlate
+  // exactly with the items being committed.  That's OK as long as we're looking
+  // for a rough estimate of extensions activity, not an precise mapping of
+  // which commits were triggered by which extension.
+  //
+  // We will push this list of extensions activity back into the
+  // ExtensionsActivityMonitor if this commit fails.  That's why we must keep a
+  // copy of these records in the session.
+  activity->GetAndClearRecords(extensions_activity_buffer);
+
+  const ExtensionsActivity::Records& records = *extensions_activity_buffer;
+  for (ExtensionsActivity::Records::const_iterator it =
+       records.begin();
+       it != records.end(); ++it) {
+    sync_pb::ChromiumExtensionsActivity* activity_message =
+        message->add_extensions_activity();
+    activity_message->set_extension_id(it->second.extension_id);
+    activity_message->set_bookmark_writes_since_last_commit(
+        it->second.bookmark_write_count);
   }
 }
 
 void BuildCommitCommand::AddClientConfigParamsToMessage(
-    SyncSession* session, sync_pb::CommitMessage* message) {
-  const ModelSafeRoutingInfo& routing_info = session->context()->routing_info();
+    ModelTypeSet enabled_types,
+    sync_pb::CommitMessage* message) {
   sync_pb::ClientConfigParams* config_params = message->mutable_config_params();
-  for (std::map<ModelType, ModelSafeGroup>::const_iterator iter =
-          routing_info.begin(); iter != routing_info.end(); ++iter) {
-    if (ProxyTypes().Has(iter->first))
+  for (ModelTypeSet::Iterator it = enabled_types.First(); it.Good(); it.Inc()) {
+    if (ProxyTypes().Has(it.Get()))
       continue;
-    int field_number = GetSpecificsFieldNumberFromModelType(iter->first);
+    int field_number = GetSpecificsFieldNumberFromModelType(it.Get());
     config_params->mutable_enabled_type_ids()->Add(field_number);
   }
   config_params->set_tabs_datatype_enabled(
-      routing_info.count(syncer::PROXY_TABS) > 0);
+      enabled_types.Has(syncer::PROXY_TABS));
 }
 
 namespace {
@@ -109,34 +89,6 @@ void SetEntrySpecifics(const Entry& meta_entry,
 }
 }  // namespace
 
-SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
-  commit_message_->set_share(session->context()->account_name());
-  commit_message_->set_message_contents(sync_pb::ClientToServerMessage::COMMIT);
-
-  sync_pb::CommitMessage* commit_message = commit_message_->mutable_commit();
-  commit_message->set_cache_guid(trans_->directory()->cache_guid());
-  AddExtensionsActivityToMessage(session, commit_message);
-  AddClientConfigParamsToMessage(session, commit_message);
-
-  for (size_t i = 0; i < batch_commit_set_.Size(); i++) {
-    int64 handle = batch_commit_set_.GetCommitHandleAt(i);
-    sync_pb::SyncEntity* sync_entry = commit_message->add_entries();
-
-    Entry meta_entry(trans_, syncable::GET_BY_HANDLE, handle);
-    CHECK(meta_entry.good());
-
-    DCHECK_NE(0UL,
-              session->context()->routing_info().count(
-                  meta_entry.GetModelType()))
-        << "Committing change to datatype that's not actively enabled.";
-
-    BuildCommitItem(meta_entry, sync_entry);
-  }
-
-
-  return SYNCER_OK;
-}
-
 // static.
 void BuildCommitCommand::BuildCommitItem(
     const syncable::Entry& meta_entry,