[Sync] Refactor sync configuration logic.

We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
 CleanupDisabledTypes and GetKey commands.

This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.

BUG=129665,133061,129825
TEST=unit/integration tests

Review URL: https://chromiumcodereview.appspot.com/10483015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142517 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 732 insertions, 552 deletions

diff --git a/chrome/browser/sync/glue/backend_data_type_configurer.h b/chrome/browser/sync/glue/backend_data_type_configurer.h
index 161bc14..f15986b 100644
--- a/chrome/browser/sync/glue/backend_data_type_configurer.h
+++ b/chrome/browser/sync/glue/backend_data_type_configurer.h
@@ -34,11 +34,11 @@ class BackendDataTypeConfigurer {
   // Nigori.
   virtual void ConfigureDataTypes(
       sync_api::ConfigureReason reason,
-      syncable::ModelTypeSet types_to_add,
-      syncable::ModelTypeSet types_to_remove,
+      const syncable::ModelTypeSet& types_to_add,
+      const syncable::ModelTypeSet& types_to_remove,
       NigoriState nigori_state,
-      base::Callback<void(syncable::ModelTypeSet)> ready_task,
-      base::Callback<void()> retry_callback) = 0;
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+      const base::Callback<void()>& retry_callback) = 0;
 
  protected:
   virtual ~BackendDataTypeConfigurer() {}
diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
index c344d48..cc0db2b 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
@@ -42,11 +42,11 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer {
 
   virtual void ConfigureDataTypes(
       sync_api::ConfigureReason reason,
-      ModelTypeSet types_to_add,
-      ModelTypeSet types_to_remove,
+      const ModelTypeSet& types_to_add,
+      const ModelTypeSet& types_to_remove,
       NigoriState nigori_state,
-      base::Callback<void(ModelTypeSet)> ready_task,
-      base::Callback<void()> retry_callback) OVERRIDE {
+      const base::Callback<void(ModelTypeSet)>& ready_task,
+      const base::Callback<void()>& retry_callback) OVERRIDE {
     last_nigori_state_ = nigori_state;
     last_ready_task_ = ready_task;
   }
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 66f6307..a4f09e7 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -131,10 +131,6 @@ class SyncBackendHost::Core
   // Called to clear server data.
   void DoRequestClearServerData();
 
-  // Called to cleanup disabled types.
-  void DoRequestCleanupDisabledTypes(
-      const browser_sync::ModelSafeRoutingInfo& routing_info);
-
   // Called to set the passphrase for encryption.
   void DoSetEncryptionPassphrase(const std::string& passphrase,
                                  bool is_explicit);
@@ -164,14 +160,18 @@ class SyncBackendHost::Core
   void DoStopSyncManagerForShutdown(const base::Closure& closure);
   void DoShutdown(bool stopping_sync);
 
-  virtual void DoRequestConfig(
-      const browser_sync::ModelSafeRoutingInfo& routing_info,
+  // Configuration methods that must execute on sync loop.
+  void DoConfigureSyncer(
+      sync_api::ConfigureReason reason,
       syncable::ModelTypeSet types_to_config,
-      sync_api::ConfigureReason reason);
-
-  // Start the configuration mode.  |callback| is called on the sync
-  // thread.
-  virtual void DoStartConfiguration(const base::Closure& callback);
+      const browser_sync::ModelSafeRoutingInfo routing_info,
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+      const base::Closure& retry_callback);
+  void DoFinishConfigureDataTypes(
+      syncable::ModelTypeSet types_to_config,
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task);
+  void DoRetryConfiguration(
+      const base::Closure& retry_callback);
 
   // Set the base request context to use when making HTTP calls.
   // This method will add a reference to the context to persist it
@@ -184,9 +184,6 @@ class SyncBackendHost::Core
   // sync databases), as well as shutdown when you're no longer syncing.
   void DeleteSyncDataFolder();
 
-  // A callback from the SyncerThread when it is safe to continue config.
-  void FinishConfigureDataTypes();
-
  private:
   friend class base::RefCountedThreadSafe<SyncBackendHost::Core>;
   friend class SyncBackendHostForProfileSyncTest;
@@ -556,60 +553,79 @@ void SyncBackendHost::Shutdown(bool sync_disabled) {
   core_ = NULL;  // Releases reference to core_.
 }
 
+// TODO(sync): remove |disabled_types| completely, as it's just the set of
+// registered types - the set of desired types.
 void SyncBackendHost::ConfigureDataTypes(
     sync_api::ConfigureReason reason,
-    syncable::ModelTypeSet types_to_add,
-    syncable::ModelTypeSet types_to_remove,
+    const syncable::ModelTypeSet& desired_types,
+    const syncable::ModelTypeSet& disabled_types,
     NigoriState nigori_state,
-    base::Callback<void(syncable::ModelTypeSet)> ready_task,
-    base::Callback<void()> retry_callback) {
-  syncable::ModelTypeSet types_to_add_with_nigori = types_to_add;
-  syncable::ModelTypeSet types_to_remove_with_nigori = types_to_remove;
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+    const base::Callback<void()>& retry_callback) {
+  syncable::ModelTypeSet desired_types_with_nigori = desired_types;
+  syncable::ModelTypeSet disabled_types_with_nigori = disabled_types;
   if (nigori_state == WITH_NIGORI) {
-    types_to_add_with_nigori.Put(syncable::NIGORI);
-    types_to_remove_with_nigori.Remove(syncable::NIGORI);
+    desired_types_with_nigori.Put(syncable::NIGORI);
+    disabled_types_with_nigori.Remove(syncable::NIGORI);
   } else {
-    types_to_add_with_nigori.Remove(syncable::NIGORI);
-    types_to_remove_with_nigori.Put(syncable::NIGORI);
+    desired_types_with_nigori.Remove(syncable::NIGORI);
+    disabled_types_with_nigori.Put(syncable::NIGORI);
   }
-  // Only one configure is allowed at a time.
-  DCHECK(!pending_config_mode_state_.get());
-  DCHECK(!pending_download_state_.get());
+  // Only one configure is allowed at a time (DataTypeManager handles user
+  // changes that happen while the syncer is reconfiguraing, and will only
+  // trigger another call to ConfigureDataTypes once the current reconfiguration
+  // completes).
   DCHECK_GT(initialization_state_, NOT_INITIALIZED);
 
-  pending_config_mode_state_.reset(new PendingConfigureDataTypesState());
-  pending_config_mode_state_->ready_task = ready_task;
-  pending_config_mode_state_->types_to_add = types_to_add_with_nigori;
-  pending_config_mode_state_->added_types =
-      registrar_->ConfigureDataTypes(types_to_add_with_nigori,
-                                     types_to_remove_with_nigori);
-  pending_config_mode_state_->reason = reason;
-  pending_config_mode_state_->retry_callback = retry_callback;
-
-  // Cleanup disabled types before starting configuration so that
-  // callers can assume that the data types are cleaned up once
-  // configuration is done.
-  if (!types_to_remove_with_nigori.Empty()) {
-    ModelSafeRoutingInfo routing_info;
-    registrar_->GetModelSafeRoutingInfo(&routing_info);
-    sync_thread_.message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&SyncBackendHost::Core::DoRequestCleanupDisabledTypes,
-                   core_.get(),
-                   routing_info));
-  }
+  // enabled_types \setunion disabled_types gives the set of all registered
+  // types (i.e. types with datatype controllers registered).
+  ModelSafeRoutingInfo routing_info;
+  registrar_->ConfigureDataTypes(desired_types_with_nigori,
+                                 disabled_types_with_nigori);
+  registrar_->GetModelSafeRoutingInfo(&routing_info);
 
-  StartConfiguration(
-      base::Bind(&SyncBackendHost::Core::FinishConfigureDataTypes,
-                 core_.get()));
-}
+  // |enabled_types| is desired_types_with_nigori \setintersect types with
+  // model safe workers (e.g. the password store may fail to start up, in which
+  // case we have no model safe worker for it and should not enable it).
+  // TODO(sync): This shouldn't be necessary. We should do whatever filtering
+  // we need beforehand as part of ProfileSyncComponentsFactory's
+  // RegisterDataTypes.
+  const syncable::ModelTypeSet enabled_types =
+      GetRoutingInfoTypes(routing_info);
 
-void SyncBackendHost::StartConfiguration(const base::Closure& callback) {
-  // Put syncer in the config mode. DTM will put us in normal mode once it is
-  // done. This is to ensure we dont do a normal sync when we are doing model
-  // association.
-  sync_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &SyncBackendHost::Core::DoStartConfiguration, core_.get(), callback));
+  // Figure out which types need to actually be downloaded. We pass those on
+  // to the syncer while it's in configuration mode so that they can be
+  // downloaded before we perform association. Once we switch to normal mode
+  // downloads will get applied normally and hit the datatype's change
+  // processor.
+  // A datatype is in need of downloading if any of the following are true:
+  // 1. it's enabled and initial_sync_ended is false (initial_sync_ended is
+  //    set after applying updates, and hence is a more conservative measure
+  //    than having a non-empty progress marker, which is set during
+  //    StoreTimestamps).
+  // 2. the type is NIGORI, and any other datatype is being downloaded (nigori
+  //    is always included if we download a datatype).
+  // TODO(sync): consider moving this logic onto the sync thread (perhaps
+  // as part of SyncManager::ConfigureSyncer).
+  syncable::ModelTypeSet initial_sync_ended_types =
+      core_->sync_manager()->InitialSyncEndedTypes();
+  initial_sync_ended_types.RetainAll(enabled_types);
+  syncable::ModelTypeSet types_to_config =
+      Difference(enabled_types, initial_sync_ended_types);
+  if (!types_to_config.Empty() && enabled_types.Has(syncable::NIGORI))
+    types_to_config.Put(syncable::NIGORI);
+
+  SDVLOG(1) << "Types "
+            << syncable::ModelTypeSetToString(types_to_config)
+            << " added; calling DoConfigureSyncer";
+  // TODO(zea): figure out how to bypass this call if no types are being
+  // configured and GetKey is not needed. For now we rely on determining the
+  // need for GetKey as part of the SyncManager::ConfigureSyncer logic.
+  RequestConfigureSyncer(reason,
+                         types_to_config,
+                         routing_info,
+                         ready_task,
+                         retry_callback);
 }
 
 void SyncBackendHost::EnableEncryptEverything() {
@@ -687,157 +703,47 @@ void SyncBackendHost::InitCore(const DoInitializeOptions& options) {
       base::Bind(&SyncBackendHost::Core::DoInitialize, core_.get(), options));
 }
 
-void SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop(
-    const SyncSessionSnapshot& snapshot) {
-  if (!frontend_)
-    return;
-  DCHECK_EQ(MessageLoop::current(), frontend_loop_);
-
-  last_snapshot_ = snapshot;
-
-  SDVLOG(1) << "Got snapshot " << snapshot.ToString();
-
-  const syncable::ModelTypeSet to_migrate =
-      snapshot.syncer_status().types_needing_local_migration;
-  if (!to_migrate.Empty())
-    frontend_->OnMigrationNeededForTypes(to_migrate);
-
-  // Process any changes to the datatypes we're syncing.
-  // TODO(sync): add support for removing types.
-  if (initialized())
-    AddExperimentalTypes();
-
-  // If we are waiting for a configuration change, check here to see
-  // if this sync cycle has initialized all of the types we've been
-  // waiting for.
-  if (pending_download_state_.get()) {
-    const syncable::ModelTypeSet types_to_add =
-        pending_download_state_->types_to_add;
-    const syncable::ModelTypeSet added_types =
-        pending_download_state_->added_types;
-    DCHECK(types_to_add.HasAll(added_types));
-    const syncable::ModelTypeSet initial_sync_ended =
-        snapshot.initial_sync_ended();
-    const syncable::ModelTypeSet failed_configuration_types =
-        Difference(added_types, initial_sync_ended);
-    SDVLOG(1)
-        << "Added types: "
-        << syncable::ModelTypeSetToString(added_types)
-        << ", configured types: "
-        << syncable::ModelTypeSetToString(initial_sync_ended)
-        << ", failed configuration types: "
-        << syncable::ModelTypeSetToString(failed_configuration_types);
-
-    if (!failed_configuration_types.Empty() &&
-        snapshot.retry_scheduled()) {
-      // Inform the caller that download failed but we are retrying.
-      if (!pending_download_state_->retry_in_progress) {
-        pending_download_state_->retry_callback.Run();
-        pending_download_state_->retry_in_progress = true;
-      }
-      // Nothing more to do.
-      return;
-    }
-
-    scoped_ptr<PendingConfigureDataTypesState> state(
-        pending_download_state_.release());
-    state->ready_task.Run(failed_configuration_types);
-
-    // Syncer did not report an error but did not download everything
-    // we requested either. So abort. The caller of the config will cleanup.
-    if (!failed_configuration_types.Empty())
-      return;
-  }
-
-  if (initialized())
-    frontend_->OnSyncCycleCompleted();
-}
-
-void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop() {
-  DCHECK_EQ(MessageLoop::current(), frontend_loop_);
-  // Nudge the syncer. This is necessary for both datatype addition/deletion.
-  //
-  // Deletions need a nudge in order to ensure the deletion occurs in a timely
-  // manner (see issue 56416).
-  //
-  // In the case of additions, on the next sync cycle, the syncer should
-  // notice that the routing info has changed and start the process of
-  // downloading updates for newly added data types.  Once this is
-  // complete, the configure_state_.ready_task_ is run via an
-  // OnInitializationComplete notification.
-
-  SDVLOG(1) << "Syncer in config mode. SBH executing "
-            << "FinishConfigureDataTypesOnFrontendLoop";
-
-
-  ModelSafeRoutingInfo routing_info;
-  registrar_->GetModelSafeRoutingInfo(&routing_info);
-  const syncable::ModelTypeSet enabled_types =
-      GetRoutingInfoTypes(routing_info);
+void SyncBackendHost::RequestConfigureSyncer(
+    sync_api::ConfigureReason reason,
+    syncable::ModelTypeSet types_to_config,
+    const browser_sync::ModelSafeRoutingInfo& routing_info,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+    const base::Closure& retry_callback) {
+  sync_thread_.message_loop()->PostTask(FROM_HERE,
+       base::Bind(&SyncBackendHost::Core::DoConfigureSyncer,
+                  core_.get(),
+                  reason,
+                  types_to_config,
+                  routing_info,
+                  ready_task,
+                  retry_callback));
+}
+
+void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop(
+    const syncable::ModelTypeSet& types_to_configure,
+    const syncable::ModelTypeSet& configured_types,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task) {
+  const syncable::ModelTypeSet failed_configuration_types =
+      Difference(types_to_configure, configured_types);
+  SDVLOG(1)
+      << "Added types: "
+      << syncable::ModelTypeSetToString(types_to_configure)
+      << ", configured types: "
+      << syncable::ModelTypeSetToString(configured_types)
+      << ", failed configuration types: "
+      << syncable::ModelTypeSetToString(failed_configuration_types);
 
   // Update |chrome_sync_notification_bridge_|'s enabled types here as it has
   // to happen on the UI thread.
-  chrome_sync_notification_bridge_.UpdateEnabledTypes(enabled_types);
-
-  if (pending_config_mode_state_->added_types.Empty() &&
-      !core_->sync_manager()->InitialSyncEndedTypes().HasAll(enabled_types)) {
-
-    // TODO(tim): Log / UMA / count this somehow?
-    // Add only the types with empty progress markers. Note: it is possible
-    // that some types have their initial_sync_ended be false but with non
-    // empty progress marker. Which is ok as the rest of the changes would
-    // be downloaded on a regular nudge and initial_sync_ended should be set
-    // to true. However this is a very corner case. So it is not explicitly
-    // handled.
-    pending_config_mode_state_->added_types =
-        sync_api::GetTypesWithEmptyProgressMarkerToken(enabled_types,
-                                                       GetUserShare());
-  }
-
-  // If we've added types, we always want to request a nudge/config (even if
-  // the initial sync is ended), in case we could not decrypt the data.
-  if (pending_config_mode_state_->added_types.Empty()) {
-    SDVLOG(1) << "No new types added; calling ready_task directly";
-    // No new types - just notify the caller that the types are available.
-    const syncable::ModelTypeSet failed_configuration_types;
-    pending_config_mode_state_->ready_task.Run(failed_configuration_types);
-  } else {
-    pending_download_state_.reset(pending_config_mode_state_.release());
-
-    // Always configure nigori if it's enabled.
-    syncable::ModelTypeSet types_to_config =
-        pending_download_state_->added_types;
-    if (IsNigoriEnabled()) {
-      // Note: Nigori is the only type that gets added with a nonempty
-      // progress marker during config. If the server returns a migration
-      // error then we will go into unrecoverable error. We dont handle it
-      // explicitly because server might help us out here by not sending a
-      // migraiton error for nigori during config.
-      types_to_config.Put(syncable::NIGORI);
-    }
-    SDVLOG(1) << "Types "
-              << syncable::ModelTypeSetToString(types_to_config)
-              << " added; calling DoRequestConfig";
-    ModelSafeRoutingInfo routing_info;
-    registrar_->GetModelSafeRoutingInfo(&routing_info);
-    sync_thread_.message_loop()->PostTask(FROM_HERE,
-         base::Bind(&SyncBackendHost::Core::DoRequestConfig,
-                    core_.get(),
-                    routing_info,
-                    types_to_config,
-                    pending_download_state_->reason));
-  }
-
-  pending_config_mode_state_.reset();
+  chrome_sync_notification_bridge_.UpdateEnabledTypes(configured_types);
 
   // Notify SyncManager (especially the notification listener) about new types.
   sync_thread_.message_loop()->PostTask(FROM_HERE,
       base::Bind(&SyncBackendHost::Core::DoUpdateEnabledTypes, core_.get(),
-                 enabled_types));
-}
+                 configured_types));
 
-bool SyncBackendHost::IsDownloadingNigoriForTest() const {
-  return initialization_state_ == DOWNLOADING_NIGORI;
+  if (!ready_task.is_null())
+    ready_task.Run(failed_configuration_types);
 }
 
 SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
@@ -894,14 +800,6 @@ SyncBackendHost::Core::~Core() {
   DCHECK(!sync_loop_);
 }
 
-SyncBackendHost::PendingConfigureDataTypesState::
-PendingConfigureDataTypesState()
-    : reason(sync_api::CONFIGURE_REASON_UNKNOWN),
-      retry_in_progress(false) {}
-
-SyncBackendHost::PendingConfigureDataTypesState::
-~PendingConfigureDataTypesState() {}
-
 void SyncBackendHost::Core::OnSyncCycleCompleted(
     const SyncSessionSnapshot& snapshot) {
   if (!sync_loop_)
@@ -1153,12 +1051,6 @@ void SyncBackendHost::Core::DoRequestClearServerData() {
   sync_manager_->RequestClearServerData();
 }
 
-void SyncBackendHost::Core::DoRequestCleanupDisabledTypes(
-    const browser_sync::ModelSafeRoutingInfo& routing_info) {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  sync_manager_->RequestCleanupDisabledTypes(routing_info);
-}
-
 void SyncBackendHost::Core::DoSetEncryptionPassphrase(
     const std::string& passphrase,
     bool is_explicit) {
@@ -1210,18 +1102,46 @@ void SyncBackendHost::Core::DoShutdown(bool sync_disabled) {
   host_.Reset();
 }
 
-void SyncBackendHost::Core::DoRequestConfig(
-    const browser_sync::ModelSafeRoutingInfo& routing_info,
+void SyncBackendHost::Core::DoConfigureSyncer(
+    sync_api::ConfigureReason reason,
     syncable::ModelTypeSet types_to_config,
-    sync_api::ConfigureReason reason) {
+    const browser_sync::ModelSafeRoutingInfo routing_info,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+    const base::Closure& retry_callback) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  sync_manager_->RequestConfig(routing_info, types_to_config, reason);
+  sync_manager_->ConfigureSyncer(
+      reason,
+      types_to_config,
+      routing_info,
+      base::Bind(&SyncBackendHost::Core::DoFinishConfigureDataTypes,
+                 this,
+                 types_to_config,
+                 ready_task),
+      base::Bind(&SyncBackendHost::Core::DoRetryConfiguration,
+                 this,
+                 retry_callback));
 }
 
-void SyncBackendHost::Core::DoStartConfiguration(
-    const base::Closure& callback) {
+void SyncBackendHost::Core::DoFinishConfigureDataTypes(
+    syncable::ModelTypeSet types_to_config,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task) {
+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  syncable::ModelTypeSet configured_types =
+      sync_manager_->InitialSyncEndedTypes();
+  configured_types.RetainAll(types_to_config);
+  host_.Call(FROM_HERE,
+             &SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop,
+             types_to_config,
+             configured_types,
+             ready_task);
+}
+
+void SyncBackendHost::Core::DoRetryConfiguration(
+    const base::Closure& retry_callback) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  sync_manager_->StartConfigurationMode(callback);
+  host_.Call(FROM_HERE,
+             &SyncBackendHost::RetryConfigurationOnFrontendLoop,
+             retry_callback);
 }
 
 void SyncBackendHost::Core::DeleteSyncDataFolder() {
@@ -1232,13 +1152,6 @@ void SyncBackendHost::Core::DeleteSyncDataFolder() {
   }
 }
 
-void SyncBackendHost::Core::FinishConfigureDataTypes() {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  host_.Call(
-      FROM_HERE,
-      &SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop);
-}
-
 void SyncBackendHost::Core::StartSavingChanges() {
   // We may already be shut down.
   if (!sync_loop_)
@@ -1289,14 +1202,6 @@ void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop(
     return;
   }
 
-  // If setup has completed, start off in DOWNLOADING_NIGORI so that
-  // we start off by refreshing nigori.
-  CHECK(sync_prefs_.get());
-  if (sync_prefs_->HasSyncSetupCompleted() &&
-      initialization_state_ < DOWNLOADING_NIGORI) {
-    initialization_state_ = DOWNLOADING_NIGORI;
-  }
-
   // Run initialization state machine.
   switch (initialization_state_) {
     case NOT_INITIALIZED:
@@ -1337,6 +1242,36 @@ void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop(
   }
 }
 
+void SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop(
+    const SyncSessionSnapshot& snapshot) {
+  if (!frontend_)
+    return;
+  DCHECK_EQ(MessageLoop::current(), frontend_loop_);
+
+  last_snapshot_ = snapshot;
+
+  SDVLOG(1) << "Got snapshot " << snapshot.ToString();
+
+  const syncable::ModelTypeSet to_migrate =
+      snapshot.syncer_status().types_needing_local_migration;
+  if (!to_migrate.Empty())
+    frontend_->OnMigrationNeededForTypes(to_migrate);
+
+  // Process any changes to the datatypes we're syncing.
+  // TODO(sync): add support for removing types.
+  if (initialized())
+    AddExperimentalTypes();
+
+  if (initialized())
+    frontend_->OnSyncCycleCompleted();
+}
+
+void SyncBackendHost::RetryConfigurationOnFrontendLoop(
+    const base::Closure& retry_callback) {
+  SDVLOG(1) << "Failed to complete configuration, informing of retry.";
+  retry_callback.Run();
+}
+
 void SyncBackendHost::PersistEncryptionBootstrapToken(
     const std::string& token) {
   CHECK(sync_prefs_.get());
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 600e516..00aa77e 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -226,15 +226,11 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // is non-empty, then an error was encountered).
   virtual void ConfigureDataTypes(
       sync_api::ConfigureReason reason,
-      syncable::ModelTypeSet types_to_add,
-      syncable::ModelTypeSet types_to_remove,
+      const syncable::ModelTypeSet& desired_types,
+      const syncable::ModelTypeSet& disabled_types,
       NigoriState nigori_state,
-      base::Callback<void(syncable::ModelTypeSet)> ready_task,
-      base::Callback<void()> retry_callback) OVERRIDE;
-
-  // Makes an asynchronous call to syncer to switch to config mode. When done
-  // syncer will call us back on FinishConfigureDataTypes.
-  virtual void StartConfiguration(const base::Closure& callback);
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+      const base::Callback<void()>& retry_callback) OVERRIDE;
 
   // Turns on encryption of all present and future sync data.
   virtual void EnableEncryptEverything();
@@ -333,16 +329,20 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // Allows tests to perform alternate core initialization work.
   virtual void InitCore(const DoInitializeOptions& options);
 
-  // Called from Core::OnSyncCycleCompleted to handle updating frontend
-  // thread components.
-  void HandleSyncCycleCompletedOnFrontendLoop(
-      const sessions::SyncSessionSnapshot& snapshot);
-
-  // Called to finish the job of ConfigureDataTypes once the syncer is in
-  // configuration mode.
-  void FinishConfigureDataTypesOnFrontendLoop();
+  // Request the syncer to reconfigure with the specfied params.
+  // Virtual for testing.
+  virtual void RequestConfigureSyncer(
+      sync_api::ConfigureReason reason,
+      syncable::ModelTypeSet types_to_config,
+      const browser_sync::ModelSafeRoutingInfo& routing_info,
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+      const base::Closure& retry_callback);
 
-  bool IsDownloadingNigoriForTest() const;
+  // Called when the syncer has finished performing a configuration.
+  void FinishConfigureDataTypesOnFrontendLoop(
+    const syncable::ModelTypeSet& types_to_configure,
+    const syncable::ModelTypeSet& configured_types,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task);
 
  private:
   // The real guts of SyncBackendHost, to keep the public client API clean.
@@ -365,47 +365,32 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
     INITIALIZED,            // Initialization is complete.
   };
 
-  struct PendingConfigureDataTypesState {
-    PendingConfigureDataTypesState();
-    ~PendingConfigureDataTypesState();
-
-    // The ready_task will be run when configuration is done with the
-    // set of all types that failed configuration (i.e., if its
-    // argument is non-empty, then an error was encountered).
-    base::Callback<void(syncable::ModelTypeSet)> ready_task;
-
-    // The retry callback will be run when the download failed due to a
-    // transient error. This is to notify DTM so it can apropriately inform
-    // the UI. Note: The retry_callback will be run only once and after
-    // that we will not notify DTM until the sync is successful or in a
-    // permanent error state.
-    base::Callback<void()> retry_callback;
-
-    // The set of types that we are waiting to be initially synced in a
-    // configuration cycle.
-    syncable::ModelTypeSet types_to_add;
-
-    // Additional details about which types were added.
-    syncable::ModelTypeSet added_types;
-    sync_api::ConfigureReason reason;
-    bool retry_in_progress;
-  };
-
   // Checks if we have received a notice to turn on experimental datatypes
   // (via the nigori node) and informs the frontend if that is the case.
   // Note: it is illegal to call this before the backend is initialized.
   void AddExperimentalTypes();
 
   // Downloading of nigori failed and will be retried.
-  virtual void OnNigoriDownloadRetry();
+  void OnNigoriDownloadRetry();
 
   // InitializationComplete passes through the SyncBackendHost to forward
   // on to |frontend_|, and so that tests can intercept here if they need to
   // set up initial conditions.
-  virtual void HandleInitializationCompletedOnFrontendLoop(
+  void HandleInitializationCompletedOnFrontendLoop(
       const WeakHandle<JsBackend>& js_backend,
       bool success);
 
+  // Called from Core::OnSyncCycleCompleted to handle updating frontend
+  // thread components.
+  void HandleSyncCycleCompletedOnFrontendLoop(
+      const sessions::SyncSessionSnapshot& snapshot);
+
+  // Called when the syncer failed to perform a configuration and will
+  // eventually retry. FinishingConfigurationOnFrontendLoop(..) will be called
+  // on successful completion.
+  void RetryConfigurationOnFrontendLoop(
+      const base::Closure& retry_callback);
+
   // Helpers to persist a token that can be used to bootstrap sync encryption
   // across browser restart to avoid requiring the user to re-enter their
   // passphrase.  |token| must be valid UTF-8 as we use the PrefService for
@@ -509,9 +494,6 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // The frontend which we serve (and are owned by).
   SyncFrontend* frontend_;
 
-  scoped_ptr<PendingConfigureDataTypesState> pending_download_state_;
-  scoped_ptr<PendingConfigureDataTypesState> pending_config_mode_state_;
-
   // We cache the cryptographer's pending keys whenever NotifyPassphraseRequired
   // is called. This way, before the UI calls SetDecryptionPassphrase on the
   // syncer, it can avoid the overhead of an asynchronous decryption call and
diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc
index 6d16734..4a89b8c 100644
--- a/chrome/browser/sync/test_profile_sync_service.cc
+++ b/chrome/browser/sync/test_profile_sync_service.cc
@@ -44,19 +44,6 @@ SyncBackendHostForProfileSyncTest::SyncBackendHostForProfileSyncTest(
 
 SyncBackendHostForProfileSyncTest::~SyncBackendHostForProfileSyncTest() {}
 
-void SyncBackendHostForProfileSyncTest::
-    SimulateSyncCycleCompletedInitialSyncEnded(
-    const tracked_objects::Location& location) {
-  syncable::ModelTypeSet sync_ended;
-  if (!fail_initial_download_)
-    sync_ended = syncable::ModelTypeSet::All();
-  syncable::ModelTypePayloadMap download_progress_markers;
-  HandleSyncCycleCompletedOnFrontendLoop(SyncSessionSnapshot(
-      SyncerStatus(), ErrorCounters(), 0, false,
-      sync_ended, download_progress_markers, false, false, 0, 0, 0, 0,
-      SyncSourceInfo(), false, 0, base::Time::Now(), false));
-}
-
 namespace {
 
 sync_api::HttpPostProviderFactory* MakeTestHttpBridgeFactory() {
@@ -85,20 +72,19 @@ void SyncBackendHostForProfileSyncTest::InitCore(
   }
 }
 
-void SyncBackendHostForProfileSyncTest::StartConfiguration(
-    const base::Closure& callback) {
-  SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop();
-  if (IsDownloadingNigoriForTest()) {
-    syncable::ModelTypeSet sync_ended;
+void SyncBackendHostForProfileSyncTest::RequestConfigureSyncer(
+    sync_api::ConfigureReason reason,
+    syncable::ModelTypeSet types_to_config,
+    const browser_sync::ModelSafeRoutingInfo& routing_info,
+    const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+    const base::Closure& retry_callback) {
+  syncable::ModelTypeSet sync_ended;
+  if (!fail_initial_download_)
+    sync_ended.PutAll(types_to_config);
 
-    if (!fail_initial_download_)
-      sync_ended.Put(syncable::NIGORI);
-    syncable::ModelTypePayloadMap download_progress_markers;
-    HandleSyncCycleCompletedOnFrontendLoop(SyncSessionSnapshot(
-        SyncerStatus(), ErrorCounters(), 0, false,
-        sync_ended, download_progress_markers, false, false, 0, 0, 0, 0,
-        SyncSourceInfo(), false, 0, base::Time::Now(), false));
-  }
+  FinishConfigureDataTypesOnFrontendLoop(types_to_config,
+                                         sync_ended,
+                                         ready_task);
 }
 
 void SyncBackendHostForProfileSyncTest::SetHistoryServiceExpectations(
diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h
index 1227e5f..efec408 100644
--- a/chrome/browser/sync/test_profile_sync_service.h
+++ b/chrome/browser/sync/test_profile_sync_service.h
@@ -45,11 +45,12 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
 
   MOCK_METHOD1(RequestNudge, void(const tracked_objects::Location&));
 
-  // Called when a nudge comes in.
-  void SimulateSyncCycleCompletedInitialSyncEnded(
-      const tracked_objects::Location&);
-
-  virtual void StartConfiguration(const base::Closure& callback) OVERRIDE;
+  virtual void RequestConfigureSyncer(
+      sync_api::ConfigureReason reason,
+      syncable::ModelTypeSet types_to_config,
+      const browser_sync::ModelSafeRoutingInfo& routing_info,
+      const base::Callback<void(syncable::ModelTypeSet)>& ready_task,
+      const base::Closure& retry_callback) OVERRIDE;
 
   static void SetHistoryServiceExpectations(ProfileMock* profile);
 
diff --git a/sync/engine/sync_scheduler.cc b/sync/engine/sync_scheduler.cc
index f798d67..e745d19 100644
--- a/sync/engine/sync_scheduler.cc
+++ b/sync/engine/sync_scheduler.cc
@@ -68,6 +68,24 @@ bool IsActionableError(
 }
 }  // namespace
 
+ConfigurationParams::ConfigurationParams()
+    : source(GetUpdatesCallerInfo::UNKNOWN),
+      keystore_key_status(KEYSTORE_KEY_UNNECESSARY) {}
+ConfigurationParams::ConfigurationParams(
+    const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource& source,
+    const syncable::ModelTypeSet& types_to_download,
+    const browser_sync::ModelSafeRoutingInfo& routing_info,
+    KeystoreKeyStatus keystore_key_status,
+    const base::Closure& ready_task)
+    : source(source),
+      types_to_download(types_to_download),
+      routing_info(routing_info),
+      keystore_key_status(keystore_key_status),
+      ready_task(ready_task) {
+  DCHECK(!ready_task.is_null());
+}
+ConfigurationParams::~ConfigurationParams() {}
+
 SyncScheduler::DelayProvider::DelayProvider() {}
 SyncScheduler::DelayProvider::~DelayProvider() {}
 
@@ -99,12 +117,16 @@ SyncScheduler::SyncSessionJob::~SyncSessionJob() {}
 
 SyncScheduler::SyncSessionJob::SyncSessionJob(SyncSessionJobPurpose purpose,
     base::TimeTicks start,
-    linked_ptr<sessions::SyncSession> session, bool is_canary_job,
-    const tracked_objects::Location& from_here) : purpose(purpose),
-        scheduled_start(start),
-        session(session),
-        is_canary_job(is_canary_job),
-        from_here(from_here) {
+    linked_ptr<sessions::SyncSession> session,
+    bool is_canary_job,
+    ConfigurationParams config_params,
+    const tracked_objects::Location& from_here)
+    : purpose(purpose),
+      scheduled_start(start),
+      session(session),
+      is_canary_job(is_canary_job),
+      config_params(config_params),
+      from_here(from_here) {
 }
 
 const char* SyncScheduler::SyncSessionJob::GetPurposeString(
@@ -247,7 +269,7 @@ void SyncScheduler::UpdateServerConnectionManagerStatus(
   connection_code_ = code;
 }
 
-void SyncScheduler::Start(Mode mode, const base::Closure& callback) {
+void SyncScheduler::Start(Mode mode) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   std::string thread_name = MessageLoop::current()->thread_name();
   if (thread_name.empty())
@@ -264,8 +286,6 @@ void SyncScheduler::Start(Mode mode, const base::Closure& callback) {
   Mode old_mode = mode_;
   mode_ = mode;
   AdjustPolling(NULL);  // Will kick start poll timer if needed.
-  if (!callback.is_null())
-    callback.Run();
 
   if (old_mode != mode_) {
     // We just changed our mode. See if there are any pending jobs that we could
@@ -284,6 +304,112 @@ void SyncScheduler::SendInitialSnapshot() {
   session_context_->NotifyListeners(event);
 }
 
+namespace {
+
+// Helper to extract the routing info and workers corresponding to types in
+// |types| from |current_routes| and |current_workers|.
+void BuildModelSafeParams(
+    const ModelTypeSet& types_to_download,
+    const ModelSafeRoutingInfo& current_routes,
+    const std::vector<ModelSafeWorker*>& current_workers,
+    ModelSafeRoutingInfo* result_routes,
+    std::vector<ModelSafeWorker*>* result_workers) {
+  std::set<ModelSafeGroup> active_groups;
+  active_groups.insert(GROUP_PASSIVE);
+  for (ModelTypeSet::Iterator iter = types_to_download.First(); iter.Good();
+       iter.Inc()) {
+    syncable::ModelType type = iter.Get();
+    ModelSafeRoutingInfo::const_iterator route = current_routes.find(type);
+    DCHECK(route != current_routes.end());
+    ModelSafeGroup group = route->second;
+    (*result_routes)[type] = group;
+    active_groups.insert(group);
+  }
+
+  for(std::vector<ModelSafeWorker*>::const_iterator iter =
+          current_workers.begin(); iter != current_workers.end(); ++iter) {
+    if (active_groups.count((*iter)->GetModelSafeGroup()) > 0)
+      result_workers->push_back(*iter);
+  }
+}
+
+}  // namespace.
+
+bool SyncScheduler::ScheduleConfiguration(const ConfigurationParams& params) {
+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  DCHECK(IsConfigRelatedUpdateSourceValue(params.source));
+  DCHECK_EQ(CONFIGURATION_MODE, mode_);
+  DCHECK(!params.ready_task.is_null());
+  SDVLOG(2) << "Reconfiguring syncer.";
+
+  // Only one configuration is allowed at a time. Verify we're not waiting
+  // for a pending configure job.
+  DCHECK(!wait_interval_.get() || !wait_interval_->pending_configure_job.get());
+
+  // TODO(sync): now that ModelChanging commands only use those workers within
+  // the routing info, we don't really need |restricted_workers|. Remove it.
+  // crbug.com/133030
+  browser_sync::ModelSafeRoutingInfo restricted_routes;
+  std::vector<ModelSafeWorker*> restricted_workers;
+  BuildModelSafeParams(params.types_to_download,
+                       params.routing_info,
+                       session_context_->workers(),
+                       &restricted_routes,
+                       &restricted_workers);
+  session_context_->set_routing_info(params.routing_info);
+
+  // We rely on this not failing, so don't need to worry about checking for
+  // success. In addition, this will be removed as part of crbug.com/131433.
+  SyncSessionJob cleanup_job(
+      SyncSessionJob::CLEANUP_DISABLED_TYPES,
+      TimeTicks::Now(),
+      make_linked_ptr(CreateSyncSession(SyncSourceInfo())),
+      false,
+      ConfigurationParams(),
+      FROM_HERE);
+  DoSyncSessionJob(cleanup_job);
+
+  if (params.keystore_key_status == ConfigurationParams::KEYSTORE_KEY_NEEDED) {
+    // TODO(zea): implement in such a way that we can handle failures and the
+    // subsequent retrys the scheduler might perform. See crbug.com/129665.
+    NOTIMPLEMENTED();
+  }
+
+  // Only reconfigure if we have types to download.
+  if (!params.types_to_download.Empty()) {
+    DCHECK(!restricted_routes.empty());
+    linked_ptr<SyncSession> session(new SyncSession(
+        session_context_,
+        this,
+        SyncSourceInfo(params.source,
+                       syncable::ModelTypePayloadMapFromRoutingInfo(
+                           restricted_routes,
+                           std::string())),
+        restricted_routes,
+        restricted_workers));
+    SyncSessionJob job(SyncSessionJob::CONFIGURATION,
+                       TimeTicks::Now(),
+                       session,
+                       false,
+                       params,
+                       FROM_HERE);
+    DoSyncSessionJob(job);
+
+    // If we failed, the job would have been saved as the pending configure
+    // job and a wait interval would have been set.
+    if (!session->Succeeded()) {
+      DCHECK(wait_interval_.get() &&
+             wait_interval_->pending_configure_job.get());
+      return false;
+    }
+  } else {
+    SDVLOG(2) << "No change in routing info, calling ready task directly.";
+    params.ready_task.Run();
+  }
+
+  return true;
+}
+
 SyncScheduler::JobProcessDecision SyncScheduler::DecideWhileInWaitInterval(
     const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
@@ -382,7 +508,8 @@ void SyncScheduler::InitOrCoalescePendingJob(const SyncSessionJob& job) {
         s->delegate(), s->source(), s->routing_info(), s->workers()));
 
     SyncSessionJob new_job(SyncSessionJob::NUDGE, job.scheduled_start,
-        make_linked_ptr(session.release()), false, job.from_here);
+        make_linked_ptr(session.release()), false,
+        ConfigurationParams(), job.from_here);
     pending_nudge_.reset(new SyncSessionJob(new_job));
 
     return;
@@ -428,11 +555,17 @@ void SyncScheduler::SaveJob(const SyncSessionJob& job) {
     DCHECK(wait_interval_.get());
     DCHECK(mode_ == CONFIGURATION_MODE);
 
+    // Config params should always get set.
+    DCHECK(!job.config_params.ready_task.is_null());
     SyncSession* old = job.session.get();
     SyncSession* s(new SyncSession(session_context_, this, old->source(),
                                    old->routing_info(), old->workers()));
-    SyncSessionJob new_job(job.purpose, TimeTicks::Now(),
-                           make_linked_ptr(s), false, job.from_here);
+    SyncSessionJob new_job(job.purpose,
+                           TimeTicks::Now(),
+                           make_linked_ptr(s),
+                           false,
+                           job.config_params,
+                           job.from_here);
     wait_interval_->pending_configure_job.reset(new SyncSessionJob(new_job));
   } // drop the rest.
   // TODO(sync): Is it okay to drop the rest?  It's weird that
@@ -451,23 +584,16 @@ struct ModelSafeWorkerGroupIs {
 
 void SyncScheduler::ClearUserData() {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  SyncSessionJob job(SyncSessionJob::CLEAR_USER_DATA, TimeTicks::Now(),
+  SyncSessionJob job(SyncSessionJob::CLEAR_USER_DATA,
+                     TimeTicks::Now(),
                      make_linked_ptr(CreateSyncSession(SyncSourceInfo())),
                      false,
+                     ConfigurationParams(),
                      FROM_HERE);
 
   DoSyncSessionJob(job);
 }
 
-void SyncScheduler::CleanupDisabledTypes() {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  SyncSessionJob job(SyncSessionJob::CLEANUP_DISABLED_TYPES, TimeTicks::Now(),
-                     make_linked_ptr(CreateSyncSession(SyncSourceInfo())),
-                     false,
-                     FROM_HERE);
-  DoSyncSessionJob(job);
-}
-
 void SyncScheduler::ScheduleNudgeAsync(
     const TimeDelta& delay,
     NudgeSource source, ModelTypeSet types,
@@ -525,7 +651,7 @@ void SyncScheduler::ScheduleNudgeImpl(
   SyncSession* session(CreateSyncSession(info));
   SyncSessionJob job(SyncSessionJob::NUDGE, TimeTicks::Now() + delay,
                      make_linked_ptr(session), is_canary_job,
-                     nudge_location);
+                     ConfigurationParams(), nudge_location);
 
   session = NULL;
   if (!ShouldRunJob(job))
@@ -556,77 +682,6 @@ void SyncScheduler::ScheduleNudgeImpl(
   ScheduleSyncSessionJob(job);
 }
 
-// Helper to extract the routing info and workers corresponding to types in
-// |types| from |current_routes| and |current_workers|.
-void GetModelSafeParamsForTypes(ModelTypeSet types,
-    const ModelSafeRoutingInfo& current_routes,
-    const std::vector<ModelSafeWorker*>& current_workers,
-    ModelSafeRoutingInfo* result_routes,
-    std::vector<ModelSafeWorker*>* result_workers) {
-  bool passive_group_added = false;
-
-  typedef std::vector<ModelSafeWorker*>::const_iterator iter;
-  for (ModelTypeSet::Iterator it = types.First();
-       it.Good(); it.Inc()) {
-    const syncable::ModelType t = it.Get();
-    ModelSafeRoutingInfo::const_iterator route = current_routes.find(t);
-    DCHECK(route != current_routes.end());
-    ModelSafeGroup group = route->second;
-
-    (*result_routes)[t] = group;
-    iter w_tmp_it = std::find_if(current_workers.begin(), current_workers.end(),
-                                 ModelSafeWorkerGroupIs(group));
-    if (w_tmp_it != current_workers.end()) {
-      iter result_workers_it = std::find_if(
-          result_workers->begin(), result_workers->end(),
-          ModelSafeWorkerGroupIs(group));
-      if (result_workers_it == result_workers->end())
-        result_workers->push_back(*w_tmp_it);
-
-      if (group == GROUP_PASSIVE)
-        passive_group_added = true;
-    } else {
-        NOTREACHED();
-    }
-  }
-
-  // Always add group passive.
-  if (passive_group_added == false) {
-    iter it = std::find_if(current_workers.begin(), current_workers.end(),
-                           ModelSafeWorkerGroupIs(GROUP_PASSIVE));
-    if (it != current_workers.end())
-      result_workers->push_back(*it);
-    else
-      NOTREACHED();
-  }
-}
-
-void SyncScheduler::ScheduleConfiguration(
-    ModelTypeSet types,
-    GetUpdatesCallerInfo::GetUpdatesSource source) {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  DCHECK(IsConfigRelatedUpdateSourceValue(source));
-  SDVLOG(2) << "Scheduling a config";
-
-  ModelSafeRoutingInfo routes;
-  std::vector<ModelSafeWorker*> workers;
-  GetModelSafeParamsForTypes(types,
-                             session_context_->routing_info(),
-                             session_context_->workers(),
-                             &routes, &workers);
-
-  SyncSession* session = new SyncSession(session_context_, this,
-      SyncSourceInfo(source,
-                     syncable::ModelTypePayloadMapFromRoutingInfo(
-                         routes, std::string())),
-      routes, workers);
-  SyncSessionJob job(SyncSessionJob::CONFIGURATION, TimeTicks::Now(),
-                     make_linked_ptr(session),
-                     false,
-                     FROM_HERE);
-  DoSyncSessionJob(job);
-}
-
 const char* SyncScheduler::GetModeString(SyncScheduler::Mode mode) {
   switch (mode) {
     ENUM_CASE(CONFIGURATION_MODE);
@@ -829,6 +884,11 @@ void SyncScheduler::FinishSyncSessionJob(const SyncSessionJob& job) {
     // here; see DCHECKs in SaveJob().  (See http://crbug.com/90868.)
     SaveJob(job);
     return;  // Nothing to do.
+  } else if (job.session->Succeeded() &&
+             !job.config_params.ready_task.is_null()) {
+    // If this was a configuration job with a ready task, invoke it now that
+    // we finished successfully.
+    job.config_params.ready_task.Run();
   }
 
   SDVLOG(2) << "Updating the next polling time after SyncMain";
@@ -931,11 +991,15 @@ void SyncScheduler::HandleContinuationError(
   wait_interval_.reset(new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF,
                                         length));
   if (old_job.purpose == SyncSessionJob::CONFIGURATION) {
+    SDVLOG(2) << "Configuration did not succeed, scheduling retry.";
+    // Config params should always get set.
+    DCHECK(!old_job.config_params.ready_task.is_null());
     SyncSession* old = old_job.session.get();
     SyncSession* s(new SyncSession(session_context_, this,
         old->source(), old->routing_info(), old->workers()));
     SyncSessionJob job(old_job.purpose, TimeTicks::Now() + length,
-                        make_linked_ptr(s), false, FROM_HERE);
+                       make_linked_ptr(s), false, old_job.config_params,
+                       FROM_HERE);
     wait_interval_->pending_configure_job.reset(new SyncSessionJob(job));
   } else {
     // We are not in configuration mode. So wait_interval's pending job
@@ -1057,6 +1121,7 @@ void SyncScheduler::PollTimerCallback() {
   SyncSessionJob job(SyncSessionJob::POLL, TimeTicks::Now(),
                      make_linked_ptr(s),
                      false,
+                     ConfigurationParams(),
                      FROM_HERE);
 
   ScheduleSyncSessionJob(job);
diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h
index d32ee25..8ebd2aa 100644
--- a/sync/engine/sync_scheduler.h
+++ b/sync/engine/sync_scheduler.h
@@ -37,6 +37,32 @@ namespace browser_sync {
 
 struct ServerConnectionEvent;
 
+struct ConfigurationParams {
+  enum KeystoreKeyStatus {
+    KEYSTORE_KEY_UNNECESSARY,
+    KEYSTORE_KEY_NEEDED
+  };
+  ConfigurationParams();
+  ConfigurationParams(
+      const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource& source,
+      const syncable::ModelTypeSet& types_to_download,
+      const browser_sync::ModelSafeRoutingInfo& routing_info,
+      KeystoreKeyStatus keystore_key_status,
+      const base::Closure& ready_task);
+  ~ConfigurationParams();
+
+  // Source for the configuration.
+  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source;
+  // The types that should be downloaded.
+  syncable::ModelTypeSet types_to_download;
+  // The new routing info (superset of types to be downloaded).
+  ModelSafeRoutingInfo routing_info;
+  // Whether we need to perform a GetKey command.
+  KeystoreKeyStatus keystore_key_status;
+  // Callback to invoke on configuration completion.
+  base::Closure ready_task;
+};
+
 class SyncScheduler : public sessions::SyncSession::Delegate {
  public:
   enum Mode {
@@ -63,10 +89,15 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
 
   // Start the scheduler with the given mode.  If the scheduler is
   // already started, switch to the given mode, although some
-  // scheduled tasks from the old mode may still run.  If non-NULL,
-  // |callback| will be invoked when the mode has been changed to
-  // |mode|.  Takes ownership of |callback|.
-  void Start(Mode mode, const base::Closure& callback);
+  // scheduled tasks from the old mode may still run.
+  virtual void Start(Mode mode);
+
+  // Schedules the configuration task specified by |params|. Returns true if
+  // the configuration task executed immediately, false if it had to be
+  // scheduled for a later attempt. |params.ready_task| is invoked whenever the
+  // configuration task executes.
+  // Note: must already be in CONFIGURATION mode.
+  virtual bool ScheduleConfiguration(const ConfigurationParams& params);
 
   // Request that any running syncer task stop as soon as possible and
   // cancel all scheduled tasks. This function can be called from any thread,
@@ -86,14 +117,6 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
       const syncable::ModelTypePayloadMap& types_with_payloads,
       const tracked_objects::Location& nudge_location);
 
-  // Schedule a configuration cycle. May execute immediately or at a later time
-  // (depending on backoff/throttle state).
-  // Note: The source argument of this function must come from the subset of
-  // GetUpdatesCallerInfo values related to configurations.
-  void ScheduleConfiguration(
-      syncable::ModelTypeSet types,
-      sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source);
-
   // TODO(tim): remove this. crbug.com/131336
   void ClearUserData();
 
@@ -164,6 +187,7 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
     SyncSessionJob();
     SyncSessionJob(SyncSessionJobPurpose purpose, base::TimeTicks start,
         linked_ptr<sessions::SyncSession> session, bool is_canary_job,
+        ConfigurationParams config_params,
         const tracked_objects::Location& nudge_location);
     ~SyncSessionJob();
     static const char* GetPurposeString(SyncSessionJobPurpose purpose);
@@ -172,6 +196,7 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
     base::TimeTicks scheduled_start;
     linked_ptr<sessions::SyncSession> session;
     bool is_canary_job;
+    ConfigurationParams config_params;
 
     // This is the location the job came from.  Used for debugging.
     // In case of multiple nudges getting coalesced this stores the
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index a6d8059..7aa30ab 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -12,6 +12,7 @@
 #include "sync/engine/syncer.h"
 #include "sync/engine/throttled_data_type_tracker.h"
 #include "sync/sessions/test_util.h"
+#include "sync/test/callback_counter.h"
 #include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/mock_connection_manager.h"
 #include "sync/test/engine/test_directory_setter_upper.h"
@@ -27,6 +28,7 @@ using testing::DoAll;
 using testing::Eq;
 using testing::Invoke;
 using testing::Mock;
+using testing::Not;
 using testing::Return;
 using testing::WithArg;
 
@@ -69,6 +71,14 @@ void PumpLoop() {
   RunLoop();
 }
 
+ModelSafeRoutingInfo TypesToRoutingInfo(const ModelTypeSet& types) {
+  ModelSafeRoutingInfo routes;
+  for (ModelTypeSet::Iterator iter = types.First(); iter.Good(); iter.Inc()) {
+    routes[iter.Get()] = GROUP_PASSIVE;
+  }
+  return routes;
+}
+
 // Convenient to use in tests wishing to analyze SyncShare calls over time.
 static const size_t kMinNumSamples = 5;
 class SyncSchedulerTest : public testing::Test {
@@ -152,7 +162,7 @@ class SyncSchedulerTest : public testing::Test {
   }
 
   void StartSyncScheduler(SyncScheduler::Mode mode) {
-    scheduler()->Start(mode, base::Closure());
+    scheduler()->Start(mode);
   }
 
   // This stops the scheduler synchronously.
@@ -291,14 +301,23 @@ TEST_F(SyncSchedulerTest, Config) {
   SyncShareRecords records;
   const ModelTypeSet model_types(syncable::BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+  EXPECT_CALL(*syncer(),
+              SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  scheduler()->ScheduleConfiguration(
-      model_types, GetUpdatesCallerInfo::RECONFIGURATION);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      model_types,
+      TypesToRoutingInfo(model_types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(1, counter.times_called());
 
   ASSERT_EQ(1U, records.snapshots.size());
   EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types,
@@ -315,7 +334,9 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   SyncShareRecords records;
   const ModelTypeSet model_types(syncable::BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+  EXPECT_CALL(*syncer(),
+              SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                       WithArg<0>(RecordSyncShare(&records))))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
@@ -324,61 +345,27 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   ASSERT_EQ(0U, records.snapshots.size());
-  scheduler()->ScheduleConfiguration(
-      model_types, GetUpdatesCallerInfo::RECONFIGURATION);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      model_types,
+      TypesToRoutingInfo(model_types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(0, counter.times_called());
 
   ASSERT_EQ(1U, records.snapshots.size());
   RunLoop();
 
   ASSERT_EQ(2U, records.snapshots.size());
+  ASSERT_EQ(1, counter.times_called());
   EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types,
       records.snapshots[1].source().types));
   EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
             records.snapshots[1].source().updates_source);
 }
 
-// Issue 2 config commands. Second one right after the first has failed
-// and make sure LATEST is executed.
-TEST_F(SyncSchedulerTest, MultipleConfigWithBackingOff) {
-  const ModelTypeSet
-      model_types1(syncable::BOOKMARKS),
-      model_types2(syncable::AUTOFILL);
-  UseMockDelayProvider();
-  EXPECT_CALL(*delay(), GetDelay(_))
-      .WillRepeatedly(Return(TimeDelta::FromMilliseconds(30)));
-  SyncShareRecords records;
-
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      WithArg<0>(RecordSyncShare(&records))))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      WithArg<0>(RecordSyncShare(&records))))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
-                      WithArg<0>(RecordSyncShare(&records))));
-
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
-
-  ASSERT_EQ(0U, records.snapshots.size());
-  scheduler()->ScheduleConfiguration(
-      model_types1, GetUpdatesCallerInfo::RECONFIGURATION);
-
-  ASSERT_EQ(1U, records.snapshots.size());
-  scheduler()->ScheduleConfiguration(
-      model_types2, GetUpdatesCallerInfo::RECONFIGURATION);
-
-  // A canary job gets posted when we go into exponential backoff.
-  RunLoop();
-
-  ASSERT_EQ(2U, records.snapshots.size());
-  RunLoop();
-
-  ASSERT_EQ(3U, records.snapshots.size());
-  EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types2,
-      records.snapshots[2].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
-            records.snapshots[2].source().updates_source);
-}
-
 // Issue a nudge when the config has failed. Make sure both the config and
 // nudge are executed.
 TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
@@ -401,19 +388,28 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   ASSERT_EQ(0U, records.snapshots.size());
-  scheduler()->ScheduleConfiguration(
-      model_types, GetUpdatesCallerInfo::RECONFIGURATION);
-
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      model_types,
+      TypesToRoutingInfo(model_types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(0, counter.times_called());
   ASSERT_EQ(1U, records.snapshots.size());
+
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, model_types, FROM_HERE);
   RunLoop();
-
   ASSERT_EQ(2U, records.snapshots.size());
+  ASSERT_EQ(0, counter.times_called());
+
   RunLoop();
+  ASSERT_EQ(3U, records.snapshots.size());
+  ASSERT_EQ(1, counter.times_called());
 
   // Now change the mode so nudge can execute.
-  ASSERT_EQ(3U, records.snapshots.size());
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   ASSERT_EQ(4U, records.snapshots.size());
@@ -718,13 +714,19 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
-// Test that no syncing occurs when throttled.
+// Test that no syncing occurs when throttled (although CleanupDisabledTypes
+// is allowed).
 TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   const ModelTypeSet types(syncable::BOOKMARKS);
   TimeDelta poll(TimeDelta::FromMilliseconds(5));
   TimeDelta throttle(TimeDelta::FromMinutes(10));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+  EXPECT_CALL(*syncer(), SyncShare(_,Not(CLEANUP_DISABLED_TYPES),
+                                     Not(CLEANUP_DISABLED_TYPES)))
       .WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
@@ -736,8 +738,15 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  scheduler()->ScheduleConfiguration(
-      types, GetUpdatesCallerInfo::RECONFIGURATION);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      types,
+      TypesToRoutingInfo(types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(0, counter.times_called());
 }
 
 TEST_F(SyncSchedulerTest, ThrottlingExpires) {
@@ -770,6 +779,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareRecords records;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
@@ -783,8 +793,15 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
 
   const ModelTypeSet config_types(syncable::BOOKMARKS);
 
-  scheduler()->ScheduleConfiguration(
-      config_types, GetUpdatesCallerInfo::RECONFIGURATION);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      config_types,
+      TypesToRoutingInfo(config_types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(1, counter.times_called());
 
   ASSERT_EQ(1U, records.snapshots.size());
   EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(config_types,
@@ -853,7 +870,7 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   UseMockDelayProvider();
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1)
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                             RecordSyncShareMultiple(&r, 1U)));
   EXPECT_CALL(*delay(), GetDelay(_)).
@@ -895,9 +912,15 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  scheduler()->CleanupDisabledTypes();
-  scheduler()->ScheduleConfiguration(
-      types, GetUpdatesCallerInfo::RECONFIGURATION);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      types,
+      TypesToRoutingInfo(types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(0, counter.times_called());
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
@@ -1066,28 +1089,49 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
-  // Configuration.
+  // Configuration (always includes a cleanup disabled types).
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
       .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  scheduler()->ScheduleConfiguration(
-      ModelTypeSet(), GetUpdatesCallerInfo::RECONFIGURATION);
-
+  syncable::ModelTypeSet model_types(syncable::BOOKMARKS);
+  CallbackCounter counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      model_types,
+      TypesToRoutingInfo(model_types),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+  ASSERT_EQ(1, counter.times_called());
+  // Runs directly so no need to pump the loop.
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
-  // Cleanup disabled types.
+  // Cleanup disabled types. Because no types are being configured, we just
+  // perform the cleanup.
   EXPECT_CALL(*syncer(),
-              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
-
-  scheduler()->CleanupDisabledTypes();
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES)).
+      WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
+  CallbackCounter counter2;
+  ConfigurationParams params2(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      ModelTypeSet(),
+      ModelSafeRoutingInfo(),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter2)));
+  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params2));
+  ASSERT_EQ(1, counter2.times_called());
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
   // Poll.
   EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
       .Times(AtLeast(1))
diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc
index 34ff7d9..390a6cd 100644
--- a/sync/engine/sync_scheduler_whitebox_unittest.cc
+++ b/sync/engine/sync_scheduler_whitebox_unittest.cc
@@ -107,6 +107,7 @@ class SyncSchedulerWhiteboxTest : public testing::Test {
     SyncScheduler::SyncSessionJob job(purpose, TimeTicks::Now(),
          make_linked_ptr(s),
          false,
+         ConfigurationParams(),
          FROM_HERE);
     return DecideOnJob(job);
   }
@@ -160,6 +161,7 @@ TEST_F(SyncSchedulerWhiteboxTest, SaveNudgeWhileTypeThrottled) {
                                     TimeTicks::Now(),
                                     make_linked_ptr(s),
                                     false,
+                                    ConfigurationParams(),
                                     FROM_HERE);
 
   SyncScheduler::JobProcessDecision decision = DecideOnJob(job);
diff --git a/sync/internal_api/sync_manager.cc b/sync/internal_api/sync_manager.cc
index 8a2ff70..4cb5127 100644
--- a/sync/internal_api/sync_manager.cc
+++ b/sync/internal_api/sync_manager.cc
@@ -59,6 +59,7 @@
 
 using base::TimeDelta;
 using browser_sync::AllStatus;
+using browser_sync::ConfigurationParams;
 using browser_sync::Cryptographer;
 using browser_sync::Encryptor;
 using browser_sync::JsArgList;
@@ -397,6 +398,8 @@ class SyncManager::SyncInternal
       const std::string& name, const JsArgList& args,
       const WeakHandle<JsReplyHandler>& reply_handler) OVERRIDE;
 
+  void SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler);
+
  private:
   struct NotificationInfo {
     int total_count;
@@ -816,46 +819,44 @@ 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());
-  if (!data_->scheduler()) {
-    LOG(INFO)
-        << "SyncManager::RequestConfig: bailing out because scheduler is "
-        << "null";
-    return;
-  }
-  StartConfigurationMode(base::Closure());
-  data_->session_context()->set_routing_info(routing_info);
-  data_->scheduler()->ScheduleConfiguration(types, GetSourceFromReason(reason));
-}
+  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). crbug.com/129665.
+  ConfigurationParams::KeystoreKeyStatus keystore_key_status =
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY;
+
+  ConfigurationParams params(GetSourceFromReason(reason),
+                             types_to_config,
+                             new_routing_info,
+                             keystore_key_status,
+                             ready_task);
 
-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";
+        << "SyncManager::ConfigureSyncer: could not configure because "
+        << "scheduler is null";
+    params.ready_task.Run();
     return;
   }
-  data_->scheduler()->Start(
-      browser_sync::SyncScheduler::CONFIGURATION_MODE, callback);
+
+  data_->scheduler()->Start(SyncScheduler::CONFIGURATION_MODE);
+  if (!data_->scheduler()->ScheduleConfiguration(params))
+    retry_task.Run();
 }
 
 bool SyncManager::SyncInternal::Init(
@@ -941,8 +942,7 @@ bool SyncManager::SyncInternal::Init(
 
   if (signed_in) {
     if (scheduler()) {
-      scheduler()->Start(
-          browser_sync::SyncScheduler::CONFIGURATION_MODE, base::Closure());
+      scheduler()->Start(SyncScheduler::CONFIGURATION_MODE);
     }
 
     initialized_ = true;
@@ -1116,9 +1116,13 @@ void SyncManager::SyncInternal::NotifyCryptographerState(
 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): We always want the newest set of routes when we switch back
+    // to normal mode. Figure out how to enforce set_routing_info is always
+    // appropriately set and that it's only modified when switching to normal
+    // mode.
     session_context()->set_routing_info(routing_info);
-    scheduler()->Start(SyncScheduler::NORMAL_MODE, base::Closure());
+    scheduler()->Start(SyncScheduler::NORMAL_MODE);
   }
 }
 
@@ -2353,6 +2357,11 @@ void SyncManager::SyncInternal::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
+void SyncManager::SyncInternal::SetSyncSchedulerForTest(
+    scoped_ptr<SyncScheduler> sync_scheduler) {
+  scheduler_ = sync_scheduler.Pass();
+}
+
 SyncStatus SyncManager::GetDetailedStatus() const {
   return data_->GetStatus();
 }
@@ -2383,6 +2392,10 @@ TimeDelta SyncManager::GetNudgeDelayTimeDelta(
   return data_->GetNudgeDelayTimeDelta(model_type);
 }
 
+void SyncManager::SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler) {
+  data_->SetSyncSchedulerForTest(scheduler.Pass());
+}
+
 syncable::ModelTypeSet SyncManager::GetEncryptedDataTypesForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return GetEncryptedTypes(&trans);
diff --git a/sync/internal_api/sync_manager.h b/sync/internal_api/sync_manager.h
index 04594fd..04181fe 100644
--- a/sync/internal_api/sync_manager.h
+++ b/sync/internal_api/sync_manager.h
@@ -26,11 +26,13 @@
 #include "sync/util/weak_handle.h"
 
 namespace browser_sync {
+struct ConfigurationParams;
 class Encryptor;
 struct Experiments;
 class ExtensionsActivityMonitor;
 class JsBackend;
 class JsEventHandler;
+class SyncScheduler;
 
 namespace sessions {
 class SyncSessionSnapshot;
@@ -431,24 +433,24 @@ class SyncManager {
   // error to call this when we don't have pending keys.
   void SetDecryptionPassphrase(const std::string& passphrase);
 
-  // Puts the SyncScheduler into a mode where no normal nudge or poll traffic
-  // will occur, but calls to RequestConfig will be supported.  If |callback|
-  // is provided, it will be invoked (from the internal SyncScheduler) when
-  // the thread has changed to configuration mode.
-  void StartConfigurationMode(const base::Closure& callback);
-
-  // Switches the mode of operation to CONFIGURATION_MODE and
-  // schedules a config task to fetch updates for |types|.
-  void RequestConfig(const browser_sync::ModelSafeRoutingInfo& routing_info,
-                     const syncable::ModelTypeSet& types,
-                     sync_api::ConfigureReason reason);
-
-  void RequestCleanupDisabledTypes(
-      const browser_sync::ModelSafeRoutingInfo& routing_info);
-
   // Request a clearing of all data on the server
   void RequestClearServerData();
 
+  // Switches the mode of operation to CONFIGURATION_MODE and performs
+  // any configuration tasks needed as determined by the params. Once complete,
+  // syncer will remain in CONFIGURATION_MODE until StartSyncingNormally is
+  // called.
+  // |ready_task| is invoked when the configuration completes.
+  // |retry_task| is invoked if the configuration job could not immediately
+  //              execute. |ready_task| will still be called when it eventually
+  //              does finish.
+  void 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);
+
   // Adds a listener to be notified of sync events.
   // NOTE: It is OK (in fact, it's probably a good idea) to call this before
   // having received OnInitializationCompleted.
@@ -543,11 +545,17 @@ class SyncManager {
   static const FilePath::CharType kSyncDatabaseFilename[];
 
  private:
+  friend class SyncManagerTest;
   FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, NudgeDelayTest);
 
   // For unit tests.
   base::TimeDelta GetNudgeDelayTimeDelta(const syncable::ModelType& model_type);
 
+  // Set the internal scheduler for testing purposes.
+  // TODO(sync): Use dependency injection instead. crbug.com/133061
+  void SetSyncSchedulerForTest(
+      scoped_ptr<browser_sync::SyncScheduler> scheduler);
+
   base::ThreadChecker thread_checker_;
 
   // An opaque pointer to the nested private class.
diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/syncapi_unittest.cc
index 991aed6..6380db3 100644
--- a/sync/internal_api/syncapi_unittest.cc
+++ b/sync/internal_api/syncapi_unittest.cc
@@ -24,6 +24,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "sync/engine/nigori_util.h"
+#include "sync/engine/sync_scheduler.h"
 #include "sync/internal_api/change_record.h"
 #include "sync/internal_api/http_post_provider_factory.h"
 #include "sync/internal_api/http_post_provider_interface.h"
@@ -54,6 +55,7 @@
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/syncable.h"
 #include "sync/syncable/syncable_id.h"
+#include "sync/test/callback_counter.h"
 #include "sync/test/fake_encryptor.h"
 #include "sync/test/fake_extensions_activity_monitor.h"
 #include "sync/util/cryptographer.h"
@@ -64,6 +66,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ExpectDictStringValue;
+using browser_sync::CallbackCounter;
+using browser_sync::ConfigurationParams;
 using browser_sync::Cryptographer;
 using browser_sync::FakeEncryptor;
 using browser_sync::FakeExtensionsActivityMonitor;
@@ -80,6 +84,7 @@ using browser_sync::MockJsReplyHandler;
 using browser_sync::ModelSafeRoutingInfo;
 using browser_sync::ModelSafeWorker;
 using browser_sync::sessions::SyncSessionSnapshot;
+using browser_sync::SyncScheduler;
 using browser_sync::TestUnrecoverableErrorHandler;
 using browser_sync::WeakHandle;
 using syncable::IS_DEL;
@@ -92,8 +97,10 @@ using syncable::SPECIFICS;
 using testing::_;
 using testing::AnyNumber;
 using testing::AtLeast;
+using testing::DoAll;
 using testing::InSequence;
 using testing::Invoke;
+using testing::Return;
 using testing::SaveArg;
 using testing::StrictMock;
 
@@ -926,6 +933,10 @@ class SyncManagerTest : public testing::Test,
     return true;
   }
 
+  void SetScheduler(scoped_ptr<SyncScheduler> scheduler) {
+    sync_manager_.SetSyncSchedulerForTest(scheduler.Pass());
+  }
+
  private:
   // Needed by |sync_manager_|.
   MessageLoop message_loop_;
@@ -2507,4 +2518,82 @@ TEST_F(SyncManagerTest, SetPreviouslyEncryptedSpecifics) {
   }
 }
 
+namespace {
+
+class MockSyncScheduler : public SyncScheduler {
+ public:
+  MockSyncScheduler() : SyncScheduler("name", NULL, NULL) {}
+  virtual ~MockSyncScheduler() {}
+
+  MOCK_METHOD1(Start, void(SyncScheduler::Mode));
+  MOCK_METHOD1(ScheduleConfiguration, bool(const ConfigurationParams&));
+};
+
+}  // namespace
+
+// Test that the configuration params are properly created and sent to
+// ScheduleConfigure. No callback should be invoked.
+TEST_F(SyncManagerTest, BasicConfiguration) {
+  ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
+  syncable::ModelTypeSet types_to_download(syncable::BOOKMARKS,
+                                           syncable::PREFERENCES);
+  browser_sync::ModelSafeRoutingInfo new_routing_info;
+  GetModelSafeRoutingInfo(&new_routing_info);
+
+  scoped_ptr<MockSyncScheduler> scheduler(new MockSyncScheduler());
+  ConfigurationParams params;
+  EXPECT_CALL(*scheduler, Start(SyncScheduler::CONFIGURATION_MODE));
+  EXPECT_CALL(*scheduler, ScheduleConfiguration(_)).
+      WillOnce(DoAll(SaveArg<0>(&params), Return(true)));
+  SetScheduler(scheduler.PassAs<SyncScheduler>());
+
+  CallbackCounter ready_task_counter, retry_task_counter;
+  sync_manager_.ConfigureSyncer(
+      reason,
+      types_to_download,
+      new_routing_info,
+      base::Bind(&CallbackCounter::Callback,
+                 base::Unretained(&ready_task_counter)),
+      base::Bind(&CallbackCounter::Callback,
+                 base::Unretained(&retry_task_counter)));
+  EXPECT_EQ(0, ready_task_counter.times_called());
+  EXPECT_EQ(0, retry_task_counter.times_called());
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
+            params.source);
+  EXPECT_TRUE(types_to_download.Equals(params.types_to_download));
+  EXPECT_EQ(new_routing_info, params.routing_info);
+}
+
+// Test that the retry callback is invoked on configuration failure.
+TEST_F(SyncManagerTest, ConfigurationRetry) {
+  ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
+  syncable::ModelTypeSet types_to_download(syncable::BOOKMARKS,
+                                           syncable::PREFERENCES);
+  browser_sync::ModelSafeRoutingInfo new_routing_info;
+  GetModelSafeRoutingInfo(&new_routing_info);
+
+  scoped_ptr<MockSyncScheduler> scheduler(new MockSyncScheduler());
+  ConfigurationParams params;
+  EXPECT_CALL(*scheduler, Start(SyncScheduler::CONFIGURATION_MODE));
+  EXPECT_CALL(*scheduler, ScheduleConfiguration(_)).
+      WillOnce(DoAll(SaveArg<0>(&params), Return(false)));
+  SetScheduler(scheduler.PassAs<SyncScheduler>());
+
+  CallbackCounter ready_task_counter, retry_task_counter;
+  sync_manager_.ConfigureSyncer(
+      reason,
+      types_to_download,
+      new_routing_info,
+      base::Bind(&CallbackCounter::Callback,
+                 base::Unretained(&ready_task_counter)),
+      base::Bind(&CallbackCounter::Callback,
+                 base::Unretained(&retry_task_counter)));
+  EXPECT_EQ(0, ready_task_counter.times_called());
+  EXPECT_EQ(1, retry_task_counter.times_called());
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
+            params.source);
+  EXPECT_TRUE(types_to_download.Equals(params.types_to_download));
+  EXPECT_EQ(new_routing_info, params.routing_info);
+}
+
 }  // namespace browser_sync
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 73e103c..5372b9d 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -375,6 +375,7 @@
         'sessions/test_util.h',
         'syncable/syncable_mock.cc',
         'syncable/syncable_mock.h',
+        'test/callback_counter.h',
         'test/engine/fake_model_worker.cc',
         'test/engine/fake_model_worker.h',
         'test/engine/mock_connection_manager.cc',
diff --git a/sync/test/callback_counter.h b/sync/test/callback_counter.h
new file mode 100644
index 0000000..71586ec
--- /dev/null
+++ b/sync/test/callback_counter.h
@@ -0,0 +1,29 @@
+// 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.
+
+#ifndef SYNC_TEST_CALLBACK_COUNTER_H_
+#define SYNC_TEST_CALLBACK_COUNTER_H_
+#pragma once
+
+namespace browser_sync {
+
+// Helper class to track how many times a callback is triggered.
+class CallbackCounter {
+ public:
+  CallbackCounter() { Reset(); }
+  ~CallbackCounter() {}
+
+  void Reset() { times_called_ = 0; }
+  void Callback() { ++times_called_; }
+  int times_called() const { return times_called_; }
+
+ private:
+  int times_called_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallbackCounter);
+};
+
+}  // namespace browser_sync
+
+#endif  // SYNC_TEST_CALLBACK_COUNTER_H_