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/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;
+
+typedef std::map<ModelType, SyncDirectoryCommitContribution*> ContributionMap;

[Nicolas Zea, 2013/12/18 22:36:10]

Do no other places declare this typedef? Does it need to be within the syncable
namespace or can it be scoped to CommitterList to avoid conflicts?

[rlarocque, 2014/01/03 01:46:41]

It could be moved into CommitterList, but I think it would have to be public,
since it is referenced in the definition of the public function
GatherCommitContributions.

It's also re-declared privately in commit.h.

We can either choose to keep this global and remove the redeclaration in
commit.h, or we can make it private in both instances.  I'm leaning towards
making them both private, since that will allow commit.h to not include
committer_list.h.

+
+// This class manages the set of per-type committer objects.
+//
+// It owns these types and hides the details of iterating over all of them.
+// 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 {
+ public:
+  CommitterList();
+  ~CommitterList();
+
+  // Builds or rebuilds the set of syncable::Directory sync type objects.
+  void SetEnabledSyncDirectoryTypes(

[Nicolas Zea, 2013/12/18 22:36:10]

Why not just SetEnabledTypes? In the bismarck world, will this class not be able
to figure out whether a type is a bismarck  type or directory type, and act
accordingly?

Also, given that you'll be calling this on every configuration, and
ModelSafeWorkers and the directory don't actually change, perhaps they can be
passed at construction time?

[rlarocque, 2014/01/03 01:46:41]

I'm thinking it won't have access to that information.  I'd prefer to leave the
choice of what kind of sync to use for which type up to
ProfileSyncComponentsFactoryImpl::RegisterDataTypes() so we can toggle it with
command line flags.

I suppose we could pass along some of the list of Bismarck types down to this
level, but I think it would be easier to track it at the ProfileSyncService
level.
Yep.  The latest upload will include something like that.

+      const std::vector<scoped_refptr<ModelSafeWorker> > workers,

[Nicolas Zea, 2013/12/18 22:36:10]

const ref

[rlarocque, 2014/01/03 01:46:41]

Removed in the most recent patch.

+      const ModelSafeRoutingInfo& routing_info,
+      syncable::Directory* dir);
+
+  // Gathers a set of contributions to be used to populate a commit message.
+  void GatherCommitContributions(
+      ModelTypeSet commit_types,
+      size_t max_entries,
+      ContributionMap* contributions);
+
+  // Registration a commit contributor directly.  Useful in tests.

[Nicolas Zea, 2013/12/18 22:36:10]

Registration -> Register

[rlarocque, 2014/01/03 01:46:41]

Done.

+  void RegisterTypeForTest(
+      ModelType type,
+      SyncDirectoryCommitContributor* contributor);
+
+ 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_;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_ENGINE_COMMITTER_LIST_H_