diff options
17 files changed, 279 insertions, 63 deletions
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index 12fe48d..018c5cf 100644 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -1019,7 +1019,8 @@ class SyncManager::SyncInternal registrar_(NULL), notification_pending_(false), initialized_(false), - ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { + ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)), + server_notifier_thread_(NULL) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } @@ -1049,6 +1050,10 @@ class SyncManager::SyncInternal // Update tokens that we're using in Sync. Email must stay the same. void UpdateCredentials(const SyncCredentials& credentials); + // Update the set of enabled sync types. Usually called when the user disables + // or enables a sync type. + void UpdateEnabledTypes(const syncable::ModelTypeSet& types); + // Tell the sync engine to start the syncing process. void StartSyncing(); @@ -1387,7 +1392,11 @@ class SyncManager::SyncInternal // actually communicating with the server). bool setup_for_test_mode_; + syncable::ModelTypeSet enabled_types_; + ScopedRunnableMethodFactory<SyncManager::SyncInternal> method_factory_; + + sync_notifier::ServerNotifierThread* server_notifier_thread_; }; const int SyncManager::SyncInternal::kDefaultNudgeDelayMilliseconds = 200; const int SyncManager::SyncInternal::kPreferencesNudgeDelayMilliseconds = 2000; @@ -1429,6 +1438,10 @@ void SyncManager::UpdateCredentials(const SyncCredentials& credentials) { data_->UpdateCredentials(credentials); } +void SyncManager::UpdateEnabledTypes(const syncable::ModelTypeSet& types) { + data_->UpdateEnabledTypes(types); +} + bool SyncManager::InitialSyncEndedForAllEnabledTypes() { return data_->InitialSyncEndedForAllEnabledTypes(); @@ -1624,7 +1637,7 @@ void SyncManager::SyncInternal::SendPendingXMPPNotification( VLOG(1) << "Not sending notification: no pending notification"; return; } - if (!talk_mediator_.get()) { + if (!talk_mediator()) { VLOG(1) << "Not sending notification: shutting down (talk_mediator_ is " "NULL)"; return; @@ -1639,7 +1652,7 @@ void SyncManager::SyncInternal::SendPendingXMPPNotification( notification_data.service_specific_data = browser_sync::kSyncServiceSpecificData; notification_data.require_subscription = true; - bool success = talk_mediator_->SendNotification(notification_data); + bool success = talk_mediator()->SendNotification(notification_data); if (success) { notification_pending_ = false; VLOG(1) << "Sent XMPP notification"; @@ -1703,6 +1716,16 @@ void SyncManager::SyncInternal::UpdateCredentials( sync_manager_->RequestNudge(); } +void SyncManager::SyncInternal::UpdateEnabledTypes( + const syncable::ModelTypeSet& types) { + DCHECK_EQ(MessageLoop::current(), core_message_loop_); + + enabled_types_ = types; + if (server_notifier_thread_ != NULL) { + server_notifier_thread_->UpdateEnabledTypes(types); + } +} + void SyncManager::SyncInternal::InitializeTalkMediator() { if (notifier_options_.notification_method == notifier::NOTIFICATION_SERVER) { @@ -1717,13 +1740,20 @@ void SyncManager::SyncInternal::InitializeTalkMediator() { base::Base64Encode(state, &encoded_state); VLOG(1) << "Read notification state: " << encoded_state; } - sync_notifier::ServerNotifierThread* server_notifier_thread = - new sync_notifier::ServerNotifierThread( - notifier_options_, state, this); + + // |talk_mediator_| takes ownership of |sync_notifier_thread_| + // but it is. guaranteed that |sync_notifier_thread_| is destroyed only + // when |talk_mediator_| is (see the comments in talk_mediator.h). + server_notifier_thread_ = new sync_notifier::ServerNotifierThread( + notifier_options_, state, this); talk_mediator_.reset( - new TalkMediatorImpl(server_notifier_thread, + new TalkMediatorImpl(server_notifier_thread_, notifier_options_.invalidate_xmpp_login, notifier_options_.allow_insecure_connection)); + + // Since we may be initialized more than once, make sure that any + // newly created server notifier thread has the latest enabled types. + server_notifier_thread_->UpdateEnabledTypes(enabled_types_); } else { notifier::MediatorThread* mediator_thread = new notifier::MediatorThreadImpl(notifier_options_); @@ -1732,8 +1762,9 @@ void SyncManager::SyncInternal::InitializeTalkMediator() { notifier_options_.invalidate_xmpp_login, notifier_options_.allow_insecure_connection)); talk_mediator_->AddSubscribedServiceUrl(browser_sync::kSyncServiceUrl); + server_notifier_thread_ = NULL; } - talk_mediator_->SetDelegate(this); + talk_mediator()->SetDelegate(this); } void SyncManager::SyncInternal::RaiseAuthNeededEvent() { @@ -1886,7 +1917,13 @@ void SyncManager::SyncInternal::Shutdown() { talk_mediator->Logout(); VLOG(1) << "P2P: Mediator logout completed."; talk_mediator.reset(); + + // |server_notifier_thread_| is owned by |talk_mediator|. We NULL + // it out here so as to not have a dangling pointer. + server_notifier_thread_= NULL; VLOG(1) << "P2P: Mediator destroyed."; + + } // Pump any messages the auth watcher, syncer thread, or talk @@ -2377,8 +2414,8 @@ void SyncManager::SyncInternal::TalkMediatorLogin( DCHECK(!email.empty()); DCHECK(!token.empty()); InitializeTalkMediator(); - talk_mediator_->SetAuthToken(email, token, SYNC_SERVICE_NAME); - talk_mediator_->Login(); + talk_mediator()->SetAuthToken(email, token, SYNC_SERVICE_NAME); + talk_mediator()->Login(); } void SyncManager::SyncInternal::OnIncomingNotification( diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h index 2be895c..35207b6 100644 --- a/chrome/browser/sync/engine/syncapi.h +++ b/chrome/browser/sync/engine/syncapi.h @@ -865,6 +865,10 @@ class SyncManager { // Update tokens that we're using in Sync. Email must stay the same. void UpdateCredentials(const SyncCredentials& credentials); + // Update the set of enabled sync types. Usually called when the user disables + // or enables a sync type. + void UpdateEnabledTypes(const syncable::ModelTypeSet& types); + // Start the SyncerThread. void StartSyncing(); diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc index dfb13b5..ef1217c 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl.cc @@ -82,6 +82,8 @@ void DataTypeManagerImpl::Configure(const TypeSet& desired_types) { return; } + backend_->UpdateEnabledTypes(desired_types); + last_requested_types_ = desired_types; // Add any data type controllers into the needs_start_ list that are // currently NOT_RUNNING or STOPPING. diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc index 6f4cd0f..b02df55 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -134,6 +134,7 @@ class DataTypeManagerImplTest : public testing::Test { } void SetBackendExpectations(int times) { + EXPECT_CALL(backend_, UpdateEnabledTypes(_)).Times(times); EXPECT_CALL(backend_, ConfigureDataTypes(_, _, _)).Times(times); EXPECT_CALL(backend_, StartSyncingWithServer()).Times(times); EXPECT_CALL(backend_, RequestPause()).Times(times); @@ -153,6 +154,7 @@ TEST_F(DataTypeManagerImplTest, NoControllers) { DataTypeManagerImpl dtm(&backend_, controllers_); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); + EXPECT_CALL(backend_, UpdateEnabledTypes(_)); dtm.Configure(types_); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state()); dtm.Stop(); @@ -248,6 +250,7 @@ TEST_F(DataTypeManagerImplTest, ConfigureOneThenAnother) { types_.insert(syncable::PREFERENCES); SetConfigureStartExpectation(); SetConfigureDoneExpectation(DataTypeManager::OK); + dtm.Configure(types_); EXPECT_EQ(DataTypeManager::CONFIGURED, dtm.state()); diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 80564de..493593c 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -208,6 +208,14 @@ void SyncBackendHost::UpdateCredentials(const SyncCredentials& credentials) { credentials)); } +void SyncBackendHost::UpdateEnabledTypes( + const syncable::ModelTypeSet& types) { + core_thread_.message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(core_.get(), + &SyncBackendHost::Core::DoUpdateEnabledTypes, + types)); +} + void SyncBackendHost::StartSyncingWithServer() { core_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoStartSyncing)); @@ -644,6 +652,12 @@ void SyncBackendHost::Core::DoUpdateCredentials( syncapi_->UpdateCredentials(credentials); } +void SyncBackendHost::Core::DoUpdateEnabledTypes( + const syncable::ModelTypeSet& types) { + DCHECK(MessageLoop::current() == host_->core_thread_.message_loop()); + syncapi_->UpdateEnabledTypes(types); +} + void SyncBackendHost::Core::DoStartSyncing() { DCHECK(MessageLoop::current() == host_->core_thread_.message_loop()); syncapi_->StartSyncing(); diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index a8056fe..4b3d164 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -131,6 +131,8 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { // Called from |frontend_loop| to update SyncCredentials. void UpdateCredentials(const sync_api::SyncCredentials& credentials); + virtual void UpdateEnabledTypes(const syncable::ModelTypeSet& types); + // This starts the SyncerThread running a Syncer object to communicate with // sync servers. Until this is called, no changes will leave or enter this // browser from the cloud / sync servers. @@ -322,6 +324,10 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { // update on behalf of SyncBackendHost::UpdateCredentials void DoUpdateCredentials(const sync_api::SyncCredentials& credentials); + // Update the set of enabled sync types. Usually called when the user disables + // or enables a sync type. + void DoUpdateEnabledTypes(const syncable::ModelTypeSet& types); + // Called on the SyncBackendHost core_thread_ to tell the syncapi to start // syncing (generally after initialization and authentication). void DoStartSyncing(); diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.h b/chrome/browser/sync/glue/sync_backend_host_mock.h index 16f2594..12c6df1 100644 --- a/chrome/browser/sync/glue/sync_backend_host_mock.h +++ b/chrome/browser/sync/glue/sync_backend_host_mock.h @@ -27,6 +27,7 @@ class SyncBackendHostMock : public SyncBackendHost { MOCK_METHOD0(RequestPause, bool()); MOCK_METHOD0(RequestResume, bool()); MOCK_METHOD0(StartSyncingWithServer, void()); + MOCK_METHOD1(UpdateEnabledTypes, void(const syncable::ModelTypeSet&)); }; } // namespace browser_sync diff --git a/chrome/browser/sync/notifier/chrome_invalidation_client.cc b/chrome/browser/sync/notifier/chrome_invalidation_client.cc index f80a346..0c38936 100644 --- a/chrome/browser/sync/notifier/chrome_invalidation_client.cc +++ b/chrome/browser/sync/notifier/chrome_invalidation_client.cc @@ -75,7 +75,6 @@ void ChromeInvalidationClient::Start( ChangeBaseTask(base_task); registration_manager_.reset( new RegistrationManager(invalidation_client_.get())); - RegisterTypes(); } void ChromeInvalidationClient::ChangeBaseTask( @@ -103,15 +102,11 @@ void ChromeInvalidationClient::Stop() { listener_ = NULL; } -void ChromeInvalidationClient::RegisterTypes() { +void ChromeInvalidationClient::RegisterTypes( + const syncable::ModelTypeSet& types) { DCHECK(non_thread_safe_.CalledOnValidThread()); - // TODO(akalin): Make this configurable instead of listening to - // notifications for all possible types. - for (int i = syncable::FIRST_REAL_MODEL_TYPE; - i < syncable::MODEL_TYPE_COUNT; ++i) { - registration_manager_->RegisterType(syncable::ModelTypeFromInt(i)); - } + registration_manager_->SetRegisteredTypes(types); } void ChromeInvalidationClient::Invalidate( diff --git a/chrome/browser/sync/notifier/chrome_invalidation_client.h b/chrome/browser/sync/notifier/chrome_invalidation_client.h index 873b877..904b91d 100644 --- a/chrome/browser/sync/notifier/chrome_invalidation_client.h +++ b/chrome/browser/sync/notifier/chrome_invalidation_client.h @@ -69,7 +69,7 @@ class ChromeInvalidationClient // Register the sync types that we're interested in getting // notifications for. Must only be called between calls to Start() // and Stop(). - void RegisterTypes(); + void RegisterTypes(const syncable::ModelTypeSet& types); // invalidation::InvalidationListener implementation. // diff --git a/chrome/browser/sync/notifier/registration_manager.cc b/chrome/browser/sync/notifier/registration_manager.cc index c03c07c..69181a4 100644 --- a/chrome/browser/sync/notifier/registration_manager.cc +++ b/chrome/browser/sync/notifier/registration_manager.cc @@ -21,20 +21,18 @@ RegistrationManager::~RegistrationManager() { DCHECK(non_thread_safe_.CalledOnValidThread()); } -void RegistrationManager::RegisterType(syncable::ModelType model_type) { +void RegistrationManager::SetRegisteredTypes( + const syncable::ModelTypeSet& types) { DCHECK(non_thread_safe_.CalledOnValidThread()); - invalidation::ObjectId object_id; - if (!RealModelTypeToObjectId(model_type, &object_id)) { - LOG(DFATAL) << "Invalid model type: " << model_type; - return; - } - RegistrationStatusMap::iterator it = - registration_status_.insert( - std::make_pair( - model_type, - invalidation::RegistrationState_UNREGISTERED)).first; - if (it->second == invalidation::RegistrationState_UNREGISTERED) { - RegisterObject(object_id, it); + + for (int i = syncable::FIRST_REAL_MODEL_TYPE; i < syncable::MODEL_TYPE_COUNT; + ++i) { + syncable::ModelType type = syncable::ModelTypeFromInt(i); + if (types.count(type) > 0) { + RegisterType(type); + } else { + UnregisterType(type); + } } } @@ -82,6 +80,40 @@ void RegistrationManager::MarkAllRegistrationsLost() { } } +void RegistrationManager::RegisterType(syncable::ModelType model_type) { + DCHECK(non_thread_safe_.CalledOnValidThread()); + invalidation::ObjectId object_id; + if (!RealModelTypeToObjectId(model_type, &object_id)) { + LOG(DFATAL) << "Invalid model type: " << model_type; + return; + } + RegistrationStatusMap::iterator it = + registration_status_.insert( + std::make_pair( + model_type, + invalidation::RegistrationState_UNREGISTERED)).first; + if (it->second == invalidation::RegistrationState_UNREGISTERED) { + RegisterObject(object_id, it); + } +} + +void RegistrationManager::UnregisterType(syncable::ModelType model_type) { + DCHECK(non_thread_safe_.CalledOnValidThread()); + invalidation::ObjectId object_id; + if (!RealModelTypeToObjectId(model_type, &object_id)) { + LOG(DFATAL) << "Invalid model type: " << model_type; + return; + } + RegistrationStatusMap::iterator it = registration_status_.find(model_type); + + if (it != registration_status_.end()) { + if (it->second == invalidation::RegistrationState_REGISTERED) { + invalidation_client_->Unregister(object_id); + } + registration_status_.erase(it); + } +} + void RegistrationManager::RegisterObject( const invalidation::ObjectId& object_id, RegistrationStatusMap::iterator it) { @@ -89,5 +121,4 @@ void RegistrationManager::RegisterObject( invalidation_client_->Register(object_id); it->second = invalidation::RegistrationState_REGISTERED; } - } // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/registration_manager.h b/chrome/browser/sync/notifier/registration_manager.h index 2694373..d7edb04 100644 --- a/chrome/browser/sync/notifier/registration_manager.h +++ b/chrome/browser/sync/notifier/registration_manager.h @@ -12,6 +12,7 @@ #include <map> #include "base/basictypes.h" +#include "base/gtest_prod_util.h" #include "base/threading/non_thread_safe.h" #include "chrome/browser/sync/syncable/model_type.h" #include "google/cacheinvalidation/invalidation-client.h" @@ -19,6 +20,13 @@ namespace sync_notifier { class RegistrationManager { + + friend class RegistrationManagerTest; + + FRIEND_TEST_ALL_PREFIXES(RegistrationManagerTest, RegisterType); + FRIEND_TEST_ALL_PREFIXES(RegistrationManagerTest, UnregisterType); + FRIEND_TEST_ALL_PREFIXES(RegistrationManagerTest, MarkRegistrationLost); + FRIEND_TEST_ALL_PREFIXES(RegistrationManagerTest, MarkAllRegistrationsLost); public: // Does not take ownership of |invalidation_client_|. explicit RegistrationManager( @@ -26,16 +34,13 @@ class RegistrationManager { ~RegistrationManager(); - // Registers the given |model_type|, which must be valid. - void RegisterType(syncable::ModelType model_type); + void SetRegisteredTypes(const syncable::ModelTypeSet& types); // Returns true iff |model_type| is currently registered. // // Currently only used by unit tests. bool IsRegistered(syncable::ModelType model_type) const; - // TODO(akalin): We will eventually need an UnregisterType(). - // Marks the registration for the |model_type| lost and re-registers // it. void MarkRegistrationLost(syncable::ModelType model_type); @@ -47,6 +52,11 @@ class RegistrationManager { typedef std::map<syncable::ModelType, invalidation::RegistrationState> RegistrationStatusMap; + // Registers the given |model_type|, which must be valid. + void RegisterType(syncable::ModelType model_type); + + void UnregisterType(syncable::ModelType model_type); + // Calls invalidation_client_->Register() on |object_id|. sets // it->second to UNREGISTERED -> PENDING. void RegisterObject(const invalidation::ObjectId& object_id, diff --git a/chrome/browser/sync/notifier/registration_manager_unittest.cc b/chrome/browser/sync/notifier/registration_manager_unittest.cc index da93ac8..ac5b236 100644 --- a/chrome/browser/sync/notifier/registration_manager_unittest.cc +++ b/chrome/browser/sync/notifier/registration_manager_unittest.cc @@ -4,6 +4,7 @@ #include "chrome/browser/sync/notifier/registration_manager.h" +#include <algorithm> #include <cstddef> #include <deque> #include <vector> @@ -15,7 +16,13 @@ #include "testing/gtest/include/gtest/gtest.h" namespace sync_notifier { -namespace { + +syncable::ModelType ObjectIdToModelType( + const invalidation::ObjectId& object_id) { + syncable::ModelType model_type = syncable::UNSPECIFIED; + EXPECT_TRUE(ObjectIdToRealModelType(object_id, &model_type)); + return model_type; +} // Fake invalidation client that just stores the args of calls to // Register(). @@ -28,11 +35,22 @@ class FakeInvalidationClient : public invalidation::InvalidationClient { virtual void Start(const std::string& state) {} virtual void Register(const invalidation::ObjectId& oid) { - registered_oids.push_back(oid); + registered_types.push_back(ObjectIdToModelType(oid)); } virtual void Unregister(const invalidation::ObjectId& oid) { - ADD_FAILURE(); + syncable::ModelType type_to_unregister = ObjectIdToModelType(oid); + std::vector<syncable::ModelType>::iterator it = std::find( + registered_types.begin(), + registered_types.end(), + type_to_unregister); + + if(it == registered_types.end()) { + // We should not be unregistering a thing that is not yet registered. + ADD_FAILURE(); + } else { + registered_types.erase(it); + } } virtual invalidation::NetworkEndpoint* network_endpoint() { @@ -40,7 +58,7 @@ class FakeInvalidationClient : public invalidation::InvalidationClient { return NULL; } - std::deque<invalidation::ObjectId> registered_oids; + std::vector<syncable::ModelType> registered_types; private: DISALLOW_COPY_AND_ASSIGN(FakeInvalidationClient); @@ -60,13 +78,6 @@ class RegistrationManagerTest : public testing::Test { DISALLOW_COPY_AND_ASSIGN(RegistrationManagerTest); }; -syncable::ModelType ObjectIdToModelType( - const invalidation::ObjectId& object_id) { - syncable::ModelType model_type = syncable::UNSPECIFIED; - EXPECT_TRUE(ObjectIdToRealModelType(object_id, &model_type)); - return model_type; -} - TEST_F(RegistrationManagerTest, RegisterType) { const syncable::ModelType kModelTypes[] = { syncable::BOOKMARKS, @@ -85,7 +96,7 @@ TEST_F(RegistrationManagerTest, RegisterType) { } ASSERT_EQ(kModelTypeCount, - fake_invalidation_client_.registered_oids.size()); + fake_invalidation_client_.registered_types.size()); // Everything should be registered. for (size_t i = 0; i < kModelTypeCount; ++i) { @@ -95,8 +106,52 @@ TEST_F(RegistrationManagerTest, RegisterType) { // Check object IDs. for (size_t i = 0; i < kModelTypeCount; ++i) { EXPECT_EQ(kModelTypes[i], - ObjectIdToModelType( - fake_invalidation_client_.registered_oids[i])); + fake_invalidation_client_.registered_types[i]); + } +} + +TEST_F(RegistrationManagerTest, UnregisterType) { + const syncable::ModelType kModelTypes[] = { + syncable::BOOKMARKS, + syncable::PREFERENCES, + syncable::THEMES, + syncable::AUTOFILL, + syncable::EXTENSIONS, + }; + const size_t kModelTypeCount = arraysize(kModelTypes); + + // Register types. + for (size_t i = 0; i < kModelTypeCount; ++i) { + // Register twice; it shouldn't matter. + registration_manager_.RegisterType(kModelTypes[i]); + } + + ASSERT_EQ(kModelTypeCount, + fake_invalidation_client_.registered_types.size()); + + // Everything should be registered. + for (size_t i = 0; i < kModelTypeCount; ++i) { + EXPECT_TRUE(registration_manager_.IsRegistered(kModelTypes[i])); + } + + // Check object IDs. + for (size_t i = 0; i < kModelTypeCount; ++i) { + EXPECT_EQ(kModelTypes[i], + fake_invalidation_client_.registered_types[i]); + } + + // Now unregister the extension. + registration_manager_.UnregisterType(syncable::EXTENSIONS); + + // Check the count and the types currently registered to ensure extensions + // is unregistered. + ASSERT_EQ(kModelTypeCount - 1, + fake_invalidation_client_.registered_types.size()); + + // Check object IDs. + for (size_t i = 0; i < kModelTypeCount - 1; ++i) { + EXPECT_EQ(kModelTypes[i], + fake_invalidation_client_.registered_types[i]); } } @@ -116,7 +171,7 @@ TEST_F(RegistrationManagerTest, MarkRegistrationLost) { } ASSERT_EQ(kModelTypeCount, - fake_invalidation_client_.registered_oids.size()); + fake_invalidation_client_.registered_types.size()); // All should be registered. for (size_t i = 0; i < kModelTypeCount; ++i) { @@ -129,7 +184,7 @@ TEST_F(RegistrationManagerTest, MarkRegistrationLost) { } ASSERT_EQ(2 * kModelTypeCount - 1, - fake_invalidation_client_.registered_oids.size()); + fake_invalidation_client_.registered_types.size()); // All should still be registered. for (size_t i = 0; i < kModelTypeCount; ++i) { @@ -153,7 +208,7 @@ TEST_F(RegistrationManagerTest, MarkAllRegistrationsLost) { } ASSERT_EQ(kModelTypeCount, - fake_invalidation_client_.registered_oids.size()); + fake_invalidation_client_.registered_types.size()); // All should be registered. for (size_t i = 0; i < kModelTypeCount; ++i) { @@ -168,7 +223,7 @@ TEST_F(RegistrationManagerTest, MarkAllRegistrationsLost) { registration_manager_.MarkAllRegistrationsLost(); ASSERT_EQ(3 * kModelTypeCount - 1, - fake_invalidation_client_.registered_oids.size()); + fake_invalidation_client_.registered_types.size()); // All should still be registered. for (size_t i = 0; i < kModelTypeCount; ++i) { @@ -176,5 +231,4 @@ TEST_F(RegistrationManagerTest, MarkAllRegistrationsLost) { } } -} // namespace } // namespace notifier diff --git a/chrome/browser/sync/notifier/server_notifier_thread.cc b/chrome/browser/sync/notifier/server_notifier_thread.cc index e8ca751..9dd62bf 100644 --- a/chrome/browser/sync/notifier/server_notifier_thread.cc +++ b/chrome/browser/sync/notifier/server_notifier_thread.cc @@ -45,7 +45,28 @@ void ServerNotifierThread::SubscribeForUpdates( worker_message_loop()->PostTask( FROM_HERE, NewRunnableMethod( - this, &ServerNotifierThread::RegisterTypesAndSignalSubscribed)); + this, &ServerNotifierThread::RegisterTypes)); + + worker_message_loop()->PostTask( + FROM_HERE, + NewRunnableMethod( + this, &ServerNotifierThread::SignalSubscribed)); +} + +void ServerNotifierThread::UpdateEnabledTypes( + const syncable::ModelTypeSet& types) { + DCHECK_EQ(MessageLoop::current(), parent_message_loop_); + worker_message_loop()->PostTask( + FROM_HERE, + NewRunnableMethod( + this, + &ServerNotifierThread::SetRegisteredTypes, + types)); + + worker_message_loop()->PostTask( + FROM_HERE, + NewRunnableMethod( + this, &ServerNotifierThread::RegisterTypes)); } void ServerNotifierThread::Logout() { @@ -124,16 +145,20 @@ void ServerNotifierThread::DoListenForUpdates() { const std::string& client_info = webkit_glue::GetUserAgent(GURL()); chrome_invalidation_client_->Start( kClientId, client_info, state_, this, this, base_task_); + RegisterTypes(); state_.clear(); } } -void ServerNotifierThread::RegisterTypesAndSignalSubscribed() { +void ServerNotifierThread::RegisterTypes() { DCHECK_EQ(MessageLoop::current(), worker_message_loop()); if (!chrome_invalidation_client_.get()) { return; } - chrome_invalidation_client_->RegisterTypes(); + chrome_invalidation_client_->RegisterTypes(registered_types_); +} + +void ServerNotifierThread::SignalSubscribed() { observers_->Notify(&Observer::OnSubscriptionStateChange, true); } @@ -142,4 +167,8 @@ void ServerNotifierThread::StopInvalidationListener() { chrome_invalidation_client_.reset(); } +void ServerNotifierThread::SetRegisteredTypes(syncable::ModelTypeSet types) { + registered_types_ = types; +} + } // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/server_notifier_thread.h b/chrome/browser/sync/notifier/server_notifier_thread.h index b0b9819..4800735 100644 --- a/chrome/browser/sync/notifier/server_notifier_thread.h +++ b/chrome/browser/sync/notifier/server_notifier_thread.h @@ -72,12 +72,16 @@ class ServerNotifierThread // StateWriter implementation. virtual void WriteState(const std::string& state); + virtual void UpdateEnabledTypes(const syncable::ModelTypeSet& types); + private: // Posted to the worker thread by ListenForUpdates(). void DoListenForUpdates(); // Posted to the worker thread by SubscribeForUpdates(). - void RegisterTypesAndSignalSubscribed(); + void RegisterTypes(); + + void SignalSubscribed(); // Posted to the worker thread by Logout(). void StopInvalidationListener(); @@ -89,6 +93,10 @@ class ServerNotifierThread // |state_writers_|. StateWriter* state_writer_; scoped_ptr<ChromeInvalidationClient> chrome_invalidation_client_; + + syncable::ModelTypeSet registered_types_; + + void SetRegisteredTypes(syncable::ModelTypeSet types); }; } // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/server_notifier_thread_unittest.cc b/chrome/browser/sync/notifier/server_notifier_thread_unittest.cc index 416ffe5..3873c42 100644 --- a/chrome/browser/sync/notifier/server_notifier_thread_unittest.cc +++ b/chrome/browser/sync/notifier/server_notifier_thread_unittest.cc @@ -20,7 +20,7 @@ namespace sync_notifier { class FakeServerNotifierThread : public ServerNotifierThread { public: - FakeServerNotifierThread() + FakeServerNotifierThread() : ServerNotifierThread(notifier::NotifierOptions(), "fake state", ALLOW_THIS_IN_INITIALIZER_LIST(this)) {} @@ -110,6 +110,18 @@ class ServerNotifierThreadTest : public testing::Test { MessageLoop message_loop_; }; +syncable::ModelTypeSet GetTypeSetWithAllTypes() { + syncable::ModelTypeSet all_types; + + for (int i = syncable::FIRST_REAL_MODEL_TYPE; + i < syncable::MODEL_TYPE_COUNT; ++i) { + syncable::ModelType model_type = syncable::ModelTypeFromInt(i); + all_types.insert(model_type); + } + + return all_types; +} + TEST_F(ServerNotifierThreadTest, Basic) { FakeServerNotifierThread server_notifier_thread; diff --git a/chrome/browser/sync/tools/sync_listen_notifications.cc b/chrome/browser/sync/tools/sync_listen_notifications.cc index 35dfeff..0eb9459 100644 --- a/chrome/browser/sync/tools/sync_listen_notifications.cc +++ b/chrome/browser/sync/tools/sync_listen_notifications.cc @@ -191,7 +191,14 @@ class ServerNotifierDelegate server_notifier_state_, &chrome_invalidation_listener_, this, base_task); - chrome_invalidation_client_.RegisterTypes(); + syncable::ModelTypeSet all_types; + for (int i = syncable::FIRST_REAL_MODEL_TYPE; + i < syncable::MODEL_TYPE_COUNT; ++i) { + syncable::ModelType model_type = syncable::ModelTypeFromInt(i); + all_types.insert(model_type); + } + + chrome_invalidation_client_.RegisterTypes(all_types); } virtual void OnError() { diff --git a/jingle/notifier/listener/talk_mediator_impl.h b/jingle/notifier/listener/talk_mediator_impl.h index 68b80cf..7af2721 100644 --- a/jingle/notifier/listener/talk_mediator_impl.h +++ b/jingle/notifier/listener/talk_mediator_impl.h @@ -24,7 +24,10 @@ namespace notifier { class TalkMediatorImpl : public TalkMediator, public MediatorThread::Observer { public: - // Takes ownership of |mediator_thread|. + // Takes ownership of |mediator_thread|. It is guaranteed that + // |mediator_thread| is destroyed only when this object is destroyed. + // This means that you can store a pointer to mediator_thread separately + // and use it until this object is destroyed. TalkMediatorImpl( MediatorThread* mediator_thread, bool invalidate_xmpp_auth_token, bool allow_insecure_connection); |