diff options
author | skrul@chromium.org <skrul@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-17 21:56:00 +0000 |
---|---|---|
committer | skrul@chromium.org <skrul@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-17 21:56:00 +0000 |
commit | 9348c1f762db498d399d07c0295e36d75ee7fa08 (patch) | |
tree | e07350bf22f6768734e2e444b048fb24498f0f8f /chrome | |
parent | 017f95fdd71c853a98777b57eca08b0e6ea49a7b (diff) | |
download | chromium_src-9348c1f762db498d399d07c0295e36d75ee7fa08.zip chromium_src-9348c1f762db498d399d07c0295e36d75ee7fa08.tar.gz chromium_src-9348c1f762db498d399d07c0295e36d75ee7fa08.tar.bz2 |
Pause the sync thread while starting data types.
A few things to note:
- I ended the plubming (it's-a me, mario!) at the SyncBackendHost (the PSS does not know about pausing). The SyncBackendHost now has RequestPause/RequestResume methods. The notifications that are sent when pause and resume is complete are dispatched to the notification service by the SyncBackendHost.
- Calls to SHB::Request(Pause|Resume) go directly to the similar named methods on the SyncerThread. This is unlike other methods on the SHB which are usually dispatched to the "core" thread before calling deeper into the onion. I did this to avoid having to deal with callbacks to carry the result value of pause/resume (since the methods can fail and return false). If this is a problem I can change the way it works.
- There is a little trickery regarding the integration unit tests since we typically use a specially initialized SBH for these tests, and the syncer thread does not really run for these tests. Since the thread is not running, pause and resume won't work properly. To get around this, the DataTypeManagerImpl in these tests is constructed with a mock SBH that will respond properly to pause and resume requests.
BUG=37154,37553
Review URL: http://codereview.chromium.org/1063002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41883 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
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, |