diff options
author | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-02-02 02:50:34 +0000 |
---|---|---|
committer | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-02-02 02:50:34 +0000 |
commit | 8f549e16bef7423f465164970b31b04b84db45b7 (patch) | |
tree | 7be207a85217b53dafe46a460dea265efc9e5c83 | |
parent | 8deeab0ebb8a3f23b2318acba78529daef49b2ac (diff) | |
download | chromium_src-8f549e16bef7423f465164970b31b04b84db45b7.zip chromium_src-8f549e16bef7423f465164970b31b04b84db45b7.tar.gz chromium_src-8f549e16bef7423f465164970b31b04b84db45b7.tar.bz2 |
[Sync] Add ChromeSyncNotificationBridge class to sync notifier framework for sessions use.
The ChromeSyncNotificationBridge listens to notifications from chrome events that should
trigger a sync notification. It is a thread-safe class that will post the
notifications to the thread the SyncNotifierObserver is on, thereby allowing,
for example, events on the UI thread to trigger notifications on the Sync
thread. At the moment the only such event is NOTIFICATION_SYNC_REFRESH, which
is only triggered by the sessions datatype.
BUG=103469
TEST=manually going to chrome://newtab#opentabs and ensuring a sync cycle
occurs.
Review URL: http://codereview.chromium.org/9079002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@120150 0039d316-1c4b-4281-b951-d872f2087c98
17 files changed, 732 insertions, 14 deletions
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index ecbbf11..1f9d9b4 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -230,6 +230,7 @@ SyncBackendHost::SyncBackendHost(const std::string& name, initialization_state_(NOT_ATTEMPTED), sync_prefs_(sync_prefs), sync_notifier_factory_( + profile_, content::GetUserAgent(GURL()), profile_->GetRequestContext(), sync_prefs, @@ -238,14 +239,15 @@ SyncBackendHost::SyncBackendHost(const std::string& name, last_auth_error_(AuthError::None()) { } -SyncBackendHost::SyncBackendHost() +SyncBackendHost::SyncBackendHost(Profile* profile) : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), sync_thread_("Chrome_SyncThread"), frontend_loop_(MessageLoop::current()), - profile_(NULL), + profile_(profile), name_("Unknown"), initialization_state_(NOT_ATTEMPTED), sync_notifier_factory_( + profile_, content::GetUserAgent(GURL()), NULL, base::WeakPtr<sync_notifier::InvalidationVersionTracker>(), @@ -553,7 +555,7 @@ void SyncBackendHost::GetModelSafeRoutingInfo( void SyncBackendHost::InitCore(const DoInitializeOptions& options) { sync_thread_.message_loop()->PostTask(FROM_HERE, - base::Bind(&SyncBackendHost::Core::DoInitialize, core_.get(),options)); + base::Bind(&SyncBackendHost::Core::DoInitialize, core_.get(), options)); } void SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop( diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 1fd547d..44ef9bc 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -152,7 +152,7 @@ class SyncBackendHost { const base::WeakPtr<SyncPrefs>& sync_prefs); // For testing. // TODO(skrul): Extract an interface so this is not needed. - SyncBackendHost(); + explicit SyncBackendHost(Profile* profile); virtual ~SyncBackendHost(); // Called on |frontend_loop_| to kick off asynchronous initialization. diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.cc b/chrome/browser/sync/glue/sync_backend_host_mock.cc index d6a323e..e14c3d8 100644 --- a/chrome/browser/sync/glue/sync_backend_host_mock.cc +++ b/chrome/browser/sync/glue/sync_backend_host_mock.cc @@ -12,7 +12,8 @@ ACTION(InvokeTask) { arg3.Run(syncable::ModelTypeSet()); } -SyncBackendHostMock::SyncBackendHostMock() { +SyncBackendHostMock::SyncBackendHostMock() + : SyncBackendHost(&profile_) { // By default, invoke the ready callback. ON_CALL(*this, ConfigureDataTypes(_, _, _, _, _, _)). WillByDefault(InvokeTask()); diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.h b/chrome/browser/sync/glue/sync_backend_host_mock.h index 6a76a67..2dbe485 100644 --- a/chrome/browser/sync/glue/sync_backend_host_mock.h +++ b/chrome/browser/sync/glue/sync_backend_host_mock.h @@ -9,6 +9,7 @@ #include <set> #include "base/callback_forward.h" +#include "chrome/test/base/testing_profile.h" #include "chrome/browser/sync/glue/sync_backend_host.h" #include "chrome/browser/sync/profile_sync_test_util.h" #include "content/public/browser/notification_types.h" @@ -29,6 +30,8 @@ class SyncBackendHostMock : public SyncBackendHost { base::Callback<void()>, bool)); MOCK_METHOD0(StartSyncingWithServer, void()); + private: + TestingProfile profile_; }; } // namespace browser_sync diff --git a/chrome/browser/sync/notifier/DEPS b/chrome/browser/sync/notifier/DEPS index 9c39f61..6355c95 100644 --- a/chrome/browser/sync/notifier/DEPS +++ b/chrome/browser/sync/notifier/DEPS @@ -6,10 +6,12 @@ include_rules = [ "+chrome/browser/sync/notifier", "+chrome/browser/sync/syncable/model_type.h", "+chrome/browser/sync/syncable/model_type_payload_map.h", + "+chrome/browser/sync/syncable/model_type_test_util.h", "+chrome/browser/sync/protocol/service_constants.h", "+chrome/browser/sync/util", "+chrome/common/chrome_switches.h", + "+chrome/common/chrome_notification_types.h", "+google/cacheinvalidation", "+jingle/notifier", diff --git a/chrome/browser/sync/notifier/bridged_sync_notifier.cc b/chrome/browser/sync/notifier/bridged_sync_notifier.cc new file mode 100644 index 0000000..f2e72d3 --- /dev/null +++ b/chrome/browser/sync/notifier/bridged_sync_notifier.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 "chrome/browser/sync/notifier/bridged_sync_notifier.h" + +#include "chrome/browser/sync/notifier/chrome_sync_notification_bridge.h" + +namespace sync_notifier { + +BridgedSyncNotifier::BridgedSyncNotifier( + ChromeSyncNotificationBridge* bridge, SyncNotifier* delegate) + : bridge_(bridge), delegate_(delegate) { + DCHECK(bridge_); + DCHECK(delegate_.get()); +} + +BridgedSyncNotifier::~BridgedSyncNotifier() { +} + +void BridgedSyncNotifier::AddObserver(SyncNotifierObserver* observer) { + delegate_->AddObserver(observer); + bridge_->AddObserver(observer); +} + +void BridgedSyncNotifier::RemoveObserver( + SyncNotifierObserver* observer) { + bridge_->RemoveObserver(observer); + delegate_->RemoveObserver(observer); +} + +void BridgedSyncNotifier::SetUniqueId(const std::string& unique_id) { + delegate_->SetUniqueId(unique_id); +} + +void BridgedSyncNotifier::SetState(const std::string& state) { + delegate_->SetState(state); +} + +void BridgedSyncNotifier::UpdateCredentials( + const std::string& email, const std::string& token) { + delegate_->UpdateCredentials(email, token); +} + +void BridgedSyncNotifier::UpdateEnabledTypes( + syncable::ModelTypeSet enabled_types) { + delegate_->UpdateEnabledTypes(enabled_types); +} + +void BridgedSyncNotifier::SendNotification( + syncable::ModelTypeSet changed_types) { + delegate_->SendNotification(changed_types); +} + +} // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/bridged_sync_notifier.h b/chrome/browser/sync/notifier/bridged_sync_notifier.h new file mode 100644 index 0000000..a7cbedb --- /dev/null +++ b/chrome/browser/sync/notifier/bridged_sync_notifier.h @@ -0,0 +1,52 @@ +// 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 CHROME_BROWSER_SYNC_NOTIFIER_BRIDGED_SYNC_NOTIFIER_H_ +#define CHROME_BROWSER_SYNC_NOTIFIER_BRIDGED_SYNC_NOTIFIER_H_ + +#include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" +#include "chrome/browser/sync/notifier/sync_notifier.h" + +namespace sync_notifier { + +class ChromeSyncNotificationBridge; + +// A SyncNotifier implementation that wraps a ChromeSyncNotificationBridge +// and a SyncNotifier delegate (which it takes ownership of). All SyncNotifier +// calls are passed straight through to the delegate, with the exception of +// AddObserver/RemoveObserver, which also result in the observer being +// registered/deregistered with the ChromeSyncNotificationBridge. +class BridgedSyncNotifier : public SyncNotifier { + public: + // Does not take ownership of |bridge|. Takes ownership of |delegate|. + BridgedSyncNotifier(ChromeSyncNotificationBridge* bridge, + SyncNotifier* delegate); + virtual ~BridgedSyncNotifier(); + + // SyncNotifier implementation. Passes through all calls to the delegate. + // AddObserver/RemoveObserver will also register/deregister |observer| with + // the bridge. + virtual void AddObserver(SyncNotifierObserver* observer) OVERRIDE; + virtual void RemoveObserver(SyncNotifierObserver* observer) OVERRIDE; + virtual void SetUniqueId(const std::string& unique_id) OVERRIDE; + virtual void SetState(const std::string& state) OVERRIDE; + virtual void UpdateCredentials( + const std::string& email, const std::string& token) OVERRIDE; + virtual void UpdateEnabledTypes( + syncable::ModelTypeSet enabled_types) OVERRIDE; + virtual void SendNotification( + syncable::ModelTypeSet changed_types) OVERRIDE; + + private: + // The notification bridge that we register the observers with. + ChromeSyncNotificationBridge* bridge_; + + // The delegate we are wrapping. + scoped_ptr<SyncNotifier> delegate_; +}; + +} // namespace sync_notifier + +#endif // CHROME_BROWSER_SYNC_NOTIFIER_BRIDGED_SYNC_NOTIFIER_H_ diff --git a/chrome/browser/sync/notifier/bridged_sync_notifier_unittest.cc b/chrome/browser/sync/notifier/bridged_sync_notifier_unittest.cc new file mode 100644 index 0000000..c6fde30 --- /dev/null +++ b/chrome/browser/sync/notifier/bridged_sync_notifier_unittest.cc @@ -0,0 +1,124 @@ +// 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 "chrome/browser/sync/notifier/bridged_sync_notifier.h" + +#include <string> + +#include "base/compiler_specific.h" +#include "base/message_loop.h" +#include "base/threading/thread.h" +#include "chrome/browser/sync/notifier/chrome_sync_notification_bridge.h" +#include "chrome/browser/sync/notifier/mock_sync_notifier_observer.h" +#include "chrome/browser/sync/notifier/sync_notifier.h" +#include "chrome/browser/sync/syncable/model_type.h" +#include "chrome/browser/sync/syncable/model_type_test_util.h" +#include "chrome/test/base/profile_mock.h" +#include "content/test/test_browser_thread.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace sync_notifier { +namespace { + +using ::testing::NiceMock; +using ::testing::StrictMock; +using content::BrowserThread; +using syncable::HasModelTypes; + +class MockChromeSyncNotificationBridge : public ChromeSyncNotificationBridge { + public: + MockChromeSyncNotificationBridge() + : ChromeSyncNotificationBridge(&mock_profile_) {} + virtual ~MockChromeSyncNotificationBridge() {} + + MOCK_METHOD1(AddObserver, void(SyncNotifierObserver*)); + MOCK_METHOD1(RemoveObserver, void(SyncNotifierObserver*)); + private: + NiceMock<ProfileMock> mock_profile_; +}; + +class MockSyncNotifier : public SyncNotifier { + public: + MockSyncNotifier() {} + virtual ~MockSyncNotifier() {} + + MOCK_METHOD1(AddObserver, void(SyncNotifierObserver*)); + MOCK_METHOD1(RemoveObserver, void(SyncNotifierObserver*)); + MOCK_METHOD1(SetUniqueId, void(const std::string&)); + MOCK_METHOD1(SetState, void(const std::string&)); + MOCK_METHOD2(UpdateCredentials, void(const std::string&, const std::string&)); + MOCK_METHOD1(UpdateEnabledTypes, void(syncable::ModelTypeSet)); + MOCK_METHOD1(SendNotification, void(syncable::ModelTypeSet)); +}; + +// All tests just verify that each call is passed through to the delegate, with +// the exception of AddObserver/RemoveObserver, which also verify the observer +// is registered with the bridge. +class BridgedSyncNotifierTest : public testing::Test { + public: + BridgedSyncNotifierTest() + : ui_thread_(BrowserThread::UI, &ui_loop_), + mock_delegate_(new MockSyncNotifier), // Owned by bridged_notifier_. + bridged_notifier_(&mock_bridge_, mock_delegate_) {} + virtual ~BridgedSyncNotifierTest() {} + + protected: + MessageLoop ui_loop_; + content::TestBrowserThread ui_thread_; + StrictMock<MockChromeSyncNotificationBridge> mock_bridge_; + MockSyncNotifier* mock_delegate_; + BridgedSyncNotifier bridged_notifier_; +}; + +TEST_F(BridgedSyncNotifierTest, AddObserver) { + MockSyncNotifierObserver observer; + EXPECT_CALL(mock_bridge_, AddObserver(&observer)); + EXPECT_CALL(*mock_delegate_, AddObserver(&observer)); + bridged_notifier_.AddObserver(&observer); +} + +TEST_F(BridgedSyncNotifierTest, RemoveObserver) { + MockSyncNotifierObserver observer; + EXPECT_CALL(mock_bridge_, RemoveObserver(&observer)); + EXPECT_CALL(*mock_delegate_, RemoveObserver(&observer)); + bridged_notifier_.RemoveObserver(&observer); +} + +TEST_F(BridgedSyncNotifierTest, SetUniqueId) { + std::string unique_id = "unique id"; + EXPECT_CALL(*mock_delegate_, SetUniqueId(unique_id)); + bridged_notifier_.SetUniqueId(unique_id); +} + +TEST_F(BridgedSyncNotifierTest, SetState) { + std::string state = "state"; + EXPECT_CALL(*mock_delegate_, SetState(state)); + bridged_notifier_.SetState(state); +} + +TEST_F(BridgedSyncNotifierTest, UpdateCredentials) { + std::string email = "email"; + std::string token = "token"; + EXPECT_CALL(*mock_delegate_, UpdateCredentials(email, token)); + bridged_notifier_.UpdateCredentials(email, token); +} + +TEST_F(BridgedSyncNotifierTest, UpdateEnabledTypes) { + syncable::ModelTypeSet enabled_types(syncable::BOOKMARKS, + syncable::PREFERENCES); + EXPECT_CALL(*mock_delegate_, + UpdateEnabledTypes(HasModelTypes(enabled_types))); + bridged_notifier_.UpdateEnabledTypes(enabled_types); +} + +TEST_F(BridgedSyncNotifierTest, SendNotification) { + syncable::ModelTypeSet changed_types(syncable::SESSIONS, + syncable::EXTENSIONS); + EXPECT_CALL(*mock_delegate_, SendNotification(HasModelTypes(changed_types))); + bridged_notifier_.SendNotification(changed_types); +} + +} // namespace +} // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/chrome_sync_notification_bridge.cc b/chrome/browser/sync/notifier/chrome_sync_notification_bridge.cc new file mode 100644 index 0000000..3a724b4 --- /dev/null +++ b/chrome/browser/sync/notifier/chrome_sync_notification_bridge.cc @@ -0,0 +1,53 @@ +// 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 "chrome/browser/sync/notifier/chrome_sync_notification_bridge.h" + +#include "chrome/browser/sync/notifier/sync_notifier_observer.h" +#include "chrome/common/chrome_notification_types.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/notification_service.h" + +using content::BrowserThread; + +namespace sync_notifier { + +ChromeSyncNotificationBridge::ChromeSyncNotificationBridge( + const Profile* profile) + : observers_(new ObserverListThreadSafe<SyncNotifierObserver>()) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(profile); + registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH, + content::Source<Profile>(profile)); +} + +ChromeSyncNotificationBridge::~ChromeSyncNotificationBridge() {} + +void ChromeSyncNotificationBridge::AddObserver(SyncNotifierObserver* observer) { + observers_->AddObserver(observer); +} + +void ChromeSyncNotificationBridge::RemoveObserver( + SyncNotifierObserver* observer) { + observers_->RemoveObserver(observer); +} + +void ChromeSyncNotificationBridge::Observe( + int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK_EQ(type, chrome::NOTIFICATION_SYNC_REFRESH); + content::Details<const syncable::ModelType> model_type_details(details); + const syncable::ModelType model_type = *(model_type_details.ptr()); + + // Currently, we only expect SESSIONS to trigger this notification. + DCHECK_EQ(syncable::SESSIONS, model_type); + syncable::ModelTypePayloadMap payload_map; + payload_map[model_type] = ""; + observers_->Notify(&SyncNotifierObserver::OnIncomingNotification, + payload_map, LOCAL_NOTIFICATION); +} + +} // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/chrome_sync_notification_bridge.h b/chrome/browser/sync/notifier/chrome_sync_notification_bridge.h new file mode 100644 index 0000000..06d8056 --- /dev/null +++ b/chrome/browser/sync/notifier/chrome_sync_notification_bridge.h @@ -0,0 +1,50 @@ +// 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 CHROME_BROWSER_SYNC_NOTIFIER_CHROME_SYNC_NOTIFICATION_BRIDGE_H_ +#define CHROME_BROWSER_SYNC_NOTIFIER_CHROME_SYNC_NOTIFICATION_BRIDGE_H_ + +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" +#include "base/observer_list_threadsafe.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" + +class Profile; + +namespace sync_notifier { + +class SyncNotifierObserver; + +// A thread-safe bridge for chrome events that can trigger sync notifications. +// Currently only listens to NOTIFICATION_SYNC_REFRESH, triggering each +// observer's OnIncomingNotification method. Only the SESSIONS datatype is +// currently expected to be able to trigger this notification. +// Note: Notifications are expected to arrive on the UI thread, but observers +// may live on any thread. +class ChromeSyncNotificationBridge : public content::NotificationObserver { + public: + explicit ChromeSyncNotificationBridge(const Profile* profile); + virtual ~ChromeSyncNotificationBridge(); + + // These can be called on any thread. + virtual void AddObserver(SyncNotifierObserver* observer); + virtual void RemoveObserver(SyncNotifierObserver* observer); + + // NotificationObserver implementation. Called on UI thread. + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE; + + private: + content::NotificationRegistrar registrar_; + + // Because [Add/Remove]Observer can be called from any thread, we need a + // thread-safe observerlist. + scoped_refptr<ObserverListThreadSafe<SyncNotifierObserver> > observers_; +}; + +} // namespace sync_notifier + +#endif // CHROME_BROWSER_SYNC_NOTIFIER_CHROME_SYNC_NOTIFICATION_BRIDGE_H_ diff --git a/chrome/browser/sync/notifier/chrome_sync_notification_bridge_unittest.cc b/chrome/browser/sync/notifier/chrome_sync_notification_bridge_unittest.cc new file mode 100644 index 0000000..ffc56f5 --- /dev/null +++ b/chrome/browser/sync/notifier/chrome_sync_notification_bridge_unittest.cc @@ -0,0 +1,213 @@ +// 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 "chrome/browser/sync/notifier/chrome_sync_notification_bridge.h" + +#include "base/compiler_specific.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop.h" +#include "base/synchronization/waitable_event.h" +#include "base/test/test_timeouts.h" +#include "base/threading/thread.h" +#include "chrome/browser/sync/syncable/model_type.h" +#include "chrome/browser/sync/syncable/model_type_payload_map.h" +#include "chrome/browser/sync/notifier/mock_sync_notifier_observer.h" +#include "chrome/browser/sync/notifier/sync_notifier_observer.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/test/base/profile_mock.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_details.h" +#include "content/test/test_browser_thread.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace sync_notifier { +namespace { + +using ::testing::Mock; +using ::testing::NiceMock; +using ::testing::StrictMock; +using content::BrowserThread; + +// Receives a ChromeSyncNotificationBridge to register to, and an expected +// ModelTypePayloadMap. ReceivedProperNotification() will return true only +// if the observer has received a notification with the proper source and +// payload. +// Note: Because this object lives on the IO thread, we use a fake (vs a mock) +// so we don't have to worry about possible thread safety issues within +// GTest/GMock. +class FakeSyncNotifierObserverIO : public SyncNotifierObserver { + public: + FakeSyncNotifierObserverIO( + ChromeSyncNotificationBridge* bridge, + const syncable::ModelTypePayloadMap& expected_payloads) + : bridge_(bridge), + received_improper_notification_(false), + notification_count_(0), + expected_payloads_(expected_payloads) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + bridge_->AddObserver(this); + } + virtual ~FakeSyncNotifierObserverIO() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + bridge_->RemoveObserver(this); + } + + // SyncNotifierObserver implementation. + virtual void OnIncomingNotification( + const syncable::ModelTypePayloadMap& type_payloads, + IncomingNotificationSource source) OVERRIDE { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + notification_count_++; + if (source != LOCAL_NOTIFICATION) { + LOG(ERROR) << "Received notification with wrong source."; + received_improper_notification_ = true; + } + if (expected_payloads_ != type_payloads) { + LOG(ERROR) << "Received wrong payload."; + received_improper_notification_ = true; + } + } + virtual void OnNotificationStateChange(bool notifications_enabled) { + NOTREACHED(); + } + virtual void StoreState(const std::string& state) OVERRIDE { + NOTREACHED(); + } + + bool ReceivedProperNotification() const { + return (notification_count_ == 1) && !received_improper_notification_; + } + + private: + ChromeSyncNotificationBridge* bridge_; + bool received_improper_notification_; + size_t notification_count_; + syncable::ModelTypePayloadMap expected_payloads_; +}; + +class ChromeSyncNotificationBridgeTest : public testing::Test { + public: + ChromeSyncNotificationBridgeTest() + : ui_thread_(BrowserThread::UI, &ui_loop_), + io_thread_(BrowserThread::IO), + io_observer_(NULL), + io_observer_notification_failure_(false), + bridge_(&mock_profile_), + done_(true, false) { + io_thread_.StartIOThread(); + } + virtual ~ChromeSyncNotificationBridgeTest() {} + + protected: + virtual void TearDown() OVERRIDE { + io_thread_.Stop(); + ui_loop_.RunAllPending(); + EXPECT_EQ(NULL, io_observer_); + if (io_observer_notification_failure_) + ADD_FAILURE() << "IO Observer did not receive proper notification."; + } + + void VerifyAndDestroyObserverOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + if (!io_observer_) { + io_observer_notification_failure_ = true; + } else { + io_observer_notification_failure_ = + !io_observer_->ReceivedProperNotification(); + delete io_observer_; + io_observer_ = NULL; + } + } + + void VerifyAndDestroyObserver() { + ASSERT_TRUE(BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + base::Bind(&ChromeSyncNotificationBridgeTest:: + VerifyAndDestroyObserverOnIOThread, + base::Unretained(this)))); + } + + void CreateObserverOnIOThread( + syncable::ModelTypePayloadMap expected_payloads) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + io_observer_ = new FakeSyncNotifierObserverIO(&bridge_, + expected_payloads); + } + + void CreateObserverWithExpectedPayload( + syncable::ModelTypePayloadMap expected_payloads) { + ASSERT_TRUE(BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + base::Bind(&ChromeSyncNotificationBridgeTest::CreateObserverOnIOThread, + base::Unretained(this), + expected_payloads))); + BlockForIOThread(); + } + + void SignalOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + done_.Signal(); + } + + void BlockForIOThread() { + done_.Reset(); + BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + base::Bind(&ChromeSyncNotificationBridgeTest::SignalOnIOThread, + base::Unretained(this))); + done_.TimedWait(TestTimeouts::action_timeout()); + if (!done_.IsSignaled()) + ADD_FAILURE() << "Timed out waiting for IO thread."; + } + + void TriggerRefreshNotification() { + const syncable::ModelType type = syncable::SESSIONS; + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_SYNC_REFRESH, + content::Source<Profile>(&mock_profile_), + content::Details<const syncable::ModelType>(&type)); + } + + MessageLoop ui_loop_; + content::TestBrowserThread ui_thread_; + content::TestBrowserThread io_thread_; + NiceMock<ProfileMock> mock_profile_; + // Created/used/destroyed on I/O thread. + FakeSyncNotifierObserverIO* io_observer_; + bool io_observer_notification_failure_; + ChromeSyncNotificationBridge bridge_; + base::WaitableEvent done_; +}; + +// Adds an observer on the UI thread, triggers a refresh notification, and +// ensures the bridge posts a LOCAL_NOTIFICATION with the proper payload to it. +TEST_F(ChromeSyncNotificationBridgeTest, Basic) { + syncable::ModelTypePayloadMap payload_map; + payload_map[syncable::SESSIONS] = ""; + StrictMock<MockSyncNotifierObserver> observer; + EXPECT_CALL(observer, + OnIncomingNotification(payload_map, LOCAL_NOTIFICATION)); + bridge_.AddObserver(&observer); + TriggerRefreshNotification(); + ui_loop_.RunAllPending(); + Mock::VerifyAndClearExpectations(&observer); +} + +// Adds an observer on the I/O thread. Then triggers a refresh notification on +// the UI thread. We finally verify the proper notification was received by the +// observer and destroy it on the I/O thread. +TEST_F(ChromeSyncNotificationBridgeTest, BasicThreaded) { + syncable::ModelTypePayloadMap payload_map; + payload_map[syncable::SESSIONS] = ""; + CreateObserverWithExpectedPayload(payload_map); + TriggerRefreshNotification(); + VerifyAndDestroyObserver(); +} + +} // namespace +} // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/sync_notifier_factory.cc b/chrome/browser/sync/notifier/sync_notifier_factory.cc index 56b2cdb..56fb236 100644 --- a/chrome/browser/sync/notifier/sync_notifier_factory.cc +++ b/chrome/browser/sync/notifier/sync_notifier_factory.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -10,6 +10,7 @@ #include "base/logging.h" #include "base/string_number_conversions.h" #include "base/string_util.h" +#include "chrome/browser/sync/notifier/bridged_sync_notifier.h" #include "chrome/browser/sync/notifier/non_blocking_invalidation_notifier.h" #include "chrome/browser/sync/notifier/p2p_notifier.h" #include "chrome/browser/sync/notifier/sync_notifier.h" @@ -114,16 +115,19 @@ SyncNotifier* CreateDefaultSyncNotifier( notifier_options, initial_max_invalidation_versions, invalidation_version_tracker, client_info); } + } // namespace SyncNotifierFactory::SyncNotifierFactory( + const Profile* profile, const std::string& client_info, const scoped_refptr<net::URLRequestContextGetter>& request_context_getter, const base::WeakPtr<InvalidationVersionTracker>& invalidation_version_tracker, const CommandLine& command_line) - : client_info_(client_info), + : chrome_notification_bridge_(profile), + client_info_(client_info), request_context_getter_(request_context_getter), initial_max_invalidation_versions_( invalidation_version_tracker.get() ? @@ -132,6 +136,7 @@ SyncNotifierFactory::SyncNotifierFactory( invalidation_version_tracker_(invalidation_version_tracker), command_line_(command_line) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(profile); } SyncNotifierFactory::~SyncNotifierFactory() { @@ -139,10 +144,12 @@ SyncNotifierFactory::~SyncNotifierFactory() { } SyncNotifier* SyncNotifierFactory::CreateSyncNotifier() { - return CreateDefaultSyncNotifier(command_line_, - request_context_getter_, - initial_max_invalidation_versions_, - invalidation_version_tracker_, - client_info_); + return new BridgedSyncNotifier( + &chrome_notification_bridge_, + CreateDefaultSyncNotifier(command_line_, + request_context_getter_, + initial_max_invalidation_versions_, + invalidation_version_tracker_, + client_info_)); } } // namespace sync_notifier diff --git a/chrome/browser/sync/notifier/sync_notifier_factory.h b/chrome/browser/sync/notifier/sync_notifier_factory.h index 2c0069ce..32d61f3 100644 --- a/chrome/browser/sync/notifier/sync_notifier_factory.h +++ b/chrome/browser/sync/notifier/sync_notifier_factory.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -9,10 +9,12 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" +#include "chrome/browser/sync/notifier/chrome_sync_notification_bridge.h" #include "chrome/browser/sync/notifier/invalidation_version_tracker.h" #include "chrome/browser/sync/util/weak_handle.h" class CommandLine; +class Profile; namespace net { class URLRequestContextGetter; @@ -30,6 +32,7 @@ class SyncNotifierFactory { // agent string. |invalidation_version_tracker| may be NULL (for // tests). SyncNotifierFactory( + const Profile* profile, const std::string& client_info, const scoped_refptr<net::URLRequestContextGetter>& request_context_getter, @@ -44,6 +47,10 @@ class SyncNotifierFactory { SyncNotifier* CreateSyncNotifier(); private: + // A thread-safe listener for handling notifications triggered by chrome + // events. + ChromeSyncNotificationBridge chrome_notification_bridge_; + const std::string client_info_; const scoped_refptr<net::URLRequestContextGetter> request_context_getter_; const InvalidationVersionMap initial_max_invalidation_versions_; diff --git a/chrome/browser/sync/notifier/sync_notifier_factory_unittest.cc b/chrome/browser/sync/notifier/sync_notifier_factory_unittest.cc new file mode 100644 index 0000000..63ece70 --- /dev/null +++ b/chrome/browser/sync/notifier/sync_notifier_factory_unittest.cc @@ -0,0 +1,140 @@ +// 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 "chrome/browser/sync/notifier/sync_notifier_factory.h" + +#include "base/command_line.h" +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop.h" +#include "base/threading/thread.h" +#include "chrome/browser/sync/notifier/invalidation_version_tracker.h" +#include "chrome/browser/sync/notifier/mock_sync_notifier_observer.h" +#include "chrome/browser/sync/notifier/sync_notifier.h" +#include "chrome/browser/sync/syncable/model_type.h" +#include "chrome/browser/sync/syncable/model_type_payload_map.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/chrome_switches.h" +#include "chrome/test/base/profile_mock.h" +#include "chrome/test/base/test_url_request_context_getter.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_details.h" +#include "content/test/test_browser_thread.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace sync_notifier { +namespace { + +using ::testing::Mock; +using ::testing::NiceMock; +using ::testing::StrictMock; +using content::BrowserThread; + +class SyncNotifierFactoryTest : public testing::Test { + public: + // TODO(zea): switch to using real io_thread instead of shared message loop. + SyncNotifierFactoryTest() + : ui_thread_(BrowserThread::UI, &message_loop_), + io_thread_(BrowserThread::IO, &message_loop_), + command_line_(CommandLine::NO_PROGRAM) {} + virtual ~SyncNotifierFactoryTest() {} + + protected: + virtual void SetUp() OVERRIDE { + request_context_getter_ = new TestURLRequestContextGetter; + factory_.reset(new SyncNotifierFactory( + &mock_profile_, + "fake_client_info", + request_context_getter_, + base::WeakPtr<sync_notifier::InvalidationVersionTracker>(), + command_line_)); + message_loop_.RunAllPending(); + } + + virtual void TearDown() OVERRIDE { + Mock::VerifyAndClearExpectations(&mock_profile_); + Mock::VerifyAndClearExpectations(&mock_observer_); + request_context_getter_ = NULL; + message_loop_.RunAllPending(); + command_line_ = CommandLine(CommandLine::NO_PROGRAM); + } + + void TriggerRefreshNotification() { + const syncable::ModelType type = syncable::SESSIONS; + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_SYNC_REFRESH, + content::Source<Profile>(&mock_profile_), + content::Details<const syncable::ModelType>(&type)); + } + + MessageLoop message_loop_; + content::TestBrowserThread ui_thread_; + content::TestBrowserThread io_thread_; + NiceMock<ProfileMock> mock_profile_; + scoped_refptr<net::URLRequestContextGetter> request_context_getter_; + StrictMock<MockSyncNotifierObserver> mock_observer_; + scoped_ptr<SyncNotifierFactory> factory_; + CommandLine command_line_; +}; + +// Test basic creation of a NonBlockingInvalidationNotifier. +TEST_F(SyncNotifierFactoryTest, Basic) { + scoped_ptr<SyncNotifier> notifier(factory_->CreateSyncNotifier()); + ASSERT_TRUE(notifier.get()); + notifier->AddObserver(&mock_observer_); + notifier->RemoveObserver(&mock_observer_); +} + +// Test basic creation of a P2PNotifier. +TEST_F(SyncNotifierFactoryTest, Basic_P2P) { + command_line_.AppendSwitchASCII(switches::kSyncNotificationMethod, "p2p"); + scoped_ptr<SyncNotifier> notifier(factory_->CreateSyncNotifier()); + ASSERT_TRUE(notifier.get()); + notifier->AddObserver(&mock_observer_); + notifier->RemoveObserver(&mock_observer_); +} + +// Create a default sync notifier (NonBlockingInvalidationNotifier wrapped by a +// BridgedSyncNotifier) and then trigger a sync refresh notification. The +// observer should receive the notification as a LOCAL_NOTIFICATION. +TEST_F(SyncNotifierFactoryTest, ChromeSyncNotification) { + syncable::ModelTypePayloadMap type_payloads; + type_payloads[syncable::SESSIONS] = ""; + EXPECT_CALL(mock_observer_, + OnIncomingNotification(type_payloads, + LOCAL_NOTIFICATION)); + + scoped_ptr<SyncNotifier> notifier(factory_->CreateSyncNotifier()); + ASSERT_TRUE(notifier.get()); + notifier->AddObserver(&mock_observer_); + TriggerRefreshNotification(); + message_loop_.RunAllPending(); + notifier->RemoveObserver(&mock_observer_); +} + + +// Create a P2P sync notifier (wrapped by a BridgedSyncNotifier) +// and then trigger a sync refresh notification. The observer should receive +// the notification as a LOCAL_NOTIFICATION. +TEST_F(SyncNotifierFactoryTest, ChromeSyncNotification_P2P) { + syncable::ModelTypePayloadMap type_payloads; + type_payloads[syncable::SESSIONS] = ""; + EXPECT_CALL(mock_observer_, + OnIncomingNotification(type_payloads, + LOCAL_NOTIFICATION)); + + command_line_.AppendSwitchASCII(switches::kSyncNotificationMethod, "p2p"); + scoped_ptr<SyncNotifier> notifier(factory_->CreateSyncNotifier()); + ASSERT_TRUE(notifier.get()); + notifier->AddObserver(&mock_observer_); + TriggerRefreshNotification(); + message_loop_.RunAllPending(); + notifier->RemoveObserver(&mock_observer_); +} + +} // namespace +} // namespace sync_notifier diff --git a/chrome/browser/sync/tools/sync_listen_notifications.cc b/chrome/browser/sync/tools/sync_listen_notifications.cc index c5836ab..7395508 100644 --- a/chrome/browser/sync/tools/sync_listen_notifications.cc +++ b/chrome/browser/sync/tools/sync_listen_notifications.cc @@ -22,6 +22,7 @@ #include "chrome/browser/sync/syncable/model_type.h" #include "chrome/browser/sync/syncable/model_type_payload_map.h" #include "chrome/test/base/test_url_request_context_getter.h" +#include "chrome/test/base/testing_profile.h" #include "content/public/browser/browser_thread.h" #include "content/test/test_browser_thread.h" @@ -127,8 +128,9 @@ int main(int argc, char* argv[]) { scoped_refptr<TestURLRequestContextGetter> request_context_getter( new TestURLRequestContextGetter()); NullInvalidationVersionTracker null_invalidation_version_tracker; + TestingProfile profile_; sync_notifier::SyncNotifierFactory sync_notifier_factory( - kClientInfo, request_context_getter, + &profile_, kClientInfo, request_context_getter, null_invalidation_version_tracker.AsWeakPtr(), command_line); scoped_ptr<sync_notifier::SyncNotifier> sync_notifier( sync_notifier_factory.CreateSyncNotifier()); diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index a3fd78a..73adf6a 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -559,10 +559,14 @@ 'type': 'static_library', 'variables': { 'enable_wexit_time_destructors': 1, }, 'sources': [ + 'browser/sync/notifier/bridged_sync_notifier.h', + 'browser/sync/notifier/bridged_sync_notifier.cc', 'browser/sync/notifier/cache_invalidation_packet_handler.cc', 'browser/sync/notifier/cache_invalidation_packet_handler.h', 'browser/sync/notifier/chrome_invalidation_client.cc', 'browser/sync/notifier/chrome_invalidation_client.h', + 'browser/sync/notifier/chrome_sync_notification_bridge.h', + 'browser/sync/notifier/chrome_sync_notification_bridge.cc', 'browser/sync/notifier/chrome_system_resources.cc', 'browser/sync/notifier/chrome_system_resources.h', 'browser/sync/notifier/invalidation_notifier.h', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index d2148f3..3357130 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -3566,13 +3566,16 @@ 'browser/sync/js/js_event_details_unittest.cc', 'browser/sync/js/js_mutation_event_observer_unittest.cc', 'browser/sync/js/js_sync_manager_observer_unittest.cc', + 'browser/sync/notifier/bridged_sync_notifier_unittest.cc', 'browser/sync/notifier/cache_invalidation_packet_handler_unittest.cc', 'browser/sync/notifier/chrome_invalidation_client_unittest.cc', 'browser/sync/notifier/chrome_system_resources_unittest.cc', + 'browser/sync/notifier/chrome_sync_notification_bridge_unittest.cc', 'browser/sync/notifier/invalidation_notifier_unittest.cc', 'browser/sync/notifier/non_blocking_invalidation_notifier_unittest.cc', 'browser/sync/notifier/p2p_notifier_unittest.cc', 'browser/sync/notifier/registration_manager_unittest.cc', + 'browser/sync/notifier/sync_notifier_factory_unittest.cc', 'browser/sync/protocol/proto_enum_conversions_unittest.cc', 'browser/sync/protocol/proto_value_conversions_unittest.cc', 'browser/sync/sessions/ordered_commit_set_unittest.cc', |