First stab at a DataTypeController.

This is my first stab at a DataTypeController, plus a BookmarkDataTypeController implementation.  It is not fully wired into the PSS yet :)

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

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

27 files changed, 1143 insertions, 206 deletions

diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 7790790..312d04c 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -237,7 +237,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   void SetURL(const BookmarkNode* node, const GURL& url);
 
   // Returns true if the model finished loading.
-  bool IsLoaded() { return loaded_; }
+  virtual bool IsLoaded() { return loaded_; }
 
   // Returns the set of nodes with the specified URL.
   void GetNodesByURL(const GURL& url, std::vector<const BookmarkNode*>* nodes);
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index 63b8c1d..a1594b2 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -46,6 +46,7 @@
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/ssl/ssl_host_state.h"
 #include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory_impl.h"
 #include "chrome/browser/thumbnail_store.h"
 #include "chrome/browser/visitedlink_master.h"
 #include "chrome/browser/visitedlink_event_listener.h"
@@ -1317,6 +1318,10 @@ ProfileSyncService* ProfileImpl::GetProfileSyncService() {
 }
 
 void ProfileImpl::InitSyncService() {
-  sync_service_.reset(new ProfileSyncService(this));
+  profile_sync_factory_.reset(
+      new ProfileSyncFactoryImpl(this,
+                                 CommandLine::ForCurrentProcess()));
+  sync_service_.reset(
+      profile_sync_factory_->CreateProfileSyncService());
   sync_service_->Initialize();
 }
diff --git a/chrome/browser/profile.h b/chrome/browser/profile.h
index 3a5ff60..eaa05b6 100644
--- a/chrome/browser/profile.h
+++ b/chrome/browser/profile.h
@@ -51,6 +51,7 @@ class PasswordStore;
 class PersonalDataManager;
 class PrefService;
 class ProfileSyncService;
+class ProfileSyncFactory;
 class SearchVersusNavigateClassifier;
 class SessionService;
 class SpellCheckHost;
@@ -538,6 +539,7 @@ class ProfileImpl : public Profile,
   scoped_refptr<WebResourceService> web_resource_service_;
   scoped_ptr<NTPResourceCache> ntp_resource_cache_;
 
+  scoped_ptr<ProfileSyncFactory> profile_sync_factory_;
   scoped_ptr<ProfileSyncService> sync_service_;
 
   scoped_refptr<ChromeURLRequestContextGetter> request_context_;
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
index 2bf9516..d41ec3e 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.cc
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -18,10 +18,11 @@
 namespace browser_sync {
 
 BookmarkChangeProcessor::BookmarkChangeProcessor(
+    BookmarkModelAssociator* model_associator,
     UnrecoverableErrorHandler* error_handler)
     : ChangeProcessor(error_handler),
       bookmark_model_(NULL),
-      model_associator_(NULL) {
+      model_associator_(model_associator) {
 }
 
 void BookmarkChangeProcessor::StartImpl(Profile* profile) {
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.h b/chrome/browser/sync/glue/bookmark_change_processor.h
index 0847919..5ae1811 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.h
+++ b/chrome/browser/sync/glue/bookmark_change_processor.h
@@ -25,13 +25,10 @@ namespace browser_sync {
 class BookmarkChangeProcessor : public BookmarkModelObserver,
                                 public ChangeProcessor {
  public:
-  explicit BookmarkChangeProcessor(UnrecoverableErrorHandler* error_handler);
+  explicit BookmarkChangeProcessor(BookmarkModelAssociator* model_associator,
+                                   UnrecoverableErrorHandler* error_handler);
   virtual ~BookmarkChangeProcessor() {}
 
-  void set_model_associator(BookmarkModelAssociator* model_associator) {
-    model_associator_ = model_associator;
-  }
-
   // BookmarkModelObserver implementation.
   // BookmarkModel -> sync_api model change application.
   virtual void Loaded(BookmarkModel* model) { NOTREACHED(); }
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller.cc b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
new file mode 100644
index 0000000..b37c899
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
@@ -0,0 +1,142 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/histogram.h"
+#include "base/logging.h"
+#include "base/time.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/bookmark_change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_data_type_controller.h"
+#include "chrome/browser/sync/glue/bookmark_model_associator.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
+
+namespace browser_sync {
+
+BookmarkDataTypeController::BookmarkDataTypeController(
+    ProfileSyncFactory* profile_sync_factory,
+    Profile* profile,
+    ProfileSyncService* sync_service)
+    : profile_sync_factory_(profile_sync_factory),
+      profile_(profile),
+      sync_service_(sync_service),
+      state_(NOT_RUNNING),
+      merge_allowed_(false) {
+  DCHECK(profile_sync_factory);
+  DCHECK(profile);
+  DCHECK(sync_service);
+}
+
+BookmarkDataTypeController::~BookmarkDataTypeController() {
+}
+
+void BookmarkDataTypeController::Start(bool merge_allowed,
+                                       StartCallback* start_callback) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  if (state_ != NOT_RUNNING) {
+    start_callback->Run(BUSY);
+    delete start_callback;
+    return;
+  }
+
+  start_callback_.reset(start_callback);
+  merge_allowed_ = merge_allowed;
+
+  if (!enabled()) {
+    FinishStart(NOT_ENABLED);
+    return;
+  }
+
+  state_ = MODEL_STARTING;
+
+  // If the bookmarks model is loaded, continue with association.
+  BookmarkModel* bookmark_model = profile_->GetBookmarkModel();
+  if (bookmark_model && bookmark_model->IsLoaded()) {
+    Associate();
+    return;
+  }
+
+  // Add an observer and continue when the bookmarks model is loaded.
+  registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED,
+                 Source<Profile>(sync_service_->profile()));
+}
+
+void BookmarkDataTypeController::Stop() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  // If Stop() is called while Start() is waiting for the bookmark
+  // model to load, abort the start.
+  if (state_ == MODEL_STARTING) {
+    FinishStart(ABORTED);
+  }
+
+  registrar_.RemoveAll();
+  if (change_processor_ != NULL)
+    sync_service_->DeactivateDataType(this, change_processor_.get());
+
+  if (model_associator_ != NULL)
+    model_associator_->DisassociateModels();
+
+  change_processor_.reset();
+  model_associator_.reset();
+
+  state_ = NOT_RUNNING;
+  merge_allowed_ = false;
+}
+
+void BookmarkDataTypeController::Observe(NotificationType type,
+                                         const NotificationSource& source,
+                                         const NotificationDetails& details) {
+  DCHECK_EQ(NotificationType::BOOKMARK_MODEL_LOADED, type.value);
+  registrar_.RemoveAll();
+  Associate();
+}
+
+void BookmarkDataTypeController::Associate() {
+  DCHECK_EQ(state_, MODEL_STARTING);
+  state_ = ASSOCIATING;
+
+  ProfileSyncFactory::BookmarkComponents bookmark_components =
+      profile_sync_factory_->CreateBookmarkComponents(sync_service_);
+  model_associator_.reset(bookmark_components.model_associator);
+  change_processor_.reset(bookmark_components.change_processor);
+
+  bool needs_merge =  model_associator_->ChromeModelHasUserCreatedNodes() &&
+      model_associator_->SyncModelHasUserCreatedNodes();
+  if (needs_merge && !merge_allowed_) {
+    model_associator_.reset();
+    change_processor_.reset();
+    state_ = NOT_RUNNING;
+    FinishStart(NEEDS_MERGE);
+    return;
+  }
+
+  base::TimeTicks start_time = base::TimeTicks::Now();
+  bool first_run = !model_associator_->SyncModelHasUserCreatedNodes();
+  bool merge_success = model_associator_->AssociateModels();
+  UMA_HISTOGRAM_TIMES("Sync.BookmarkAssociationTime",
+                      base::TimeTicks::Now() - start_time);
+  if (!merge_success) {
+    model_associator_.reset();
+    change_processor_.reset();
+    state_ = NOT_RUNNING;
+    FinishStart(ASSOCIATION_FAILED);
+    return;
+  }
+
+  sync_service_->ActivateDataType(this, change_processor_.get());
+  state_ = RUNNING;
+  FinishStart(first_run ? OK_FIRST_RUN : OK);
+}
+
+void BookmarkDataTypeController::FinishStart(StartResult result) {
+  start_callback_->Run(result);
+  start_callback_.reset();
+}
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller.h b/chrome/browser/sync/glue/bookmark_data_type_controller.h
new file mode 100644
index 0000000..f0719de
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller.h
@@ -0,0 +1,86 @@
+// Copyright (c) 2009 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 CHROME_BROWSER_SYNC_GLUE_BOOKMARK_DATA_TYPE_CONTROLLER_H__
+#define CHROME_BROWSER_SYNC_GLUE_BOOKMARK_DATA_TYPE_CONTROLLER_H__
+
+#include "base/basictypes.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_observer.h"
+
+class NotificationDetails;
+class NotificationType;
+class NotificationSource;
+class Profile;
+class ProfileSyncService;
+class ProfileSyncFactory;
+
+namespace browser_sync {
+
+class AssociatorInterface;
+class ChangeProcessor;
+
+// A class that manages the startup and shutdown of bookmark sync.
+class BookmarkDataTypeController : public DataTypeController,
+                                   public NotificationObserver {
+ public:
+  BookmarkDataTypeController(
+      ProfileSyncFactory* profile_sync_factory,
+      Profile* profile,
+      ProfileSyncService* sync_service);
+  virtual ~BookmarkDataTypeController();
+
+  // DataTypeController interface.
+  virtual void Start(bool merge_allowed, StartCallback* start_callback);
+
+  virtual void Stop();
+
+  virtual bool enabled() {
+    return true;
+  }
+
+  virtual syncable::ModelType type() {
+    return syncable::BOOKMARKS;
+  }
+
+  virtual browser_sync::ModelSafeGroup model_safe_group() {
+    return browser_sync::GROUP_UI;
+  }
+
+  virtual State state() {
+    return state_;
+  }
+
+  // NotificationObserver interface.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  // Runs model association and change processor registration.
+  void Associate();
+
+  // Helper method to run the stashed start callback with a given result.
+  void FinishStart(StartResult result);
+
+  ProfileSyncFactory* profile_sync_factory_;
+  Profile* profile_;
+  ProfileSyncService* sync_service_;
+
+  State state_;
+  bool merge_allowed_;
+
+  scoped_ptr<StartCallback> start_callback_;
+  scoped_ptr<AssociatorInterface> model_associator_;
+  scoped_ptr<ChangeProcessor> change_processor_;
+  NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkDataTypeController);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_BOOKMARK_DATA_TYPE_CONTROLLER_H__
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc
new file mode 100644
index 0000000..26f025b
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc
@@ -0,0 +1,217 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include "base/message_loop.h"
+#include "base/scoped_ptr.h"
+#include "base/task.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/bookmark_data_type_controller.h"
+#include "chrome/browser/sync/glue/change_processor_mock.h"
+#include "chrome/browser/sync/glue/model_associator_mock.h"
+#include "chrome/browser/sync/profile_sync_factory_mock.h"
+#include "chrome/browser/sync/profile_sync_service_mock.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
+#include "chrome/test/testing_profile.h"
+
+using browser_sync::BookmarkDataTypeController;
+using browser_sync::ChangeProcessorMock;
+using browser_sync::DataTypeController;
+using browser_sync::ModelAssociatorMock;
+using testing::_;
+using testing::Return;
+
+class StartCallback {
+ public:
+  MOCK_METHOD1(Run, void(DataTypeController::StartResult result));
+};
+
+class ProfileMock : public TestingProfile {
+ public:
+  MOCK_METHOD0(GetBookmarkModel, BookmarkModel*());
+};
+
+class BookmarkModelMock : public BookmarkModel {
+ public:
+  BookmarkModelMock() : BookmarkModel(NULL) {}
+  MOCK_METHOD0(IsLoaded, bool());
+};
+
+class BookmarkDataTypeControllerTest : public testing::Test {
+ public:
+  BookmarkDataTypeControllerTest()
+      : ui_thread_(ChromeThread::UI, &message_loop_) {}
+
+  virtual void SetUp() {
+    model_associator_ = new ModelAssociatorMock();
+    change_processor_ = new ChangeProcessorMock();
+    profile_sync_factory_.reset(
+        new ProfileSyncFactoryMock(model_associator_,
+                                   change_processor_));
+    bookmark_dtc_.reset(
+        new BookmarkDataTypeController(profile_sync_factory_.get(),
+                                       &profile_,
+                                       &service_));
+  }
+
+ protected:
+  void SetStartExpectations() {
+    EXPECT_CALL(profile_, GetBookmarkModel()).
+        WillOnce(Return(&bookmark_model_));
+    EXPECT_CALL(bookmark_model_, IsLoaded()).WillRepeatedly(Return(true));
+  }
+
+  void SetAssociateExpectations() {
+    EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+    EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+        WillRepeatedly(Return(false));
+    EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+        WillRepeatedly(Return(true));
+    EXPECT_CALL(*model_associator_, AssociateModels()).
+        WillRepeatedly(Return(true));
+    EXPECT_CALL(service_, ActivateDataType(_, _));
+  }
+
+  void SetStopExpectations() {
+    EXPECT_CALL(service_, DeactivateDataType(_, _));
+    EXPECT_CALL(*model_associator_, DisassociateModels());
+  }
+
+  MessageLoopForUI message_loop_;
+  ChromeThread ui_thread_;
+  scoped_ptr<BookmarkDataTypeController> bookmark_dtc_;
+  scoped_ptr<ProfileSyncFactoryMock> profile_sync_factory_;
+  ProfileMock profile_;
+  BookmarkModelMock bookmark_model_;
+  ProfileSyncServiceMock service_;
+  ModelAssociatorMock* model_associator_;
+  ChangeProcessorMock* change_processor_;
+  StartCallback start_callback_;
+};
+
+TEST_F(BookmarkDataTypeControllerTest, StartBookmarkModelReady) {
+  SetStartExpectations();
+  SetAssociateExpectations();
+
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state());
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartBookmarkModelNotReady) {
+  SetStartExpectations();
+  EXPECT_CALL(bookmark_model_, IsLoaded()).WillRepeatedly(Return(false));
+  SetAssociateExpectations();
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::MODEL_STARTING, bookmark_dtc_->state());
+
+  // Send the notification that the bookmark model has started.
+  NotificationService::current()->Notify(
+      NotificationType::BOOKMARK_MODEL_LOADED,
+      Source<Profile>(&profile_),
+      NotificationService::NoDetails());
+  EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state());
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartFirstRun) {
+  SetStartExpectations();
+  SetAssociateExpectations();
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(false));
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartBusy) {
+  SetStartExpectations();
+  EXPECT_CALL(bookmark_model_, IsLoaded()).WillRepeatedly(Return(false));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::BUSY));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartNeedsMerge) {
+  SetStartExpectations();
+  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+  EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::NEEDS_MERGE));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartMergeAllowed) {
+  SetStartExpectations();
+  SetAssociateExpectations();
+  EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  bookmark_dtc_->Start(true,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartAssociationFailed) {
+  SetStartExpectations();
+  // Set up association to fail.
+  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+  EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(false));
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+  EXPECT_CALL(*model_associator_, AssociateModels()).
+      WillRepeatedly(Return(false));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::ASSOCIATION_FAILED));
+  bookmark_dtc_->Start(true,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
+}
+
+TEST_F(BookmarkDataTypeControllerTest, StartAborted) {
+  SetStartExpectations();
+  EXPECT_CALL(bookmark_model_, IsLoaded()).WillRepeatedly(Return(false));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  bookmark_dtc_->Stop();
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
+}
+
+TEST_F(BookmarkDataTypeControllerTest, Stop) {
+  SetStartExpectations();
+  SetAssociateExpectations();
+  SetStopExpectations();
+
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  bookmark_dtc_->Start(false,
+                       NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state());
+  bookmark_dtc_->Stop();
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
+
+}
diff --git a/chrome/browser/sync/glue/change_processor_mock.h b/chrome/browser/sync/glue/change_processor_mock.h
new file mode 100644
index 0000000..42bad36
--- /dev/null
+++ b/chrome/browser/sync/glue/change_processor_mock.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_GLUE_CHANGE_PROCESSOR_MOCK_H__
+#define CHROME_BROWSER_SYNC_GLUE_CHANGE_PROCESSOR_MOCK_H__
+
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/syncable/syncable.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace browser_sync {
+
+class ChangeProcessorMock : public ChangeProcessor {
+ public:
+  ChangeProcessorMock() : ChangeProcessor(NULL) {}
+  MOCK_METHOD3(ApplyChangesFromSyncModel,
+               void(const sync_api::BaseTransaction* trans,
+                    const sync_api::SyncManager::ChangeRecord* changes,
+                    int change_count));
+  MOCK_METHOD1(StartImpl, void(Profile* profile));
+  MOCK_METHOD0(StopImpl, void());
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_CHANGE_PROCESSOR_MOCK_H__
diff --git a/chrome/browser/sync/glue/data_type_controller.h b/chrome/browser/sync/glue/data_type_controller.h
new file mode 100644
index 0000000..62709ff
--- /dev/null
+++ b/chrome/browser/sync/glue/data_type_controller.h
@@ -0,0 +1,81 @@
+// Copyright (c) 2009 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 CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_H__
+#define CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_H__
+
+#include "base/task.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
+#include "chrome/browser/sync/syncable/model_type.h"
+
+namespace browser_sync {
+
+class DataTypeController {
+ public:
+  enum State {
+    NOT_RUNNING,    // The controller has never been started or has
+                    // previously been stopped.  Must be in this state to start.
+    MODEL_STARTING, // The controller is waiting on dependent services
+                    // that need to be available before model
+                    // association.
+    ASSOCIATING,    // Model association is in progress.
+    RUNNING,        // The controller is running and the data type is
+                    // in sync with the cloud.
+    STOPPING        // The controller is in the process of stopping
+                    // and is waiting for dependent services to stop.
+  };
+
+  enum StartResult {
+    OK,                 // The data type has started normally.
+    OK_FIRST_RUN,       // Same as OK, but sent on first successful
+                        // start for this type for this user as
+                        // determined by cloud state.
+    BUSY,               // Start() was called while already in progress.
+    NOT_ENABLED,        // This data type is not enabled for the current user.
+    NEEDS_MERGE,        // Can't start without explicit permission to
+                        // perform a data merge.  Re-starting with
+                        // merge_allowed = true will allow this data
+                        // type to start.
+    ASSOCIATION_FAILED, // An error occurred during model association.
+    ABORTED             // Start was aborted by calling Stop().
+  };
+
+  typedef Callback1<StartResult>::Type StartCallback;
+
+  virtual ~DataTypeController() {}
+
+  // Begins asynchronous start up of this data type.  Start up will
+  // wait for all other dependent services to be available, then
+  // proceed with model association and then change processor
+  // activation.  Upon completion, the start_callback will be invoked
+  // on the UI thread.  The merge_allowed parameter gives the data
+  // type permission to perform a data merge at start time.  See the
+  // StartResult enum above for details on the possible start results.
+  virtual void Start(bool merge_allowed, StartCallback* start_callback) = 0;
+
+  // Synchronously stops the data type.  If called after Start() is
+  // called but before the start callback is called, the start is
+  // aborted and the start callback is invoked with the ABORTED start
+  // result.
+  virtual void Stop() = 0;
+
+  // Returns true if the user has indicated that they want this data
+  // type to be enabled.
+  virtual bool enabled() = 0;
+
+  // Unique model type for this data type controller.
+  virtual syncable::ModelType type() = 0;
+
+  // The model safe group of this data type.  This should reflect the
+  // thread that should be used to modify the data type's native
+  // model.
+  virtual browser_sync::ModelSafeGroup model_safe_group() = 0;
+
+  // Current state of the data type controller.
+  virtual State state() = 0;
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_H__
diff --git a/chrome/browser/sync/glue/model_associator_mock.h b/chrome/browser/sync/glue/model_associator_mock.h
new file mode 100644
index 0000000..7852dba
--- /dev/null
+++ b/chrome/browser/sync/glue/model_associator_mock.h
@@ -0,0 +1,23 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_GLUE_MODEL_ASSOCIATOR_MOCK_H__
+#define CHROME_BROWSER_SYNC_GLUE_MODEL_ASSOCIATOR_MOCK_H__
+
+#include "chrome/browser/sync/glue/model_associator.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace browser_sync {
+
+class ModelAssociatorMock : public AssociatorInterface {
+ public:
+  MOCK_METHOD0(AssociateModels, bool());
+  MOCK_METHOD0(DisassociateModels, bool());
+  MOCK_METHOD0(SyncModelHasUserCreatedNodes, bool());
+  MOCK_METHOD0(ChromeModelHasUserCreatedNodes, bool());
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_MODEL_ASSOCIATOR_MOCK_H__
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 9b18d90..c154137 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -8,6 +8,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/glue/http_bridge.h"
 #include "webkit/glue/webkit_glue.h"
@@ -23,12 +24,10 @@ typedef GoogleServiceAuthError AuthError;
 namespace browser_sync {
 
 SyncBackendHost::SyncBackendHost(SyncFrontend* frontend,
-                                 const FilePath& profile_path,
-                                 std::set<ChangeProcessor*> processors)
+                                 const FilePath& profile_path)
     : core_thread_("Chrome_SyncCoreThread"),
       frontend_loop_(MessageLoop::current()),
       frontend_(frontend),
-      processors_(processors),
       sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
       last_auth_error_(AuthError::None()) {
 
@@ -62,7 +61,6 @@ void SyncBackendHost::Initialize(
   // when a new type is synced as the worker may already exist and you just
   // need to update routing_info_.
   registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
-  registrar_.routing_info[syncable::BOOKMARKS] = GROUP_UI;
 
   core_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoInitialize,
@@ -118,6 +116,39 @@ void SyncBackendHost::Shutdown(bool sync_disabled) {
   core_ = NULL;  // Releases reference to core_.
 }
 
+void SyncBackendHost::ActivateDataType(
+    DataTypeController* data_type_controller,
+    ChangeProcessor* change_processor) {
+  // Ensure that the given data type is in the PASSIVE group.
+  browser_sync::ModelSafeRoutingInfo::iterator i =
+      registrar_.routing_info.find(data_type_controller->type());
+  DCHECK(i != registrar_.routing_info.end());
+  DCHECK((*i).second == GROUP_PASSIVE);
+
+  // Change the data type's routing info to its group.
+  registrar_.routing_info[data_type_controller->type()] =
+      data_type_controller->model_safe_group();
+
+  // Add the data type's change processor to the list of change
+  // processors so it can receive updates.
+  std::pair<std::set<ChangeProcessor*>::iterator, bool> result =
+      processors_.insert(change_processor);
+  DCHECK(result.second);
+}
+
+void SyncBackendHost::DeactivateDataType(
+    DataTypeController* data_type_controller,
+    ChangeProcessor* change_processor) {
+  registrar_.routing_info.erase(data_type_controller->type());
+
+  std::set<ChangeProcessor*>::size_type erased =
+      processors_.erase(change_processor);
+  DCHECK(erased == 1);
+
+  // TODO(sync): At this point we need to purge the data associated
+  // with this data type from the sync db.
+}
+
 void SyncBackendHost::Core::NotifyFrontend(FrontendNotification notification) {
   if (!host_ || !host_->frontend_) {
     return;  // This can happen in testing because the UI loop processes tasks
@@ -278,6 +309,18 @@ void SyncBackendHost::Core::OnChangesApplied(
     return;
   }
 
+  // Right now we only support bookmarks, and until bookmark model
+  // association happens, the processors list will be empty.  During
+  // this time, it is OK to drop changes on the floor (since model
+  // association has not happened yet).  When the bookmark data type
+  // is activated, model association takes place then the change
+  // processor is added to the processors_ list.  This all happens on
+  // the UI thread so we will never drop any changes after model
+  // association.
+  // TODO(sync): Replace this with proper change filtering.
+  if (!host_->processors_.size())
+    return;
+
   // ChangesApplied is the one exception that should come over from the sync
   // backend already on the service_loop_ thanks to our UIModelWorker.
   // SyncFrontend changes exclusively on the UI loop, because it updates
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index be9e90b..731a172 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -26,6 +26,7 @@
 namespace browser_sync {
 
 class ChangeProcessor;
+class DataTypeController;
 
 // SyncFrontend is the interface used by SyncBackendHost to communicate with
 // the entity that created it and, presumably, is interested in sync-related
@@ -69,10 +70,8 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
 
   // Create a SyncBackendHost with a reference to the |frontend| that it serves
   // and communicates to via the SyncFrontend interface (on the same thread
-  // it used to call the constructor), and push changes from sync_api through
-  // |processor|.
-  SyncBackendHost(SyncFrontend* frontend, const FilePath& profile_path,
-                  std::set<ChangeProcessor*> processor);
+  // it used to call the constructor).
+  SyncBackendHost(SyncFrontend* frontend, const FilePath& profile_path);
   ~SyncBackendHost();
 
   // Called on |frontend_loop_| to kick off asynchronous initialization.
@@ -94,6 +93,17 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
   // See the implementation and Core::DoShutdown for details.
   void Shutdown(bool sync_disabled);
 
+  // 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
+  // processor activation.
+  void ActivateDataType(DataTypeController* data_type_controller,
+                        ChangeProcessor* change_processor);
+
+  // Deactivates change processing for the given data type.
+  void DeactivateDataType(DataTypeController* data_type_controller,
+                          ChangeProcessor* change_processor);
+
   // Called on |frontend_loop_| to obtain a handle to the UserShare needed
   // for creating transactions.
   UserShareHandle GetUserShareHandle() const;
@@ -127,7 +137,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
     if (!core_thread_.Start())
       return;
     registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
-    registrar_.routing_info[syncable::BOOKMARKS] = GROUP_UI;
 
     core_thread_.message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(core_.get(),
diff --git a/chrome/browser/sync/profile_sync_factory.h b/chrome/browser/sync/profile_sync_factory.h
new file mode 100644
index 0000000..107807e
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_H__
+#define CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_H__
+
+#include <utility>
+#include "base/task.h"
+
+class ProfileSyncService;
+
+namespace browser_sync {
+class AssociatorInterface;
+class ChangeProcessor;
+}
+
+// Factory class for all profile sync related classes.
+class ProfileSyncFactory {
+ public:
+  struct BookmarkComponents {
+    browser_sync::AssociatorInterface* model_associator;
+    browser_sync::ChangeProcessor* change_processor;
+    BookmarkComponents(browser_sync::AssociatorInterface* ma,
+                       browser_sync::ChangeProcessor* cp)
+        : model_associator(ma), change_processor(cp) {}
+  };
+
+  virtual ~ProfileSyncFactory() {}
+
+  // Instantiates and initializes a new ProfileSyncService.  Enabled
+  // data types are registered with the service.  The return pointer
+  // is owned by the caller.
+  virtual ProfileSyncService* CreateProfileSyncService() = 0;
+
+  // Instantiates both a model associator and change processor for the
+  // bookmark data type.  Ideally we would have separate factory
+  // methods for these components, but the bookmark implementation of
+  // these are tightly coupled and must be created at the same time.
+  // The pointers in the return struct are owned by the caller.
+  virtual BookmarkComponents CreateBookmarkComponents(
+      ProfileSyncService* profile_sync_service) = 0;
+};
+
+#endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_H__
diff --git a/chrome/browser/sync/profile_sync_factory_impl.cc b/chrome/browser/sync/profile_sync_factory_impl.cc
new file mode 100644
index 0000000..d7658de
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory_impl.cc
@@ -0,0 +1,42 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_data_type_controller.h"
+#include "chrome/browser/sync/glue/bookmark_model_associator.h"
+#include "chrome/browser/sync/glue/model_associator.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory_impl.h"
+#include "chrome/common/chrome_switches.h"
+
+ProfileSyncFactoryImpl::ProfileSyncFactoryImpl(
+    Profile* profile,
+    CommandLine* command_line)
+    : profile_(profile), command_line_(command_line) {
+}
+
+ProfileSyncService* ProfileSyncFactoryImpl::CreateProfileSyncService() {
+  ProfileSyncService* pss = new ProfileSyncService(profile_);
+
+  // Bookmark sync is enabled by default.  Register unless explicitly disabled.
+  if (!command_line_->HasSwitch(switches::kDisableSyncBookmarks)) {
+    pss->RegisterDataTypeController(
+        new browser_sync::BookmarkDataTypeController(this, profile_, pss));
+  }
+  return pss;
+}
+
+ProfileSyncFactory::BookmarkComponents
+ProfileSyncFactoryImpl::CreateBookmarkComponents(
+    ProfileSyncService* profile_sync_service) {
+  browser_sync::BookmarkModelAssociator* model_associator =
+      new browser_sync::BookmarkModelAssociator(profile_sync_service);
+  browser_sync::BookmarkChangeProcessor* change_processor =
+      new browser_sync::BookmarkChangeProcessor(model_associator,
+                                                profile_sync_service);
+  return BookmarkComponents(model_associator, change_processor);
+}
diff --git a/chrome/browser/sync/profile_sync_factory_impl.h b/chrome/browser/sync/profile_sync_factory_impl.h
new file mode 100644
index 0000000..638a508
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory_impl.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_IMPL_H__
+#define CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_IMPL_H__
+
+#include "base/basictypes.h"
+#include "chrome/browser/sync/profile_sync_factory.h"
+
+class CommandLine;
+class Profile;
+
+class ProfileSyncFactoryImpl : public ProfileSyncFactory {
+ public:
+  ProfileSyncFactoryImpl(Profile* profile, CommandLine* command_line);
+  virtual ~ProfileSyncFactoryImpl() {}
+
+  virtual ProfileSyncService* CreateProfileSyncService();
+
+  virtual BookmarkComponents CreateBookmarkComponents(
+      ProfileSyncService* profile_sync_service);
+
+ private:
+  Profile* profile_;
+  CommandLine* command_line_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileSyncFactoryImpl);
+};
+
+#endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_IMPL_H__
diff --git a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
new file mode 100644
index 0000000..1dbbdb3
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
@@ -0,0 +1,54 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "base/command_line.h"
+#include "base/file_path.h"
+#include "base/message_loop.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory_impl.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/testing_profile.h"
+
+class ProfileSyncFactoryImplTest : public testing::Test {
+ protected:
+  ProfileSyncFactoryImplTest()
+      : ui_thread_(ChromeThread::UI, &message_loop_) {}
+
+  virtual void SetUp() {
+    profile_.reset(new TestingProfile());
+    FilePath program_path(FILE_PATH_LITERAL("chrome.exe"));
+    command_line_.reset(new CommandLine(program_path));
+    profile_sync_service_factory_.reset(
+        new ProfileSyncFactoryImpl(profile_.get(), command_line_.get()));
+  }
+
+  MessageLoop message_loop_;
+  ChromeThread ui_thread_;
+  scoped_ptr<Profile> profile_;
+  scoped_ptr<CommandLine> command_line_;
+  scoped_ptr<ProfileSyncFactoryImpl> profile_sync_service_factory_;
+};
+
+TEST_F(ProfileSyncFactoryImplTest, CreatePSSDefault) {
+  scoped_ptr<ProfileSyncService> pss;
+  pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
+  ProfileSyncService::DataTypeControllerMap controllers(
+      pss->data_type_controllers());
+  EXPECT_EQ(1U, controllers.size());
+  EXPECT_EQ(1U, controllers.count(syncable::BOOKMARKS));
+}
+
+TEST_F(ProfileSyncFactoryImplTest, CreatePSSDisableBookmarks) {
+  command_line_->AppendSwitch(switches::kDisableSyncBookmarks);
+  scoped_ptr<ProfileSyncService> pss;
+  pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
+  ProfileSyncService::DataTypeControllerMap controllers(
+      pss->data_type_controllers());
+  EXPECT_EQ(0U, controllers.size());
+  EXPECT_EQ(0U, controllers.count(syncable::BOOKMARKS));
+}
diff --git a/chrome/browser/sync/profile_sync_factory_mock.cc b/chrome/browser/sync/profile_sync_factory_mock.cc
new file mode 100644
index 0000000..c30331f
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory_mock.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/model_associator.h"
+#include "chrome/browser/sync/profile_sync_factory_mock.h"
+
+using browser_sync::AssociatorInterface;
+using browser_sync::ChangeProcessor;
+using testing::_;
+using testing::InvokeWithoutArgs;
+
+ProfileSyncFactoryMock::ProfileSyncFactoryMock(
+    AssociatorInterface* bookmark_model_associator,
+    ChangeProcessor* bookmark_change_processor)
+    : bookmark_model_associator_(bookmark_model_associator),
+      bookmark_change_processor_(bookmark_change_processor) {
+  ON_CALL(*this, CreateBookmarkComponents(_)).
+      WillByDefault(
+          InvokeWithoutArgs(
+              this,
+              &ProfileSyncFactoryMock::MakeBookmarkComponents));
+}
+
+ProfileSyncFactory::BookmarkComponents
+ProfileSyncFactoryMock::MakeBookmarkComponents() {
+  return BookmarkComponents(bookmark_model_associator_.release(),
+                            bookmark_change_processor_.release());
+}
diff --git a/chrome/browser/sync/profile_sync_factory_mock.h b/chrome/browser/sync/profile_sync_factory_mock.h
new file mode 100644
index 0000000..3ac7d51
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_factory_mock.h
@@ -0,0 +1,36 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_MOCK_H__
+#define CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_MOCK_H__
+
+#include "base/scoped_ptr.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace browser_sync {
+class AssociatorInterface;
+class ChangeProcessor;
+}
+
+class ProfileSyncFactoryMock : public ProfileSyncFactory {
+ public:
+  ProfileSyncFactoryMock(
+      browser_sync::AssociatorInterface* bookmark_model_associator,
+      browser_sync::ChangeProcessor* bookmark_change_processor);
+
+  MOCK_METHOD0(CreateProfileSyncService,
+               ProfileSyncService*(void));
+  MOCK_METHOD1(CreateBookmarkComponents,
+               BookmarkComponents(ProfileSyncService* profile_sync_service));
+
+ private:
+  BookmarkComponents MakeBookmarkComponents();
+
+  scoped_ptr<browser_sync::AssociatorInterface> bookmark_model_associator_;
+  scoped_ptr<browser_sync::ChangeProcessor> bookmark_change_processor_;
+};
+
+#endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_FACTORY_MOCK_H__
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 88e080e..df65f79 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -4,28 +4,20 @@
 
 #include "chrome/browser/sync/profile_sync_service.h"
 
-#include <stack>
-#include <vector>
-
 #include "app/l10n_util.h"
-#include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/histogram.h"
+#include "base/stl_util-inl.h"
 #include "base/string_util.h"
-#include "base/time.h"
-#include "chrome/browser/bookmarks/bookmark_utils.h"
+#include "base/task.h"
 #include "chrome/browser/defaults.h"
-#include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/history_types.h"
-#include "chrome/browser/profile.h"
 #include "chrome/browser/sync/engine/syncapi.h"
-#include "chrome/browser/sync/glue/bookmark_change_processor.h"
-#include "chrome/browser/sync/glue/bookmark_model_associator.h"
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
 #include "chrome/common/time_format.h"
@@ -33,10 +25,8 @@
 #include "net/base/cookie_monster.h"
 #include "views/window/window.h"
 
-using browser_sync::BookmarkChangeProcessor;
-using browser_sync::BookmarkModelAssociator;
 using browser_sync::ChangeProcessor;
-using browser_sync::ModelAssociator;
+using browser_sync::DataTypeController;
 using browser_sync::SyncBackendHost;
 
 typedef GoogleServiceAuthError AuthError;
@@ -48,8 +38,6 @@ ProfileSyncService::ProfileSyncService(Profile* profile)
     : last_auth_error_(AuthError::None()),
       profile_(profile),
       sync_service_url_(kSyncServerUrl),
-      ALLOW_THIS_IN_INITIALIZER_LIST(model_associator_(
-          new ModelAssociator(this))),
       backend_initialized_(false),
       expecting_first_run_auth_needed_event_(false),
       is_auth_in_progress_(false),
@@ -59,6 +47,7 @@ ProfileSyncService::ProfileSyncService(Profile* profile)
 
 ProfileSyncService::~ProfileSyncService() {
   Shutdown(false);
+  STLDeleteValues(&data_type_controllers_);
 }
 
 void ProfileSyncService::Initialize() {
@@ -79,6 +68,13 @@ void ProfileSyncService::Initialize() {
   }
 }
 
+void ProfileSyncService::RegisterDataTypeController(
+    DataTypeController* data_type_controller) {
+  DCHECK(data_type_controllers_.count(data_type_controller->type()) == 0);
+  data_type_controllers_[data_type_controller->type()] =
+      data_type_controller;
+}
+
 void ProfileSyncService::InitSettings() {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
 
@@ -155,21 +151,10 @@ void ProfileSyncService::StartUp() {
   if (backend_.get())
     return;
 
-  // Register associator impls for all currently synced data types, and hook
-  // them up to the associated change processors.  If you add a new data type
-  // and want that data type to be synced, call CreateGlue with appropriate
-  // association and change processing implementations.
-  // Bookmarks.
-  InstallGlue<BookmarkModelAssociator, BookmarkChangeProcessor>();
-
   last_synced_time_ = base::Time::FromInternalValue(
       profile_->GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
 
-  backend_.reset(new SyncBackendHost(this, profile_->GetPath(),
-                                     change_processors_));
-
-  registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED,
-                 Source<Profile>(profile_));
+  backend_.reset(new SyncBackendHost(this, profile_->GetPath()));
 
   // Initialize the backend.  Every time we start up a new SyncBackendHost,
   // we'll want to start from a fresh SyncDB, so delete any old one that might
@@ -178,22 +163,15 @@ void ProfileSyncService::StartUp() {
 }
 
 void ProfileSyncService::Shutdown(bool sync_disabled) {
-  registrar_.RemoveAll();
-
   if (backend_.get())
     backend_->Shutdown(sync_disabled);
 
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    (*it)->Stop();
-  }
-  backend_.reset();
+  // Stop all data type controllers.
+  // TODO(skrul): Change this to support multiple data type controllers.
+  if (data_type_controllers_.count(syncable::BOOKMARKS))
+    data_type_controllers_[syncable::BOOKMARKS]->Stop();
 
-  // Clear all associations and throw away the association manager instance.
-  model_associator_->DisassociateModels();
-  model_associator_->CleanupAllAssociators();
-
-  STLDeleteElements(&change_processors_);
+  backend_.reset();
 
   // Clear various flags.
   is_auth_in_progress_ = false;
@@ -224,23 +202,6 @@ void ProfileSyncService::DisableForUser() {
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
 }
 
-void ProfileSyncService::Observe(NotificationType type,
-                                 const NotificationSource& source,
-                                 const NotificationDetails& details) {
-  DCHECK_EQ(NotificationType::BOOKMARK_MODEL_LOADED, type.value);
-  registrar_.RemoveAll();
-  StartProcessingChangesIfReady();
-}
-
-bool ProfileSyncService::MergeAndSyncAcceptanceNeeded() const {
-  // If we've shown the dialog before, don't show it again.
-  if (profile_->GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted))
-    return false;
-
-  return model_associator_->ChromeModelHasUserCreatedNodes() &&
-         model_associator_->SyncModelHasUserCreatedNodes();
-}
-
 bool ProfileSyncService::HasSyncSetupCompleted() const {
   return profile_->GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted);
 }
@@ -262,10 +223,9 @@ void ProfileSyncService::UpdateLastSyncedTime() {
 // to do as little work as possible, to avoid further corruption or crashes.
 void ProfileSyncService::OnUnrecoverableError() {
   unrecoverable_error_detected_ = true;
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    (*it)->Stop();
-  }
+
+  // TODO(skrul): Change this to support multiple data type controllers.
+  data_type_controllers_[syncable::BOOKMARKS]->Stop();
 
   // Tell the wizard so it can inform the user only if it is already open.
   wizard_.Step(SyncSetupWizard::FATAL_ERROR);
@@ -392,27 +352,12 @@ void ProfileSyncService::OnUserSubmittedAuth(
 }
 
 void ProfileSyncService::OnUserAcceptedMergeAndSync() {
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  // TODO(sync): Figure out what do to when a single associator fails.
-  // http://crbug.com/30038
-  bool not_first_run = model_associator_->SyncModelHasUserCreatedNodes();
-  bool merge_success = model_associator_->AssociateModels();
-  UMA_HISTOGRAM_MEDIUM_TIMES("Sync.UserPerceivedBookmarkAssociation",
-                             base::TimeTicks::Now() - start_time);
-  if (!merge_success) {
-    LOG(ERROR) << "Model association failed.";
-    OnUnrecoverableError();
-    return;
-  }
-
-  wizard_.Step(not_first_run ? SyncSetupWizard::DONE :
-               SyncSetupWizard::DONE_FIRST_TIME);
-
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    (*it)->Start(profile(), backend_->GetUserShareHandle());
-  }
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  // Start each data type with "merge_allowed" to correspond to the
+  // user's acceptance of merge.
+  // TODO(skrul): Change this to support multiple data type controllers.
+  data_type_controllers_[syncable::BOOKMARKS]->Start(
+      true,
+      NewCallback(this, &ProfileSyncService::BookmarkStartCallback));
 }
 
 void ProfileSyncService::OnUserCancelledDialog() {
@@ -426,53 +371,63 @@ void ProfileSyncService::OnUserCancelledDialog() {
 }
 
 void ProfileSyncService::StartProcessingChangesIfReady() {
-  BookmarkModel* model = profile_->GetBookmarkModel();
-
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    DCHECK(!(*it)->IsRunning());
-  }
+  DCHECK(backend_initialized_);
 
-  // First check if the subsystems are ready.  We can't proceed until they
-  // both have finished loading.
-  if (!model->IsLoaded())
-    return;
-  if (!backend_initialized_)
-    return;
+  // If the user has completed sync setup, we are always allowed to
+  // merge data.
+  bool merge_allowed =
+      profile_->GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted);
 
+  // If we're running inside Chrome OS, always allow merges and
+  // consider the sync setup complete.
   if (browser_defaults::kBootstrapSyncAuthentication) {
-    // If we're running inside Chrome OS, skip the merge warning and consider
-    // the sync setup complete.
+    merge_allowed = true;
     SetSyncSetupCompleted();
-  } else {
-    // Show the sync merge warning dialog if needed.
-    if (MergeAndSyncAcceptanceNeeded()) {
+  }
+
+  // Start data types.
+  // TODO(skrul): Change this to support multiple data type controllers.
+  data_type_controllers_[syncable::BOOKMARKS]->Start(
+      merge_allowed,
+      NewCallback(this, &ProfileSyncService::BookmarkStartCallback));
+}
+
+void ProfileSyncService::BookmarkStartCallback(
+    DataTypeController::StartResult result) {
+  switch (result) {
+    case DataTypeController::OK:
+    case DataTypeController::OK_FIRST_RUN: {
+      wizard_.Step(result == DataTypeController::OK ? SyncSetupWizard::DONE :
+                   SyncSetupWizard::DONE_FIRST_TIME);
+      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+      break;
+    }
+
+    case DataTypeController::NEEDS_MERGE: {
       ProfileSyncService::SyncEvent(MERGE_AND_SYNC_NEEDED);
       wizard_.Step(SyncSetupWizard::MERGE_AND_SYNC);
-      return;
+      break;
     }
-  }
 
-  // We're ready to merge the models.
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  bool not_first_run = model_associator_->SyncModelHasUserCreatedNodes();
-  bool merge_success = model_associator_->AssociateModels();
-  UMA_HISTOGRAM_TIMES("Sync.BookmarkAssociationTime",
-                      base::TimeTicks::Now() - start_time);
-  if (!merge_success) {
-      LOG(ERROR) << "Model association failed.";
+    default: {
+      LOG(ERROR) << "Bookmark start failed";
       OnUnrecoverableError();
-      return;
+    }
   }
+}
 
-  wizard_.Step(not_first_run ? SyncSetupWizard::DONE :
-               SyncSetupWizard::DONE_FIRST_TIME);
+void ProfileSyncService::ActivateDataType(
+    DataTypeController* data_type_controller,
+    ChangeProcessor* change_processor) {
+  change_processor->Start(profile(), backend_->GetUserShareHandle());
+  backend_->ActivateDataType(data_type_controller, change_processor);
+}
 
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    (*it)->Start(profile(), backend_->GetUserShareHandle());
-  }
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+void ProfileSyncService::DeactivateDataType(
+    DataTypeController* data_type_controller,
+    ChangeProcessor* change_processor) {
+  change_processor->Stop();
+  backend_->DeactivateDataType(data_type_controller, change_processor);
 }
 
 void ProfileSyncService::AddObserver(Observer* observer) {
@@ -497,9 +452,9 @@ bool ProfileSyncService::ShouldPushChanges() {
   // True only after all bootstrapping has succeeded: the bookmark model is
   // loaded, the sync backend is initialized, the two domains are
   // consistent with one another, and no unrecoverable error has transpired.
-  for (std::set<ChangeProcessor*>::const_iterator it =
-       change_processors_.begin(); it != change_processors_.end(); ++it) {
-    if (!(*it)->IsRunning()) return false;
+  if (data_type_controllers_.count(syncable::BOOKMARKS)) {
+    return data_type_controllers_[syncable::BOOKMARKS]->state() ==
+        DataTypeController::RUNNING;
   }
-  return true;
+  return false;
 }
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 33be503..0c1f8ae 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -7,27 +7,25 @@
 
 #include <string>
 #include <map>
-#include <vector>
 
 #include "base/basictypes.h"
-#include "base/file_path.h"
 #include "base/observer_list.h"
 #include "base/scoped_ptr.h"
+#include "base/time.h"
 #include "chrome/browser/google_service_auth_error.h"
-#include "chrome/browser/sync/glue/change_processor.h"
-#include "chrome/browser/sync/glue/model_associator.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"  // For StartResult.
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/sync_setup_wizard.h"
-#include "chrome/common/notification_registrar.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
-class CommandLine;
-class MessageLoop;
-class Profile;
-
 namespace browser_sync {
 
+class ChangeProcessor;
+class DataTypeController;
+
 class UnrecoverableErrorHandler {
  public:
   // Call this when normal operation detects that the bookmark model and the
@@ -58,10 +56,11 @@ class ProfileSyncServiceObserver {
 
 // ProfileSyncService is the layer between browser subsystems like bookmarks,
 // and the sync backend.
-class ProfileSyncService : public NotificationObserver,
-                           public browser_sync::SyncFrontend,
+class ProfileSyncService : public browser_sync::SyncFrontend,
                            public browser_sync::UnrecoverableErrorHandler {
  public:
+  typedef std::map<syncable::ModelType, browser_sync::DataTypeController*>
+      DataTypeControllerMap;
   typedef ProfileSyncServiceObserver Observer;
   typedef browser_sync::SyncBackendHost::Status Status;
 
@@ -96,6 +95,17 @@ class ProfileSyncService : public NotificationObserver,
   // class is constructed.
   void Initialize();
 
+  // Registers a data type controller with the sync service.  This
+  // makes the data type controller available for use, it does not
+  // enable or activate the synchronization of the data type (see
+  // ActivateDataType).  Takes ownership of the pointer.
+  void RegisterDataTypeController(
+      browser_sync::DataTypeController* data_type_controller);
+
+  const DataTypeControllerMap& data_type_controllers() const {
+    return data_type_controllers_;
+  }
+
   // Enables/disables sync for user.
   virtual void EnableForUser();
   virtual void DisableForUser();
@@ -104,11 +114,6 @@ class ProfileSyncService : public NotificationObserver,
   bool HasSyncSetupCompleted() const;
   void SetSyncSetupCompleted();
 
-  // NotificationObserver implementation.
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
   // SyncFrontend implementation.
   virtual void OnBackendInitialized();
   virtual void OnSyncCycleCompleted();
@@ -204,6 +209,13 @@ class ProfileSyncService : public NotificationObserver,
 
   browser_sync::SyncBackendHost* backend() { return backend_.get(); }
 
+  virtual void ActivateDataType(
+      browser_sync::DataTypeController* data_type_controller,
+      browser_sync::ChangeProcessor* change_processor);
+  virtual void DeactivateDataType(
+      browser_sync::DataTypeController* data_type_controller,
+      browser_sync::ChangeProcessor* change_processor);
+
  protected:
   // Call this after any of the subsystems being synced (the bookmark
   // model and the sync backend) finishes its initialization.  When everything
@@ -225,30 +237,14 @@ class ProfileSyncService : public NotificationObserver,
   void RegisterPreferences();
   void ClearPreferences();
 
+  void BookmarkStartCallback(
+      browser_sync::DataTypeController::StartResult result);
+
   // Tests need to override this.  If |delete_sync_data_folder| is true, then
   // this method will delete all previous "Sync Data" folders. (useful if the
   // folder is partial/corrupt)
   virtual void InitializeBackend(bool delete_sync_data_folder);
 
-  template <class AssociatorImpl, class ChangeProcessorImpl>
-  void InstallGlue() {
-    model_associator_->CreateAndRegisterPerDataTypeImpl<AssociatorImpl>();
-    // TODO(tim): Keep a map instead of a set so we can register/unregister
-    // data type specific ChangeProcessors, once we have more than one.
-    STLDeleteElements(processors());
-    ChangeProcessorImpl* processor = new ChangeProcessorImpl(this);
-    change_processors_.insert(processor);
-    processor->set_model_associator(
-        model_associator_->GetImpl<AssociatorImpl>());
-  }
-
-  browser_sync::ModelAssociator* associator() {
-    return model_associator_.get();
-  }
-  std::set<browser_sync::ChangeProcessor*>* processors() {
-    return &change_processors_;
-  }
-
   // We keep track of the last auth error observed so we can cover up the first
   // "expected" auth failure from observers.
   // TODO(timsteele): Same as expecting_first_run_auth_needed_event_. Remove
@@ -267,9 +263,6 @@ class ProfileSyncService : public NotificationObserver,
   // Initializes the various settings from the command line.
   void InitSettings();
 
-  // Whether the sync merge warning should be shown.
-  bool MergeAndSyncAcceptanceNeeded() const;
-
   // Sets the last synced time to the current time.
   void UpdateLastSyncedTime();
 
@@ -298,13 +291,8 @@ class ProfileSyncService : public NotificationObserver,
   // other threads.
   scoped_ptr<browser_sync::SyncBackendHost> backend_;
 
-  // Model association manager instance.
-  scoped_ptr<browser_sync::ModelAssociator> model_associator_;
-
-  // The change processors that handle the different data types.
-  std::set<browser_sync::ChangeProcessor*> change_processors_;
-
-  NotificationRegistrar registrar_;
+  // List of available data type controllers.
+  DataTypeControllerMap data_type_controllers_;
 
   // Whether the SyncBackendHost has been initialized.
   bool backend_initialized_;
diff --git a/chrome/browser/sync/profile_sync_service_mock.h b/chrome/browser/sync/profile_sync_service_mock.h
new file mode 100644
index 0000000..e2b4dde
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_service_mock.h
@@ -0,0 +1,44 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_MOCK_H_
+#define CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_MOCK_H_
+
+#include <string>
+#include "base/string16.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+class ProfileSyncServiceMock : public ProfileSyncService {
+ public:
+  ProfileSyncServiceMock() : ProfileSyncService(NULL) {}
+  virtual ~ProfileSyncServiceMock() {}
+
+  MOCK_METHOD0(EnableForUser, void());
+  MOCK_METHOD0(DisableForUser, void());
+  MOCK_METHOD0(OnBackendInitialized, void());
+  MOCK_METHOD0(OnSyncCycleCompleted, void());
+  MOCK_METHOD0(OnAuthError, void());
+  MOCK_METHOD3(OnUserSubmittedAuth,
+               void(const std::string& username,
+                    const std::string& password,
+                    const std::string& captcha));
+  MOCK_METHOD0(OnUserAcceptedMergeAndSync, void());
+  MOCK_METHOD0(OnUserCancelledDialog, void());
+  MOCK_CONST_METHOD0(GetAuthenticatedUsername, string16());
+  MOCK_METHOD0(OnUnrecoverableError, void());
+  MOCK_METHOD2(ActivateDataType,
+               void(browser_sync::DataTypeController* data_type_controller,
+                    browser_sync::ChangeProcessor* change_processor));
+  MOCK_METHOD2(DeactivateDataType,
+               void(browser_sync::DataTypeController* data_type_controller,
+                    browser_sync::ChangeProcessor* change_processor));
+
+  MOCK_METHOD0(InitializeBackend, void());
+};
+
+#endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_MOCK_H_
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index 6057bf2..584b77d 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -7,22 +7,28 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "base/message_loop.h"
+#include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/string16.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/bookmark_change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_data_type_controller.h"
 #include "chrome/browser/sync/glue/bookmark_model_associator.h"
+#include "chrome/browser/sync/glue/model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/testing_profile.h"
 #include "chrome/test/sync/test_http_bridge_factory.h"
 
 using std::vector;
+using browser_sync::AssociatorInterface;
 using browser_sync::BookmarkChangeProcessor;
 using browser_sync::BookmarkModelAssociator;
 using browser_sync::ChangeProcessor;
@@ -30,6 +36,34 @@ using browser_sync::ModelAssociator;
 using browser_sync::SyncBackendHost;
 using browser_sync::TestHttpBridgeFactory;
 
+class TestProfileSyncFactory : public ProfileSyncFactory {
+ public:
+  TestProfileSyncFactory(ProfileSyncService* profile_sync_service,
+                         AssociatorInterface* model_associator,
+                         ChangeProcessor* change_processor)
+      : profile_sync_service_(profile_sync_service),
+        model_associator_(model_associator),
+        change_processor_(change_processor) {}
+  virtual ~TestProfileSyncFactory() {}
+
+  virtual ProfileSyncService* CreateProfileSyncService() {
+    return profile_sync_service_.release();
+  }
+
+  virtual BookmarkComponents CreateBookmarkComponents(
+      ProfileSyncService* service) {
+    return BookmarkComponents(model_associator_.release(),
+                              change_processor_.release());
+  }
+
+ private:
+  scoped_ptr<ProfileSyncService> profile_sync_service_;
+  scoped_ptr<AssociatorInterface> model_associator_;
+  scoped_ptr<ChangeProcessor> change_processor_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestProfileSyncFactory);
+};
+
 class TestModelAssociator : public BookmarkModelAssociator {
  public:
   explicit TestModelAssociator(ProfileSyncService* service)
@@ -94,7 +128,6 @@ class TestProfileSyncService : public ProfileSyncService {
   }
 
   virtual void InitializeBackend(bool delete_sync_data_folder) {
-    InstallGlue<TestModelAssociator, BookmarkChangeProcessor>();
     TestHttpBridgeFactory* factory = new TestHttpBridgeFactory();
     TestHttpBridgeFactory* factory2 = new TestHttpBridgeFactory();
     backend()->InitializeForTestMode(L"testuser", factory, factory2,
@@ -111,18 +144,11 @@ class TestProfileSyncService : public ProfileSyncService {
     MessageLoop::current()->Quit();
   }
 
+  // TODO(skrul): how to handle this?
   virtual bool MergeAndSyncAcceptanceNeeded() {
     // Never show the dialog.
     return false;
   }
-
-  BookmarkChangeProcessor* change_processor() {
-    // TODO(tim): The service should have a way to get specific processors.
-    return static_cast<BookmarkChangeProcessor*>(*processors()->begin());
-  }
-  BookmarkModelAssociator* model_associator() {
-    return associator()->GetImpl<BookmarkModelAssociator>();
-  }
 };
 
 // FakeServerChange constructs a list of sync_api::ChangeRecords while modifying
@@ -278,7 +304,9 @@ class ProfileSyncServiceTest : public testing::Test {
   ProfileSyncServiceTest()
       : ui_thread_(ChromeThread::UI, &message_loop_),
         file_thread_(ChromeThread::FILE, &message_loop_),
-        model_(NULL) {
+        model_(NULL),
+        model_associator_(NULL),
+        change_processor_(NULL) {
     profile_.reset(new TestingProfile());
     profile_->set_has_history_service(true);
   }
@@ -292,18 +320,28 @@ class ProfileSyncServiceTest : public testing::Test {
   }
 
   BookmarkModelAssociator* associator() {
-    DCHECK(service_.get());
-    return service_->model_associator();
+    return model_associator_;
   }
 
   BookmarkChangeProcessor* change_processor() {
-    DCHECK(service_.get());
-    return service_->change_processor();
+    return change_processor_;
   }
 
   void StartSyncService() {
     if (!service_.get()) {
       service_.reset(new TestProfileSyncService(profile_.get()));
+
+      // Register the bookmark data type.
+      model_associator_ = new TestModelAssociator(service_.get());
+      change_processor_ = new BookmarkChangeProcessor(model_associator_,
+                                                      service_.get());
+      factory_.reset(new TestProfileSyncFactory(NULL,
+                                                model_associator_,
+                                                change_processor_));
+      service_->RegisterDataTypeController(
+          new browser_sync::BookmarkDataTypeController(factory_.get(),
+                                                       profile_.get(),
+                                                       service_.get()));
       service_->Initialize();
     }
     // The service may have already started sync automatically if it's already
@@ -483,7 +521,10 @@ class ProfileSyncServiceTest : public testing::Test {
 
   scoped_ptr<TestProfileSyncService> service_;
   scoped_ptr<TestingProfile> profile_;
+  scoped_ptr<TestProfileSyncFactory> factory_;
   BookmarkModel* model_;
+  TestModelAssociator* model_associator_;
+  BookmarkChangeProcessor* change_processor_;
 };
 
 TEST_F(ProfileSyncServiceTest, InitialState) {
@@ -740,6 +781,7 @@ TEST_F(ProfileSyncServiceTest, DISABLED_ServerChangeWithInvalidURL) {
   ExpectModelMatch();
 }
 
+
 // Test strings that might pose a problem if the titles ever became used as
 // file names in the sync backend.
 TEST_F(ProfileSyncServiceTest, CornerCaseNames) {
@@ -841,18 +883,15 @@ TEST_F(ProfileSyncServiceTest, UnrecoverableErrorSuspendsService) {
   EXPECT_TRUE(service_->ShouldPushChanges());
 
   // Add a child to the inconsistent node.  This should cause detection of the
-  // problem.
-  const BookmarkNode* nested = model_->AddGroup(node, 0, L"nested");
+  // problem and the syncer should stop processing changes.
+  model_->AddGroup(node, 0, L"nested");
   EXPECT_FALSE(service_->ShouldPushChanges());
-  ExpectSyncerNodeUnknown(nested);
 
   // Try to add a node under a totally different parent.  This should also
   // fail -- the ProfileSyncService should stop processing changes after
   // encountering a consistency violation.
-  const BookmarkNode* unrelated = model_->AddGroup(
-      model_->GetBookmarkBarNode(), 0, L"unrelated");
+  model_->AddGroup(model_->GetBookmarkBarNode(), 0, L"unrelated");
   EXPECT_FALSE(service_->ShouldPushChanges());
-  ExpectSyncerNodeUnknown(unrelated);
 
   // TODO(ncarter): We ought to test the ProfileSyncService state machine
   // directly here once that's formalized and exposed.
@@ -1327,6 +1366,7 @@ TEST_F(ProfileSyncServiceTestWithData, RecoverAfterDeletingSyncDataDirectory) {
 
 // Make sure that things still work if sync is not enabled, but some old sync
 // databases are lingering in the "Sync Data" folder.
+
 TEST_F(ProfileSyncServiceTestWithData, MAYBE_TestStartupWithOldSyncData) {
   const char* nonsense1 = "reginald";
   const char* nonsense2 = "beartato";
@@ -1345,6 +1385,18 @@ TEST_F(ProfileSyncServiceTestWithData, MAYBE_TestStartupWithOldSyncData) {
   if (!service_.get()) {
     service_.reset(new TestProfileSyncService(profile_.get()));
     profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
+
+    model_associator_ = new TestModelAssociator(service_.get());
+    change_processor_ = new BookmarkChangeProcessor(model_associator_,
+                                                    service_.get());
+    factory_.reset(new TestProfileSyncFactory(NULL,
+                                              model_associator_,
+                                              change_processor_));
+    service_->RegisterDataTypeController(
+        new browser_sync::BookmarkDataTypeController(factory_.get(),
+                                                     profile_.get(),
+                                                     service_.get()));
+
     service_->Initialize(); // will call disableForUser because sync setup
                             // hasn't been completed.
   }
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index eae3c48..0d1208c 100755
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1661,10 +1661,14 @@
         'browser/sync/engine/syncapi.h',
         'browser/sync/glue/bookmark_change_processor.h',
         'browser/sync/glue/bookmark_change_processor.cc',
+        'browser/sync/glue/bookmark_change_processor.h',
+        'browser/sync/glue/bookmark_data_type_controller.cc',
+        'browser/sync/glue/bookmark_data_type_controller.h',
         'browser/sync/glue/bookmark_model_associator.h',
         'browser/sync/glue/bookmark_model_associator.cc',
         'browser/sync/glue/change_processor.cc',
         'browser/sync/glue/change_processor.h',
+        'browser/sync/glue/data_type_controller.h',
         'browser/sync/glue/database_model_worker.cc',
         'browser/sync/glue/database_model_worker.h',
         'browser/sync/glue/http_bridge.cc',
@@ -1680,6 +1684,9 @@
         'browser/sync/glue/ui_model_worker.h',
         'browser/sync/profile_sync_service.cc',
         'browser/sync/profile_sync_service.h',
+        'browser/sync/profile_sync_factory.h',
+        'browser/sync/profile_sync_factory_impl.cc',
+        'browser/sync/profile_sync_factory_impl.h',
         'browser/sync/sync_setup_flow.cc',
         'browser/sync/sync_setup_flow.h',
         'browser/sync/sync_setup_wizard.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 5f6fa02..1520411 100755
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -804,10 +804,15 @@
         'browser/shell_integration_unittest.cc',
         'browser/spellchecker_platform_engine_unittest.cc',
         'browser/ssl/ssl_host_state_unittest.cc',
+        'browser/sync/glue/change_processor_mock.h',
         'browser/sync/glue/database_model_worker_unittest.cc',
         'browser/sync/glue/http_bridge_unittest.cc',
         'browser/sync/glue/ui_model_worker_unittest.cc',
+        'browser/sync/profile_sync_service_mock.h',
         'browser/sync/profile_sync_service_unittest.cc',
+        'browser/sync/profile_sync_factory_impl_unittest.cc',
+        'browser/sync/profile_sync_factory_mock.cc',
+        'browser/sync/profile_sync_factory_mock.h',
         'browser/sync/sync_setup_wizard_unittest.cc',
         'browser/sync/sync_ui_util_mac_unittest.mm',
         'browser/tab_contents/navigation_controller_unittest.cc',
@@ -1454,8 +1459,12 @@
         'browser/sync/engine/syncer_thread_unittest.cc',
         'browser/sync/engine/syncer_unittest.cc',
         'browser/sync/engine/syncproto_unittest.cc',
+        'browser/sync/glue/bookmark_data_type_controller_unittest.cc',
+        'browser/sync/glue/change_processor_mock.h',
         'browser/sync/notifier/base/mac/network_status_detector_task_mac_unittest.cc',
         'browser/sync/notifier/listener/talk_mediator_unittest.cc',
+        'browser/sync/profile_sync_factory_mock.cc',
+        'browser/sync/profile_sync_factory_mock.h',
         'browser/sync/sessions/status_controller_unittest.cc',
         'browser/sync/sessions/sync_session_unittest.cc',
         'browser/sync/syncable/directory_backing_store_unittest.cc',
@@ -1491,6 +1500,7 @@
         'common',
         'debugger',
         '../skia/skia.gyp:skia',
+        '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         'syncapi',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 8b8fa20..69087d9 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -151,9 +151,12 @@ const char kDisableSharedWorkers[]          = "disable-shared-workers";
 // Disable site-specific tailoring to compatibility issues in WebKit.
 const char kDisableSiteSpecificQuirks[]     = "disable-site-specific-quirks";
 
-// Disable syncing bookmarks to a Google Account.
+// Disable syncing browser data to a Google Account.
 const char kDisableSync[]                   = "disable-sync";
 
+// Disable syncing of bookmarks.
+const char kDisableSyncBookmarks[]          = "disable-sync-bookmarks";
+
 // Enables the backend service for web resources, used in the new tab page for
 // loading tips and recommendations from a JSON feed.
 const char kDisableWebResources[]           = "disable-web-resources";
@@ -268,9 +271,12 @@ const char kEnableSessionStorage[]          = "enable-session-storage";
 // Enables StatsTable, logging statistics to a global named shared memory table.
 const char kEnableStatsTable[]              = "enable-stats-table";
 
-// Enable syncing bookmarks to a Google Account.
+// Enable syncing browser data to a Google Account.
 const char kEnableSync[]                    = "enable-sync";
 
+// Enable syncing browser bookmarks.
+const char kEnableSyncBookmarks[]           = "enable-sync-bookmarks";
+
 // Whether the multiple profiles feature based on the user-data-dir flag is
 // enabled or not.
 const char kEnableUserDataDirProfiles[]     = "enable-udd-profiles";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 141082e..953c5cb 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -59,6 +59,7 @@ extern const char kDisableRemoteFonts[];
 extern const char kDisableSharedWorkers[];
 extern const char kDisableSiteSpecificQuirks[];
 extern const char kDisableSync[];
+extern const char kDisableSyncBookmarks[];
 extern const char kDisableWebResources[];
 extern const char kDisableWebSecurity[];
 extern const char kDisableWebSockets[];
@@ -95,6 +96,7 @@ extern const char kEnableSeccompSandbox[];
 extern const char kEnableSessionStorage[];
 extern const char kEnableStatsTable[];
 extern const char kEnableSync[];
+extern const char kEnableSyncBookmarks[];
 extern const char kEnableUserDataDirProfiles[];
 extern const char kEnableVerticalTabs[];
 extern const char kEnableWatchdog[];