[Sync] Refactor sync configuration logic.

sync/internal_api/sync_manager.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.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 40 matching lines @@
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/directory_change_delegate.h"
 #include "sync/syncable/syncable.h"
 #include "sync/util/cryptographer.h"
 #include "sync/util/experiments.h"
 #include "sync/util/get_session_name.h"
 #include "sync/util/time.h"
 
 using base::TimeDelta;
 using browser_sync::AllStatus;
+using browser_sync::ConfigureParams;
 using browser_sync::Cryptographer;
 using browser_sync::Encryptor;
 using browser_sync::JsArgList;
 using browser_sync::JsBackend;
 using browser_sync::JsEventDetails;
 using browser_sync::JsEventHandler;
 using browser_sync::JsEventHandler;
 using browser_sync::JsReplyHandler;
 using browser_sync::JsMutationEventObserver;
 using browser_sync::JsSyncManagerObserver;
@@ skipping 764 matching lines @@
 
 bool SyncManager::EncryptEverythingEnabledForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return trans.GetCryptographer()->encrypt_everything();
 }
 
 bool SyncManager::IsUsingExplicitPassphrase() {
   return data_ && data_->IsUsingExplicitPassphrase();
 }
 
-void SyncManager::RequestCleanupDisabledTypes(
-    const browser_sync::ModelSafeRoutingInfo& routing_info) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (data_->scheduler()) {
-    data_->session_context()->set_routing_info(routing_info);
-    data_->scheduler()->CleanupDisabledTypes();
-  }
-}
-
 void SyncManager::RequestClearServerData() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (data_->scheduler())
     data_->scheduler()->ClearUserData();
 }
 
-void SyncManager::RequestConfig(
-    const browser_sync::ModelSafeRoutingInfo& routing_info,
-    const ModelTypeSet& types, ConfigureReason reason) {
+void SyncManager::ConfigureSyncer(
+    ConfigureReason reason,
+    const syncable::ModelTypeSet& types_to_config,
+    const browser_sync::ModelSafeRoutingInfo& new_routing_info,
+    const base::Closure& ready_task,
+    const base::Closure& retry_task) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!ready_task.is_null());
+  DCHECK(!retry_task.is_null());
+
+  // TODO(zea): set this based on whether cryptographer has keystore
+  // encryption key or not (requires opening a transaction).
+  bool need_encryption_key = false;
+
+  ConfigureParams params(GetSourceFromReason(reason),
+                         types_to_config,
+                         new_routing_info,
+                         need_encryption_key,
+                         ready_task);
+
+  // Returns false if a retry has been scheduled. Either way, will invoke the
+  // ready task when the configure job completes.
+  if (!DoConfigureSyncer(params))
+    retry_task.Run();
+}
+
+bool SyncManager::DoConfigureSyncer(const ConfigureParams& params) {
   if (!data_->scheduler()) {
     LOG(INFO)
-        << "SyncManager::RequestConfig: bailing out because scheduler is "
-        << "null";
-    return;
+        << "SyncManager::DoConfigureSyncer: could not configure because "
+        << "scheduler is null";
+    params.ready_task.Run();
+    return true;
   }
-  StartConfigurationMode(base::Closure());
-  data_->session_context()->set_routing_info(routing_info);
-  data_->scheduler()->Configure(types, GetSourceFromReason(reason));
-}
 
-void SyncManager::StartConfigurationMode(const base::Closure& callback) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (!data_->scheduler()) {
-    LOG(INFO)
-        << "SyncManager::StartConfigurationMode: could not start "
-        << "configuration mode because because scheduler is null";
-    return;
-  }
-  data_->scheduler()->Start(
-      browser_sync::SyncScheduler::CONFIGURATION_MODE, callback);
+  data_->scheduler()->Start(SyncScheduler::CONFIGURATION_MODE);
+  return data_->scheduler()->Configure(params);
 }
 
 bool SyncManager::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,
     HttpPostProviderFactory* post_factory,
@@ skipping 64 matching lines @@
         &debug_info_event_listener_,
         &traffic_recorder_));
     session_context()->set_account_name(credentials.email);
     scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));
   }
 
   bool signed_in = SignIn(credentials);
 
   if (signed_in) {
     if (scheduler()) {
-      scheduler()->Start(
-          browser_sync::SyncScheduler::CONFIGURATION_MODE, base::Closure());
+      scheduler()->Start(SyncScheduler::CONFIGURATION_MODE);
     }
 
     initialized_ = true;
 
     // Cryptographer should only be accessed while holding a
     // transaction.  Grabbing the user share for the transaction
     // checks the initialization state, so this must come after
     // |initialized_| is set to true.
     ReadTransaction trans(FROM_HERE, GetUserShare());
     trans.GetCryptographer()->Bootstrap(restored_key_for_bootstrapping);
@@ skipping 153 matching lines @@
   allstatus_.SetCryptographerReady(cryptographer->is_ready());
   allstatus_.SetCryptoHasPendingKeys(cryptographer->has_pending_keys());
   debug_info_event_listener_.SetCryptographerReady(cryptographer->is_ready());
   debug_info_event_listener_.SetCrytographerHasPendingKeys(
       cryptographer->has_pending_keys());
 }
 
 void SyncManager::SyncInternal::StartSyncingNormally(
     const browser_sync::ModelSafeRoutingInfo& routing_info) {
   // Start the sync scheduler.
-  if (scheduler()) { // NULL during certain unittests.
+  if (scheduler()) {  // NULL during certain unittests.
+    // TODO(sync): pass this into the scheduler somehow.

[rlarocque, 2012/06/12 17:45:13]

What is 'this' referring to?

[Nicolas Zea, 2012/06/12 20:44:44]

This comment has been updated in the newer patch.

     session_context()->set_routing_info(routing_info);
-    scheduler()->Start(SyncScheduler::NORMAL_MODE, base::Closure());
+    scheduler()->Start(SyncScheduler::NORMAL_MODE);
   }
 }
 
 bool SyncManager::SyncInternal::OpenDirectory() {
   DCHECK(!initialized_) << "Should only happen once";
 
   // Set before Open().
   change_observer_ =
       browser_sync::MakeWeakHandle(js_mutation_event_observer_.AsWeakPtr());
   WeakHandle<syncable::TransactionObserver> transaction_observer(
@@ skipping 1345 matching lines @@
     share->directory->GetDownloadProgress(i.Get(), &marker);
 
     if (marker.token().empty())
       result.Put(i.Get());
 
   }
   return result;
 }
 
 }  // namespace sync_api