diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-01 23:06:08 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-01 23:06:08 +0000 |
commit | 5252c891ca0f5e1fad81adcd86d7889ecf0abc9f (patch) | |
tree | 3dfbbf52e2072f978683d740a13488860564a553 /sync | |
parent | 418feeea43d284ae95dc022d386fe06e4fb78bc8 (diff) | |
download | chromium_src-5252c891ca0f5e1fad81adcd86d7889ecf0abc9f.zip chromium_src-5252c891ca0f5e1fad81adcd86d7889ecf0abc9f.tar.gz chromium_src-5252c891ca0f5e1fad81adcd86d7889ecf0abc9f.tar.bz2 |
Remove sync's ModelSafeWorkerRegistrar
The ModelSafeWorkerRegistrar interface was a wrapper around the
SyncBackendRegistrar. When the sync thread wanted to access the
SyncBackendRegistrar's information regarding currently enabled types or
workers, it would use the ModelSafeWorkerRegistrar interface to do it.
This change removes this implicit inter-thread communication. Where
necessary, it modifies the SyncBackendHost, SyncManager, SyncScheduler
and the SyncSessionContext to ensure that these classes have access to a
fresh copy of the SyncBackendRegistrar's data whenever it is required.
The most biggest consequence of this patch is that the
SyncSessionContext now maintains a copy of the list of ModelSafeWorkers
and routing info, rather than a pointer to the ModelSafeWorkerRegistrar.
Various functions along the path to CleanupDisabledTypes, Configure and
StartSyncingNormally have been updated to ensure that the latest routing
info is made available to the session context.
Future patches may refactor this code to reduce the amount of duplicated
state. This work was intentionally left to future patches to reduce the
risk of the current change. The refactoring should be much easier once
we're convinced that the new, explicit inter-thread communication
mechanisms are no more buggy than the existing code.
BUG=103326
TEST=sync_integration_tests
Review URL: https://chromiumcodereview.appspot.com/10388187
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140122 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r-- | sync/engine/model_safe_worker.h | 25 | ||||
-rw-r--r-- | sync/engine/sync_scheduler.cc | 72 | ||||
-rw-r--r-- | sync/engine/sync_scheduler.h | 4 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_unittest.cc | 22 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_whitebox_unittest.cc | 18 | ||||
-rw-r--r-- | sync/engine/syncer_unittest.cc | 31 | ||||
-rw-r--r-- | sync/internal_api/sync_manager.cc | 154 | ||||
-rw-r--r-- | sync/internal_api/sync_manager.h | 23 | ||||
-rw-r--r-- | sync/internal_api/syncapi_unittest.cc | 39 | ||||
-rw-r--r-- | sync/sessions/sync_session_context.cc | 7 | ||||
-rw-r--r-- | sync/sessions/sync_session_context.h | 26 | ||||
-rw-r--r-- | sync/sessions/sync_session_unittest.cc | 16 | ||||
-rw-r--r-- | sync/sync.gyp | 2 | ||||
-rw-r--r-- | sync/syncable/syncable.cc | 5 | ||||
-rw-r--r-- | sync/syncable/syncable.h | 1 | ||||
-rw-r--r-- | sync/test/engine/fake_model_safe_worker_registrar.cc | 42 | ||||
-rw-r--r-- | sync/test/engine/fake_model_safe_worker_registrar.h | 36 | ||||
-rw-r--r-- | sync/test/engine/syncer_command_test.h | 18 |
18 files changed, 245 insertions, 296 deletions
diff --git a/sync/engine/model_safe_worker.h b/sync/engine/model_safe_worker.h index ced63d6..e3f3964 100644 --- a/sync/engine/model_safe_worker.h +++ b/sync/engine/model_safe_worker.h @@ -82,31 +82,6 @@ syncable::ModelTypeSet GetRoutingInfoTypes( ModelSafeGroup GetGroupForModelType(const syncable::ModelType type, const ModelSafeRoutingInfo& routes); -// Maintain the up-to-date state regarding which ModelSafeWorkers exist and -// which types get routed to which worker. When a sync session begins, it will -// snapshot the state at that instant, and will use that for the entire -// session. This means if a model becomes synced (or unsynced) by the user -// during a sync session, that session will complete and be unaware of this -// change -- it will only get picked up for the next session. -// TODO(tim): That's really the only way I can make sense of it in the Syncer -// HOWEVER, it is awkward for running ModelAssociation. We need to make sure -// we don't run such a thing until an active session wraps up. -class ModelSafeWorkerRegistrar { - public: - ModelSafeWorkerRegistrar() { } - // Get the current list of active ModelSafeWorkers. Should be threadsafe. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) = 0; - - // Get the current routing information for all enabled model types. - // If a model type is not enabled (that is, if the syncer should not - // be trying to sync it), it is not in this map. - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) = 0; - protected: - virtual ~ModelSafeWorkerRegistrar() {} - private: - DISALLOW_COPY_AND_ASSIGN(ModelSafeWorkerRegistrar); -}; - } // namespace browser_sync #endif // SYNC_ENGINE_MODEL_SAFE_WORKER_H_ diff --git a/sync/engine/sync_scheduler.cc b/sync/engine/sync_scheduler.cc index 9ddd26c..1e9da41 100644 --- a/sync/engine/sync_scheduler.cc +++ b/sync/engine/sync_scheduler.cc @@ -266,7 +266,7 @@ void SyncScheduler::Start(Mode mode, const base::Closure& callback) { void SyncScheduler::SendInitialSnapshot() { DCHECK_EQ(MessageLoop::current(), sync_loop_); - scoped_ptr<SyncSession> dummy(new SyncSession(session_context_.get(), this, + scoped_ptr<SyncSession> dummy(new SyncSession(session_context_, this, SyncSourceInfo(), ModelSafeRoutingInfo(), std::vector<ModelSafeWorker*>())); SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED); @@ -439,8 +439,8 @@ void SyncScheduler::SaveJob(const SyncSessionJob& job) { DCHECK(mode_ == CONFIGURATION_MODE); SyncSession* old = job.session.get(); - SyncSession* s(new SyncSession(session_context_.get(), this, - old->source(), old->routing_info(), old->workers())); + SyncSession* s(new SyncSession(session_context_, this, old->source(), + old->routing_info(), old->workers())); SyncSessionJob new_job(job.purpose, TimeTicks::Now(), make_linked_ptr(s), false, job.from_here); wait_interval_->pending_configure_job.reset(new SyncSessionJob(new_job)); @@ -582,32 +582,33 @@ void SyncScheduler::ScheduleNudgeImpl( } // Helper to extract the routing info and workers corresponding to types in -// |types| from |registrar|. +// |types| from |current_routes| and |current_workers|. void GetModelSafeParamsForTypes(ModelTypeSet types, - ModelSafeWorkerRegistrar* registrar, ModelSafeRoutingInfo* routes, - std::vector<ModelSafeWorker*>* workers) { - ModelSafeRoutingInfo r_tmp; - std::vector<ModelSafeWorker*> w_tmp; - registrar->GetModelSafeRoutingInfo(&r_tmp); - registrar->GetWorkers(&w_tmp); - + const ModelSafeRoutingInfo& current_routes, + const std::vector<ModelSafeWorker*>& current_workers, + ModelSafeRoutingInfo* result_routes, + std::vector<ModelSafeWorker*>* result_workers) { bool passive_group_added = false; typedef std::vector<ModelSafeWorker*>::const_iterator iter; for (ModelTypeSet::Iterator it = types.First(); it.Good(); it.Inc()) { const syncable::ModelType t = it.Get(); - DCHECK_EQ(1U, r_tmp.count(t)); - (*routes)[t] = r_tmp[t]; - iter w_tmp_it = std::find_if(w_tmp.begin(), w_tmp.end(), - ModelSafeWorkerGroupIs(r_tmp[t])); - if (w_tmp_it != w_tmp.end()) { - iter workers_it = std::find_if(workers->begin(), workers->end(), - ModelSafeWorkerGroupIs(r_tmp[t])); - if (workers_it == workers->end()) - workers->push_back(*w_tmp_it); - - if (r_tmp[t] == GROUP_PASSIVE) + ModelSafeRoutingInfo::const_iterator route = current_routes.find(t); + DCHECK(route != current_routes.end()); + ModelSafeGroup group = route->second; + + (*result_routes)[t] = group; + iter w_tmp_it = std::find_if(current_workers.begin(), current_workers.end(), + ModelSafeWorkerGroupIs(group)); + if (w_tmp_it != current_workers.end()) { + iter result_workers_it = std::find_if( + result_workers->begin(), result_workers->end(), + ModelSafeWorkerGroupIs(group)); + if (result_workers_it == result_workers->end()) + result_workers->push_back(*w_tmp_it); + + if (group == GROUP_PASSIVE) passive_group_added = true; } else { NOTREACHED(); @@ -616,10 +617,10 @@ void GetModelSafeParamsForTypes(ModelTypeSet types, // Always add group passive. if (passive_group_added == false) { - iter it = std::find_if(w_tmp.begin(), w_tmp.end(), + iter it = std::find_if(current_workers.begin(), current_workers.end(), ModelSafeWorkerGroupIs(GROUP_PASSIVE)); - if (it != w_tmp.end()) - workers->push_back(*it); + if (it != current_workers.end()) + result_workers->push_back(*it); else NOTREACHED(); } @@ -631,9 +632,12 @@ void SyncScheduler::ScheduleConfig( DCHECK_EQ(MessageLoop::current(), sync_loop_); DCHECK(IsConfigRelatedUpdateSourceValue(source)); SDVLOG(2) << "Scheduling a config"; + ModelSafeRoutingInfo routes; std::vector<ModelSafeWorker*> workers; - GetModelSafeParamsForTypes(types, session_context_->registrar(), + GetModelSafeParamsForTypes(types, + session_context_->routing_info(), + session_context_->workers(), &routes, &workers); PostTask(FROM_HERE, "ScheduleConfigImpl", @@ -652,7 +656,7 @@ void SyncScheduler::ScheduleConfigImpl( SDVLOG(2) << "In ScheduleConfigImpl"; // TODO(tim): config-specific GetUpdatesCallerInfo value? - SyncSession* session = new SyncSession(session_context_.get(), this, + SyncSession* session = new SyncSession(session_context_, this, SyncSourceInfo(source, syncable::ModelTypePayloadMapFromRoutingInfo( routing_info, std::string())), @@ -971,7 +975,7 @@ void SyncScheduler::HandleContinuationError( length)); if (old_job.purpose == SyncSessionJob::CONFIGURATION) { SyncSession* old = old_job.session.get(); - SyncSession* s(new SyncSession(session_context_.get(), this, + SyncSession* s(new SyncSession(session_context_, this, old->source(), old->routing_info(), old->workers())); SyncSessionJob job(old_job.purpose, TimeTicks::Now() + length, make_linked_ptr(s), false, FROM_HERE); @@ -1075,16 +1079,12 @@ void SyncScheduler::DoPendingJobIfPossible(bool is_canary_job) { SyncSession* SyncScheduler::CreateSyncSession(const SyncSourceInfo& source) { DCHECK_EQ(MessageLoop::current(), sync_loop_); - ModelSafeRoutingInfo routes; - std::vector<ModelSafeWorker*> workers; - session_context_->registrar()->GetModelSafeRoutingInfo(&routes); DVLOG(2) << "Creating sync session with routes " - << ModelSafeRoutingInfoToString(routes); - session_context_->registrar()->GetWorkers(&workers); - SyncSourceInfo info(source); + << ModelSafeRoutingInfoToString(session_context_->routing_info()); - SyncSession* session(new SyncSession(session_context_.get(), this, info, - routes, workers)); + SyncSourceInfo info(source); + SyncSession* session(new SyncSession(session_context_, this, info, + session_context_->routing_info(), session_context_->workers())); return session; } diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h index 4ec621d..dfd63e4 100644 --- a/sync/engine/sync_scheduler.h +++ b/sync/engine/sync_scheduler.h @@ -54,7 +54,7 @@ class SyncScheduler : public sessions::SyncSession::Delegate { // (except for RequestEarlyExit()). // |name| is a display string to identify the syncer thread. Takes - // |ownership of both |context| and |syncer|. + // |ownership of |syncer|. SyncScheduler(const std::string& name, sessions::SyncSessionContext* context, Syncer* syncer); @@ -409,7 +409,7 @@ class SyncScheduler : public sessions::SyncSession::Delegate { // Invoked to run through the sync cycle. scoped_ptr<Syncer> syncer_; - scoped_ptr<sessions::SyncSessionContext> session_context_; + sessions::SyncSessionContext *session_context_; DISALLOW_COPY_AND_ASSIGN(SyncScheduler); }; diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index e8bbd65..312ed19 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -11,7 +11,7 @@ #include "sync/engine/sync_scheduler.h" #include "sync/engine/syncer.h" #include "sync/sessions/test_util.h" -#include "sync/test/engine/fake_model_safe_worker_registrar.h" +#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" @@ -87,16 +87,27 @@ class SyncSchedulerTest : public testing::Test { dir_maker_.SetUp(); syncer_ = new MockSyncer(); delay_ = NULL; + ModelSafeRoutingInfo routing_info; routing_info[syncable::BOOKMARKS] = GROUP_UI; routing_info[syncable::AUTOFILL] = GROUP_DB; routing_info[syncable::THEMES] = GROUP_UI; routing_info[syncable::NIGORI] = GROUP_PASSIVE; - registrar_.reset(new FakeModelSafeWorkerRegistrar(routing_info)); + + workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI))); + workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_DB))); + workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE))); + + std::vector<ModelSafeWorker*> workers; + for (std::vector<scoped_refptr<FakeModelWorker> >::iterator it = + workers_.begin(); it != workers_.end(); ++it) { + workers.push_back(it->get()); + } + connection_.reset(new MockConnectionManager(directory())); connection_->SetServerReachable(); context_ = new SyncSessionContext( - connection_.get(), directory(), registrar_.get(), + connection_.get(), directory(), routing_info, workers, &extensions_activity_monitor_, std::vector<SyncEngineEventListener*>(), NULL, NULL); context_->set_notifications_enabled(true); @@ -200,7 +211,7 @@ class SyncSchedulerTest : public testing::Test { SyncSessionContext* context_; MockSyncer* syncer_; MockDelayProvider* delay_; - scoped_ptr<FakeModelSafeWorkerRegistrar> registrar_; + std::vector<scoped_refptr<FakeModelWorker> > workers_; FakeExtensionsActivityMonitor extensions_activity_monitor_; }; @@ -1142,8 +1153,7 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) { TEST_F(SyncSchedulerTest, SetsPreviousRoutingInfo) { ModelSafeRoutingInfo info; EXPECT_TRUE(info == context()->previous_session_routing_info()); - ModelSafeRoutingInfo expected; - context()->registrar()->GetModelSafeRoutingInfo(&expected); + ModelSafeRoutingInfo expected(context()->routing_info()); ASSERT_FALSE(expected.empty()); EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1); diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc index 1b67a18..bd71a0d 100644 --- a/sync/engine/sync_scheduler_whitebox_unittest.cc +++ b/sync/engine/sync_scheduler_whitebox_unittest.cc @@ -7,7 +7,7 @@ #include "sync/engine/sync_scheduler.h" #include "sync/sessions/sync_session_context.h" #include "sync/sessions/test_util.h" -#include "sync/test/engine/fake_model_safe_worker_registrar.h" +#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" @@ -29,15 +29,25 @@ class SyncSchedulerWhiteboxTest : public testing::Test { virtual void SetUp() { dir_maker_.SetUp(); Syncer* syncer = new Syncer(); + ModelSafeRoutingInfo routes; routes[syncable::BOOKMARKS] = GROUP_UI; routes[syncable::NIGORI] = GROUP_PASSIVE; - registrar_.reset(new FakeModelSafeWorkerRegistrar(routes)); + + workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI))); + workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE))); + + std::vector<ModelSafeWorker*> workers; + for (std::vector<scoped_refptr<FakeModelWorker> >::iterator it = + workers_.begin(); it != workers_.end(); ++it) { + workers.push_back(it->get()); + } + connection_.reset(new MockConnectionManager(NULL)); context_ = new SyncSessionContext( connection_.get(), dir_maker_.directory(), - registrar_.get(), &extensions_activity_monitor_, + routes, workers, &extensions_activity_monitor_, std::vector<SyncEngineEventListener*>(), NULL, NULL); context_->set_notifications_enabled(true); context_->set_account_name("Test"); @@ -107,7 +117,7 @@ class SyncSchedulerWhiteboxTest : public testing::Test { MessageLoop message_loop_; scoped_ptr<MockConnectionManager> connection_; SyncSessionContext* context_; - scoped_ptr<FakeModelSafeWorkerRegistrar> registrar_; + std::vector<scoped_refptr<FakeModelWorker> > workers_; FakeExtensionsActivityMonitor extensions_activity_monitor_; TestDirectorySetterUpper dir_maker_; }; diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index fdba503..0f698f4 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -113,7 +113,6 @@ using sessions::SyncSession; class SyncerTest : public testing::Test, public SyncSession::Delegate, - public ModelSafeWorkerRegistrar, public SyncEngineEventListener { protected: SyncerTest() @@ -147,12 +146,11 @@ class SyncerTest : public testing::Test, const sessions::SyncSessionSnapshot& snapshot) OVERRIDE { } - // ModelSafeWorkerRegistrar implementation. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE { + void GetWorkers(std::vector<ModelSafeWorker*>* out) { out->push_back(worker_.get()); } - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE { + void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) { // We're just testing the sync engine here, so we shunt everything to // the SyncerThread. Datatypes which aren't enabled aren't in the map. for (syncable::ModelTypeSet::Iterator it = enabled_datatypes_.First(); @@ -227,9 +225,16 @@ class SyncerTest : public testing::Test, worker_ = new FakeModelWorker(GROUP_PASSIVE); std::vector<SyncEngineEventListener*> listeners; listeners.push_back(this); + + ModelSafeRoutingInfo routing_info; + std::vector<ModelSafeWorker*> workers; + + GetModelSafeRoutingInfo(&routing_info); + GetWorkers(&workers); + context_.reset( new SyncSessionContext( - mock_server_.get(), directory(), this, + mock_server_.get(), directory(), routing_info, workers, &extensions_activity_monitor_, listeners, NULL, &traffic_recorder_)); ASSERT_FALSE(context_->resolver()); @@ -453,11 +458,27 @@ class SyncerTest : public testing::Test, void EnableDatatype(syncable::ModelType model_type) { enabled_datatypes_.Put(model_type); + + ModelSafeRoutingInfo routing_info; + GetModelSafeRoutingInfo(&routing_info); + + if (context_.get()) { + context_->set_routing_info(routing_info); + } + mock_server_->ExpectGetUpdatesRequestTypes(enabled_datatypes_); } void DisableDatatype(syncable::ModelType model_type) { enabled_datatypes_.Remove(model_type); + + ModelSafeRoutingInfo routing_info; + GetModelSafeRoutingInfo(&routing_info); + + if (context_.get()) { + context_->set_routing_info(routing_info); + } + mock_server_->ExpectGetUpdatesRequestTypes(enabled_datatypes_); } diff --git a/sync/internal_api/sync_manager.cc b/sync/internal_api/sync_manager.cc index 9da3e27..b25c8848 100644 --- a/sync/internal_api/sync_manager.cc +++ b/sync/internal_api/sync_manager.cc @@ -68,7 +68,6 @@ using browser_sync::JsEventHandler; using browser_sync::JsReplyHandler; using browser_sync::JsMutationEventObserver; using browser_sync::JsSyncManagerObserver; -using browser_sync::ModelSafeWorkerRegistrar; using browser_sync::kNigoriTag; using browser_sync::KeyParams; using browser_sync::ModelSafeRoutingInfo; @@ -141,7 +140,6 @@ class SyncManager::SyncInternal explicit SyncInternal(const std::string& name) : name_(name), weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), - registrar_(NULL), change_delegate_(NULL), initialized_(false), testing_mode_(NON_TEST), @@ -197,7 +195,8 @@ class SyncManager::SyncInternal bool use_ssl, const scoped_refptr<base::TaskRunner>& blocking_task_runner, HttpPostProviderFactory* post_factory, - ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const browser_sync::ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<browser_sync::ModelSafeWorker*>& workers, browser_sync::ExtensionsActivityMonitor* extensions_activity_monitor, ChangeDelegate* change_delegate, @@ -221,10 +220,11 @@ class SyncManager::SyncInternal void UpdateCredentials(const SyncCredentials& credentials); // Called when the user disables or enables a sync type. - void UpdateEnabledTypes(); + void UpdateEnabledTypes(const ModelTypeSet& enabled_types); // Tell the sync engine to start the syncing process. - void StartSyncingNormally(); + void StartSyncingNormally( + const browser_sync::ModelSafeRoutingInfo& routing_info); // Whether or not the Nigori node is encrypted using an explicit passphrase. bool IsUsingExplicitPassphrase(); @@ -333,6 +333,7 @@ class SyncManager::SyncInternal SyncAPIServerConnectionManager* connection_manager() { return connection_manager_.get(); } + SyncSessionContext* session_context() { return session_context_.get(); } SyncScheduler* scheduler() const { return scheduler_.get(); } UserShare* GetUserShare() { DCHECK(initialized_); @@ -375,16 +376,9 @@ class SyncManager::SyncInternal // Called only by our NetworkChangeNotifier. virtual void OnIPAddressChanged() OVERRIDE; - bool InitialSyncEndedForAllEnabledTypes() { - syncable::ModelTypeSet types; - ModelSafeRoutingInfo enabled_types; - registrar_->GetModelSafeRoutingInfo(&enabled_types); - for (ModelSafeRoutingInfo::const_iterator i = enabled_types.begin(); - i != enabled_types.end(); ++i) { - types.Put(i->first); - } - - return InitialSyncEndedForTypes(types, &share_); + syncable::ModelTypeSet InitialSyncEndedTypes() { + DCHECK(initialized_); + return directory()->initial_sync_ended_types(); } // SyncEngineEventListener implementation. @@ -562,6 +556,10 @@ class SyncManager::SyncInternal // client (the Syncer) and the sync server. scoped_ptr<SyncAPIServerConnectionManager> connection_manager_; + // A container of various bits of information used by the SyncScheduler to + // create SyncSessions. Must outlive the SyncScheduler. + scoped_ptr<SyncSessionContext> session_context_; + // The scheduler that runs the Syncer. Needs to be explicitly // Start()ed. scoped_ptr<SyncScheduler> scheduler_; @@ -581,10 +579,6 @@ class SyncManager::SyncInternal // TRANSACTION_COMPLETE step by HandleTransactionCompleteChangeEvent. ChangeReorderBuffer change_buffers_[syncable::MODEL_TYPE_COUNT]; - // The entity that provides us with information about which types to sync. - // The instance is shared between the SyncManager and the Syncer. - ModelSafeWorkerRegistrar* registrar_; - SyncManager::ChangeDelegate* change_delegate_; // Set to true once Init has been called. @@ -745,7 +739,8 @@ bool SyncManager::Init( bool use_ssl, const scoped_refptr<base::TaskRunner>& blocking_task_runner, HttpPostProviderFactory* post_factory, - ModelSafeWorkerRegistrar* registrar, + const browser_sync::ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<browser_sync::ModelSafeWorker*>& workers, browser_sync::ExtensionsActivityMonitor* extensions_activity_monitor, ChangeDelegate* change_delegate, const std::string& user_agent, @@ -767,7 +762,8 @@ bool SyncManager::Init( use_ssl, blocking_task_runner, post_factory, - registrar, + model_safe_routing_info, + workers, extensions_activity_monitor, change_delegate, user_agent, @@ -785,9 +781,9 @@ void SyncManager::UpdateCredentials(const SyncCredentials& credentials) { data_->UpdateCredentials(credentials); } -void SyncManager::UpdateEnabledTypes() { +void SyncManager::UpdateEnabledTypes(const ModelTypeSet& enabled_types) { DCHECK(thread_checker_.CalledOnValidThread()); - data_->UpdateEnabledTypes(); + data_->UpdateEnabledTypes(enabled_types); } void SyncManager::ThrowUnrecoverableError() { @@ -797,13 +793,14 @@ void SyncManager::ThrowUnrecoverableError() { FROM_HERE, "Simulating unrecoverable error for testing purposes."); } -bool SyncManager::InitialSyncEndedForAllEnabledTypes() { - return data_->InitialSyncEndedForAllEnabledTypes(); +syncable::ModelTypeSet SyncManager::InitialSyncEndedTypes() { + return data_->InitialSyncEndedTypes(); } -void SyncManager::StartSyncingNormally() { +void SyncManager::StartSyncingNormally( + const browser_sync::ModelSafeRoutingInfo& routing_info) { DCHECK(thread_checker_.CalledOnValidThread()); - data_->StartSyncingNormally(); + data_->StartSyncingNormally(routing_info); } void SyncManager::SetEncryptionPassphrase(const std::string& passphrase, @@ -844,10 +841,13 @@ bool SyncManager::IsUsingExplicitPassphrase() { return data_ && data_->IsUsingExplicitPassphrase(); } -void SyncManager::RequestCleanupDisabledTypes() { +void SyncManager::RequestCleanupDisabledTypes( + const browser_sync::ModelSafeRoutingInfo& routing_info) { DCHECK(thread_checker_.CalledOnValidThread()); - if (data_->scheduler()) + if (data_->scheduler()) { + data_->session_context()->set_routing_info(routing_info); data_->scheduler()->ScheduleCleanupDisabledTypes(); + } } void SyncManager::RequestClearServerData() { @@ -857,7 +857,8 @@ void SyncManager::RequestClearServerData() { } void SyncManager::RequestConfig( - ModelTypeSet types, ConfigureReason reason) { + const browser_sync::ModelSafeRoutingInfo& routing_info, + const ModelTypeSet& types, ConfigureReason reason) { DCHECK(thread_checker_.CalledOnValidThread()); if (!data_->scheduler()) { LOG(INFO) @@ -866,6 +867,7 @@ void SyncManager::RequestConfig( return; } StartConfigurationMode(base::Closure()); + data_->session_context()->set_routing_info(routing_info); data_->scheduler()->ScheduleConfig(types, GetSourceFromReason(reason)); } @@ -889,7 +891,8 @@ bool SyncManager::SyncInternal::Init( bool use_ssl, const scoped_refptr<base::TaskRunner>& blocking_task_runner, HttpPostProviderFactory* post_factory, - ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const browser_sync::ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<browser_sync::ModelSafeWorker*>& workers, browser_sync::ExtensionsActivityMonitor* extensions_activity_monitor, ChangeDelegate* change_delegate, const std::string& user_agent, @@ -910,7 +913,6 @@ bool SyncManager::SyncInternal::Init( blocking_task_runner_ = blocking_task_runner; - registrar_ = model_safe_worker_registrar; change_delegate_ = change_delegate; testing_mode_ = testing_mode; @@ -939,7 +941,6 @@ bool SyncManager::SyncInternal::Init( connection_manager()->AddListener(this); - // Test mode does not use a syncer context or syncer thread. if (testing_mode_ == NON_TEST) { // Build a SyncSessionContext and store the worker in it. @@ -947,17 +948,17 @@ bool SyncManager::SyncInternal::Init( std::vector<SyncEngineEventListener*> listeners; listeners.push_back(&allstatus_); listeners.push_back(this); - SyncSessionContext* context = new SyncSessionContext( + session_context_.reset(new SyncSessionContext( connection_manager_.get(), directory(), - model_safe_worker_registrar, + model_safe_routing_info, + workers, extensions_activity_monitor, listeners, &debug_info_event_listener_, - &traffic_recorder_); - context->set_account_name(credentials.email); - // The SyncScheduler takes ownership of |context|. - scheduler_.reset(new SyncScheduler(name_, context, new Syncer())); + &traffic_recorder_)); + session_context()->set_account_name(credentials.email); + scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer())); } bool signed_in = SignIn(credentials); @@ -1136,10 +1137,13 @@ void SyncManager::SyncInternal::NotifyCryptographerState( cryptographer->has_pending_keys()); } -void SyncManager::SyncInternal::StartSyncingNormally() { +void SyncManager::SyncInternal::StartSyncingNormally( + const browser_sync::ModelSafeRoutingInfo& routing_info) { // Start the sync scheduler. - if (scheduler()) // NULL during certain unittests. + if (scheduler()) { // NULL during certain unittests. + session_context()->set_routing_info(routing_info); scheduler()->Start(SyncScheduler::NORMAL_MODE, base::Closure()); + } } bool SyncManager::SyncInternal::OpenDirectory() { @@ -1194,7 +1198,6 @@ bool SyncManager::SyncInternal::SignIn(const SyncCredentials& credentials) { sync_notifier_->SetStateDeprecated(state); UpdateCredentials(credentials); - UpdateEnabledTypes(); return true; } @@ -1216,11 +1219,9 @@ void SyncManager::SyncInternal::UpdateCredentials( } } -void SyncManager::SyncInternal::UpdateEnabledTypes() { +void SyncManager::SyncInternal::UpdateEnabledTypes( + const ModelTypeSet& enabled_types) { DCHECK(thread_checker_.CalledOnValidThread()); - ModelSafeRoutingInfo routes; - registrar_->GetModelSafeRoutingInfo(&routes); - const ModelTypeSet enabled_types = GetRoutingInfoTypes(routes); sync_notifier_->UpdateEnabledTypes(enabled_types); } @@ -1586,29 +1587,25 @@ void SyncManager::SyncInternal::RefreshEncryption() { ReEncryptEverything(&trans); } +// This function iterates over all encrypted types. There are many scenarios in +// which data for some or all types is not currently available. In that case, +// the lookup of the root node will fail and we will skip encryption for that +// type. void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) { Cryptographer* cryptographer = trans->GetCryptographer(); if (!cryptographer || !cryptographer->is_ready()) return; syncable::ModelTypeSet encrypted_types = GetEncryptedTypes(trans); - ModelSafeRoutingInfo routes; - registrar_->GetModelSafeRoutingInfo(&routes); - std::string tag; for (syncable::ModelTypeSet::Iterator iter = encrypted_types.First(); iter.Good(); iter.Inc()) { if (iter.Get() == syncable::PASSWORDS || - iter.Get() == syncable::NIGORI || - routes.count(iter.Get()) == 0) - continue; + iter.Get() == syncable::NIGORI) + continue; // These types handle encryption differently. + ReadNode type_root(trans); - tag = syncable::ModelTypeToRootTag(iter.Get()); - if (type_root.InitByTagLookup(tag) != sync_api::BaseNode::INIT_OK) { - // This can happen when we enable a datatype for the first time on restart - // (for example when we upgrade) and therefore haven't done the initial - // download for that type at the time we RefreshEncryption. There's - // nothing we can do for now, so just move on to the next type. - continue; - } + std::string tag = syncable::ModelTypeToRootTag(iter.Get()); + if (type_root.InitByTagLookup(tag) != sync_api::BaseNode::INIT_OK) + continue; // Don't try to reencrypt if the type's data is unavailable. // Iterate through all children of this datatype. std::queue<int64> to_visit; @@ -1637,25 +1634,22 @@ void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) { } } - if (routes.count(syncable::PASSWORDS) > 0) { - // Passwords are encrypted with their own legacy scheme. - ReadNode passwords_root(trans); - std::string passwords_tag = - syncable::ModelTypeToRootTag(syncable::PASSWORDS); - // It's possible we'll have the password routing info and not the password - // root if we attempted to set a passphrase before passwords was enabled. - if (passwords_root.InitByTagLookup(passwords_tag) == - sync_api::BaseNode::INIT_OK) { - int64 child_id = passwords_root.GetFirstChildId(); - while (child_id != kInvalidId) { - WriteNode child(trans); - if (child.InitByIdLookup(child_id) != sync_api::BaseNode::INIT_OK) { - NOTREACHED(); - return; - } - child.SetPasswordSpecifics(child.GetPasswordSpecifics()); - child_id = child.GetSuccessorId(); + // Passwords are encrypted with their own legacy scheme. Passwords are always + // encrypted so we don't need to check GetEncryptedTypes() here. + ReadNode passwords_root(trans); + std::string passwords_tag = + syncable::ModelTypeToRootTag(syncable::PASSWORDS); + if (passwords_root.InitByTagLookup(passwords_tag) == + sync_api::BaseNode::INIT_OK) { + int64 child_id = passwords_root.GetFirstChildId(); + while (child_id != kInvalidId) { + WriteNode child(trans); + if (child.InitByIdLookup(child_id) != sync_api::BaseNode::INIT_OK) { + NOTREACHED(); + return; } + child.SetPasswordSpecifics(child.GetPasswordSpecifics()); + child_id = child.GetSuccessorId(); } } @@ -1708,6 +1702,7 @@ void SyncManager::SyncInternal::ShutdownOnSyncThread() { js_mutation_event_observer_.InvalidateWeakPtrs(); scheduler_.reset(); + session_context_.reset(); SetJsEventHandler(WeakHandle<JsEventHandler>()); RemoveObserver(&js_sync_manager_observer_); @@ -1740,7 +1735,6 @@ void SyncManager::SyncInternal::ShutdownOnSyncThread() { share_.directory.reset(); change_delegate_ = NULL; - registrar_ = NULL; initialized_ = false; @@ -2013,8 +2007,6 @@ void SyncManager::SyncInternal::OnSyncEngineEvent( // Notifications are sent at the end of every sync cycle, regardless of // whether we should sync again. if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) { - ModelSafeRoutingInfo enabled_types; - registrar_->GetModelSafeRoutingInfo(&enabled_types); { // Check to see if we need to notify the frontend that we have newly // encrypted types or that we require a passphrase. diff --git a/sync/internal_api/sync_manager.h b/sync/internal_api/sync_manager.h index 2ecbbdc..6076ab7 100644 --- a/sync/internal_api/sync_manager.h +++ b/sync/internal_api/sync_manager.h @@ -15,6 +15,7 @@ #include "base/task_runner.h" #include "base/threading/thread_checker.h" #include "base/time.h" +#include "sync/engine/model_safe_worker.h" #include "sync/internal_api/change_record.h" #include "sync/internal_api/configure_reason.h" #include "sync/protocol/sync_protocol_error.h" @@ -29,7 +30,6 @@ struct Experiments; class ExtensionsActivityMonitor; class JsBackend; class JsEventHandler; -class ModelSafeWorkerRegistrar; namespace sessions { class SyncSessionSnapshot; @@ -447,7 +447,8 @@ class SyncManager { bool use_ssl, const scoped_refptr<base::TaskRunner>& blocking_task_runner, HttpPostProviderFactory* post_factory, - browser_sync::ModelSafeWorkerRegistrar* registrar, + const browser_sync::ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<browser_sync::ModelSafeWorker*>& workers, browser_sync::ExtensionsActivityMonitor* extensions_activity_monitor, ChangeDelegate* change_delegate, @@ -466,20 +467,18 @@ class SyncManager { // testing). void ThrowUnrecoverableError(); - // Check if the database has been populated with a full "initial" download of - // sync items for each data type currently present in the routing info. - // Prerequisite for calling this is that OnInitializationComplete has been - // called. May be called from any thread. - bool InitialSyncEndedForAllEnabledTypes(); + // Returns the set of types for which we have stored some sync data. + syncable::ModelTypeSet InitialSyncEndedTypes(); // Update tokens that we're using in Sync. Email must stay the same. void UpdateCredentials(const SyncCredentials& credentials); // Called when the user disables or enables a sync type. - void UpdateEnabledTypes(); + void UpdateEnabledTypes(const syncable::ModelTypeSet& enabled_types); // Put the syncer in normal mode ready to perform nudges and polls. - void StartSyncingNormally(); + void StartSyncingNormally( + const browser_sync::ModelSafeRoutingInfo& routing_info); // Attempts to re-encrypt encrypted data types using the passphrase provided. // Notifies observers of the result of the operation via OnPassphraseAccepted @@ -506,10 +505,12 @@ class SyncManager { // Switches the mode of operation to CONFIGURATION_MODE and // schedules a config task to fetch updates for |types|. - void RequestConfig(syncable::ModelTypeSet types, + void RequestConfig(const browser_sync::ModelSafeRoutingInfo& routing_info, + const syncable::ModelTypeSet& types, sync_api::ConfigureReason reason); - void RequestCleanupDisabledTypes(); + void RequestCleanupDisabledTypes( + const browser_sync::ModelSafeRoutingInfo& routing_info); // Request a clearing of all data on the server void RequestClearServerData(); diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/syncapi_unittest.cc index b1e88fe..5c70618 100644 --- a/sync/internal_api/syncapi_unittest.cc +++ b/sync/internal_api/syncapi_unittest.cc @@ -79,7 +79,6 @@ using browser_sync::MockJsEventHandler; using browser_sync::MockJsReplyHandler; using browser_sync::ModelSafeRoutingInfo; using browser_sync::ModelSafeWorker; -using browser_sync::ModelSafeWorkerRegistrar; using browser_sync::sessions::SyncSessionSnapshot; using browser_sync::TestUnrecoverableErrorHandler; using browser_sync::WeakHandle; @@ -724,7 +723,6 @@ class SyncNotifierMock : public sync_notifier::SyncNotifier { } // namespace class SyncManagerTest : public testing::Test, - public ModelSafeWorkerRegistrar, public SyncManager::ChangeDelegate { protected: enum NigoriStatus { @@ -764,9 +762,8 @@ class SyncManagerTest : public testing::Test, EXPECT_CALL(*sync_notifier_mock_, UpdateCredentials(credentials.email, credentials.sync_token)); EXPECT_CALL(*sync_notifier_mock_, UpdateEnabledTypes(_)). - Times(AtLeast(1)). - WillRepeatedly( - Invoke(this, &SyncManagerTest::SyncNotifierUpdateEnabledTypes)); + WillRepeatedly( + Invoke(this, &SyncManagerTest::SyncNotifierUpdateEnabledTypes)); EXPECT_CALL(*sync_notifier_mock_, RemoveObserver(_)). WillOnce(Invoke(this, &SyncManagerTest::SyncNotifierRemoveObserver)); @@ -777,12 +774,16 @@ class SyncManagerTest : public testing::Test, EXPECT_FALSE(sync_notifier_observer_); EXPECT_FALSE(js_backend_.IsInitialized()); + std::vector<ModelSafeWorker*> workers; + ModelSafeRoutingInfo routing_info; + GetModelSafeRoutingInfo(&routing_info); + // Takes ownership of |sync_notifier_mock_|. sync_manager_.Init(temp_dir_.path(), WeakHandle<JsEventHandler>(), "bogus", 0, false, base::MessageLoopProxy::current(), - new TestHttpPostProviderFactory(), this, + new TestHttpPostProviderFactory(), routing_info, workers, &extensions_activity_monitor_, this, "bogus", credentials, sync_notifier_mock_, "", @@ -794,12 +795,10 @@ class SyncManagerTest : public testing::Test, EXPECT_TRUE(sync_notifier_observer_); EXPECT_TRUE(js_backend_.IsInitialized()); - EXPECT_EQ(1, update_enabled_types_call_count_); + EXPECT_EQ(0, update_enabled_types_call_count_); - ModelSafeRoutingInfo routes; - GetModelSafeRoutingInfo(&routes); - for (ModelSafeRoutingInfo::iterator i = routes.begin(); i != routes.end(); - ++i) { + for (ModelSafeRoutingInfo::iterator i = routing_info.begin(); + i != routing_info.end(); ++i) { type_roots_[i->first] = MakeServerNodeForType( sync_manager_.GetUserShare(), i->first); } @@ -814,12 +813,7 @@ class SyncManagerTest : public testing::Test, PumpLoop(); } - // ModelSafeWorkerRegistrar implementation. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE { - NOTIMPLEMENTED(); - out->clear(); - } - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE { + void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) { (*out)[syncable::NIGORI] = browser_sync::GROUP_PASSIVE; (*out)[syncable::BOOKMARKS] = browser_sync::GROUP_PASSIVE; (*out)[syncable::THEMES] = browser_sync::GROUP_PASSIVE; @@ -953,10 +947,15 @@ class SyncManagerTest : public testing::Test, }; TEST_F(SyncManagerTest, UpdateEnabledTypes) { + EXPECT_EQ(0, update_enabled_types_call_count_); + + ModelSafeRoutingInfo routes; + GetModelSafeRoutingInfo(&routes); + const syncable::ModelTypeSet enabled_types = + GetRoutingInfoTypes(routes); + + sync_manager_.UpdateEnabledTypes(enabled_types); EXPECT_EQ(1, update_enabled_types_call_count_); - // Triggers SyncNotifierUpdateEnabledTypes. - sync_manager_.UpdateEnabledTypes(); - EXPECT_EQ(2, update_enabled_types_call_count_); } TEST_F(SyncManagerTest, ProcessJsMessage) { diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc index dbbaaf91..8af1955 100644 --- a/sync/sessions/sync_session_context.cc +++ b/sync/sessions/sync_session_context.cc @@ -16,7 +16,8 @@ const unsigned int kMaxMessageSizeToRecord = 5 * 1024; SyncSessionContext::SyncSessionContext( ServerConnectionManager* connection_manager, syncable::Directory* directory, - ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* extensions_activity_monitor, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, @@ -24,7 +25,8 @@ SyncSessionContext::SyncSessionContext( : resolver_(NULL), connection_manager_(connection_manager), directory_(directory), - registrar_(model_safe_worker_registrar), + routing_info_(model_safe_routing_info), + workers_(workers), extensions_activity_monitor_(extensions_activity_monitor), notifications_enabled_(false), max_commit_batch_size_(kDefaultMaxCommitBatchSize), @@ -38,7 +40,6 @@ SyncSessionContext::SyncSessionContext( SyncSessionContext::SyncSessionContext() : connection_manager_(NULL), directory_(NULL), - registrar_(NULL), extensions_activity_monitor_(NULL), debug_info_getter_(NULL), traffic_recorder_(NULL) { diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h index 12a9a44..f8de52e 100644 --- a/sync/sessions/sync_session_context.h +++ b/sync/sessions/sync_session_context.h @@ -39,7 +39,6 @@ namespace browser_sync { class ConflictResolver; class ExtensionsActivityMonitor; -class ModelSafeWorkerRegistrar; class ServerConnectionManager; // Default number of items a client can commit in a single message. @@ -53,7 +52,8 @@ class SyncSessionContext { public: SyncSessionContext(ServerConnectionManager* connection_manager, syncable::Directory* directory, - ModelSafeWorkerRegistrar* model_safe_worker_registrar, + const ModelSafeRoutingInfo& model_safe_routing_info, + const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* extensions_activity_monitor, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, @@ -71,9 +71,18 @@ class SyncSessionContext { return directory_; } - ModelSafeWorkerRegistrar* registrar() { - return registrar_; + const ModelSafeRoutingInfo& routing_info() const { + return routing_info_; } + + void set_routing_info(const ModelSafeRoutingInfo& routing_info) { + routing_info_ = routing_info; + } + + const std::vector<ModelSafeWorker*> workers() const { + return workers_; + } + ExtensionsActivityMonitor* extensions_monitor() { return extensions_activity_monitor_; } @@ -150,9 +159,12 @@ class SyncSessionContext { ServerConnectionManager* const connection_manager_; syncable::Directory* const directory_; - // A registrar of workers capable of processing work closures on a thread - // that is guaranteed to be safe for model modifications. - ModelSafeWorkerRegistrar* registrar_; + // A cached copy of SyncBackendRegistrar's routing info. + // Must be updated manually when SBR's state is modified. + ModelSafeRoutingInfo routing_info_; + + // The set of ModelSafeWorkers. Used to execute tasks of various threads. + const std::vector<ModelSafeWorker*> workers_; // We use this to stuff extensions activity into CommitMessages so the server // can correlate commit traffic with extension-related bookmark mutations. diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index cf6b293..012f33d 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -27,8 +27,7 @@ namespace sessions { namespace { class SyncSessionTest : public testing::Test, - public SyncSession::Delegate, - public ModelSafeWorkerRegistrar { + public SyncSession::Delegate { public: SyncSessionTest() : controller_invocations_allowed_(false) {} @@ -40,9 +39,15 @@ class SyncSessionTest : public testing::Test, } virtual void SetUp() { + ModelSafeRoutingInfo routing_info; + std::vector<ModelSafeWorker*> workers; + + GetModelSafeRoutingInfo(&routing_info); + GetWorkers(&workers); + context_.reset( new SyncSessionContext( - NULL, NULL, this, &extensions_activity_monitor_, + NULL, NULL, routing_info, workers, &extensions_activity_monitor_, std::vector<SyncEngineEventListener*>(), NULL, NULL)); routes_.clear(); routes_[syncable::BOOKMARKS] = GROUP_UI; @@ -91,15 +96,14 @@ class SyncSessionTest : public testing::Test, FailControllerInvocationIfDisabled("SyncProtocolError"); } - // ModelSafeWorkerRegistrar implementation. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE { + void GetWorkers(std::vector<ModelSafeWorker*>* out) const { out->clear(); for (std::vector<scoped_refptr<ModelSafeWorker> >::const_iterator it = workers_.begin(); it != workers_.end(); ++it) { out->push_back(it->get()); } } - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE { + void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) const { *out = routes_; } diff --git a/sync/sync.gyp b/sync/sync.gyp index fcbd96a..4a949c7 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -373,8 +373,6 @@ 'test/null_transaction_observer.h', 'test/engine/test_directory_setter_upper.cc', 'test/engine/test_directory_setter_upper.h', - 'test/engine/fake_model_safe_worker_registrar.cc', - 'test/engine/fake_model_safe_worker_registrar.h', 'test/engine/fake_model_worker.cc', 'test/engine/fake_model_worker.h', 'test/engine/mock_connection_manager.cc', diff --git a/sync/syncable/syncable.cc b/sync/syncable/syncable.cc index 082c7b3..a9c1263 100644 --- a/sync/syncable/syncable.cc +++ b/sync/syncable/syncable.cc @@ -1020,6 +1020,11 @@ void Directory::SetDownloadProgress( kernel_->info_status = KERNEL_SHARE_INFO_DIRTY; } +ModelTypeSet Directory::initial_sync_ended_types() const { + ScopedKernelLock lock(this); + return kernel_->persisted_info.initial_sync_ended; +} + bool Directory::initial_sync_ended_for_type(ModelType type) const { ScopedKernelLock lock(this); return kernel_->persisted_info.initial_sync_ended.Has(type); diff --git a/sync/syncable/syncable.h b/sync/syncable/syncable.h index 0d0699f..c9acc2e 100644 --- a/sync/syncable/syncable.h +++ b/sync/syncable/syncable.h @@ -868,6 +868,7 @@ class Directory { ModelType type, const sync_pb::DataTypeProgressMarker& value); + ModelTypeSet initial_sync_ended_types() const; bool initial_sync_ended_for_type(ModelType type) const; void set_initial_sync_ended_for_type(ModelType type, bool value); diff --git a/sync/test/engine/fake_model_safe_worker_registrar.cc b/sync/test/engine/fake_model_safe_worker_registrar.cc deleted file mode 100644 index be4a0d9..0000000 --- a/sync/test/engine/fake_model_safe_worker_registrar.cc +++ /dev/null @@ -1,42 +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/engine/fake_model_safe_worker_registrar.h" - -#include "sync/test/engine/fake_model_worker.h" - -namespace browser_sync { - -FakeModelSafeWorkerRegistrar::FakeModelSafeWorkerRegistrar( - const ModelSafeRoutingInfo& routes) : routes_(routes) { - std::set<ModelSafeGroup> groups; - for (ModelSafeRoutingInfo::const_iterator it = routes_.begin(); - it != routes_.end(); ++it) { - groups.insert(it->second); - } - // Sessions always expect a passive worker to be present. - groups.insert(GROUP_PASSIVE); - - for (std::set<ModelSafeGroup>::const_iterator it = groups.begin(); - it != groups.end(); ++it) { - workers_.push_back(make_scoped_refptr(new FakeModelWorker(*it))); - } -} - -FakeModelSafeWorkerRegistrar::~FakeModelSafeWorkerRegistrar() {} - -void FakeModelSafeWorkerRegistrar::GetWorkers( - std::vector<ModelSafeWorker*>* out) { - for (std::vector<scoped_refptr<ModelSafeWorker> >::const_iterator it = - workers_.begin(); it != workers_.end(); ++it) { - out->push_back(it->get()); - } -} - -void FakeModelSafeWorkerRegistrar::GetModelSafeRoutingInfo( - ModelSafeRoutingInfo* out) { - *out = routes_; -} - -} // namespace browser_sync diff --git a/sync/test/engine/fake_model_safe_worker_registrar.h b/sync/test/engine/fake_model_safe_worker_registrar.h deleted file mode 100644 index 2eff29b..0000000 --- a/sync/test/engine/fake_model_safe_worker_registrar.h +++ /dev/null @@ -1,36 +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_ENGINE_FAKE_MODEL_SAFE_WORKER_REGISTRAR_H_ -#define SYNC_TEST_ENGINE_FAKE_MODEL_SAFE_WORKER_REGISTRAR_H_ -#pragma once - -#include <vector> - -#include "base/basictypes.h" -#include "base/compiler_specific.h" -#include "base/memory/ref_counted.h" -#include "sync/engine/model_safe_worker.h" - -namespace browser_sync { - -class FakeModelSafeWorkerRegistrar : public ModelSafeWorkerRegistrar { - public: - explicit FakeModelSafeWorkerRegistrar(const ModelSafeRoutingInfo& routes); - - virtual ~FakeModelSafeWorkerRegistrar(); - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE; - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE; - - private: - const ModelSafeRoutingInfo routes_; - std::vector<scoped_refptr<ModelSafeWorker> > workers_; - - DISALLOW_COPY_AND_ASSIGN(FakeModelSafeWorkerRegistrar); -}; - -} // namespace browser_sync - -#endif // SYNC_TEST_ENGINE_FAKE_MODEL_SAFE_WORKER_REGISTRAR_H_ - diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index bfa567f..1fe919b 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -42,8 +42,7 @@ class MockDebugInfoGetter : public browser_sync::sessions::DebugInfoGetter { // SyncerCommands, providing convenient access to a test directory // and a syncer session. class SyncerCommandTestBase : public testing::Test, - public sessions::SyncSession::Delegate, - public ModelSafeWorkerRegistrar { + public sessions::SyncSession::Delegate { public: enum UseMockDirectory { USE_MOCK_DIRECTORY @@ -77,13 +76,14 @@ class SyncerCommandTestBase : public testing::Test, return; } - // ModelSafeWorkerRegistrar implementation. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE { + std::vector<ModelSafeWorker*> GetWorkers() { + std::vector<ModelSafeWorker*> workers; std::vector<scoped_refptr<ModelSafeWorker> >::iterator it; for (it = workers_.begin(); it != workers_.end(); ++it) - out->push_back(*it); + workers.push_back(*it); + return workers; } - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE { + void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) { ModelSafeRoutingInfo copy(routing_info_); out->swap(copy); } @@ -97,7 +97,6 @@ class SyncerCommandTestBase : public testing::Test, sessions::SyncSessionContext* context() const { return context_.get(); } sessions::SyncSession::Delegate* delegate() { return this; } - ModelSafeWorkerRegistrar* registrar() { return this; } // Lazily create a session requesting all datatypes with no payload. sessions::SyncSession* session() { @@ -110,8 +109,7 @@ class SyncerCommandTestBase : public testing::Test, // Create a session with the provided source. sessions::SyncSession* session(const sessions::SyncSourceInfo& source) { if (!session_.get()) { - std::vector<ModelSafeWorker*> workers; - GetWorkers(&workers); + std::vector<ModelSafeWorker*> workers = GetWorkers(); session_.reset(new sessions::SyncSession(context(), delegate(), source, routing_info_, workers)); } @@ -125,7 +123,7 @@ class SyncerCommandTestBase : public testing::Test, void ResetContext() { context_.reset(new sessions::SyncSessionContext( mock_server_.get(), directory(), - registrar(), &extensions_activity_monitor_, + routing_info_, GetWorkers(), &extensions_activity_monitor_, std::vector<SyncEngineEventListener*>(), &mock_debug_info_getter_, &traffic_recorder_)); |