Create preferences DTC and wire it into the profile sync factory.

Also includes a small valgrind error fix.

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

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

21 files changed, 419 insertions, 49 deletions

diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
index d41ec3e..4b7d9d6 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.cc
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -23,6 +23,8 @@ BookmarkChangeProcessor::BookmarkChangeProcessor(
     : ChangeProcessor(error_handler),
       bookmark_model_(NULL),
       model_associator_(model_associator) {
+  DCHECK(model_associator);
+  DCHECK(error_handler);
 }
 
 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 5ae1811..5e26dac 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.h
+++ b/chrome/browser/sync/glue/bookmark_change_processor.h
@@ -25,8 +25,8 @@ namespace browser_sync {
 class BookmarkChangeProcessor : public BookmarkModelObserver,
                                 public ChangeProcessor {
  public:
-  explicit BookmarkChangeProcessor(BookmarkModelAssociator* model_associator,
-                                   UnrecoverableErrorHandler* error_handler);
+  BookmarkChangeProcessor(BookmarkModelAssociator* model_associator,
+                          UnrecoverableErrorHandler* error_handler);
   virtual ~BookmarkChangeProcessor() {}
 
   // BookmarkModelObserver implementation.
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller.cc b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
index b37c899..744b7e6 100644
--- a/chrome/browser/sync/glue/bookmark_data_type_controller.cc
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
@@ -101,10 +101,10 @@ 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);
+  ProfileSyncFactory::SyncComponents sync_components =
+      profile_sync_factory_->CreateBookmarkSyncComponents(sync_service_);
+  model_associator_.reset(sync_components.model_associator);
+  change_processor_.reset(sync_components.change_processor);
 
   bool needs_merge =  model_associator_->ChromeModelHasUserCreatedNodes() &&
       model_associator_->SyncModelHasUserCreatedNodes();
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc
index 26f025b..4598d66 100644
--- a/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc
@@ -68,7 +68,7 @@ class BookmarkDataTypeControllerTest : public testing::Test {
   }
 
   void SetAssociateExpectations() {
-    EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+    EXPECT_CALL(*profile_sync_factory_, CreateBookmarkSyncComponents(_));
     EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
         WillRepeatedly(Return(false));
     EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
@@ -148,7 +148,7 @@ TEST_F(BookmarkDataTypeControllerTest, StartBusy) {
 
 TEST_F(BookmarkDataTypeControllerTest, StartNeedsMerge) {
   SetStartExpectations();
-  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkSyncComponents(_));
   EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
       WillRepeatedly(Return(true));
   EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
@@ -175,7 +175,7 @@ TEST_F(BookmarkDataTypeControllerTest, StartMergeAllowed) {
 TEST_F(BookmarkDataTypeControllerTest, StartAssociationFailed) {
   SetStartExpectations();
   // Set up association to fail.
-  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkComponents(_));
+  EXPECT_CALL(*profile_sync_factory_, CreateBookmarkSyncComponents(_));
   EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
       WillRepeatedly(Return(false));
   EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
diff --git a/chrome/browser/sync/glue/preference_change_processor.cc b/chrome/browser/sync/glue/preference_change_processor.cc
index 4252a6f..d16b7e6 100644
--- a/chrome/browser/sync/glue/preference_change_processor.cc
+++ b/chrome/browser/sync/glue/preference_change_processor.cc
@@ -6,6 +6,7 @@
 
 #include "base/string_util.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/sync/glue/preference_model_associator.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/protocol/preference_specifics.pb.h"
 #include "chrome/common/json_value_serializer.h"
@@ -15,10 +16,13 @@
 namespace browser_sync {
 
 PreferenceChangeProcessor::PreferenceChangeProcessor(
+    PreferenceModelAssociator* model_associator,
     UnrecoverableErrorHandler* error_handler)
     : ChangeProcessor(error_handler),
       pref_service_(NULL),
-      model_associator_(NULL) {
+      model_associator_(model_associator) {
+  DCHECK(model_associator);
+  DCHECK(error_handler);
 }
 
 void PreferenceChangeProcessor::Observe(NotificationType type,
@@ -148,7 +152,7 @@ void PreferenceChangeProcessor::StopImpl() {
 
 
 void PreferenceChangeProcessor::StartObserving() {
-  DCHECK(model_associator_ && pref_service_);
+  DCHECK(pref_service_);
   for (std::set<std::wstring>::const_iterator it =
       model_associator_->synced_preferences().begin();
       it != model_associator_->synced_preferences().end(); ++it) {
@@ -157,7 +161,7 @@ void PreferenceChangeProcessor::StartObserving() {
 }
 
 void PreferenceChangeProcessor::StopObserving() {
-  DCHECK(model_associator_ && pref_service_);
+  DCHECK(pref_service_);
   for (std::set<std::wstring>::const_iterator it =
       model_associator_->synced_preferences().begin();
       it != model_associator_->synced_preferences().end(); ++it) {
diff --git a/chrome/browser/sync/glue/preference_change_processor.h b/chrome/browser/sync/glue/preference_change_processor.h
index e3b7a96..c9787c8 100644
--- a/chrome/browser/sync/glue/preference_change_processor.h
+++ b/chrome/browser/sync/glue/preference_change_processor.h
@@ -8,13 +8,13 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/glue/change_processor.h"
-#include "chrome/browser/sync/glue/preference_model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/pref_service.h"
 
 namespace browser_sync {
 
+class PreferenceModelAssociator;
 class UnrecoverableErrorHandler;
 
 // This class is responsible for taking changes from the PrefService and
@@ -23,13 +23,10 @@ class UnrecoverableErrorHandler;
 class PreferenceChangeProcessor : public ChangeProcessor,
                                   public NotificationObserver {
  public:
-  explicit PreferenceChangeProcessor(UnrecoverableErrorHandler* error_handler);
+  PreferenceChangeProcessor(PreferenceModelAssociator* model_associator,
+                            UnrecoverableErrorHandler* error_handler);
   virtual ~PreferenceChangeProcessor() {}
 
-  void set_model_associator(PreferenceModelAssociator* model_associator) {
-    model_associator_ = model_associator;
-  }
-
   // NotificationObserver implementation.
   // PrefService -> sync_api model change application.
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/sync/glue/preference_data_type_controller.cc b/chrome/browser/sync/glue/preference_data_type_controller.cc
new file mode 100644
index 0000000..312e4f8
--- /dev/null
+++ b/chrome/browser/sync/glue/preference_data_type_controller.cc
@@ -0,0 +1,88 @@
+// 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/histogram.h"
+#include "base/logging.h"
+#include "base/time.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/sync/glue/preference_change_processor.h"
+#include "chrome/browser/sync/glue/preference_data_type_controller.h"
+#include "chrome/browser/sync/glue/preference_model_associator.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_factory.h"
+
+namespace browser_sync {
+
+PreferenceDataTypeController::PreferenceDataTypeController(
+    ProfileSyncFactory* profile_sync_factory,
+    ProfileSyncService* sync_service)
+    : profile_sync_factory_(profile_sync_factory),
+      sync_service_(sync_service),
+      state_(NOT_RUNNING) {
+  DCHECK(profile_sync_factory);
+  DCHECK(sync_service);
+}
+
+PreferenceDataTypeController::~PreferenceDataTypeController() {
+}
+
+void PreferenceDataTypeController::Start(bool merge_allowed,
+                                         StartCallback* start_callback) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  if (state_ != NOT_RUNNING) {
+    start_callback->Run(BUSY);
+    delete start_callback;
+    return;
+  }
+
+  // No additional services need to be started before we can proceed
+  // with model association.
+  ProfileSyncFactory::SyncComponents sync_components =
+      profile_sync_factory_->CreatePreferenceSyncComponents(sync_service_);
+  model_associator_.reset(sync_components.model_associator);
+  change_processor_.reset(sync_components.change_processor);
+  bool needs_merge =  model_associator_->ChromeModelHasUserCreatedNodes() &&
+      model_associator_->SyncModelHasUserCreatedNodes();
+  if (needs_merge && !merge_allowed) {
+    model_associator_.reset();
+    change_processor_.reset();
+    start_callback->Run(NEEDS_MERGE);
+    delete start_callback;
+    return;
+  }
+
+  base::TimeTicks start_time = base::TimeTicks::Now();
+  bool first_run = !model_associator_->SyncModelHasUserCreatedNodes();
+  bool merge_success = model_associator_->AssociateModels();
+  UMA_HISTOGRAM_TIMES("Sync.PreferenceAssociationTime",
+                      base::TimeTicks::Now() - start_time);
+  if (!merge_success) {
+    model_associator_.reset();
+    change_processor_.reset();
+    start_callback->Run(ASSOCIATION_FAILED);
+    delete start_callback;
+    return;
+  }
+
+  sync_service_->ActivateDataType(this, change_processor_.get());
+  state_ = RUNNING;
+  start_callback->Run(first_run ? OK_FIRST_RUN : OK);
+  delete start_callback;
+}
+
+void PreferenceDataTypeController::Stop() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  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;
+}
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/preference_data_type_controller.h b/chrome/browser/sync/glue/preference_data_type_controller.h
new file mode 100644
index 0000000..bffd6b2
--- /dev/null
+++ b/chrome/browser/sync/glue/preference_data_type_controller.h
@@ -0,0 +1,61 @@
+// 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_PREFERENCE_DATA_TYPE_CONTROLLER_H__
+#define CHROME_BROWSER_SYNC_GLUE_PREFERENCE_DATA_TYPE_CONTROLLER_H__
+
+#include "base/basictypes.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
+
+class ProfileSyncFactory;
+class ProfileSyncService;
+
+namespace browser_sync {
+
+class AssociatorInterface;
+class ChangeProcessor;
+
+class PreferenceDataTypeController : public DataTypeController {
+ public:
+  PreferenceDataTypeController(
+      ProfileSyncFactory* profile_sync_factory,
+      ProfileSyncService* sync_service);
+  virtual ~PreferenceDataTypeController();
+
+  virtual void Start(bool merge_allowed, StartCallback* start_callback);
+
+  virtual void Stop();
+
+  virtual bool enabled() {
+    return true;
+  }
+
+  virtual syncable::ModelType type() {
+    return syncable::PREFERENCES;
+  }
+
+  virtual browser_sync::ModelSafeGroup model_safe_group() {
+    return browser_sync::GROUP_UI;
+  }
+
+  virtual State state() {
+    return state_;
+  }
+
+ private:
+  ProfileSyncFactory* profile_sync_factory_;
+  ProfileSyncService* sync_service_;
+
+  State state_;
+
+  scoped_ptr<AssociatorInterface> model_associator_;
+  scoped_ptr<ChangeProcessor> change_processor_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreferenceDataTypeController);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_PREFERENCE_DATA_TYPE_CONTROLLER_H__
diff --git a/chrome/browser/sync/glue/preference_data_type_controller_unittest.cc b/chrome/browser/sync/glue/preference_data_type_controller_unittest.cc
new file mode 100644
index 0000000..47f7cb6
--- /dev/null
+++ b/chrome/browser/sync/glue/preference_data_type_controller_unittest.cc
@@ -0,0 +1,145 @@
+// 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/chrome_thread.h"
+#include "chrome/browser/sync/glue/preference_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"
+
+using browser_sync::PreferenceDataTypeController;
+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 PreferenceDataTypeControllerTest : public testing::Test {
+ public:
+  PreferenceDataTypeControllerTest()
+      : ui_thread_(ChromeThread::UI, &message_loop_) {}
+
+  virtual void SetUp() {
+    profile_sync_factory_.reset(new ProfileSyncFactoryMock());
+    preference_dtc_.reset(
+        new PreferenceDataTypeController(profile_sync_factory_.get(),
+                                         &service_));
+  }
+
+ protected:
+  void SetAssociateExpectations() {
+    model_associator_ = new ModelAssociatorMock();
+    change_processor_ = new ChangeProcessorMock();
+    EXPECT_CALL(*profile_sync_factory_, CreatePreferenceSyncComponents(_)).
+        WillOnce(Return(ProfileSyncFactory::SyncComponents(model_associator_,
+                                                           change_processor_)));
+    EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+        WillRepeatedly(Return(false));
+    EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+        WillRepeatedly(Return(true));
+    EXPECT_CALL(*model_associator_, AssociateModels()).
+        WillRepeatedly(Return(true));
+  }
+
+  void SetActivateExpectations() {
+    EXPECT_CALL(service_, ActivateDataType(_, _));
+  }
+
+  void SetStopExpectations() {
+    EXPECT_CALL(service_, DeactivateDataType(_, _));
+    EXPECT_CALL(*model_associator_, DisassociateModels());
+  }
+
+  MessageLoopForUI message_loop_;
+  ChromeThread ui_thread_;
+  scoped_ptr<PreferenceDataTypeController> preference_dtc_;
+  scoped_ptr<ProfileSyncFactoryMock> profile_sync_factory_;
+  ProfileSyncServiceMock service_;
+  ModelAssociatorMock* model_associator_;
+  ChangeProcessorMock* change_processor_;
+  StartCallback start_callback_;
+};
+
+TEST_F(PreferenceDataTypeControllerTest, Start) {
+  SetAssociateExpectations();
+  SetActivateExpectations();
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state());
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  preference_dtc_->Start(false,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::RUNNING, preference_dtc_->state());
+}
+
+TEST_F(PreferenceDataTypeControllerTest, StartFirstRun) {
+  SetAssociateExpectations();
+  SetActivateExpectations();
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(false));
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN));
+  preference_dtc_->Start(false,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(PreferenceDataTypeControllerTest, StartNeedsMerge) {
+  SetAssociateExpectations();
+  EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::NEEDS_MERGE));
+  preference_dtc_->Start(false,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(PreferenceDataTypeControllerTest, StartMergeAllowed) {
+  SetAssociateExpectations();
+  SetActivateExpectations();
+  EXPECT_CALL(*model_associator_, ChromeModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+  EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes()).
+      WillRepeatedly(Return(true));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  preference_dtc_->Start(true,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+}
+
+TEST_F(PreferenceDataTypeControllerTest, StartAssociationFailed) {
+  SetAssociateExpectations();
+  EXPECT_CALL(*model_associator_, AssociateModels()).
+      WillRepeatedly(Return(false));
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::ASSOCIATION_FAILED));
+  preference_dtc_->Start(true,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state());
+}
+
+TEST_F(PreferenceDataTypeControllerTest, Stop) {
+  SetAssociateExpectations();
+  SetActivateExpectations();
+  SetStopExpectations();
+
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state());
+
+  EXPECT_CALL(start_callback_, Run(DataTypeController::OK));
+  preference_dtc_->Start(false,
+                         NewCallback(&start_callback_, &StartCallback::Run));
+  EXPECT_EQ(DataTypeController::RUNNING, preference_dtc_->state());
+  preference_dtc_->Stop();
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state());
+
+}
diff --git a/chrome/browser/sync/profile_sync_factory.h b/chrome/browser/sync/profile_sync_factory.h
index 107807e..dc728af 100644
--- a/chrome/browser/sync/profile_sync_factory.h
+++ b/chrome/browser/sync/profile_sync_factory.h
@@ -18,11 +18,15 @@ class ChangeProcessor;
 // Factory class for all profile sync related classes.
 class ProfileSyncFactory {
  public:
-  struct BookmarkComponents {
+  // The various factory methods for the data type model associators
+  // and change processors all return this struct.  This is needed
+  // because the change processors typically require a type-specific
+  // model associator at construction time.
+  struct SyncComponents {
     browser_sync::AssociatorInterface* model_associator;
     browser_sync::ChangeProcessor* change_processor;
-    BookmarkComponents(browser_sync::AssociatorInterface* ma,
-                       browser_sync::ChangeProcessor* cp)
+    SyncComponents(browser_sync::AssociatorInterface* ma,
+                   browser_sync::ChangeProcessor* cp)
         : model_associator(ma), change_processor(cp) {}
   };
 
@@ -34,11 +38,15 @@ class ProfileSyncFactory {
   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(
+  // bookmark data type.  The pointers in the return struct are owned
+  // by the caller.
+  virtual SyncComponents CreateBookmarkSyncComponents(
+      ProfileSyncService* profile_sync_service) = 0;
+
+  // Instantiates both a model associator and change processor for the
+  // preference data type.  The pointers in the return struct are
+  // owned by the caller.
+  virtual SyncComponents CreatePreferenceSyncComponents(
       ProfileSyncService* profile_sync_service) = 0;
 };
 
diff --git a/chrome/browser/sync/profile_sync_factory_impl.cc b/chrome/browser/sync/profile_sync_factory_impl.cc
index d7658de..f3a4379 100644
--- a/chrome/browser/sync/profile_sync_factory_impl.cc
+++ b/chrome/browser/sync/profile_sync_factory_impl.cc
@@ -8,11 +8,21 @@
 #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/preference_change_processor.h"
+#include "chrome/browser/sync/glue/preference_data_type_controller.h"
+#include "chrome/browser/sync/glue/preference_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"
 
+using browser_sync::BookmarkDataTypeController;
+using browser_sync::PreferenceDataTypeController;
+using browser_sync::BookmarkChangeProcessor;
+using browser_sync::BookmarkModelAssociator;
+using browser_sync::PreferenceChangeProcessor;
+using browser_sync::PreferenceModelAssociator;
+
 ProfileSyncFactoryImpl::ProfileSyncFactoryImpl(
     Profile* profile,
     CommandLine* command_line)
@@ -22,21 +32,41 @@ ProfileSyncFactoryImpl::ProfileSyncFactoryImpl(
 ProfileSyncService* ProfileSyncFactoryImpl::CreateProfileSyncService() {
   ProfileSyncService* pss = new ProfileSyncService(profile_);
 
-  // Bookmark sync is enabled by default.  Register unless explicitly disabled.
+  // 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));
+        new BookmarkDataTypeController(this, profile_, pss));
+  }
+
+  // Preference sync is disabled by default.  Register only if
+  // explicitly enabled.
+  if (command_line_->HasSwitch(switches::kEnableSyncPreferences)) {
+    pss->RegisterDataTypeController(
+        new PreferenceDataTypeController(this, pss));
   }
+
   return pss;
 }
 
-ProfileSyncFactory::BookmarkComponents
-ProfileSyncFactoryImpl::CreateBookmarkComponents(
+ProfileSyncFactory::SyncComponents
+ProfileSyncFactoryImpl::CreateBookmarkSyncComponents(
+    ProfileSyncService* profile_sync_service) {
+  BookmarkModelAssociator* model_associator =
+      new BookmarkModelAssociator(profile_sync_service);
+  BookmarkChangeProcessor* change_processor =
+      new BookmarkChangeProcessor(model_associator,
+                                  profile_sync_service);
+  return SyncComponents(model_associator, change_processor);
+}
+
+ProfileSyncFactory::SyncComponents
+ProfileSyncFactoryImpl::CreatePreferenceSyncComponents(
     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);
+  PreferenceModelAssociator* model_associator =
+      new PreferenceModelAssociator(profile_sync_service);
+  PreferenceChangeProcessor* change_processor =
+      new PreferenceChangeProcessor(model_associator,
+                                    profile_sync_service);
+  return SyncComponents(model_associator, change_processor);
 }
diff --git a/chrome/browser/sync/profile_sync_factory_impl.h b/chrome/browser/sync/profile_sync_factory_impl.h
index 638a508..a6c9547 100644
--- a/chrome/browser/sync/profile_sync_factory_impl.h
+++ b/chrome/browser/sync/profile_sync_factory_impl.h
@@ -16,9 +16,13 @@ class ProfileSyncFactoryImpl : public ProfileSyncFactory {
   ProfileSyncFactoryImpl(Profile* profile, CommandLine* command_line);
   virtual ~ProfileSyncFactoryImpl() {}
 
+  // ProfileSyncFactory interface.
   virtual ProfileSyncService* CreateProfileSyncService();
 
-  virtual BookmarkComponents CreateBookmarkComponents(
+  virtual SyncComponents CreateBookmarkSyncComponents(
+      ProfileSyncService* profile_sync_service);
+
+  virtual SyncComponents CreatePreferenceSyncComponents(
       ProfileSyncService* profile_sync_service);
 
  private:
diff --git a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
index 1dbbdb3..9645ec0 100644
--- a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
+++ b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
@@ -52,3 +52,14 @@ TEST_F(ProfileSyncFactoryImplTest, CreatePSSDisableBookmarks) {
   EXPECT_EQ(0U, controllers.size());
   EXPECT_EQ(0U, controllers.count(syncable::BOOKMARKS));
 }
+
+TEST_F(ProfileSyncFactoryImplTest, CreatePSSEnablePreferences) {
+  command_line_->AppendSwitch(switches::kEnableSyncPreferences);
+  scoped_ptr<ProfileSyncService> pss;
+  pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
+  ProfileSyncService::DataTypeControllerMap controllers(
+      pss->data_type_controllers());
+  EXPECT_EQ(2U, controllers.size());
+  EXPECT_EQ(1U, controllers.count(syncable::BOOKMARKS));
+  EXPECT_EQ(1U, controllers.count(syncable::PREFERENCES));
+}
diff --git a/chrome/browser/sync/profile_sync_factory_mock.cc b/chrome/browser/sync/profile_sync_factory_mock.cc
index c30331f..1a5179a 100644
--- a/chrome/browser/sync/profile_sync_factory_mock.cc
+++ b/chrome/browser/sync/profile_sync_factory_mock.cc
@@ -16,15 +16,15 @@ ProfileSyncFactoryMock::ProfileSyncFactoryMock(
     ChangeProcessor* bookmark_change_processor)
     : bookmark_model_associator_(bookmark_model_associator),
       bookmark_change_processor_(bookmark_change_processor) {
-  ON_CALL(*this, CreateBookmarkComponents(_)).
+  ON_CALL(*this, CreateBookmarkSyncComponents(_)).
       WillByDefault(
           InvokeWithoutArgs(
               this,
-              &ProfileSyncFactoryMock::MakeBookmarkComponents));
+              &ProfileSyncFactoryMock::MakeBookmarkSyncComponents));
 }
 
-ProfileSyncFactory::BookmarkComponents
-ProfileSyncFactoryMock::MakeBookmarkComponents() {
-  return BookmarkComponents(bookmark_model_associator_.release(),
-                            bookmark_change_processor_.release());
+ProfileSyncFactory::SyncComponents
+ProfileSyncFactoryMock::MakeBookmarkSyncComponents() {
+  return SyncComponents(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
index 3ac7d51..ec2ff2c 100644
--- a/chrome/browser/sync/profile_sync_factory_mock.h
+++ b/chrome/browser/sync/profile_sync_factory_mock.h
@@ -17,17 +17,20 @@ class ChangeProcessor;
 
 class ProfileSyncFactoryMock : public ProfileSyncFactory {
  public:
+  ProfileSyncFactoryMock() {}
   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));
+  MOCK_METHOD1(CreateBookmarkSyncComponents,
+               SyncComponents(ProfileSyncService* profile_sync_service));
+  MOCK_METHOD1(CreatePreferenceSyncComponents,
+               SyncComponents(ProfileSyncService* profile_sync_service));
 
  private:
-  BookmarkComponents MakeBookmarkComponents();
+  SyncComponents MakeBookmarkSyncComponents();
 
   scoped_ptr<browser_sync::AssociatorInterface> bookmark_model_associator_;
   scoped_ptr<browser_sync::ChangeProcessor> bookmark_change_processor_;
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index 2e54c5b..1753370 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -51,10 +51,15 @@ class TestProfileSyncFactory : public ProfileSyncFactory {
     return profile_sync_service_.release();
   }
 
-  virtual BookmarkComponents CreateBookmarkComponents(
+  virtual SyncComponents CreateBookmarkSyncComponents(
       ProfileSyncService* service) {
-    return BookmarkComponents(model_associator_.release(),
-                              change_processor_.release());
+    return SyncComponents(model_associator_.release(),
+                          change_processor_.release());
+  }
+
+  virtual SyncComponents CreatePreferenceSyncComponents(
+      ProfileSyncService* service) {
+    return SyncComponents(NULL, NULL);
   }
 
  private:
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index d16a4d8..ff37766 100755
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1682,6 +1682,8 @@
         'browser/sync/glue/model_associator.h',
         'browser/sync/glue/preference_change_processor.cc',
         'browser/sync/glue/preference_change_processor.h',
+        'browser/sync/glue/preference_data_type_controller.cc',
+        'browser/sync/glue/preference_data_type_controller.h',
         'browser/sync/glue/preference_model_associator.cc',
         'browser/sync/glue/preference_model_associator.h',
         'browser/sync/glue/sync_backend_host.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index fea4898..1a2c4b3 100755
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1463,6 +1463,7 @@
         'browser/sync/engine/syncproto_unittest.cc',
         'browser/sync/glue/bookmark_data_type_controller_unittest.cc',
         'browser/sync/glue/change_processor_mock.h',
+        'browser/sync/glue/preference_data_type_controller_unittest.cc',
         'browser/sync/notifier/base/mac/network_status_detector_task_mac_unittest.cc',
         'browser/sync/notifier/listener/talk_mediator_unittest.cc',
         'browser/sync/notifier/listener/send_update_task_unittest.cc',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index eba0a6e..89b4c59 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -157,6 +157,9 @@ const char kDisableSync[]                   = "disable-sync";
 // Disable syncing of bookmarks.
 const char kDisableSyncBookmarks[]          = "disable-sync-bookmarks";
 
+// Disable syncing of preferences.
+const char kDisableSyncPreferences[]        = "disable-sync-preferences";
+
 // 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";
@@ -277,6 +280,9 @@ const char kEnableSync[]                    = "enable-sync";
 // Enable syncing browser bookmarks.
 const char kEnableSyncBookmarks[]           = "enable-sync-bookmarks";
 
+// Enable syncing browser preferences.
+const char kEnableSyncPreferences[]         = "enable-sync-preferences";
+
 // 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 0cd2873..5fa0fef 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -60,6 +60,7 @@ extern const char kDisableSharedWorkers[];
 extern const char kDisableSiteSpecificQuirks[];
 extern const char kDisableSync[];
 extern const char kDisableSyncBookmarks[];
+extern const char kDisableSyncPreferences[];
 extern const char kDisableWebResources[];
 extern const char kDisableWebSecurity[];
 extern const char kDisableWebSockets[];
@@ -97,6 +98,7 @@ extern const char kEnableSessionStorage[];
 extern const char kEnableStatsTable[];
 extern const char kEnableSync[];
 extern const char kEnableSyncBookmarks[];
+extern const char kEnableSyncPreferences[];
 extern const char kEnableUserDataDirProfiles[];
 extern const char kEnableVerticalTabs[];
 extern const char kEnableWatchdog[];
diff --git a/chrome/test/sync/engine/mock_server_connection.cc b/chrome/test/sync/engine/mock_server_connection.cc
index cbea2d9..ce3b03d 100755
--- a/chrome/test/sync/engine/mock_server_connection.cc
+++ b/chrome/test/sync/engine/mock_server_connection.cc
@@ -46,6 +46,7 @@ MockConnectionManager::MockConnectionManager(DirectoryManager* dirmgr,
       mid_commit_callback_(NULL),
       mid_commit_observer_(NULL),
       throttling_(false),
+      fail_with_auth_invalid_(false),
       fail_non_periodic_get_updates_(false),
       client_command_(NULL),
       next_position_in_parent_(2),