sync: Implement per-type commit interface

sync/engine/build_commit_command.h

Index: sync/engine/build_commit_command.h
diff --git a/sync/engine/build_commit_command.h b/sync/engine/build_commit_command.h
index a47c62afe0c97bbb7e7da1095563be010998697c..bab819fae30bcdb9d81ac94a75b1b32fa79bbacb 100644
--- a/sync/engine/build_commit_command.h
+++ b/sync/engine/build_commit_command.h
@@ -10,15 +10,12 @@
 #include "base/gtest_prod_util.h"
 #include "sync/base/sync_export.h"
 #include "sync/engine/syncer_command.h"
+#include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/syncable/entry_kernel.h"
 #include "sync/util/extensions_activity.h"
 
 namespace syncer {
 
-namespace sessions {
-class OrderedCommitSet;
-}
-
 namespace syncable {
 class Entry;
 class BaseTransaction;
@@ -33,48 +30,24 @@ class BaseTransaction;
 // See SyncerCommand documentation for more info.
 class SYNC_EXPORT_PRIVATE BuildCommitCommand : public SyncerCommand {

[Nicolas Zea, 2013/10/09 00:17:39]

Why not just make these all standalone functions at this point? (possibly as
helper functions within Commit.cc)

[rlarocque, 2013/10/09 20:00:19]

For now, I was trying to keep the diffs readable by not moving code between
files if I can avoid it.  I'd like to write a follow-up CL to clean up this mess
after this Cl has landed.

If you'd like, I could easily make these functions standalone and leave them in
this file.

[Nicolas Zea, 2013/10/10 21:34:45]

I think this patch is already significant enough, and the diffs are already
confused enough due to moving logic around, that it's too late to keep the diff
clean. I'd rather have the final patch itself result in clean self-contained
code. For example, the comment above no longer applies.

Perhaps just rename this file commit_util or something, and put the methods in a
commit_util namespace? Having them within a "command" is confusing I find.

[rlarocque, 2013/10/11 23:03:30]

Fair enough.  I moved everything into the files 'commit_util.cc/h' and namespace
syncer::commit_util.

  public:
-  // The batch_commit_set parameter contains a set of references to the items
-  // that should be committed.
-  //
-  // The commit_message parameter is an output parameter which will contain the
-  // fully initialized commit message once ExecuteImpl() has been called.
-  BuildCommitCommand(
-      syncable::BaseTransaction* trans,
-      const sessions::OrderedCommitSet& batch_commit_set,
-      sync_pb::ClientToServerMessage* commit_message,
-      ExtensionsActivity::Records* extensions_activity_buffer);
-  virtual ~BuildCommitCommand();
-
-  // SyncerCommand implementation.
-  virtual SyncerError ExecuteImpl(sessions::SyncSession* session) OVERRIDE;
-
   // Helper function that takes a snapshot of |meta_entry| and puts it into a
   // protobuf suitable for use in a commit request message.
   static void BuildCommitItem(const syncable::Entry& meta_entry,
                               sync_pb::SyncEntity* sync_entry);
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(BuildCommitCommandTest, InterpolatePosition);
-
-  void AddExtensionsActivityToMessage(sessions::SyncSession* session,
-                                      sync_pb::CommitMessage* message);
+  // Adds bookmark extensions activity report to |message|.
+  static void AddExtensionsActivityToMessage(
+      ExtensionsActivity* activity,
+      ExtensionsActivity::Records* extensions_activity_buffer,
+      sync_pb::CommitMessage* message);
 
   // Fills the config_params field of |message|.
-  void AddClientConfigParamsToMessage(sessions::SyncSession* session,
-                                      sync_pb::CommitMessage* message);
+  static void AddClientConfigParamsToMessage(
+      ModelTypeSet enabled_types,
+      sync_pb::CommitMessage* message);
 
-  DISALLOW_COPY_AND_ASSIGN(BuildCommitCommand);
-
-  // A pointer to a valid transaction not owned by this class.
-  syncable::BaseTransaction* trans_;
-
-  // Input parameter; see constructor comment.
-  const sessions::OrderedCommitSet& batch_commit_set_;
-
-  // Output parameter; see constructor comment.
-  sync_pb::ClientToServerMessage* commit_message_;
-
-  ExtensionsActivity::Records* extensions_activity_buffer_;
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(BuildCommitCommand);
 };
 
 }  // namespace syncer