diff options
author | haitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-25 16:22:21 +0000 |
---|---|---|
committer | haitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-25 16:22:21 +0000 |
commit | bfd87698e2430dcd86c355b5d2cb4a4485d59790 (patch) | |
tree | 58c89db9b99b9d469fa98a5df645e578936e6141 /sync | |
parent | f43fb604df45f0747c12b66b20976f0512ee1308 (diff) | |
download | chromium_src-bfd87698e2430dcd86c355b5d2cb4a4485d59790.zip chromium_src-bfd87698e2430dcd86c355b5d2cb4a4485d59790.tar.gz chromium_src-bfd87698e2430dcd86c355b5d2cb4a4485d59790.tar.bz2 |
Lock-free shutdown of profile sync service. Changes include:
* Disconnect non-frontend processor/associator to stop accessing directory
so that sync backend can be shut down without waiting.
* Change non-frontend controller so that creation/destruction of
processor/associator doesn't depend on valid controller. So
scoped wait on stopping controller can be removed.
* Move sync thread to PSS. It's created when starting first backend and
destroyed on last browser thread.
BUG=19757
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210333
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210955
Review URL: https://chromiumcodereview.appspot.com/16770005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@213642 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
42 files changed, 271 insertions, 264 deletions
diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc index a846cfd..1bfac0f 100644 --- a/sync/engine/build_commit_command.cc +++ b/sync/engine/build_commit_command.cc @@ -42,7 +42,7 @@ BuildCommitCommand::BuildCommitCommand( syncable::BaseTransaction* trans, const sessions::OrderedCommitSet& batch_commit_set, sync_pb::ClientToServerMessage* commit_message, - ExtensionsActivityMonitor::Records* extensions_activity_buffer) + ExtensionsActivity::Records* extensions_activity_buffer) : trans_(trans), batch_commit_set_(batch_commit_set), commit_message_(commit_message), @@ -55,7 +55,7 @@ void BuildCommitCommand::AddExtensionsActivityToMessage( SyncSession* session, sync_pb::CommitMessage* message) { // We only send ExtensionsActivity to the server if bookmarks are being // committed. - ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor(); + ExtensionsActivity* activity = session->context()->extensions_activity(); if (batch_commit_set_.HasBookmarkCommitId()) { // This isn't perfect, since the set of extensions activity may not // correlate exactly with the items being committed. That's OK as @@ -66,11 +66,11 @@ void BuildCommitCommand::AddExtensionsActivityToMessage( // We will push this list of extensions activity back into the // ExtensionsActivityMonitor if this commit fails. That's why we must keep // a copy of these records in the session. - monitor->GetAndClearRecords(extensions_activity_buffer_); + activity->GetAndClearRecords(extensions_activity_buffer_); - const ExtensionsActivityMonitor::Records& records = + const ExtensionsActivity::Records& records = *extensions_activity_buffer_; - for (ExtensionsActivityMonitor::Records::const_iterator it = + for (ExtensionsActivity::Records::const_iterator it = records.begin(); it != records.end(); ++it) { sync_pb::ChromiumExtensionsActivity* activity_message = diff --git a/sync/engine/build_commit_command.h b/sync/engine/build_commit_command.h index a55a0b4..405cdff 100644 --- a/sync/engine/build_commit_command.h +++ b/sync/engine/build_commit_command.h @@ -11,7 +11,7 @@ #include "sync/base/sync_export.h" #include "sync/engine/syncer_command.h" #include "sync/syncable/entry_kernel.h" -#include "sync/util/extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" namespace syncer { @@ -42,7 +42,7 @@ class SYNC_EXPORT_PRIVATE BuildCommitCommand : public SyncerCommand { syncable::BaseTransaction* trans, const sessions::OrderedCommitSet& batch_commit_set, sync_pb::ClientToServerMessage* commit_message, - ExtensionsActivityMonitor::Records* extensions_activity_buffer); + ExtensionsActivity::Records* extensions_activity_buffer); virtual ~BuildCommitCommand(); // SyncerCommand implementation. @@ -69,7 +69,7 @@ class SYNC_EXPORT_PRIVATE BuildCommitCommand : public SyncerCommand { // Output parameter; see constructor comment. sync_pb::ClientToServerMessage* commit_message_; - ExtensionsActivityMonitor::Records* extensions_activity_buffer_; + ExtensionsActivity::Records* extensions_activity_buffer_; }; } // namespace syncer diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc index 5081ab3..7596d4b 100644 --- a/sync/engine/commit.cc +++ b/sync/engine/commit.cc @@ -66,7 +66,7 @@ bool PrepareCommitMessage( ModelTypeSet requested_types, sessions::OrderedCommitSet* commit_set, sync_pb::ClientToServerMessage* commit_message, - ExtensionsActivityMonitor::Records* extensions_activity_buffer) { + ExtensionsActivity::Records* extensions_activity_buffer) { TRACE_EVENT0("sync", "PrepareCommitMessage"); commit_set->Clear(); @@ -104,7 +104,7 @@ SyncerError BuildAndPostCommitsImpl(ModelTypeSet requested_types, sessions::OrderedCommitSet* commit_set) { while (!syncer->ExitRequested()) { sync_pb::ClientToServerMessage commit_message; - ExtensionsActivityMonitor::Records extensions_activity_buffer; + ExtensionsActivity::Records extensions_activity_buffer; if (!PrepareCommitMessage(session, requested_types, @@ -154,9 +154,9 @@ SyncerError BuildAndPostCommitsImpl(ModelTypeSet requested_types, // If the commit failed, return the data to the ExtensionsActivityMonitor. if (session->status_controller(). model_neutral_state().num_successful_bookmark_commits == 0) { - ExtensionsActivityMonitor* extensions_activity_monitor = - session->context()->extensions_monitor(); - extensions_activity_monitor->PutRecords(extensions_activity_buffer); + ExtensionsActivity* extensions_activity = + session->context()->extensions_activity(); + extensions_activity->PutRecords(extensions_activity_buffer); } if (processing_result != SYNCER_OK) { diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h index 8eb361e..1dd1018 100644 --- a/sync/engine/sync_scheduler.h +++ b/sync/engine/sync_scheduler.h @@ -79,9 +79,7 @@ class SYNC_EXPORT_PRIVATE SyncScheduler // cancel all scheduled tasks. This function can be called from any thread, // and should in fact be called from a thread that isn't the sync loop to // allow preempting ongoing sync cycles. - // Invokes |callback| from the sync loop once syncer is idle and all tasks - // are cancelled. - virtual void RequestStop(const base::Closure& callback) = 0; + virtual void RequestStop() = 0; // The meat and potatoes. All three of the following methods will post a // delayed task to attempt the actual nudge (see ScheduleNudgeImpl). diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 3a904d1..52e1d49 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -174,7 +174,7 @@ SyncSchedulerImpl::SyncSchedulerImpl(const std::string& name, SyncSchedulerImpl::~SyncSchedulerImpl() { DCHECK(CalledOnValidThread()); - StopImpl(base::Closure()); + StopImpl(); } void SyncSchedulerImpl::OnCredentialsUpdated() { @@ -656,16 +656,15 @@ void SyncSchedulerImpl::RestartWaiting() { } } -void SyncSchedulerImpl::RequestStop(const base::Closure& callback) { +void SyncSchedulerImpl::RequestStop() { syncer_->RequestEarlyExit(); // Safe to call from any thread. DCHECK(weak_handle_this_.IsInitialized()); SDVLOG(3) << "Posting StopImpl"; weak_handle_this_.Call(FROM_HERE, - &SyncSchedulerImpl::StopImpl, - callback); + &SyncSchedulerImpl::StopImpl); } -void SyncSchedulerImpl::StopImpl(const base::Closure& callback) { +void SyncSchedulerImpl::StopImpl() { DCHECK(CalledOnValidThread()); SDVLOG(2) << "StopImpl called"; @@ -678,8 +677,6 @@ void SyncSchedulerImpl::StopImpl(const base::Closure& callback) { pending_configure_params_.reset(); if (started_) started_ = false; - if (!callback.is_null()) - callback.Run(); } // This is the only place where we invoke DoSyncSessionJob with canary diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 59468e7..f0245a9 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -55,7 +55,7 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl virtual void Start(Mode mode) OVERRIDE; virtual bool ScheduleConfiguration( const ConfigurationParams& params) OVERRIDE; - virtual void RequestStop(const base::Closure& callback) OVERRIDE; + virtual void RequestStop() OVERRIDE; virtual void ScheduleLocalNudge( const base::TimeDelta& desired_delay, ModelTypeSet types, @@ -183,7 +183,7 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl bool CanRunNudgeJobNow(JobPriority priority); // 'Impl' here refers to real implementation of public functions. - void StopImpl(const base::Closure& callback); + void StopImpl(); // If the scheduler's current state supports it, this will create a job based // on the passed in parameters and coalesce it with any other pending jobs, diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index 3fb2042..a09079a 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -17,7 +17,7 @@ #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/mock_connection_manager.h" #include "sync/test/engine/test_directory_setter_upper.h" -#include "sync/test/fake_extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -126,6 +126,7 @@ class SyncSchedulerTest : public testing::Test { dir_maker_.SetUp(); syncer_ = new MockSyncer(); delay_ = NULL; + extensions_activity_ = new ExtensionsActivity(); routing_info_[BOOKMARKS] = GROUP_UI; routing_info_[AUTOFILL] = GROUP_DB; @@ -147,7 +148,7 @@ class SyncSchedulerTest : public testing::Test { connection_->SetServerReachable(); context_.reset(new SyncSessionContext( connection_.get(), directory(), workers, - &extensions_activity_monitor_, + extensions_activity_.get(), std::vector<SyncEngineEventListener*>(), NULL, NULL, true, // enable keystore encryption false, // force enable pre-commit GU avoidance @@ -202,8 +203,10 @@ class SyncSchedulerTest : public testing::Test { // This stops the scheduler synchronously. void StopSyncScheduler() { - scheduler()->RequestStop(base::Bind(&SyncSchedulerTest::DoQuitLoopNow, - weak_ptr_factory_.GetWeakPtr())); + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&SyncSchedulerTest::DoQuitLoopNow, + weak_ptr_factory_.GetWeakPtr())); RunLoop(); } @@ -233,8 +236,8 @@ class SyncSchedulerTest : public testing::Test { return dir_maker_.directory(); } + base::MessageLoop loop_; base::WeakPtrFactory<SyncSchedulerTest> weak_ptr_factory_; - base::MessageLoop message_loop_; TestDirectorySetterUpper dir_maker_; scoped_ptr<MockConnectionManager> connection_; scoped_ptr<SyncSessionContext> context_; @@ -242,7 +245,7 @@ class SyncSchedulerTest : public testing::Test { MockSyncer* syncer_; MockDelayProvider* delay_; std::vector<scoped_refptr<FakeModelWorker> > workers_; - FakeExtensionsActivityMonitor extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; ModelSafeRoutingInfo routing_info_; }; diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h index f2e3c73..eae6d87 100644 --- a/sync/engine/syncer.h +++ b/sync/engine/syncer.h @@ -17,7 +17,7 @@ #include "sync/engine/syncer_types.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/sessions/sync_session.h" -#include "sync/util/extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" namespace syncer { diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index a2ccccf..db7d1a1 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -49,9 +49,9 @@ #include "sync/test/engine/test_id_factory.h" #include "sync/test/engine/test_syncable_utils.h" #include "sync/test/fake_encryptor.h" -#include "sync/test/fake_extensions_activity_monitor.h" #include "sync/test/fake_sync_encryption_handler.h" #include "sync/util/cryptographer.h" +#include "sync/util/extensions_activity.h" #include "sync/util/time.h" #include "testing/gtest/include/gtest/gtest.h" @@ -113,7 +113,8 @@ class SyncerTest : public testing::Test, public SyncEngineEventListener { protected: SyncerTest() - : syncer_(NULL), + : extensions_activity_(new ExtensionsActivity), + syncer_(NULL), saw_syncer_event_(false), last_client_invalidation_hint_buffer_size_(10), traffic_recorder_(0, 0) { @@ -236,7 +237,7 @@ class SyncerTest : public testing::Test, context_.reset( new SyncSessionContext( mock_server_.get(), directory(), workers, - &extensions_activity_monitor_, + extensions_activity_, listeners, NULL, &traffic_recorder_, true, // enable keystore encryption false, // force enable pre-commit GU avoidance experiment @@ -563,7 +564,7 @@ class SyncerTest : public testing::Test, TestDirectorySetterUpper dir_maker_; FakeEncryptor encryptor_; - FakeExtensionsActivityMonitor extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; scoped_ptr<MockConnectionManager> mock_server_; Syncer* syncer_; @@ -4800,18 +4801,18 @@ TEST_P(MixedResult, ExtensionsActivity) { // Put some extenions activity records into the monitor. { - ExtensionsActivityMonitor::Records records; + ExtensionsActivity::Records records; records["ABC"].extension_id = "ABC"; records["ABC"].bookmark_write_count = 2049U; records["xyz"].extension_id = "xyz"; records["xyz"].bookmark_write_count = 4U; - context_->extensions_monitor()->PutRecords(records); + context_->extensions_activity()->PutRecords(records); } SyncShareNudge(); - ExtensionsActivityMonitor::Records final_monitor_records; - context_->extensions_monitor()->GetAndClearRecords(&final_monitor_records); + ExtensionsActivity::Records final_monitor_records; + context_->extensions_activity()->GetAndClearRecords(&final_monitor_records); if (ShouldFailBookmarkCommit()) { ASSERT_EQ(2U, final_monitor_records.size()) << "Should restore records after unsuccessful bookmark commit."; diff --git a/sync/internal_api/internal_components_factory_impl.cc b/sync/internal_api/internal_components_factory_impl.cc index 1cc3b11..d5fc102 100644 --- a/sync/internal_api/internal_components_factory_impl.cc +++ b/sync/internal_api/internal_components_factory_impl.cc @@ -37,14 +37,14 @@ InternalComponentsFactoryImpl::BuildContext( ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* monitor, + ExtensionsActivity* extensions_activity, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, const std::string& invalidation_client_id) { return scoped_ptr<sessions::SyncSessionContext>( new sessions::SyncSessionContext( - connection_manager, directory, workers, monitor, + connection_manager, directory, workers, extensions_activity, listeners, debug_info_getter, traffic_recorder, switches_.encryption_method == ENCRYPTION_KEYSTORE, diff --git a/sync/internal_api/public/engine/model_safe_worker.cc b/sync/internal_api/public/engine/model_safe_worker.cc index 44e0a24..7179ed5 100644 --- a/sync/internal_api/public/engine/model_safe_worker.cc +++ b/sync/internal_api/public/engine/model_safe_worker.cc @@ -4,6 +4,7 @@ #include "sync/internal_api/public/engine/model_safe_worker.h" +#include "base/bind.h" #include "base/json/json_writer.h" #include "base/memory/scoped_ptr.h" #include "base/values.h" @@ -84,7 +85,9 @@ std::string ModelSafeGroupToString(ModelSafeGroup group) { ModelSafeWorker::ModelSafeWorker(WorkerLoopDestructionObserver* observer) : stopped_(false), work_done_or_stopped_(false, false), - observer_(observer) {} + observer_(observer), + working_loop_(NULL), + working_loop_set_wait_(true, false) {} ModelSafeWorker::~ModelSafeWorker() {} @@ -135,4 +138,33 @@ void ModelSafeWorker::WillDestroyCurrentMessageLoop() { observer_->OnWorkerLoopDestroyed(GetModelSafeGroup()); } +void ModelSafeWorker::SetWorkingLoopToCurrent() { + DCHECK(!working_loop_); + working_loop_ = base::MessageLoop::current(); + working_loop_set_wait_.Signal(); +} + +void ModelSafeWorker::UnregisterForLoopDestruction( + base::Callback<void(ModelSafeGroup)> unregister_done_callback) { + // Ok to wait until |working_loop_| is set because this is called on sync + // loop. + working_loop_set_wait_.Wait(); + + // Should be called on sync loop. + DCHECK_NE(base::MessageLoop::current(), working_loop_); + DCHECK(working_loop_); + working_loop_->PostTask( + FROM_HERE, + base::Bind(&ModelSafeWorker::UnregisterForLoopDestructionAsync, + this, unregister_done_callback)); +} + +void ModelSafeWorker::UnregisterForLoopDestructionAsync( + base::Callback<void(ModelSafeGroup)> unregister_done_callback) { + DCHECK(stopped_); + DCHECK_EQ(base::MessageLoop::current(), working_loop_); + base::MessageLoop::current()->RemoveDestructionObserver(this); + unregister_done_callback.Run(GetModelSafeGroup()); +} + } // namespace syncer diff --git a/sync/internal_api/public/engine/model_safe_worker.h b/sync/internal_api/public/engine/model_safe_worker.h index 680c493..f6b7ea6 100644 --- a/sync/internal_api/public/engine/model_safe_worker.h +++ b/sync/internal_api/public/engine/model_safe_worker.h @@ -69,9 +69,16 @@ class SYNC_EXPORT ModelSafeWorker public base::MessageLoop::DestructionObserver { public: // Subclass should implement to observe destruction of the loop where - // it actually does work. + // it actually does work. Called on UI thread immediately after worker is + // created. virtual void RegisterForLoopDestruction() = 0; + // Called on sync loop from SyncBackendRegistrar::ShutDown(). Post task to + // working loop to stop observing loop destruction and invoke + // |unregister_done_callback|. + virtual void UnregisterForLoopDestruction( + base::Callback<void(ModelSafeGroup)> unregister_done_callback); + // If not stopped, call DoWorkAndWaitUntilDoneImpl() to do work. Otherwise // return CANNOT_DO_WORK. SyncerError DoWorkAndWaitUntilDone(const WorkCallback& work); @@ -103,7 +110,14 @@ class SYNC_EXPORT ModelSafeWorker // Return true if the worker was stopped. Thread safe. bool IsStopped(); + // Subclass should call this in RegisterForLoopDestruction() from the loop + // where work is done. + void SetWorkingLoopToCurrent(); + private: + void UnregisterForLoopDestructionAsync( + base::Callback<void(ModelSafeGroup)> unregister_done_callback); + // Whether the worker should/can do more work. Set when sync is disabled or // when the worker's working thread is to be destroyed. base::Lock stopped_lock_; @@ -115,6 +129,11 @@ class SYNC_EXPORT ModelSafeWorker // Notified when working thread of the worker is to be destroyed. WorkerLoopDestructionObserver* observer_; + + // Remember working loop for posting task to unregister destruction + // observation from sync thread when shutting down sync. + base::MessageLoop* working_loop_; + base::WaitableEvent working_loop_set_wait_; }; // A map that details which ModelSafeGroup each ModelType diff --git a/sync/internal_api/public/engine/passive_model_worker.cc b/sync/internal_api/public/engine/passive_model_worker.cc index 9bd5a35..02036d5 100644 --- a/sync/internal_api/public/engine/passive_model_worker.cc +++ b/sync/internal_api/public/engine/passive_model_worker.cc @@ -18,7 +18,8 @@ PassiveModelWorker::~PassiveModelWorker() { } void PassiveModelWorker::RegisterForLoopDestruction() { - NOTREACHED(); + base::MessageLoop::current()->AddDestructionObserver(this); + SetWorkingLoopToCurrent(); } SyncerError PassiveModelWorker::DoWorkAndWaitUntilDoneImpl( diff --git a/sync/internal_api/public/engine/passive_model_worker.h b/sync/internal_api/public/engine/passive_model_worker.h index f834c83..783731b 100644 --- a/sync/internal_api/public/engine/passive_model_worker.h +++ b/sync/internal_api/public/engine/passive_model_worker.h @@ -11,10 +11,6 @@ #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/internal_api/public/util/syncer_error.h" -namespace base { -class MessageLoop; -} - namespace syncer { // Implementation of ModelSafeWorker for passive types. All work is diff --git a/sync/internal_api/public/internal_components_factory.h b/sync/internal_api/public/internal_components_factory.h index 96fd640..86509ec 100644 --- a/sync/internal_api/public/internal_components_factory.h +++ b/sync/internal_api/public/internal_components_factory.h @@ -18,7 +18,7 @@ namespace syncer { -class ExtensionsActivityMonitor; +class ExtensionsActivity; class ServerConnectionManager; class SyncEngineEventListener; class SyncScheduler; @@ -81,7 +81,7 @@ class SYNC_EXPORT InternalComponentsFactory { ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* monitor, + ExtensionsActivity* extensions_activity, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/public/internal_components_factory_impl.h b/sync/internal_api/public/internal_components_factory_impl.h index 6fec27f..148dd07 100644 --- a/sync/internal_api/public/internal_components_factory_impl.h +++ b/sync/internal_api/public/internal_components_factory_impl.h @@ -27,7 +27,7 @@ class SYNC_EXPORT InternalComponentsFactoryImpl ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* monitor, + ExtensionsActivity* extensions_activity, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h index bc6a137..3f5316b 100644 --- a/sync/internal_api/public/sync_manager.h +++ b/sync/internal_api/public/sync_manager.h @@ -22,6 +22,7 @@ #include "sync/internal_api/public/engine/sync_status.h" #include "sync/internal_api/public/sync_encryption_handler.h" #include "sync/internal_api/public/util/report_unrecoverable_error_function.h" +#include "sync/internal_api/public/util/unrecoverable_error_handler.h" #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/invalidation_handler.h" #include "sync/protocol/sync_protocol_error.h" @@ -36,14 +37,13 @@ class BaseTransaction; class DataTypeDebugInfoListener; class Encryptor; struct Experiments; -class ExtensionsActivityMonitor; +class ExtensionsActivity; class HttpPostProviderFactory; class InternalComponentsFactory; class JsBackend; class JsEventHandler; class SyncEncryptionHandler; class SyncScheduler; -class UnrecoverableErrorHandler; struct UserShare; namespace sessions { @@ -310,15 +310,15 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler { bool use_ssl, scoped_ptr<HttpPostProviderFactory> post_factory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, ChangeDelegate* change_delegate, const SyncCredentials& credentials, const std::string& invalidator_client_id, const std::string& restored_key_for_bootstrapping, const std::string& restored_keystore_key_for_bootstrapping, - scoped_ptr<InternalComponentsFactory> internal_components_factory, + InternalComponentsFactory* internal_components_factory, Encryptor* encryptor, - UnrecoverableErrorHandler* unrecoverable_error_handler, + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function, bool use_oauth2_token) = 0; @@ -401,7 +401,7 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler { // If no scheduler exists, the callback is run immediately (from the loop // this was created on, which is the sync loop), as sync is effectively // stopped. - virtual void StopSyncingForShutdown(const base::Closure& callback) = 0; + virtual void StopSyncingForShutdown() = 0; // Issue a final SaveChanges, and close sqlite handles. virtual void ShutdownOnSyncThread() = 0; diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h index ed69a52..be06ea5 100644 --- a/sync/internal_api/public/test/fake_sync_manager.h +++ b/sync/internal_api/public/test/fake_sync_manager.h @@ -82,17 +82,16 @@ class FakeSyncManager : public SyncManager { bool use_ssl, scoped_ptr<HttpPostProviderFactory> post_factory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, ChangeDelegate* change_delegate, const SyncCredentials& credentials, const std::string& invalidator_client_id, const std::string& restored_key_for_bootstrapping, const std::string& restored_keystore_key_for_bootstrapping, - scoped_ptr<InternalComponentsFactory> internal_components_factory, + InternalComponentsFactory* internal_components_factory, Encryptor* encryptor, - UnrecoverableErrorHandler* unrecoverable_error_handler, - ReportUnrecoverableErrorFunction - report_unrecoverable_error_function, + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler, + ReportUnrecoverableErrorFunction report_unrecoverable_error_function, bool use_oauth2_token) OVERRIDE; virtual void ThrowUnrecoverableError() OVERRIDE; virtual ModelTypeSet InitialSyncEndedTypes() OVERRIDE; @@ -115,7 +114,7 @@ class FakeSyncManager : public SyncManager { virtual void RemoveObserver(Observer* observer) OVERRIDE; virtual SyncStatus GetDetailedStatus() const OVERRIDE; virtual void SaveChanges() OVERRIDE; - virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE; + virtual void StopSyncingForShutdown() OVERRIDE; virtual void ShutdownOnSyncThread() OVERRIDE; virtual UserShare* GetUserShare() OVERRIDE; virtual const std::string cache_guid() OVERRIDE; diff --git a/sync/internal_api/public/test/test_internal_components_factory.h b/sync/internal_api/public/test/test_internal_components_factory.h index b608154..d846094 100644 --- a/sync/internal_api/public/test/test_internal_components_factory.h +++ b/sync/internal_api/public/test/test_internal_components_factory.h @@ -33,7 +33,7 @@ class TestInternalComponentsFactory : public InternalComponentsFactory { ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* monitor, + ExtensionsActivity* monitor, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/public/util/unrecoverable_error_handler.h b/sync/internal_api/public/util/unrecoverable_error_handler.h index db2f2c0..2bd2475 100644 --- a/sync/internal_api/public/util/unrecoverable_error_handler.h +++ b/sync/internal_api/public/util/unrecoverable_error_handler.h @@ -19,8 +19,7 @@ class UnrecoverableErrorHandler { // further, and will report an error status if queried. virtual void OnUnrecoverableError(const tracked_objects::Location& from_here, const std::string& message) = 0; - protected: - virtual ~UnrecoverableErrorHandler() { } + virtual ~UnrecoverableErrorHandler() {} }; } // namespace syncer diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 3d6158d..a541032 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -175,7 +175,6 @@ SyncManagerImpl::SyncManagerImpl(const std::string& name) invalidator_state_(DEFAULT_INVALIDATION_ERROR), traffic_recorder_(kMaxMessagesToRecord, kMaxMessageSizeToRecord), encryptor_(NULL), - unrecoverable_error_handler_(NULL), report_unrecoverable_error_function_(NULL) { // Pre-fill |notification_info_map_|. for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { @@ -346,15 +345,15 @@ void SyncManagerImpl::Init( bool use_ssl, scoped_ptr<HttpPostProviderFactory> post_factory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, SyncManager::ChangeDelegate* change_delegate, const SyncCredentials& credentials, const std::string& invalidator_client_id, const std::string& restored_key_for_bootstrapping, const std::string& restored_keystore_key_for_bootstrapping, - scoped_ptr<InternalComponentsFactory> internal_components_factory, + InternalComponentsFactory* internal_components_factory, Encryptor* encryptor, - UnrecoverableErrorHandler* unrecoverable_error_handler, + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function, bool use_oauth2_token) { CHECK(!initialized_); @@ -376,7 +375,7 @@ void SyncManagerImpl::Init( database_path_ = database_location.Append( syncable::Directory::kSyncDatabaseFilename); encryptor_ = encryptor; - unrecoverable_error_handler_ = unrecoverable_error_handler; + unrecoverable_error_handler_ = unrecoverable_error_handler.Pass(); report_unrecoverable_error_function_ = report_unrecoverable_error_function; allstatus_.SetHasKeystoreKey( @@ -402,7 +401,7 @@ void SyncManagerImpl::Init( share_.directory.reset( new syncable::Directory( backing_store.release(), - unrecoverable_error_handler_, + unrecoverable_error_handler_.get(), report_unrecoverable_error_function_, sync_encryption_handler_.get(), sync_encryption_handler_->GetCryptographerUnsafe())); @@ -442,7 +441,7 @@ void SyncManagerImpl::Init( connection_manager_.get(), directory(), workers, - extensions_activity_monitor, + extensions_activity, listeners, &debug_info_event_listener_, &traffic_recorder_, @@ -620,9 +619,9 @@ void SyncManagerImpl::RemoveObserver(SyncManager::Observer* observer) { observers_.RemoveObserver(observer); } -void SyncManagerImpl::StopSyncingForShutdown(const base::Closure& callback) { +void SyncManagerImpl::StopSyncingForShutdown() { DVLOG(2) << "StopSyncingForShutdown"; - scheduler_->RequestStop(callback); + scheduler_->RequestStop(); if (connection_manager_) connection_manager_->TerminateAllIO(); } diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index 153df81..712c268 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -70,15 +70,15 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : bool use_ssl, scoped_ptr<HttpPostProviderFactory> post_factory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, SyncManager::ChangeDelegate* change_delegate, const SyncCredentials& credentials, const std::string& invalidator_client_id, const std::string& restored_key_for_bootstrapping, const std::string& restored_keystore_key_for_bootstrapping, - scoped_ptr<InternalComponentsFactory> internal_components_factory, + InternalComponentsFactory* internal_components_factory, Encryptor* encryptor, - UnrecoverableErrorHandler* unrecoverable_error_handler, + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function, bool use_oauth2_token) OVERRIDE; @@ -106,7 +106,7 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : virtual void RemoveObserver(SyncManager::Observer* observer) OVERRIDE; virtual SyncStatus GetDetailedStatus() const OVERRIDE; virtual void SaveChanges() OVERRIDE; - virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE; + virtual void StopSyncingForShutdown() OVERRIDE; virtual void ShutdownOnSyncThread() OVERRIDE; virtual UserShare* GetUserShare() OVERRIDE; virtual const std::string cache_guid() OVERRIDE; @@ -360,7 +360,7 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : TrafficRecorder traffic_recorder_; Encryptor* encryptor_; - UnrecoverableErrorHandler* unrecoverable_error_handler_; + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler_; ReportUnrecoverableErrorFunction report_unrecoverable_error_function_; // Sync's encryption handler. It tracks the set of encrypted types, manages diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc index 51144b6..06bc783 100644 --- a/sync/internal_api/sync_manager_impl_unittest.cc +++ b/sync/internal_api/sync_manager_impl_unittest.cc @@ -68,9 +68,8 @@ #include "sync/test/engine/fake_sync_scheduler.h" #include "sync/test/engine/test_id_factory.h" #include "sync/test/fake_encryptor.h" -#include "sync/test/fake_extensions_activity_monitor.h" #include "sync/util/cryptographer.h" -#include "sync/util/extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" #include "sync/util/test_unrecoverable_error_handler.h" #include "sync/util/time.h" #include "testing/gmock/include/gmock/gmock.h" @@ -800,6 +799,8 @@ class SyncManagerTest : public testing::Test, void SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + extensions_activity_ = new ExtensionsActivity(); + SyncCredentials credentials; credentials.email = "foo@bar.com"; credentials.sync_token = "sometoken"; @@ -823,15 +824,16 @@ class SyncManagerTest : public testing::Test, false, scoped_ptr<HttpPostProviderFactory>(new TestHttpPostProviderFactory()), workers, - &extensions_activity_monitor_, + extensions_activity_.get(), this, credentials, "fake_invalidator_client_id", std::string(), std::string(), // bootstrap tokens - scoped_ptr<InternalComponentsFactory>(GetFactory()), + scoped_ptr<InternalComponentsFactory>(GetFactory()).get(), &encryptor_, - &handler_, + scoped_ptr<UnrecoverableErrorHandler>( + new TestUnrecoverableErrorHandler).Pass(), NULL, false); @@ -1010,11 +1012,10 @@ class SyncManagerTest : public testing::Test, base::ScopedTempDir temp_dir_; // Sync Id's for the roots of the enabled datatypes. std::map<ModelType, int64> type_roots_; - FakeExtensionsActivityMonitor extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; protected: FakeEncryptor encryptor_; - TestUnrecoverableErrorHandler handler_; SyncManagerImpl sync_manager_; WeakHandle<JsBackend> js_backend_; StrictMock<SyncManagerObserverMock> manager_observer_; diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc index 7a6d6a4..cac310b 100644 --- a/sync/internal_api/test/fake_sync_manager.cc +++ b/sync/internal_api/test/fake_sync_manager.cc @@ -81,17 +81,16 @@ void FakeSyncManager::Init( bool use_ssl, scoped_ptr<HttpPostProviderFactory> post_factory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, ChangeDelegate* change_delegate, const SyncCredentials& credentials, const std::string& invalidator_client_id, const std::string& restored_key_for_bootstrapping, const std::string& restored_keystore_key_for_bootstrapping, - scoped_ptr<InternalComponentsFactory> internal_components_factory, + InternalComponentsFactory* internal_components_factory, Encryptor* encryptor, - UnrecoverableErrorHandler* unrecoverable_error_handler, - ReportUnrecoverableErrorFunction - report_unrecoverable_error_function, + scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler, + ReportUnrecoverableErrorFunction report_unrecoverable_error_function, bool use_oauth2_token) { sync_task_runner_ = base::ThreadTaskRunnerHandle::Get(); PurgePartiallySyncedTypes(); @@ -210,10 +209,7 @@ void FakeSyncManager::SaveChanges() { // Do nothing. } -void FakeSyncManager::StopSyncingForShutdown(const base::Closure& callback) { - if (!sync_task_runner_->PostTask(FROM_HERE, callback)) { - NOTREACHED(); - } +void FakeSyncManager::StopSyncingForShutdown() { } void FakeSyncManager::ShutdownOnSyncThread() { diff --git a/sync/internal_api/test/test_internal_components_factory.cc b/sync/internal_api/test/test_internal_components_factory.cc index 154c58cc..03d2d82 100644 --- a/sync/internal_api/test/test_internal_components_factory.cc +++ b/sync/internal_api/test/test_internal_components_factory.cc @@ -31,7 +31,7 @@ TestInternalComponentsFactory::BuildContext( ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* monitor, + ExtensionsActivity* monitor, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/notifier/non_blocking_invalidator.cc b/sync/notifier/non_blocking_invalidator.cc index 2834f28..d4c602b 100644 --- a/sync/notifier/non_blocking_invalidator.cc +++ b/sync/notifier/non_blocking_invalidator.cc @@ -169,7 +169,7 @@ NonBlockingInvalidator::~NonBlockingInvalidator() { FROM_HERE, base::Bind(&NonBlockingInvalidator::Core::Teardown, core_.get()))) { - NOTREACHED(); + DVLOG(1) << "Network thread stopped before invalidator is destroyed."; } } diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc index c48c32b..98ab5f0 100644 --- a/sync/sessions/sync_session_context.cc +++ b/sync/sessions/sync_session_context.cc @@ -5,7 +5,7 @@ #include "sync/sessions/sync_session_context.h" #include "sync/sessions/debug_info_getter.h" -#include "sync/util/extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" namespace syncer { namespace sessions { @@ -17,7 +17,7 @@ SyncSessionContext::SyncSessionContext( ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -26,7 +26,7 @@ SyncSessionContext::SyncSessionContext( const std::string& invalidator_client_id) : connection_manager_(connection_manager), directory_(directory), - extensions_activity_monitor_(extensions_activity_monitor), + extensions_activity_(extensions_activity), notifications_enabled_(false), max_commit_batch_size_(kDefaultMaxCommitBatchSize), debug_info_getter_(debug_info_getter), diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h index 7617095..718cc6c 100644 --- a/sync/sessions/sync_session_context.h +++ b/sync/sessions/sync_session_context.h @@ -32,7 +32,7 @@ namespace syncer { -class ExtensionsActivityMonitor; +class ExtensionsActivity; class ServerConnectionManager; namespace syncable { @@ -50,7 +50,7 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { SyncSessionContext(ServerConnectionManager* connection_manager, syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, - ExtensionsActivityMonitor* extensions_activity_monitor, + ExtensionsActivity* extensions_activity, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -79,8 +79,8 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { return workers_; } - ExtensionsActivityMonitor* extensions_monitor() { - return extensions_activity_monitor_; + ExtensionsActivity* extensions_activity() { + return extensions_activity_.get(); } DebugInfoGetter* debug_info_getter() { @@ -159,7 +159,7 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { // We use this to stuff extensions activity into CommitMessages so the server // can correlate commit traffic with extension-related bookmark mutations. - ExtensionsActivityMonitor* extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; // Kept up to date with talk events to determine whether notifications are // enabled. True only if the notification channel is authorized and open. diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index 4acee56..979ddbd 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -16,7 +16,7 @@ #include "sync/syncable/syncable_write_transaction.h" #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/test_directory_setter_upper.h" -#include "sync/test/fake_extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { @@ -38,6 +38,8 @@ class SyncSessionTest : public testing::Test, } virtual void SetUp() { + extensions_activity_ = new ExtensionsActivity(); + routes_.clear(); routes_[BOOKMARKS] = GROUP_UI; routes_[AUTOFILL] = GROUP_DB; @@ -60,7 +62,7 @@ class SyncSessionTest : public testing::Test, NULL, NULL, workers, - &extensions_activity_monitor_, + extensions_activity_.get(), std::vector<SyncEngineEventListener*>(), NULL, NULL, @@ -146,7 +148,7 @@ class SyncSessionTest : public testing::Test, scoped_ptr<SyncSessionContext> context_; std::vector<scoped_refptr<ModelSafeWorker> > workers_; ModelSafeRoutingInfo routes_; - FakeExtensionsActivityMonitor extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; }; TEST_F(SyncSessionTest, MoreToDownloadIfDownloadFailed) { diff --git a/sync/sync_core.gypi b/sync/sync_core.gypi index 542040c..2b97fe7 100644 --- a/sync/sync_core.gypi +++ b/sync/sync_core.gypi @@ -169,8 +169,8 @@ 'util/cryptographer.h', 'util/data_type_histogram.h', 'util/encryptor.h', - 'util/extensions_activity_monitor.cc', - 'util/extensions_activity_monitor.h', + 'util/extensions_activity.cc', + 'util/extensions_activity.h', 'util/get_session_name.cc', 'util/get_session_name.h', 'util/get_session_name_ios.mm', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 32867f1..847d862 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -49,8 +49,6 @@ 'test/fake_encryptor.h', 'test/fake_sync_encryption_handler.h', 'test/fake_sync_encryption_handler.cc', - 'test/fake_extensions_activity_monitor.cc', - 'test/fake_extensions_activity_monitor.h', 'test/test_transaction_observer.cc', 'test/test_transaction_observer.h', 'test/null_directory_change_delegate.cc', diff --git a/sync/test/engine/fake_sync_scheduler.cc b/sync/test/engine/fake_sync_scheduler.cc index 200edb0..585248e 100644 --- a/sync/test/engine/fake_sync_scheduler.cc +++ b/sync/test/engine/fake_sync_scheduler.cc @@ -6,16 +6,14 @@ namespace syncer { -FakeSyncScheduler::FakeSyncScheduler() - : created_on_loop_(base::MessageLoop::current()) {} +FakeSyncScheduler::FakeSyncScheduler() {} FakeSyncScheduler::~FakeSyncScheduler() {} void FakeSyncScheduler::Start(Mode mode) { } -void FakeSyncScheduler::RequestStop(const base::Closure& callback) { - created_on_loop_->PostTask(FROM_HERE, callback); +void FakeSyncScheduler::RequestStop() { } void FakeSyncScheduler::ScheduleLocalNudge( diff --git a/sync/test/engine/fake_sync_scheduler.h b/sync/test/engine/fake_sync_scheduler.h index 97deb91..29757e2 100644 --- a/sync/test/engine/fake_sync_scheduler.h +++ b/sync/test/engine/fake_sync_scheduler.h @@ -20,7 +20,7 @@ class FakeSyncScheduler : public SyncScheduler { virtual ~FakeSyncScheduler(); virtual void Start(Mode mode) OVERRIDE; - virtual void RequestStop(const base::Closure& callback) OVERRIDE; + virtual void RequestStop() OVERRIDE; virtual void ScheduleLocalNudge( const base::TimeDelta& desired_delay, ModelTypeSet types, @@ -58,9 +58,6 @@ class FakeSyncScheduler : public SyncScheduler { virtual void OnShouldStopSyncingPermanently() OVERRIDE; virtual void OnSyncProtocolError( const sessions::SyncSessionSnapshot& snapshot) OVERRIDE; - - private: - base::MessageLoop* const created_on_loop_; }; } // namespace syncer diff --git a/sync/test/engine/syncer_command_test.cc b/sync/test/engine/syncer_command_test.cc index 833ed9a..68edbbd 100644 --- a/sync/test/engine/syncer_command_test.cc +++ b/sync/test/engine/syncer_command_test.cc @@ -17,6 +17,8 @@ SyncerCommandTestBase::~SyncerCommandTestBase() { } void SyncerCommandTestBase::SetUp() { + extensions_activity_ = new ExtensionsActivity(); + // The session always expects there to be a passive worker. workers()->push_back( make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE))); diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index 93106b9..89c2aba 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -22,7 +22,7 @@ #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/mock_connection_manager.h" #include "sync/test/engine/test_directory_setter_upper.h" -#include "sync/test/fake_extensions_activity_monitor.h" +#include "sync/util/extensions_activity.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -131,7 +131,7 @@ class SyncerCommandTestBase : public testing::Test, void ResetContext() { context_.reset(new sessions::SyncSessionContext( mock_server_.get(), directory(), - GetWorkers(), &extensions_activity_monitor_, + GetWorkers(), extensions_activity_.get(), std::vector<SyncEngineEventListener*>(), &mock_debug_info_getter_, &traffic_recorder_, @@ -210,7 +210,7 @@ class SyncerCommandTestBase : public testing::Test, std::vector<scoped_refptr<ModelSafeWorker> > workers_; ModelSafeRoutingInfo routing_info_; NiceMock<MockDebugInfoGetter> mock_debug_info_getter_; - FakeExtensionsActivityMonitor extensions_activity_monitor_; + scoped_refptr<ExtensionsActivity> extensions_activity_; TrafficRecorder traffic_recorder_; DISALLOW_COPY_AND_ASSIGN(SyncerCommandTestBase); }; diff --git a/sync/test/fake_extensions_activity_monitor.cc b/sync/test/fake_extensions_activity_monitor.cc deleted file mode 100644 index 9fb20a6..0000000 --- a/sync/test/fake_extensions_activity_monitor.cc +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "sync/test/fake_extensions_activity_monitor.h" - -#include "base/logging.h" - -namespace syncer { - -FakeExtensionsActivityMonitor::FakeExtensionsActivityMonitor() {} - -FakeExtensionsActivityMonitor::~FakeExtensionsActivityMonitor() { - DCHECK(CalledOnValidThread()); -} - -void FakeExtensionsActivityMonitor::GetAndClearRecords(Records* buffer) { - DCHECK(CalledOnValidThread()); - buffer->clear(); - buffer->swap(records_); -} - -void FakeExtensionsActivityMonitor::PutRecords(const Records& records) { - DCHECK(CalledOnValidThread()); - for (Records::const_iterator i = records.begin(); i != records.end(); ++i) { - records_[i->first].extension_id = i->second.extension_id; - records_[i->first].bookmark_write_count += i->second.bookmark_write_count; - } -} - -} // namespace syncer diff --git a/sync/test/fake_extensions_activity_monitor.h b/sync/test/fake_extensions_activity_monitor.h deleted file mode 100644 index a0702e1..0000000 --- a/sync/test/fake_extensions_activity_monitor.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012 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 SYNC_TEST_FAKE_EXTENSIONS_ACTIVITY_MONITOR_H_ -#define SYNC_TEST_FAKE_EXTENSIONS_ACTIVITY_MONITOR_H_ - -#include "base/compiler_specific.h" -#include "base/threading/non_thread_safe.h" -#include "sync/util/extensions_activity_monitor.h" - -namespace syncer { - -// Fake non-thread-safe implementation of ExtensionsActivityMonitor -// suitable to be used in single-threaded sync tests. -class FakeExtensionsActivityMonitor - : public ExtensionsActivityMonitor, - public base::NonThreadSafe { - public: - FakeExtensionsActivityMonitor(); - virtual ~FakeExtensionsActivityMonitor(); - - // ExtensionsActivityMonitor implementation. - virtual void GetAndClearRecords(Records* buffer) OVERRIDE; - virtual void PutRecords(const Records& records) OVERRIDE; - - private: - Records records_; -}; - -} // namespace syncer - -#endif // SYNC_TEST_FAKE_EXTENSIONS_ACTIVITY_MONITOR_H_ diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc index 2db5a04..e382546 100644 --- a/sync/tools/sync_client.cc +++ b/sync/tools/sync_client.cc @@ -341,12 +341,11 @@ int SyncClientMain(int argc, char* argv[]) { base::Bind(&StubNetworkTimeUpdateCallback))); // Used only when committing bookmarks, so it's okay to leave this // as NULL. - ExtensionsActivityMonitor* extensions_activity_monitor = NULL; + ExtensionsActivity* extensions_activity = NULL; LoggingChangeDelegate change_delegate; const char kRestoredKeyForBootstrapping[] = ""; const char kRestoredKeystoreKeyForBootstrapping[] = ""; NullEncryptor null_encryptor; - LoggingUnrecoverableErrorHandler unrecoverable_error_handler; InternalComponentsFactoryImpl::Switches factory_switches = { InternalComponentsFactory::ENCRYPTION_KEYSTORE, InternalComponentsFactory::BACKOFF_NORMAL @@ -360,16 +359,16 @@ int SyncClientMain(int argc, char* argv[]) { kUseSsl, post_factory.Pass(), workers, - extensions_activity_monitor, + extensions_activity, &change_delegate, credentials, invalidator_id, kRestoredKeyForBootstrapping, kRestoredKeystoreKeyForBootstrapping, - scoped_ptr<InternalComponentsFactory>( - new InternalComponentsFactoryImpl(factory_switches)), + new InternalComponentsFactoryImpl(factory_switches), &null_encryptor, - &unrecoverable_error_handler, + scoped_ptr<UnrecoverableErrorHandler>( + new LoggingUnrecoverableErrorHandler).Pass(), &LogUnrecoverableErrorContext, false); // TODO(akalin): Avoid passing in model parameters multiple times by // organizing handling of model types. diff --git a/sync/util/extensions_activity.cc b/sync/util/extensions_activity.cc new file mode 100644 index 0000000..dcccb26 --- /dev/null +++ b/sync/util/extensions_activity.cc @@ -0,0 +1,39 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sync/util/extensions_activity.h" + +namespace syncer { + +ExtensionsActivity::Record::Record() + : bookmark_write_count(0U) {} + +ExtensionsActivity::Record::~Record() {} + +ExtensionsActivity::ExtensionsActivity() {} + +ExtensionsActivity::~ExtensionsActivity() {} + +void ExtensionsActivity::GetAndClearRecords(Records* buffer) { + base::AutoLock lock(records_lock_); + buffer->clear(); + buffer->swap(records_); +} + +void ExtensionsActivity::PutRecords(const Records& records) { + base::AutoLock lock(records_lock_); + for (Records::const_iterator i = records.begin(); i != records.end(); ++i) { + records_[i->first].extension_id = i->second.extension_id; + records_[i->first].bookmark_write_count += i->second.bookmark_write_count; + } +} + +void ExtensionsActivity::UpdateRecord(const std::string& extension_id) { + base::AutoLock lock(records_lock_); + Record& record = records_[extension_id]; + record.extension_id = extension_id; + record.bookmark_write_count++; +} + +} // namespace syncer diff --git a/sync/util/extensions_activity.h b/sync/util/extensions_activity.h new file mode 100644 index 0000000..8178760 --- /dev/null +++ b/sync/util/extensions_activity.h @@ -0,0 +1,64 @@ +// Copyright 2013 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 SYNC_UTIL_EXTENSIONS_ACTIVITY_H_ +#define SYNC_UTIL_EXTENSIONS_ACTIVITY_H_ + +#include <map> +#include <string> + +#include "base/basictypes.h" +#include "base/memory/ref_counted.h" +#include "base/synchronization/lock.h" +#include "sync/base/sync_export.h" + +namespace syncer { + +// A storage to record usage of extensions APIs to send to sync +// servers, with the ability to purge data once sync servers have +// acknowledged it (successful commit response). +class SYNC_EXPORT ExtensionsActivity + : public base::RefCountedThreadSafe<ExtensionsActivity> { + public: + // A data record of activity performed by extension |extension_id|. + struct SYNC_EXPORT Record { + Record(); + ~Record(); + + // The human-readable ID identifying the extension responsible + // for the activity reported in this Record. + std::string extension_id; + + // How many times the extension successfully invoked a write + // operation through the bookmarks API since the last CommitMessage. + uint32 bookmark_write_count; + }; + + typedef std::map<std::string, Record> Records; + + ExtensionsActivity(); + + // Fill |buffer| with all current records and then clear the + // internal records. Called on sync thread to append records to sync commit + // message. + void GetAndClearRecords(Records* buffer); + + // Merge |records| with the current set of records. Called on sync thread to + // put back records if sync commit failed. + void PutRecords(const Records& records); + + // Increment write count of the specified extension. + void UpdateRecord(const std::string& extension_id); + + private: + friend class base::RefCountedThreadSafe<ExtensionsActivity>; + ~ExtensionsActivity(); + + Records records_; + mutable base::Lock records_lock_; +}; + +} // namespace syncer + +#endif // SYNC_UTIL_EXTENSIONS_ACTIVITY_H_ diff --git a/sync/util/extensions_activity_monitor.cc b/sync/util/extensions_activity_monitor.cc deleted file mode 100644 index fabe50b..0000000 --- a/sync/util/extensions_activity_monitor.cc +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "sync/util/extensions_activity_monitor.h" - -namespace syncer { - -ExtensionsActivityMonitor::Record::Record() - : bookmark_write_count(0U) {} - -ExtensionsActivityMonitor::Record::~Record() {} - -ExtensionsActivityMonitor::~ExtensionsActivityMonitor() {} - -} // namespace syncer diff --git a/sync/util/extensions_activity_monitor.h b/sync/util/extensions_activity_monitor.h deleted file mode 100644 index 699f7b5..0000000 --- a/sync/util/extensions_activity_monitor.h +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2012 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 SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_ -#define SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_ - -#include <map> -#include <string> - -#include "base/basictypes.h" -#include "sync/base/sync_export.h" - -namespace syncer { - -// An interface to monitor usage of extensions APIs to send to sync -// servers, with the ability to purge data once sync servers have -// acknowledged it (successful commit response). -// -// All abstract methods are called from the sync thread. -class SYNC_EXPORT ExtensionsActivityMonitor { - public: - // A data record of activity performed by extension |extension_id|. - struct SYNC_EXPORT Record { - Record(); - ~Record(); - - // The human-readable ID identifying the extension responsible - // for the activity reported in this Record. - std::string extension_id; - - // How many times the extension successfully invoked a write - // operation through the bookmarks API since the last CommitMessage. - uint32 bookmark_write_count; - }; - - typedef std::map<std::string, Record> Records; - - // Fill |buffer| with all current records and then clear the - // internal records. - virtual void GetAndClearRecords(Records* buffer) = 0; - - // Merge |records| with the current set of records, adding the - // bookmark write counts for common Records. - virtual void PutRecords(const Records& records) = 0; - - protected: - virtual ~ExtensionsActivityMonitor(); -}; - -} // namespace syncer - -#endif // SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_ |