Make SyncBackendRegistrar aware of loaded data

sync/internal_api/sync_manager_impl.cc

Index: sync/internal_api/sync_manager_impl.cc
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 107ed606e743b2bdbd91d550541e84f238a5b35f..aff087c4da742a0e95c5ec8d23a458dbc6cbeb6a 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -177,7 +177,6 @@ class SyncManagerImpl::SyncInternal
             bool use_ssl,
             const scoped_refptr<base::TaskRunner>& blocking_task_runner,
             scoped_ptr<HttpPostProviderFactory> post_factory,
-            const syncer::ModelSafeRoutingInfo& model_safe_routing_info,
             const std::vector<syncer::ModelSafeWorker*>& workers,
             syncer::ExtensionsActivityMonitor*
                 extensions_activity_monitor,
@@ -710,7 +709,6 @@ bool SyncManagerImpl::Init(
     bool use_ssl,
     const scoped_refptr<base::TaskRunner>& blocking_task_runner,
     scoped_ptr<HttpPostProviderFactory> post_factory,
-    const syncer::ModelSafeRoutingInfo& model_safe_routing_info,
     const std::vector<syncer::ModelSafeWorker*>& workers,
     syncer::ExtensionsActivityMonitor* extensions_activity_monitor,
     SyncManager::ChangeDelegate* change_delegate,
@@ -732,7 +730,6 @@ bool SyncManagerImpl::Init(
                      use_ssl,
                      blocking_task_runner,
                      post_factory.Pass(),
-                     model_safe_routing_info,
                      workers,
                      extensions_activity_monitor,
                      change_delegate,
@@ -862,7 +859,6 @@ bool SyncManagerImpl::SyncInternal::Init(
     bool use_ssl,
     const scoped_refptr<base::TaskRunner>& blocking_task_runner,
     scoped_ptr<HttpPostProviderFactory> post_factory,
-    const syncer::ModelSafeRoutingInfo& model_safe_routing_info,
     const std::vector<syncer::ModelSafeWorker*>& workers,
     syncer::ExtensionsActivityMonitor* extensions_activity_monitor,
     SyncManager::ChangeDelegate* change_delegate,
@@ -927,6 +923,8 @@ bool SyncManagerImpl::SyncInternal::Init(
 
   connection_manager()->AddListener(this);
 
+  bool success = SignIn(credentials);

[tim (not reviewing), 2012/07/20 21:00:57]

It makes me a bit nervous to start doing "real work" like SignIn before we've
created a SyncSessionContext / SyncScheduler.  I'm reminded of crashes we've had
in the past with notifications being sent (for dir open) and listeners expecting
things to be around that aren't.

[rlarocque, 2012/07/20 21:17:59]

Good point.

Could we get rid of the SignIn() function now that we're refactoring out the
"for test" code?  I'd like to move the directory opening part of it to here,
then leave UpdateCredentials() until later.  

+
   // Test mode does not use a syncer context or syncer thread.
   if (testing_mode_ == NON_TEST) {
     // Build a SyncSessionContext and store the worker in it.
@@ -937,7 +935,6 @@ bool SyncManagerImpl::SyncInternal::Init(
     session_context_.reset(new SyncSessionContext(
         connection_manager_.get(),
         directory(),
-        model_safe_routing_info,

[Nicolas Zea, 2012/07/20 21:35:39]

This concerns me a bit. Could you set the routing info manually right before
notifying OnInitializationComplete?

[rlarocque, 2012/07/23 20:50:30]

This shouldn't have any practical impact because this will eventually be set by
the post-OnBackendInit() configure job, right?

Whether or not this makes sense depends on what the purpose of the context's
routing info is supposed to be.  

I'd prefer to think of the context's routing info as the set of types that the
SyncManager has been asked to sync.  I don't think initializing it based on the
initial_sync_ended types makes sense in all cases.  

[Nicolas Zea, 2012/07/23 23:54:56]

Hmm, it'll in fact get set by the initial Nigori-configure job. Yeah, I guess
that's fine, although I'm a bit concerned about something in the scheduler
trying to use it in some way before then. But theoretically the scheduler
shouldn't be doing anything until it gets configured I think, so this is
probably fine.

         workers,
         extensions_activity_monitor,
         &throttled_data_type_tracker_,
@@ -948,8 +945,6 @@ bool SyncManagerImpl::SyncInternal::Init(
     scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));
   }
 
-  bool success = SignIn(credentials);
-
   if (success) {
     if (scheduler()) {
       scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);
@@ -988,7 +983,8 @@ bool SyncManagerImpl::SyncInternal::Init(
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
                         MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),
-                        success));
+                        success, success ? InitialSyncEndedTypes()
+                                         : syncer::ModelTypeSet()));
 
   if (!success && testing_mode_ == NON_TEST)
     return false;