diff options
author | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-09-07 19:19:16 +0000 |
---|---|---|
committer | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-09-07 19:19:16 +0000 |
commit | 08a6f99997bad56869412e1da065acda3dc41538 (patch) | |
tree | cf28f8848aed5fa3f641e8f76380469500efa852 | |
parent | c12229d169345b5ea6fc69cfc4f23d57ac3a9fcc (diff) | |
download | chromium_src-08a6f99997bad56869412e1da065acda3dc41538.zip chromium_src-08a6f99997bad56869412e1da065acda3dc41538.tar.gz chromium_src-08a6f99997bad56869412e1da065acda3dc41538.tar.bz2 |
[Invalidations] Add GetInvalidatorState() to Invalidator{,Frontend}
Combine OnNotifications{Enabled,Disabled}() into OnInvalidatorStateChange(). Replace NotificationsDisabledReason with InvalidatorState.
Rename OnIncomingNotification to OnIncomingInvalidation. Also change
some references of "notification" to "invalidation".
Set the initial invalidator state in ChromeToMobileService. Also remove OnNotificationsEnabled() call from OnIncomingInvalidation().
Instantiate InvalidatorTest template for ProfileSyncService.
Move comments for invalidation-related functions from ProfileSyncService to InvalidatorFrontend.
Put DISALLOW_COPY_AND_ASSIGN on some classes.
Fix comment in invalidator.h.
BUG=142475
TBR=brettw@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10916131
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@155448 0039d316-1c4b-4281-b951-d872f2087c98
56 files changed, 1031 insertions, 983 deletions
diff --git a/chrome/browser/chrome_to_mobile_service.cc b/chrome/browser/chrome_to_mobile_service.cc index af269af..8e282ef 100644 --- a/chrome/browser/chrome_to_mobile_service.cc +++ b/chrome/browser/chrome_to_mobile_service.cc @@ -183,8 +183,10 @@ ChromeToMobileService::ChromeToMobileService(Profile* profile) if (profile_sync_service) { CloudPrintURL cloud_print_url(profile_); cloud_print_url_ = cloud_print_url.GetCloudPrintServiceURL(); + sync_invalidation_enabled_ = + (profile_sync_service->GetInvalidatorState() == + syncer::INVALIDATIONS_ENABLED); // Register for cloud print device list invalidation notifications. - // TODO(msw|akalin): Initialize |sync_invalidation_enabled_| properly. profile_sync_service->RegisterInvalidationHandler(this); syncer::ObjectIdSet ids; ids.insert(invalidation::ObjectId( @@ -363,27 +365,20 @@ void ChromeToMobileService::OnGetTokenFailure( this, &ChromeToMobileService::RequestAccessToken); } -void ChromeToMobileService::OnNotificationsEnabled() { - sync_invalidation_enabled_ = true; +void ChromeToMobileService::OnInvalidatorStateChange( + syncer::InvalidatorState state) { + sync_invalidation_enabled_ = (state == syncer::INVALIDATIONS_ENABLED); UpdateCommandState(); } -void ChromeToMobileService::OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { - sync_invalidation_enabled_ = false; - UpdateCommandState(); -} - -void ChromeToMobileService::OnIncomingNotification( +void ChromeToMobileService::OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { + syncer::IncomingInvalidationSource source) { DCHECK_EQ(1U, id_state_map.size()); DCHECK_EQ(1U, id_state_map.count(invalidation::ObjectId( ipc::invalidation::ObjectSource::CHROME_COMPONENTS, kSyncInvalidationObjectIdChromeToMobileDeviceList))); RequestDeviceSearch(); - // TODO(msw|akalin): This may not necessarily mean notifications are enabled. - OnNotificationsEnabled(); } const std::string& ChromeToMobileService::GetAccessTokenForTest() const { diff --git a/chrome/browser/chrome_to_mobile_service.h b/chrome/browser/chrome_to_mobile_service.h index 89b8d4d..21245d7 100644 --- a/chrome/browser/chrome_to_mobile_service.h +++ b/chrome/browser/chrome_to_mobile_service.h @@ -156,12 +156,11 @@ class ChromeToMobileService : public ProfileKeyedService, virtual void OnGetTokenFailure(const GoogleServiceAuthError& error) OVERRIDE; // syncer::InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange( + syncer::InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) OVERRIDE; + syncer::IncomingInvalidationSource source) OVERRIDE; // Expose access token accessors for test purposes. const std::string& GetAccessTokenForTest() const; diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc index d232019..882dfe8 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc @@ -112,20 +112,15 @@ void PushMessagingInvalidationHandler::UnregisterExtension( UpdateRegistrations(); } -void PushMessagingInvalidationHandler::OnNotificationsEnabled() { +void PushMessagingInvalidationHandler::OnInvalidatorStateChange( + syncer::InvalidatorState state) { DCHECK(thread_checker_.CalledOnValidThread()); // Nothing to do. } -void PushMessagingInvalidationHandler::OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { - DCHECK(thread_checker_.CalledOnValidThread()); - // Nothing to do. -} - -void PushMessagingInvalidationHandler::OnIncomingNotification( +void PushMessagingInvalidationHandler::OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { + syncer::IncomingInvalidationSource source) { DCHECK(thread_checker_.CalledOnValidThread()); for (syncer::ObjectIdStateMap::const_iterator it = id_state_map.begin(); it != id_state_map.end(); ++it) { diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h index 91182f38..52ec385 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h @@ -40,12 +40,11 @@ class PushMessagingInvalidationHandler : public PushMessagingInvalidationMapper, virtual void UnregisterExtension(const std::string& extension_id) OVERRIDE; // InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange( + syncer::InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) OVERRIDE; + syncer::IncomingInvalidationSource source) OVERRIDE; const std::set<std::string>& GetRegisteredExtensionsForTest() const { return registered_extensions_; diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc index d6377c8..1db7eaf 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc @@ -36,6 +36,7 @@ class MockInvalidationFrontend : public InvalidationFrontend { void(syncer::InvalidationHandler*, const syncer::ObjectIdSet&)); MOCK_METHOD1(UnregisterInvalidationHandler, void(syncer::InvalidationHandler*)); + MOCK_CONST_METHOD0(GetInvalidatorState, syncer::InvalidatorState()); private: DISALLOW_COPY_AND_ASSIGN(MockInvalidationFrontend); @@ -163,8 +164,8 @@ TEST_F(PushMessagingInvalidationHandlerTest, Dispatch) { OnMessage("dddddddddddddddddddddddddddddddd", 0, "payload")); EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 3, "payload")); - handler_->OnIncomingNotification(ObjectIdSetToStateMap(ids, "payload"), - syncer::REMOTE_NOTIFICATION); + handler_->OnIncomingInvalidation(ObjectIdSetToStateMap(ids, "payload"), + syncer::REMOTE_INVALIDATION); } // Tests that malformed object IDs don't trigger spurious callbacks. @@ -194,8 +195,8 @@ TEST_F(PushMessagingInvalidationHandlerTest, DispatchInvalidObjectIds) { ids.insert(invalidation::ObjectId( kSourceId, "U/dddddddddddddddddddddddddddddddd/4")); - handler_->OnIncomingNotification(ObjectIdSetToStateMap(ids, "payload"), - syncer::REMOTE_NOTIFICATION); + handler_->OnIncomingInvalidation(ObjectIdSetToStateMap(ids, "payload"), + syncer::REMOTE_INVALIDATION); } } // namespace extensions diff --git a/chrome/browser/sync/glue/bridged_invalidator.cc b/chrome/browser/sync/glue/bridged_invalidator.cc index c85c70b..0146ad4 100644 --- a/chrome/browser/sync/glue/bridged_invalidator.cc +++ b/chrome/browser/sync/glue/bridged_invalidator.cc @@ -10,8 +10,11 @@ namespace browser_sync { BridgedInvalidator::BridgedInvalidator( ChromeSyncNotificationBridge* bridge, - syncer::Invalidator* delegate) - : bridge_(bridge), delegate_(delegate) { + syncer::Invalidator* delegate, + syncer::InvalidatorState default_invalidator_state) + : bridge_(bridge), + delegate_(delegate), + default_invalidator_state_(default_invalidator_state) { DCHECK(bridge_); } @@ -33,6 +36,13 @@ void BridgedInvalidator::UpdateRegisteredIds( bridge_->UpdateRegisteredIds(handler, ids); } +syncer::InvalidatorState BridgedInvalidator::GetInvalidatorState() const { + return + delegate_.get() ? + delegate_->GetInvalidatorState() : + default_invalidator_state_; +} + void BridgedInvalidator::UnregisterHandler( syncer::InvalidationHandler* handler) { if (delegate_.get()) @@ -56,10 +66,10 @@ void BridgedInvalidator::UpdateCredentials( delegate_->UpdateCredentials(email, token); } -void BridgedInvalidator::SendNotification( +void BridgedInvalidator::SendInvalidation( const syncer::ObjectIdStateMap& id_state_map) { if (delegate_.get()) - delegate_->SendNotification(id_state_map); + delegate_->SendInvalidation(id_state_map); } } // namespace browser_sync diff --git a/chrome/browser/sync/glue/bridged_invalidator.h b/chrome/browser/sync/glue/bridged_invalidator.h index 018e876..63d0ea4 100644 --- a/chrome/browser/sync/glue/bridged_invalidator.h +++ b/chrome/browser/sync/glue/bridged_invalidator.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_SYNC_GLUE_BRIDGED_INVALIDATOR_H_ #define CHROME_BROWSER_SYNC_GLUE_BRIDGED_INVALIDATOR_H_ +#include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "sync/notifier/invalidator.h" @@ -21,10 +22,12 @@ class ChromeSyncNotificationBridge; // and unregistered with the ChromeSyncNotificationBridge, respectively. class BridgedInvalidator : public syncer::Invalidator { public: - // Does not take ownership of |bridge|. Takes ownership of |delegate|. - // |delegate| may be NULL. + // Does not take ownership of |bridge|. Takes ownership of + // |delegate|. |delegate| may be NULL. |default_invalidator_state| + // is used by GetInvalidatorState() if |delegate| is NULL. BridgedInvalidator(ChromeSyncNotificationBridge* bridge, - syncer::Invalidator* delegate); + syncer::Invalidator* delegate, + syncer::InvalidatorState default_invalidator_state); virtual ~BridgedInvalidator(); // Invalidator implementation. Passes through all calls to the delegate. @@ -34,19 +37,24 @@ class BridgedInvalidator : public syncer::Invalidator { virtual void UpdateRegisteredIds(syncer::InvalidationHandler * handler, const syncer::ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(syncer::InvalidationHandler* handler) OVERRIDE; + virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; virtual void SetStateDeprecated(const std::string& state) OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; - virtual void SendNotification( + virtual void SendInvalidation( const syncer::ObjectIdStateMap& id_state_map) OVERRIDE; private: // The notification bridge that we register the observers with. - ChromeSyncNotificationBridge* bridge_; + ChromeSyncNotificationBridge* const bridge_; // The delegate we are wrapping. - scoped_ptr<syncer::Invalidator> delegate_; + const scoped_ptr<syncer::Invalidator> delegate_; + + const syncer::InvalidatorState default_invalidator_state_; + + DISALLOW_COPY_AND_ASSIGN(BridgedInvalidator); }; } // namespace browser_sync diff --git a/chrome/browser/sync/glue/bridged_invalidator_unittest.cc b/chrome/browser/sync/glue/bridged_invalidator_unittest.cc index 139f360..ecd8084 100644 --- a/chrome/browser/sync/glue/bridged_invalidator_unittest.cc +++ b/chrome/browser/sync/glue/bridged_invalidator_unittest.cc @@ -52,7 +52,8 @@ class BridgedInvalidatorTestDelegate { DCHECK(!fake_delegate_); DCHECK(!invalidator_.get()); fake_delegate_ = new syncer::FakeInvalidator(); - invalidator_.reset(new BridgedInvalidator(&bridge_, fake_delegate_)); + invalidator_.reset(new BridgedInvalidator( + &bridge_, fake_delegate_, syncer::DEFAULT_INVALIDATION_ERROR)); } BridgedInvalidator* GetInvalidator() { @@ -78,19 +79,14 @@ class BridgedInvalidatorTestDelegate { // Do nothing. } - void TriggerOnNotificationsEnabled() { - fake_delegate_->EmitOnNotificationsEnabled(); + void TriggerOnInvalidatorStateChange(syncer::InvalidatorState state) { + fake_delegate_->EmitOnInvalidatorStateChange(state); } - void TriggerOnIncomingNotification( + void TriggerOnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { - fake_delegate_->EmitOnIncomingNotification(id_state_map, source); - } - - void TriggerOnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { - fake_delegate_->EmitOnNotificationsDisabled(reason); + syncer::IncomingInvalidationSource source) { + fake_delegate_->EmitOnIncomingInvalidation(id_state_map, source); } static bool InvalidatorHandlesDeprecatedState() { @@ -126,7 +122,7 @@ class BridgedInvalidatorTest : public testing::Test { BridgedInvalidatorTestDelegate delegate_; }; -TEST_F(BridgedInvalidatorTest, RegisterHandler) { +TEST_F(BridgedInvalidatorTest, HandlerMethods) { syncer::ObjectIdSet ids; ids.insert(invalidation::ObjectId(1, "id1")); @@ -145,6 +141,11 @@ TEST_F(BridgedInvalidatorTest, RegisterHandler) { EXPECT_FALSE(delegate_.GetBridge()->IsHandlerRegisteredForTest(&handler)); } +TEST_F(BridgedInvalidatorTest, GetInvalidatorState) { + EXPECT_EQ(syncer::DEFAULT_INVALIDATION_ERROR, + delegate_.GetInvalidator()->GetInvalidatorState()); +} + TEST_F(BridgedInvalidatorTest, SetUniqueId) { const std::string& unique_id = "unique id"; delegate_.GetInvalidator()->SetUniqueId(unique_id); @@ -165,13 +166,13 @@ TEST_F(BridgedInvalidatorTest, UpdateCredentials) { EXPECT_EQ(token, delegate_.GetFakeDelegate()->GetCredentialsToken()); } -TEST_F(BridgedInvalidatorTest, SendNotification) { +TEST_F(BridgedInvalidatorTest, SendInvalidation) { syncer::ObjectIdSet ids; ids.insert(invalidation::ObjectId(1, "id1")); ids.insert(invalidation::ObjectId(2, "id2")); const syncer::ObjectIdStateMap& id_state_map = syncer::ObjectIdSetToStateMap(ids, "payload"); - delegate_.GetInvalidator()->SendNotification(id_state_map); + delegate_.GetInvalidator()->SendInvalidation(id_state_map); EXPECT_THAT(id_state_map, Eq(delegate_.GetFakeDelegate()->GetLastSentIdStateMap())); } diff --git a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc index 371c9c1..933cbc9 100644 --- a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc +++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc @@ -36,9 +36,9 @@ class ChromeSyncNotificationBridge::Core const syncer::ObjectIdSet& ids); void UnregisterHandler(syncer::InvalidationHandler* handler); - void EmitNotification( + void EmitInvalidation( const syncer::ObjectIdStateMap& state_map, - syncer::IncomingNotificationSource notification_source); + syncer::IncomingInvalidationSource invalidation_source); bool IsHandlerRegisteredForTest(syncer::InvalidationHandler* handler) const; syncer::ObjectIdSet GetRegisteredIdsForTest( @@ -96,9 +96,9 @@ void ChromeSyncNotificationBridge::Core::UnregisterHandler( invalidator_registrar_->UnregisterHandler(handler); } -void ChromeSyncNotificationBridge::Core::EmitNotification( +void ChromeSyncNotificationBridge::Core::EmitInvalidation( const syncer::ObjectIdStateMap& state_map, - syncer::IncomingNotificationSource notification_source) { + syncer::IncomingInvalidationSource invalidation_source) { DCHECK(sync_task_runner_->RunsTasksOnCurrentThread()); const syncer::ObjectIdStateMap& effective_state_map = state_map.empty() ? @@ -107,7 +107,7 @@ void ChromeSyncNotificationBridge::Core::EmitNotification( state_map; invalidator_registrar_->DispatchInvalidationsToHandlers( - effective_state_map, notification_source); + effective_state_map, invalidation_source); } bool ChromeSyncNotificationBridge::Core::IsHandlerRegisteredForTest( @@ -187,13 +187,13 @@ void ChromeSyncNotificationBridge::Observe( const content::NotificationDetails& details) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - syncer::IncomingNotificationSource notification_source; + syncer::IncomingInvalidationSource invalidation_source; if (type == chrome::NOTIFICATION_SYNC_REFRESH_LOCAL) { - notification_source = syncer::LOCAL_NOTIFICATION; + invalidation_source = syncer::LOCAL_INVALIDATION; } else if (type == chrome::NOTIFICATION_SYNC_REFRESH_REMOTE) { - notification_source = syncer::REMOTE_NOTIFICATION; + invalidation_source = syncer::REMOTE_INVALIDATION; } else { - NOTREACHED() << "Unexpected notification type: " << type; + NOTREACHED() << "Unexpected invalidation type: " << type; return; } @@ -205,10 +205,10 @@ void ChromeSyncNotificationBridge::Observe( const syncer::ModelTypeStateMap& state_map = *(state_details.ptr()); if (!sync_task_runner_->PostTask( FROM_HERE, - base::Bind(&Core::EmitNotification, + base::Bind(&Core::EmitInvalidation, core_, ModelTypeStateMapToObjectIdStateMap(state_map), - notification_source))) { + invalidation_source))) { NOTREACHED(); } } diff --git a/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc b/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc index f9250bf..382e63c 100644 --- a/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc +++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc @@ -69,7 +69,7 @@ class ChromeSyncNotificationBridgeTest : public testing::Test { void VerifyAndDestroyObserver( const syncer::ModelTypeStateMap& expected_states, - syncer::IncomingNotificationSource expected_source) { + syncer::IncomingInvalidationSource expected_source) { ASSERT_TRUE(sync_thread_.message_loop_proxy()->PostTask( FROM_HERE, base::Bind(&ChromeSyncNotificationBridgeTest:: @@ -113,15 +113,15 @@ class ChromeSyncNotificationBridgeTest : public testing::Test { private: void VerifyAndDestroyObserverOnSyncThread( const syncer::ModelTypeStateMap& expected_states, - syncer::IncomingNotificationSource expected_source) { + syncer::IncomingInvalidationSource expected_source) { DCHECK(sync_thread_.message_loop_proxy()->RunsTasksOnCurrentThread()); if (sync_handler_.get()) { sync_handler_notification_success_ = - (sync_handler_->GetNotificationCount() == 1) && + (sync_handler_->GetInvalidationCount() == 1) && ObjectIdStateMapEquals( - sync_handler_->GetLastNotificationIdStateMap(), + sync_handler_->GetLastInvalidationIdStateMap(), syncer::ModelTypeStateMapToObjectIdStateMap(expected_states)) && - (sync_handler_->GetLastNotificationSource() == expected_source); + (sync_handler_->GetLastInvalidationSource() == expected_source); bridge_->UnregisterHandler(sync_handler_.get()); } else { sync_handler_notification_success_ = false; @@ -164,7 +164,7 @@ class ChromeSyncNotificationBridgeTest : public testing::Test { }; // Adds an observer on the sync thread, triggers a local refresh -// notification, and ensures the bridge posts a LOCAL_NOTIFICATION +// invalidation, and ensures the bridge posts a LOCAL_INVALIDATION // with the proper state to it. TEST_F(ChromeSyncNotificationBridgeTest, LocalNotification) { syncer::ModelTypeStateMap state_map; @@ -174,11 +174,11 @@ TEST_F(ChromeSyncNotificationBridgeTest, LocalNotification) { UpdateEnabledTypes(syncer::ModelTypeSet(syncer::SESSIONS)); TriggerRefreshNotification(chrome::NOTIFICATION_SYNC_REFRESH_LOCAL, state_map); - VerifyAndDestroyObserver(state_map, syncer::LOCAL_NOTIFICATION); + VerifyAndDestroyObserver(state_map, syncer::LOCAL_INVALIDATION); } // Adds an observer on the sync thread, triggers a remote refresh -// notification, and ensures the bridge posts a REMOTE_NOTIFICATION +// invalidation, and ensures the bridge posts a REMOTE_INVALIDATION // with the proper state to it. TEST_F(ChromeSyncNotificationBridgeTest, RemoteNotification) { syncer::ModelTypeStateMap state_map; @@ -188,12 +188,12 @@ TEST_F(ChromeSyncNotificationBridgeTest, RemoteNotification) { UpdateEnabledTypes(syncer::ModelTypeSet(syncer::SESSIONS)); TriggerRefreshNotification(chrome::NOTIFICATION_SYNC_REFRESH_REMOTE, state_map); - VerifyAndDestroyObserver(state_map, syncer::REMOTE_NOTIFICATION); + VerifyAndDestroyObserver(state_map, syncer::REMOTE_INVALIDATION); } // Adds an observer on the sync thread, triggers a local refresh // notification with empty state map and ensures the bridge posts a -// LOCAL_NOTIFICATION with the proper state to it. +// LOCAL_INVALIDATION with the proper state to it. TEST_F(ChromeSyncNotificationBridgeTest, LocalNotificationEmptyPayloadMap) { const syncer::ModelTypeSet enabled_types( syncer::BOOKMARKS, syncer::PASSWORDS); @@ -204,12 +204,12 @@ TEST_F(ChromeSyncNotificationBridgeTest, LocalNotificationEmptyPayloadMap) { TriggerRefreshNotification(chrome::NOTIFICATION_SYNC_REFRESH_LOCAL, syncer::ModelTypeStateMap()); VerifyAndDestroyObserver( - enabled_types_state_map, syncer::LOCAL_NOTIFICATION); + enabled_types_state_map, syncer::LOCAL_INVALIDATION); } // Adds an observer on the sync thread, triggers a remote refresh // notification with empty state map and ensures the bridge posts a -// REMOTE_NOTIFICATION with the proper state to it. +// REMOTE_INVALIDATION with the proper state to it. TEST_F(ChromeSyncNotificationBridgeTest, RemoteNotificationEmptyPayloadMap) { const syncer::ModelTypeSet enabled_types( syncer::BOOKMARKS, syncer::TYPED_URLS); @@ -220,7 +220,7 @@ TEST_F(ChromeSyncNotificationBridgeTest, RemoteNotificationEmptyPayloadMap) { TriggerRefreshNotification(chrome::NOTIFICATION_SYNC_REFRESH_REMOTE, syncer::ModelTypeStateMap()); VerifyAndDestroyObserver( - enabled_types_state_map, syncer::REMOTE_NOTIFICATION); + enabled_types_state_map, syncer::REMOTE_INVALIDATION); } } // namespace diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index cae3a90..8341618 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -21,6 +21,7 @@ #include "base/timer.h" #include "base/tracked_objects.h" #include "base/utf_string_conversions.h" +#include "build/build_config.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/token_service.h" #include "chrome/browser/sync/glue/bridged_invalidator.h" @@ -120,12 +121,11 @@ class SyncBackendHost::Core virtual void OnPassphraseStateChanged(syncer::PassphraseState state) OVERRIDE; // syncer::InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange( + syncer::InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) OVERRIDE; + syncer::IncomingInvalidationSource source) OVERRIDE; // Note: // @@ -1038,32 +1038,24 @@ void SyncBackendHost::Core::OnActionableError( sync_error); } -void SyncBackendHost::Core::OnNotificationsEnabled() { - if (!sync_loop_) - return; - DCHECK_EQ(MessageLoop::current(), sync_loop_); - host_.Call(FROM_HERE, - &SyncBackendHost::HandleNotificationsEnabledOnFrontendLoop); -} - -void SyncBackendHost::Core::OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { +void SyncBackendHost::Core::OnInvalidatorStateChange( + syncer::InvalidatorState state) { if (!sync_loop_) return; DCHECK_EQ(MessageLoop::current(), sync_loop_); host_.Call(FROM_HERE, - &SyncBackendHost::HandleNotificationsDisabledOnFrontendLoop, - reason); + &SyncBackendHost::HandleInvalidatorStateChangeOnFrontendLoop, + state); } -void SyncBackendHost::Core::OnIncomingNotification( +void SyncBackendHost::Core::OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { + syncer::IncomingInvalidationSource source) { if (!sync_loop_) return; DCHECK_EQ(MessageLoop::current(), sync_loop_); host_.Call(FROM_HERE, - &SyncBackendHost::HandleIncomingNotificationOnFrontendLoop, + &SyncBackendHost::HandleIncomingInvalidationOnFrontendLoop, id_state_map, source); } @@ -1092,6 +1084,15 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) { chrome_sync_notification_bridge_ = options.chrome_sync_notification_bridge; DCHECK(chrome_sync_notification_bridge_); +#if defined(OS_ANDROID) + // Android uses ChromeSyncNotificationBridge exclusively. + const syncer::InvalidatorState kDefaultInvalidatorState = + syncer::INVALIDATIONS_ENABLED; +#else + const syncer::InvalidatorState kDefaultInvalidatorState = + syncer::DEFAULT_INVALIDATION_ERROR; +#endif + sync_manager_ = options.sync_manager_factory->CreateSyncManager(name_); sync_manager_->AddObserver(this); sync_manager_->Init( @@ -1108,7 +1109,8 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) { options.credentials, scoped_ptr<syncer::Invalidator>(new BridgedInvalidator( options.chrome_sync_notification_bridge, - options.invalidator_factory->CreateInvalidator())), + options.invalidator_factory->CreateInvalidator(), + kDefaultInvalidatorState)), options.restored_key_for_bootstrapping, options.restored_keystore_key_for_bootstrapping, scoped_ptr<InternalComponentsFactory>( @@ -1430,28 +1432,21 @@ void SyncBackendHost::HandleActionableErrorEventOnFrontendLoop( frontend_->OnActionableError(sync_error); } -void SyncBackendHost::HandleNotificationsEnabledOnFrontendLoop() { - if (!frontend_) - return; - DCHECK_EQ(MessageLoop::current(), frontend_loop_); - frontend_->OnNotificationsEnabled(); -} - -void SyncBackendHost::HandleNotificationsDisabledOnFrontendLoop( - syncer::NotificationsDisabledReason reason) { +void SyncBackendHost::HandleInvalidatorStateChangeOnFrontendLoop( + syncer::InvalidatorState state) { if (!frontend_) return; DCHECK_EQ(MessageLoop::current(), frontend_loop_); - frontend_->OnNotificationsDisabled(reason); + frontend_->OnInvalidatorStateChange(state); } -void SyncBackendHost::HandleIncomingNotificationOnFrontendLoop( +void SyncBackendHost::HandleIncomingInvalidationOnFrontendLoop( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { + syncer::IncomingInvalidationSource source) { if (!frontend_) return; DCHECK_EQ(MessageLoop::current(), frontend_loop_); - frontend_->OnIncomingNotification(id_state_map, source); + frontend_->OnIncomingInvalidation(id_state_map, source); } bool SyncBackendHost::CheckPassphraseAgainstCachedPendingKeys( diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 872af93..f632c16 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -466,12 +466,11 @@ class SyncBackendHost : public BackendDataTypeConfigurer { syncer::ModelTypeSet failed_configuration_types); // syncer::InvalidationHandler-like functions. - void HandleNotificationsEnabledOnFrontendLoop(); - void HandleNotificationsDisabledOnFrontendLoop( - syncer::NotificationsDisabledReason reason); - void HandleIncomingNotificationOnFrontendLoop( + void HandleInvalidatorStateChangeOnFrontendLoop( + syncer::InvalidatorState state); + void HandleIncomingInvalidationOnFrontendLoop( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source); + syncer::IncomingInvalidationSource source); // Handles stopping the core's SyncManager, accounting for whether // initialization is done yet. diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc index 5a4ae88..945de64 100644 --- a/chrome/browser/sync/glue/sync_backend_host_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc @@ -23,7 +23,7 @@ #include "sync/internal_api/public/sync_manager_factory.h" #include "sync/internal_api/public/test/fake_sync_manager.h" #include "sync/internal_api/public/util/experiments.h" -#include "sync/notifier/notifications_disabled_reason.h" +#include "sync/notifier/invalidator_state.h" #include "sync/notifier/object_id_state_map_test_util.h" #include "sync/protocol/encryption.pb.h" #include "sync/protocol/sync_protocol_error.h" @@ -58,12 +58,11 @@ class MockSyncFrontend : public SyncFrontend { public: virtual ~MockSyncFrontend() {} - MOCK_METHOD0(OnNotificationsEnabled, void()); - MOCK_METHOD1(OnNotificationsDisabled, - void(syncer::NotificationsDisabledReason)); - MOCK_METHOD2(OnIncomingNotification, + MOCK_METHOD1(OnInvalidatorStateChange, + void(syncer::InvalidatorState)); + MOCK_METHOD2(OnIncomingInvalidation, void(const syncer::ObjectIdStateMap&, - syncer::IncomingNotificationSource)); + syncer::IncomingInvalidationSource)); MOCK_METHOD2(OnBackendInitialized, void(const syncer::WeakHandle<syncer::JsBackend>&, bool)); MOCK_METHOD0(OnSyncCycleCompleted, void()); @@ -576,55 +575,38 @@ TEST_F(SyncBackendHostTest, Invalidate) { EXPECT_CALL( mock_frontend_, - OnIncomingNotification(id_state_map, syncer::REMOTE_NOTIFICATION)) + OnIncomingInvalidation(id_state_map, syncer::REMOTE_INVALIDATION)) .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); backend_->UpdateRegisteredInvalidationIds(ids); - fake_manager_->Invalidate(id_state_map, syncer::REMOTE_NOTIFICATION); + fake_manager_->Invalidate(id_state_map, syncer::REMOTE_INVALIDATION); ui_loop_.PostDelayedTask( FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); ui_loop_.Run(); } -// Register for some IDs and turn on notifications. This should -// propagate all the way to the frontend. -TEST_F(SyncBackendHostTest, EnableNotifications) { - InitializeBackend(); - - EXPECT_CALL(mock_frontend_, OnNotificationsEnabled()) - .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); - - syncer::ObjectIdSet ids; - ids.insert(invalidation::ObjectId(3, "id3")); - backend_->UpdateRegisteredInvalidationIds(ids); - fake_manager_->EnableNotifications(); - ui_loop_.PostDelayedTask( - FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); - ui_loop_.Run(); -} - -// Register for some IDs and turn off notifications. This should -// propagate all the way to the frontend. -TEST_F(SyncBackendHostTest, DisableNotifications) { +// Register for some IDs and update the invalidator state. This +// should propagate all the way to the frontend. +TEST_F(SyncBackendHostTest, UpdateInvalidatorState) { InitializeBackend(); EXPECT_CALL(mock_frontend_, - OnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR)) + OnInvalidatorStateChange(syncer::INVALIDATIONS_ENABLED)) .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); syncer::ObjectIdSet ids; - ids.insert(invalidation::ObjectId(4, "id4")); + ids.insert(invalidation::ObjectId(3, "id3")); backend_->UpdateRegisteredInvalidationIds(ids); - fake_manager_->DisableNotifications(syncer::TRANSIENT_NOTIFICATION_ERROR); + fake_manager_->UpdateInvalidatorState(syncer::INVALIDATIONS_ENABLED); ui_loop_.PostDelayedTask( FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); ui_loop_.Run(); } -// Call StopSyncingForShutdown() on the backend and fire some notifications +// Call StopSyncingForShutdown() on the backend and fire some invalidations // before calling Shutdown(). Then start up and shut down the backend again. // Those notifications shouldn't propagate to the frontend. -TEST_F(SyncBackendHostTest, NotificationsAfterStopSyncingForShutdown) { +TEST_F(SyncBackendHostTest, InvalidationsAfterStopSyncingForShutdown) { InitializeBackend(); syncer::ObjectIdSet ids; @@ -634,11 +616,11 @@ TEST_F(SyncBackendHostTest, NotificationsAfterStopSyncingForShutdown) { backend_->StopSyncingForShutdown(); // Should not trigger anything. - fake_manager_->DisableNotifications(syncer::TRANSIENT_NOTIFICATION_ERROR); - fake_manager_->EnableNotifications(); + fake_manager_->UpdateInvalidatorState(syncer::TRANSIENT_INVALIDATION_ERROR); + fake_manager_->UpdateInvalidatorState(syncer::INVALIDATIONS_ENABLED); const syncer::ObjectIdStateMap& id_state_map = syncer::ObjectIdSetToStateMap(ids, "payload"); - fake_manager_->Invalidate(id_state_map, syncer::REMOTE_NOTIFICATION); + fake_manager_->Invalidate(id_state_map, syncer::REMOTE_INVALIDATION); // Make sure the above calls take effect before we continue. fake_manager_->WaitForSyncThread(); diff --git a/chrome/browser/sync/invalidation_frontend.h b/chrome/browser/sync/invalidation_frontend.h index 87d6a97..3076b61 100644 --- a/chrome/browser/sync/invalidation_frontend.h +++ b/chrome/browser/sync/invalidation_frontend.h @@ -13,6 +13,43 @@ class InvalidationHandler; // Interface for classes that handle invalidation registrations and send out // invalidations to register handlers. +// +// Invalidation clients should follow the pattern below: +// +// When starting the client: +// +// frontend->RegisterInvalidationHandler(client_handler); +// +// When the set of IDs to register changes for the client during its lifetime +// (i.e., between calls to RegisterInvalidationHandler(client_handler) and +// UnregisterInvalidationHandler(client_handler): +// +// frontend->UpdateRegisteredInvalidationIds(client_handler, client_ids); +// +// When shutting down the client for browser shutdown: +// +// frontend->UnregisterInvalidationHandler(client_handler); +// +// Note that there's no call to UpdateRegisteredIds() -- this is because the +// invalidation API persists registrations across browser restarts. +// +// When permanently shutting down the client, e.g. when disabling the related +// feature: +// +// frontend->UpdateRegisteredInvalidationIds(client_handler, ObjectIdSet()); +// frontend->UnregisterInvalidationHandler(client_handler); +// +// If an invalidation handler cares about the invalidator state, it should also +// do the following when starting the client: +// +// invalidator_state = frontend->GetInvalidatorState(); +// +// It can also do the above in OnInvalidatorStateChange(), or it can use the +// argument to OnInvalidatorStateChange(). +// +// NOTE(akalin): Invalidations that come in during browser shutdown may get +// dropped. This won't matter once we have an Acknowledge API, though: see +// http://crbug.com/78462 and http://crbug.com/124149. class InvalidationFrontend { public: // Starts sending notifications to |handler|. |handler| must not be NULL, @@ -39,6 +76,11 @@ class InvalidationFrontend { virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) = 0; + // Returns the current invalidator state. When called from within + // InvalidationHandler::OnInvalidatorStateChange(), this must return + // the updated state. + virtual syncer::InvalidatorState GetInvalidatorState() const = 0; + protected: virtual ~InvalidationFrontend() { } }; diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 2b47345..ebac81c 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -467,6 +467,10 @@ void ProfileSyncService::UnregisterInvalidationHandler( invalidator_registrar_.UnregisterHandler(handler); } +syncer::InvalidatorState ProfileSyncService::GetInvalidatorState() const { + return invalidator_registrar_.GetInvalidatorState(); +} + void ProfileSyncService::Shutdown() { ShutdownImpl(false); } @@ -659,18 +663,14 @@ void ProfileSyncService::DisableBrokenDatatype( weak_factory_.GetWeakPtr())); } -void ProfileSyncService::OnNotificationsEnabled() { - invalidator_registrar_.EmitOnNotificationsEnabled(); -} - -void ProfileSyncService::OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { - invalidator_registrar_.EmitOnNotificationsDisabled(reason); +void ProfileSyncService::OnInvalidatorStateChange( + syncer::InvalidatorState state) { + invalidator_registrar_.UpdateInvalidatorState(state); } -void ProfileSyncService::OnIncomingNotification( +void ProfileSyncService::OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) { + syncer::IncomingInvalidationSource source) { invalidator_registrar_.DispatchInvalidationsToHandlers(id_state_map, source); } diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index e6c8439..216ef1c 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -259,12 +259,11 @@ class ProfileSyncService : public browser_sync::SyncFrontend, virtual void SetSyncSetupCompleted(); // syncer::InvalidationHandler implementation (via SyncFrontend). - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange( + syncer::InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - syncer::IncomingNotificationSource source) OVERRIDE; + syncer::IncomingInvalidationSource source) OVERRIDE; // SyncFrontend implementation. virtual void OnBackendInitialized( @@ -563,58 +562,15 @@ class ProfileSyncService : public browser_sync::SyncFrontend, // been cleared yet. Virtual for testing purposes. virtual bool waiting_for_auth() const; - // Invalidation clients should follow the pattern below: - // - // When starting the client: - // - // pss->RegisterInvalidationHandler(client_handler); - // - // When the set of IDs to register changes for the client during its lifetime - // (i.e., between calls to RegisterInvalidationHandler(client_handler) and - // UnregisterInvalidationHandler(client_handler): - // - // pss->UpdateRegisteredInvalidationIds(client_handler, client_ids); - // - // When shutting down the client for browser shutdown: - // - // pss->UnregisterInvalidationHandler(client_handler); - // - // Note that there's no call to UpdateRegisteredIds() -- this is because the - // invalidation API persists registrations across browser restarts. - // - // When permanently shutting down the client, e.g. when disabling the related - // feature: - // - // pss->UpdateRegisteredInvalidationIds(client_handler, ObjectIdSet()); - // pss->UnregisterInvalidationHandler(client_handler); - - // NOTE(akalin): Invalidations that come in during browser shutdown may get - // dropped. This won't matter once we have an Acknowledge API, though: see - // http://crbug.com/78462 and http://crbug.com/124149. - - // Starts sending notifications to |handler|. |handler| must not be NULL, - // and it must already be registered. - // - // Handler registrations are persisted across restarts of sync. + // InvalidationFrontend implementation. virtual void RegisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; - - // Updates the set of ObjectIds associated with |handler|. |handler| must - // not be NULL, and must already be registered. An ID must be registered for - // at most one handler. - // - // Registered IDs are persisted across restarts of sync. virtual void UpdateRegisteredInvalidationIds( syncer::InvalidationHandler* handler, const syncer::ObjectIdSet& ids) OVERRIDE; - - // Stops sending notifications to |handler|. |handler| must not be NULL, and - // it must already be registered. Note that this doesn't unregister the IDs - // associated with |handler|. - // - // Handler registrations are persisted across restarts of sync. virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; + virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; // ProfileKeyedService implementation. virtual void Shutdown() OVERRIDE; diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index 1671d52..9a2c173 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" @@ -26,6 +28,8 @@ #include "sync/js/js_event_details.h" #include "sync/js/js_test_util.h" #include "sync/notifier/fake_invalidation_handler.h" +#include "sync/notifier/invalidator.h" +#include "sync/notifier/invalidator_test_template.h" #include "sync/notifier/object_id_state_map_test_util.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -47,21 +51,21 @@ using testing::Mock; using testing::Return; using testing::StrictMock; -class ProfileSyncServiceTest : public testing::Test { - protected: - ProfileSyncServiceTest() +class ProfileSyncServiceTestHarness { + public: + ProfileSyncServiceTestHarness() : ui_thread_(BrowserThread::UI, &ui_loop_), db_thread_(BrowserThread::DB), file_thread_(BrowserThread::FILE), io_thread_(BrowserThread::IO) {} - virtual ~ProfileSyncServiceTest() {} + ~ProfileSyncServiceTestHarness() {} - virtual void SetUp() { + void SetUp() { file_thread_.Start(); io_thread_.StartIOThread(); - profile_.reset(new TestingProfile()); - profile_->CreateRequestContext(); + profile.reset(new TestingProfile()); + profile->CreateRequestContext(); // We need to set the user agent before the backend host can call // webkit_glue::GetUserAgent(). @@ -69,11 +73,11 @@ class ProfileSyncServiceTest : public testing::Test { false); } - virtual void TearDown() { + void TearDown() { // Kill the service before the profile. - service_.reset(); - profile_.reset(); - // Pump messages posted by the sync core thread (which may end up + service.reset(); + profile.reset(); + // Pump messages posted by the sync thread (which may end up // posting on the IO thread). ui_loop_.RunAllPending(); io_thread_.Stop(); @@ -95,26 +99,26 @@ class ProfileSyncServiceTest : public testing::Test { bool synchronous_sync_configuration, bool sync_setup_completed, syncer::StorageOption storage_option) { - if (!service_.get()) { + if (!service.get()) { SigninManager* signin = - SigninManagerFactory::GetForProfile(profile_.get()); + SigninManagerFactory::GetForProfile(profile.get()); signin->SetAuthenticatedUsername("test"); ProfileSyncComponentsFactoryMock* factory = new ProfileSyncComponentsFactoryMock(); - service_.reset(new TestProfileSyncService( + service.reset(new TestProfileSyncService( factory, - profile_.get(), + profile.get(), signin, ProfileSyncService::AUTO_START, true, base::Closure())); if (!set_initial_sync_ended) - service_->dont_set_initial_sync_ended_on_init(); + service->dont_set_initial_sync_ended_on_init(); if (synchronous_sync_configuration) - service_->set_synchronous_sync_configuration(); - service_->set_storage_option(storage_option); + service->set_synchronous_sync_configuration(); + service->set_storage_option(storage_option); if (!sync_setup_completed) - profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false); + profile->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false); // Register the bookmark data type. ON_CALL(*factory, CreateDataTypeManager(_, _, _)). @@ -123,72 +127,89 @@ class ProfileSyncServiceTest : public testing::Test { if (issue_auth_token) { IssueTestTokens(); } - service_->Initialize(); + service->Initialize(); } } void IssueTestTokens() { TokenService* token_service = - TokenServiceFactory::GetForProfile(profile_.get()); + TokenServiceFactory::GetForProfile(profile.get()); token_service->IssueAuthTokenForTest( GaiaConstants::kSyncService, "token1"); token_service->IssueAuthTokenForTest( GaiaConstants::kGaiaOAuth2LoginRefreshToken, "token2"); } + scoped_ptr<TestProfileSyncService> service; + scoped_ptr<TestingProfile> profile; + + private: MessageLoop ui_loop_; - // Needed by |service_|. + // Needed by |service|. content::TestBrowserThread ui_thread_; content::TestBrowserThread db_thread_; // Needed by DisableAndEnableSyncTemporarily test case. content::TestBrowserThread file_thread_; - // Needed by |service| and |profile_|'s request context. + // Needed by |service| and |profile|'s request context. content::TestBrowserThread io_thread_; +}; + +class ProfileSyncServiceTest : public testing::Test { + protected: + virtual void SetUp() { + harness_.SetUp(); + } + + virtual void TearDown() { + harness_.TearDown(); + } - scoped_ptr<TestProfileSyncService> service_; - scoped_ptr<TestingProfile> profile_; + ProfileSyncServiceTestHarness harness_; }; TEST_F(ProfileSyncServiceTest, InitialState) { - SigninManager* signin = SigninManagerFactory::GetForProfile(profile_.get()); - service_.reset(new TestProfileSyncService( + SigninManager* signin = + SigninManagerFactory::GetForProfile(harness_.profile.get()); + harness_.service.reset(new TestProfileSyncService( new ProfileSyncComponentsFactoryMock(), - profile_.get(), + harness_.profile.get(), signin, ProfileSyncService::MANUAL_START, true, base::Closure())); EXPECT_TRUE( - service_->sync_service_url().spec() == + harness_.service->sync_service_url().spec() == ProfileSyncService::kSyncServerUrl || - service_->sync_service_url().spec() == + harness_.service->sync_service_url().spec() == ProfileSyncService::kDevServerUrl); } TEST_F(ProfileSyncServiceTest, DisabledByPolicy) { - profile_->GetTestingPrefService()->SetManagedPref( + harness_.profile->GetTestingPrefService()->SetManagedPref( prefs::kSyncManaged, Value::CreateBooleanValue(true)); - SigninManager* signin = SigninManagerFactory::GetForProfile(profile_.get()); - service_.reset(new TestProfileSyncService( + SigninManager* signin = + SigninManagerFactory::GetForProfile(harness_.profile.get()); + harness_.service.reset(new TestProfileSyncService( new ProfileSyncComponentsFactoryMock(), - profile_.get(), + harness_.profile.get(), signin, ProfileSyncService::MANUAL_START, true, base::Closure())); - service_->Initialize(); - EXPECT_TRUE(service_->IsManaged()); + harness_.service->Initialize(); + EXPECT_TRUE(harness_.service->IsManaged()); } TEST_F(ProfileSyncServiceTest, AbortedByShutdown) { - SigninManager* signin = SigninManagerFactory::GetForProfile(profile_.get()); + SigninManager* signin = + SigninManagerFactory::GetForProfile(harness_.profile.get()); signin->SetAuthenticatedUsername("test"); ProfileSyncComponentsFactoryMock* factory = new ProfileSyncComponentsFactoryMock(); - service_.reset(new TestProfileSyncService( + harness_.service.reset(new TestProfileSyncService( factory, - profile_.get(), + harness_.profile.get(), signin, ProfileSyncService::AUTO_START, true, @@ -196,23 +217,24 @@ TEST_F(ProfileSyncServiceTest, AbortedByShutdown) { EXPECT_CALL(*factory, CreateDataTypeManager(_, _, _)).Times(0); EXPECT_CALL(*factory, CreateBookmarkSyncComponents(_, _)). Times(0); - service_->RegisterDataTypeController( - new BookmarkDataTypeController(service_->factory(), - profile_.get(), - service_.get())); + harness_.service->RegisterDataTypeController( + new BookmarkDataTypeController(harness_.service->factory(), + harness_.profile.get(), + harness_.service.get())); - service_->Initialize(); - service_.reset(); + harness_.service->Initialize(); + harness_.service.reset(); } TEST_F(ProfileSyncServiceTest, DisableAndEnableSyncTemporarily) { - SigninManager* signin = SigninManagerFactory::GetForProfile(profile_.get()); + SigninManager* signin = + SigninManagerFactory::GetForProfile(harness_.profile.get()); signin->SetAuthenticatedUsername("test"); ProfileSyncComponentsFactoryMock* factory = new ProfileSyncComponentsFactoryMock(); - service_.reset(new TestProfileSyncService( + harness_.service.reset(new TestProfileSyncService( factory, - profile_.get(), + harness_.profile.get(), signin, ProfileSyncService::AUTO_START, true, @@ -221,28 +243,31 @@ TEST_F(ProfileSyncServiceTest, DisableAndEnableSyncTemporarily) { EXPECT_CALL(*factory, CreateDataTypeManager(_, _, _)). WillRepeatedly(ReturnNewDataTypeManager()); - IssueTestTokens(); + harness_.IssueTestTokens(); - service_->Initialize(); - EXPECT_TRUE(service_->sync_initialized()); - EXPECT_TRUE(service_->GetBackendForTest() != NULL); - EXPECT_FALSE(profile_->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); + harness_.service->Initialize(); + EXPECT_TRUE(harness_.service->sync_initialized()); + EXPECT_TRUE(harness_.service->GetBackendForTest() != NULL); + EXPECT_FALSE( + harness_.profile->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); - service_->StopAndSuppress(); - EXPECT_FALSE(service_->sync_initialized()); - EXPECT_TRUE(profile_->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); + harness_.service->StopAndSuppress(); + EXPECT_FALSE(harness_.service->sync_initialized()); + EXPECT_TRUE( + harness_.profile->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); - service_->UnsuppressAndStart(); - EXPECT_TRUE(service_->sync_initialized()); - EXPECT_FALSE(profile_->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); + harness_.service->UnsuppressAndStart(); + EXPECT_TRUE(harness_.service->sync_initialized()); + EXPECT_FALSE( + harness_.profile->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart)); } TEST_F(ProfileSyncServiceTest, JsControllerHandlersBasic) { - StartSyncService(); - EXPECT_TRUE(service_->sync_initialized()); - EXPECT_TRUE(service_->GetBackendForTest() != NULL); + harness_.StartSyncService(); + EXPECT_TRUE(harness_.service->sync_initialized()); + EXPECT_TRUE(harness_.service->GetBackendForTest() != NULL); - syncer::JsController* js_controller = service_->GetJsController(); + syncer::JsController* js_controller = harness_.service->GetJsController(); StrictMock<syncer::MockJsEventHandler> event_handler; js_controller->AddJsEventHandler(&event_handler); js_controller->RemoveJsEventHandler(&event_handler); @@ -250,70 +275,68 @@ TEST_F(ProfileSyncServiceTest, JsControllerHandlersBasic) { TEST_F(ProfileSyncServiceTest, JsControllerHandlersDelayedBackendInitialization) { - StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, - syncer::STORAGE_IN_MEMORY); + harness_.StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, + syncer::STORAGE_IN_MEMORY); StrictMock<syncer::MockJsEventHandler> event_handler; EXPECT_CALL(event_handler, HandleJsEvent(_, _)).Times(AtLeast(1)); - EXPECT_EQ(NULL, service_->GetBackendForTest()); - EXPECT_FALSE(service_->sync_initialized()); + EXPECT_EQ(NULL, harness_.service->GetBackendForTest()); + EXPECT_FALSE(harness_.service->sync_initialized()); - syncer::JsController* js_controller = service_->GetJsController(); + syncer::JsController* js_controller = harness_.service->GetJsController(); js_controller->AddJsEventHandler(&event_handler); // Since we're doing synchronous initialization, backend should be // initialized by this call. - IssueTestTokens(); - EXPECT_TRUE(service_->sync_initialized()); + harness_.IssueTestTokens(); + EXPECT_TRUE(harness_.service->sync_initialized()); js_controller->RemoveJsEventHandler(&event_handler); } TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasic) { - StartSyncService(); + harness_.StartSyncService(); StrictMock<syncer::MockJsReplyHandler> reply_handler; ListValue arg_list1; - arg_list1.Append(Value::CreateStringValue("TRANSIENT_NOTIFICATION_ERROR")); + arg_list1.Append(Value::CreateStringValue("TRANSIENT_INVALIDATION_ERROR")); syncer::JsArgList args1(&arg_list1); EXPECT_CALL(reply_handler, HandleJsReply("getNotificationState", HasArgs(args1))); { - syncer::JsController* js_controller = service_->GetJsController(); + syncer::JsController* js_controller = harness_.service->GetJsController(); js_controller->ProcessJsMessage("getNotificationState", args1, reply_handler.AsWeakHandle()); } // This forces the sync thread to process the message and reply. - service_.reset(); - ui_loop_.RunAllPending(); + harness_.TearDown(); } TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasicDelayedBackendInitialization) { - StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, - syncer::STORAGE_IN_MEMORY); + harness_.StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, + syncer::STORAGE_IN_MEMORY); StrictMock<syncer::MockJsReplyHandler> reply_handler; ListValue arg_list1; - arg_list1.Append(Value::CreateStringValue("TRANSIENT_NOTIFICATION_ERROR")); + arg_list1.Append(Value::CreateStringValue("TRANSIENT_INVALIDATION_ERROR")); syncer::JsArgList args1(&arg_list1); EXPECT_CALL(reply_handler, HandleJsReply("getNotificationState", HasArgs(args1))); { - syncer::JsController* js_controller = service_->GetJsController(); + syncer::JsController* js_controller = harness_.service->GetJsController(); js_controller->ProcessJsMessage("getNotificationState", args1, reply_handler.AsWeakHandle()); } - IssueTestTokens(); + harness_.IssueTestTokens(); // This forces the sync thread to process the message and reply. - service_.reset(); - ui_loop_.RunAllPending(); + harness_.TearDown(); } // Make sure that things still work if sync is not enabled, but some old sync @@ -322,7 +345,8 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) { const char* nonsense1 = "reginald"; const char* nonsense2 = "beartato"; const char* nonsense3 = "harrison"; - FilePath temp_directory = profile_->GetPath().AppendASCII("Sync Data"); + FilePath temp_directory = + harness_.profile->GetPath().AppendASCII("Sync Data"); FilePath sync_file1 = temp_directory.AppendASCII("BookmarkSyncSettings.sqlite3"); FilePath sync_file2 = temp_directory.AppendASCII("SyncData.sqlite3"); @@ -335,18 +359,18 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) { ASSERT_NE(-1, file_util::WriteFile(sync_file3, nonsense3, strlen(nonsense3))); - StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, - syncer::STORAGE_ON_DISK); - EXPECT_FALSE(service_->HasSyncSetupCompleted()); - EXPECT_FALSE(service_->sync_initialized()); + harness_.StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, + syncer::STORAGE_ON_DISK); + EXPECT_FALSE(harness_.service->HasSyncSetupCompleted()); + EXPECT_FALSE(harness_.service->sync_initialized()); // Since we're doing synchronous initialization, backend should be // initialized by this call. - IssueTestTokens(); + harness_.IssueTestTokens(); // Stop the service so we can read the new Sync Data files that were // created. - service_.reset(); + harness_.service.reset(); // This file should have been deleted when the whole directory was nuked. ASSERT_FALSE(file_util::PathExists(sync_file3)); @@ -363,89 +387,164 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) { // recreate it. This test is useful mainly when it is run under valgrind. Its // expectations are not very interesting. TEST_F(ProfileSyncServiceTest, FailToOpenDatabase) { - StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, - syncer::STORAGE_INVALID); + harness_.StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, + syncer::STORAGE_INVALID); // The backend is not ready. Ensure the PSS knows this. - EXPECT_FALSE(service_->sync_initialized()); + EXPECT_FALSE(harness_.service->sync_initialized()); } -// Register for some IDs with the ProfileSyncService and trigger some -// invalidation messages. They should be received by the handler. -// Then unregister and trigger the invalidation messages again. Those -// shouldn't be received by the handler. -TEST_F(ProfileSyncServiceTest, UpdateRegisteredInvalidationIds) { - StartSyncService(); +// Register for some IDs with the ProfileSyncService, restart sync, +// and trigger some invalidation messages. They should still be +// received by the handler. +TEST_F(ProfileSyncServiceTest, UpdateRegisteredInvalidationIdsPersistence) { + harness_.StartSyncService(); syncer::ObjectIdSet ids; - ids.insert(invalidation::ObjectId(1, "id1")); - ids.insert(invalidation::ObjectId(2, "id2")); + ids.insert(invalidation::ObjectId(3, "id3")); const syncer::ObjectIdStateMap& states = syncer::ObjectIdSetToStateMap(ids, "payload"); syncer::FakeInvalidationHandler handler; - service_->RegisterInvalidationHandler(&handler); - service_->UpdateRegisteredInvalidationIds(&handler, ids); + harness_.service->RegisterInvalidationHandler(&handler); + harness_.service->UpdateRegisteredInvalidationIds(&handler, ids); + + harness_.service->StopAndSuppress(); + harness_.service->UnsuppressAndStart(); SyncBackendHostForProfileSyncTest* const backend = - service_->GetBackendForTest(); + harness_.service->GetBackendForTest(); - backend->EmitOnNotificationsEnabled(); - EXPECT_EQ(syncer::NO_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); + backend->EmitOnInvalidatorStateChange(syncer::INVALIDATIONS_ENABLED); + EXPECT_EQ(syncer::INVALIDATIONS_ENABLED, handler.GetInvalidatorState()); - backend->EmitOnIncomingNotification(states, syncer::REMOTE_NOTIFICATION); - EXPECT_THAT(states, Eq(handler.GetLastNotificationIdStateMap())); - EXPECT_EQ(syncer::REMOTE_NOTIFICATION, handler.GetLastNotificationSource()); + backend->EmitOnIncomingInvalidation(states, syncer::REMOTE_INVALIDATION); + EXPECT_THAT(states, Eq(handler.GetLastInvalidationIdStateMap())); + EXPECT_EQ(syncer::REMOTE_INVALIDATION, handler.GetLastInvalidationSource()); - backend->EmitOnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(syncer::TRANSIENT_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); + backend->EmitOnInvalidatorStateChange(syncer::TRANSIENT_INVALIDATION_ERROR); + EXPECT_EQ(syncer::TRANSIENT_INVALIDATION_ERROR, + handler.GetInvalidatorState()); +} - Mock::VerifyAndClearExpectations(&handler); +// Thin Invalidator wrapper around ProfileSyncService. +class ProfileSyncServiceInvalidator : public syncer::Invalidator { + public: + explicit ProfileSyncServiceInvalidator(ProfileSyncService* service) + : service_(service) {} - service_->UnregisterInvalidationHandler(&handler); + virtual ~ProfileSyncServiceInvalidator() {} - backend->EmitOnNotificationsEnabled(); - backend->EmitOnIncomingNotification(states, syncer::REMOTE_NOTIFICATION); - backend->EmitOnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR); -} + // Invalidator implementation. + virtual void RegisterHandler(syncer::InvalidationHandler* handler) OVERRIDE { + service_->RegisterInvalidationHandler(handler); + } -// Register for some IDs with the ProfileSyncService, restart sync, -// and trigger some invalidation messages. They should still be -// received by the handler. -TEST_F(ProfileSyncServiceTest, UpdateRegisteredInvalidationIdsPersistence) { - StartSyncService(); + virtual void UpdateRegisteredIds(syncer::InvalidationHandler* handler, + const syncer::ObjectIdSet& ids) OVERRIDE { + service_->UpdateRegisteredInvalidationIds(handler, ids); + } - syncer::ObjectIdSet ids; - ids.insert(invalidation::ObjectId(3, "id3")); - const syncer::ObjectIdStateMap& states = - syncer::ObjectIdSetToStateMap(ids, "payload"); + virtual void UnregisterHandler( + syncer::InvalidationHandler* handler) OVERRIDE { + service_->UnregisterInvalidationHandler(handler); + } - syncer::FakeInvalidationHandler handler; + virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE { + return service_->GetInvalidatorState(); + } - service_->RegisterInvalidationHandler(&handler); - service_->UpdateRegisteredInvalidationIds(&handler, ids); + virtual void SetUniqueId(const std::string& unique_id) OVERRIDE { + // Do nothing. + } - service_->StopAndSuppress(); - service_->UnsuppressAndStart(); + virtual void SetStateDeprecated(const std::string& state) OVERRIDE { + // Do nothing. + } - SyncBackendHostForProfileSyncTest* const backend = - service_->GetBackendForTest(); + virtual void UpdateCredentials( + const std::string& email, const std::string& token) OVERRIDE { + // Do nothing. + } - backend->EmitOnNotificationsEnabled(); - EXPECT_EQ(syncer::NO_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); + virtual void SendInvalidation( + const syncer::ObjectIdStateMap& id_state_map) OVERRIDE { + // Do nothing. + } - backend->EmitOnIncomingNotification(states, syncer::REMOTE_NOTIFICATION); - EXPECT_THAT(states, Eq(handler.GetLastNotificationIdStateMap())); - EXPECT_EQ(syncer::REMOTE_NOTIFICATION, handler.GetLastNotificationSource()); + private: + ProfileSyncService* const service_; - backend->EmitOnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(syncer::TRANSIENT_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); -} + DISALLOW_COPY_AND_ASSIGN(ProfileSyncServiceInvalidator); +}; } // namespace + + +// ProfileSyncServiceInvalidatorTestDelegate has to be visible from +// the syncer namespace (where InvalidatorTest lives). +class ProfileSyncServiceInvalidatorTestDelegate { + public: + ProfileSyncServiceInvalidatorTestDelegate() {} + + ~ProfileSyncServiceInvalidatorTestDelegate() { + DestroyInvalidator(); + } + + void CreateInvalidator( + const std::string& initial_state, + const base::WeakPtr<syncer::InvalidationStateTracker>& + invalidation_state_tracker) { + DCHECK(!invalidator_.get()); + harness_.SetUp(); + harness_.StartSyncService(); + invalidator_.reset( + new ProfileSyncServiceInvalidator(harness_.service.get())); + } + + ProfileSyncServiceInvalidator* GetInvalidator() { + return invalidator_.get(); + } + + void DestroyInvalidator() { + invalidator_.reset(); + harness_.TearDown(); + } + + void WaitForInvalidator() { + // Do nothing. + } + + void TriggerOnInvalidatorStateChange(syncer::InvalidatorState state) { + harness_.service->GetBackendForTest()->EmitOnInvalidatorStateChange(state); + } + + void TriggerOnIncomingInvalidation( + const syncer::ObjectIdStateMap& id_state_map, + syncer::IncomingInvalidationSource source) { + harness_.service->GetBackendForTest()->EmitOnIncomingInvalidation( + id_state_map, source); + } + + static bool InvalidatorHandlesDeprecatedState() { + return false; + } + + private: + ProfileSyncServiceTestHarness harness_; + scoped_ptr<ProfileSyncServiceInvalidator> invalidator_; +}; + } // namespace browser_sync + +namespace syncer { +namespace { + +// ProfileSyncService should behave just like an invalidator. +INSTANTIATE_TYPED_TEST_CASE_P( + ProfileSyncServiceInvalidatorTest, InvalidatorTest, + ::browser_sync::ProfileSyncServiceInvalidatorTestDelegate); + +} // namespace +} // namespace syncer diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc index 422588c..6fab4d3 100644 --- a/chrome/browser/sync/test/integration/sync_test.cc +++ b/chrome/browser/sync/test/integration/sync_test.cc @@ -676,7 +676,7 @@ void SyncTest::TriggerNotification(syncer::ModelTypeSet changed_types) { syncer::NOTIFY_ALL, syncer::ObjectIdSetToStateMap( syncer::ModelTypeSetToObjectIdSet(changed_types), ""), - syncer::REMOTE_NOTIFICATION).ToString(); + syncer::REMOTE_INVALIDATION).ToString(); const std::string& path = std::string("chromiumsync/sendnotification?channel=") + syncer::kSyncP2PNotificationChannel + "&data=" + data; diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc index a4efa92..c2724176 100644 --- a/chrome/browser/sync/test_profile_sync_service.cc +++ b/chrome/browser/sync/test_profile_sync_service.cc @@ -146,19 +146,15 @@ void SyncBackendHostForProfileSyncTest::SetInitialSyncEndedForAllTypes() { } } -void SyncBackendHostForProfileSyncTest::EmitOnNotificationsEnabled() { - frontend()->OnNotificationsEnabled(); +void SyncBackendHostForProfileSyncTest::EmitOnInvalidatorStateChange( + syncer::InvalidatorState state) { + frontend()->OnInvalidatorStateChange(state); } -void SyncBackendHostForProfileSyncTest::EmitOnNotificationsDisabled( - syncer::NotificationsDisabledReason reason) { - frontend()->OnNotificationsDisabled(reason); -} - -void SyncBackendHostForProfileSyncTest::EmitOnIncomingNotification( +void SyncBackendHostForProfileSyncTest::EmitOnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - const syncer::IncomingNotificationSource source) { - frontend()->OnIncomingNotification(id_state_map, source); + const syncer::IncomingInvalidationSource source) { + frontend()->OnIncomingInvalidation(id_state_map, source); } } // namespace browser_sync diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h index 9be433d..7bd5fac 100644 --- a/chrome/browser/sync/test_profile_sync_service.h +++ b/chrome/browser/sync/test_profile_sync_service.h @@ -64,12 +64,10 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost { void SetInitialSyncEndedForAllTypes(); - void EmitOnNotificationsEnabled(); - void EmitOnNotificationsDisabled( - syncer::NotificationsDisabledReason reason); - void EmitOnIncomingNotification( + void EmitOnInvalidatorStateChange(syncer::InvalidatorState state); + void EmitOnIncomingInvalidation( const syncer::ObjectIdStateMap& id_state_map, - const syncer::IncomingNotificationSource source); + const syncer::IncomingInvalidationSource source); protected: virtual void InitCore(const DoInitializeOptions& options) OVERRIDE; diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h index df0e3e0..b050aca 100644 --- a/sync/internal_api/public/test/fake_sync_manager.h +++ b/sync/internal_api/public/test/fake_sync_manager.h @@ -55,13 +55,10 @@ class FakeSyncManager : public SyncManager { // Posts a method to invalidate the given IDs on the sync thread. void Invalidate(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source); + IncomingInvalidationSource source); - // Posts a method to enable notifications on the sync thread. - void EnableNotifications(); - - // Posts a method to disable notifications on the sync thread. - void DisableNotifications(NotificationsDisabledReason reason); + // Posts a method to update the invalidator state on the sync thread. + void UpdateInvalidatorState(InvalidatorState state); // Block until the sync thread has finished processing any pending messages. void WaitForSyncThread(); @@ -125,9 +122,8 @@ class FakeSyncManager : public SyncManager { private: void InvalidateOnSyncThread( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source); - void EnableNotificationsOnSyncThread(); - void DisableNotificationsOnSyncThread(NotificationsDisabledReason reason); + IncomingInvalidationSource source); + void UpdateInvalidatorStateOnSyncThread(InvalidatorState state); scoped_refptr<base::SequencedTaskRunner> sync_task_runner_; diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index ddd8ad1..0823e71 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -168,7 +168,7 @@ SyncManagerImpl::SyncManagerImpl(const std::string& name) change_delegate_(NULL), initialized_(false), observing_ip_address_changes_(false), - notifications_disabled_reason_(TRANSIENT_NOTIFICATION_ERROR), + invalidator_state_(DEFAULT_INVALIDATION_ERROR), throttled_data_type_tracker_(&allstatus_), traffic_recorder_(kMaxMessagesToRecord, kMaxMessageSizeToRecord), encryptor_(NULL), @@ -1010,9 +1010,9 @@ void SyncManagerImpl::OnSyncEngineEvent(const SyncEngineEvent& event) { if (invalidator_.get()) { const ObjectIdStateMap& id_state_map = ModelTypeStateMapToObjectIdStateMap(event.snapshot.source().types); - invalidator_->SendNotification(id_state_map); + invalidator_->SendInvalidation(id_state_map); } else { - DVLOG(1) << "Not sending notification: invalidator_ is NULL"; + DVLOG(1) << "Not sending invalidation: invalidator_ is NULL"; } } } @@ -1080,29 +1080,6 @@ void SyncManagerImpl::BindJsMessageHandler( base::Bind(unbound_message_handler, base::Unretained(this)); } -void SyncManagerImpl::OnNotificationStateChange( - NotificationsDisabledReason reason) { - const std::string& reason_str = NotificationsDisabledReasonToString(reason); - notifications_disabled_reason_ = reason; - DVLOG(1) << "Notification state changed to: " << reason_str; - const bool notifications_enabled = - (notifications_disabled_reason_ == NO_NOTIFICATION_ERROR); - allstatus_.SetNotificationsEnabled(notifications_enabled); - scheduler_->SetNotificationsEnabled(notifications_enabled); - - // TODO(akalin): Treat a CREDENTIALS_REJECTED state as an auth - // error. - - if (js_event_handler_.IsInitialized()) { - DictionaryValue details; - details.Set("state", Value::CreateStringValue(reason_str)); - js_event_handler_.Call(FROM_HERE, - &JsEventHandler::HandleJsEvent, - "onNotificationStateChange", - JsEventDetails(&details)); - } -} - DictionaryValue* SyncManagerImpl::NotificationInfoToValue( const NotificationInfoMap& notification_info) { DictionaryValue* value = new DictionaryValue(); @@ -1128,7 +1105,7 @@ std::string SyncManagerImpl::NotificationInfoToString( JsArgList SyncManagerImpl::GetNotificationState( const JsArgList& args) { const std::string& notification_state = - NotificationsDisabledReasonToString(notifications_disabled_reason_); + InvalidatorStateToString(invalidator_state_); DVLOG(1) << "GetNotificationState: " << notification_state; ListValue return_args; return_args.Append(Value::CreateStringValue(notification_state)); @@ -1260,22 +1237,35 @@ void SyncManagerImpl::UpdateNotificationInfo( } } -void SyncManagerImpl::OnNotificationsEnabled() { - OnNotificationStateChange(NO_NOTIFICATION_ERROR); -} +void SyncManagerImpl::OnInvalidatorStateChange(InvalidatorState state) { + const std::string& state_str = InvalidatorStateToString(state); + invalidator_state_ = state; + DVLOG(1) << "Invalidator state changed to: " << state_str; + const bool notifications_enabled = + (invalidator_state_ == INVALIDATIONS_ENABLED); + allstatus_.SetNotificationsEnabled(notifications_enabled); + scheduler_->SetNotificationsEnabled(notifications_enabled); + + // TODO(akalin): Treat a CREDENTIALS_REJECTED state as an auth + // error. -void SyncManagerImpl::OnNotificationsDisabled( - NotificationsDisabledReason reason) { - OnNotificationStateChange(reason); + if (js_event_handler_.IsInitialized()) { + DictionaryValue details; + details.SetString("state", state_str); + js_event_handler_.Call(FROM_HERE, + &JsEventHandler::HandleJsEvent, + "onNotificationStateChange", + JsEventDetails(&details)); + } } -void SyncManagerImpl::OnIncomingNotification( +void SyncManagerImpl::OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { DCHECK(thread_checker_.CalledOnValidThread()); const ModelTypeStateMap& type_state_map = ObjectIdStateMapToModelTypeStateMap(id_state_map); - if (source == LOCAL_NOTIFICATION) { + if (source == LOCAL_INVALIDATION) { scheduler_->ScheduleNudgeWithStatesAsync( TimeDelta::FromMilliseconds(kSyncRefreshDelayMsec), NUDGE_SOURCE_LOCAL_REFRESH, @@ -1289,7 +1279,7 @@ void SyncManagerImpl::OnIncomingNotification( UpdateNotificationInfo(type_state_map); debug_info_event_listener_.OnIncomingNotification(type_state_map); } else { - LOG(WARNING) << "Sync received notification without any type information."; + LOG(WARNING) << "Sync received invalidation without any type information."; } if (js_event_handler_.IsInitialized()) { @@ -1302,8 +1292,8 @@ void SyncManagerImpl::OnIncomingNotification( ModelTypeToString(it->first); changed_types->Append(Value::CreateStringValue(model_type_str)); } - details.SetString("source", (source == LOCAL_NOTIFICATION) ? - "LOCAL_NOTIFICATION" : "REMOTE_NOTIFICATION"); + details.SetString("source", (source == LOCAL_INVALIDATION) ? + "LOCAL_INVALIDATION" : "REMOTE_INVALIDATION"); js_event_handler_.Call(FROM_HERE, &JsEventHandler::HandleJsEvent, "onIncomingNotification", diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index a721fa2..152cff6 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -23,7 +23,7 @@ #include "sync/internal_api/sync_encryption_handler_impl.h" #include "sync/js/js_backend.h" #include "sync/notifier/invalidation_handler.h" -#include "sync/notifier/notifications_disabled_reason.h" +#include "sync/notifier/invalidator_state.h" #include "sync/syncable/directory_change_delegate.h" #include "sync/util/cryptographer.h" #include "sync/util/time.h" @@ -167,12 +167,10 @@ class SyncManagerImpl : public SyncManager, syncable::BaseTransaction* trans) OVERRIDE; // InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) OVERRIDE; + IncomingInvalidationSource source) OVERRIDE; // Called only by our NetworkChangeNotifier. virtual void OnIPAddressChanged() OVERRIDE; @@ -264,9 +262,6 @@ class SyncManagerImpl : public SyncManager, void BindJsMessageHandler( const std::string& name, UnboundJsMessageHandler unbound_message_handler); - // Helper function used by OnNotifications{Enabled,Disabled}(). - void OnNotificationStateChange(NotificationsDisabledReason reason); - // Returned pointer is owned by the caller. static DictionaryValue* NotificationInfoToValue( const NotificationInfoMap& notification_info); @@ -353,7 +348,7 @@ class SyncManagerImpl : public SyncManager, bool observing_ip_address_changes_; - NotificationsDisabledReason notifications_disabled_reason_; + InvalidatorState invalidator_state_; // Map used to store the notification info to be displayed in // about:sync page. diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc index 9f5538e..d2ffa6c 100644 --- a/sync/internal_api/sync_manager_impl_unittest.cc +++ b/sync/internal_api/sync_manager_impl_unittest.cc @@ -906,24 +906,18 @@ class SyncManagerTest : public testing::Test, GetEncryptedTypes(trans->GetWrappedTrans()); } - void SimulateEnableNotificationsForTest() { + void SimulateInvalidatorStateChangeForTest(InvalidatorState state) { DCHECK(sync_manager_.thread_checker_.CalledOnValidThread()); - sync_manager_.OnNotificationsEnabled(); - } - - void SimulateDisableNotificationsForTest( - NotificationsDisabledReason reason) { - DCHECK(sync_manager_.thread_checker_.CalledOnValidThread()); - sync_manager_.OnNotificationsDisabled(reason); + sync_manager_.OnInvalidatorStateChange(state); } void TriggerOnIncomingNotificationForTest(ModelTypeSet model_types) { DCHECK(sync_manager_.thread_checker_.CalledOnValidThread()); ModelTypeStateMap type_state_map = ModelTypeSetToStateMap(model_types, std::string()); - sync_manager_.OnIncomingNotification( + sync_manager_.OnIncomingInvalidation( ModelTypeStateMapToObjectIdStateMap(type_state_map), - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); } void SetProgressMarkerForType(ModelType type, bool set) { @@ -996,7 +990,7 @@ TEST_F(SyncManagerTest, ProcessJsMessage) { ListValue disabled_args; disabled_args.Append( - Value::CreateStringValue("TRANSIENT_NOTIFICATION_ERROR")); + Value::CreateStringValue("TRANSIENT_INVALIDATION_ERROR")); EXPECT_CALL(reply_handler, HandleJsReply("getNotificationState", @@ -1284,9 +1278,9 @@ TEST_F(SyncManagerTest, OnNotificationStateChange) { StrictMock<MockJsEventHandler> event_handler; DictionaryValue enabled_details; - enabled_details.SetString("state", "NO_NOTIFICATION_ERROR"); + enabled_details.SetString("state", "INVALIDATIONS_ENABLED"); DictionaryValue disabled_details; - disabled_details.SetString("state", "TRANSIENT_NOTIFICATION_ERROR"); + disabled_details.SetString("state", "TRANSIENT_INVALIDATION_ERROR"); EXPECT_CALL(event_handler, HandleJsEvent("onNotificationStateChange", @@ -1295,16 +1289,16 @@ TEST_F(SyncManagerTest, OnNotificationStateChange) { HandleJsEvent("onNotificationStateChange", HasDetailsAsDictionary(disabled_details))); - SimulateEnableNotificationsForTest(); - SimulateDisableNotificationsForTest(TRANSIENT_NOTIFICATION_ERROR); + SimulateInvalidatorStateChangeForTest(INVALIDATIONS_ENABLED); + SimulateInvalidatorStateChangeForTest(TRANSIENT_INVALIDATION_ERROR); SetJsEventHandler(event_handler.AsWeakHandle()); - SimulateEnableNotificationsForTest(); - SimulateDisableNotificationsForTest(TRANSIENT_NOTIFICATION_ERROR); + SimulateInvalidatorStateChangeForTest(INVALIDATIONS_ENABLED); + SimulateInvalidatorStateChangeForTest(TRANSIENT_INVALIDATION_ERROR); SetJsEventHandler(WeakHandle<JsEventHandler>()); - SimulateEnableNotificationsForTest(); - SimulateDisableNotificationsForTest(TRANSIENT_NOTIFICATION_ERROR); + SimulateInvalidatorStateChangeForTest(INVALIDATIONS_ENABLED); + SimulateInvalidatorStateChangeForTest(TRANSIENT_INVALIDATION_ERROR); // Should trigger the replies. PumpLoop(); @@ -1322,7 +1316,7 @@ TEST_F(SyncManagerTest, OnIncomingNotification) { DictionaryValue expected_details; { ListValue* model_type_list = new ListValue(); - expected_details.SetString("source", "REMOTE_NOTIFICATION"); + expected_details.SetString("source", "REMOTE_INVALIDATION"); expected_details.Set("changedTypes", model_type_list); for (ModelTypeSet::Iterator it = model_types.First(); it.Good(); it.Inc()) { diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc index affca7d..15794b0 100644 --- a/sync/internal_api/test/fake_sync_manager.cc +++ b/sync/internal_api/test/fake_sync_manager.cc @@ -16,8 +16,8 @@ #include "sync/internal_api/public/http_post_provider_factory.h" #include "sync/internal_api/public/internal_components_factory.h" #include "sync/internal_api/public/util/weak_handle.h" -#include "sync/notifier/notifications_disabled_reason.h" #include "sync/notifier/invalidator.h" +#include "sync/notifier/invalidator_state.h" #include "sync/notifier/object_id_state_map.h" #include "sync/test/fake_sync_encryption_handler.h" @@ -53,7 +53,7 @@ ModelTypeSet FakeSyncManager::GetAndResetEnabledTypes() { } void FakeSyncManager::Invalidate(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { if (!sync_task_runner_->PostTask( FROM_HERE, base::Bind(&FakeSyncManager::InvalidateOnSyncThread, @@ -62,21 +62,11 @@ void FakeSyncManager::Invalidate(const ObjectIdStateMap& id_state_map, } } -void FakeSyncManager::EnableNotifications() { +void FakeSyncManager::UpdateInvalidatorState(InvalidatorState state) { if (!sync_task_runner_->PostTask( FROM_HERE, - base::Bind(&FakeSyncManager::EnableNotificationsOnSyncThread, - base::Unretained(this)))) { - NOTREACHED(); - } -} - -void FakeSyncManager::DisableNotifications( - NotificationsDisabledReason reason) { - if (!sync_task_runner_->PostTask( - FROM_HERE, - base::Bind(&FakeSyncManager::DisableNotificationsOnSyncThread, - base::Unretained(this), reason))) { + base::Bind(&FakeSyncManager::UpdateInvalidatorStateOnSyncThread, + base::Unretained(this), state))) { NOTREACHED(); } } @@ -262,20 +252,15 @@ SyncEncryptionHandler* FakeSyncManager::GetEncryptionHandler() { void FakeSyncManager::InvalidateOnSyncThread( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { DCHECK(sync_task_runner_->RunsTasksOnCurrentThread()); registrar_.DispatchInvalidationsToHandlers(id_state_map, source); } -void FakeSyncManager::EnableNotificationsOnSyncThread() { - DCHECK(sync_task_runner_->RunsTasksOnCurrentThread()); - registrar_.EmitOnNotificationsEnabled(); -} - -void FakeSyncManager::DisableNotificationsOnSyncThread( - NotificationsDisabledReason reason) { +void FakeSyncManager::UpdateInvalidatorStateOnSyncThread( + InvalidatorState state) { DCHECK(sync_task_runner_->RunsTasksOnCurrentThread()); - registrar_.EmitOnNotificationsDisabled(reason); + registrar_.UpdateInvalidatorState(state); } } // namespace syncer diff --git a/sync/notifier/fake_invalidation_handler.cc b/sync/notifier/fake_invalidation_handler.cc index 6c06587..d63a549 100644 --- a/sync/notifier/fake_invalidation_handler.cc +++ b/sync/notifier/fake_invalidation_handler.cc @@ -7,46 +7,40 @@ namespace syncer { FakeInvalidationHandler::FakeInvalidationHandler() - : reason_(TRANSIENT_NOTIFICATION_ERROR), - last_source_(LOCAL_NOTIFICATION), - notification_count_(0) {} + : state_(DEFAULT_INVALIDATION_ERROR), + last_source_(LOCAL_INVALIDATION), + invalidation_count_(0) {} FakeInvalidationHandler::~FakeInvalidationHandler() {} -NotificationsDisabledReason -FakeInvalidationHandler::GetNotificationsDisabledReason() const { - return reason_; +InvalidatorState FakeInvalidationHandler::GetInvalidatorState() const { + return state_; } const ObjectIdStateMap& -FakeInvalidationHandler::GetLastNotificationIdStateMap() const { +FakeInvalidationHandler::GetLastInvalidationIdStateMap() const { return last_id_state_map_; } -IncomingNotificationSource -FakeInvalidationHandler::GetLastNotificationSource() const { +IncomingInvalidationSource +FakeInvalidationHandler::GetLastInvalidationSource() const { return last_source_; } -int FakeInvalidationHandler::GetNotificationCount() const { - return notification_count_; +int FakeInvalidationHandler::GetInvalidationCount() const { + return invalidation_count_; } -void FakeInvalidationHandler::OnNotificationsEnabled() { - reason_ = NO_NOTIFICATION_ERROR; +void FakeInvalidationHandler::OnInvalidatorStateChange(InvalidatorState state) { + state_ = state; } -void FakeInvalidationHandler::OnNotificationsDisabled( - NotificationsDisabledReason reason) { - reason_ = reason; -} - -void FakeInvalidationHandler::OnIncomingNotification( +void FakeInvalidationHandler::OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { last_id_state_map_ = id_state_map; last_source_ = source; - ++notification_count_; + ++invalidation_count_; } } // namespace syncer diff --git a/sync/notifier/fake_invalidation_handler.h b/sync/notifier/fake_invalidation_handler.h index 1d9dcfbd..fd6ffb3 100644 --- a/sync/notifier/fake_invalidation_handler.h +++ b/sync/notifier/fake_invalidation_handler.h @@ -7,6 +7,7 @@ #include <string> +#include "base/basictypes.h" #include "base/compiler_specific.h" #include "sync/notifier/invalidation_handler.h" @@ -17,24 +18,24 @@ class FakeInvalidationHandler : public InvalidationHandler { FakeInvalidationHandler(); virtual ~FakeInvalidationHandler(); - NotificationsDisabledReason GetNotificationsDisabledReason() const; - const ObjectIdStateMap& GetLastNotificationIdStateMap() const; - IncomingNotificationSource GetLastNotificationSource() const; - int GetNotificationCount() const; + InvalidatorState GetInvalidatorState() const; + const ObjectIdStateMap& GetLastInvalidationIdStateMap() const; + IncomingInvalidationSource GetLastInvalidationSource() const; + int GetInvalidationCount() const; // InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) OVERRIDE; + IncomingInvalidationSource source) OVERRIDE; private: - NotificationsDisabledReason reason_; + InvalidatorState state_; ObjectIdStateMap last_id_state_map_; - IncomingNotificationSource last_source_; - int notification_count_; + IncomingInvalidationSource last_source_; + int invalidation_count_; + + DISALLOW_COPY_AND_ASSIGN(FakeInvalidationHandler); }; } // namespace syncer diff --git a/sync/notifier/fake_invalidator.cc b/sync/notifier/fake_invalidator.cc index 7468dae..c2b7135 100644 --- a/sync/notifier/fake_invalidator.cc +++ b/sync/notifier/fake_invalidator.cc @@ -39,21 +39,16 @@ const ObjectIdStateMap& FakeInvalidator::GetLastSentIdStateMap() const { return last_sent_id_state_map_; } -void FakeInvalidator::EmitOnNotificationsEnabled() { - registrar_.EmitOnNotificationsEnabled(); +void FakeInvalidator::EmitOnInvalidatorStateChange(InvalidatorState state) { + registrar_.UpdateInvalidatorState(state); } -void FakeInvalidator::EmitOnIncomingNotification( +void FakeInvalidator::EmitOnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { registrar_.DispatchInvalidationsToHandlers(id_state_map, source); } -void FakeInvalidator::EmitOnNotificationsDisabled( - NotificationsDisabledReason reason) { - registrar_.EmitOnNotificationsDisabled(reason); -} - void FakeInvalidator::RegisterHandler(InvalidationHandler* handler) { registrar_.RegisterHandler(handler); } @@ -67,6 +62,10 @@ void FakeInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } +InvalidatorState FakeInvalidator::GetInvalidatorState() const { + return registrar_.GetInvalidatorState(); +} + void FakeInvalidator::SetUniqueId(const std::string& unique_id) { unique_id_ = unique_id; } @@ -81,7 +80,7 @@ void FakeInvalidator::UpdateCredentials( token_ = token; } -void FakeInvalidator::SendNotification(const ObjectIdStateMap& id_state_map) { +void FakeInvalidator::SendInvalidation(const ObjectIdStateMap& id_state_map) { last_sent_id_state_map_ = id_state_map; } diff --git a/sync/notifier/fake_invalidator.h b/sync/notifier/fake_invalidator.h index 07690dd..dff7d21 100644 --- a/sync/notifier/fake_invalidator.h +++ b/sync/notifier/fake_invalidator.h @@ -27,20 +27,20 @@ class FakeInvalidator : public Invalidator { const std::string& GetCredentialsToken() const; const ObjectIdStateMap& GetLastSentIdStateMap() const; - void EmitOnNotificationsEnabled(); - void EmitOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source); - void EmitOnNotificationsDisabled(NotificationsDisabledReason reason); + void EmitOnInvalidatorStateChange(InvalidatorState state); + void EmitOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source); virtual void RegisterHandler(InvalidationHandler* handler) OVERRIDE; virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; + virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; virtual void SetStateDeprecated(const std::string& state) OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; - virtual void SendNotification(const ObjectIdStateMap& id_state_map) OVERRIDE; + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) OVERRIDE; private: InvalidatorRegistrar registrar_; diff --git a/sync/notifier/fake_invalidator_unittest.cc b/sync/notifier/fake_invalidator_unittest.cc index 05b3e0b..6b5decc 100644 --- a/sync/notifier/fake_invalidator_unittest.cc +++ b/sync/notifier/fake_invalidator_unittest.cc @@ -40,17 +40,13 @@ class FakeInvalidatorTestDelegate { // Do Nothing. } - void TriggerOnNotificationsEnabled() { - invalidator_->EmitOnNotificationsEnabled(); + void TriggerOnInvalidatorStateChange(InvalidatorState state) { + invalidator_->EmitOnInvalidatorStateChange(state); } - void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { - invalidator_->EmitOnIncomingNotification(id_state_map, source); - } - - void TriggerOnNotificationsDisabled(NotificationsDisabledReason reason) { - invalidator_->EmitOnNotificationsDisabled(reason); + void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source) { + invalidator_->EmitOnIncomingInvalidation(id_state_map, source); } static bool InvalidatorHandlesDeprecatedState() { diff --git a/sync/notifier/invalidation_handler.h b/sync/notifier/invalidation_handler.h index c1252ac..58f3a53 100644 --- a/sync/notifier/invalidation_handler.h +++ b/sync/notifier/invalidation_handler.h @@ -5,33 +5,29 @@ #ifndef SYNC_NOTIFIER_INVALIDATION_HANDLER_H_ #define SYNC_NOTIFIER_INVALIDATION_HANDLER_H_ +#include "sync/notifier/invalidator_state.h" #include "sync/notifier/object_id_state_map.h" -#include "sync/notifier/notifications_disabled_reason.h" namespace syncer { -enum IncomingNotificationSource { - // The server is notifying us that one or more datatypes have stale data. - REMOTE_NOTIFICATION, - // A chrome datatype is requesting an optimistic refresh of its data. - LOCAL_NOTIFICATION, +enum IncomingInvalidationSource { + // The server is notifying us that one or more objects have stale data. + REMOTE_INVALIDATION, + // Something locally is requesting an optimistic refresh of its data. + LOCAL_INVALIDATION, }; class InvalidationHandler { public: - // Called when notifications are enabled. - virtual void OnNotificationsEnabled() = 0; + // Called when the invalidator state changes. + virtual void OnInvalidatorStateChange(InvalidatorState state) = 0; - // Called when notifications are disabled, with the reason in - // |reason|. - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) = 0; - - // Called when a notification is received. The per-id states - // are in |id_state_map| and the source is in |source|. - virtual void OnIncomingNotification( + // Called when a invalidation is received. The per-id states are in + // |id_state_map| and the source is in |source|. Note that this may be + // called regardless of the current invalidator state. + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) = 0; + IncomingInvalidationSource source) = 0; protected: virtual ~InvalidationHandler() {} diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc index 81a268b..aaab368 100644 --- a/sync/notifier/invalidation_notifier.cc +++ b/sync/notifier/invalidation_notifier.cc @@ -52,6 +52,11 @@ void InvalidationNotifier::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } +InvalidatorState InvalidationNotifier::GetInvalidatorState() const { + DCHECK(CalledOnValidThread()); + return registrar_.GetInvalidatorState(); +} + void InvalidationNotifier::SetUniqueId(const std::string& unique_id) { DCHECK(CalledOnValidThread()); client_id_ = unique_id; @@ -94,7 +99,7 @@ void InvalidationNotifier::UpdateCredentials( invalidation_listener_.UpdateCredentials(email, token); } -void InvalidationNotifier::SendNotification( +void InvalidationNotifier::SendInvalidation( const ObjectIdStateMap& id_state_map) { DCHECK(CalledOnValidThread()); // Do nothing. @@ -102,18 +107,12 @@ void InvalidationNotifier::SendNotification( void InvalidationNotifier::OnInvalidate(const ObjectIdStateMap& id_state_map) { DCHECK(CalledOnValidThread()); - registrar_.DispatchInvalidationsToHandlers(id_state_map, REMOTE_NOTIFICATION); -} - -void InvalidationNotifier::OnNotificationsEnabled() { - DCHECK(CalledOnValidThread()); - registrar_.EmitOnNotificationsEnabled(); + registrar_.DispatchInvalidationsToHandlers(id_state_map, REMOTE_INVALIDATION); } -void InvalidationNotifier::OnNotificationsDisabled( - NotificationsDisabledReason reason) { +void InvalidationNotifier::OnInvalidatorStateChange(InvalidatorState state) { DCHECK(CalledOnValidThread()); - registrar_.EmitOnNotificationsDisabled(reason); + registrar_.UpdateInvalidatorState(state); } } // namespace syncer diff --git a/sync/notifier/invalidation_notifier.h b/sync/notifier/invalidation_notifier.h index 789c30d..a1c4328 100644 --- a/sync/notifier/invalidation_notifier.h +++ b/sync/notifier/invalidation_notifier.h @@ -54,17 +54,16 @@ class InvalidationNotifier virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; + virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; virtual void SetStateDeprecated(const std::string& state) OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; - virtual void SendNotification(const ObjectIdStateMap& id_state_map) OVERRIDE; + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) OVERRIDE; // SyncInvalidationListener::Delegate implementation. virtual void OnInvalidate(const ObjectIdStateMap& id_state_map) OVERRIDE; - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE; + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; private: // We start off in the STOPPED state. When we get our initial diff --git a/sync/notifier/invalidation_notifier_unittest.cc b/sync/notifier/invalidation_notifier_unittest.cc index 4265f7e..2e94420 100644 --- a/sync/notifier/invalidation_notifier_unittest.cc +++ b/sync/notifier/invalidation_notifier_unittest.cc @@ -64,20 +64,16 @@ class InvalidationNotifierTestDelegate { message_loop_.RunAllPending(); } - void TriggerOnNotificationsEnabled() { - invalidator_->OnNotificationsEnabled(); + void TriggerOnInvalidatorStateChange(InvalidatorState state) { + invalidator_->OnInvalidatorStateChange(state); } - void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source) { // None of the tests actually care about |source|. invalidator_->OnInvalidate(id_state_map); } - void TriggerOnNotificationsDisabled(NotificationsDisabledReason reason) { - invalidator_->OnNotificationsDisabled(reason); - } - static bool InvalidatorHandlesDeprecatedState() { return true; } diff --git a/sync/notifier/invalidator.h b/sync/notifier/invalidator.h index de33f19..d5435bc 100644 --- a/sync/notifier/invalidator.h +++ b/sync/notifier/invalidator.h @@ -13,6 +13,7 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/notifier/invalidation_util.h" +#include "sync/notifier/invalidator_state.h" #include "sync/notifier/object_id_state_map.h" namespace syncer { @@ -49,7 +50,7 @@ class Invalidator { // invalidator->UnregisterHandler(client_handler); // Starts sending notifications to |handler|. |handler| must not be NULL, - // and it must already be registered. + // and it must not already be registered. virtual void RegisterHandler(InvalidationHandler* handler) = 0; // Updates the set of ObjectIds associated with |handler|. |handler| must @@ -63,6 +64,11 @@ class Invalidator { // associated with |handler|. virtual void UnregisterHandler(InvalidationHandler* handler) = 0; + // Returns the current invalidator state. When called from within + // InvalidationHandler::OnInvalidatorStateChange(), this must return + // the updated state. + virtual InvalidatorState GetInvalidatorState() const = 0; + // SetUniqueId must be called once, before any call to // UpdateCredentials. |unique_id| should be a non-empty globally // unique string. @@ -83,7 +89,7 @@ class Invalidator { // which is still used by sync integration tests. // TODO(akalin): Remove this once we move the integration tests off p2p // notifications. - virtual void SendNotification(const ObjectIdStateMap& id_state_map) = 0; + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) = 0; }; } // namespace syncer diff --git a/sync/notifier/invalidator_factory_unittest.cc b/sync/notifier/invalidator_factory_unittest.cc index 2b17e35..d552efd 100644 --- a/sync/notifier/invalidator_factory_unittest.cc +++ b/sync/notifier/invalidator_factory_unittest.cc @@ -33,7 +33,7 @@ class InvalidatorFactoryTest : public testing::Test { virtual void TearDown() OVERRIDE { message_loop_.RunAllPending(); - EXPECT_EQ(0, fake_handler_.GetNotificationCount()); + EXPECT_EQ(0, fake_handler_.GetInvalidationCount()); } MessageLoop message_loop_; diff --git a/sync/notifier/invalidator_registrar.cc b/sync/notifier/invalidator_registrar.cc index 03b401a..65cb318 100644 --- a/sync/notifier/invalidator_registrar.cc +++ b/sync/notifier/invalidator_registrar.cc @@ -11,7 +11,8 @@ namespace syncer { -InvalidatorRegistrar::InvalidatorRegistrar() {} +InvalidatorRegistrar::InvalidatorRegistrar() + : state_(DEFAULT_INVALIDATION_ERROR) {} InvalidatorRegistrar::~InvalidatorRegistrar() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -88,7 +89,7 @@ ObjectIdSet InvalidatorRegistrar::GetAllRegisteredIds() const { void InvalidatorRegistrar::DispatchInvalidationsToHandlers( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { DCHECK(thread_checker_.CalledOnValidThread()); // If we have no handlers, there's nothing to do. if (!handlers_.might_have_observers()) { @@ -111,20 +112,20 @@ void InvalidatorRegistrar::DispatchInvalidationsToHandlers( while ((handler = it.GetNext()) != NULL) { DispatchMap::const_iterator dispatch_it = dispatch_map.find(handler); if (dispatch_it != dispatch_map.end()) - handler->OnIncomingNotification(dispatch_it->second, source); + handler->OnIncomingInvalidation(dispatch_it->second, source); } } -void InvalidatorRegistrar::EmitOnNotificationsEnabled() { +void InvalidatorRegistrar::UpdateInvalidatorState(InvalidatorState state) { DCHECK(thread_checker_.CalledOnValidThread()); - FOR_EACH_OBSERVER(InvalidationHandler, handlers_, OnNotificationsEnabled()); + state_ = state; + FOR_EACH_OBSERVER(InvalidationHandler, handlers_, + OnInvalidatorStateChange(state)); } -void InvalidatorRegistrar::EmitOnNotificationsDisabled( - NotificationsDisabledReason reason) { +InvalidatorState InvalidatorRegistrar::GetInvalidatorState() const { DCHECK(thread_checker_.CalledOnValidThread()); - FOR_EACH_OBSERVER(InvalidationHandler, handlers_, - OnNotificationsDisabled(reason)); + return state_; } bool InvalidatorRegistrar::IsHandlerRegisteredForTest( diff --git a/sync/notifier/invalidator_registrar.h b/sync/notifier/invalidator_registrar.h index 485b73e..1589c5c 100644 --- a/sync/notifier/invalidator_registrar.h +++ b/sync/notifier/invalidator_registrar.h @@ -55,11 +55,17 @@ class InvalidatorRegistrar { // Invalidations for IDs with no corresponding handler are dropped, as are // invalidations for handlers that are not added. void DispatchInvalidationsToHandlers(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source); + IncomingInvalidationSource source); - // Calls the given handler method for each handler that has registered IDs. - void EmitOnNotificationsEnabled(); - void EmitOnNotificationsDisabled(NotificationsDisabledReason reason); + // Updates the invalidator state to the given one and then notifies + // all handlers. Note that the order is important; handlers that + // call GetInvalidatorState() when notified will see the new state. + void UpdateInvalidatorState(InvalidatorState state); + + // Returns the current invalidator state. When called from within + // InvalidationHandler::OnInvalidatorStateChange(), this returns the + // updated state. + InvalidatorState GetInvalidatorState() const; bool IsHandlerRegisteredForTest(InvalidationHandler* handler) const; @@ -76,6 +82,7 @@ class InvalidatorRegistrar { base::ThreadChecker thread_checker_; ObserverList<InvalidationHandler> handlers_; IdHandlerMap id_to_handler_map_; + InvalidatorState state_; DISALLOW_COPY_AND_ASSIGN(InvalidatorRegistrar); }; diff --git a/sync/notifier/invalidator_registrar_unittest.cc b/sync/notifier/invalidator_registrar_unittest.cc index 90ac22d..f9f478b 100644 --- a/sync/notifier/invalidator_registrar_unittest.cc +++ b/sync/notifier/invalidator_registrar_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "google/cacheinvalidation/types.pb.h" @@ -21,6 +22,7 @@ namespace { // Thin Invalidator wrapper around InvalidatorRegistrar. class RegistrarInvalidator : public Invalidator { public: + RegistrarInvalidator() {} virtual ~RegistrarInvalidator() {} InvalidatorRegistrar* GetRegistrar() { @@ -41,6 +43,10 @@ class RegistrarInvalidator : public Invalidator { registrar_.UnregisterHandler(handler); } + virtual InvalidatorState GetInvalidatorState() const OVERRIDE { + return registrar_.GetInvalidatorState(); + } + virtual void SetUniqueId(const std::string& unique_id) OVERRIDE { // Do nothing. } @@ -54,13 +60,14 @@ class RegistrarInvalidator : public Invalidator { // Do nothing. } - virtual void SendNotification( - const ObjectIdStateMap& id_state_map) OVERRIDE { + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) OVERRIDE { // Do nothing. } private: InvalidatorRegistrar registrar_; + + DISALLOW_COPY_AND_ASSIGN(RegistrarInvalidator); }; class RegistrarInvalidatorTestDelegate { @@ -91,20 +98,16 @@ class RegistrarInvalidatorTestDelegate { // Do nothing. } - void TriggerOnNotificationsEnabled() { - invalidator_->GetRegistrar()->EmitOnNotificationsEnabled(); + void TriggerOnInvalidatorStateChange(InvalidatorState state) { + invalidator_->GetRegistrar()->UpdateInvalidatorState(state); } - void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source) { invalidator_->GetRegistrar()->DispatchInvalidationsToHandlers( id_state_map, source); } - void TriggerOnNotificationsDisabled(NotificationsDisabledReason reason) { - invalidator_->GetRegistrar()->EmitOnNotificationsDisabled(reason); - } - static bool InvalidatorHandlesDeprecatedState() { return false; } diff --git a/sync/notifier/invalidator_state.cc b/sync/notifier/invalidator_state.cc new file mode 100644 index 0000000..0e19222 --- /dev/null +++ b/sync/notifier/invalidator_state.cc @@ -0,0 +1,55 @@ +// 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/notifier/invalidator_state.h" + +#include "base/logging.h" + +namespace syncer { + +const char* InvalidatorStateToString(InvalidatorState state) { + switch (state) { + case TRANSIENT_INVALIDATION_ERROR: + return "TRANSIENT_INVALIDATION_ERROR"; + case INVALIDATION_CREDENTIALS_REJECTED: + return "INVALIDATION_CREDENTIALS_REJECTED"; + case INVALIDATIONS_ENABLED: + return "INVALIDATIONS_ENABLED"; + default: + NOTREACHED(); + return "UNKNOWN_INVALIDATOR_STATE"; + } +} + +InvalidatorState FromNotifierReason( + notifier::NotificationsDisabledReason reason) { + switch (reason) { + case notifier::NO_NOTIFICATION_ERROR: + return INVALIDATIONS_ENABLED; + case notifier::TRANSIENT_NOTIFICATION_ERROR: + return TRANSIENT_INVALIDATION_ERROR; + case notifier::NOTIFICATION_CREDENTIALS_REJECTED: + return INVALIDATION_CREDENTIALS_REJECTED; + default: + NOTREACHED(); + return DEFAULT_INVALIDATION_ERROR; + } +} + +notifier::NotificationsDisabledReason ToNotifierReasonForTest( + InvalidatorState state) { + switch (state) { + case TRANSIENT_INVALIDATION_ERROR: + return notifier::TRANSIENT_NOTIFICATION_ERROR; + case INVALIDATION_CREDENTIALS_REJECTED: + return notifier::NOTIFICATION_CREDENTIALS_REJECTED; + case INVALIDATIONS_ENABLED: + // Fall through. + default: + NOTREACHED(); + return notifier::TRANSIENT_NOTIFICATION_ERROR; + } +} + +} // namespace syncer diff --git a/sync/notifier/invalidator_state.h b/sync/notifier/invalidator_state.h new file mode 100644 index 0000000..f06da6f --- /dev/null +++ b/sync/notifier/invalidator_state.h @@ -0,0 +1,37 @@ +// 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_NOTIFIER_INVALIDATOR_STATE_H_ +#define SYNC_NOTIFIER_INVALIDATOR_STATE_H_ + +#include "jingle/notifier/listener/push_client_observer.h" + +namespace syncer { + +enum InvalidatorState { + // Failure states + // -------------- + // There is an underlying transient problem (e.g., network- or + // XMPP-related). + TRANSIENT_INVALIDATION_ERROR, + DEFAULT_INVALIDATION_ERROR = TRANSIENT_INVALIDATION_ERROR, + // Our credentials have been rejected. + INVALIDATION_CREDENTIALS_REJECTED, + + // Invalidations are fully working. + INVALIDATIONS_ENABLED +}; + +const char* InvalidatorStateToString(InvalidatorState state); + +InvalidatorState FromNotifierReason( + notifier::NotificationsDisabledReason reason); + +// Should not be called when |state| == INVALIDATIONS_ENABLED. +notifier::NotificationsDisabledReason ToNotifierReasonForTest( + InvalidatorState state); + +} // namespace syncer + +#endif // SYNC_NOTIFIER_INVALIDATOR_STATE_H_ diff --git a/sync/notifier/invalidator_test_template.cc b/sync/notifier/invalidator_test_template.cc new file mode 100644 index 0000000..7fc8385 --- /dev/null +++ b/sync/notifier/invalidator_test_template.cc @@ -0,0 +1,28 @@ +// 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/notifier/invalidator_test_template.h" + +namespace syncer { +namespace internal { + +BoundFakeInvalidationHandler::BoundFakeInvalidationHandler( + const Invalidator& invalidator) + : invalidator_(invalidator), + last_retrieved_state_(DEFAULT_INVALIDATION_ERROR) {} + +BoundFakeInvalidationHandler::~BoundFakeInvalidationHandler() {} + +InvalidatorState BoundFakeInvalidationHandler::GetLastRetrievedState() const { + return last_retrieved_state_; +} + +void BoundFakeInvalidationHandler::OnInvalidatorStateChange( + InvalidatorState state) { + FakeInvalidationHandler::OnInvalidatorStateChange(state); + last_retrieved_state_ = invalidator_.GetInvalidatorState(); +} + +} // namespace internal +} // namespace syncer diff --git a/sync/notifier/invalidator_test_template.h b/sync/notifier/invalidator_test_template.h index df25107..8aadade 100644 --- a/sync/notifier/invalidator_test_template.h +++ b/sync/notifier/invalidator_test_template.h @@ -46,25 +46,18 @@ // // The Trigger* functions below should block until the effects of // // the call are visible on the current thread. // -// // Should cause OnNotificationsEnabled() to be called on all -// // observers of the Invalidator implementation. -// void TriggerOnNotificationsEnabled() { -// ... -// } -// -// // Should cause OnIncomingNotification() to be called on all +// // Should cause OnInvalidatorStateChange() to be called on all // // observers of the Invalidator implementation with the given // // parameters. -// void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, -// IncomingNotificationSource source) { +// void TriggerOnInvalidatorStateChange(InvalidatorState state) { // ... // } // -// // Should cause OnNotificationsDisabled() to be called on all +// // Should cause OnIncomingInvalidation() to be called on all // // observers of the Invalidator implementation with the given // // parameters. -// void TriggerOnNotificationsDisabled( -// NotificationsDisabledReason reason) { +// void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, +// IncomingInvalidationSource source) { // ... // } // @@ -90,6 +83,8 @@ #ifndef SYNC_NOTIFIER_INVALIDATOR_TEST_TEMPLATE_H_ #define SYNC_NOTIFIER_INVALIDATOR_TEST_TEMPLATE_H_ +#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "google/cacheinvalidation/include/types.h" #include "google/cacheinvalidation/types.pb.h" #include "sync/notifier/fake_invalidation_handler.h" @@ -159,28 +154,27 @@ TYPED_TEST_P(InvalidatorTest, Basic) { states[this->id3].payload = "3"; // Should be ignored since no IDs are registered to |handler|. - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); - EXPECT_EQ(0, handler.GetNotificationCount()); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); + EXPECT_EQ(0, handler.GetInvalidationCount()); ObjectIdSet ids; ids.insert(this->id1); ids.insert(this->id2); invalidator->UpdateRegisteredIds(&handler, ids); - this->delegate_.TriggerOnNotificationsEnabled(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(INVALIDATIONS_ENABLED); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler.GetInvalidatorState()); ObjectIdStateMap expected_states; expected_states[this->id1].payload = "1"; expected_states[this->id2].payload = "2"; - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); - EXPECT_EQ(1, handler.GetNotificationCount()); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); + EXPECT_EQ(1, handler.GetInvalidationCount()); EXPECT_THAT( expected_states, - Eq(handler.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, handler.GetLastNotificationSource()); + Eq(handler.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, handler.GetLastInvalidationSource()); ids.erase(this->id1); ids.insert(this->id3); @@ -190,33 +184,33 @@ TYPED_TEST_P(InvalidatorTest, Basic) { expected_states[this->id3].payload = "3"; // Removed object IDs should not be notified, newly-added ones should. - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); - EXPECT_EQ(2, handler.GetNotificationCount()); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); + EXPECT_EQ(2, handler.GetInvalidationCount()); EXPECT_THAT( expected_states, - Eq(handler.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, handler.GetLastNotificationSource()); + Eq(handler.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, handler.GetLastInvalidationSource()); - this->delegate_.TriggerOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(TRANSIENT_INVALIDATION_ERROR); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, + handler.GetInvalidatorState()); - this->delegate_.TriggerOnNotificationsDisabled( - NOTIFICATION_CREDENTIALS_REJECTED); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - handler.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange( + INVALIDATION_CREDENTIALS_REJECTED); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, + handler.GetInvalidatorState()); invalidator->UnregisterHandler(&handler); // Should be ignored since |handler| isn't registered anymore. - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); - EXPECT_EQ(2, handler.GetNotificationCount()); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); + EXPECT_EQ(2, handler.GetInvalidationCount()); } // Register handlers and some IDs for those handlers, register a handler with // no IDs, and register a handler with some IDs but unregister it. Then, -// dispatch some notifications and invalidations. Handlers that are registered -// should get notifications, and the ones that have registered IDs should +// dispatch some invalidations and invalidations. Handlers that are registered +// should get invalidations, and the ones that have registered IDs should // receive invalidations for those IDs. TYPED_TEST_P(InvalidatorTest, MultipleHandlers) { Invalidator* const invalidator = this->CreateAndInitializeInvalidator(); @@ -254,15 +248,11 @@ TYPED_TEST_P(InvalidatorTest, MultipleHandlers) { invalidator->UnregisterHandler(&handler4); - this->delegate_.TriggerOnNotificationsEnabled(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler1.GetNotificationsDisabledReason()); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler2.GetNotificationsDisabledReason()); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler3.GetNotificationsDisabledReason()); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler4.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(INVALIDATIONS_ENABLED); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler1.GetInvalidatorState()); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler2.GetInvalidatorState()); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler3.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler4.GetInvalidatorState()); { ObjectIdStateMap states; @@ -270,40 +260,36 @@ TYPED_TEST_P(InvalidatorTest, MultipleHandlers) { states[this->id2].payload = "2"; states[this->id3].payload = "3"; states[this->id4].payload = "4"; - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); ObjectIdStateMap expected_states; expected_states[this->id1].payload = "1"; expected_states[this->id2].payload = "2"; - EXPECT_EQ(1, handler1.GetNotificationCount()); + EXPECT_EQ(1, handler1.GetInvalidationCount()); EXPECT_THAT( expected_states, - Eq(handler1.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, handler1.GetLastNotificationSource()); + Eq(handler1.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, handler1.GetLastInvalidationSource()); expected_states.clear(); expected_states[this->id3].payload = "3"; - EXPECT_EQ(1, handler2.GetNotificationCount()); + EXPECT_EQ(1, handler2.GetInvalidationCount()); EXPECT_THAT( expected_states, - Eq(handler2.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, handler2.GetLastNotificationSource()); + Eq(handler2.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, handler2.GetLastInvalidationSource()); - EXPECT_EQ(0, handler3.GetNotificationCount()); - EXPECT_EQ(0, handler4.GetNotificationCount()); + EXPECT_EQ(0, handler3.GetInvalidationCount()); + EXPECT_EQ(0, handler4.GetInvalidationCount()); } - this->delegate_.TriggerOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler1.GetNotificationsDisabledReason()); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler2.GetNotificationsDisabledReason()); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler3.GetNotificationsDisabledReason()); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler4.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(TRANSIENT_INVALIDATION_ERROR); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler1.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler2.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler3.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler4.GetInvalidatorState()); } // Make sure that passing an empty set to UpdateRegisteredIds clears the @@ -336,27 +322,65 @@ TYPED_TEST_P(InvalidatorTest, EmptySetUnregisters) { // further invalidations. invalidator->UpdateRegisteredIds(&handler1, ObjectIdSet()); - this->delegate_.TriggerOnNotificationsEnabled(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler1.GetNotificationsDisabledReason()); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - handler2.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(INVALIDATIONS_ENABLED); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler1.GetInvalidatorState()); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler2.GetInvalidatorState()); { ObjectIdStateMap states; states[this->id1].payload = "1"; states[this->id2].payload = "2"; states[this->id3].payload = "3"; - this->delegate_.TriggerOnIncomingNotification(states, REMOTE_NOTIFICATION); - EXPECT_EQ(0, handler1.GetNotificationCount()); - EXPECT_EQ(1, handler2.GetNotificationCount()); + this->delegate_.TriggerOnIncomingInvalidation(states, REMOTE_INVALIDATION); + EXPECT_EQ(0, handler1.GetInvalidationCount()); + EXPECT_EQ(1, handler2.GetInvalidationCount()); } - this->delegate_.TriggerOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler1.GetNotificationsDisabledReason()); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - handler2.GetNotificationsDisabledReason()); + this->delegate_.TriggerOnInvalidatorStateChange(TRANSIENT_INVALIDATION_ERROR); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler1.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler2.GetInvalidatorState()); +} + +namespace internal { + +// A FakeInvalidationHandler that is "bound" to a specific +// Invalidator. This is for cross-referencing state information with +// the bound Invalidator. +class BoundFakeInvalidationHandler : public FakeInvalidationHandler { + public: + explicit BoundFakeInvalidationHandler(const Invalidator& invalidator); + virtual ~BoundFakeInvalidationHandler(); + + // Returns the last return value of GetInvalidatorState() on the + // bound invalidator from the last time the invalidator state + // changed. + InvalidatorState GetLastRetrievedState() const; + + // InvalidationHandler implementation. + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; + + private: + const Invalidator& invalidator_; + InvalidatorState last_retrieved_state_; + + DISALLOW_COPY_AND_ASSIGN(BoundFakeInvalidationHandler); +}; + +} // namespace internal + +TYPED_TEST_P(InvalidatorTest, GetInvalidatorStateAlwaysCurrent) { + Invalidator* const invalidator = this->CreateAndInitializeInvalidator(); + + internal::BoundFakeInvalidationHandler handler(*invalidator); + invalidator->RegisterHandler(&handler); + + this->delegate_.TriggerOnInvalidatorStateChange(INVALIDATIONS_ENABLED); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler.GetInvalidatorState()); + EXPECT_EQ(INVALIDATIONS_ENABLED, handler.GetLastRetrievedState()); + + this->delegate_.TriggerOnInvalidatorStateChange(TRANSIENT_INVALIDATION_ERROR); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler.GetInvalidatorState()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, handler.GetLastRetrievedState()); } // Initialize the invalidator with an empty initial state. Call the deprecated @@ -386,7 +410,7 @@ TYPED_TEST_P(InvalidatorTest, MigrateState) { // Pretend that Chrome has shut down. this->delegate_.DestroyInvalidator(); this->delegate_.CreateInvalidator("fake_state", - this->fake_tracker_.AsWeakPtr()); + this->fake_tracker_.AsWeakPtr()); invalidator = this->delegate_.GetInvalidator(); // Should do nothing. @@ -397,7 +421,7 @@ TYPED_TEST_P(InvalidatorTest, MigrateState) { REGISTER_TYPED_TEST_CASE_P(InvalidatorTest, Basic, MultipleHandlers, EmptySetUnregisters, - MigrateState); + GetInvalidatorStateAlwaysCurrent, MigrateState); } // namespace syncer diff --git a/sync/notifier/non_blocking_invalidator.cc b/sync/notifier/non_blocking_invalidator.cc index 1f68cb3..0574415 100644 --- a/sync/notifier/non_blocking_invalidator.cc +++ b/sync/notifier/non_blocking_invalidator.cc @@ -42,12 +42,10 @@ class NonBlockingInvalidator::Core // InvalidationHandler implementation (all called on I/O thread by // InvalidationNotifier). - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange(InvalidatorState reason) OVERRIDE; + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) OVERRIDE; + IncomingInvalidationSource source) OVERRIDE; private: friend class @@ -123,24 +121,18 @@ void NonBlockingInvalidator::Core::UpdateCredentials(const std::string& email, invalidation_notifier_->UpdateCredentials(email, token); } -void NonBlockingInvalidator::Core::OnNotificationsEnabled() { - DCHECK(network_task_runner_->BelongsToCurrentThread()); - delegate_observer_.Call(FROM_HERE, - &InvalidationHandler::OnNotificationsEnabled); -} - -void NonBlockingInvalidator::Core::OnNotificationsDisabled( - NotificationsDisabledReason reason) { +void NonBlockingInvalidator::Core::OnInvalidatorStateChange( + InvalidatorState reason) { DCHECK(network_task_runner_->BelongsToCurrentThread()); delegate_observer_.Call( - FROM_HERE, &InvalidationHandler::OnNotificationsDisabled, reason); + FROM_HERE, &InvalidationHandler::OnInvalidatorStateChange, reason); } -void NonBlockingInvalidator::Core::OnIncomingNotification( - const ObjectIdStateMap& id_state_map, IncomingNotificationSource source) { +void NonBlockingInvalidator::Core::OnIncomingInvalidation( + const ObjectIdStateMap& id_state_map, IncomingInvalidationSource source) { DCHECK(network_task_runner_->BelongsToCurrentThread()); delegate_observer_.Call(FROM_HERE, - &InvalidationHandler::OnIncomingNotification, + &InvalidationHandler::OnIncomingInvalidation, id_state_map, source); } @@ -207,6 +199,11 @@ void NonBlockingInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } +InvalidatorState NonBlockingInvalidator::GetInvalidatorState() const { + DCHECK(parent_task_runner_->BelongsToCurrentThread()); + return registrar_.GetInvalidatorState(); +} + void NonBlockingInvalidator::SetUniqueId(const std::string& unique_id) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); if (!network_task_runner_->PostTask( @@ -239,27 +236,21 @@ void NonBlockingInvalidator::UpdateCredentials(const std::string& email, } } -void NonBlockingInvalidator::SendNotification( +void NonBlockingInvalidator::SendInvalidation( const ObjectIdStateMap& id_state_map) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - // InvalidationClient doesn't implement SendNotification(), so no + // InvalidationClient doesn't implement SendInvalidation(), so no // need to forward on the call. } -void NonBlockingInvalidator::OnNotificationsEnabled() { - DCHECK(parent_task_runner_->BelongsToCurrentThread()); - registrar_.EmitOnNotificationsEnabled(); -} - -void NonBlockingInvalidator::OnNotificationsDisabled( - NotificationsDisabledReason reason) { +void NonBlockingInvalidator::OnInvalidatorStateChange(InvalidatorState state) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - registrar_.EmitOnNotificationsDisabled(reason); + registrar_.UpdateInvalidatorState(state); } -void NonBlockingInvalidator::OnIncomingNotification( +void NonBlockingInvalidator::OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + IncomingInvalidationSource source) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); registrar_.DispatchInvalidationsToHandlers(id_state_map, source); } diff --git a/sync/notifier/non_blocking_invalidator.h b/sync/notifier/non_blocking_invalidator.h index 8e93219..35e870b 100644 --- a/sync/notifier/non_blocking_invalidator.h +++ b/sync/notifier/non_blocking_invalidator.h @@ -50,19 +50,18 @@ class NonBlockingInvalidator virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; + virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; virtual void SetStateDeprecated(const std::string& state) OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; - virtual void SendNotification(const ObjectIdStateMap& id_state_map) OVERRIDE; + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) OVERRIDE; // InvalidationHandler implementation. - virtual void OnNotificationsEnabled() OVERRIDE; - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE; - virtual void OnIncomingNotification( + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) OVERRIDE; + IncomingInvalidationSource source) OVERRIDE; private: class Core; diff --git a/sync/notifier/non_blocking_invalidator_unittest.cc b/sync/notifier/non_blocking_invalidator_unittest.cc index ed7e6bf..f865b4a 100644 --- a/sync/notifier/non_blocking_invalidator_unittest.cc +++ b/sync/notifier/non_blocking_invalidator_unittest.cc @@ -76,17 +76,13 @@ class NonBlockingInvalidatorTestDelegate { run_loop.Run(); } - void TriggerOnNotificationsEnabled() { - invalidator_->OnNotificationsEnabled(); + void TriggerOnInvalidatorStateChange(InvalidatorState state) { + invalidator_->OnInvalidatorStateChange(state); } - void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { - invalidator_->OnIncomingNotification(id_state_map, source); - } - - void TriggerOnNotificationsDisabled(NotificationsDisabledReason reason) { - invalidator_->OnNotificationsDisabled(reason); + void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source) { + invalidator_->OnIncomingInvalidation(id_state_map, source); } static bool InvalidatorHandlesDeprecatedState() { diff --git a/sync/notifier/notifications_disabled_reason.cc b/sync/notifier/notifications_disabled_reason.cc deleted file mode 100644 index 8c1a1aa..0000000 --- a/sync/notifier/notifications_disabled_reason.cc +++ /dev/null @@ -1,56 +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/notifier/notifications_disabled_reason.h" - -#include "base/logging.h" - -namespace syncer { - -const char* NotificationsDisabledReasonToString( - NotificationsDisabledReason reason) { - switch (reason) { - case NO_NOTIFICATION_ERROR: - return "NO_NOTIFICATION_ERROR"; - case TRANSIENT_NOTIFICATION_ERROR: - return "TRANSIENT_NOTIFICATION_ERROR"; - case NOTIFICATION_CREDENTIALS_REJECTED: - return "NOTIFICATION_CREDENTIALS_REJECTED"; - default: - NOTREACHED(); - return "UNKNOWN"; - } -} - -NotificationsDisabledReason FromNotifierReason( - notifier::NotificationsDisabledReason reason) { - switch (reason) { - case notifier::NO_NOTIFICATION_ERROR: - return NO_NOTIFICATION_ERROR; - case notifier::TRANSIENT_NOTIFICATION_ERROR: - return TRANSIENT_NOTIFICATION_ERROR; - case notifier::NOTIFICATION_CREDENTIALS_REJECTED: - return NOTIFICATION_CREDENTIALS_REJECTED; - default: - NOTREACHED(); - return TRANSIENT_NOTIFICATION_ERROR; - } -} - -notifier::NotificationsDisabledReason ToNotifierReasonForTest( - NotificationsDisabledReason reason) { - switch (reason) { - case NO_NOTIFICATION_ERROR: - return notifier::NO_NOTIFICATION_ERROR; - case TRANSIENT_NOTIFICATION_ERROR: - return notifier::TRANSIENT_NOTIFICATION_ERROR; - case NOTIFICATION_CREDENTIALS_REJECTED: - return notifier::NOTIFICATION_CREDENTIALS_REJECTED; - default: - NOTREACHED(); - return notifier::TRANSIENT_NOTIFICATION_ERROR; - } -} - -} // namespace syncer diff --git a/sync/notifier/notifications_disabled_reason.h b/sync/notifier/notifications_disabled_reason.h deleted file mode 100644 index 40805aa..0000000 --- a/sync/notifier/notifications_disabled_reason.h +++ /dev/null @@ -1,35 +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_NOTIFIER_NOTIFICATIONS_DISABLED_REASON_H_ -#define SYNC_NOTIFIER_NOTIFICATIONS_DISABLED_REASON_H_ - -#include "jingle/notifier/listener/push_client_observer.h" - -namespace syncer { - -enum NotificationsDisabledReason { - // There is an underlying transient problem (e.g., network- or - // XMPP-related). - TRANSIENT_NOTIFICATION_ERROR, - DEFAULT_NOTIFICATION_ERROR = TRANSIENT_NOTIFICATION_ERROR, - // Our credentials have been rejected. - NOTIFICATION_CREDENTIALS_REJECTED, - // No error (useful as a default value or to avoid keeping a - // separate bool for notifications enabled/disabled). - NO_NOTIFICATION_ERROR -}; - -const char* NotificationsDisabledReasonToString( - NotificationsDisabledReason reason); - -NotificationsDisabledReason FromNotifierReason( - notifier::NotificationsDisabledReason reason); - -notifier::NotificationsDisabledReason ToNotifierReasonForTest( - NotificationsDisabledReason reason); - -} // namespace syncer - -#endif // SYNC_NOTIFIER_NOTIFICATIONS_DISABLED_REASON_H_ diff --git a/sync/notifier/p2p_invalidator.cc b/sync/notifier/p2p_invalidator.cc index 38c81fb..e5d3b98 100644 --- a/sync/notifier/p2p_invalidator.cc +++ b/sync/notifier/p2p_invalidator.cc @@ -61,21 +61,21 @@ P2PNotificationTarget P2PNotificationTargetFromString( return NOTIFY_SELF; } -IncomingNotificationSource P2PNotificationSourceFromInteger(int source_num) { - if (source_num == LOCAL_NOTIFICATION) { - return LOCAL_NOTIFICATION; +IncomingInvalidationSource P2PNotificationSourceFromInteger(int source_num) { + if (source_num == LOCAL_INVALIDATION) { + return LOCAL_INVALIDATION; } - return REMOTE_NOTIFICATION; + return REMOTE_INVALIDATION; } P2PNotificationData::P2PNotificationData() - : target_(NOTIFY_SELF), source_(REMOTE_NOTIFICATION) {} + : target_(NOTIFY_SELF), source_(REMOTE_INVALIDATION) {} P2PNotificationData::P2PNotificationData( const std::string& sender_id, P2PNotificationTarget target, const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) + IncomingInvalidationSource source) : sender_id_(sender_id), target_(target), id_state_map_(id_state_map), @@ -101,7 +101,7 @@ const ObjectIdStateMap& P2PNotificationData::GetIdStateMap() const { return id_state_map_; } -IncomingNotificationSource P2PNotificationData::GetSource() const { +IncomingInvalidationSource P2PNotificationData::GetSource() const { return source_; } @@ -188,7 +188,7 @@ void P2PInvalidator::UpdateRegisteredIds(InvalidationHandler* handler, registrar_.UpdateRegisteredIds(handler, ids); const P2PNotificationData notification_data( unique_id_, NOTIFY_SELF, ObjectIdSetToStateMap(new_ids, ""), - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); SendNotificationData(notification_data); } @@ -197,6 +197,11 @@ void P2PInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } +InvalidatorState P2PInvalidator::GetInvalidatorState() const { + DCHECK(thread_checker_.CalledOnValidThread()); + return registrar_.GetInvalidatorState(); +} + void P2PInvalidator::SetUniqueId(const std::string& unique_id) { DCHECK(thread_checker_.CalledOnValidThread()); unique_id_ = unique_id; @@ -224,11 +229,11 @@ void P2PInvalidator::UpdateCredentials( logged_in_ = true; } -void P2PInvalidator::SendNotification(const ObjectIdStateMap& id_state_map) { +void P2PInvalidator::SendInvalidation(const ObjectIdStateMap& id_state_map) { DCHECK(thread_checker_.CalledOnValidThread()); const P2PNotificationData notification_data( unique_id_, send_notification_target_, id_state_map, - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); SendNotificationData(notification_data); } @@ -236,12 +241,12 @@ void P2PInvalidator::OnNotificationsEnabled() { DCHECK(thread_checker_.CalledOnValidThread()); bool just_turned_on = (notifications_enabled_ == false); notifications_enabled_ = true; - registrar_.EmitOnNotificationsEnabled(); + registrar_.UpdateInvalidatorState(INVALIDATIONS_ENABLED); if (just_turned_on) { const P2PNotificationData notification_data( unique_id_, NOTIFY_SELF, ObjectIdSetToStateMap(registrar_.GetAllRegisteredIds(), ""), - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); SendNotificationData(notification_data); } } @@ -249,7 +254,7 @@ void P2PInvalidator::OnNotificationsEnabled() { void P2PInvalidator::OnNotificationsDisabled( notifier::NotificationsDisabledReason reason) { DCHECK(thread_checker_.CalledOnValidThread()); - registrar_.EmitOnNotificationsDisabled(FromNotifierReason(reason)); + registrar_.UpdateInvalidatorState(FromNotifierReason(reason)); } void P2PInvalidator::OnIncomingNotification( @@ -276,7 +281,7 @@ void P2PInvalidator::OnIncomingNotification( P2PNotificationData( unique_id_, NOTIFY_ALL, ObjectIdSetToStateMap(registrar_.GetAllRegisteredIds(), ""), - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); } if (!notification_data.IsTargeted(unique_id_)) { DVLOG(1) << "Not a target of the notification -- " @@ -285,7 +290,7 @@ void P2PInvalidator::OnIncomingNotification( } registrar_.DispatchInvalidationsToHandlers( notification_data.GetIdStateMap(), - REMOTE_NOTIFICATION); + REMOTE_INVALIDATION); } void P2PInvalidator::SendNotificationDataForTest( diff --git a/sync/notifier/p2p_invalidator.h b/sync/notifier/p2p_invalidator.h index 99d7fb81..dd462ef 100644 --- a/sync/notifier/p2p_invalidator.h +++ b/sync/notifier/p2p_invalidator.h @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // -// An invalidator that uses p2p notifications based on XMPP push +// An invalidator that uses p2p invalidations based on XMPP push // notifications. Used only for sync integration tests. #ifndef SYNC_NOTIFIER_P2P_INVALIDATOR_H_ @@ -20,7 +20,7 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/notifier/invalidator.h" #include "sync/notifier/invalidator_registrar.h" -#include "sync/notifier/notifications_disabled_reason.h" +#include "sync/notifier/invalidator_state.h" namespace notifier { class PushClient; @@ -57,7 +57,7 @@ class P2PNotificationData { P2PNotificationData(const std::string& sender_id, P2PNotificationTarget target, const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source); + IncomingInvalidationSource source); ~P2PNotificationData(); @@ -66,7 +66,7 @@ class P2PNotificationData { const ObjectIdStateMap& GetIdStateMap() const; - IncomingNotificationSource GetSource() const; + IncomingInvalidationSource GetSource() const; bool Equals(const P2PNotificationData& other) const; @@ -83,8 +83,8 @@ class P2PNotificationData { P2PNotificationTarget target_; // The state map for the notification. ObjectIdStateMap id_state_map_; - // The source of the notification. - IncomingNotificationSource source_; + // The source of the invalidation. + IncomingInvalidationSource source_; }; class P2PInvalidator : public Invalidator, @@ -100,16 +100,17 @@ class P2PInvalidator : public Invalidator, virtual ~P2PInvalidator(); - // Invalidator implementation + // Invalidator implementation. virtual void RegisterHandler(InvalidationHandler* handler) OVERRIDE; virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; + virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; virtual void SetStateDeprecated(const std::string& state) OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; - virtual void SendNotification(const ObjectIdStateMap& id_state_map) OVERRIDE; + virtual void SendInvalidation(const ObjectIdStateMap& id_state_map) OVERRIDE; // PushClientObserver implementation. virtual void OnNotificationsEnabled() OVERRIDE; diff --git a/sync/notifier/p2p_invalidator_unittest.cc b/sync/notifier/p2p_invalidator_unittest.cc index 21abdf2..82e81fe 100644 --- a/sync/notifier/p2p_invalidator_unittest.cc +++ b/sync/notifier/p2p_invalidator_unittest.cc @@ -56,12 +56,16 @@ class P2PInvalidatorTestDelegate { // Do Nothing. } - void TriggerOnNotificationsEnabled() { - fake_push_client_->EnableNotifications(); + void TriggerOnInvalidatorStateChange(InvalidatorState state) { + if (state == INVALIDATIONS_ENABLED) { + fake_push_client_->EnableNotifications(); + } else { + fake_push_client_->DisableNotifications(ToNotifierReasonForTest(state)); + } } - void TriggerOnIncomingNotification(const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) { + void TriggerOnIncomingInvalidation(const ObjectIdStateMap& id_state_map, + IncomingInvalidationSource source) { const P2PNotificationData notification_data( "", NOTIFY_ALL, id_state_map, source); notifier::Notification notification; @@ -70,10 +74,6 @@ class P2PInvalidatorTestDelegate { fake_push_client_->SimulateIncomingNotification(notification); } - void TriggerOnNotificationsDisabled(NotificationsDisabledReason reason) { - fake_push_client_->DisableNotifications(ToNotifierReasonForTest(reason)); - } - static bool InvalidatorHandlesDeprecatedState() { return false; } @@ -136,21 +136,21 @@ TEST_F(P2PInvalidatorTest, P2PNotificationTarget) { TEST_F(P2PInvalidatorTest, P2PNotificationDataIsTargeted) { { const P2PNotificationData notification_data( - "sender", NOTIFY_SELF, ObjectIdStateMap(), REMOTE_NOTIFICATION); + "sender", NOTIFY_SELF, ObjectIdStateMap(), REMOTE_INVALIDATION); EXPECT_TRUE(notification_data.IsTargeted("sender")); EXPECT_FALSE(notification_data.IsTargeted("other1")); EXPECT_FALSE(notification_data.IsTargeted("other2")); } { const P2PNotificationData notification_data( - "sender", NOTIFY_OTHERS, ObjectIdStateMap(), REMOTE_NOTIFICATION); + "sender", NOTIFY_OTHERS, ObjectIdStateMap(), REMOTE_INVALIDATION); EXPECT_FALSE(notification_data.IsTargeted("sender")); EXPECT_TRUE(notification_data.IsTargeted("other1")); EXPECT_TRUE(notification_data.IsTargeted("other2")); } { const P2PNotificationData notification_data( - "sender", NOTIFY_ALL, ObjectIdStateMap(), REMOTE_NOTIFICATION); + "sender", NOTIFY_ALL, ObjectIdStateMap(), REMOTE_INVALIDATION); EXPECT_TRUE(notification_data.IsTargeted("sender")); EXPECT_TRUE(notification_data.IsTargeted("other1")); EXPECT_TRUE(notification_data.IsTargeted("other2")); @@ -182,7 +182,7 @@ TEST_F(P2PInvalidatorTest, P2PNotificationDataNonDefault) { ObjectIdSetToStateMap( ModelTypeSetToObjectIdSet(ModelTypeSet(BOOKMARKS, THEMES)), ""); const P2PNotificationData notification_data( - "sender", NOTIFY_ALL, id_state_map, LOCAL_NOTIFICATION); + "sender", NOTIFY_ALL, id_state_map, LOCAL_INVALIDATION); EXPECT_TRUE(notification_data.IsTargeted("sender")); EXPECT_TRUE(notification_data.IsTargeted("other1")); EXPECT_TRUE(notification_data.IsTargeted("other2")); @@ -233,15 +233,14 @@ TEST_F(P2PInvalidatorTest, NotificationsBasic) { ReflectSentNotifications(); push_client->EnableNotifications(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - fake_handler_.GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, fake_handler_.GetInvalidatorState()); ReflectSentNotifications(); - EXPECT_EQ(1, fake_handler_.GetNotificationCount()); + EXPECT_EQ(1, fake_handler_.GetInvalidationCount()); EXPECT_THAT( ModelTypeStateMapToObjectIdStateMap(MakeStateMap(enabled_types)), - Eq(fake_handler_.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, fake_handler_.GetLastNotificationSource()); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, fake_handler_.GetLastInvalidationSource()); // Sent with target NOTIFY_OTHERS so should not be propagated to // |fake_handler_|. @@ -249,11 +248,11 @@ TEST_F(P2PInvalidatorTest, NotificationsBasic) { const ObjectIdStateMap& id_state_map = ObjectIdSetToStateMap( ModelTypeSetToObjectIdSet(ModelTypeSet(THEMES, APPS)), ""); - invalidator->SendNotification(id_state_map); + invalidator->SendInvalidation(id_state_map); } ReflectSentNotifications(); - EXPECT_EQ(1, fake_handler_.GetNotificationCount()); + EXPECT_EQ(1, fake_handler_.GetInvalidationCount()); } // Set up the P2PInvalidator and send out notifications with various @@ -279,20 +278,19 @@ TEST_F(P2PInvalidatorTest, SendNotificationData) { ReflectSentNotifications(); push_client->EnableNotifications(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, - fake_handler_.GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, fake_handler_.GetInvalidatorState()); ReflectSentNotifications(); - EXPECT_EQ(1, fake_handler_.GetNotificationCount()); + EXPECT_EQ(1, fake_handler_.GetInvalidationCount()); EXPECT_THAT( ModelTypeStateMapToObjectIdStateMap(MakeStateMap(enabled_types)), - Eq(fake_handler_.GetLastNotificationIdStateMap())); - EXPECT_EQ(REMOTE_NOTIFICATION, fake_handler_.GetLastNotificationSource()); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); + EXPECT_EQ(REMOTE_INVALIDATION, fake_handler_.GetLastInvalidationSource()); // Should be dropped. invalidator->SendNotificationDataForTest(P2PNotificationData()); ReflectSentNotifications(); - EXPECT_EQ(1, fake_handler_.GetNotificationCount()); + EXPECT_EQ(1, fake_handler_.GetInvalidationCount()); const ObjectIdStateMap& expected_ids = ModelTypeStateMapToObjectIdStateMap(MakeStateMap(expected_types)); @@ -300,77 +298,77 @@ TEST_F(P2PInvalidatorTest, SendNotificationData) { // Should be propagated. invalidator->SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_SELF, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(2, fake_handler_.GetNotificationCount()); + EXPECT_EQ(2, fake_handler_.GetInvalidationCount()); EXPECT_THAT( expected_ids, - Eq(fake_handler_.GetLastNotificationIdStateMap())); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); // Should be dropped. invalidator->SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_SELF, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(2, fake_handler_.GetNotificationCount()); + EXPECT_EQ(2, fake_handler_.GetInvalidationCount()); // Should be dropped. invalidator->SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_SELF, - ObjectIdStateMap(), REMOTE_NOTIFICATION)); + ObjectIdStateMap(), REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(2, fake_handler_.GetNotificationCount()); + EXPECT_EQ(2, fake_handler_.GetInvalidationCount()); // Should be dropped. invalidator->SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_OTHERS, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(2, fake_handler_.GetNotificationCount()); + EXPECT_EQ(2, fake_handler_.GetInvalidationCount()); // Should be propagated. invalidator->SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_OTHERS, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(3, fake_handler_.GetNotificationCount()); + EXPECT_EQ(3, fake_handler_.GetInvalidationCount()); EXPECT_THAT( expected_ids, - Eq(fake_handler_.GetLastNotificationIdStateMap())); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); // Should be dropped. invalidator->SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_OTHERS, - ObjectIdStateMap(), REMOTE_NOTIFICATION)); + ObjectIdStateMap(), REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(3, fake_handler_.GetNotificationCount()); + EXPECT_EQ(3, fake_handler_.GetInvalidationCount()); // Should be propagated. invalidator->SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_ALL, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(4, fake_handler_.GetNotificationCount()); + EXPECT_EQ(4, fake_handler_.GetInvalidationCount()); EXPECT_THAT( expected_ids, - Eq(fake_handler_.GetLastNotificationIdStateMap())); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); // Should be propagated. invalidator->SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_ALL, - id_state_map, REMOTE_NOTIFICATION)); + id_state_map, REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(5, fake_handler_.GetNotificationCount()); + EXPECT_EQ(5, fake_handler_.GetInvalidationCount()); EXPECT_THAT( expected_ids, - Eq(fake_handler_.GetLastNotificationIdStateMap())); + Eq(fake_handler_.GetLastInvalidationIdStateMap())); // Should be dropped. invalidator->SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_ALL, - ObjectIdStateMap(), REMOTE_NOTIFICATION)); + ObjectIdStateMap(), REMOTE_INVALIDATION)); ReflectSentNotifications(); - EXPECT_EQ(5, fake_handler_.GetNotificationCount()); + EXPECT_EQ(5, fake_handler_.GetInvalidationCount()); } INSTANTIATE_TYPED_TEST_CASE_P( diff --git a/sync/notifier/sync_invalidation_listener.cc b/sync/notifier/sync_invalidation_listener.cc index b75b15c..9e8b2ad 100644 --- a/sync/notifier/sync_invalidation_listener.cc +++ b/sync/notifier/sync_invalidation_listener.cc @@ -34,8 +34,8 @@ SyncInvalidationListener::SyncInvalidationListener( sync_system_resources_(push_client.Pass(), ALLOW_THIS_IN_INITIALIZER_LIST(this)), delegate_(NULL), - ticl_state_(DEFAULT_NOTIFICATION_ERROR), - push_client_state_(DEFAULT_NOTIFICATION_ERROR) { + ticl_state_(DEFAULT_INVALIDATION_ERROR), + push_client_state_(DEFAULT_INVALIDATION_ERROR) { DCHECK(CalledOnValidThread()); push_client_->AddObserver(this); } @@ -105,10 +105,10 @@ void SyncInvalidationListener::UpdateCredentials( void SyncInvalidationListener::UpdateRegisteredIds(const ObjectIdSet& ids) { DCHECK(CalledOnValidThread()); registered_ids_ = ids; - // |ticl_state_| can go to NO_NOTIFICATION_ERROR even without a + // |ticl_state_| can go to INVALIDATIONS_ENABLED even without a // working XMPP connection (as observed by us), so check it instead // of GetState() (see http://crbug.com/139424). - if (ticl_state_ == NO_NOTIFICATION_ERROR && registration_manager_.get()) { + if (ticl_state_ == INVALIDATIONS_ENABLED && registration_manager_.get()) { DoRegistrationUpdate(); } } @@ -117,7 +117,7 @@ void SyncInvalidationListener::Ready( invalidation::InvalidationClient* client) { DCHECK(CalledOnValidThread()); DCHECK_EQ(client, invalidation_client_.get()); - ticl_state_ = NO_NOTIFICATION_ERROR; + ticl_state_ = INVALIDATIONS_ENABLED; EmitStateChange(); DoRegistrationUpdate(); } @@ -274,9 +274,9 @@ void SyncInvalidationListener::InformError( << error_info.error_message() << " (transient = " << error_info.is_transient() << ")"; if (error_info.error_reason() == invalidation::ErrorReason::AUTH_FAILURE) { - ticl_state_ = NOTIFICATION_CREDENTIALS_REJECTED; + ticl_state_ = INVALIDATION_CREDENTIALS_REJECTED; } else { - ticl_state_ = TRANSIENT_NOTIFICATION_ERROR; + ticl_state_ = TRANSIENT_INVALIDATION_ERROR; } EmitStateChange(); } @@ -316,40 +316,36 @@ void SyncInvalidationListener::Stop() { invalidation_state_tracker_.Reset(); max_invalidation_versions_.clear(); - ticl_state_ = DEFAULT_NOTIFICATION_ERROR; - push_client_state_ = DEFAULT_NOTIFICATION_ERROR; + ticl_state_ = DEFAULT_INVALIDATION_ERROR; + push_client_state_ = DEFAULT_INVALIDATION_ERROR; } -NotificationsDisabledReason SyncInvalidationListener::GetState() const { +InvalidatorState SyncInvalidationListener::GetState() const { DCHECK(CalledOnValidThread()); - if (ticl_state_ == NOTIFICATION_CREDENTIALS_REJECTED || - push_client_state_ == NOTIFICATION_CREDENTIALS_REJECTED) { + if (ticl_state_ == INVALIDATION_CREDENTIALS_REJECTED || + push_client_state_ == INVALIDATION_CREDENTIALS_REJECTED) { // If either the ticl or the push client rejected our credentials, - // return NOTIFICATION_CREDENTIALS_REJECTED. - return NOTIFICATION_CREDENTIALS_REJECTED; + // return INVALIDATION_CREDENTIALS_REJECTED. + return INVALIDATION_CREDENTIALS_REJECTED; } - if (ticl_state_ == NO_NOTIFICATION_ERROR && - push_client_state_ == NO_NOTIFICATION_ERROR) { + if (ticl_state_ == INVALIDATIONS_ENABLED && + push_client_state_ == INVALIDATIONS_ENABLED) { // If the ticl is ready and the push client notifications are - // enabled, return NO_NOTIFICATION_ERROR. - return NO_NOTIFICATION_ERROR; + // enabled, return INVALIDATIONS_ENABLED. + return INVALIDATIONS_ENABLED; } // Otherwise, we have a transient error. - return TRANSIENT_NOTIFICATION_ERROR; + return TRANSIENT_INVALIDATION_ERROR; } void SyncInvalidationListener::EmitStateChange() { DCHECK(CalledOnValidThread()); - if (GetState() == NO_NOTIFICATION_ERROR) { - delegate_->OnNotificationsEnabled(); - } else { - delegate_->OnNotificationsDisabled(GetState()); - } + delegate_->OnInvalidatorStateChange(GetState()); } void SyncInvalidationListener::OnNotificationsEnabled() { DCHECK(CalledOnValidThread()); - push_client_state_ = NO_NOTIFICATION_ERROR; + push_client_state_ = INVALIDATIONS_ENABLED; EmitStateChange(); } diff --git a/sync/notifier/sync_invalidation_listener.h b/sync/notifier/sync_invalidation_listener.h index 860f6e7..ed7f031 100644 --- a/sync/notifier/sync_invalidation_listener.h +++ b/sync/notifier/sync_invalidation_listener.h @@ -20,7 +20,7 @@ #include "jingle/notifier/listener/push_client_observer.h" #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/invalidation_state_tracker.h" -#include "sync/notifier/notifications_disabled_reason.h" +#include "sync/notifier/invalidator_state.h" #include "sync/notifier/object_id_state_map.h" #include "sync/notifier/state_writer.h" #include "sync/notifier/sync_system_resources.h" @@ -58,10 +58,7 @@ class SyncInvalidationListener virtual void OnInvalidate(const ObjectIdStateMap& id_state_map) = 0; - virtual void OnNotificationsEnabled() = 0; - - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) = 0; + virtual void OnInvalidatorStateChange(InvalidatorState state) = 0; }; explicit SyncInvalidationListener( @@ -135,7 +132,7 @@ class SyncInvalidationListener private: void Stop(); - NotificationsDisabledReason GetState() const; + InvalidatorState GetState() const; void EmitStateChange(); @@ -152,10 +149,9 @@ class SyncInvalidationListener // Stored to pass to |registration_manager_| on start. ObjectIdSet registered_ids_; - // The states of the ticl and the push client (with - // NO_NOTIFICATION_ERROR meaning notifications are enabled). - NotificationsDisabledReason ticl_state_; - NotificationsDisabledReason push_client_state_; + // The states of the ticl and the push client. + InvalidatorState ticl_state_; + InvalidatorState push_client_state_; DISALLOW_COPY_AND_ASSIGN(SyncInvalidationListener); }; diff --git a/sync/notifier/sync_invalidation_listener_unittest.cc b/sync/notifier/sync_invalidation_listener_unittest.cc index 2f0deac..11e35fc 100644 --- a/sync/notifier/sync_invalidation_listener_unittest.cc +++ b/sync/notifier/sync_invalidation_listener_unittest.cc @@ -131,7 +131,7 @@ class FakeInvalidationClient : public invalidation::InvalidationClient { // and state. class FakeDelegate : public SyncInvalidationListener::Delegate { public: - FakeDelegate() : reason_(TRANSIENT_NOTIFICATION_ERROR) {} + FakeDelegate() : state_(TRANSIENT_INVALIDATION_ERROR) {} virtual ~FakeDelegate() {} int GetInvalidationCount(const ObjectId& id) const { @@ -144,9 +144,8 @@ class FakeDelegate : public SyncInvalidationListener::Delegate { return (it == states_.end()) ? "" : it->second.payload; } - // NO_NOTIFICATION_ERROR is the enabled state. - NotificationsDisabledReason GetNotificationsDisabledReason() const { - return reason_; + InvalidatorState GetInvalidatorState() const { + return state_; } // SyncInvalidationListener::Delegate implementation. @@ -159,19 +158,15 @@ class FakeDelegate : public SyncInvalidationListener::Delegate { } } - virtual void OnNotificationsEnabled() { - reason_ = NO_NOTIFICATION_ERROR; - } - - virtual void OnNotificationsDisabled(NotificationsDisabledReason reason) { - reason_ = reason; + virtual void OnInvalidatorStateChange(InvalidatorState state) { + state_ = state; } private: typedef std::map<ObjectId, int, ObjectIdLessThan> ObjectIdCountMap; ObjectIdCountMap invalidation_counts_; ObjectIdStateMap states_; - NotificationsDisabledReason reason_; + InvalidatorState state_; }; invalidation::InvalidationClient* CreateFakeInvalidationClient( @@ -222,8 +217,8 @@ class SyncInvalidationListenerTest : public testing::Test { return fake_delegate_.GetPayload(id); } - NotificationsDisabledReason GetNotificationsDisabledReason() const { - return fake_delegate_.GetNotificationsDisabledReason(); + InvalidatorState GetInvalidatorState() const { + return fake_delegate_.GetInvalidatorState(); } int64 GetMaxVersion(const ObjectId& id) const { @@ -645,55 +640,51 @@ TEST_F(SyncInvalidationListenerTest, RegisterTypesPreserved) { } // Without readying the client, disable notifications, then enable -// them. The delegate should still think notifications are disabled. +// them. The listener should still think notifications are disabled. TEST_F(SyncInvalidationListenerTest, EnableNotificationsNotReady) { - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, + GetInvalidatorState()); DisableNotifications( notifier::TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); - DisableNotifications( - notifier::NOTIFICATION_CREDENTIALS_REJECTED); + DisableNotifications(notifier::NOTIFICATION_CREDENTIALS_REJECTED); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); EnableNotifications(); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, - GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); } // Enable notifications then Ready the invalidation client. The // delegate should then be ready. TEST_F(SyncInvalidationListenerTest, EnableNotificationsThenReady) { - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); EnableNotifications(); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); client_.Ready(fake_invalidation_client_); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); } // Ready the invalidation client then enable notifications. The // delegate should then be ready. TEST_F(SyncInvalidationListenerTest, ReadyThenEnableNotifications) { - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); client_.Ready(fake_invalidation_client_); - EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(TRANSIENT_INVALIDATION_ERROR, GetInvalidatorState()); EnableNotifications(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); } // Enable notifications and ready the client. Then disable @@ -703,17 +694,16 @@ TEST_F(SyncInvalidationListenerTest, PushClientAuthError) { EnableNotifications(); client_.Ready(fake_invalidation_client_); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); DisableNotifications( notifier::NOTIFICATION_CREDENTIALS_REJECTED); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); EnableNotifications(); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); } // Enable notifications and ready the client. Then simulate an auth @@ -724,7 +714,7 @@ TEST_F(SyncInvalidationListenerTest, InvalidationClientAuthError) { EnableNotifications(); client_.Ready(fake_invalidation_client_); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); client_.InformError( fake_invalidation_client_, @@ -734,29 +724,23 @@ TEST_F(SyncInvalidationListenerTest, InvalidationClientAuthError) { "auth error", invalidation::ErrorContext())); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); - DisableNotifications( - notifier::TRANSIENT_NOTIFICATION_ERROR); + DisableNotifications(notifier::TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); - DisableNotifications( - notifier::TRANSIENT_NOTIFICATION_ERROR); + DisableNotifications(notifier::TRANSIENT_NOTIFICATION_ERROR); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); EnableNotifications(); - EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED, - GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATION_CREDENTIALS_REJECTED, GetInvalidatorState()); client_.Ready(fake_invalidation_client_); - EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason()); + EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); } } // namespace diff --git a/sync/sync.gyp b/sync/sync.gyp index ab54597..decc13c 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -269,8 +269,8 @@ 'notifier/invalidator.h', 'notifier/invalidator_registrar.cc', 'notifier/invalidator_registrar.h', - 'notifier/notifications_disabled_reason.cc', - 'notifier/notifications_disabled_reason.h', + 'notifier/invalidator_state.cc', + 'notifier/invalidator_state.h', 'notifier/object_id_state_map.cc', 'notifier/object_id_state_map.h', ], @@ -474,16 +474,17 @@ { 'target_name': 'test_support_sync_notifier', 'type': 'static_library', - 'variables': { 'enable_wexit_time_destructors': 1, }, 'include_dirs': [ '..', ], 'dependencies': [ '../testing/gmock.gyp:gmock', + '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation_proto_cpp', 'sync_notifier', ], 'export_dependent_settings': [ '../testing/gmock.gyp:gmock', + '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation_proto_cpp', 'sync_notifier', ], 'sources': [ @@ -493,6 +494,7 @@ 'notifier/fake_invalidator.h', 'notifier/fake_invalidation_handler.cc', 'notifier/fake_invalidation_handler.h', + 'notifier/invalidator_test_template.cc', 'notifier/invalidator_test_template.h', 'notifier/object_id_state_map_test_util.cc', 'notifier/object_id_state_map_test_util.h', diff --git a/sync/tools/sync_listen_notifications.cc b/sync/tools/sync_listen_notifications.cc index 24789fd7..5a832c2 100644 --- a/sync/tools/sync_listen_notifications.cc +++ b/sync/tools/sync_listen_notifications.cc @@ -54,25 +54,20 @@ class NotificationPrinter : public InvalidationHandler { NotificationPrinter() {} virtual ~NotificationPrinter() {} - virtual void OnNotificationsEnabled() OVERRIDE { - LOG(INFO) << "Notifications enabled"; + virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE { + LOG(INFO) << "Invalidator state changed to " + << InvalidatorStateToString(state); } - virtual void OnNotificationsDisabled( - NotificationsDisabledReason reason) OVERRIDE { - LOG(INFO) << "Notifications disabled with reason " - << NotificationsDisabledReasonToString(reason); - } - - virtual void OnIncomingNotification( + virtual void OnIncomingInvalidation( const ObjectIdStateMap& id_state_map, - IncomingNotificationSource source) OVERRIDE { + IncomingInvalidationSource source) OVERRIDE { const ModelTypeStateMap& type_state_map = ObjectIdStateMapToModelTypeStateMap(id_state_map); for (ModelTypeStateMap::const_iterator it = type_state_map.begin(); it != type_state_map.end(); ++it) { - LOG(INFO) << (source == REMOTE_NOTIFICATION ? "Remote" : "Local") - << " Notification: type = " + LOG(INFO) << (source == REMOTE_INVALIDATION ? "Remote" : "Local") + << " Invalidation: type = " << ModelTypeToString(it->first) << ", payload = " << it->second.payload; } |