diff options
21 files changed, 348 insertions, 51 deletions
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index a149a08..484e6c3 100644 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -1249,6 +1249,14 @@ void SyncManager::Authenticate(const char* username, const char* password, std::string(captcha)); } +bool SyncManager::RequestPause() { + return data_->syncer_thread()->RequestPause(); +} + +bool SyncManager::RequestResume() { + return data_->syncer_thread()->RequestResume(); +} + const std::string& SyncManager::GetAuthenticatedUsername() { DCHECK(data_); return data_->username_for_share(); @@ -1745,6 +1753,16 @@ void SyncManager::SyncInternal::HandleSyncerEvent(const SyncerEvent& event) { << " talk_mediator(): " << talk_mediator(); } } + + if (event.what_happened == SyncerEvent::PAUSED) { + observer_->OnPaused(); + return; + } + + if (event.what_happened == SyncerEvent::RESUMED) { + observer_->OnResumed(); + return; + } } void SyncManager::SyncInternal::HandleAuthWatcherEvent( diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h index 0b0572f..459ff40 100644 --- a/chrome/browser/sync/engine/syncapi.h +++ b/chrome/browser/sync/engine/syncapi.h @@ -570,6 +570,12 @@ class SyncManager { // message, unless otherwise specified, produces undefined behavior. virtual void OnInitializationComplete() = 0; + // The syncer thread has been paused. + virtual void OnPaused() = 0; + + // The syncer thread has been resumed. + virtual void OnResumed() = 0; + private: DISALLOW_COPY_AND_ASSIGN(Observer); }; @@ -644,6 +650,18 @@ class SyncManager { void Authenticate(const char* username, const char* password, const char* captcha); + // Requests the syncer thread to pause. The observer's OnPause + // method will be called when the syncer thread is paused. Returns + // false if the syncer thread can not be paused (e.g. if it is not + // started). + bool RequestPause(); + + // Requests the syncer thread to resume. The observer's OnResume + // method will be called when the syncer thread is resumed. Returns + // false if the syncer thread can not be resumed (e.g. if it is not + // paused). + bool RequestResume(); + // Adds a listener to be notified of sync events. // NOTE: It is OK (in fact, it's probably a good idea) to call this before // having received OnInitializationCompleted. diff --git a/chrome/browser/sync/glue/data_type_manager.h b/chrome/browser/sync/glue/data_type_manager.h index 652fc06..401af56 100644 --- a/chrome/browser/sync/glue/data_type_manager.h +++ b/chrome/browser/sync/glue/data_type_manager.h @@ -16,10 +16,12 @@ namespace browser_sync { class DataTypeManager { public: enum State { - STOPPED, // No data types are currently running. - STARTING, // Data types are being started. - STARTED, // All enabled data types are running. - STOPPING // Data types are being stopped. + STOPPED, // No data types are currently running. + PAUSE_PENDING, // Waiting for the sync backend to pause. + STARTING, // Data types are being started. + RESUME_PENDING, // Waiting for the sync backend to resume. + STARTED, // All enabled data types are running. + STOPPING // Data types are being stopped. }; enum StartResult { diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc index 5713e91..aba040c 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl.cc @@ -6,6 +6,10 @@ #include "base/task.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/sync/glue/data_type_manager_impl.h" +#include "chrome/browser/sync/glue/sync_backend_host.h" +#include "chrome/common/notification_details.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_source.h" static const int kStartOrderStart = -1; @@ -22,10 +26,13 @@ static const syncable::ModelType kStartOrder[] = { namespace browser_sync { DataTypeManagerImpl::DataTypeManagerImpl( + SyncBackendHost* backend, const DataTypeController::TypeMap& controllers) - : controllers_(controllers), + : backend_(backend), + controllers_(controllers), state_(DataTypeManager::STOPPED), current_type_(kStartOrderStart) { + DCHECK(backend_); DCHECK(arraysize(kStartOrder) > 0); // Ensure all data type controllers are stopped. for (DataTypeController::TypeMap::const_iterator it = controllers_.begin(); @@ -42,10 +49,18 @@ void DataTypeManagerImpl::Start(StartCallback* start_callback) { return; } - state_ = STARTING; + state_ = PAUSE_PENDING; start_callback_.reset(start_callback); current_type_ = kStartOrderStart; - StartNextType(); + + // Pause the sync backend before starting the data types. + AddObserver(NotificationType::SYNC_PAUSED); + if (!backend_->RequestPause()) { + RemoveObserver(NotificationType::SYNC_PAUSED); + state_ = STOPPED; + start_callback_->Run(UNRECOVERABLE_ERROR); + start_callback_.reset(); + } } void DataTypeManagerImpl::StartNextType() { @@ -67,11 +82,17 @@ void DataTypeManagerImpl::StartNextType() { } } - // No more startable types found, we must be done. + // No more startable types found, we must be done. Resume the sync + // backend to finish. DCHECK_EQ(state_, STARTING); - state_ = STARTED; - start_callback_->Run(OK); - start_callback_.reset(); + AddObserver(NotificationType::SYNC_RESUMED); + state_ = RESUME_PENDING; + if (!backend_->RequestResume()) { + RemoveObserver(NotificationType::SYNC_RESUMED); + FinishStop(); + start_callback_->Run(UNRECOVERABLE_ERROR); + start_callback_.reset(); + } } void DataTypeManagerImpl::TypeStartCallback( @@ -143,7 +164,7 @@ void DataTypeManagerImpl::Stop() { } void DataTypeManagerImpl::FinishStop() { - DCHECK(state_== STARTING || state_ == STOPPING); + DCHECK(state_== STARTING || state_ == STOPPING || state_ == RESUME_PENDING); // Simply call the Stop() method on all running data types. for (unsigned int i = 0; i < arraysize(kStartOrder); ++i) { syncable::ModelType type = kStartOrder[i]; @@ -164,4 +185,38 @@ bool DataTypeManagerImpl::IsEnabled(syncable::ModelType type) { return IsRegistered(type) && controllers_[type]->enabled(); } +void DataTypeManagerImpl::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch(type.value) { + case NotificationType::SYNC_PAUSED: + DCHECK_EQ(state_, PAUSE_PENDING); + state_ = STARTING; + RemoveObserver(NotificationType::SYNC_PAUSED); + StartNextType(); + break; + case NotificationType::SYNC_RESUMED: + DCHECK_EQ(state_, RESUME_PENDING); + state_ = STARTED; + RemoveObserver(NotificationType::SYNC_RESUMED); + start_callback_->Run(OK); + start_callback_.reset(); + break; + default: + NOTREACHED(); + } +} + +void DataTypeManagerImpl::AddObserver(NotificationType type) { + notification_registrar_.Add(this, + type, + NotificationService::AllSources()); +} + +void DataTypeManagerImpl::RemoveObserver(NotificationType type) { + notification_registrar_.Remove(this, + type, + NotificationService::AllSources()); +} + } // namespace browser_sync diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h index a9b87d4..623c8f5 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.h +++ b/chrome/browser/sync/glue/data_type_manager_impl.h @@ -5,15 +5,26 @@ #ifndef CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_MANAGER_IMPL_H__ #define CHROME_BROWSER_SYNC_GLUE_DATA_TYPE_MANAGER_IMPL_H__ +#include "chrome/browser/sync/glue/data_type_manager.h" + #include "base/basictypes.h" #include "base/scoped_ptr.h" -#include "chrome/browser/sync/glue/data_type_manager.h" +#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_type.h" + +class NotificationSource; +class NotificationDetails; namespace browser_sync { -class DataTypeManagerImpl : public DataTypeManager { +class SyncBackendHost; + +class DataTypeManagerImpl : public DataTypeManager, + public NotificationObserver { public: - explicit DataTypeManagerImpl(const DataTypeController::TypeMap& controllers); + DataTypeManagerImpl(SyncBackendHost* backend, + const DataTypeController::TypeMap& controllers); virtual ~DataTypeManagerImpl() {} // DataTypeManager interface. @@ -33,6 +44,11 @@ class DataTypeManagerImpl : public DataTypeManager { return state_; } + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + private: // Starts the next data type in the kStartOrder list, indicated by // the current_type_ member. If there are no more data types to @@ -45,11 +61,16 @@ class DataTypeManagerImpl : public DataTypeManager { // Stops all data types. void FinishStop(); + void AddObserver(NotificationType type); + void RemoveObserver(NotificationType type); + + SyncBackendHost* backend_; DataTypeController::TypeMap controllers_; State state_; int current_type_; scoped_ptr<StartCallback> start_callback_; + NotificationRegistrar notification_registrar_; DISALLOW_COPY_AND_ASSIGN(DataTypeManagerImpl); }; diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc index 60115b4..a7843f1 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -11,13 +11,18 @@ #include "chrome/browser/sync/glue/data_type_controller.h" #include "chrome/browser/sync/glue/data_type_controller_mock.h" #include "chrome/browser/sync/glue/data_type_manager_impl.h" +#include "chrome/browser/sync/glue/sync_backend_host_mock.h" +#include "chrome/browser/sync/profile_sync_test_util.h" +#include "chrome/common/notification_type.h" #include "testing/gmock/include/gmock/gmock.h" using browser_sync::DataTypeManager; using browser_sync::DataTypeManagerImpl; using browser_sync::DataTypeController; using browser_sync::DataTypeControllerMock; +using browser_sync::SyncBackendHostMock; using testing::_; +using testing::DoAll; using testing::Return; using testing::SaveArg; @@ -78,10 +83,11 @@ class DataTypeManagerImplTest : public testing::Test { ChromeThread ui_thread_; DataTypeController::TypeMap controllers_; StartCallback callback_; + SyncBackendHostMock backend_; }; TEST_F(DataTypeManagerImplTest, NoControllers) { - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_CALL(callback_, Run(DataTypeManager::OK)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STARTED, dtm.state()); @@ -98,7 +104,7 @@ TEST_F(DataTypeManagerImplTest, OneDisabledController) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_CALL(callback_, Run(DataTypeManager::OK)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STARTED, dtm.state()); @@ -111,7 +117,7 @@ TEST_F(DataTypeManagerImplTest, OneEnabledController) { SetStartStopExpectations(bookmark_dtc); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_CALL(callback_, Run(DataTypeManager::OK)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STARTED, dtm.state()); @@ -130,7 +136,9 @@ TEST_F(DataTypeManagerImplTest, OneFailingController) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); + EXPECT_CALL(backend_, RequestPause()). + WillOnce(DoAll(Notify(NotificationType::SYNC_PAUSED), Return(true))); EXPECT_CALL(callback_, Run(DataTypeManager::ASSOCIATION_FAILED)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STOPPED, dtm.state()); @@ -145,7 +153,7 @@ TEST_F(DataTypeManagerImplTest, TwoEnabledControllers) { SetStartStopExpectations(preference_dtc); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_CALL(callback_, Run(DataTypeManager::OK)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STARTED, dtm.state()); @@ -168,7 +176,9 @@ TEST_F(DataTypeManagerImplTest, InterruptedStart) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); + EXPECT_CALL(backend_, RequestPause()). + WillOnce(DoAll(Notify(NotificationType::SYNC_PAUSED), Return(true))); EXPECT_CALL(callback_, Run(DataTypeManager::ABORTED)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STARTING, dtm.state()); @@ -185,7 +195,7 @@ TEST_F(DataTypeManagerImplTest, SecondControllerFails) { SetStartStopExpectations(bookmark_dtc); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeControllerMock* preference_dtc = MakeBookmarkDTC(); + DataTypeControllerMock* preference_dtc = MakePreferenceDTC(); EXPECT_CALL(*preference_dtc, Start(true, _)). WillOnce(InvokeCallback((DataTypeController::ASSOCIATION_FAILED))); EXPECT_CALL(*preference_dtc, Stop()).Times(0); @@ -193,14 +203,16 @@ TEST_F(DataTypeManagerImplTest, SecondControllerFails) { WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); + EXPECT_CALL(backend_, RequestPause()). + WillOnce(DoAll(Notify(NotificationType::SYNC_PAUSED), Return(true))); EXPECT_CALL(callback_, Run(DataTypeManager::ASSOCIATION_FAILED)); dtm.Start(NewCallback(&callback_, &StartCallback::Run)); EXPECT_EQ(DataTypeManager::STOPPED, dtm.state()); } TEST_F(DataTypeManagerImplTest, IsRegisteredNone) { - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_FALSE(dtm.IsRegistered(syncable::BOOKMARKS)); } @@ -209,7 +221,7 @@ TEST_F(DataTypeManagerImplTest, IsRegisteredButNoMatch) { EXPECT_CALL(*preference_dtc, state()). WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::PREFERENCES] = preference_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_FALSE(dtm.IsRegistered(syncable::BOOKMARKS)); } @@ -218,7 +230,7 @@ TEST_F(DataTypeManagerImplTest, IsRegisteredMatch) { EXPECT_CALL(*bookmark_dtc, state()). WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_TRUE(dtm.IsRegistered(syncable::BOOKMARKS)); } @@ -229,7 +241,7 @@ TEST_F(DataTypeManagerImplTest, IsNotEnabled) { EXPECT_CALL(*bookmark_dtc, enabled()). WillRepeatedly(Return(false)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_FALSE(dtm.IsEnabled(syncable::BOOKMARKS)); } @@ -240,6 +252,38 @@ TEST_F(DataTypeManagerImplTest, IsEnabled) { EXPECT_CALL(*bookmark_dtc, enabled()). WillRepeatedly(Return(true)); controllers_[syncable::BOOKMARKS] = bookmark_dtc; - DataTypeManagerImpl dtm(controllers_); + DataTypeManagerImpl dtm(&backend_, controllers_); EXPECT_TRUE(dtm.IsEnabled(syncable::BOOKMARKS)); } + +TEST_F(DataTypeManagerImplTest, PauseFailed) { + DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC(); + EXPECT_CALL(*bookmark_dtc, Start(_, _)).Times(0); + EXPECT_CALL(*bookmark_dtc, state()). + WillRepeatedly(Return(DataTypeController::NOT_RUNNING)); + controllers_[syncable::BOOKMARKS] = bookmark_dtc; + + DataTypeManagerImpl dtm(&backend_, controllers_); + EXPECT_CALL(backend_, RequestPause()). + WillOnce(Return(false)); + + EXPECT_CALL(callback_, Run(DataTypeManager::UNRECOVERABLE_ERROR)); + dtm.Start(NewCallback(&callback_, &StartCallback::Run)); + EXPECT_EQ(DataTypeManager::STOPPED, dtm.state()); +} + +TEST_F(DataTypeManagerImplTest, ResumeFailed) { + DataTypeControllerMock* bookmark_dtc = MakeBookmarkDTC(); + SetStartStopExpectations(bookmark_dtc); + controllers_[syncable::BOOKMARKS] = bookmark_dtc; + + DataTypeManagerImpl dtm(&backend_, controllers_); + EXPECT_CALL(backend_, RequestPause()). + WillOnce(DoAll(Notify(NotificationType::SYNC_PAUSED), Return(true))); + EXPECT_CALL(backend_, RequestResume()). + WillOnce(Return(false)); + + EXPECT_CALL(callback_, Run(DataTypeManager::UNRECOVERABLE_ERROR)); + dtm.Start(NewCallback(&callback_, &StartCallback::Run)); + EXPECT_EQ(DataTypeManager::STOPPED, dtm.state()); +} diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 1a16776..113a4b1 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -11,6 +11,8 @@ #include "chrome/browser/sync/glue/database_model_worker.h" #include "chrome/browser/sync/glue/sync_backend_host.h" #include "chrome/browser/sync/glue/http_bridge.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" #include "webkit/glue/webkit_glue.h" static const int kSaveChangesIntervalSeconds = 10; @@ -39,6 +41,13 @@ SyncBackendHost::SyncBackendHost( core_ = new Core(this); } +SyncBackendHost::SyncBackendHost() + : core_thread_("Chrome_SyncCoreThread"), + frontend_loop_(MessageLoop::current()), + frontend_(NULL), + last_auth_error_(AuthError::None()) { +} + SyncBackendHost::~SyncBackendHost() { DCHECK(!core_ && !frontend_) << "Must call Shutdown before destructor."; DCHECK(registrar_.workers.empty()); @@ -168,6 +177,14 @@ void SyncBackendHost::DeactivateDataType( // with this data type from the sync db. } +bool SyncBackendHost::RequestPause() { + return core_->syncapi()->RequestPause(); +} + +bool SyncBackendHost::RequestResume() { + return core_->syncapi()->RequestResume(); +} + void SyncBackendHost::Core::NotifyFrontend(FrontendNotification notification) { if (!host_ || !host_->frontend_) { return; // This can happen in testing because the UI loop processes tasks @@ -184,6 +201,18 @@ void SyncBackendHost::Core::NotifyFrontend(FrontendNotification notification) { } } +void SyncBackendHost::Core::NotifyPaused() { + NotificationService::current()->Notify(NotificationType::SYNC_PAUSED, + NotificationService::AllSources(), + NotificationService::NoDetails()); +} + +void SyncBackendHost::Core::NotifyResumed() { + NotificationService::current()->Notify(NotificationType::SYNC_RESUMED, + NotificationService::AllSources(), + NotificationService::NoDetails()); +} + SyncBackendHost::UserShareHandle SyncBackendHost::GetUserShareHandle() const { return core_->syncapi()->GetUserShare(); } @@ -342,6 +371,10 @@ void SyncBackendHost::Core::OnChangesApplied( return; ChangeProcessor* processor = it->second; + // Ensure the change processor is willing to accept changes. + if (!processor->IsRunning()) + return; + processor->ApplyChangesFromSyncModel(trans, changes, change_count); } @@ -373,6 +406,18 @@ void SyncBackendHost::Core::OnAuthError(const AuthError& auth_error) { auth_error)); } +void SyncBackendHost::Core::OnPaused() { + host_->frontend_loop_->PostTask( + FROM_HERE, + NewRunnableMethod(this, &Core::NotifyPaused)); +} + +void SyncBackendHost::Core::OnResumed() { + host_->frontend_loop_->PostTask( + FROM_HERE, + NewRunnableMethod(this, &Core::NotifyResumed)); +} + void SyncBackendHost::Core::HandleAuthErrorEventOnFrontendLoop( const GoogleServiceAuthError& new_auth_error) { if (!host_ || !host_->frontend_) diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index fe425dc..405fe35 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -76,6 +76,9 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { SyncBackendHost(SyncFrontend* frontend, const FilePath& profile_path, const DataTypeController::TypeMap& data_type_controllers); + // For testing. + // TODO(skrul): Extract an interface so this is not needed. + SyncBackendHost(); ~SyncBackendHost(); // Called on |frontend_loop_| to kick off asynchronous initialization. @@ -110,6 +113,16 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { void DeactivateDataType(DataTypeController* data_type_controller, ChangeProcessor* change_processor); + // Requests the backend to pause. Returns true if the request is + // sent sucessfully. When the backend does pause, a SYNC_PAUSED + // notification is sent to the notification service. + virtual bool RequestPause(); + + // Requests the backend to resume. Returns true if the request is + // sent sucessfully. When the backend does resume, a SYNC_RESUMED + // notification is sent to the notification service. + virtual bool RequestResume(); + // Called on |frontend_loop_| to obtain a handle to the UserShare needed // for creating transactions. UserShareHandle GetUserShareHandle() const; @@ -177,6 +190,8 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { virtual void OnSyncCycleCompleted(); virtual void OnInitializationComplete(); virtual void OnAuthError(const GoogleServiceAuthError& auth_error); + virtual void OnPaused(); + virtual void OnResumed(); struct DoInitializeOptions { DoInitializeOptions( @@ -288,6 +303,14 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { // |frontend_loop_|. void NotifyFrontend(FrontendNotification notification); + // Sends a SYNC_PAUSED notification to the notification service on + // the UI thread. + void NotifyPaused(); + + // Sends a SYNC_RESUMED notification to the notification service + // on the UI thread. + void NotifyResumed(); + // Invoked when initialization of syncapi is complete and we can start // our timer. // This must be called from the thread on which SaveChanges is intended to diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.h b/chrome/browser/sync/glue/sync_backend_host_mock.h new file mode 100644 index 0000000..fc27b02 --- /dev/null +++ b/chrome/browser/sync/glue/sync_backend_host_mock.h @@ -0,0 +1,34 @@ +// 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_SYNC_BACKEND_HOST_MOCK_H__ +#define CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_MOCK_H__ + +#include "chrome/browser/sync/glue/sync_backend_host.h" +#include "chrome/browser/sync/profile_sync_test_util.h" +#include "chrome/common/notification_type.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace browser_sync { + +class SyncBackendHostMock : public SyncBackendHost { + public: + SyncBackendHostMock() { + // By default, the RequestPause and RequestResume methods will + // send the confirmation notification and return true. + ON_CALL(*this, RequestPause()). + WillByDefault(testing::DoAll(Notify(NotificationType::SYNC_PAUSED), + testing::Return(true))); + ON_CALL(*this, RequestResume()). + WillByDefault(testing::DoAll(Notify(NotificationType::SYNC_RESUMED), + testing::Return(true))); + } + + MOCK_METHOD0(RequestPause, bool()); + MOCK_METHOD0(RequestResume, bool()); +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_MOCK_H__ diff --git a/chrome/browser/sync/profile_sync_factory.h b/chrome/browser/sync/profile_sync_factory.h index 6aef77f..ae61f05 100644 --- a/chrome/browser/sync/profile_sync_factory.h +++ b/chrome/browser/sync/profile_sync_factory.h @@ -17,6 +17,7 @@ class WebDatabase; namespace browser_sync { class DataTypeManager; +class SyncBackendHost; class UnrecoverableErrorHandler; } @@ -42,9 +43,11 @@ class ProfileSyncFactory { // is owned by the caller. virtual ProfileSyncService* CreateProfileSyncService() = 0; - // Instantiates a new DataTypeManager with a list of data type - // controllers. The return pointer is owned by the caller. + // Instantiates a new DataTypeManager with a SyncBackendHost and a + // list of data type controllers. The return pointer is owned by + // the caller. virtual browser_sync::DataTypeManager* CreateDataTypeManager( + browser_sync::SyncBackendHost* backend, const browser_sync::DataTypeController::TypeMap& controllers) = 0; // Instantiates both a model associator and change processor for the diff --git a/chrome/browser/sync/profile_sync_factory_impl.cc b/chrome/browser/sync/profile_sync_factory_impl.cc index 8f3e1c2..a687e3b 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_factory_impl.cc @@ -15,6 +15,7 @@ #include "chrome/browser/sync/glue/preference_change_processor.h" #include "chrome/browser/sync/glue/preference_data_type_controller.h" #include "chrome/browser/sync/glue/preference_model_associator.h" +#include "chrome/browser/sync/glue/sync_backend_host.h" #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/profile_sync_factory_impl.h" #include "chrome/browser/webdata/web_data_service.h" @@ -34,6 +35,7 @@ using browser_sync::PreferenceDataTypeController; using browser_sync::PreferenceChangeProcessor; using browser_sync::PreferenceDataTypeController; using browser_sync::PreferenceModelAssociator; +using browser_sync::SyncBackendHost; using browser_sync::UnrecoverableErrorHandler; ProfileSyncFactoryImpl::ProfileSyncFactoryImpl( @@ -73,8 +75,9 @@ ProfileSyncService* ProfileSyncFactoryImpl::CreateProfileSyncService() { } DataTypeManager* ProfileSyncFactoryImpl::CreateDataTypeManager( + SyncBackendHost* backend, const DataTypeController::TypeMap& controllers) { - return new DataTypeManagerImpl(controllers); + return new DataTypeManagerImpl(backend, controllers); } ProfileSyncFactory::SyncComponents diff --git a/chrome/browser/sync/profile_sync_factory_impl.h b/chrome/browser/sync/profile_sync_factory_impl.h index c47718e..2989c53 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.h +++ b/chrome/browser/sync/profile_sync_factory_impl.h @@ -20,6 +20,7 @@ class ProfileSyncFactoryImpl : public ProfileSyncFactory { virtual ProfileSyncService* CreateProfileSyncService(); virtual browser_sync::DataTypeManager* CreateDataTypeManager( + browser_sync::SyncBackendHost* backend, const browser_sync::DataTypeController::TypeMap& controllers); virtual SyncComponents CreateAutofillSyncComponents( diff --git a/chrome/browser/sync/profile_sync_factory_mock.h b/chrome/browser/sync/profile_sync_factory_mock.h index 71f65fd..d635b6e 100644 --- a/chrome/browser/sync/profile_sync_factory_mock.h +++ b/chrome/browser/sync/profile_sync_factory_mock.h @@ -24,10 +24,10 @@ class ProfileSyncFactoryMock : public ProfileSyncFactory { MOCK_METHOD0(CreateProfileSyncService, ProfileSyncService*(void)); - MOCK_METHOD1(CreateDataTypeManager, + MOCK_METHOD2(CreateDataTypeManager, browser_sync::DataTypeManager*( - const browser_sync::DataTypeController::TypeMap& - controllers)); + browser_sync::SyncBackendHost*, + const browser_sync::DataTypeController::TypeMap&)); MOCK_METHOD3(CreateAutofillSyncComponents, SyncComponents( ProfileSyncService* profile_sync_service, diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index fd22c67..256f443 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -411,7 +411,7 @@ void ProfileSyncService::StartProcessingChangesIfReady() { } data_type_manager_.reset( - factory_->CreateDataTypeManager(data_type_controllers_)); + factory_->CreateDataTypeManager(backend_.get(), data_type_controllers_)); data_type_manager_->Start( NewCallback(this, &ProfileSyncService::DataTypeManagerStartCallback)); } diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc index aed7a9d..9d3db02 100644 --- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc @@ -18,7 +18,7 @@ #include "chrome/browser/sync/glue/autofill_change_processor.h" #include "chrome/browser/sync/glue/autofill_data_type_controller.h" #include "chrome/browser/sync/glue/autofill_model_associator.h" -#include "chrome/browser/sync/glue/sync_backend_host.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" @@ -40,7 +40,7 @@ using base::WaitableEvent; using browser_sync::AutofillChangeProcessor; using browser_sync::AutofillDataTypeController; using browser_sync::AutofillModelAssociator; -using browser_sync::SyncBackendHost; +using browser_sync::SyncBackendHostMock; using browser_sync::TestIdFactory; using browser_sync::UnrecoverableErrorHandler; using sync_api::SyncManager; @@ -243,8 +243,8 @@ class ProfileSyncServiceAutofillTest : public testing::Test { WillOnce(MakeAutofillSyncComponents(service_.get(), &web_database_, data_type_controller)); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). - WillOnce(MakeDataTypeManager()); + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). + WillOnce(MakeDataTypeManager(&backend_)); EXPECT_CALL(profile_, GetWebDataService(_)). WillOnce(Return(web_data_service_.get())); @@ -363,6 +363,7 @@ class ProfileSyncServiceAutofillTest : public testing::Test { ProfileMock profile_; ProfileSyncFactoryMock factory_; ProfileSyncServiceObserverMock observer_; + SyncBackendHostMock backend_; WebDatabaseMock web_database_; scoped_refptr<WebDataService> web_data_service_; @@ -406,6 +407,10 @@ class AddAutofillEntriesTask : public Task { // TODO(skrul): Test processing of cloud changes. TEST_F(ProfileSyncServiceAutofillTest, FailModelAssociation) { + // Backend will be paused but not resumed. + EXPECT_CALL(backend_, RequestPause()). + WillOnce(testing::DoAll(Notify(NotificationType::SYNC_PAUSED), + testing::Return(true))); // Don't create the root autofill node so startup fails. StartSyncService(NULL); EXPECT_TRUE(service_->unrecoverable_error_detected()); diff --git a/chrome/browser/sync/profile_sync_service_preference_unittest.cc b/chrome/browser/sync/profile_sync_service_preference_unittest.cc index f17afa2..76bc94b 100644 --- a/chrome/browser/sync/profile_sync_service_preference_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_preference_unittest.cc @@ -8,6 +8,7 @@ #include "chrome/browser/sync/glue/preference_data_type_controller.h" #include "chrome/browser/sync/glue/preference_model_associator.h" #include "chrome/browser/sync/glue/sync_backend_host.h" +#include "chrome/browser/sync/glue/sync_backend_host_mock.h" #include "chrome/browser/sync/profile_sync_factory_mock.h" #include "chrome/browser/sync/profile_sync_test_util.h" #include "chrome/browser/sync/protocol/preference_specifics.pb.h" @@ -23,6 +24,7 @@ using browser_sync::PreferenceChangeProcessor; using browser_sync::PreferenceDataTypeController; using browser_sync::PreferenceModelAssociator; using browser_sync::SyncBackendHost; +using browser_sync::SyncBackendHostMock; using sync_api::SyncManager; using testing::_; using testing::Return; @@ -65,8 +67,8 @@ class ProfileSyncServicePreferenceTest : public testing::Test { EXPECT_CALL(factory_, CreatePreferenceSyncComponents(_, _)). WillOnce(Return(ProfileSyncFactory::SyncComponents( model_associator_, change_processor_))); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). - WillOnce(MakeDataTypeManager()); + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). + WillOnce(MakeDataTypeManager(&backend_mock_)); service_->RegisterDataTypeController( new PreferenceDataTypeController(&factory_, @@ -133,6 +135,7 @@ class ProfileSyncServicePreferenceTest : public testing::Test { scoped_ptr<TestProfileSyncService> service_; scoped_ptr<TestingProfile> profile_; ProfileSyncFactoryMock factory_; + SyncBackendHostMock backend_mock_; PreferenceModelAssociator* model_associator_; PreferenceChangeProcessor* change_processor_; diff --git a/chrome/browser/sync/profile_sync_service_startup_unittest.cc b/chrome/browser/sync/profile_sync_service_startup_unittest.cc index 5be62fc..e828109 100644 --- a/chrome/browser/sync/profile_sync_service_startup_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_startup_unittest.cc @@ -59,7 +59,7 @@ class ProfileSyncServiceStartupTest : public testing::Test { protected: DataTypeManagerMock* SetUpDataTypeManager() { DataTypeManagerMock* data_type_manager = new DataTypeManagerMock(); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). WillOnce(Return(data_type_manager)); return data_type_manager; } diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index fd550f6..515d7d1 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -21,6 +21,7 @@ #include "chrome/browser/sync/glue/data_type_controller.h" #include "chrome/browser/sync/glue/model_associator.h" #include "chrome/browser/sync/glue/sync_backend_host.h" +#include "chrome/browser/sync/glue/sync_backend_host_mock.h" #include "chrome/browser/sync/notification_method.h" #include "chrome/browser/sync/profile_sync_factory.h" #include "chrome/browser/sync/profile_sync_factory_mock.h" @@ -39,6 +40,7 @@ using browser_sync::ChangeProcessor; using browser_sync::DataTypeController; using browser_sync::ModelAssociator; using browser_sync::SyncBackendHost; +using browser_sync::SyncBackendHostMock; using testing::_; using testing::Return; @@ -239,8 +241,8 @@ class ProfileSyncServiceTest : public testing::Test { EXPECT_CALL(factory_, CreateBookmarkSyncComponents(_, _)). WillOnce(Return(ProfileSyncFactory::SyncComponents( model_associator_, change_processor_))); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). - WillOnce(MakeDataTypeManager()); + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). + WillOnce(MakeDataTypeManager(&backend_mock_)); service_->RegisterDataTypeController( new browser_sync::BookmarkDataTypeController(&factory_, @@ -426,6 +428,7 @@ class ProfileSyncServiceTest : public testing::Test { scoped_ptr<TestProfileSyncService> service_; scoped_ptr<TestingProfile> profile_; ProfileSyncFactoryMock factory_; + SyncBackendHostMock backend_mock_; BookmarkModel* model_; TestBookmarkModelAssociator* model_associator_; BookmarkChangeProcessor* change_processor_; @@ -443,8 +446,8 @@ TEST_F(ProfileSyncServiceTest, InitialState) { TEST_F(ProfileSyncServiceTest, AbortedByShutdown) { service_.reset(new TestProfileSyncService(&factory_, profile_.get(), false)); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). - WillOnce(MakeDataTypeManager()); + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). + WillOnce(MakeDataTypeManager(&backend_mock_)); EXPECT_CALL(factory_, CreateBookmarkSyncComponents(_, _)).Times(0); service_->RegisterDataTypeController( new browser_sync::BookmarkDataTypeController(&factory_, @@ -1330,8 +1333,8 @@ TEST_F(ProfileSyncServiceTestWithData, MAYBE_TestStartupWithOldSyncData) { EXPECT_CALL(factory_, CreateBookmarkSyncComponents(_, _)). WillOnce(Return(ProfileSyncFactory::SyncComponents( model_associator_, change_processor_))); - EXPECT_CALL(factory_, CreateDataTypeManager(_)). - WillOnce(MakeDataTypeManager()); + EXPECT_CALL(factory_, CreateDataTypeManager(_, _)). + WillOnce(MakeDataTypeManager(&backend_mock_)); service_->RegisterDataTypeController( new browser_sync::BookmarkDataTypeController(&factory_, diff --git a/chrome/browser/sync/profile_sync_test_util.h b/chrome/browser/sync/profile_sync_test_util.h index db67101..e480e337 100644 --- a/chrome/browser/sync/profile_sync_test_util.h +++ b/chrome/browser/sync/profile_sync_test_util.h @@ -16,13 +16,23 @@ #include "chrome/browser/sync/profile_sync_factory.h" #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/unrecoverable_error_handler.h" +#include "chrome/common/notification_service.h" #include "chrome/test/sync/test_http_bridge_factory.h" #include "testing/gmock/include/gmock/gmock.h" +ACTION_P(Notify, type) { + NotificationService::current()->Notify(type, + NotificationService::AllSources(), + NotificationService::NoDetails()); +} + // This action is used to mock out the ProfileSyncFactory -// CreateDataTypeManager method. -ACTION(MakeDataTypeManager) { - return new browser_sync::DataTypeManagerImpl(arg0); +// CreateDataTypeManager method. Note that we swap out the supplied +// SyncBackendHost with a mock since using the live SyncBackendHost in +// tests does not run the syncer thread and can not properly respond +// to pause and resume requests. +ACTION_P(MakeDataTypeManager, backend_mock) { + return new browser_sync::DataTypeManagerImpl(backend_mock, arg1); } ACTION_P(InvokeTask, task) { diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 00ebcee..0034769 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -865,6 +865,7 @@ 'browser/sync/glue/database_model_worker_unittest.cc', 'browser/sync/glue/http_bridge_unittest.cc', 'browser/sync/glue/preference_data_type_controller_unittest.cc', + 'browser/sync/glue/sync_backend_host_mock.h', 'browser/sync/glue/ui_model_worker_unittest.cc', 'browser/sync/profile_sync_service_mock.h', 'browser/sync/profile_sync_service_startup_unittest.cc', diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index d1bb966..40211d5 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -868,6 +868,14 @@ class NotificationType { // object, the details are ContentSettingsNotificationsDetails. CONTENT_SETTINGS_CHANGED, + // Sync -------------------------------------------------------------------- + + // Sent when the sync backend has been paused. + SYNC_PAUSED, + + // Sent when the sync backend has been resumed. + SYNC_RESUMED, + #if defined(OS_CHROMEOS) // Sent when a chromium os user logs in. LOGIN_USER_CHANGED, |