sync: Implement per-type commit interface

sync/engine/commit.h

Index: sync/engine/commit.h
diff --git a/sync/engine/commit.h b/sync/engine/commit.h
index 168d950c276a1242695cb70f476dcc0845c2672b..629509c28b384f3880f9680f0db38f285c665891 100644
--- a/sync/engine/commit.h
+++ b/sync/engine/commit.h
@@ -5,33 +5,67 @@
 #ifndef SYNC_ENGINE_COMMIT_H_
 #define SYNC_ENGINE_COMMIT_H_
 
+#include <map>
+
+#include "base/stl_util.h"
+#include "sync/base/sync_export.h"
+#include "sync/engine/sync_directory_commit_contributor.h"
 #include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/util/syncer_error.h"
+#include "sync/protocol/sync.pb.h"
+#include "sync/util/extensions_activity.h"
 
 namespace syncer {
 
 namespace sessions {
+class StatusController;
 class SyncSession;
 }
 
+class SyncDirectoryCommitContribution;
 class Syncer;
 
-// This function will commit batches of unsynced items to the server until the
-// number of unsynced and ready to commit items reaches zero or an error is
-// encountered.  A request to exit early will be treated as an error and will
-// abort any blocking operations.
-//
-// The Syncer parameter is provided only for access to its ExitRequested()
-// method.  This is technically unnecessary since an early exit request should
-// be detected as we attempt to contact the sync server.
+// This class wraps the actions related to building and executing a single
+// commit operation.
 //
-// The SyncSession parameter contains pointers to various bits of state,
-// including the syncable::Directory that contains all sync items and the
-// ServerConnectionManager used to contact the server.
-SyncerError BuildAndPostCommits(
-    ModelTypeSet request_types,
-    Syncer* syncer,
-    sessions::SyncSession* session);
+// This class' most important responsibility is to manage the ContributionsMap.
+// This class serves as a container for those objects.  Although it would have
+// been acceptable to let this class be a dumb container object, it turns out
+// that there was no other convenient place to put the Init() and
+// PostAndProcessCommitResponse() functions.  So they ended up here.
+class SYNC_EXPORT_PRIVATE Commit {
+ public:
+  static Commit* Init(

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

I think technically the constructor/destructor should be first in the
declaration order

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

Wasn't there also some rule about static members coming first?  The style
guideline appears to be offline at the moment so I can't check.

[Nicolas Zea, 2013/10/14 21:45:35]

That's for static const values, not methods. All methods (static or not) should
be below constructor/destructor declarations.

[rlarocque, 2013/10/14 23:23:28]

OK.  Fixed.

+      ModelTypeSet requested_types,
+      size_t max_entries,
+      const std::string& account_name,
+      const std::string& cache_guid,
+      CommitContributorMap* contributor_map,
+      ExtensionsActivity* extensions_activity);
+
+  explicit Commit(

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

remove explicit

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

Oops, that regressed when I was moving this code around.  Thanks for catching it
again.

+      const std::map<ModelType, SyncDirectoryCommitContribution*>&
+          contributions,
+      const sync_pb::ClientToServerMessage& message,
+      ExtensionsActivity::Records extensions_activity_buffer);
+  ~Commit();
+
+  SyncerError PostAndProcessResponse(
+      sessions::SyncSession* session,
+      sessions::StatusController* status,
+      ExtensionsActivity* extensions_activity);
+
+ private:
+  typedef std::map<ModelType, SyncDirectoryCommitContribution*> ContributionMap;
+
+  ContributionMap contributions_;
+  STLValueDeleter<ContributionMap> deleter_;
+
+  sync_pb::ClientToServerMessage message_;
+  sync_pb::ClientToServerResponse response_;
+  ExtensionsActivity::Records extensions_activity_buffer_;
+};
 
 }  // namespace syncer