Make SyncBackendRegistrar aware of loaded data

sync/internal_api/sync_manager_impl.cc

 // Copyright (c) 2012 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.
 
 #include "sync/internal_api/sync_manager_impl.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 159 matching lines @@
     CHECK(!initialized_);
   }
 
   bool Init(const FilePath& database_location,
             const WeakHandle<JsEventHandler>& event_handler,
             const std::string& sync_server_and_path,
             int port,
             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,
             const SyncCredentials& credentials,
             scoped_ptr<syncer::SyncNotifier> sync_notifier,
             const std::string& restored_key_for_bootstrapping,
             SyncManager::TestingMode testing_mode,
             Encryptor* encryptor,
             UnrecoverableErrorHandler* unrecoverable_error_handler,
@@ skipping 512 matching lines @@
     : data_(new SyncInternal(name)) {}
 
 bool SyncManagerImpl::Init(
     const FilePath& database_location,
     const WeakHandle<JsEventHandler>& event_handler,
     const std::string& sync_server_and_path,
     int sync_server_port,
     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,
     const SyncCredentials& credentials,
     scoped_ptr<syncer::SyncNotifier> sync_notifier,
     const std::string& restored_key_for_bootstrapping,
     SyncManager::TestingMode testing_mode,
     Encryptor* encryptor,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(post_factory.get());
   DVLOG(1) << "SyncManager starting Init...";
   std::string server_string(sync_server_and_path);
   return data_->Init(database_location,
                      event_handler,
                      server_string,
                      sync_server_port,
                      use_ssl,
                      blocking_task_runner,
                      post_factory.Pass(),
-                     model_safe_routing_info,
                      workers,
                      extensions_activity_monitor,
                      change_delegate,
                      credentials,
                      sync_notifier.Pass(),
                      restored_key_for_bootstrapping,
                      testing_mode,
                      encryptor,
                      unrecoverable_error_handler,
                      report_unrecoverable_error_function);
@@ skipping 109 matching lines @@
 }
 
 bool SyncManagerImpl::SyncInternal::Init(
     const FilePath& database_location,
     const WeakHandle<JsEventHandler>& event_handler,
     const std::string& sync_server_and_path,
     int port,
     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,
     const SyncCredentials& credentials,
     scoped_ptr<syncer::SyncNotifier> sync_notifier,
     const std::string& restored_key_for_bootstrapping,
     SyncManager::TestingMode testing_mode,
     Encryptor* encryptor,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {
@@ skipping 44 matching lines @@
                               backing_store));
 
   connection_manager_.reset(new SyncAPIServerConnectionManager(
       sync_server_and_path, port, use_ssl, post_factory.release()));
 
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
   observing_ip_address_changes_ = true;
 
   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.
     DVLOG(1) << "Sync is bringing up SyncSessionContext.";
     std::vector<SyncEngineEventListener*> listeners;
     listeners.push_back(&allstatus_);
     listeners.push_back(this);
     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_,
         listeners,
         &debug_info_event_listener_,
         &traffic_recorder_));
     session_context()->set_account_name(credentials.email);
     scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));
   }
 
-  bool success = SignIn(credentials);
-
   if (success) {
     if (scheduler()) {
       scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);
     }
 
     initialized_ = true;
 
     // Unapplied datatypes (those that do not have initial sync ended set) get
     // re-downloaded during any configuration. But, it's possible for a datatype
     // to have a progress marker but not have initial sync ended yet, making
@@ skipping 18 matching lines @@
   }
 
   // Notify that initialization is complete. Note: This should be the last to
   // execute if |signed_in| is false. Reason being in that case we would
   // post a task to shutdown sync. But if this function posts any other tasks
   // on the UI thread and if shutdown wins then that tasks would execute on
   // a freed pointer. This is because UI thread is not shut down.
   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;
 
   sync_notifier_->AddObserver(this);
 
   return success;
 }
 
 void SyncManagerImpl::SyncInternal::UpdateCryptographerAndNigori(
@@ skipping 1491 matching lines @@
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer