summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/sync/notifier/p2p_notifier.cc9
-rw-r--r--jingle/jingle.gyp2
-rw-r--r--jingle/notifier/base/proxy_resolving_client_socket_unittest.cc5
-rw-r--r--jingle/notifier/listener/mediator_thread_impl.cc26
-rw-r--r--jingle/notifier/listener/mediator_thread_impl.h10
-rw-r--r--jingle/notifier/listener/mediator_thread_unittest.cc123
-rw-r--r--jingle/notifier/listener/notification_defines.cc13
-rw-r--r--jingle/notifier/listener/notification_defines.h2
-rw-r--r--jingle/notifier/listener/talk_mediator.h2
-rw-r--r--jingle/notifier/listener/talk_mediator_impl.cc10
-rw-r--r--jingle/notifier/listener/talk_mediator_impl.h6
-rw-r--r--jingle/notifier/listener/talk_mediator_unittest.cc21
12 files changed, 193 insertions, 36 deletions
diff --git a/chrome/browser/sync/notifier/p2p_notifier.cc b/chrome/browser/sync/notifier/p2p_notifier.cc
index 25c3182..46d9a9e 100644
--- a/chrome/browser/sync/notifier/p2p_notifier.cc
+++ b/chrome/browser/sync/notifier/p2p_notifier.cc
@@ -94,12 +94,7 @@ void P2PNotifier::SendNotification() {
notifier::Notification notification;
notification.channel = kSyncNotificationChannel;
notification.data = kSyncNotificationData;
- bool success = talk_mediator_->SendNotification(notification);
- if (success) {
- VLOG(1) << "Sent XMPP notification";
- } else {
- VLOG(1) << "Could not send XMPP notification";
- }
+ talk_mediator_->SendNotification(notification);
}
void P2PNotifier::OnNotificationStateChange(bool notifications_enabled) {
@@ -115,7 +110,7 @@ void P2PNotifier::OnIncomingNotification(
CheckOrSetValidThread();
VLOG(1) << "Sync received P2P notification.";
if (notification.channel != kSyncNotificationChannel) {
- LOG(WARNING) << "Notification fron unexpected source: "
+ LOG(WARNING) << "Notification from unexpected source: "
<< notification.channel;
}
MaybeEmitNotification();
diff --git a/jingle/jingle.gyp b/jingle/jingle.gyp
index aa5a720..fc600f5 100644
--- a/jingle/jingle.gyp
+++ b/jingle/jingle.gyp
@@ -80,6 +80,7 @@
'notifier/listener/mediator_thread_impl.h',
'notifier/listener/notification_constants.cc',
'notifier/listener/notification_constants.h',
+ 'notifier/listener/notification_defines.cc',
'notifier/listener/notification_defines.h',
'notifier/listener/push_notifications_listen_task.cc',
'notifier/listener/push_notifications_listen_task.h',
@@ -158,6 +159,7 @@
'notifier/communicator/xmpp_connection_generator_unittest.cc',
'notifier/listener/mediator_thread_mock.cc',
'notifier/listener/mediator_thread_mock.h',
+ 'notifier/listener/mediator_thread_unittest.cc',
'notifier/listener/push_notifications_send_update_task_unittest.cc',
'notifier/listener/push_notifications_subscribe_task_unittest.cc',
'notifier/listener/talk_mediator_unittest.cc',
diff --git a/jingle/notifier/base/proxy_resolving_client_socket_unittest.cc b/jingle/notifier/base/proxy_resolving_client_socket_unittest.cc
index b8aa4b4..22babba 100644
--- a/jingle/notifier/base/proxy_resolving_client_socket_unittest.cc
+++ b/jingle/notifier/base/proxy_resolving_client_socket_unittest.cc
@@ -59,13 +59,14 @@ class ProxyResolvingClientSocketTest : public testing::Test {
MOCK_METHOD1(NetCallback, void(int status));
// Needed by XmppConnection.
- MessageLoop message_loop_;
+ MessageLoopForIO message_loop_;
scoped_refptr<TestURLRequestContextGetter> url_request_context_getter_;
net::CapturingNetLog capturing_net_log_;
net::CompletionCallbackImpl<ProxyResolvingClientSocketTest> connect_callback_;
};
-TEST_F(ProxyResolvingClientSocketTest, ConnectError) {
+// TODO(sanjeevr): Fix this test on Linux.
+TEST_F(ProxyResolvingClientSocketTest, DISABLED_ConnectError) {
net::HostPortPair dest("0.0.0.0", 0);
ProxyResolvingClientSocket proxy_resolving_socket(
url_request_context_getter_,
diff --git a/jingle/notifier/listener/mediator_thread_impl.cc b/jingle/notifier/listener/mediator_thread_impl.cc
index 38520e4..6826e78 100644
--- a/jingle/notifier/listener/mediator_thread_impl.cc
+++ b/jingle/notifier/listener/mediator_thread_impl.cc
@@ -61,13 +61,16 @@ class MediatorThreadImpl::Core
private:
friend class base::RefCountedThreadSafe<MediatorThreadImpl::Core>;
// Invoked on either the caller thread or the I/O thread.
- ~Core();
+ virtual ~Core();
scoped_refptr<ObserverListThreadSafe<Observer> > observers_;
base::WeakPtr<talk_base::Task> base_task_;
const NotifierOptions notifier_options_;
scoped_ptr<notifier::Login> login_;
+
+ std::vector<Notification> pending_notifications_to_send_;
+
DISALLOW_COPY_AND_ASSIGN(Core);
};
@@ -158,6 +161,9 @@ void MediatorThreadImpl::Core::SendNotification(const Notification& data) {
DCHECK(notifier_options_.request_context_getter->GetIOMessageLoopProxy()->
BelongsToCurrentThread());
if (!base_task_.get()) {
+ VLOG(1) << "P2P: Cannot send notification " << data.ToString()
+ << "; sending later";
+ pending_notifications_to_send_.push_back(data);
return;
}
// Owned by |base_task_|.
@@ -186,6 +192,14 @@ void MediatorThreadImpl::Core::OnConnect(
BelongsToCurrentThread());
base_task_ = base_task;
observers_->Notify(&Observer::OnConnectionStateChange, true);
+ std::vector<Notification> notifications_to_send;
+ notifications_to_send.swap(pending_notifications_to_send_);
+ for (std::vector<Notification>::const_iterator it =
+ notifications_to_send.begin();
+ it != notifications_to_send.end(); ++it) {
+ VLOG(1) << "P2P: Sending pending notification " << it->ToString();
+ SendNotification(*it);
+ }
}
void MediatorThreadImpl::Core::OnDisconnect() {
@@ -276,6 +290,16 @@ void MediatorThreadImpl::UpdateXmppSettings(
settings));
}
+void MediatorThreadImpl::TriggerOnConnectForTest(
+ base::WeakPtr<talk_base::Task> base_task) {
+ CheckOrSetValidThread();
+ io_message_loop_proxy_->PostTask(
+ FROM_HERE,
+ NewRunnableMethod(core_.get(),
+ &MediatorThreadImpl::Core::OnConnect,
+ base_task));
+}
+
void MediatorThreadImpl::LogoutImpl() {
io_message_loop_proxy_->PostTask(
FROM_HERE,
diff --git a/jingle/notifier/listener/mediator_thread_impl.h b/jingle/notifier/listener/mediator_thread_impl.h
index 4015a81..0656757 100644
--- a/jingle/notifier/listener/mediator_thread_impl.h
+++ b/jingle/notifier/listener/mediator_thread_impl.h
@@ -25,6 +25,7 @@
#include "base/basictypes.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
#include "base/task.h"
#include "jingle/notifier/base/notifier_options.h"
#include "jingle/notifier/listener/mediator_thread.h"
@@ -37,6 +38,10 @@ namespace buzz {
class XmppClientSettings;
} // namespace buzz
+namespace talk_base {
+class Task;
+} // namespace talk_base
+
namespace notifier {
class MediatorThreadImpl : public MediatorThread {
@@ -59,6 +64,11 @@ class MediatorThreadImpl : public MediatorThread {
virtual void SendNotification(const Notification& data);
virtual void UpdateXmppSettings(const buzz::XmppClientSettings& settings);
+ // Used by unit tests. Make sure that tests that use this have the
+ // IO message loop proxy passed in via |notifier_options| pointing
+ // to the current thread.
+ void TriggerOnConnectForTest(base::WeakPtr<talk_base::Task> base_task);
+
private:
void CheckOrSetValidThread();
// The logic of Logout without the thread check so it can be called in the
diff --git a/jingle/notifier/listener/mediator_thread_unittest.cc b/jingle/notifier/listener/mediator_thread_unittest.cc
new file mode 100644
index 0000000..b97929d
--- /dev/null
+++ b/jingle/notifier/listener/mediator_thread_unittest.cc
@@ -0,0 +1,123 @@
+// Copyright (c) 2011 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 "jingle/notifier/listener/mediator_thread_impl.h"
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "jingle/notifier/base/fake_base_task.h"
+#include "jingle/notifier/base/notifier_options.h"
+#include "jingle/notifier/listener/notification_defines.h"
+#include "net/base/capturing_net_log.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace notifier {
+
+namespace {
+
+using ::testing::StrictMock;
+
+// TODO(sanjeevr): Move this to net_test_support.
+// Used to return a dummy context.
+class TestURLRequestContextGetter : public net::URLRequestContextGetter {
+ public:
+ TestURLRequestContextGetter()
+ : message_loop_proxy_(base::MessageLoopProxy::CreateForCurrentThread()) {
+ }
+ virtual ~TestURLRequestContextGetter() { }
+
+ // net::URLRequestContextGetter:
+ virtual net::URLRequestContext* GetURLRequestContext() {
+ if (!context_)
+ context_ = new TestURLRequestContext();
+ return context_.get();
+ }
+ virtual scoped_refptr<base::MessageLoopProxy> GetIOMessageLoopProxy() const {
+ return message_loop_proxy_;
+ }
+
+ private:
+ scoped_refptr<net::URLRequestContext> context_;
+ scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
+};
+
+class MockObserver : public MediatorThread::Observer {
+ public:
+ MOCK_METHOD1(OnConnectionStateChange, void(bool));
+ MOCK_METHOD1(OnSubscriptionStateChange, void(bool));
+ MOCK_METHOD1(OnIncomingNotification, void(const Notification&));
+ MOCK_METHOD0(OnOutgoingNotification, void());
+};
+
+} // namespace
+
+class MediatorThreadTest : public testing::Test {
+ protected:
+ MediatorThreadTest() {
+ notifier_options_.request_context_getter =
+ new TestURLRequestContextGetter();
+ }
+
+ virtual ~MediatorThreadTest() {}
+
+ virtual void SetUp() {
+ mediator_thread_.reset(new MediatorThreadImpl(notifier_options_));
+ mediator_thread_->AddObserver(&mock_observer_);
+ }
+
+ virtual void TearDown() {
+ // Clear out any messages posted by MediatorThread's
+ // destructor.
+ message_loop_.RunAllPending();
+ mediator_thread_->RemoveObserver(&mock_observer_);
+ mediator_thread_.reset();
+ }
+
+ // Needed by TestURLRequestContextGetter.
+ MessageLoopForIO message_loop_;
+ NotifierOptions notifier_options_;
+ StrictMock<MockObserver> mock_observer_;
+ scoped_ptr<MediatorThreadImpl> mediator_thread_;
+ FakeBaseTask fake_base_task_;
+};
+
+TEST_F(MediatorThreadTest, SendNotificationBasic) {
+ EXPECT_CALL(mock_observer_, OnConnectionStateChange(true));
+ EXPECT_CALL(mock_observer_, OnOutgoingNotification());
+
+ mediator_thread_->TriggerOnConnectForTest(fake_base_task_.AsWeakPtr());
+ mediator_thread_->SendNotification(Notification());
+ mediator_thread_->Logout();
+
+ // Shouldn't trigger.
+ mediator_thread_->SendNotification(Notification());
+}
+
+TEST_F(MediatorThreadTest, SendNotificationDelayed) {
+ EXPECT_CALL(mock_observer_, OnConnectionStateChange(true));
+ EXPECT_CALL(mock_observer_, OnOutgoingNotification()).Times(5);
+
+ for (int i = 0; i < 5; ++i) {
+ mediator_thread_->SendNotification(Notification());
+ }
+ mediator_thread_->TriggerOnConnectForTest(fake_base_task_.AsWeakPtr());
+}
+
+TEST_F(MediatorThreadTest, SendNotificationDelayedTwice) {
+ EXPECT_CALL(mock_observer_, OnConnectionStateChange(true)).Times(2);
+ EXPECT_CALL(mock_observer_, OnOutgoingNotification()).Times(5);
+
+ for (int i = 0; i < 5; ++i) {
+ mediator_thread_->SendNotification(Notification());
+ }
+ mediator_thread_->TriggerOnConnectForTest(
+ base::WeakPtr<talk_base::Task>());
+ mediator_thread_->TriggerOnConnectForTest(fake_base_task_.AsWeakPtr());
+}
+
+} // namespace notifier
diff --git a/jingle/notifier/listener/notification_defines.cc b/jingle/notifier/listener/notification_defines.cc
new file mode 100644
index 0000000..61b099a
--- /dev/null
+++ b/jingle/notifier/listener/notification_defines.cc
@@ -0,0 +1,13 @@
+// Copyright (c) 2011 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 "jingle/notifier/listener/notification_defines.h"
+
+namespace notifier {
+
+std::string Notification::ToString() const {
+ return "{ channel: \"" + channel + "\", data: \"" + data + "\" }";
+}
+
+} // namespace notifier
diff --git a/jingle/notifier/listener/notification_defines.h b/jingle/notifier/listener/notification_defines.h
index 5cf9f72..b17407f 100644
--- a/jingle/notifier/listener/notification_defines.h
+++ b/jingle/notifier/listener/notification_defines.h
@@ -26,6 +26,8 @@ struct Notification {
std::string channel;
// The notification data payload.
std::string data;
+
+ std::string ToString() const;
};
} // namespace notifier
diff --git a/jingle/notifier/listener/talk_mediator.h b/jingle/notifier/listener/talk_mediator.h
index 7ea23b0..7a6fa79 100644
--- a/jingle/notifier/listener/talk_mediator.h
+++ b/jingle/notifier/listener/talk_mediator.h
@@ -50,7 +50,7 @@ class TalkMediator {
// Method for the owner of this object to notify peers that an update has
// occurred.
- virtual bool SendNotification(const Notification& data) = 0;
+ virtual void SendNotification(const Notification& data) = 0;
// Add a subscription to subscribe to.
virtual void AddSubscription(const Subscription& subscription) = 0;
diff --git a/jingle/notifier/listener/talk_mediator_impl.cc b/jingle/notifier/listener/talk_mediator_impl.cc
index 5bd70dc..5b90663 100644
--- a/jingle/notifier/listener/talk_mediator_impl.cc
+++ b/jingle/notifier/listener/talk_mediator_impl.cc
@@ -45,7 +45,6 @@ bool TalkMediatorImpl::Logout() {
state_.started = 0;
state_.logging_in = 0;
state_.logged_in = 0;
- state_.subscribed = 0;
// We do not want to be called back during logout since we may be
// closing.
mediator_thread_->RemoveObserver(this);
@@ -55,13 +54,9 @@ bool TalkMediatorImpl::Logout() {
return false;
}
-bool TalkMediatorImpl::SendNotification(const Notification& data) {
+void TalkMediatorImpl::SendNotification(const Notification& data) {
CheckOrSetValidThread();
- if (state_.logged_in && state_.subscribed) {
- mediator_thread_->SendNotification(data);
- return true;
- }
- return false;
+ mediator_thread_->SendNotification(data);
}
void TalkMediatorImpl::SetDelegate(TalkMediator::Delegate* delegate) {
@@ -116,7 +111,6 @@ void TalkMediatorImpl::OnConnectionStateChange(bool logged_in) {
void TalkMediatorImpl::OnSubscriptionStateChange(bool subscribed) {
CheckOrSetValidThread();
- state_.subscribed = subscribed;
VLOG(1) << "P2P: " << (subscribed ? "subscribed" : "unsubscribed");
if (delegate_)
delegate_->OnNotificationStateChange(subscribed);
diff --git a/jingle/notifier/listener/talk_mediator_impl.h b/jingle/notifier/listener/talk_mediator_impl.h
index d563ed0..718bb89 100644
--- a/jingle/notifier/listener/talk_mediator_impl.h
+++ b/jingle/notifier/listener/talk_mediator_impl.h
@@ -52,7 +52,7 @@ class TalkMediatorImpl
// Users must call Logout once Login is called.
virtual bool Logout();
- virtual bool SendNotification(const Notification& data);
+ virtual void SendNotification(const Notification& data);
virtual void AddSubscription(const Subscription& subscription);
@@ -69,8 +69,7 @@ class TalkMediatorImpl
private:
struct TalkMediatorState {
TalkMediatorState()
- : started(0), initialized(0), logging_in(0),
- logged_in(0), subscribed(0) {
+ : started(0), initialized(0), logging_in(0), logged_in(0) {
}
unsigned int started : 1; // Background thread has started.
@@ -78,7 +77,6 @@ class TalkMediatorImpl
unsigned int logging_in : 1; // Logging in to the mediator's
// authenticator.
unsigned int logged_in : 1; // Logged in the mediator's authenticator.
- unsigned int subscribed : 1; // Subscribed to the xmpp receiving channel.
};
void CheckOrSetValidThread();
diff --git a/jingle/notifier/listener/talk_mediator_unittest.cc b/jingle/notifier/listener/talk_mediator_unittest.cc
index 7b93d64..c72e0e9 100644
--- a/jingle/notifier/listener/talk_mediator_unittest.cc
+++ b/jingle/notifier/listener/talk_mediator_unittest.cc
@@ -110,29 +110,25 @@ TEST_F(TalkMediatorImplTest, SendNotification) {
MockMediatorThread* mock = new MockMediatorThread();
scoped_ptr<TalkMediatorImpl> talk1(NewMockedTalkMediator(mock));
- // Failure due to not being logged in.
Notification data;
- EXPECT_FALSE(talk1->SendNotification(data));
- EXPECT_EQ(0, mock->send_calls);
+ talk1->SendNotification(data);
+ EXPECT_EQ(1, mock->send_calls);
talk1->SetAuthToken("chromium@gmail.com", "token", "fake_service");
EXPECT_TRUE(talk1->Login());
talk1->OnConnectionStateChange(true);
EXPECT_EQ(1, mock->login_calls);
- // Should be subscribed now.
- EXPECT_TRUE(talk1->state_.subscribed);
- EXPECT_TRUE(talk1->SendNotification(data));
- EXPECT_EQ(1, mock->send_calls);
- EXPECT_TRUE(talk1->SendNotification(data));
+ talk1->SendNotification(data);
EXPECT_EQ(2, mock->send_calls);
+ talk1->SendNotification(data);
+ EXPECT_EQ(3, mock->send_calls);
EXPECT_TRUE(talk1->Logout());
EXPECT_EQ(1, mock->logout_calls);
- // Failure due to being logged out.
- EXPECT_FALSE(talk1->SendNotification(data));
- EXPECT_EQ(2, mock->send_calls);
+ talk1->SendNotification(data);
+ EXPECT_EQ(4, mock->send_calls);
}
TEST_F(TalkMediatorImplTest, MediatorThreadCallbacks) {
@@ -154,12 +150,11 @@ TEST_F(TalkMediatorImplTest, MediatorThreadCallbacks) {
// The message triggers calls to listen and subscribe.
EXPECT_EQ(1, mock->listen_calls);
EXPECT_EQ(1, mock->subscribe_calls);
- EXPECT_TRUE(talk1->state_.subscribed);
// After subscription success is receieved, the talk mediator will allow
// sending of notifications.
Notification outgoing_data;
- EXPECT_TRUE(talk1->SendNotification(outgoing_data));
+ talk1->SendNotification(outgoing_data);
EXPECT_EQ(1, mock->send_calls);
Notification incoming_data;