[Sync] Tidy up SyncBackendHost interface.

Converted passphrase-related notifications to SyncFrontend methods.

Removed some unneeded parameters from SyncBackendHost constructor.

Moved some function definitions into .cc files.

BUG=None
TEST=unit_tests

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

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

chrome/browser/sync/glue/data_type_manager_impl.cc

@@ -160,6 +160,7 @@ void DataTypeManagerImpl::Restart() {
   // The task will be invoked when updates are downloaded.
   state_ = DOWNLOAD_PENDING;
   backend_->ConfigureDataTypes(
+      controllers_,
       last_requested_types_,
       method_factory_.NewRunnableMethod(&DataTypeManagerImpl::DownloadReady));
 

chrome/browser/sync/glue/data_type_manager_impl_unittest.cc

@@ -120,7 +120,7 @@ class DataTypeManagerImplTest : public testing::Test {
   }
 
   void SetBackendExpectations(int times) {
-    EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(times);
+    EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(times);
     EXPECT_CALL(backend_, StartSyncingWithServer()).Times(times);
     EXPECT_CALL(backend_, RequestPause()).Times(times);
     EXPECT_CALL(backend_, RequestResume()).Times(times);
@@ -290,7 +290,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhilePausePending) {
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
   // Don't notify the first time pause is called.
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(2);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(2);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(2);
   EXPECT_CALL(backend_, RequestPause()).
       WillOnce(Return(true)).
@@ -326,7 +326,7 @@ TEST_F(DataTypeManagerImplTest, StopWhilePausePending) {
   SetNotUsedExpectations(bookmark_dtc);
   controllers_[syncable::BOOKMARKS] = bookmark_dtc;
 
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   // Never notify when RequestPause is called.
   EXPECT_CALL(backend_, RequestPause()).WillOnce(Return(true));
@@ -357,7 +357,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileResumePending) {
   SetStartStopExpectations(preference_dtc);
   controllers_[syncable::PREFERENCES] = preference_dtc;
 
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(2);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(2);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(2);
   EXPECT_CALL(backend_, RequestPause()).Times(2);
   // Don't notify the first time resume is called.
@@ -394,7 +394,7 @@ TEST_F(DataTypeManagerImplTest, StopWhileResumePending) {
   SetStartStopExpectations(bookmark_dtc);
   controllers_[syncable::BOOKMARKS] = bookmark_dtc;
 
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   // Never notify pause resumed.
@@ -429,7 +429,7 @@ TEST_F(DataTypeManagerImplTest, OneFailingController) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED);
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
@@ -457,7 +457,7 @@ TEST_F(DataTypeManagerImplTest, StopWhileInFlight) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ABORTED);
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
@@ -492,7 +492,7 @@ TEST_F(DataTypeManagerImplTest, SecondControllerFails) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::ASSOCIATION_FAILED);
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).Times(0);
@@ -513,7 +513,7 @@ TEST_F(DataTypeManagerImplTest, PauseFailed) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR);
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).WillOnce(Return(false));
   EXPECT_CALL(backend_, RequestResume()).Times(0);
@@ -531,7 +531,7 @@ TEST_F(DataTypeManagerImplTest, ResumeFailed) {
   DataTypeManagerImpl dtm(&backend_, controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::UNRECOVERABLE_ERROR);
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).Times(1);
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(1);
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(1);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
   EXPECT_CALL(backend_, RequestResume()).WillOnce(Return(false));
@@ -556,8 +556,8 @@ TEST_F(DataTypeManagerImplTest, ConfigureWhileDownloadPending) {
   CancelableTask* task;
   // Grab the task the first time this is called so we can configure
   // before it is finished.
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).
-      WillOnce(SaveArg<1>(&task)).
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).
+      WillOnce(SaveArg<2>(&task)).
       WillOnce(DoDefault());
   EXPECT_CALL(backend_, StartSyncingWithServer()).Times(2);
   EXPECT_CALL(backend_, RequestPause()).Times(1);
@@ -596,8 +596,8 @@ TEST_F(DataTypeManagerImplTest, StopWhileDownloadPending) {
   CancelableTask* task;
   // Grab the task the first time this is called so we can stop
   // before it is finished.
-  EXPECT_CALL(backend_, ConfigureDataTypes(_, _)).
-      WillOnce(SaveArg<1>(&task));
+  EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).
+      WillOnce(SaveArg<2>(&task));
   EXPECT_CALL(backend_, StartSyncingWithServer());
   EXPECT_CALL(backend_, RequestPause()).Times(0);
   EXPECT_CALL(backend_, RequestResume()).Times(0);

chrome/browser/sync/glue/sync_backend_host.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/compiler_specific.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/task.h"
@@ -50,21 +51,16 @@ namespace browser_sync {
 using sessions::SyncSessionSnapshot;
 using sync_api::SyncCredentials;
 
-SyncBackendHost::SyncBackendHost(
-    SyncFrontend* frontend,
-    Profile* profile,
-    const FilePath& profile_path,
-    const DataTypeController::TypeMap& data_type_controllers)
-    : core_thread_("Chrome_SyncCoreThread"),
+SyncBackendHost::SyncBackendHost(SyncFrontend* frontend, Profile* profile)
+    : core_(new Core(ALLOW_THIS_IN_INITIALIZER_LIST(this))),
+      core_thread_("Chrome_SyncCoreThread"),
       frontend_loop_(MessageLoop::current()),
       profile_(profile),
       frontend_(frontend),
-      sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
-      data_type_controllers_(data_type_controllers),
+      sync_data_folder_path_(
+          profile_->GetPath().Append(kSyncDataFolderName)),
       last_auth_error_(AuthError::None()),
       syncapi_initialized_(false) {
-
-  core_ = new Core(this);
 }
 
 SyncBackendHost::SyncBackendHost()
@@ -322,8 +318,10 @@ void SyncBackendHost::ConfigureAutofillMigration() {
   }
 }
 
-void SyncBackendHost::ConfigureDataTypes(const syncable::ModelTypeSet& types,
-                                         CancelableTask* ready_task) {
+void SyncBackendHost::ConfigureDataTypes(
+    const DataTypeController::TypeMap& data_type_controllers,
+    const syncable::ModelTypeSet& types,
+    CancelableTask* ready_task) {
   // Only one configure is allowed at a time.
   DCHECK(!configure_ready_task_.get());
   DCHECK(syncapi_initialized_);
@@ -337,8 +335,8 @@ void SyncBackendHost::ConfigureDataTypes(const syncable::ModelTypeSet& types,
   {
     AutoLock lock(registrar_lock_);
     for (DataTypeController::TypeMap::const_iterator it =
-             data_type_controllers_.begin();
-         it != data_type_controllers_.end(); ++it) {
+             data_type_controllers.begin();
+         it != data_type_controllers.end(); ++it) {
       syncable::ModelType type = (*it).first;
 
       // If a type is not specified, remove it from the routing_info.
@@ -459,21 +457,23 @@ void SyncBackendHost::Core::NotifyResumed() {
 }
 
 void SyncBackendHost::Core::NotifyPassphraseRequired(bool for_decryption) {
-  NotificationService::current()->Notify(
-      NotificationType::SYNC_PASSPHRASE_REQUIRED,
-      Source<SyncBackendHost>(host_),
-      Details<bool>(&for_decryption));
+  if (!host_ || !host_->frontend_)
+    return;
+
+  DCHECK_EQ(MessageLoop::current(), host_->frontend_loop_);
+
+  host_->frontend_->OnPassphraseRequired(for_decryption);
 }
 
 void SyncBackendHost::Core::NotifyPassphraseAccepted(
     const std::string& bootstrap_token) {
-  if (!host_)
+  if (!host_ || !host_->frontend_)
     return;
+
+  DCHECK_EQ(MessageLoop::current(), host_->frontend_loop_);
+
   host_->PersistEncryptionBootstrapToken(bootstrap_token);
-  NotificationService::current()->Notify(
-      NotificationType::SYNC_PASSPHRASE_ACCEPTED,
-      Source<SyncBackendHost>(host_),
-      NotificationService::NoDetails());
+  host_->frontend_->OnPassphraseAccepted();
 }
 
 void SyncBackendHost::Core::NotifyUpdatedToken(const std::string& token) {

chrome/browser/sync/glue/sync_backend_host.h

@@ -71,6 +71,19 @@ class SyncFrontend {
   virtual void OnClearServerDataSucceeded() = 0;
   virtual void OnClearServerDataFailed() = 0;
 
+  // The syncer requires a passphrase to decrypt sensitive
+  // updates. This is called when the first sensitive data type is
+  // setup by the user as well as anytime any the passphrase is
+  // changed in another synced client.  if
+  // |passphrase_required_for_decryption| is false, the passphrase is
+  // required only for encryption.
+  virtual void OnPassphraseRequired(bool for_decryption) = 0;
+
+  // Called when the passphrase provided by the user is
+  // accepted. After this is called, updates to sensitive nodes are
+  // encrypted using the accepted passphrase.
+  virtual void OnPassphraseAccepted() = 0;
+
  protected:
   // Don't delete through SyncFrontend interface.
   virtual ~SyncFrontend() {
@@ -94,10 +107,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,
-                  Profile* profile,
-                  const FilePath& profile_path,
-                  const DataTypeController::TypeMap& data_type_controllers);
+  SyncBackendHost(SyncFrontend* frontend, Profile* profile);
   // For testing.
   // TODO(skrul): Extract an interface so this is not needed.
   SyncBackendHost();
@@ -139,8 +149,10 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
   // The ready_task will be run when all of the requested data types
   // are up-to-date and ready for activation.  The task will cancelled
   // upon shutdown.  The method takes ownership of the task pointer.
-  virtual void ConfigureDataTypes(const syncable::ModelTypeSet& types,
-                                  CancelableTask* ready_task);
+  virtual void ConfigureDataTypes(
+      const DataTypeController::TypeMap& data_type_controllers,
+      const syncable::ModelTypeSet& types,
+      CancelableTask* ready_task);
 
   syncable::AutofillMigrationState
       GetAutofillMigrationState();
@@ -506,9 +518,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_;
-
   // A task that should be called once data type configuration is
   // complete.
   scoped_ptr<CancelableTask> configure_ready_task_;

chrome/browser/sync/glue/sync_backend_host_mock.cc

@@ -6,6 +6,11 @@
 
 namespace browser_sync {
 
+ACTION(InvokeTask) {
+  arg2->Run();
+  delete arg2;
+}
+
 SyncBackendHostMock::SyncBackendHostMock() {
   // By default, the RequestPause and RequestResume methods will
   // send the confirmation notification and return true.
@@ -17,7 +22,7 @@ SyncBackendHostMock::SyncBackendHostMock() {
                                    testing::Return(true)));
 
   // By default, invoke the ready callback.
-  ON_CALL(*this, ConfigureDataTypes(testing::_, testing::_)).
+  ON_CALL(*this, ConfigureDataTypes(testing::_, testing::_, testing::_)).
       WillByDefault(InvokeTask());
 }
 

chrome/browser/sync/glue/sync_backend_host_mock.h

@@ -16,18 +16,14 @@
 
 namespace browser_sync {
 
-ACTION(InvokeTask) {
-  arg1->Run();
-  delete arg1;
-}
-
 class SyncBackendHostMock : public SyncBackendHost {
  public:
   SyncBackendHostMock();
   virtual ~SyncBackendHostMock();
 
-  MOCK_METHOD2(ConfigureDataTypes,
-               void(const std::set<syncable::ModelType>&, CancelableTask*));
+  MOCK_METHOD3(ConfigureDataTypes,
+               void(const DataTypeController::TypeMap&,
+                    const std::set<syncable::ModelType>&, CancelableTask*));
   MOCK_METHOD0(RequestPause, bool());
   MOCK_METHOD0(RequestResume, bool());
   MOCK_METHOD0(StartSyncingWithServer, void());

chrome/browser/sync/profile_sync_service.cc

@@ -404,12 +404,6 @@ void ProfileSyncService::InitializeBackend(bool delete_sync_data_folder) {
     return;
   }
 
-  // TODO(chron): Reimplement invalidate XMPP login / Sync login
-  //              command line switches. Perhaps make it a command
-  //              line in the TokenService itself to pass an arbitrary
-  //              token.
-
-
   syncable::ModelTypeSet types;
   // If sync setup hasn't finished, we don't want to initialize routing info
   // for any data types so that we don't download updates for types that the
@@ -429,9 +423,7 @@ void ProfileSyncService::InitializeBackend(bool delete_sync_data_folder) {
 }
 
 void ProfileSyncService::CreateBackend() {
-  backend_.reset(
-      new SyncBackendHost(this, profile_, profile_->GetPath(),
-                          data_type_controllers_));
+  backend_.reset(new SyncBackendHost(this, profile_));
 }
 
 void ProfileSyncService::StartUp() {
@@ -448,13 +440,6 @@ void ProfileSyncService::StartUp() {
 
   CreateBackend();
 
-  registrar_.Add(this,
-                 NotificationType::SYNC_PASSPHRASE_REQUIRED,
-                 Source<SyncBackendHost>(backend_.get()));
-  registrar_.Add(this,
-                 NotificationType::SYNC_PASSPHRASE_ACCEPTED,
-                 Source<SyncBackendHost>(backend_.get()));
-
   // 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
   // be there.
@@ -483,13 +468,6 @@ void ProfileSyncService::Shutdown(bool sync_disabled) {
   if (doomed_backend.get()) {
     doomed_backend->Shutdown(sync_disabled);
 
-    registrar_.Remove(this,
-                   NotificationType::SYNC_PASSPHRASE_REQUIRED,
-                   Source<SyncBackendHost>(doomed_backend.get()));
-    registrar_.Remove(this,
-                   NotificationType::SYNC_PASSPHRASE_ACCEPTED,
-                   Source<SyncBackendHost>(doomed_backend.get()));
-
     doomed_backend.reset();
   }
 
@@ -519,7 +497,7 @@ void ProfileSyncService::DisableForUser() {
     signin_->SignOut();
   }
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 }
 
 bool ProfileSyncService::HasSyncSetupCompleted() const {
@@ -545,6 +523,10 @@ void ProfileSyncService::UpdateLastSyncedTime() {
   profile_->GetPrefs()->ScheduleSavePersistentPrefs();
 }
 
+void ProfileSyncService::NotifyObservers() {
+  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+}
+
 // static
 const char* ProfileSyncService::GetPrefNameForDataType(
     syncable::ModelType data_type) {
@@ -591,7 +573,7 @@ void ProfileSyncService::OnUnrecoverableError(
   // Tell the wizard so it can inform the user only if it is already open.
   wizard_.Step(SyncSetupWizard::FATAL_ERROR);
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
   LOG(ERROR) << "Unrecoverable error detected -- ProfileSyncService unusable."
       << message;
   std::string location;
@@ -613,7 +595,7 @@ void ProfileSyncService::OnBackendInitialized() {
   if (last_synced_time_.is_null()) {
     UpdateLastSyncedTime();
   }
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 
   if (!cros_user_.empty()) {
     if (profile_->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)) {
@@ -630,7 +612,7 @@ void ProfileSyncService::OnBackendInitialized() {
 
 void ProfileSyncService::OnSyncCycleCompleted() {
   UpdateLastSyncedTime();
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 }
 
 void ProfileSyncService::UpdateAuthErrorState(
@@ -654,7 +636,7 @@ void ProfileSyncService::UpdateAuthErrorState(
 
   is_auth_in_progress_ = false;
   // Fan the notification out to interested UI-thread components.
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 }
 
 void ProfileSyncService::OnAuthError() {
@@ -674,7 +656,7 @@ void ProfileSyncService::OnClearServerDataTimeout() {
   if (clear_server_data_state_ != CLEAR_SUCCEEDED &&
       clear_server_data_state_ != CLEAR_FAILED) {
     clear_server_data_state_ = CLEAR_FAILED;
-    FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+    NotifyObservers();
   }
 }
 
@@ -687,7 +669,7 @@ void ProfileSyncService::OnClearServerDataFailed() {
   if (clear_server_data_state_ != CLEAR_SUCCEEDED &&
       clear_server_data_state_ != CLEAR_FAILED) {
     clear_server_data_state_ = CLEAR_FAILED;
-    FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+    NotifyObservers();
   }
 }
 
@@ -698,8 +680,44 @@ void ProfileSyncService::OnClearServerDataSucceeded() {
   // we want UI to update itself and no longer allow the user to press "clear"
   if (clear_server_data_state_ != CLEAR_SUCCEEDED) {
     clear_server_data_state_ = CLEAR_SUCCEEDED;
-    FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+    NotifyObservers();
+  }
+}
+
+void ProfileSyncService::OnPassphraseRequired(bool for_decryption) {
+  DCHECK(backend_.get());
+  DCHECK(backend_->IsNigoriEnabled());
+  observed_passphrase_required_ = true;
+  passphrase_required_for_decryption_ = for_decryption;
+
+  if (!cached_passphrase_.value.empty()) {
+    SetPassphrase(cached_passphrase_.value,
+                  cached_passphrase_.is_explicit,
+                  cached_passphrase_.is_creation);
+    cached_passphrase_ = CachedPassphrase();
+    return;
   }
+
+  if (WizardIsVisible()) {
+    wizard_.Step(SyncSetupWizard::ENTER_PASSPHRASE);
+  }
+
+  NotifyObservers();
+}
+
+void ProfileSyncService::OnPassphraseAccepted() {
+  // Make sure the data types that depend on the passphrase are started at
+  // this time.
+  syncable::ModelTypeSet types;
+  GetPreferredDataTypes(&types);
+  data_type_manager_->Configure(types);
+
+  NotifyObservers();
+  observed_passphrase_required_ = false;
+  tried_setting_explicit_passphrase_ = false;
+  tried_creating_explicit_passphrase_ = false;
+
+  wizard_.Step(SyncSetupWizard::DONE);
 }
 
 void ProfileSyncService::ShowLoginDialog(gfx::NativeWindow parent_window) {
@@ -723,7 +741,7 @@ void ProfileSyncService::ShowLoginDialog(gfx::NativeWindow parent_window) {
   wizard_.SetParent(parent_window);
   wizard_.Step(SyncSetupWizard::GAIA_LOGIN);
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 }
 
 void ProfileSyncService::ShowErrorUI(gfx::NativeWindow parent_window) {
@@ -843,7 +861,7 @@ void ProfileSyncService::OnUserSubmittedAuth(
     const std::string& captcha, const std::string& access_code) {
   last_attempted_user_email_ = username;
   is_auth_in_progress_ = true;
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 
   auth_start_time_ = base::TimeTicks::Now();
 
@@ -908,7 +926,7 @@ void ProfileSyncService::OnUserCancelledDialog() {
   // good if invalid creds were provided, but it's an edge case and the user
   // can of course get themselves out of it.
   is_auth_in_progress_ = false;
-  FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+  NotifyObservers();
 }
 
 void ProfileSyncService::ChangePreferredDataTypes(
@@ -1055,7 +1073,7 @@ void ProfileSyncService::Observe(NotificationType type,
                                  const NotificationDetails& details) {
   switch (type.value) {
     case NotificationType::SYNC_CONFIGURE_START: {
-      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+      NotifyObservers();
       // TODO(sync): Maybe toast?
       break;
     }
@@ -1083,31 +1101,10 @@ void ProfileSyncService::Observe(NotificationType type,
       // TODO(sync): Less wizard, more toast.
       if (!observed_passphrase_required_)
         wizard_.Step(SyncSetupWizard::DONE);
-      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+      NotifyObservers();
 
       break;
     }
-    case NotificationType::SYNC_PASSPHRASE_REQUIRED: {
-      DCHECK(backend_.get());
-      DCHECK(backend_->IsNigoriEnabled());
-      observed_passphrase_required_ = true;
-      passphrase_required_for_decryption_ = *(Details<bool>(details).ptr());
-
-      if (!cached_passphrase_.value.empty()) {
-        SetPassphrase(cached_passphrase_.value,
-                      cached_passphrase_.is_explicit,
-                      cached_passphrase_.is_creation);
-        cached_passphrase_ = CachedPassphrase();
-        break;
-      }
-
-      if (WizardIsVisible()) {
-        wizard_.Step(SyncSetupWizard::ENTER_PASSPHRASE);
-      }
-
-      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
-      break;
-    }
     case NotificationType::SYNC_DATA_TYPES_UPDATED: {
       if (!HasSyncSetupCompleted()) break;
 
@@ -1116,25 +1113,10 @@ void ProfileSyncService::Observe(NotificationType type,
       OnUserChoseDatatypes(false, types);
       break;
     }
-    case NotificationType::SYNC_PASSPHRASE_ACCEPTED: {
-      // Make sure the data types that depend on the passphrase are started at
-      // this time.
-      syncable::ModelTypeSet types;
-      GetPreferredDataTypes(&types);
-      data_type_manager_->Configure(types);
-
-      FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
-      observed_passphrase_required_ = false;
-      tried_setting_explicit_passphrase_ = false;
-      tried_creating_explicit_passphrase_ = false;
-
-      wizard_.Step(SyncSetupWizard::DONE);
-      break;
-    }
     case NotificationType::PREF_CHANGED: {
       std::string* pref_name = Details<std::string>(details).ptr();
       if (*pref_name == prefs::kSyncManaged) {
-        FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+        NotifyObservers();
         if (*pref_sync_managed_) {
           DisableForUser();
         } else if (HasSyncSetupCompleted() && AreCredentialsAvailable()) {

chrome/browser/sync/profile_sync_service.h

@@ -185,6 +185,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   virtual void OnClearServerDataFailed();
   virtual void OnClearServerDataTimeout();
   virtual void OnClearServerDataSucceeded();
+  virtual void OnPassphraseRequired(bool for_decryption);
+  virtual void OnPassphraseAccepted();
 
   // Called when a user enters credentials through UI.
   virtual void OnUserSubmittedAuth(const std::string& username,
@@ -470,6 +472,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Sets the last synced time to the current time.
   void UpdateLastSyncedTime();
 
+  void NotifyObservers();
+
   static const char* GetPrefNameForDataType(syncable::ModelType data_type);
 
   // Time at which we begin an attempt a GAIA authorization.

chrome/browser/sync/profile_sync_service_password_unittest.cc

@@ -15,7 +15,6 @@
 #include "chrome/browser/sync/glue/password_change_processor.h"
 #include "chrome/browser/sync/glue/password_data_type_controller.h"
 #include "chrome/browser/sync/glue/password_model_associator.h"
-#include "chrome/browser/sync/glue/sync_backend_host_mock.h"
 #include "chrome/browser/sync/profile_sync_factory.h"
 #include "chrome/browser/sync/profile_sync_factory_mock.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ -38,7 +37,6 @@ using base::Time;
 using browser_sync::PasswordChangeProcessor;
 using browser_sync::PasswordDataTypeController;
 using browser_sync::PasswordModelAssociator;
-using browser_sync::SyncBackendHostMock;
 using browser_sync::TestIdFactory;
 using browser_sync::UnrecoverableErrorHandler;
 using sync_api::SyncManager;
@@ -101,6 +99,38 @@ class MockPasswordStore : public PasswordStore {
       bool(std::vector<PasswordForm*>*));
 };
 
+class PasswordTestProfileSyncService : public TestProfileSyncService {
+ public:
+  PasswordTestProfileSyncService(ProfileSyncFactory* factory,
+                                 Profile* profile,
+                                 const std::string& test_user,
+                                 bool synchronous_backend_initialization,
+                                 Task* initial_condition_setup_task,
+                                 Task* passphrase_accept_task)
+      : TestProfileSyncService(factory, profile, test_user,
+                               synchronous_backend_initialization,
+                               initial_condition_setup_task),
+        passphrase_accept_task_(passphrase_accept_task) {}
+
+  virtual ~PasswordTestProfileSyncService() {}
+
+  virtual void OnPassphraseRequired(bool for_decryption) {
+    TestProfileSyncService::OnPassphraseRequired(for_decryption);
+    ADD_FAILURE();
+  }
+
+  virtual void OnPassphraseAccepted() {
+    TestProfileSyncService::OnPassphraseAccepted();
+
+    if (passphrase_accept_task_) {
+      passphrase_accept_task_->Run();
+    }
+  }
+
+ private:
+  Task* passphrase_accept_task_;
+};
+
 class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
  protected:
   ProfileSyncServicePasswordTest()
@@ -113,17 +143,9 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
 
     notification_service_ = new ThreadNotificationService(&db_thread_);
     notification_service_->Init();
-    registrar_.Add(&observer_,
-        NotificationType::SYNC_PASSPHRASE_ACCEPTED,
-        NotificationService::AllSources());
     registrar_.Add(&observer_,
         NotificationType::SYNC_CONFIGURE_DONE,
         NotificationService::AllSources());
-
-    // We shouldn't ever get this. Gmock will complain if we do.
-    registrar_.Add(&observer_,
-        NotificationType::SYNC_PASSPHRASE_REQUIRED,
-        NotificationService::AllSources());
   }
 
   virtual void TearDown() {
@@ -141,8 +163,8 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
                         int num_resume_expectations,
                         int num_pause_expectations) {
     if (!service_.get()) {
-      service_.reset(new TestProfileSyncService(&factory_, &profile_,
-                                                "test_user", false, root_task));
+      service_.reset(new PasswordTestProfileSyncService(
+          &factory_, &profile_, "test_user", false, root_task, node_task));
       service_->RegisterPreferences();
       profile_.GetPrefs()->SetBoolean(prefs::kSyncPasswords, true);
       service_->set_num_expected_resumes(num_resume_expectations);
@@ -178,13 +200,6 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
       service_->Initialize();
       MessageLoop::current()->Run();
 
-
-      EXPECT_CALL(
-          observer_,
-          Observe(
-              NotificationType(NotificationType::SYNC_PASSPHRASE_ACCEPTED),
-              _,_)).
-          WillOnce(InvokeTask(node_task));
       EXPECT_CALL(
           observer_,
           Observe(

chrome/browser/sync/profile_sync_test_util.h

@@ -46,14 +46,9 @@ ACTION(QuitUIMessageLoop) {
   MessageLoop::current()->Quit();
 }
 
-ACTION_P(InvokeTask, task) {
-  if (task)
-    task->Run();
-}
-
 class TestModelAssociatorHelper {
  public:
-  TestModelAssociatorHelper(browser_sync::TestIdFactory* id_factory)
+  explicit TestModelAssociatorHelper(browser_sync::TestIdFactory* id_factory)
       : id_factory_(id_factory) {
   }
 

chrome/browser/sync/test_profile_sync_service.cc

@@ -11,15 +11,12 @@ namespace browser_sync {
 SyncBackendHostForProfileSyncTest::SyncBackendHostForProfileSyncTest(
     TestProfileSyncService* service,
     Profile* profile,
-    const FilePath& profile_path,
-    const DataTypeController::TypeMap& data_type_controllers,
     Task* initial_condition_setup_task,
     int num_expected_resumes,
     int num_expected_pauses,
     bool set_initial_sync_ended_on_init,
     bool synchronous_init)
-    : browser_sync::SyncBackendHost(service, profile, profile_path,
-                                    data_type_controllers),
+    : browser_sync::SyncBackendHost(service, profile),
       initial_condition_setup_task_(initial_condition_setup_task),
       set_initial_sync_ended_on_init_(set_initial_sync_ended_on_init),
       synchronous_init_(synchronous_init),
@@ -44,35 +41,43 @@ SyncBackendHostForProfileSyncTest::SyncBackendHostForProfileSyncTest(
 }
 
 void
-SyncBackendHostForProfileSyncTest::HandleInitializationCompletedOnFrontendLoop() {
-    set_syncapi_initialized();  // Need to do this asap so task below works.
+SyncBackendHostForProfileSyncTest::
+    HandleInitializationCompletedOnFrontendLoop() {
+  set_syncapi_initialized();  // Need to do this asap so task below works.
 
-    // Set up any nodes the test wants around before model association.
-    if (initial_condition_setup_task_) {
-      initial_condition_setup_task_->Run();
-    }
-
-    // Pretend we downloaded initial updates and set initial sync ended bits
-    // if we were asked to.
-    if (set_initial_sync_ended_on_init_) {
-      UserShare* user_share = core_->syncapi()->GetUserShare();
-      DirectoryManager* dir_manager = user_share->dir_manager.get();
+  // Set up any nodes the test wants around before model association.
+  if (initial_condition_setup_task_) {
+    initial_condition_setup_task_->Run();
+  }
 
-      ScopedDirLookup dir(dir_manager, user_share->name);
-      if (!dir.good())
-        FAIL();
+  // Pretend we downloaded initial updates and set initial sync ended bits
+  // if we were asked to.
+  if (set_initial_sync_ended_on_init_) {
+    UserShare* user_share = core_->syncapi()->GetUserShare();
+    DirectoryManager* dir_manager = user_share->dir_manager.get();
 
-      if (!dir->initial_sync_ended_for_type(syncable::NIGORI)) {
-        ProfileSyncServiceTestHelper::CreateRoot(
-            syncable::NIGORI, test_service_, test_service_->id_factory());
-      }
+    ScopedDirLookup dir(dir_manager, user_share->name);
+    if (!dir.good())
+      FAIL();
 
-      SetInitialSyncEndedForEnabledTypes();
+    if (!dir->initial_sync_ended_for_type(syncable::NIGORI)) {
+      ProfileSyncServiceTestHelper::CreateRoot(
+          syncable::NIGORI, test_service_, test_service_->id_factory());
     }
 
-    SyncBackendHost::HandleInitializationCompletedOnFrontendLoop();
+    SetInitialSyncEndedForEnabledTypes();
   }
 
+  SyncBackendHost::HandleInitializationCompletedOnFrontendLoop();
+}
 
+}  // namespace browser_sync
 
-} // namespace
+void TestProfileSyncService::CreateBackend() {
+  backend_.reset(new browser_sync::SyncBackendHostForProfileSyncTest(
+      this, profile(),
+      initial_condition_setup_task_.release(),
+      num_expected_resumes_, num_expected_pauses_,
+      set_initial_sync_ended_on_init_,
+      synchronous_backend_initialization_));
+}

chrome/browser/sync/test_profile_sync_service.h

@@ -65,8 +65,6 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
   SyncBackendHostForProfileSyncTest(
       TestProfileSyncService* service,
       Profile* profile,
-      const FilePath& profile_path,
-      const DataTypeController::TypeMap& data_type_controllers,
       Task* initial_condition_setup_task,
       int num_expected_resumes,
       int num_expected_pauses,
@@ -93,10 +91,13 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
     }
   }
 
-  virtual void ConfigureDataTypes(const syncable::ModelTypeSet& types,
+  virtual void ConfigureDataTypes(
+      const DataTypeController::TypeMap& data_type_controllers,
+      const syncable::ModelTypeSet& types,
       CancelableTask* ready_task) {
     SetAutofillMigrationState(syncable::MIGRATED);
-    SyncBackendHost::ConfigureDataTypes(types, ready_task);
+    SyncBackendHost::ConfigureDataTypes(
+        data_type_controllers, types, ready_task);
   }
 
   // Called when a nudge comes in.
@@ -164,9 +165,7 @@ class TestProfileSyncService : public ProfileSyncService {
                          const std::string& test_user,
                          bool synchronous_backend_initialization,
                          Task* initial_condition_setup_task)
-      : ProfileSyncService(factory, profile,
-                           !test_user.empty() ?
-                           test_user : ""),
+      : ProfileSyncService(factory, profile, test_user),
         synchronous_backend_initialization_(
             synchronous_backend_initialization),
         synchronous_sync_configuration_(false),
@@ -179,16 +178,6 @@ class TestProfileSyncService : public ProfileSyncService {
   }
   virtual ~TestProfileSyncService() { }
 
-  virtual void CreateBackend() {
-    backend_.reset(new browser_sync::SyncBackendHostForProfileSyncTest(
-        this, profile(),
-        profile()->GetPath(), data_type_controllers(),
-        initial_condition_setup_task_.release(),
-        num_expected_resumes_, num_expected_pauses_,
-        set_initial_sync_ended_on_init_,
-        synchronous_backend_initialization_));
-  }
-
   virtual void OnBackendInitialized() {
     ProfileSyncService::OnBackendInitialized();
     // TODO(akalin): Figure out a better way to do this.
@@ -222,6 +211,9 @@ class TestProfileSyncService : public ProfileSyncService {
 
   browser_sync::TestIdFactory* id_factory() { return &id_factory_; }
 
+ protected:
+  virtual void CreateBackend();
+
  private:
   // When testing under ChromiumOS, this method must not return an empty
   // value value in order for the profile sync service to start.

chrome/common/notification_type.h

@@ -1155,20 +1155,6 @@ class NotificationType {
     // session data.
     FOREIGN_SESSION_DISABLED,
 
-    // The syncer requires a passphrase to decrypt sensitive updates. This
-    // notification is sent when the first sensitive data type is setup by the
-    // user as well as anytime any the passphrase is changed in another synced
-    // client.  The source is the SyncBackendHost wanting a passphrase.  The
-    // details are a boolean: true if the passphrase is required for decryption,
-    // false if only required for encryption.
-    SYNC_PASSPHRASE_REQUIRED,
-
-    // Sent when the passphrase provided by the user is accepted. After this
-    // notification is sent, updates to sensitive nodes are encrypted using the
-    // accepted passphrase.  The source is the SyncBackendHost that accepted
-    // the passphrase.  No details.
-    SYNC_PASSPHRASE_ACCEPTED,
-
     // Sent when the set of data types that should be synced has been modified
     // externally (eg. by the dom_ui options screen).
     // The source is the Profile, there are no details.