Add a "download" step into the DataTypeManager

The download step invokies SBH::ConfigureDataTypes() and waits for the task callback.  Currently SBH::ConfigureDataTypes() is stubbed out, it will be implemented in the next change.

I also added some additional unit tests to the DTMImpl to make sure Stop() works while pasue and resume are pending, and to make sure the SYNC_CONFIGURE_DONE notification is always sent.

Review URL: http://codereview.chromium.org/1311001

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

6 files changed, 267 insertions, 27 deletions

diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc
index a166d11..4e9a4f1 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl.cc
@@ -51,7 +51,8 @@ DataTypeManagerImpl::DataTypeManagerImpl(
     : backend_(backend),
       controllers_(controllers),
       state_(DataTypeManager::STOPPED),
-      current_dtc_(NULL) {
+      current_dtc_(NULL),
+      download_ready_task_(NULL) {
   DCHECK(backend_);
   DCHECK_GT(arraysize(kStartOrder), 0U);
   // Ensure all data type controllers are stopped.
@@ -65,6 +66,11 @@ DataTypeManagerImpl::DataTypeManagerImpl(
     start_order_[kStartOrder[i]] = i;
 }
 
+DataTypeManagerImpl::~DataTypeManagerImpl() {
+  if (download_ready_task_)
+    download_ready_task_->Cancel();
+}
+
 void DataTypeManagerImpl::Configure(const TypeSet& desired_types) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   if (state_ == STOPPING) {
@@ -73,6 +79,7 @@ void DataTypeManagerImpl::Configure(const TypeSet& desired_types) {
     return;
   }
 
+  last_requested_types_ = desired_types;
   // Add any data type controllers into the needs_start_ list that are
   // currently NOT_RUNNING or STOPPING.
   needs_start_.clear();
@@ -146,7 +153,22 @@ void DataTypeManagerImpl::Restart() {
   }
   needs_stop_.clear();
 
-  // TODO(sync): Get updates for new data types here.
+  // Tell the backend about the new set of data types we wish to sync.
+  // The task will be invoked when updated are downloaded.
+  state_ = DOWNLOAD_PENDING;
+  download_ready_task_ = new DownloadReadyTask(this);
+  backend_->ConfigureDataTypes(last_requested_types_, download_ready_task_);
+}
+
+void DataTypeManagerImpl::DownloadReady() {
+  DCHECK(state_ == DOWNLOAD_PENDING || state_ == RESTARTING);
+  download_ready_task_ = NULL;
+
+  // If we had a restart while waiting for downloads, just restart.
+  if (state_ == RESTARTING) {
+    Restart();
+    return;
+  }
 
   // Pause the sync backend before starting the data types.
   state_ = PAUSE_PENDING;
@@ -198,8 +220,7 @@ void DataTypeManagerImpl::TypeStartCallback(
   // was starting.  Now that it has finished starting, we can finish
   // stopping the DataTypeManager.  This is considered an ABORT.
   if (state_ == STOPPING) {
-    FinishStop();
-    NotifyDone(ABORTED);
+    FinishStopAndNotify(ABORTED);
     return;
   }
 
@@ -222,7 +243,6 @@ void DataTypeManagerImpl::TypeStartCallback(
   // managed to start up to this point and pass the result to the
   // callback.
   LOG(INFO) << "Failed " << started_dtc->name();
-  FinishStop();
   ConfigureResult configure_result = DataTypeManager::ABORTED;
   switch (result) {
     case DataTypeController::ABORTED:
@@ -238,7 +258,7 @@ void DataTypeManagerImpl::TypeStartCallback(
       NOTREACHED();
       break;
   }
-  NotifyDone(configure_result);
+  FinishStopAndNotify(configure_result);
 }
 
 void DataTypeManagerImpl::Stop() {
@@ -257,13 +277,28 @@ void DataTypeManagerImpl::Stop() {
 
   // If Stop() is called while waiting for pause or resume, we no
   // longer care about this.
-  if (state_ == PAUSE_PENDING)
+  bool aborted = false;
+  if (state_ == PAUSE_PENDING) {
     RemoveObserver(NotificationType::SYNC_PAUSED);
-  if (state_ == RESUME_PENDING)
+    aborted = true;
+  }
+  if (state_ == RESUME_PENDING) {
     RemoveObserver(NotificationType::SYNC_RESUMED);
+    aborted = true;
+  }
+
+  // If Stop() is called while waiting for download, cancel the task.
+  if (state_ == DOWNLOAD_PENDING) {
+    download_ready_task_->Cancel();
+    download_ready_task_ = NULL;
+    aborted = true;
+  }
 
   state_ = STOPPING;
-  FinishStop();
+  if (aborted)
+    FinishStopAndNotify(ABORTED);
+  else
+    FinishStop();
 }
 
 void DataTypeManagerImpl::FinishStop() {
@@ -283,6 +318,11 @@ void DataTypeManagerImpl::FinishStop() {
   state_ = STOPPED;
 }
 
+void DataTypeManagerImpl::FinishStopAndNotify(ConfigureResult result) {
+  FinishStop();
+  NotifyDone(result);
+}
+
 void DataTypeManagerImpl::Observe(NotificationType type,
                                   const NotificationSource& source,
                                   const NotificationDetails& details) {
@@ -349,8 +389,7 @@ void DataTypeManagerImpl::ResumeSyncer() {
   AddObserver(NotificationType::SYNC_RESUMED);
   if (!backend_->RequestResume()) {
     RemoveObserver(NotificationType::SYNC_RESUMED);
-    FinishStop();
-    NotifyDone(UNRECOVERABLE_ERROR);
+    FinishStopAndNotify(UNRECOVERABLE_ERROR);
   }
 }
 
@@ -358,8 +397,7 @@ void DataTypeManagerImpl::PauseSyncer() {
   AddObserver(NotificationType::SYNC_PAUSED);
   if (!backend_->RequestPause()) {
     RemoveObserver(NotificationType::SYNC_PAUSED);
-    FinishStop();
-    NotifyDone(UNRECOVERABLE_ERROR);
+    FinishStopAndNotify(UNRECOVERABLE_ERROR);
   }
 }
 
diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h
index b8be7715..ef4ecba 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.h
+++ b/chrome/browser/sync/glue/data_type_manager_impl.h
@@ -12,6 +12,7 @@
 
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
+#include "base/task.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_type.h"
@@ -29,7 +30,7 @@ class DataTypeManagerImpl : public DataTypeManager,
  public:
   DataTypeManagerImpl(SyncBackendHost* backend,
                       const DataTypeController::TypeMap& controllers);
-  virtual ~DataTypeManagerImpl() {}
+  virtual ~DataTypeManagerImpl();
 
   // DataTypeManager interface.
   virtual void Configure(const TypeSet& desired_types);
@@ -50,6 +51,24 @@ class DataTypeManagerImpl : public DataTypeManager,
                        const NotificationDetails& details);
 
  private:
+  // This task is used to handle the "download ready" callback from
+  // the SyncBackendHost in response to our ConfigureDataTypes() call.
+  // We don't use a raw callback here so we can handle the case where
+  // this instance gets destroyed before the callback is invoked.
+  class DownloadReadyTask : public CancelableTask {
+   public:
+    explicit DownloadReadyTask(DataTypeManagerImpl* dtm) : dtm_(dtm) {}
+    virtual void Run() {
+      if (dtm_)
+        dtm_->DownloadReady();
+    }
+    virtual void Cancel() {
+      dtm_ = NULL;
+    }
+   private:
+    DataTypeManagerImpl* dtm_;
+  };
+
   // Starts the next data type in the kStartOrder list, indicated by
   // the current_type_ member.  If there are no more data types to
   // start, the stashed start_callback_ is invoked.
@@ -60,8 +79,10 @@ class DataTypeManagerImpl : public DataTypeManager,
 
   // Stops all data types.
   void FinishStop();
+  void FinishStopAndNotify(ConfigureResult result);
 
   void Restart();
+  void DownloadReady();
   void AddObserver(NotificationType type);
   void RemoveObserver(NotificationType type);
   void NotifyStart();
@@ -75,7 +96,9 @@ class DataTypeManagerImpl : public DataTypeManager,
   DataTypeController::TypeMap controllers_;
   State state_;
   DataTypeController* current_dtc_;
+  CancelableTask* download_ready_task_;
   std::map<syncable::ModelType, int> start_order_;
+  TypeSet last_requested_types_;
   std::vector<DataTypeController*> needs_start_;
   std::vector<DataTypeController*> needs_stop_;
 
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 d17025b..43718b8 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
@@ -89,6 +89,13 @@ class DataTypeManagerImplTest : public testing::Test {
         WillRepeatedly(Return(DataTypeController::NOT_RUNNING));
   }
 
+  void SetNotUsedExpectations(DataTypeControllerMock* mock_dtc) {
+    EXPECT_CALL(*mock_dtc, Start(_, _)).Times(0);
+    EXPECT_CALL(*mock_dtc, Stop()).Times(0);
+    EXPECT_CALL(*mock_dtc, state()).
+        WillRepeatedly(Return(DataTypeController::NOT_RUNNING));
+  }
+
   void SetConfigureStartExpectation() {
     EXPECT_CALL(
         observer_,
@@ -104,6 +111,12 @@ class DataTypeManagerImplTest : public testing::Test {
                          Pointee(result))));
   }
 
+  void SetBackendExpectations(int times) {
+    EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(times);
+    EXPECT_CALL(backend_, RequestPause()).Times(times);
+    EXPECT_CALL(backend_, RequestResume()).Times(times);
+  }
+
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
   DataTypeController::TypeMap controllers_;
@@ -127,8 +140,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOne) {
   DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
   SetStartStopExpectations(bookmark_dtc);
   controllers_[syncable::BOOKMARKS] = bookmark_dtc;
-  EXPECT_CALL(backend_, RequestPause()).Times(1);
-  EXPECT_CALL(backend_, RequestResume()).Times(1);
+  SetBackendExpectations(1);
   DataTypeManagerImpl dtm(&backend_, controllers_);
   types_.insert(syncable::BOOKMARKS);
   SetConfigureStartExpectation();
@@ -147,8 +159,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneThenAnother) {
   SetStartStopExpectations(preference_dtc);
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
-  EXPECT_CALL(backend_, RequestPause()).Times(2);
-  EXPECT_CALL(backend_, RequestResume()).Times(2);
+  SetBackendExpectations(2);
   DataTypeManagerImpl dtm(&backend_, controllers_);
   types_.insert(syncable::BOOKMARKS);
 
@@ -175,8 +186,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneThenSwitch) {
   SetStartStopExpectations(preference_dtc);
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
-  EXPECT_CALL(backend_, RequestPause()).Times(2);
-  EXPECT_CALL(backend_, RequestResume()).Times(2);
+  SetBackendExpectations(2);
   DataTypeManagerImpl dtm(&backend_, controllers_);
   types_.insert(syncable::BOOKMARKS);
 
@@ -218,8 +228,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileOneInFlight) {
   SetStartStopExpectations(preference_dtc);
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
-  EXPECT_CALL(backend_, RequestPause()).Times(2);
-  EXPECT_CALL(backend_, RequestResume()).Times(2);
+  SetBackendExpectations(2);
   DataTypeManagerImpl dtm(&backend_, controllers_);
   types_.insert(syncable::BOOKMARKS);
 
@@ -249,6 +258,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhilePausePending) {
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
   // Don't notify the first time pause is called.
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(2);
   EXPECT_CALL(backend_, RequestPause()).
       WillOnce(Return(true)).
       WillOnce(DoDefault());
@@ -265,7 +275,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhilePausePending) {
   types_.insert(syncable::PREFERENCES);
   dtm.Configure(types_);
 
-  // Should still be PAUSE_PENDING.
+  // Should now be RESTARTING.
   EXPECT_EQ(DataTypeManager::RESTARTING, dtm.state());
 
   // Send the SYNC_PAUSED notification.  This will allow the DTM to
@@ -278,6 +288,33 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhilePausePending) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
 }
 
+TEST_F(DataTypeManagerImplTest, StopWhilePausePending) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  SetNotUsedExpectations(bookmark_dtc);
+  controllers_[syncable::BOOKMARKS] = bookmark_dtc;
+
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  // Never notify when RequestPause is called.
+  EXPECT_CALL(backend_, RequestPause()).WillOnce(Return(true));
+  EXPECT_CALL(backend_, RequestResume()).Times(0);
+  DataTypeManagerImpl dtm(&backend_, controllers_);
+
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::ABORTED);
+  types_.insert(syncable::BOOKMARKS);
+  dtm.Configure(types_);
+  EXPECT_EQ(DataTypeManager::PAUSE_PENDING, dtm.state());
+
+  // Stop while pause pending.
+  dtm.Stop();
+  EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
+
+  // We should be able to safely handle a SYNC_PAUSED notification.
+  NotificationService::current()->Notify(NotificationType::SYNC_PAUSED,
+                                         NotificationService::AllSources(),
+                                         NotificationService::NoDetails());
+}
+
 TEST_F(DataTypeManagerImplTest, ConfigureWhileResumePending) {
   DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
   SetStartStopExpectations(bookmark_dtc);
@@ -286,6 +323,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileResumePending) {
   SetStartStopExpectations(preference_dtc);
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(2);
   EXPECT_CALL(backend_, RequestPause()).Times(2);
   // Don't notify the first time resume is called.
   EXPECT_CALL(backend_, RequestResume()).
@@ -303,7 +341,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileResumePending) {
   types_.insert(syncable::PREFERENCES);
   dtm.Configure(types_);
 
-  // Should still be PAUSE_PENDING.
+  // Should now be RESTARTING.
   EXPECT_EQ(DataTypeManager::RESTARTING, dtm.state());
 
   // Send the SYNC_PAUSED notification.  This will allow the DTM to
@@ -316,6 +354,33 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileResumePending) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
 }
 
+TEST_F(DataTypeManagerImplTest, StopWhileResumePending) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  SetStartStopExpectations(bookmark_dtc);
+  controllers_[syncable::BOOKMARKS] = bookmark_dtc;
+
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, RequestPause()).Times(1);
+  // Never notify pause resumed.
+  EXPECT_CALL(backend_, RequestResume()).WillOnce(Return(true));
+  DataTypeManagerImpl dtm(&backend_, controllers_);
+
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::ABORTED);
+  types_.insert(syncable::BOOKMARKS);
+  dtm.Configure(types_);
+  EXPECT_EQ(DataTypeManager::RESUME_PENDING, dtm.state());
+
+  // Stop while pause pending.
+  dtm.Stop();
+  EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
+
+  // We should be able to safely handle a SYNC_RESUMED notification.
+  NotificationService::current()->Notify(NotificationType::SYNC_RESUMED,
+                                         NotificationService::AllSources(),
+                                         NotificationService::NoDetails());
+}
+
 TEST_F(DataTypeManagerImplTest, OneFailingController) {
   DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
   EXPECT_CALL(*bookmark_dtc, Start(true, _)).
@@ -328,6 +393,7 @@ TEST_F(DataTypeManagerImplTest, OneFailingController) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
 
@@ -336,7 +402,7 @@ TEST_F(DataTypeManagerImplTest, OneFailingController) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
 }
 
-TEST_F(DataTypeManagerImplTest, InterruptedStart) {
+TEST_F(DataTypeManagerImplTest, StopWhileInFlight) {
   DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
   SetStartStopExpectations(bookmark_dtc);
   controllers_[syncable::BOOKMARKS] = bookmark_dtc;
@@ -354,6 +420,7 @@ TEST_F(DataTypeManagerImplTest, InterruptedStart) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ABORTED);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
 
@@ -387,6 +454,7 @@ TEST_F(DataTypeManagerImplTest, SecondControllerFails) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
 
@@ -406,6 +474,7 @@ TEST_F(DataTypeManagerImplTest, PauseFailed) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
   EXPECT_CALL(backend_, RequestPause()).WillOnce(Return(false));
   EXPECT_CALL(backend_, RequestResume()).Times(0);
 
@@ -422,6 +491,7 @@ TEST_F(DataTypeManagerImplTest, ResumeFailed) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).WillOnce(Return(false));
 
@@ -429,3 +499,77 @@ TEST_F(DataTypeManagerImplTest, ResumeFailed) {
   dtm.Configure(types_);
   EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
 }
+
+TEST_F(DataTypeManagerImplTest, ConfigureWhileDownloadPending) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  SetStartStopExpectations(bookmark_dtc);
+  controllers_[syncable::BOOKMARKS] = bookmark_dtc;
+
+  DataTypeControllerMock* preference_dtc = MakePreferenceDTC();
+  SetStartStopExpectations(preference_dtc);
+  controllers_[syncable::PREFERENCES] = preference_dtc;
+
+  DataTypeManagerImpl dtm(&backend_, controllers_);
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::OK);
+  CancelableTask* task;
+  // Grab the task the first time this is called so we can configure
+  // before it is finished.
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).
+      WillOnce(SaveArg<1>(&task)).
+      WillOnce(DoDefault());
+  EXPECT_CALL(backend_, RequestPause()).Times(1);
+  EXPECT_CALL(backend_, RequestResume()).Times(1);
+
+  types_.insert(syncable::BOOKMARKS);
+  dtm.Configure(types_);
+  // Configure should stop in the DOWNLOAD_PENDING state because we
+  // are waiting for the download ready task to be run.
+  EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+
+  types_.insert(syncable::PREFERENCES);
+  dtm.Configure(types_);
+
+  // Should now be RESTARTING.
+  EXPECT_EQ(DataTypeManager::RESTARTING, dtm.state());
+
+  // Run the task, and this should finish the restart and eventually
+  // get us configured.
+  task->Run();
+  delete task;
+  EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state());
+
+  dtm.Stop();
+  EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
+}
+
+TEST_F(DataTypeManagerImplTest, StopWhileDownloadPending) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  SetNotUsedExpectations(bookmark_dtc);
+  controllers_[syncable::BOOKMARKS] = bookmark_dtc;
+
+  DataTypeManagerImpl dtm(&backend_, controllers_);
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::ABORTED);
+  CancelableTask* task;
+  // Grab the task the first time this is called so we can stop
+  // before it is finished.
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).
+      WillOnce(SaveArg<1>(&task));
+  EXPECT_CALL(backend_, RequestPause()).Times(0);
+  EXPECT_CALL(backend_, RequestResume()).Times(0);
+
+  types_.insert(syncable::BOOKMARKS);
+  dtm.Configure(types_);
+  // Configure should stop in the DOWNLOAD_PENDING state because we
+  // are waiting for the download ready task to be run.
+  EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm.state());
+
+  dtm.Stop();
+  EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
+
+  // It should be perfectly safe to run this task even though the DTM
+  // has been stopped.
+  task->Run();
+  delete task;
+}
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index ae3519d..e5326b7 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -5,6 +5,7 @@
 #include "build/build_config.h"
 #include "base/file_util.h"
 #include "base/file_version_info.h"
+#include "base/task.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/profile.h"
@@ -156,6 +157,17 @@ void SyncBackendHost::Shutdown(bool sync_disabled) {
   core_ = NULL;  // Releases reference to core_.
 }
 
+void SyncBackendHost::ConfigureDataTypes(
+    const std::set<syncable::ModelType>& types,
+    CancelableTask* ready_task) {
+  // TODO(skrul):
+  //   DCHECK for existing task
+  //   Update routing info to match the requested types.
+  //   Nudge the syncer.
+  ready_task->Run();
+  delete ready_task;
+}
+
 void SyncBackendHost::ActivateDataType(
     DataTypeController* data_type_controller,
     ChangeProcessor* change_processor) {
@@ -172,7 +184,7 @@ void SyncBackendHost::ActivateDataType(
 
   // Add the data type's change processor to the list of change
   // processors so it can receive updates.
-  DCHECK(processors_.count(type) == 0);
+  DCHECK_EQ(processors_.count(type), 0U);
   processors_[type] = change_processor;
 }
 
@@ -183,7 +195,7 @@ void SyncBackendHost::DeactivateDataType(
 
   std::map<syncable::ModelType, ChangeProcessor*>::size_type erased =
       processors_.erase(data_type_controller->type());
-  DCHECK(erased == 1);
+  DCHECK_EQ(erased, 1U);
 
   // TODO(sync): At this point we need to purge the data associated
   // with this data type from the sync db.
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index e362d09..1f7e79c 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_H_
 #define CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_H_
 
+#include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ -25,6 +26,7 @@
 #include "chrome/browser/sync/syncable/model_type.h"
 #include "googleurl/src/gurl.h"
 
+class CancelableTask;
 class Profile;
 
 namespace browser_sync {
@@ -109,6 +111,13 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
   // See the implementation and Core::DoShutdown for details.
   void Shutdown(bool sync_disabled);
 
+  // Changes the set of data types that are currently being synced.
+  // The ready_task will be run when all of the requested data types
+  // are up-to-date and ready for activation.  The task will cancelled
+  // upon shutdown.  The method takes ownership of the task pointer.
+  virtual void ConfigureDataTypes(const std::set<syncable::ModelType>& types,
+                                  CancelableTask* ready_task);
+
   // Activates change processing for the given data type.  This must
   // be called synchronously with the data type's model association so
   // no changes are dropped between model association and change
diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.h b/chrome/browser/sync/glue/sync_backend_host_mock.h
index fc27b02..e80850d 100644
--- a/chrome/browser/sync/glue/sync_backend_host_mock.h
+++ b/chrome/browser/sync/glue/sync_backend_host_mock.h
@@ -5,13 +5,21 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_MOCK_H__
 #define CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_MOCK_H__
 
+#include <set>
+
 #include "chrome/browser/sync/glue/sync_backend_host.h"
+#include "base/task.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
 #include "chrome/common/notification_type.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace browser_sync {
 
+ACTION(InvokeTask) {
+  arg1->Run();
+  delete arg1;
+}
+
 class SyncBackendHostMock : public SyncBackendHost {
  public:
   SyncBackendHostMock() {
@@ -23,8 +31,14 @@ class SyncBackendHostMock : public SyncBackendHost {
     ON_CALL(*this, RequestResume()).
         WillByDefault(testing::DoAll(Notify(NotificationType::SYNC_RESUMED),
                                      testing::Return(true)));
+
+    // By default, invoke the ready callback.
+    ON_CALL(*this, ConfigureDataTypes(testing::_, testing::_)).
+        WillByDefault(InvokeTask());
   }
 
+  MOCK_METHOD2(ConfigureDataTypes,
+               void(const std::set<syncable::ModelType>&, CancelableTask*));
   MOCK_METHOD0(RequestPause, bool());
   MOCK_METHOD0(RequestResume, bool());
 };