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.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=39143

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

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

13 files changed, 488 insertions, 90 deletions

diff --git a/chrome/browser/sync/glue/data_type_controller.h b/chrome/browser/sync/glue/data_type_controller.h
index 62709ff..00dbe5b 100644
--- a/chrome/browser/sync/glue/data_type_controller.h
+++ b/chrome/browser/sync/glue/data_type_controller.h
@@ -5,6 +5,8 @@
 #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"
@@ -43,6 +45,8 @@ 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
new file mode 100644
index 0000000..c2fb102
--- /dev/null
+++ b/chrome/browser/sync/glue/data_type_controller_mock.h
@@ -0,0 +1,25 @@
+// 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 58adb28..6d16dc30 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -8,7 +8,6 @@
 #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"
@@ -19,16 +18,21 @@ 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)
+SyncBackendHost::SyncBackendHost(
+    SyncFrontend* frontend,
+    const FilePath& profile_path,
+    const DataTypeController::TypeMap& data_type_controllers)
     : 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);
@@ -60,8 +64,11 @@ 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.
-  // TODO(ncarter): Wire this up to some external control.
-  registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+  for (DataTypeController::TypeMap::const_iterator it =
+           data_type_controllers_.begin();
+       it != data_type_controllers_.end(); ++it) {
+    registrar_.routing_info[(*it).first] = 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 1b4b3ce..10d1534 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -20,6 +20,7 @@
 #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"
@@ -72,7 +73,9 @@ 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);
+  SyncBackendHost(SyncFrontend* frontend,
+                  const FilePath& profile_path,
+                  const DataTypeController::TypeMap& data_type_controllers);
   ~SyncBackendHost();
 
   // Called on |frontend_loop_| to kick off asynchronous initialization.
@@ -358,6 +361,9 @@ 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.cc b/chrome/browser/sync/profile_sync_factory_impl.cc
index f3a4379..2ae13f7 100644
--- a/chrome/browser/sync/profile_sync_factory_impl.cc
+++ b/chrome/browser/sync/profile_sync_factory_impl.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "chrome/browser/defaults.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/bookmark_change_processor.h"
@@ -30,7 +31,9 @@ ProfileSyncFactoryImpl::ProfileSyncFactoryImpl(
 }
 
 ProfileSyncService* ProfileSyncFactoryImpl::CreateProfileSyncService() {
-  ProfileSyncService* pss = new ProfileSyncService(profile_);
+  ProfileSyncService* pss =
+      new ProfileSyncService(profile_,
+                             browser_defaults::kBootstrapSyncAuthentication);
 
   // Bookmark sync is enabled by default.  Register unless explicitly
   // disabled.
diff --git a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
index 9645ec0..2913c81 100644
--- a/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
+++ b/chrome/browser/sync/profile_sync_factory_impl_unittest.cc
@@ -9,11 +9,14 @@
 #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()
@@ -37,8 +40,7 @@ class ProfileSyncFactoryImplTest : public testing::Test {
 TEST_F(ProfileSyncFactoryImplTest, CreatePSSDefault) {
   scoped_ptr<ProfileSyncService> pss;
   pss.reset(profile_sync_service_factory_->CreateProfileSyncService());
-  ProfileSyncService::DataTypeControllerMap controllers(
-      pss->data_type_controllers());
+  DataTypeController::TypeMap controllers(pss->data_type_controllers());
   EXPECT_EQ(1U, controllers.size());
   EXPECT_EQ(1U, controllers.count(syncable::BOOKMARKS));
 }
@@ -47,8 +49,7 @@ 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());
+  DataTypeController::TypeMap controllers(pss->data_type_controllers());
   EXPECT_EQ(0U, controllers.size());
   EXPECT_EQ(0U, controllers.count(syncable::BOOKMARKS));
 }
@@ -57,8 +58,7 @@ 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());
+  DataTypeController::TypeMap 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 c6baf7f..38ddc41 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -12,7 +12,6 @@
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/task.h"
-#include "chrome/browser/defaults.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/glue/change_processor.h"
@@ -34,15 +33,18 @@ typedef GoogleServiceAuthError AuthError;
 // Default sync server URL.
 static const char kSyncServerUrl[] = "https://clients4.google.com/chrome-sync";
 
-ProfileSyncService::ProfileSyncService(Profile* profile)
+ProfileSyncService::ProfileSyncService(Profile* profile,
+                                       bool bootstrap_sync_authentication)
     : last_auth_error_(AuthError::None()),
       profile_(profile),
+      bootstrap_sync_authentication_(bootstrap_sync_authentication),
       sync_service_url_(kSyncServerUrl),
       backend_initialized_(false),
       expecting_first_run_auth_needed_event_(false),
       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) {
 }
 
@@ -59,11 +61,9 @@ void ProfileSyncService::Initialize() {
     DisableForUser();  // Clean up in case of previous crash / setup abort.
     // If the LSID is empty, we're in a UI test that is not testing sync
     // behavior, so we don't want the sync service to start.
-    // profile()->GetRequestContext() also checks if this is in a test.
-    if (browser_defaults::kBootstrapSyncAuthentication &&
-        profile()->GetRequestContext() &&
+    if (bootstrap_sync_authentication_ &&
         !GetLsidForAuthBootstraping().empty())
-      StartUp();  // We always start sync for Chrome OS.
+    StartUp();  // We always start sync for Chromium OS.
   } else {
     StartUp();
   }
@@ -120,14 +120,14 @@ void ProfileSyncService::ClearPreferences() {
 }
 
 // The domain and name of the LSID cookie which we use to bootstrap the sync
-// authentication in Chrome OS.
+// authentication in Chromium OS.
 const char kLsidCookieDomain[] = "www.google.com";
 const char kLsidCookieName[]   = "LSID";
 
 std::string ProfileSyncService::GetLsidForAuthBootstraping() {
-  if (browser_defaults::kBootstrapSyncAuthentication) {
-    // If we're running inside Chrome OS, bootstrap the sync authentication by
-    // using the LSID cookie provided by the Chrome OS login manager.
+  if (bootstrap_sync_authentication_) {
+    // If we're running inside Chromium OS, bootstrap the sync authentication by
+    // using the LSID cookie provided by the Chromium OS login manager.
     net::CookieMonster::CookieList cookies = profile()->GetRequestContext()->
         GetCookieStore()->GetCookieMonster()->GetAllCookies();
     for (net::CookieMonster::CookieList::const_iterator it = cookies.begin();
@@ -162,7 +162,8 @@ void ProfileSyncService::StartUp() {
   last_synced_time_ = base::Time::FromInternalValue(
       profile_->GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
 
-  backend_.reset(new SyncBackendHost(this, profile_->GetPath()));
+  backend_.reset(
+      new SyncBackendHost(this, profile_->GetPath(), data_type_controllers_));
 
   // 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
@@ -176,8 +177,8 @@ void ProfileSyncService::Shutdown(bool sync_disabled) {
 
   // 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();
+  StopDataType(syncable::BOOKMARKS);
+  StopDataType(syncable::PREFERENCES);
 
   backend_.reset();
 
@@ -233,7 +234,8 @@ void ProfileSyncService::OnUnrecoverableError() {
   unrecoverable_error_detected_ = true;
 
   // TODO(skrul): Change this to support multiple data type controllers.
-  data_type_controllers_[syncable::BOOKMARKS]->Stop();
+  StopDataType(syncable::BOOKMARKS);
+  StopDataType(syncable::PREFERENCES);
 
   // Tell the wizard so it can inform the user only if it is already open.
   wizard_.Step(SyncSetupWizard::FATAL_ERROR);
@@ -348,6 +350,14 @@ 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) {
@@ -380,24 +390,33 @@ 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.
   bool merge_allowed =
       profile_->GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted);
 
-  // If we're running inside Chrome OS, always allow merges and
+  // If we're running inside Chromium OS, always allow merges and
   // consider the sync setup complete.
-  if (browser_defaults::kBootstrapSyncAuthentication) {
+  if (bootstrap_sync_authentication_) {
     merge_allowed = true;
     SetSyncSetupCompleted();
   }
 
   // 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));
+  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));
+    }
+  }
 }
 
 void ProfileSyncService::BookmarkStartCallback(
@@ -405,9 +424,22 @@ void ProfileSyncService::BookmarkStartCallback(
   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());
+      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);
+      }
       break;
     }
 
@@ -420,6 +452,27 @@ 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;
     }
   }
 }
@@ -460,9 +513,23 @@ 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.
-  if (data_type_controllers_.count(syncable::BOOKMARKS)) {
-    return data_type_controllers_[syncable::BOOKMARKS]->state() ==
-        DataTypeController::RUNNING;
-  }
-  return false;
+
+  // 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;
 }
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 349e819f..e1aa38d 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"  // For StartResult.
+#include "chrome/browser/sync/glue/data_type_controller.h"
 #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,7 +25,6 @@
 namespace browser_sync {
 
 class ChangeProcessor;
-class DataTypeController;
 
 class UnrecoverableErrorHandler {
  public:
@@ -60,8 +59,6 @@ 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;
 
@@ -89,7 +86,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
     MAX_SYNC_EVENT_CODE
   };
 
-  explicit ProfileSyncService(Profile* profile);
+  ProfileSyncService(Profile* profile, bool bootstrap_sync_authentication);
   virtual ~ProfileSyncService();
 
   // Initializes the object. This should be called every time an object of this
@@ -103,7 +100,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   void RegisterDataTypeController(
       browser_sync::DataTypeController* data_type_controller);
 
-  const DataTypeControllerMap& data_type_controllers() const {
+  const browser_sync::DataTypeController::TypeMap& data_type_controllers()
+      const {
     return data_type_controllers_;
   }
 
@@ -240,6 +238,8 @@ 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
@@ -269,7 +269,10 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
 
   // When running inside Chrome OS, extract the LSID cookie from the cookie
   // store to bootstrap the authentication process.
-  std::string GetLsidForAuthBootstraping();
+  virtual 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_;
@@ -280,6 +283,12 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // The profile whose data we are synchronizing.
   Profile* profile_;
 
+  // True if the profile sync service should attempt to use an LSID
+  // cookie for authentication.  This is typically set to true in
+  // ChromiumOS since we want to use the system level authentication
+  // for sync.
+  bool bootstrap_sync_authentication_;
+
   // TODO(ncarter): Put this in a profile, once there is UI for it.
   // This specifies where to find the sync server.
   GURL sync_service_url_;
@@ -293,7 +302,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   scoped_ptr<browser_sync::SyncBackendHost> backend_;
 
   // List of available data type controllers.
-  DataTypeControllerMap data_type_controllers_;
+  browser_sync::DataTypeController::TypeMap data_type_controllers_;
 
   // Whether the SyncBackendHost has been initialized.
   bool backend_initialized_;
@@ -322,6 +331,11 @@ 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_mock.h b/chrome/browser/sync/profile_sync_service_mock.h
index e2b4dde..661ada8 100644
--- a/chrome/browser/sync/profile_sync_service_mock.h
+++ b/chrome/browser/sync/profile_sync_service_mock.h
@@ -15,7 +15,7 @@
 
 class ProfileSyncServiceMock : public ProfileSyncService {
  public:
-  ProfileSyncServiceMock() : ProfileSyncService(NULL) {}
+  ProfileSyncServiceMock() : ProfileSyncService(NULL, false) {}
   virtual ~ProfileSyncServiceMock() {}
 
   MOCK_METHOD0(EnableForUser, void());
diff --git a/chrome/browser/sync/profile_sync_service_startup_unittest.cc b/chrome/browser/sync/profile_sync_service_startup_unittest.cc
new file mode 100644
index 0000000..4072ad3
--- /dev/null
+++ b/chrome/browser/sync/profile_sync_service_startup_unittest.cc
@@ -0,0 +1,254 @@
+// 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::Mock;
+using testing::Return;
+
+ACTION_P(InvokeCallback, callback_result) {
+  arg1->Run(callback_result);
+  delete arg1;
+}
+
+// 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 ProfileSyncServiceObserverMock : public ProfileSyncServiceObserver {
+ public:
+  MOCK_METHOD0(OnStateChanged, void());
+};
+
+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_, false));
+    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(StartFirstTime)) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  EXPECT_CALL(*bookmark_dtc, Start(false, _)).Times(0);
+  EXPECT_CALL(*bookmark_dtc, state()).
+      WillRepeatedly(Return(DataTypeController::NOT_RUNNING));
+
+  EXPECT_CALL(observer_, OnStateChanged()).Times(1);
+
+  profile_.GetPrefs()->ClearPref(prefs::kSyncHasSetupCompleted);
+  service_->RegisterDataTypeController(bookmark_dtc);
+  // Should not actually start, rather just clean things up and wait
+  // to be enabled.
+  service_->Initialize();
+
+  // Preferences should be back to defaults.
+  EXPECT_EQ(0, profile_.GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
+  EXPECT_FALSE(profile_.GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted));
+  Mock::VerifyAndClearExpectations(&bookmark_dtc);
+
+  // Then start things up.
+  SetStartStopExpectations(bookmark_dtc);
+  EXPECT_CALL(observer_, OnStateChanged()).Times(3);
+  service_->EnableForUser();
+}
+
+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);
+
+  // Set kSyncHasSetupCompleted to false so the first attempt to start
+  // the bookmark dtc is done with merge_allowed = false.  This has
+  // the side effect of disabling sync, so we must explicitly call
+  // EnableForUser.
+  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());
+}
+
+class ProfileSyncServiceStartupBootstrapTest :
+    public ProfileSyncServiceStartupTest {
+ public:
+  ProfileSyncServiceStartupBootstrapTest() {}
+  virtual ~ProfileSyncServiceStartupBootstrapTest() {}
+
+  virtual void SetUp() {
+    service_.reset(new TestProfileSyncService(&profile_, true));
+    service_->AddObserver(&observer_);
+  }
+};
+
+TEST_F(ProfileSyncServiceStartupBootstrapTest, SKIP_MACOSX(StartFirstTime)) {
+  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::NOT_RUNNING)).
+      WillOnce(Return(DataTypeController::RUNNING));
+  EXPECT_CALL(observer_, OnStateChanged()).Times(3);
+
+  profile_.GetPrefs()->ClearPref(prefs::kSyncHasSetupCompleted);
+  service_->RegisterDataTypeController(bookmark_dtc);
+  // Will start sync even though setup hasn't been completed (since
+  // setup is bypassed when bootstrapping is enabled).
+  service_->Initialize();
+}
+
+TEST_F(ProfileSyncServiceStartupBootstrapTest,
+       SKIP_MACOSX(StartBookmarksWithMerge)) {
+  DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC();
+  EXPECT_CALL(*bookmark_dtc, Start(true, _)).
+      WillOnce(InvokeCallback((DataTypeController::OK)));
+  EXPECT_CALL(*bookmark_dtc, Stop()).Times(1);
+  EXPECT_CALL(*bookmark_dtc, state()).
+      WillOnce(Return(DataTypeController::NOT_RUNNING)).
+      WillOnce(Return(DataTypeController::RUNNING));
+
+  EXPECT_CALL(observer_, OnStateChanged()).Times(3);
+
+  // Set kSyncHasSetupCompleted to false so the first attempt to start
+  // the bookmark dtc is done with merge_allowed = false.  This has
+  // the side effect of disabling sync, but this does not impede
+  // startup when bootstrapping is enabled.
+  profile_.GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
+  service_->RegisterDataTypeController(bookmark_dtc);
+  service_->Initialize();
+}
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index 1753370..f7b97e5 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -21,12 +21,11 @@
 #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;
@@ -35,7 +34,6 @@ 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:
@@ -123,40 +121,6 @@ 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
@@ -335,7 +299,7 @@ class ProfileSyncServiceTest : public testing::Test {
 
   void StartSyncService() {
     if (!service_.get()) {
-      service_.reset(new TestProfileSyncService(profile_.get()));
+      service_.reset(new TestProfileSyncService(profile_.get(), false));
 
       // Register the bookmark data type.
       model_associator_ = new TestModelAssociator(service_.get());
@@ -1389,7 +1353,7 @@ TEST_F(ProfileSyncServiceTestWithData, MAYBE_TestStartupWithOldSyncData) {
 
   LoadBookmarkModel(LOAD_FROM_STORAGE, SAVE_TO_STORAGE);
   if (!service_.get()) {
-    service_.reset(new TestProfileSyncService(profile_.get()));
+    service_.reset(new TestProfileSyncService(profile_.get(), false));
     profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
 
     model_associator_ = new TestModelAssociator(service_.get());
diff --git a/chrome/browser/sync/sync_setup_wizard_unittest.cc b/chrome/browser/sync/sync_setup_wizard_unittest.cc
index b1ea56b..2d79bac 100644
--- a/chrome/browser/sync/sync_setup_wizard_unittest.cc
+++ b/chrome/browser/sync/sync_setup_wizard_unittest.cc
@@ -30,8 +30,9 @@ typedef GoogleServiceAuthError AuthError;
 class ProfileSyncServiceForWizardTest : public ProfileSyncService {
  public:
   explicit ProfileSyncServiceForWizardTest(Profile* profile)
-    : ProfileSyncService(profile), user_accepted_merge_and_sync_(false),
-      user_cancelled_dialog_(false) {
+      : ProfileSyncService(profile, false),
+        user_accepted_merge_and_sync_(false),
+        user_cancelled_dialog_(false) {
     RegisterPreferences();
   }
 
diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h
new file mode 100644
index 0000000..cb86c49
--- /dev/null
+++ b/chrome/browser/sync/test_profile_sync_service.h
@@ -0,0 +1,53 @@
+// 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 <string>
+
+#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,
+                                  bool bootstrap_sync_authentication)
+      : ProfileSyncService(profile, bootstrap_sync_authentication) {
+    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();
+  }
+
+ private:
+  // When testing under ChromiumOS, this method must not return an empty
+  // value value in order for the profile sync service to start.
+  virtual std::string GetLsidForAuthBootstraping() {
+    return "foo";
+  }
+};
+
+#endif  // CHROME_BROWSER_SYNC_TEST_PROFILE_SYNC_SERVICE_H_