sync: Introduce ModelTypeRegistry and helpers

sync/sessions/sync_session_context.h

Index: sync/sessions/sync_session_context.h
diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h
index 5995ab151dbcbba209872310ce532fcfa3030da7..82a0d20de478ee071e75ada9ec3bbe2aa313f61f 100644
--- a/sync/sessions/sync_session_context.h
+++ b/sync/sessions/sync_session_context.h
@@ -15,24 +15,18 @@
 #ifndef SYNC_SESSIONS_SYNC_SESSION_CONTEXT_H_
 #define SYNC_SESSIONS_SYNC_SESSION_CONTEXT_H_
 
-#include <map>
 #include <string>
-#include <vector>
 
-#include "base/stl_util.h"
 #include "sync/base/sync_export.h"
-#include "sync/engine/sync_directory_commit_contributor.h"
-#include "sync/engine/sync_directory_update_handler.h"
+#include "sync/engine/model_type_registry.h"
 #include "sync/engine/sync_engine_event.h"
-#include "sync/engine/syncer_types.h"
 #include "sync/engine/traffic_recorder.h"
-#include "sync/internal_api/public/engine/model_safe_worker.h"
-#include "sync/protocol/sync.pb.h"
 #include "sync/sessions/debug_info_getter.h"
 
 namespace syncer {
 
 class ExtensionsActivity;
+class ModelTypeRegistry;
 class ServerConnectionManager;
 
 namespace syncable {
@@ -49,11 +43,11 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
  public:
   SyncSessionContext(ServerConnectionManager* connection_manager,
                      syncable::Directory* directory,
-                     const std::vector<ModelSafeWorker*>& workers,
                      ExtensionsActivity* extensions_activity,
                      const std::vector<SyncEngineEventListener*>& listeners,
                      DebugInfoGetter* debug_info_getter,
                      TrafficRecorder* traffic_recorder,
+                     ModelTypeRegistry* model_type_registry,
                      bool keystore_encryption_enabled,
                      bool client_enabled_pre_commit_update_avoidance,
                      const std::string& invalidator_client_id);
@@ -71,15 +65,10 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
     return enabled_types_;
   }
 
-  void set_routing_info(const ModelSafeRoutingInfo& routing_info);
+  void SetRoutingInfo(const ModelSafeRoutingInfo& routing_info);
 
-  UpdateHandlerMap* update_handler_map() {
-    return &update_handler_map_;
-  }
-
-  CommitContributorMap* commit_contributor_map() {
-    return &commit_contributor_map_;
-  }
+  UpdaterList* GetUpdaterList();
+  CommitterList* GetCommitterList();
 
   ExtensionsActivity* extensions_activity() {
     return extensions_activity_.get();
@@ -156,25 +145,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
   // set_routing_info().
   ModelTypeSet enabled_types_;
 
-  // A map of 'update handlers', one for each enabled type.
-  // This must be kept in sync with the routing info.  Our temporary solution to
-  // that problem is to initialize this map in set_routing_info().
-  UpdateHandlerMap update_handler_map_;
-
-  // Deleter for the |update_handler_map_|.
-  STLValueDeleter<UpdateHandlerMap> update_handler_deleter_;
-
-  // A map of 'commit contributors', one for each enabled type.
-  // This must be kept in sync with the routing info.  Our temporary solution to
-  // that problem is to initialize this map in set_routing_info().
-  CommitContributorMap commit_contributor_map_;
-
-  // Deleter for the |commit_contributor_map_|.
-  STLValueDeleter<CommitContributorMap> commit_contributor_deleter_;
-
-  // The set of ModelSafeWorkers.  Used to execute tasks of various threads.
-  std::map<ModelSafeGroup, scoped_refptr<ModelSafeWorker> > workers_;
-
   // We use this to stuff extensions activity into CommitMessages so the server
   // can correlate commit traffic with extension-related bookmark mutations.
   scoped_refptr<ExtensionsActivity> extensions_activity_;
@@ -195,6 +165,11 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
 
   TrafficRecorder* traffic_recorder_;
 
+  ModelTypeRegistry* model_type_registry_;
+
+  scoped_ptr<CommitterList> committer_list_;
+  scoped_ptr<UpdaterList> updater_list_;

[tim (not reviewing), 2014/01/14 20:26:46]

In the sessions/engine approach this patch is shooting for, UpdaterList
(GetUpdatesProcessor) is an engine concept, and SyncSessionContext isn't the
right place for it to live.

Syncer is probably the right place. If possible, better than having the Syncer
own an UpdaterList (which still feels like a longer-than-necessary-lifetime?)
would be for the DownloadAndApplyUpdates function to take an UpdaterList
(GetUpdatesProcessor) that was newly constructed on the stack by the Syncer,
using either a const ModelTypeRegistry that was passed in to NormalSyncShare
(wlog) or the SSContext.

Having NormalSyncShare (wlog) build the UpdaterList and eventually *not*
perpetuate the Session or Context down the chain from there moves us towards
reducing SyncSessionContext dependencies deep in syncer code by passing in just
the parts we need at the start of a sync share operation.

(I still feel that CommitterList and UpdaterList are different here because
"UpdaterList" is concerned with building actual GU messages directly. That said
I'm not strongly opposed to moving CommitterList as well; it seems like it would
make more sense from a lifetime perspective because it's tied to a *SyncShare
operation.)

[rlarocque, 2014/01/15 01:16:47]

Done, I think.

+
   // Satus information to be sent up to the server.
   sync_pb::ClientStatus client_status_;