Reverting change: broke unit tests on chrome OS.

Revert 39143 - This change will include preferences as part of the usual startup sequence of the PSS that already includes bookmarks.  This is a temporary solution until we have a proper component to manage the startup and shutdown of multiple data types.

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

TBR=skrul@chromium.org
Review URL: http://codereview.chromium.org/611006

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

11 files changed, 70 insertions, 374 deletions

diff --git a/chrome/browser/sync/glue/data_type_controller.h b/chrome/browser/sync/glue/data_type_controller.h
index 00dbe5b..62709ff 100644
--- a/chrome/browser/sync/glue/data_type_controller.h
+++ b/chrome/browser/sync/glue/data_type_controller.h
@@ -5,8 +5,6 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_H__
 #define CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_H__
 
-#include <map>
-
 #include "base/task.h"
 #include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/syncable/model_type.h"
@@ -45,8 +43,6 @@ class DataTypeController {
 
   typedef Callback1<StartResult>::Type StartCallback;
 
-  typedef std::map<syncable::ModelType, DataTypeController*> TypeMap;
-
   virtual ~DataTypeController() {}
 
   // Begins asynchronous start up of this data type.  Start up will
diff --git a/chrome/browser/sync/glue/data_type_controller_mock.h b/chrome/browser/sync/glue/data_type_controller_mock.h
deleted file mode 100644
index c2fb102..0000000
--- a/chrome/browser/sync/glue/data_type_controller_mock.h
+++ /dev/null
@@ -1,25 +0,0 @@
-// 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_DATA_TYPE_CONTROLLER_MOCK_H__
-#define CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_MOCK_H__
-
-#include "chrome/browser/sync/glue/data_type_controller.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace browser_sync {
-
-class DataTypeControllerMock : public DataTypeController {
- public:
-  MOCK_METHOD2(Start, void(bool merge_allowed, StartCallback* start_callback));
-  MOCK_METHOD0(Stop, void());
-  MOCK_METHOD0(enabled, bool());
-  MOCK_METHOD0(type, syncable::ModelType());
-  MOCK_METHOD0(model_safe_group, browser_sync::ModelSafeGroup());
-  MOCK_METHOD0(state, State());
-};
-
-}  // namespace browser_sync
-
-#endif  // CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_CONTROLLER_MOCK_H__
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 6d16dc30..58adb28 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"
@@ -18,21 +19,16 @@ static const char kGaiaSourceForChrome[] = "ChromiumBrowser";
 static const FilePath::CharType kSyncDataFolderName[] =
     FILE_PATH_LITERAL("Sync Data");
 
-using browser_sync::DataTypeController;
-
 typedef GoogleServiceAuthError AuthError;
 
 namespace browser_sync {
 
-SyncBackendHost::SyncBackendHost(
-    SyncFrontend* frontend,
-    const FilePath& profile_path,
-    const DataTypeController::TypeMap& data_type_controllers)
+SyncBackendHost::SyncBackendHost(SyncFrontend* frontend,
+                                 const FilePath& profile_path)
     : core_thread_("Chrome_SyncCoreThread"),
       frontend_loop_(MessageLoop::current()),
       frontend_(frontend),
       sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
-      data_type_controllers_(data_type_controllers),
       last_auth_error_(AuthError::None()) {
 
   core_ = new Core(this);
@@ -64,11 +60,8 @@ void SyncBackendHost::Initialize(
   // Any datatypes that we want the syncer to pull down must
   // be in the routing_info map.  We set them to group passive, meaning that
   // updates will be applied, but not dispatched to the UI thread yet.
-  for (DataTypeController::TypeMap::const_iterator it =
-           data_type_controllers_.begin();
-       it != data_type_controllers_.end(); ++it) {
-    registrar_.routing_info[(*it).first] = GROUP_PASSIVE;
-  }
+  // TODO(ncarter): Wire this up to some external control.
+  registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
 
   core_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoInitialize,
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 10d1534..1b4b3ce 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -20,7 +20,6 @@
 #include "chrome/browser/sync/notification_method.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/engine/model_safe_worker.h"
-#include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/ui_model_worker.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 #include "googleurl/src/gurl.h"
@@ -73,9 +72,7 @@ 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).
-  SyncBackendHost(SyncFrontend* frontend,
-                  const FilePath& profile_path,
-                  const DataTypeController::TypeMap& data_type_controllers);
+  SyncBackendHost(SyncFrontend* frontend, const FilePath& profile_path);
   ~SyncBackendHost();
 
   // Called on |frontend_loop_| to kick off asynchronous initialization.
@@ -361,9 +358,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
   // Path of the folder that stores the sync data files.
   FilePath sync_data_folder_path_;
 
-  // List of registered data type controllers.
-  DataTypeController::TypeMap data_type_controllers_;
-
   // UI-thread cache of the last AuthErrorState received from syncapi.
   GoogleServiceAuthError last_auth_error_;
 
diff --git a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
index 2913c81..9645ec0 100644
--- a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
+++ b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
@@ -9,14 +9,11 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/sync/engine/syncapi.h"
-#include "chrome/browser/sync/glue/data_type_controller.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"
 
-using browser_sync::DataTypeController;
-
 class ProfileSyncFactoryImplTest : public testing::Test {
  protected:
   ProfileSyncFactoryImplTest()
@@ -40,7 +37,8 @@ class ProfileSyncFactoryImplTest : public testing::Test {
 TEST_F(ProfileSyncFactoryImplTest, CreatePSSDefault) {
   scoped_ptr<ProfileSyncService> pss;
   pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
-  DataTypeController::TypeMap controllers(pss->data_type_controllers());
+  ProfileSyncService::DataTypeControllerMap controllers(
+      pss->data_type_controllers());
   EXPECT_EQ(1U, controllers.size());
   EXPECT_EQ(1U, controllers.count(syncable::BOOKMARKS));
 }
@@ -49,7 +47,8 @@ TEST_F(ProfileSyncFactoryImplTest, CreatePSSDisableBookmarks) {
   command_line_->AppendSwitch(switches::kDisableSyncBookmarks);
   scoped_ptr<ProfileSyncService> pss;
   pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
-  DataTypeController::TypeMap controllers(pss->data_type_controllers());
+  ProfileSyncService::DataTypeControllerMap controllers(
+      pss->data_type_controllers());
   EXPECT_EQ(0U, controllers.size());
   EXPECT_EQ(0U, controllers.count(syncable::BOOKMARKS));
 }
@@ -58,7 +57,8 @@ TEST_F(ProfileSyncFactoryImplTest, CreatePSSEnablePreferences) {
   command_line_->AppendSwitch(switches::kEnableSyncPreferences);
   scoped_ptr<ProfileSyncService> pss;
   pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
-  DataTypeController::TypeMap controllers(pss->data_type_controllers());
+  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_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 9932d72..c6baf7f 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -43,7 +43,6 @@ ProfileSyncService::ProfileSyncService(Profile* profile)
       is_auth_in_progress_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(wizard_(this)),
       unrecoverable_error_detected_(false),
-      startup_had_first_time_(false),
       notification_method_(browser_sync::kDefaultNotificationMethod) {
 }
 
@@ -163,8 +162,7 @@ void ProfileSyncService::StartUp() {
   last_synced_time_ = base::Time::FromInternalValue(
       profile_->GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
 
-  backend_.reset(
-      new SyncBackendHost(this, profile_->GetPath(), data_type_controllers_));
+  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,8 +176,8 @@ void ProfileSyncService::Shutdown(bool sync_disabled) {
 
   // Stop all data type controllers.
   // TODO(skrul): Change this to support multiple data type controllers.
-  StopDataType(syncable::BOOKMARKS);
-  StopDataType(syncable::PREFERENCES);
+  if (data_type_controllers_.count(syncable::BOOKMARKS))
+    data_type_controllers_[syncable::BOOKMARKS]->Stop();
 
   backend_.reset();
 
@@ -235,8 +233,7 @@ void ProfileSyncService::OnUnrecoverableError() {
   unrecoverable_error_detected_ = true;
 
   // TODO(skrul): Change this to support multiple data type controllers.
-  StopDataType(syncable::BOOKMARKS);
-  StopDataType(syncable::PREFERENCES);
+  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);
@@ -351,14 +348,6 @@ string16 ProfileSyncService::GetAuthenticatedUsername() const {
   return backend_->GetAuthenticatedUsername();
 }
 
-void ProfileSyncService::StopDataType(syncable::ModelType model_type) {
-  if (data_type_controllers_.count(model_type) &&
-      data_type_controllers_[model_type]->state() !=
-        DataTypeController::NOT_RUNNING) {
-    data_type_controllers_[model_type]->Stop();
-  }
-}
-
 void ProfileSyncService::OnUserSubmittedAuth(
     const std::string& username, const std::string& password,
     const std::string& captcha) {
@@ -391,7 +380,6 @@ void ProfileSyncService::OnUserCancelledDialog() {
 
 void ProfileSyncService::StartProcessingChangesIfReady() {
   DCHECK(backend_initialized_);
-  startup_had_first_time_ = false;
 
   // If the user has completed sync setup, we are always allowed to
   // merge data.
@@ -407,17 +395,9 @@ void ProfileSyncService::StartProcessingChangesIfReady() {
 
   // Start data types.
   // TODO(skrul): Change this to support multiple data type controllers.
-  if (data_type_controllers_.count(syncable::BOOKMARKS)) {
-    data_type_controllers_[syncable::BOOKMARKS]->Start(
-        merge_allowed,
-        NewCallback(this, &ProfileSyncService::BookmarkStartCallback));
-  } else {
-    if (data_type_controllers_.count(syncable::PREFERENCES)) {
-      data_type_controllers_[syncable::PREFERENCES]->Start(
-          true,
-          NewCallback(this, &ProfileSyncService::PreferenceStartCallback));
-    }
-  }
+  data_type_controllers_[syncable::BOOKMARKS]->Start(
+      merge_allowed,
+      NewCallback(this, &ProfileSyncService::BookmarkStartCallback));
 }
 
 void ProfileSyncService::BookmarkStartCallback(
@@ -425,22 +405,9 @@ void ProfileSyncService::BookmarkStartCallback(
   switch (result) {
     case DataTypeController::OK:
     case DataTypeController::OK_FIRST_RUN: {
-      startup_had_first_time_ |= result == DataTypeController::OK_FIRST_RUN;
-
-      // If the preference data type was registered, start it here.
-      // Since we only care about presenting the merge warning dialog
-      // for bookmarks, pass a "true" for merge_allowed so preferences
-      // will always start.  If preferences is not registered, just
-      // call the callback directly so we can finish startup.
-      // TODO(skrul): Change this to support multiple data type
-      // controllers.
-      if (data_type_controllers_.count(syncable::PREFERENCES)) {
-        data_type_controllers_[syncable::PREFERENCES]->Start(
-            true,
-            NewCallback(this, &ProfileSyncService::PreferenceStartCallback));
-      } else {
-        PreferenceStartCallback(DataTypeController::OK);
-      }
+      wizard_.Step(result == DataTypeController::OK ? SyncSetupWizard::DONE :
+                   SyncSetupWizard::DONE_FIRST_TIME);
+      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
       break;
     }
 
@@ -453,27 +420,6 @@ void ProfileSyncService::BookmarkStartCallback(
     default: {
       LOG(ERROR) << "Bookmark start failed";
       OnUnrecoverableError();
-      break;
-    }
-  }
-}
-
-void ProfileSyncService::PreferenceStartCallback(
-    DataTypeController::StartResult result) {
-  switch (result) {
-    case DataTypeController::OK:
-    case DataTypeController::OK_FIRST_RUN: {
-      startup_had_first_time_ |= result == DataTypeController::OK_FIRST_RUN;
-
-      wizard_.Step(startup_had_first_time_ ? SyncSetupWizard:: DONE_FIRST_TIME :
-                   SyncSetupWizard::DONE);
-      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
-      break;
-    }
-    default: {
-      LOG(ERROR) << "Preference start failed";
-      OnUnrecoverableError();
-      break;
     }
   }
 }
@@ -514,23 +460,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.
-
-  // Don't push changes if there are no data type controllers registered.
-  if (data_type_controllers_.size() == 0)
-    return false;
-
-  // TODO: make this size_t
-  DataTypeController::TypeMap::size_type running_data_type_controllers = 0;
-  if (data_type_controllers_.count(syncable::BOOKMARKS) &&
-      data_type_controllers_[syncable::BOOKMARKS]->state() ==
-      DataTypeController::RUNNING)
-    running_data_type_controllers++;
-
-  if (data_type_controllers_.count(syncable::PREFERENCES) &&
-      data_type_controllers_[syncable::PREFERENCES]->state() ==
-      DataTypeController::RUNNING)
-    running_data_type_controllers++;
-
-  // Return true only if all data type controllers are running.
-  return data_type_controllers_.size() == running_data_type_controllers;
+  if (data_type_controllers_.count(syncable::BOOKMARKS)) {
+    return data_type_controllers_[syncable::BOOKMARKS]->state() ==
+        DataTypeController::RUNNING;
+  }
+  return false;
 }
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 569db52..349e819f 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -14,7 +14,7 @@
 #include "base/time.h"
 #include "chrome/browser/google_service_auth_error.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/sync/glue/data_type_controller.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/notification_method.h"
 #include "chrome/browser/sync/sync_setup_wizard.h"
@@ -25,6 +25,7 @@
 namespace browser_sync {
 
 class ChangeProcessor;
+class DataTypeController;
 
 class UnrecoverableErrorHandler {
  public:
@@ -59,6 +60,8 @@ class ProfileSyncServiceObserver {
 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;
 
@@ -100,8 +103,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   void RegisterDataTypeController(
       browser_sync::DataTypeController* data_type_controller);
 
-  const browser_sync::DataTypeController::TypeMap& data_type_controllers()
-      const {
+  const DataTypeControllerMap& data_type_controllers() const {
     return data_type_controllers_;
   }
 
@@ -238,8 +240,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
 
   void BookmarkStartCallback(
       browser_sync::DataTypeController::StartResult result);
-  void PreferenceStartCallback(
-      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
@@ -271,9 +271,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // store to bootstrap the authentication process.
   std::string GetLsidForAuthBootstraping();
 
-  // Stops a data type.
-  void StopDataType(syncable::ModelType model_type);
-
   // Time at which we begin an attempt a GAIA authorization.
   base::TimeTicks auth_start_time_;
 
@@ -296,7 +293,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   scoped_ptr<browser_sync::SyncBackendHost> backend_;
 
   // List of available data type controllers.
-  browser_sync::DataTypeController::TypeMap data_type_controllers_;
+  DataTypeControllerMap data_type_controllers_;
 
   // Whether the SyncBackendHost has been initialized.
   bool backend_initialized_;
@@ -325,11 +322,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // doing any work that might corrupt things further.
   bool unrecoverable_error_detected_;
 
-  // True if at least one of the data types started up was started for
-  // the first time.  TODO(sync): Remove this when we have full
-  // support for starting multiple data types.
-  bool startup_had_first_time_;
-
   // Which peer-to-peer notification method to use.
   browser_sync::NotificationMethod notification_method_;
 
diff --git a/chrome/browser/sync/profile_sync_service_startup_unittest.cc b/chrome/browser/sync/profile_sync_service_startup_unittest.cc
deleted file mode 100644
index 388e23f..0000000
--- a/chrome/browser/sync/profile_sync_service_startup_unittest.cc
+++ /dev/null
@@ -1,176 +0,0 @@
-// 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 "chrome/browser/chrome_thread.h"
-#include "chrome/browser/sync/glue/data_type_controller.h"
-#include "chrome/browser/sync/glue/data_type_controller_mock.h"
-#include "chrome/browser/sync/test_profile_sync_service.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/test/testing_profile.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-using browser_sync::DataTypeController;
-using browser_sync::DataTypeControllerMock;
-using testing::_;
-using testing::InvokeArgument;
-using testing::Return;
-
-ACTION_P(InvokeCallback, callback_result) {
-  arg1->Run(callback_result);
-  delete arg1;
-}
-
-class ProfileSyncServiceObserverMock : public ProfileSyncServiceObserver {
- public:
-  MOCK_METHOD0(OnStateChanged, void());
-};
-
-// TODO(skrul) This test fails on the mac. See http://crbug.com/33443
-#if defined(OS_MACOSX)
-#define SKIP_MACOSX(test) DISABLED_##test
-#else
-#define SKIP_MACOSX(test) test
-#endif
-
-class ProfileSyncServiceStartupTest : public testing::Test {
- public:
-  ProfileSyncServiceStartupTest()
-      : ui_thread_(ChromeThread::UI, &message_loop_) {}
-
-  virtual ~ProfileSyncServiceStartupTest() {
-    // The PSS has some deletes that are scheduled on the main thread
-    // so we must delete the service and run the message loop.
-    service_.reset();
-    MessageLoop::current()->RunAllPending();
-  }
-
-  virtual void SetUp() {
-    service_.reset(new TestProfileSyncService(&profile_));
-    service_->AddObserver(&observer_);
-  }
-
-  virtual void TearDown() {
-    service_->RemoveObserver(&observer_);
-  }
-
- protected:
-  DataTypeControllerMock* MakeBookmarkDTC() {
-    DataTypeControllerMock* dtc = new DataTypeControllerMock();
-    EXPECT_CALL(*dtc, enabled()).WillRepeatedly(Return(true));
-    EXPECT_CALL(*dtc, model_safe_group()).
-        WillRepeatedly(Return(browser_sync::GROUP_UI));
-    EXPECT_CALL(*dtc, type()).WillRepeatedly(Return(syncable::BOOKMARKS));
-    return dtc;
-  }
-
-  DataTypeControllerMock* MakePreferenceDTC() {
-    DataTypeControllerMock* dtc = new DataTypeControllerMock();
-    EXPECT_CALL(*dtc, enabled()).WillRepeatedly(Return(true));
-    EXPECT_CALL(*dtc, model_safe_group()).
-        WillRepeatedly(Return(browser_sync::GROUP_UI));
-    EXPECT_CALL(*dtc, type()).WillRepeatedly(Return(syncable::PREFERENCES));
-    return dtc;
-  }
-
-  void SetStartStopExpectations(DataTypeControllerMock* mock_dtc) {
-    EXPECT_CALL(*mock_dtc, Start(_, _)).
-        WillOnce(InvokeCallback((DataTypeController::OK)));
-    EXPECT_CALL(*mock_dtc, Stop()).Times(1);
-    EXPECT_CALL(*mock_dtc, state()).
-        WillOnce(Return(DataTypeController::RUNNING));
-  }
-
-  MessageLoop message_loop_;
-  ChromeThread ui_thread_;
-  TestingProfile profile_;
-  scoped_ptr<TestProfileSyncService> service_;
-  ProfileSyncServiceObserverMock observer_;
-};
-
-TEST_F(ProfileSyncServiceStartupTest, SKIP_MACOSX(StartBookmarks)) {
-  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
-  SetStartStopExpectations(bookmark_dtc);
-
-  EXPECT_CALL(observer_, OnStateChanged()).Times(2);
-
-  service_->RegisterDataTypeController(bookmark_dtc);
-  service_->Initialize();
-}
-
-TEST_F(ProfileSyncServiceStartupTest, SKIP_MACOSX(StartBookmarksWithMerge)) {
-  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
-  EXPECT_CALL(*bookmark_dtc, Start(false, _)).
-      WillOnce(InvokeCallback((DataTypeController::NEEDS_MERGE)));
-  EXPECT_CALL(*bookmark_dtc, Start(true, _)).
-      WillOnce(InvokeCallback((DataTypeController::OK)));
-  EXPECT_CALL(*bookmark_dtc, state()).
-      WillOnce(Return(DataTypeController::NOT_RUNNING)).
-      WillOnce(Return(DataTypeController::RUNNING));
-
-  EXPECT_CALL(*bookmark_dtc, Stop()).Times(1);
-
-  EXPECT_CALL(observer_, OnStateChanged()).Times(4);
-
-  profile_.GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
-  service_->RegisterDataTypeController(bookmark_dtc);
-  service_->Initialize();
-  service_->EnableForUser();
-  service_->OnUserAcceptedMergeAndSync();
-}
-
-TEST_F(ProfileSyncServiceStartupTest, SKIP_MACOSX(StartPreferences)) {
-  DataTypeControllerMock* preference_dtc = MakePreferenceDTC();
-  SetStartStopExpectations(preference_dtc);
-
-  EXPECT_CALL(observer_, OnStateChanged()).Times(2);
-
-  service_->RegisterDataTypeController(preference_dtc);
-  service_->Initialize();
-}
-
-TEST_F(ProfileSyncServiceStartupTest,
-       SKIP_MACOSX(StartBookmarksAndPreferences)) {
-  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
-  SetStartStopExpectations(bookmark_dtc);
-
-  DataTypeControllerMock* preference_dtc = MakePreferenceDTC();
-  SetStartStopExpectations(preference_dtc);
-
-  EXPECT_CALL(observer_, OnStateChanged()).Times(2);
-
-  service_->RegisterDataTypeController(bookmark_dtc);
-  service_->RegisterDataTypeController(preference_dtc);
-  service_->Initialize();
-}
-
-TEST_F(ProfileSyncServiceStartupTest,
-       SKIP_MACOSX(StartBookmarksAndPreferencesFail)) {
-  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
-  EXPECT_CALL(*bookmark_dtc, Start(_, _)).
-      WillOnce(InvokeCallback((DataTypeController::OK)));
-  EXPECT_CALL(*bookmark_dtc, Stop()).Times(1);
-  EXPECT_CALL(*bookmark_dtc, state()).
-      WillOnce(Return(DataTypeController::RUNNING)).
-      WillOnce(Return(DataTypeController::NOT_RUNNING));
-
-  DataTypeControllerMock* preference_dtc = MakePreferenceDTC();
-  EXPECT_CALL(*preference_dtc, Start(_, _)).
-      WillOnce(InvokeCallback((DataTypeController::ASSOCIATION_FAILED)));
-  EXPECT_CALL(*preference_dtc, state()).
-      WillOnce(Return(DataTypeController::NOT_RUNNING)).
-      WillOnce(Return(DataTypeController::NOT_RUNNING));
-
-  EXPECT_CALL(observer_, OnStateChanged()).Times(2);
-
-  service_->RegisterDataTypeController(bookmark_dtc);
-  service_->RegisterDataTypeController(preference_dtc);
-  service_->Initialize();
-  EXPECT_TRUE(service_->unrecoverable_error_detected());
-}
-
-#undef SKIP_MACOSX
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index 6f0726e..1753370 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -21,11 +21,12 @@
 #include "chrome/browser/sync/glue/model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/notification_method.h"
+#include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_factory.h"
-#include "chrome/browser/sync/test_profile_sync_service.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;
@@ -34,6 +35,7 @@ using browser_sync::BookmarkModelAssociator;
 using browser_sync::ChangeProcessor;
 using browser_sync::ModelAssociator;
 using browser_sync::SyncBackendHost;
+using browser_sync::TestHttpBridgeFactory;
 
 class TestProfileSyncFactory : public ProfileSyncFactory {
  public:
@@ -121,6 +123,40 @@ class TestModelAssociator : public BookmarkModelAssociator {
   ~TestModelAssociator() {}
 };
 
+class TestProfileSyncService : public ProfileSyncService {
+ public:
+  explicit TestProfileSyncService(Profile* profile)
+      : ProfileSyncService(profile) {
+    RegisterPreferences();
+    SetSyncSetupCompleted();
+  }
+  virtual ~TestProfileSyncService() {
+  }
+
+  virtual void InitializeBackend(bool delete_sync_data_folder) {
+    TestHttpBridgeFactory* factory = new TestHttpBridgeFactory();
+    TestHttpBridgeFactory* factory2 = new TestHttpBridgeFactory();
+    backend()->InitializeForTestMode(L"testuser", factory, factory2,
+        delete_sync_data_folder, browser_sync::kDefaultNotificationMethod);
+    // The SyncBackend posts a task to the current loop when initialization
+    // completes.
+    MessageLoop::current()->Run();
+    // Initialization is synchronous for test mode, so we should be good to go.
+    DCHECK(sync_initialized());
+  }
+
+  virtual void OnBackendInitialized() {
+    ProfileSyncService::OnBackendInitialized();
+    MessageLoop::current()->Quit();
+  }
+
+  // TODO(skrul): how to handle this?
+  virtual bool MergeAndSyncAcceptanceNeeded() {
+    // Never show the dialog.
+    return false;
+  }
+};
+
 // FakeServerChange constructs a list of sync_api::ChangeRecords while modifying
 // the sync model, and can pass the ChangeRecord list to a
 // sync_api::SyncObserver (i.e., the ProfileSyncService) to test the client
diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h
deleted file mode 100644
index 96d57da..0000000
--- a/chrome/browser/sync/test_profile_sync_service.h
+++ /dev/null
@@ -1,43 +0,0 @@
-// 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_TEST_PROFILE_SYNC_SERVICE_H_
-#define CHROME_BROWSER_SYNC_TEST_PROFILE_SYNC_SERVICE_H_
-
-#include "base/message_loop.h"
-#include "chrome/browser/profile.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "chrome/test/sync/test_http_bridge_factory.h"
-
-class TestProfileSyncService : public ProfileSyncService {
- public:
-  explicit TestProfileSyncService(Profile* profile)
-      : ProfileSyncService(profile) {
-    RegisterPreferences();
-    SetSyncSetupCompleted();
-  }
-  virtual ~TestProfileSyncService() {
-  }
-
-  virtual void InitializeBackend(bool delete_sync_data_folder) {
-    browser_sync::TestHttpBridgeFactory* factory =
-        new browser_sync::TestHttpBridgeFactory();
-    browser_sync::TestHttpBridgeFactory* factory2 =
-        new browser_sync::TestHttpBridgeFactory();
-    backend()->InitializeForTestMode(L"testuser", factory, factory2,
-        delete_sync_data_folder, browser_sync::kDefaultNotificationMethod);
-    // The SyncBackend posts a task to the current loop when initialization
-    // completes.
-    MessageLoop::current()->Run();
-    // Initialization is synchronous for test mode, so we should be good to go.
-    DCHECK(sync_initialized());
-  }
-
-  virtual void OnBackendInitialized() {
-    ProfileSyncService::OnBackendInitialized();
-    MessageLoop::current()->Quit();
-  }
-};
-
-#endif  // CHROME_BROWSER_SYNC_TEST_PROFILE_SYNC_SERVICE_H_
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index d54a728..b151ce3 100755
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -813,19 +813,16 @@
         'browser/spellchecker_platform_engine_unittest.cc',
         'browser/ssl/ssl_host_state_unittest.cc',
         'browser/sync/glue/change_processor_mock.h',
-        'browser/sync/glue/data_type_controller_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_startup_unittest.cc',
         '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/sync/test_profile_sync_service.h',
         'browser/tab_contents/navigation_controller_unittest.cc',
         'browser/tab_contents/navigation_entry_unittest.cc',
         'browser/tab_contents/render_view_host_manager_unittest.cc',