diff options
author | dcheng@chromium.org <dcheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-07-21 19:40:33 +0000 |
---|---|---|
committer | dcheng@chromium.org <dcheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-07-21 19:40:33 +0000 |
commit | 877197584abc8883cbccc3bd83240a9be6c2e16e (patch) | |
tree | 36da6824a41c80c87f1b95dc1c8cf1f4a9e4571c /sync/notifier | |
parent | 8e2dd2741c41e5090aee5f8bc31349ff6d9c0fed (diff) | |
download | chromium_src-877197584abc8883cbccc3bd83240a9be6c2e16e.zip chromium_src-877197584abc8883cbccc3bd83240a9be6c2e16e.tar.gz chromium_src-877197584abc8883cbccc3bd83240a9be6c2e16e.tar.bz2 |
Refactor sync-specific parts out of SyncNotifier/SyncNotifierObserver
Sort of. SendNotification() is still there. Perhaps we want to split the interfaces completely.
BUG=124149
TEST=tests should still pass, no observable behavior change
Review URL: https://chromiumcodereview.appspot.com/10702074
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@147801 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/notifier')
21 files changed, 544 insertions, 199 deletions
diff --git a/sync/notifier/chrome_invalidation_client.h b/sync/notifier/chrome_invalidation_client.h index ddd5bb0..2328f99a 100644 --- a/sync/notifier/chrome_invalidation_client.h +++ b/sync/notifier/chrome_invalidation_client.h @@ -8,7 +8,6 @@ #ifndef SYNC_NOTIFIER_CHROME_INVALIDATION_CLIENT_H_ #define SYNC_NOTIFIER_CHROME_INVALIDATION_CLIENT_H_ -#include <map> #include <string> #include "base/basictypes.h" @@ -21,8 +20,8 @@ #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/chrome_system_resources.h" #include "sync/notifier/invalidation_state_tracker.h" -#include "sync/notifier/invalidation_util.h" #include "sync/notifier/notifications_disabled_reason.h" +#include "sync/notifier/object_id_payload_map.h" #include "sync/notifier/state_writer.h" namespace buzz { @@ -37,10 +36,6 @@ namespace syncer { class RegistrationManager; -typedef std::map<invalidation::ObjectId, - std::string, - ObjectIdLessThan> ObjectIdPayloadMap; - // ChromeInvalidationClient is not thread-safe and lives on the sync // thread. class ChromeInvalidationClient diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc index 0c629c6..3a72f32 100644 --- a/sync/notifier/invalidation_notifier.cc +++ b/sync/notifier/invalidation_notifier.cc @@ -34,14 +34,10 @@ InvalidationNotifier::~InvalidationNotifier() { DCHECK(CalledOnValidThread()); } -void InvalidationNotifier::AddObserver(SyncNotifierObserver* observer) { +void InvalidationNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) { DCHECK(CalledOnValidThread()); - observers_.AddObserver(observer); -} - -void InvalidationNotifier::RemoveObserver(SyncNotifierObserver* observer) { - DCHECK(CalledOnValidThread()); - observers_.RemoveObserver(observer); + invalidation_client_.RegisterIds(helper_.UpdateRegisteredIds(handler, ids)); } void InvalidationNotifier::SetUniqueId(const std::string& unique_id) { @@ -85,22 +81,6 @@ void InvalidationNotifier::UpdateCredentials( invalidation_client_.UpdateCredentials(email, token); } -void InvalidationNotifier::UpdateEnabledTypes(ModelTypeSet enabled_types) { - DCHECK(CalledOnValidThread()); - CHECK(!invalidation_client_id_.empty()); - ObjectIdSet ids; - for (ModelTypeSet::Iterator it = enabled_types.First(); it.Good(); - it.Inc()) { - invalidation::ObjectId id; - if (!RealModelTypeToObjectId(it.Get(), &id)) { - DLOG(WARNING) << "Invalid model type " << it.Get(); - continue; - } - ids.insert(id); - } - invalidation_client_.RegisterIds(ids); -} - void InvalidationNotifier::SendNotification(ModelTypeSet changed_types) { DCHECK(CalledOnValidThread()); // Do nothing. @@ -108,33 +88,18 @@ void InvalidationNotifier::SendNotification(ModelTypeSet changed_types) { void InvalidationNotifier::OnInvalidate(const ObjectIdPayloadMap& id_payloads) { DCHECK(CalledOnValidThread()); - // TODO(dcheng): This should probably be a utility function somewhere... - ModelTypePayloadMap type_payloads; - for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin(); - it != id_payloads.end(); ++it) { - ModelType model_type; - if (!ObjectIdToRealModelType(it->first, &model_type)) { - DLOG(WARNING) << "Invalid object ID: " << ObjectIdToString(it->first); - continue; - } - type_payloads[model_type] = it->second; - } - FOR_EACH_OBSERVER( - SyncNotifierObserver, observers_, - OnIncomingNotification(type_payloads, REMOTE_NOTIFICATION)); + helper_.DispatchInvalidationsToHandlers(id_payloads, REMOTE_NOTIFICATION); } void InvalidationNotifier::OnNotificationsEnabled() { DCHECK(CalledOnValidThread()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observers_, - OnNotificationsEnabled()); + helper_.EmitOnNotificationsEnabled(); } void InvalidationNotifier::OnNotificationsDisabled( NotificationsDisabledReason reason) { DCHECK(CalledOnValidThread()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observers_, - OnNotificationsDisabled(reason)); + helper_.EmitOnNotificationsDisabled(reason); } } // namespace syncer diff --git a/sync/notifier/invalidation_notifier.h b/sync/notifier/invalidation_notifier.h index c287efb..8d74552 100644 --- a/sync/notifier/invalidation_notifier.h +++ b/sync/notifier/invalidation_notifier.h @@ -17,13 +17,13 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" -#include "base/observer_list.h" #include "base/threading/non_thread_safe.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/chrome_invalidation_client.h" #include "sync/notifier/invalidation_state_tracker.h" #include "sync/notifier/sync_notifier.h" +#include "sync/notifier/sync_notifier_helper.h" namespace notifier { class PushClient; @@ -49,13 +49,12 @@ class InvalidationNotifier virtual ~InvalidationNotifier(); // SyncNotifier implementation. - virtual void AddObserver(SyncNotifierObserver* observer) OVERRIDE; - virtual void RemoveObserver(SyncNotifierObserver* observer) OVERRIDE; + virtual void UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) 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 UpdateEnabledTypes(ModelTypeSet enabled_types) OVERRIDE; virtual void SendNotification(ModelTypeSet changed_types) OVERRIDE; // ChromeInvalidationClient::Listener implementation. @@ -76,6 +75,8 @@ class InvalidationNotifier }; State state_; + SyncNotifierHelper helper_; + // Passed to |invalidation_client_|. const InvalidationVersionMap initial_max_invalidation_versions_; @@ -86,9 +87,6 @@ class InvalidationNotifier // Passed to |invalidation_client_|. const std::string client_info_; - // Our observers (which must live on the same thread). - ObserverList<SyncNotifierObserver> observers_; - // The client ID to pass to |chrome_invalidation_client_|. std::string invalidation_client_id_; diff --git a/sync/notifier/invalidation_notifier_unittest.cc b/sync/notifier/invalidation_notifier_unittest.cc index b0b64c7..4a07678 100644 --- a/sync/notifier/invalidation_notifier_unittest.cc +++ b/sync/notifier/invalidation_notifier_unittest.cc @@ -50,11 +50,10 @@ class InvalidationNotifierTest : public testing::Test { initial_invalidation_state, MakeWeakHandle(mock_tracker_.AsWeakPtr()), "fake_client_info")); - invalidation_notifier_->AddObserver(&mock_observer_); } void ResetNotifier() { - invalidation_notifier_->RemoveObserver(&mock_observer_); + invalidation_notifier_->UpdateRegisteredIds(&mock_observer_, ObjectIdSet()); // Stopping the invalidation notifier stops its scheduler, which deletes any // pending tasks without running them. Some tasks "run and delete" another // task, so they must be run in order to avoid leaking the inner task. @@ -75,15 +74,16 @@ TEST_F(InvalidationNotifierTest, Basic) { CreateAndObserveNotifier("fake_state"); InSequence dummy; - ModelTypePayloadMap type_payloads; - type_payloads[PREFERENCES] = "payload"; - type_payloads[BOOKMARKS] = "payload"; - type_payloads[AUTOFILL] = "payload"; + ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL); + invalidation_notifier_->UpdateRegisteredIds( + &mock_observer_, ModelTypeSetToObjectIdSet(models)); + const ModelTypePayloadMap& type_payloads = + ModelTypePayloadMapFromEnumSet(models, "payload"); EXPECT_CALL(mock_observer_, OnNotificationsEnabled()); - EXPECT_CALL(mock_observer_, - OnIncomingNotification(type_payloads, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(type_payloads), + REMOTE_NOTIFICATION)); EXPECT_CALL(mock_observer_, OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR)); EXPECT_CALL(mock_observer_, @@ -99,14 +99,8 @@ TEST_F(InvalidationNotifierTest, Basic) { invalidation_notifier_->OnNotificationsEnabled(); - ObjectIdPayloadMap id_payloads; - for (ModelTypePayloadMap::const_iterator it = type_payloads.begin(); - it != type_payloads.end(); ++it) { - invalidation::ObjectId id; - ASSERT_TRUE(RealModelTypeToObjectId(it->first, &id)); - id_payloads[id] = "payload"; - } - invalidation_notifier_->OnInvalidate(id_payloads); + invalidation_notifier_->OnInvalidate( + ModelTypePayloadMapToObjectIdPayloadMap(type_payloads)); invalidation_notifier_->OnNotificationsDisabled( TRANSIENT_NOTIFICATION_ERROR); diff --git a/sync/notifier/invalidation_util.cc b/sync/notifier/invalidation_util.cc index 0065b50..5b18d66 100644 --- a/sync/notifier/invalidation_util.cc +++ b/sync/notifier/invalidation_util.cc @@ -33,6 +33,32 @@ bool ObjectIdToRealModelType(const invalidation::ObjectId& object_id, return NotificationTypeToRealModelType(object_id.name(), model_type); } +ObjectIdSet ModelTypeSetToObjectIdSet(const ModelTypeSet& model_types) { + ObjectIdSet ids; + for (ModelTypeSet::Iterator it = model_types.First(); it.Good(); it.Inc()) { + invalidation::ObjectId model_type_as_id; + if (!RealModelTypeToObjectId(it.Get(), &model_type_as_id)) { + DLOG(WARNING) << "Invalid model type " << it.Get(); + continue; + } + ids.insert(model_type_as_id); + } + return ids; +} + +ModelTypeSet ObjectIdSetToModelTypeSet(const ObjectIdSet& ids) { + ModelTypeSet model_types; + for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { + ModelType model_type; + if (!ObjectIdToRealModelType(*it, &model_type)) { + DLOG(WARNING) << "Invalid object ID " << ObjectIdToString(*it); + continue; + } + model_types.Put(model_type); + } + return model_types; +} + std::string ObjectIdToString( const invalidation::ObjectId& object_id) { std::stringstream ss; diff --git a/sync/notifier/invalidation_util.h b/sync/notifier/invalidation_util.h index 1a706ef..bc43523 100644 --- a/sync/notifier/invalidation_util.h +++ b/sync/notifier/invalidation_util.h @@ -34,6 +34,9 @@ bool RealModelTypeToObjectId(ModelType model_type, bool ObjectIdToRealModelType(const invalidation::ObjectId& object_id, ModelType* model_type); +ObjectIdSet ModelTypeSetToObjectIdSet(const ModelTypeSet& models); +ModelTypeSet ObjectIdSetToModelTypeSet(const ObjectIdSet& ids); + std::string ObjectIdToString(const invalidation::ObjectId& object_id); std::string InvalidationToString( diff --git a/sync/notifier/mock_sync_notifier_observer.h b/sync/notifier/mock_sync_notifier_observer.h index b3baeb9..fc88c52 100644 --- a/sync/notifier/mock_sync_notifier_observer.h +++ b/sync/notifier/mock_sync_notifier_observer.h @@ -20,7 +20,7 @@ class MockSyncNotifierObserver : public SyncNotifierObserver { MOCK_METHOD0(OnNotificationsEnabled, void()); MOCK_METHOD1(OnNotificationsDisabled, void(NotificationsDisabledReason)); MOCK_METHOD2(OnIncomingNotification, - void(const ModelTypePayloadMap&, IncomingNotificationSource)); + void(const ObjectIdPayloadMap&, IncomingNotificationSource)); }; } // namespace syncer diff --git a/sync/notifier/non_blocking_invalidation_notifier.cc b/sync/notifier/non_blocking_invalidation_notifier.cc index 6ec7849..ac0f76a 100644 --- a/sync/notifier/non_blocking_invalidation_notifier.cc +++ b/sync/notifier/non_blocking_invalidation_notifier.cc @@ -33,10 +33,10 @@ class NonBlockingInvalidationNotifier::Core const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, const std::string& client_info); void Teardown(); + void UpdateRegisteredIds(const ObjectIdSet& ids); void SetUniqueId(const std::string& unique_id); void SetStateDeprecated(const std::string& state); void UpdateCredentials(const std::string& email, const std::string& token); - void UpdateEnabledTypes(ModelTypeSet enabled_types); // SyncNotifierObserver implementation (all called on I/O thread by // InvalidationNotifier). @@ -44,7 +44,7 @@ class NonBlockingInvalidationNotifier::Core virtual void OnNotificationsDisabled( NotificationsDisabledReason reason) OVERRIDE; virtual void OnIncomingNotification( - const ModelTypePayloadMap& type_payloads, + const ObjectIdPayloadMap& id_payloads, IncomingNotificationSource source) OVERRIDE; private: @@ -89,17 +89,22 @@ void NonBlockingInvalidationNotifier::Core::Initialize( initial_invalidation_state, invalidation_state_tracker, client_info)); - invalidation_notifier_->AddObserver(this); } void NonBlockingInvalidationNotifier::Core::Teardown() { DCHECK(network_task_runner_->BelongsToCurrentThread()); - invalidation_notifier_->RemoveObserver(this); + invalidation_notifier_->UpdateRegisteredIds(this, ObjectIdSet()); invalidation_notifier_.reset(); network_task_runner_ = NULL; } +void NonBlockingInvalidationNotifier::Core::UpdateRegisteredIds( + const ObjectIdSet& ids) { + DCHECK(network_task_runner_->BelongsToCurrentThread()); + invalidation_notifier_->UpdateRegisteredIds(this, ids); +} + void NonBlockingInvalidationNotifier::Core::SetUniqueId( const std::string& unique_id) { DCHECK(network_task_runner_->BelongsToCurrentThread()); @@ -118,12 +123,6 @@ void NonBlockingInvalidationNotifier::Core::UpdateCredentials( invalidation_notifier_->UpdateCredentials(email, token); } -void NonBlockingInvalidationNotifier::Core::UpdateEnabledTypes( - ModelTypeSet enabled_types) { - DCHECK(network_task_runner_->BelongsToCurrentThread()); - invalidation_notifier_->UpdateEnabledTypes(enabled_types); -} - void NonBlockingInvalidationNotifier::Core::OnNotificationsEnabled() { DCHECK(network_task_runner_->BelongsToCurrentThread()); delegate_observer_.Call(FROM_HERE, @@ -138,12 +137,11 @@ void NonBlockingInvalidationNotifier::Core::OnNotificationsDisabled( } void NonBlockingInvalidationNotifier::Core::OnIncomingNotification( - const ModelTypePayloadMap& type_payloads, - IncomingNotificationSource source) { + const ObjectIdPayloadMap& id_payloads, IncomingNotificationSource source) { DCHECK(network_task_runner_->BelongsToCurrentThread()); delegate_observer_.Call(FROM_HERE, &SyncNotifierObserver::OnIncomingNotification, - type_payloads, + id_payloads, source); } @@ -185,16 +183,19 @@ NonBlockingInvalidationNotifier::~NonBlockingInvalidationNotifier() { } } -void NonBlockingInvalidationNotifier::AddObserver( - SyncNotifierObserver* observer) { +void NonBlockingInvalidationNotifier::UpdateRegisteredIds( + SyncNotifierObserver* handler, const ObjectIdSet& ids) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - observers_.AddObserver(observer); -} - -void NonBlockingInvalidationNotifier::RemoveObserver( - SyncNotifierObserver* observer) { - DCHECK(parent_task_runner_->BelongsToCurrentThread()); - observers_.RemoveObserver(observer); + const ObjectIdSet& all_registered_ids = + helper_.UpdateRegisteredIds(handler, ids); + if (!network_task_runner_->PostTask( + FROM_HERE, + base::Bind( + &NonBlockingInvalidationNotifier::Core::UpdateRegisteredIds, + core_.get(), + all_registered_ids))) { + NOTREACHED(); + } } void NonBlockingInvalidationNotifier::SetUniqueId( @@ -231,17 +232,6 @@ void NonBlockingInvalidationNotifier::UpdateCredentials( } } -void NonBlockingInvalidationNotifier::UpdateEnabledTypes( - ModelTypeSet enabled_types) { - DCHECK(parent_task_runner_->BelongsToCurrentThread()); - if (!network_task_runner_->PostTask( - FROM_HERE, - base::Bind(&NonBlockingInvalidationNotifier::Core::UpdateEnabledTypes, - core_.get(), enabled_types))) { - NOTREACHED(); - } -} - void NonBlockingInvalidationNotifier::SendNotification( ModelTypeSet changed_types) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); @@ -251,23 +241,20 @@ void NonBlockingInvalidationNotifier::SendNotification( void NonBlockingInvalidationNotifier::OnNotificationsEnabled() { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observers_, - OnNotificationsEnabled()); + helper_.EmitOnNotificationsEnabled(); } void NonBlockingInvalidationNotifier::OnNotificationsDisabled( NotificationsDisabledReason reason) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observers_, - OnNotificationsDisabled(reason)); + helper_.EmitOnNotificationsDisabled(reason); } void NonBlockingInvalidationNotifier::OnIncomingNotification( - const ModelTypePayloadMap& type_payloads, + const ObjectIdPayloadMap& id_payloads, IncomingNotificationSource source) { DCHECK(parent_task_runner_->BelongsToCurrentThread()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observers_, - OnIncomingNotification(type_payloads, source)); + helper_.DispatchInvalidationsToHandlers(id_payloads, source); } } // namespace syncer diff --git a/sync/notifier/non_blocking_invalidation_notifier.h b/sync/notifier/non_blocking_invalidation_notifier.h index 4756a81..f76b37b 100644 --- a/sync/notifier/non_blocking_invalidation_notifier.h +++ b/sync/notifier/non_blocking_invalidation_notifier.h @@ -14,11 +14,11 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" -#include "base/observer_list.h" #include "jingle/notifier/base/notifier_options.h" #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/invalidation_state_tracker.h" #include "sync/notifier/sync_notifier.h" +#include "sync/notifier/sync_notifier_helper.h" #include "sync/notifier/sync_notifier_observer.h" namespace base { @@ -44,13 +44,12 @@ class NonBlockingInvalidationNotifier virtual ~NonBlockingInvalidationNotifier(); // SyncNotifier implementation. - virtual void AddObserver(SyncNotifierObserver* observer) OVERRIDE; - virtual void RemoveObserver(SyncNotifierObserver* observer) OVERRIDE; + virtual void UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) 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 UpdateEnabledTypes(ModelTypeSet enabled_types) OVERRIDE; virtual void SendNotification(ModelTypeSet changed_types) OVERRIDE; // SyncNotifierObserver implementation. @@ -58,7 +57,7 @@ class NonBlockingInvalidationNotifier virtual void OnNotificationsDisabled( NotificationsDisabledReason reason) OVERRIDE; virtual void OnIncomingNotification( - const ModelTypePayloadMap& type_payloads, + const ObjectIdPayloadMap& id_payloads, IncomingNotificationSource source) OVERRIDE; private: @@ -66,8 +65,7 @@ class NonBlockingInvalidationNotifier base::WeakPtrFactory<NonBlockingInvalidationNotifier> weak_ptr_factory_; - // Our observers (which must live on the parent thread). - ObserverList<SyncNotifierObserver> observers_; + SyncNotifierHelper helper_; // The real guts of NonBlockingInvalidationNotifier, which allows // this class to live completely on the parent thread. diff --git a/sync/notifier/non_blocking_invalidation_notifier_unittest.cc b/sync/notifier/non_blocking_invalidation_notifier_unittest.cc index 7a30f58..36f4532 100644 --- a/sync/notifier/non_blocking_invalidation_notifier_unittest.cc +++ b/sync/notifier/non_blocking_invalidation_notifier_unittest.cc @@ -8,6 +8,7 @@ #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/threading/thread.h" +#include "google/cacheinvalidation/v2/types.pb.h" #include "jingle/notifier/base/fake_base_task.h" #include "net/url_request/url_request_test_util.h" #include "sync/internal_api/public/base/model_type.h" @@ -45,11 +46,10 @@ class NonBlockingInvalidationNotifierTest : public testing::Test { std::string(), // initial_invalidation_state MakeWeakHandle(base::WeakPtr<InvalidationStateTracker>()), "fake_client_info")); - invalidation_notifier_->AddObserver(&mock_observer_); } virtual void TearDown() { - invalidation_notifier_->RemoveObserver(&mock_observer_); + invalidation_notifier_->UpdateRegisteredIds(&mock_observer_, ObjectIdSet()); invalidation_notifier_.reset(); request_context_getter_ = NULL; io_thread_.Stop(); @@ -67,15 +67,16 @@ class NonBlockingInvalidationNotifierTest : public testing::Test { TEST_F(NonBlockingInvalidationNotifierTest, Basic) { InSequence dummy; - ModelTypePayloadMap type_payloads; - type_payloads[PREFERENCES] = "payload"; - type_payloads[BOOKMARKS] = ""; - type_payloads[AUTOFILL] = ""; + ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL); + invalidation_notifier_->UpdateRegisteredIds( + &mock_observer_, ModelTypeSetToObjectIdSet(models)); + const ModelTypePayloadMap& type_payloads = + ModelTypePayloadMapFromEnumSet(models, "payload"); EXPECT_CALL(mock_observer_, OnNotificationsEnabled()); - EXPECT_CALL(mock_observer_, - OnIncomingNotification(type_payloads, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(type_payloads), + REMOTE_NOTIFICATION)); EXPECT_CALL(mock_observer_, OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR)); EXPECT_CALL(mock_observer_, @@ -86,8 +87,9 @@ TEST_F(NonBlockingInvalidationNotifierTest, Basic) { invalidation_notifier_->UpdateCredentials("foo@bar.com", "fake_token"); invalidation_notifier_->OnNotificationsEnabled(); - invalidation_notifier_->OnIncomingNotification(type_payloads, - REMOTE_NOTIFICATION); + invalidation_notifier_->OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(type_payloads), + REMOTE_NOTIFICATION); invalidation_notifier_->OnNotificationsDisabled( TRANSIENT_NOTIFICATION_ERROR); invalidation_notifier_->OnNotificationsDisabled( diff --git a/sync/notifier/object_id_payload_map.cc b/sync/notifier/object_id_payload_map.cc new file mode 100644 index 0000000..179f8be --- /dev/null +++ b/sync/notifier/object_id_payload_map.cc @@ -0,0 +1,40 @@ +// 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/object_id_payload_map.h" + +namespace syncer { + +ModelTypePayloadMap ObjectIdPayloadMapToModelTypePayloadMap( + const ObjectIdPayloadMap& id_payloads) { + ModelTypePayloadMap types_with_payloads; + for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin(); + it != id_payloads.end(); ++it) { + ModelType model_type; + if (!ObjectIdToRealModelType(it->first, &model_type)) { + DLOG(WARNING) << "Invalid object ID: " + << ObjectIdToString(it->first); + continue; + } + types_with_payloads[model_type] = it->second; + } + return types_with_payloads; +} + +ObjectIdPayloadMap ModelTypePayloadMapToObjectIdPayloadMap( + const ModelTypePayloadMap& type_payloads) { + ObjectIdPayloadMap id_payloads; + for (ModelTypePayloadMap::const_iterator it = type_payloads.begin(); + it != type_payloads.end(); ++it) { + invalidation::ObjectId id; + if (!RealModelTypeToObjectId(it->first, &id)) { + DLOG(WARNING) << "Invalid model type " << it->first; + continue; + } + id_payloads[id] = it->second; + } + return id_payloads; +} + +} // namespace syncer diff --git a/sync/notifier/object_id_payload_map.h b/sync/notifier/object_id_payload_map.h new file mode 100644 index 0000000..9205326 --- /dev/null +++ b/sync/notifier/object_id_payload_map.h @@ -0,0 +1,29 @@ +// 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_OBJECT_ID_PAYLOAD_MAP_H_ +#define SYNC_NOTIFIER_OBJECT_ID_PAYLOAD_MAP_H_ + +#include <map> +#include <string> + +#include "google/cacheinvalidation/include/types.h" +#include "sync/internal_api/public/base/model_type_payload_map.h" +#include "sync/notifier/invalidation_util.h" + +namespace syncer { + +typedef std::map<invalidation::ObjectId, + std::string, + ObjectIdLessThan> ObjectIdPayloadMap; + +// Converts between ObjectIdPayloadMaps and ModelTypePayloadMaps. +ModelTypePayloadMap ObjectIdPayloadMapToModelTypePayloadMap( + const ObjectIdPayloadMap& id_payloads); +ObjectIdPayloadMap ModelTypePayloadMapToObjectIdPayloadMap( + const ModelTypePayloadMap& type_payloads); + +} // namespace syncer + +#endif // HOME_DCHENG_SRC_CHROMIUM_SRC_SYNC_NOTIFIER_OBJECT_ID_PAYLOAD_MAP_H_ diff --git a/sync/notifier/p2p_notifier.cc b/sync/notifier/p2p_notifier.cc index e2a4209..856c86b 100644 --- a/sync/notifier/p2p_notifier.cc +++ b/sync/notifier/p2p_notifier.cc @@ -12,6 +12,7 @@ #include "base/values.h" #include "jingle/notifier/listener/push_client.h" #include "sync/internal_api/public/base/model_type_payload_map.h" +#include "sync/notifier/invalidation_util.h" #include "sync/notifier/sync_notifier_observer.h" namespace syncer { @@ -156,14 +157,16 @@ P2PNotifier::~P2PNotifier() { push_client_->RemoveObserver(this); } -void P2PNotifier::AddObserver(SyncNotifierObserver* observer) { - DCHECK(thread_checker_.CalledOnValidThread()); - observer_list_.AddObserver(observer); -} - -void P2PNotifier::RemoveObserver(SyncNotifierObserver* observer) { - DCHECK(thread_checker_.CalledOnValidThread()); - observer_list_.RemoveObserver(observer); +void P2PNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) { + const ModelTypeSet enabled_types = ObjectIdSetToModelTypeSet( + helper_.UpdateRegisteredIds(handler, ids)); + const ModelTypeSet new_enabled_types = + Difference(enabled_types, enabled_types_); + const P2PNotificationData notification_data( + unique_id_, NOTIFY_SELF, new_enabled_types); + SendNotificationData(notification_data); + enabled_types_ = enabled_types; } void P2PNotifier::SetUniqueId(const std::string& unique_id) { @@ -193,16 +196,6 @@ void P2PNotifier::UpdateCredentials( logged_in_ = true; } -void P2PNotifier::UpdateEnabledTypes(ModelTypeSet enabled_types) { - DCHECK(thread_checker_.CalledOnValidThread()); - const ModelTypeSet new_enabled_types = - Difference(enabled_types, enabled_types_); - enabled_types_ = enabled_types; - const P2PNotificationData notification_data( - unique_id_, NOTIFY_SELF, new_enabled_types); - SendNotificationData(notification_data); -} - void P2PNotifier::SendNotification(ModelTypeSet changed_types) { DCHECK(thread_checker_.CalledOnValidThread()); const P2PNotificationData notification_data( @@ -214,9 +207,7 @@ void P2PNotifier::OnNotificationsEnabled() { DCHECK(thread_checker_.CalledOnValidThread()); bool just_turned_on = (notifications_enabled_ == false); notifications_enabled_ = true; - FOR_EACH_OBSERVER( - SyncNotifierObserver, observer_list_, - OnNotificationsEnabled()); + helper_.EmitOnNotificationsEnabled(); if (just_turned_on) { const P2PNotificationData notification_data( unique_id_, NOTIFY_SELF, enabled_types_); @@ -227,9 +218,7 @@ void P2PNotifier::OnNotificationsEnabled() { void P2PNotifier::OnNotificationsDisabled( notifier::NotificationsDisabledReason reason) { DCHECK(thread_checker_.CalledOnValidThread()); - FOR_EACH_OBSERVER( - SyncNotifierObserver, observer_list_, - OnNotificationsDisabled(FromNotifierReason(reason))); + helper_.EmitOnNotificationsDisabled(FromNotifierReason(reason)); } void P2PNotifier::OnIncomingNotification( @@ -264,11 +253,11 @@ void P2PNotifier::OnIncomingNotification( DVLOG(1) << "No changed types -- not emitting notification"; return; } - const ModelTypePayloadMap& type_payloads = - ModelTypePayloadMapFromEnumSet( - notification_data.GetChangedTypes(), std::string()); - FOR_EACH_OBSERVER(SyncNotifierObserver, observer_list_, - OnIncomingNotification(type_payloads, REMOTE_NOTIFICATION)); + const ModelTypePayloadMap& type_payloads = ModelTypePayloadMapFromEnumSet( + notification_data.GetChangedTypes(), std::string()); + helper_.DispatchInvalidationsToHandlers( + ModelTypePayloadMapToObjectIdPayloadMap(type_payloads), + REMOTE_NOTIFICATION); } void P2PNotifier::SendNotificationDataForTest( diff --git a/sync/notifier/p2p_notifier.h b/sync/notifier/p2p_notifier.h index a360f25..d2de89d 100644 --- a/sync/notifier/p2p_notifier.h +++ b/sync/notifier/p2p_notifier.h @@ -20,6 +20,7 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/notifier/notifications_disabled_reason.h" #include "sync/notifier/sync_notifier.h" +#include "sync/notifier/sync_notifier_helper.h" namespace notifier { class PushClient; @@ -81,9 +82,8 @@ class P2PNotificationData { ModelTypeSet changed_types_; }; -class P2PNotifier - : public SyncNotifier, - public notifier::PushClientObserver { +class P2PNotifier : public SyncNotifier, + public notifier::PushClientObserver { public: // The |send_notification_target| parameter was added to allow us to send // self-notifications in some cases, but not others. The value should be @@ -96,13 +96,12 @@ class P2PNotifier virtual ~P2PNotifier(); // SyncNotifier implementation - virtual void AddObserver(SyncNotifierObserver* observer) OVERRIDE; - virtual void RemoveObserver(SyncNotifierObserver* observer) OVERRIDE; + virtual void UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) 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 UpdateEnabledTypes(ModelTypeSet enabled_types) OVERRIDE; virtual void SendNotification(ModelTypeSet changed_types) OVERRIDE; // PushClientObserver implementation. @@ -120,7 +119,7 @@ class P2PNotifier base::ThreadChecker thread_checker_; - ObserverList<SyncNotifierObserver> observer_list_; + SyncNotifierHelper helper_; // The push client. scoped_ptr<notifier::PushClient> push_client_; diff --git a/sync/notifier/p2p_notifier_unittest.cc b/sync/notifier/p2p_notifier_unittest.cc index e6e406a..2f7b2c9 100644 --- a/sync/notifier/p2p_notifier_unittest.cc +++ b/sync/notifier/p2p_notifier_unittest.cc @@ -8,6 +8,7 @@ #include "jingle/notifier/listener/fake_push_client.h" #include "sync/internal_api/public/base/model_type.h" +#include "sync/internal_api/public/base/model_type_payload_map.h" #include "sync/notifier/mock_sync_notifier_observer.h" #include "testing/gtest/include/gtest/gtest.h" @@ -27,15 +28,14 @@ class P2PNotifierTest : public testing::Test { scoped_ptr<notifier::PushClient>(fake_push_client_), NOTIFY_OTHERS), next_sent_notification_to_reflect_(0) { - p2p_notifier_.AddObserver(&mock_observer_); } virtual ~P2PNotifierTest() { - p2p_notifier_.RemoveObserver(&mock_observer_); + p2p_notifier_.UpdateRegisteredIds(&mock_observer_, ObjectIdSet()); } ModelTypePayloadMap MakePayloadMap(ModelTypeSet types) { - return ModelTypePayloadMapFromEnumSet(types, ""); + return ModelTypePayloadMapFromEnumSet(types, std::string()); } // Simulate receiving all the notifications we sent out since last @@ -142,10 +142,13 @@ TEST_F(P2PNotifierTest, P2PNotificationDataNonDefault) { TEST_F(P2PNotifierTest, NotificationsBasic) { ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES); + p2p_notifier_.UpdateRegisteredIds(&mock_observer_, + ModelTypeSetToObjectIdSet(enabled_types)); + EXPECT_CALL(mock_observer_, OnNotificationsEnabled()); - EXPECT_CALL(mock_observer_, - OnIncomingNotification(MakePayloadMap(enabled_types), - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(MakePayloadMap(enabled_types)), + REMOTE_NOTIFICATION)); p2p_notifier_.SetUniqueId("sender"); @@ -163,8 +166,6 @@ TEST_F(P2PNotifierTest, NotificationsBasic) { EXPECT_EQ(kEmail, fake_push_client_->email()); EXPECT_EQ(kToken, fake_push_client_->token()); - p2p_notifier_.UpdateEnabledTypes(enabled_types); - ReflectSentNotifications(); fake_push_client_->EnableNotifications(); @@ -184,19 +185,24 @@ TEST_F(P2PNotifierTest, NotificationsBasic) { TEST_F(P2PNotifierTest, SendNotificationData) { ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES); - ModelTypeSet changed_types(THEMES, APPS); + p2p_notifier_.UpdateRegisteredIds(&mock_observer_, + ModelTypeSetToObjectIdSet(enabled_types)); + + ModelTypeSet changed_types(BOOKMARKS, APPS); + ModelTypeSet expected_changed_types(BOOKMARKS); - const ModelTypePayloadMap& changed_payload_map = - MakePayloadMap(changed_types); + const ModelTypePayloadMap& expected_changed_payload_map = + MakePayloadMap(expected_changed_types); EXPECT_CALL(mock_observer_, OnNotificationsEnabled()); EXPECT_CALL(mock_observer_, - OnIncomingNotification(MakePayloadMap(enabled_types), - REMOTE_NOTIFICATION)); + OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap( + MakePayloadMap(enabled_types)), + REMOTE_NOTIFICATION)); p2p_notifier_.SetUniqueId("sender"); p2p_notifier_.UpdateCredentials("foo@bar.com", "fake_token"); - p2p_notifier_.UpdateEnabledTypes(enabled_types); ReflectSentNotifications(); fake_push_client_->EnableNotifications(); @@ -211,8 +217,9 @@ TEST_F(P2PNotifierTest, SendNotificationData) { // Should be propagated. Mock::VerifyAndClearExpectations(&mock_observer_); - EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(expected_changed_payload_map), + REMOTE_NOTIFICATION)); p2p_notifier_.SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_SELF, changed_types)); @@ -240,8 +247,9 @@ TEST_F(P2PNotifierTest, SendNotificationData) { // Should be propagated. Mock::VerifyAndClearExpectations(&mock_observer_); - EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(expected_changed_payload_map), + REMOTE_NOTIFICATION)); p2p_notifier_.SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_OTHERS, changed_types)); @@ -256,8 +264,9 @@ TEST_F(P2PNotifierTest, SendNotificationData) { // Should be propagated. Mock::VerifyAndClearExpectations(&mock_observer_); - EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(expected_changed_payload_map), + REMOTE_NOTIFICATION)); p2p_notifier_.SendNotificationDataForTest( P2PNotificationData("sender", NOTIFY_ALL, changed_types)); @@ -265,8 +274,9 @@ TEST_F(P2PNotifierTest, SendNotificationData) { // Should be propagated. Mock::VerifyAndClearExpectations(&mock_observer_); - EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map, - REMOTE_NOTIFICATION)); + EXPECT_CALL(mock_observer_, OnIncomingNotification( + ModelTypePayloadMapToObjectIdPayloadMap(expected_changed_payload_map), + REMOTE_NOTIFICATION)); p2p_notifier_.SendNotificationDataForTest( P2PNotificationData("sender2", NOTIFY_ALL, changed_types)); diff --git a/sync/notifier/sync_notifier.h b/sync/notifier/sync_notifier.h index 0c775f4..369ced1 100644 --- a/sync/notifier/sync_notifier.h +++ b/sync/notifier/sync_notifier.h @@ -12,6 +12,7 @@ #include <string> #include "sync/internal_api/public/base/model_type.h" +#include "sync/notifier/invalidation_util.h" namespace syncer { class SyncNotifierObserver; @@ -21,8 +22,11 @@ class SyncNotifier { SyncNotifier() {} virtual ~SyncNotifier() {} - virtual void AddObserver(SyncNotifierObserver* observer) = 0; - virtual void RemoveObserver(SyncNotifierObserver* observer) = 0; + // Updates the set of ObjectIds associated with a given |handler|. Passing an + // empty ObjectIdSet will unregister |handler|. If two different handlers + // attempt to register for the same object ID, the first registration wins. + virtual void UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids) = 0; // SetUniqueId must be called once, before any call to // UpdateCredentials. |unique_id| should be a non-empty globally @@ -40,8 +44,6 @@ class SyncNotifier { virtual void UpdateCredentials( const std::string& email, const std::string& token) = 0; - virtual void UpdateEnabledTypes(ModelTypeSet enabled_types) = 0; - // This is here only to support the old p2p notification implementation, // which is still used by sync integration tests. // TODO(akalin): Remove this once we move the integration tests off p2p diff --git a/sync/notifier/sync_notifier_factory_unittest.cc b/sync/notifier/sync_notifier_factory_unittest.cc index 52fd505..8036c1a 100644 --- a/sync/notifier/sync_notifier_factory_unittest.cc +++ b/sync/notifier/sync_notifier_factory_unittest.cc @@ -60,8 +60,9 @@ TEST_F(SyncNotifierFactoryTest, Basic) { ASSERT_FALSE(notifier.get()); #else ASSERT_TRUE(notifier.get()); - notifier->AddObserver(&mock_observer_); - notifier->RemoveObserver(&mock_observer_); + ObjectIdSet ids = ModelTypeSetToObjectIdSet(ModelTypeSet(syncer::BOOKMARKS)); + notifier->UpdateRegisteredIds(&mock_observer_, ids); + notifier->UpdateRegisteredIds(&mock_observer_, ObjectIdSet()); #endif } @@ -77,8 +78,9 @@ TEST_F(SyncNotifierFactoryTest, Basic_P2P) { ASSERT_FALSE(notifier.get()); #else ASSERT_TRUE(notifier.get()); - notifier->AddObserver(&mock_observer_); - notifier->RemoveObserver(&mock_observer_); + ObjectIdSet ids = ModelTypeSetToObjectIdSet(ModelTypeSet(syncer::BOOKMARKS)); + notifier->UpdateRegisteredIds(&mock_observer_, ids); + notifier->UpdateRegisteredIds(&mock_observer_, ObjectIdSet()); #endif } diff --git a/sync/notifier/sync_notifier_helper.cc b/sync/notifier/sync_notifier_helper.cc new file mode 100644 index 0000000..af3b2ea --- /dev/null +++ b/sync/notifier/sync_notifier_helper.cc @@ -0,0 +1,95 @@ +// 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/sync_notifier_helper.h" + +#include "base/logging.h" + +namespace syncer { + +SyncNotifierHelper::SyncNotifierHelper() {} +SyncNotifierHelper::~SyncNotifierHelper() {} + +ObjectIdSet SyncNotifierHelper::UpdateRegisteredIds( + SyncNotifierObserver* handler, const ObjectIdSet& ids) { + if (ids.empty()) { + handlers_.RemoveObserver(handler); + } else if (!handlers_.HasObserver(handler)) { + handlers_.AddObserver(handler); + } + + ObjectIdSet registered_ids(ids); + // Remove all existing entries for |handler| and fill |registered_ids| with + // the rest. + for (ObjectIdHandlerMap::iterator it = id_to_handler_map_.begin(); + it != id_to_handler_map_.end(); ) { + if (it->second == handler) { + ObjectIdHandlerMap::iterator erase_it = it; + ++it; + id_to_handler_map_.erase(erase_it); + } else { + registered_ids.insert(it->first); + ++it; + } + } + + // Now add the entries for |handler|. We keep track of the last insertion + // point so we only traverse the map once to insert all the new entries. + ObjectIdHandlerMap::iterator insert_it = id_to_handler_map_.begin(); + for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { + insert_it = id_to_handler_map_.insert(insert_it, + std::make_pair(*it, handler)); + CHECK_EQ(handler, insert_it->second) << "Duplicate registration for " + << ObjectIdToString(insert_it->first); + } + if (logging::DEBUG_MODE) { + // The mapping shouldn't contain any handlers that aren't in |handlers_|. + for (ObjectIdHandlerMap::const_iterator it = id_to_handler_map_.begin(); + it != id_to_handler_map_.end(); ++it) { + CHECK(handlers_.HasObserver(it->second)); + } + } + return registered_ids; +} + +void SyncNotifierHelper::DispatchInvalidationsToHandlers( + const ObjectIdPayloadMap& id_payloads, + IncomingNotificationSource source) { + typedef std::map<SyncNotifierObserver*, ObjectIdPayloadMap> DispatchMap; + DispatchMap dispatch_map; + for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin(); + it != id_payloads.end(); ++it) { + ObjectIdHandlerMap::const_iterator find_it = + id_to_handler_map_.find(it->first); + // If we get an invalidation with a source type that we can't map to an + // observer, just drop it--the observer was unregistered while the + // invalidation was in flight. + if (find_it == id_to_handler_map_.end()) + continue; + dispatch_map[find_it->second].insert(*it); + } + + if (handlers_.might_have_observers()) { + ObserverListBase<SyncNotifierObserver>::Iterator it(handlers_); + SyncNotifierObserver* handler = NULL; + 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); + } + } + } +} + +void SyncNotifierHelper::EmitOnNotificationsEnabled() { + FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_, OnNotificationsEnabled()); +} + +void SyncNotifierHelper::EmitOnNotificationsDisabled( + NotificationsDisabledReason reason) { + FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_, + OnNotificationsDisabled(reason)); +} + +} // namespace syncer diff --git a/sync/notifier/sync_notifier_helper.h b/sync/notifier/sync_notifier_helper.h new file mode 100644 index 0000000..28ff187 --- /dev/null +++ b/sync/notifier/sync_notifier_helper.h @@ -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. + +#ifndef SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_ +#define SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_ + +#include <map> + +#include "base/basictypes.h" +#include "base/observer_list.h" +#include "sync/notifier/invalidation_util.h" +#include "sync/notifier/object_id_payload_map.h" +#include "sync/notifier/sync_notifier_observer.h" + +namespace syncer { + +// A helper class for classes that want to implement the SyncNotifier interface. +// It helps keep track of registered handlers and which object ID registrations +// are associated with which handlers, so implementors can just reuse the logic +// here to dispatch invalidations and other interesting notifications. +class SyncNotifierHelper { + public: + SyncNotifierHelper(); + ~SyncNotifierHelper(); + + // Updates the set of ObjectIds associated with a given |handler|. Passing an + // empty ObjectIdSet will unregister |handler|. The return value is an + // ObjectIdSet containing values for all existing handlers. + ObjectIdSet UpdateRegisteredIds(SyncNotifierObserver* handler, + const ObjectIdSet& ids); + + // Helper that sorts incoming invalidations into a bucket for each handler + // and then dispatches the batched invalidations to the corresponding handler. + void DispatchInvalidationsToHandlers(const ObjectIdPayloadMap& id_payloads, + IncomingNotificationSource source); + + // Calls the given handler method for each handler that has registered IDs. + void EmitOnNotificationsEnabled(); + void EmitOnNotificationsDisabled(NotificationsDisabledReason reason); + + private: + typedef std::map<invalidation::ObjectId, + SyncNotifierObserver*, + ObjectIdLessThan> ObjectIdHandlerMap; + + ObserverList<SyncNotifierObserver> handlers_; + ObjectIdHandlerMap id_to_handler_map_; + + DISALLOW_COPY_AND_ASSIGN(SyncNotifierHelper); +}; + +} // namespace syncer + +#endif // SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_ diff --git a/sync/notifier/sync_notifier_helper_unittest.cc b/sync/notifier/sync_notifier_helper_unittest.cc new file mode 100644 index 0000000..ba2d768 --- /dev/null +++ b/sync/notifier/sync_notifier_helper_unittest.cc @@ -0,0 +1,156 @@ +// 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 "google/cacheinvalidation/v2/types.pb.h" +#include "sync/notifier/sync_notifier_helper.h" +#include "sync/notifier/mock_sync_notifier_observer.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::StrictMock; + +namespace syncer { + +class SyncNotifierHelperTest : public testing::Test { + protected: + SyncNotifierHelperTest() + : kObjectId1(ipc::invalidation::ObjectSource::TEST, "a"), + kObjectId2(ipc::invalidation::ObjectSource::TEST, "b"), + kObjectId3(ipc::invalidation::ObjectSource::TEST, "c") { + } + + invalidation::ObjectId kObjectId1; + invalidation::ObjectId kObjectId2; + invalidation::ObjectId kObjectId3; +}; + +// Basic check that registrations are correctly updated for one handler. +TEST_F(SyncNotifierHelperTest, Basic) { + SyncNotifierHelper helper; + StrictMock<MockSyncNotifierObserver> observer; + ObjectIdSet ids; + ids.insert(kObjectId1); + ids.insert(kObjectId2); + helper.UpdateRegisteredIds(&observer, ids); + + ObjectIdPayloadMap dispatched_payloads; + dispatched_payloads[kObjectId1] = "1"; + dispatched_payloads[kObjectId2] = "2"; + dispatched_payloads[kObjectId3] = "3"; + + // A object ID with no registration should be ignored. + ObjectIdPayloadMap expected_payload1; + expected_payload1[kObjectId1] = "1"; + expected_payload1[kObjectId2] = "2"; + EXPECT_CALL(observer, OnIncomingNotification(expected_payload1, + REMOTE_NOTIFICATION)); + helper.DispatchInvalidationsToHandlers(dispatched_payloads, + REMOTE_NOTIFICATION); + + // Removed object IDs should not be notified, newly-added ones should. + ids.erase(kObjectId1); + ids.insert(kObjectId3); + helper.UpdateRegisteredIds(&observer, ids); + + ObjectIdPayloadMap expected_payload2; + expected_payload2[kObjectId2] = "2"; + expected_payload2[kObjectId3] = "3"; + EXPECT_CALL(observer, OnIncomingNotification(expected_payload2, + REMOTE_NOTIFICATION)); + helper.DispatchInvalidationsToHandlers(dispatched_payloads, + REMOTE_NOTIFICATION); +} + +// Tests that we correctly bucket and dispatch invalidations on multiple objects +// to the corresponding handlers. +TEST_F(SyncNotifierHelperTest, MultipleHandlers) { + SyncNotifierHelper helper; + StrictMock<MockSyncNotifierObserver> observer; + ObjectIdSet ids; + ids.insert(kObjectId1); + ids.insert(kObjectId2); + helper.UpdateRegisteredIds(&observer, ids); + StrictMock<MockSyncNotifierObserver> observer2; + ObjectIdSet ids2; + ids2.insert(kObjectId3); + helper.UpdateRegisteredIds(&observer2, ids2); + + ObjectIdPayloadMap expected_payload1; + expected_payload1[kObjectId1] = "1"; + expected_payload1[kObjectId2] = "2"; + EXPECT_CALL(observer, OnIncomingNotification(expected_payload1, + REMOTE_NOTIFICATION)); + ObjectIdPayloadMap expected_payload2; + expected_payload2[kObjectId3] = "3"; + EXPECT_CALL(observer2, OnIncomingNotification(expected_payload2, + REMOTE_NOTIFICATION)); + + ObjectIdPayloadMap dispatched_payloads; + dispatched_payloads[kObjectId1] = "1"; + dispatched_payloads[kObjectId2] = "2"; + dispatched_payloads[kObjectId3] = "3"; + helper.DispatchInvalidationsToHandlers(dispatched_payloads, + REMOTE_NOTIFICATION); + + // Also verify that the callbacks for OnNotificationsEnabled/Disabled work. + EXPECT_CALL(observer, OnNotificationsEnabled()); + EXPECT_CALL(observer2, OnNotificationsEnabled()); + EXPECT_CALL(observer, + OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR)); + EXPECT_CALL(observer2, + OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR)); + helper.EmitOnNotificationsEnabled(); + helper.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR); +} + +// Multiple registrations by different handlers on the same object ID should +// cause a CHECK. +TEST_F(SyncNotifierHelperTest, MultipleRegistration) { + SyncNotifierHelper helper; + StrictMock<MockSyncNotifierObserver> observer; + ObjectIdSet ids; + ids.insert(kObjectId1); + ids.insert(kObjectId2); + helper.UpdateRegisteredIds(&observer, ids); + + StrictMock<MockSyncNotifierObserver> observer2; + EXPECT_DEATH({ helper.UpdateRegisteredIds(&observer2, ids); }, + "Duplicate registration for .*"); +} + +// Make sure that passing an empty set to UpdateRegisteredIds clears the +// corresponding entries for the handler. +TEST_F(SyncNotifierHelperTest, EmptySetUnregisters) { + SyncNotifierHelper helper; + StrictMock<MockSyncNotifierObserver> observer; + ObjectIdSet ids; + ids.insert(kObjectId1); + ids.insert(kObjectId2); + helper.UpdateRegisteredIds(&observer, ids); + // Control observer. + StrictMock<MockSyncNotifierObserver> observer2; + ObjectIdSet ids2; + ids2.insert(kObjectId3); + helper.UpdateRegisteredIds(&observer2, ids2); + // Unregister the first observer. It should not receive any further callbacks. + helper.UpdateRegisteredIds(&observer, ObjectIdSet()); + + ObjectIdPayloadMap expected_payload2; + expected_payload2[kObjectId3] = "3"; + EXPECT_CALL(observer2, OnIncomingNotification(expected_payload2, + REMOTE_NOTIFICATION)); + EXPECT_CALL(observer2, OnNotificationsEnabled()); + EXPECT_CALL(observer2, + OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR)); + + ObjectIdPayloadMap dispatched_payloads; + dispatched_payloads[kObjectId1] = "1"; + dispatched_payloads[kObjectId2] = "2"; + dispatched_payloads[kObjectId3] = "3"; + helper.DispatchInvalidationsToHandlers(dispatched_payloads, + REMOTE_NOTIFICATION); + helper.EmitOnNotificationsEnabled(); + helper.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR); +} + +} // namespace syncer diff --git a/sync/notifier/sync_notifier_observer.h b/sync/notifier/sync_notifier_observer.h index eaea91f..b79f838 100644 --- a/sync/notifier/sync_notifier_observer.h +++ b/sync/notifier/sync_notifier_observer.h @@ -5,7 +5,7 @@ #ifndef SYNC_NOTIFIER_SYNC_NOTIFIER_OBSERVER_H_ #define SYNC_NOTIFIER_SYNC_NOTIFIER_OBSERVER_H_ -#include "sync/internal_api/public/base/model_type_payload_map.h" +#include "sync/notifier/object_id_payload_map.h" #include "sync/notifier/notifications_disabled_reason.h" namespace syncer { @@ -27,10 +27,10 @@ class SyncNotifierObserver { virtual void OnNotificationsDisabled( NotificationsDisabledReason reason) = 0; - // Called when a notification is received. The per-type payloads + // Called when a notification is received. The per-id payloads // are in |type_payloads| and the source is in |source|. virtual void OnIncomingNotification( - const ModelTypePayloadMap& type_payloads, + const ObjectIdPayloadMap& id_payloads, IncomingNotificationSource source) = 0; protected: |