sync: Introduce ModelTypeRegistry and helpers

sync/engine/committer_list.h

+// Copyright (c) 2013 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.
+
+#ifndef SYNC_ENGINE_COMMITTER_LIST_H_
+#define SYNC_ENGINE_COMMITTER_LIST_H_
+
+#include <map>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/stl_util.h"
+#include "sync/base/sync_export.h"
+#include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/engine/model_safe_worker.h"
+
+namespace syncer {
+
+namespace syncable {
+class Directory;
+}  // namespace syncable
+
+class SyncDirectoryCommitContributor;
+class SyncDirectoryCommitContribution;
+
+// This class manages the set of per-type committer objects.
+//
+// It owns these types and hides the details of iterating over all of them.

[tim (not reviewing), 2014/01/08 19:33:42]

Stuff responsible for shielding engine/ code from the full Directory and adding
knowledge of and scoping required for sync sessions like this (a good cause!)
today lives in sessions/.  The intent was to make it clear that engine code
should almost* always use sessions:: layer constructs instead of interacting
with the Directory directly. 

So I think this class, and (tangential to this review) things like
SyncDirectoryUpdateHandler and SyncDirectoryCommitContributor should shuffle
over to sessions/ (and they also shouldn't have Sync* in their name.  A follow
up patch to move/rename is fine, sorry I didn't see this (or review?) the
initial patch!).  We can discuss alternatives if there is strong protest to
continue having a sessions/ concept like that, but for now that's the way the
code is structured.

* at least in every case where we're iterating over or stashing entries from the
Directory for consumption or modification by engine/ code. A deps rule
forbidding includes is probably a pipe dream at this point although would be
nice when learning the code.)

[rlarocque, 2014/01/09 00:31:18]

Can we defer the move to a future CL?  That way we can move them all at once. 
And probably rename do the s/SyncDirectory/Directory/ change while we're at it.

+// Many methods allow the caller to specify a subset of types on which the
+// operation is to be applied.  It is a logic error if the supplied set of types
+// contains a type which was not previously registered.
+class SYNC_EXPORT_PRIVATE CommitterList {

[tim (not reviewing), 2014/01/08 19:33:42]

(if you've read my UpdaterList comment, CommitterList seems like a much more
fitting name in this case, given the operations it exposes.)

+ public:
+  typedef std::map<ModelType, SyncDirectoryCommitContribution*> ContributionMap;
+
+  CommitterList();
+  ~CommitterList();
+
+  // Registers a commit contributor directly.
+  void RegisterType(
+      ModelType type,
+      SyncDirectoryCommitContributor* contributor);
+
+  // Gathers a set of contributions to be used to populate a commit message.
+  void GatherCommitContributions(
+      ModelTypeSet commit_types,
+      size_t max_entries,
+      ContributionMap* contributions);
+
+ private:
+  typedef std::map<ModelType, SyncDirectoryCommitContributor*>
+      CommitContributorMap;
+
+  // A map of 'commit contributors', one for each enabled type.
+  CommitContributorMap commit_contributor_map_;
+
+  // Deleter for the |commit_contributor_map_|.
+  STLValueDeleter<CommitContributorMap> commit_contributor_deleter_;
+
+  DISALLOW_COPY_AND_ASSIGN(CommitterList);
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_ENGINE_COMMITTER_LIST_H_