Converted TalkMediator to use a delegate instead of a channel

Simplified handling of TalkMediator events.

BUG=none
TEST=unittests + manual testing of notifications

Review URL: http://codereview.chromium.org/2232004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48652 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 123 insertions, 133 deletions

diff --git a/chrome/common/net/notifier/DEPS b/chrome/common/net/notifier/DEPS
index 95cce2f..2d7d13f 100644
--- a/chrome/common/net/notifier/DEPS
+++ b/chrome/common/net/notifier/DEPS
@@ -3,6 +3,4 @@ include_rules = [
   "+talk/base",
   "+talk/xmpp",
   "+talk/xmllite",
-  "+talk/p2p/base",  # TODO(ncarter): Determine if this is necessary/proper.
-
 ]
diff --git a/chrome/common/net/notifier/listener/talk_mediator.h b/chrome/common/net/notifier/listener/talk_mediator.h
index 5580691..c842b1b 100644
--- a/chrome/common/net/notifier/listener/talk_mediator.h
+++ b/chrome/common/net/notifier/listener/talk_mediator.h
@@ -19,39 +19,29 @@
 #include <string>
 
 #include "chrome/common/net/notifier/listener/notification_defines.h"
-#include "chrome/common/deprecated/event_sys.h"
 
 namespace notifier {
 
-struct TalkMediatorEvent {
-  enum WhatHappened {
-    LOGIN_SUCCEEDED,
-    LOGOUT_SUCCEEDED,
-    SUBSCRIPTIONS_ON,
-    SUBSCRIPTIONS_OFF,
-    NOTIFICATION_RECEIVED,
-    NOTIFICATION_SENT,
-    TALKMEDIATOR_DESTROYED,
-  };
+class TalkMediator {
+ public:
+  TalkMediator() {}
+  virtual ~TalkMediator() {}
 
-  // Required by EventChannel.
-  typedef TalkMediatorEvent EventType;
+  class Delegate {
+   public:
+    virtual ~Delegate() {}
 
-  static inline bool IsChannelShutdownEvent(const TalkMediatorEvent& event) {
-    return event.what_happened == TALKMEDIATOR_DESTROYED;
-  }
+    virtual void OnNotificationStateChange(
+        bool notifications_enabled) = 0;
 
-  WhatHappened what_happened;
-  // Data in the case of a NOTIFICATION_RECEIVED event
-  IncomingNotificationData notification_data;
-};
+    virtual void OnIncomingNotification(
+        const IncomingNotificationData& notification_data) = 0;
 
-typedef EventChannel<TalkMediatorEvent, Lock> TalkMediatorChannel;
+    virtual void OnOutgoingNotification() = 0;
+  };
 
-class TalkMediator {
- public:
-  TalkMediator() {}
-  virtual ~TalkMediator() {}
+  // |delegate| may be NULL.
+  virtual void SetDelegate(Delegate* delegate) = 0;
 
   // The following methods are for authorizaiton of the xmpp client.
   virtual bool SetAuthToken(const std::string& email,
@@ -64,9 +54,6 @@ class TalkMediator {
   // occurred.
   virtual bool SendNotification(const OutgoingNotificationData& data) = 0;
 
-  // Channel by which talk mediator events are signaled.
-  virtual TalkMediatorChannel* channel() const = 0;
-
   // Add a URL to subscribe to for notifications.
   virtual void AddSubscribedServiceUrl(const std::string& service_url) = 0;
 };
diff --git a/chrome/common/net/notifier/listener/talk_mediator_impl.cc b/chrome/common/net/notifier/listener/talk_mediator_impl.cc
index f2e3d2e..ff5c62c 100644
--- a/chrome/common/net/notifier/listener/talk_mediator_impl.cc
+++ b/chrome/common/net/notifier/listener/talk_mediator_impl.cc
@@ -7,7 +7,6 @@
 #include "base/logging.h"
 #include "base/singleton.h"
 #include "chrome/common/net/notifier/listener/mediator_thread_impl.h"
-#include "chrome/common/deprecated/event_sys-inl.h"
 #include "talk/base/cryptstring.h"
 #include "talk/base/ssladapter.h"
 #include "talk/xmpp/xmppclientsettings.h"
@@ -43,7 +42,8 @@ TalkMediatorImpl::TalkMediatorImpl(
       chrome_common_net::NetworkChangeNotifierThread*
           network_change_notifier_thread,
       bool invalidate_xmpp_auth_token)
-    : mediator_thread_(
+    : delegate_(NULL),
+      mediator_thread_(
         new MediatorThreadImpl(network_change_notifier_thread)),
       invalidate_xmpp_auth_token_(invalidate_xmpp_auth_token) {
   // Ensure the SSL library is initialized.
@@ -54,15 +54,14 @@ TalkMediatorImpl::TalkMediatorImpl(
 }
 
 TalkMediatorImpl::TalkMediatorImpl(MediatorThread *thread)
-    : mediator_thread_(thread),
+    : delegate_(NULL),
+      mediator_thread_(thread),
       invalidate_xmpp_auth_token_(false) {
   // When testing we do not initialize the SSL library.
   TalkMediatorInitialization(true);
 }
 
 void TalkMediatorImpl::TalkMediatorInitialization(bool should_connect) {
-  TalkMediatorEvent done = { TalkMediatorEvent::TALKMEDIATOR_DESTROYED };
-  channel_.reset(new TalkMediatorChannel(done));
   if (should_connect) {
     mediator_thread_->SignalStateChange.connect(
         this,
@@ -130,8 +129,9 @@ bool TalkMediatorImpl::SendNotification(const OutgoingNotificationData& data) {
   return false;
 }
 
-TalkMediatorChannel* TalkMediatorImpl::channel() const {
-  return channel_.get();
+void TalkMediatorImpl::SetDelegate(Delegate* delegate) {
+  AutoLock lock(mutex_);
+  delegate_ = delegate;
 }
 
 bool TalkMediatorImpl::SetAuthToken(const std::string& email,
@@ -197,9 +197,9 @@ void TalkMediatorImpl::MediatorThreadNotificationHandler(
     const IncomingNotificationData& notification_data) {
   LOG(INFO) << "P2P: Updates are available on the server.";
   AutoLock lock(mutex_);
-  TalkMediatorEvent event = { TalkMediatorEvent::NOTIFICATION_RECEIVED };
-  event.notification_data = notification_data;
-  channel_->NotifyListeners(event);
+  if (delegate_) {
+    delegate_->OnIncomingNotification(notification_data);
+  }
 }
 
 
@@ -213,8 +213,6 @@ void TalkMediatorImpl::OnLogin() {
   mediator_thread_->ListenForUpdates();
   // Now subscribe for updates to all the services we are interested in
   mediator_thread_->SubscribeForUpdates(subscribed_services_list_);
-  TalkMediatorEvent event = { TalkMediatorEvent::LOGIN_SUCCEEDED };
-  channel_->NotifyListeners(event);
 }
 
 void TalkMediatorImpl::OnLogout() {
@@ -223,8 +221,6 @@ void TalkMediatorImpl::OnLogout() {
   AutoLock lock(mutex_);
   state_.logging_in = 0;
   state_.logged_in = 0;
-  TalkMediatorEvent event = { TalkMediatorEvent::LOGOUT_SUCCEEDED };
-  channel_->NotifyListeners(event);
 }
 
 void TalkMediatorImpl::OnSubscriptionSuccess() {
@@ -238,24 +234,27 @@ void TalkMediatorImpl::OnSubscriptionSuccess() {
   // Notifying listeners with a lock held can cause the lock to be
   // recursively taken if the listener decides to call back into us
   // in the event handler.
-  TalkMediatorEvent event = { TalkMediatorEvent::SUBSCRIPTIONS_ON };
-  channel_->NotifyListeners(event);
+  if (delegate_) {
+    delegate_->OnNotificationStateChange(true);
+  }
 }
 
 void TalkMediatorImpl::OnSubscriptionFailure() {
   LOG(INFO) << "P2P: Update subscription failure.";
   AutoLock lock(mutex_);
   state_.subscribed = 0;
-  TalkMediatorEvent event = { TalkMediatorEvent::SUBSCRIPTIONS_OFF };
-  channel_->NotifyListeners(event);
+  if (delegate_) {
+    delegate_->OnNotificationStateChange(false);
+  }
 }
 
 void TalkMediatorImpl::OnNotificationSent() {
   LOG(INFO) <<
       "P2P: Peers were notified that updates are available on the server.";
   AutoLock lock(mutex_);
-  TalkMediatorEvent event = { TalkMediatorEvent::NOTIFICATION_SENT };
-  channel_->NotifyListeners(event);
+  if (delegate_) {
+    delegate_->OnOutgoingNotification();
+  }
 }
 
 }  // namespace notifier
diff --git a/chrome/common/net/notifier/listener/talk_mediator_impl.h b/chrome/common/net/notifier/listener/talk_mediator_impl.h
index cb965b7..de28fc5 100644
--- a/chrome/common/net/notifier/listener/talk_mediator_impl.h
+++ b/chrome/common/net/notifier/listener/talk_mediator_impl.h
@@ -20,8 +20,6 @@
 #include "talk/xmpp/xmppclientsettings.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"  // For FRIEND_TEST
 
-class EventListenerHookup;
-
 namespace chrome_common_net {
 class NetworkChangeNotifierThread;
 }  // namespace chrome_common_net
@@ -40,6 +38,9 @@ class TalkMediatorImpl
   virtual ~TalkMediatorImpl();
 
   // Overriden from TalkMediator.
+
+  virtual void SetDelegate(Delegate* delegate);
+
   virtual bool SetAuthToken(const std::string& email,
                             const std::string& token,
                             const std::string& token_service);
@@ -48,8 +49,6 @@ class TalkMediatorImpl
 
   virtual bool SendNotification(const OutgoingNotificationData& data);
 
-  TalkMediatorChannel* channel() const;
-
   virtual void AddSubscribedServiceUrl(const std::string& service_url);
 
  private:
@@ -91,21 +90,18 @@ class TalkMediatorImpl
   // this mutex.
   Lock mutex_;
 
+  // Delegate.  May be NULL.
+  Delegate* delegate_;
+
   // Internal state.
   TalkMediatorState state_;
 
   // Cached and verfied from the SetAuthToken method.
   buzz::XmppClientSettings xmpp_settings_;
 
-  // Interface to listen to authentication events.
-  scoped_ptr<EventListenerHookup> auth_hookup_;
-
   // The worker thread through which talk events are posted and received.
   scoped_ptr<MediatorThread> mediator_thread_;
 
-  // Channel through which to broadcast events.
-  scoped_ptr<TalkMediatorChannel> channel_;
-
   bool invalidate_xmpp_auth_token_;
 
   std::vector<std::string> subscribed_services_list_;
diff --git a/chrome/common/net/notifier/listener/talk_mediator_unittest.cc b/chrome/common/net/notifier/listener/talk_mediator_unittest.cc
index 3bdac80..621e688 100644
--- a/chrome/common/net/notifier/listener/talk_mediator_unittest.cc
+++ b/chrome/common/net/notifier/listener/talk_mediator_unittest.cc
@@ -4,81 +4,88 @@
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/logging.h"
 #include "chrome/common/net/fake_network_change_notifier_thread.h"
 #include "chrome/common/net/notifier/listener/mediator_thread_mock.h"
 #include "chrome/common/net/notifier/listener/talk_mediator_impl.h"
-#include "chrome/common/deprecated/event_sys-inl.h"
 #include "talk/xmpp/xmppengine.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace notifier {
 
-class TalkMediatorImplTest : public testing::Test {
+using ::testing::_;
+
+class MockTalkMediatorDelegate : public TalkMediator::Delegate {
  public:
-  void HandleTalkMediatorEvent(
-      const notifier::TalkMediatorEvent& event) {
-    last_message_ = event.what_happened;
-  }
+  MockTalkMediatorDelegate() {}
+  virtual ~MockTalkMediatorDelegate() {}
 
- protected:
-  TalkMediatorImplTest() {}
-  ~TalkMediatorImplTest() {}
+  MOCK_METHOD1(OnNotificationStateChange,
+               void(bool notification_changed));
+  MOCK_METHOD1(OnIncomingNotification,
+               void(const IncomingNotificationData& data));
+  MOCK_METHOD0(OnOutgoingNotification, void());
 
-  virtual void SetUp() {
-    last_message_ = -1;
-  }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockTalkMediatorDelegate);
+};
 
-  virtual void TearDown() {
-  }
+class TalkMediatorImplTest : public testing::Test {
+ protected:
+  TalkMediatorImplTest() {}
+  virtual ~TalkMediatorImplTest() {}
 
   chrome_common_net::FakeNetworkChangeNotifierThread
       fake_network_change_notifier_thread_;
   int last_message_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TalkMediatorImplTest);
 };
 
 TEST_F(TalkMediatorImplTest, ConstructionOfTheClass) {
   // Constructing a single talk mediator enables SSL through the singleton.
   scoped_ptr<TalkMediatorImpl> talk1(
       new TalkMediatorImpl(&fake_network_change_notifier_thread_, false));
-  talk1.reset(NULL);
 }
 
 TEST_F(TalkMediatorImplTest, SetAuthTokenWithBadInput) {
   scoped_ptr<TalkMediatorImpl> talk1(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_FALSE(talk1->SetAuthToken("@missinguser.com", "", "fake_service"));
-  ASSERT_TRUE(talk1->state_.initialized == 0);
+  EXPECT_FALSE(talk1->SetAuthToken("@missinguser.com", "", "fake_service"));
+  EXPECT_FALSE(talk1->state_.initialized);
 
   scoped_ptr<TalkMediatorImpl> talk2(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_FALSE(talk2->SetAuthToken("", "1234567890", "fake_service"));
-  ASSERT_TRUE(talk2->state_.initialized == 0);
+  EXPECT_FALSE(talk2->SetAuthToken("", "1234567890", "fake_service"));
+  EXPECT_FALSE(talk2->state_.initialized);
 
   scoped_ptr<TalkMediatorImpl> talk3(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_FALSE(talk3->SetAuthToken("missingdomain", "abcde",  "fake_service"));
-  ASSERT_TRUE(talk3->state_.initialized == 0);
+  EXPECT_FALSE(talk3->SetAuthToken("missingdomain", "abcde",  "fake_service"));
+  EXPECT_FALSE(talk3->state_.initialized);
 }
 
 TEST_F(TalkMediatorImplTest, SetAuthTokenWithGoodInput) {
   scoped_ptr<TalkMediatorImpl> talk1(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
+  EXPECT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
                                   "fake_service"));
-  ASSERT_TRUE(talk1->state_.initialized == 1);
+  EXPECT_TRUE(talk1->state_.initialized);
 
   scoped_ptr<TalkMediatorImpl> talk2(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_TRUE(talk2->SetAuthToken("chromium@mail.google.com", "token",
+  EXPECT_TRUE(talk2->SetAuthToken("chromium@mail.google.com", "token",
                                   "fake_service"));
-  ASSERT_TRUE(talk2->state_.initialized == 1);
+  EXPECT_TRUE(talk2->state_.initialized);
 
   scoped_ptr<TalkMediatorImpl> talk3(new TalkMediatorImpl(
       new MockMediatorThread()));
-  ASSERT_TRUE(talk3->SetAuthToken("chromium@chromium.org", "token",
+  EXPECT_TRUE(talk3->SetAuthToken("chromium@chromium.org", "token",
                                   "fake_service"));
-  ASSERT_TRUE(talk3->state_.initialized == 1);
+  EXPECT_TRUE(talk3->state_.initialized);
 }
 
 TEST_F(TalkMediatorImplTest, LoginWiring) {
@@ -87,25 +94,25 @@ TEST_F(TalkMediatorImplTest, LoginWiring) {
   scoped_ptr<TalkMediatorImpl> talk1(new TalkMediatorImpl(mock));
 
   // Login checks states for initialization.
-  ASSERT_TRUE(talk1->Login() == false);
-  ASSERT_TRUE(mock->login_calls == 0);
+  EXPECT_FALSE(talk1->Login());
+  EXPECT_EQ(0, mock->login_calls);
 
-  ASSERT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
-                                  "fake_service") == true);
-  ASSERT_TRUE(talk1->Login() == true);
-  ASSERT_TRUE(mock->login_calls == 1);
+  EXPECT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
+                                  "fake_service"));
+  EXPECT_TRUE(talk1->Login());
+  EXPECT_EQ(1, mock->login_calls);
 
   // Successive calls to login will fail.  One needs to create a new talk
   // mediator object.
-  ASSERT_TRUE(talk1->Login() == false);
-  ASSERT_TRUE(mock->login_calls == 1);
+  EXPECT_FALSE(talk1->Login());
+  EXPECT_EQ(1, mock->login_calls);
 
-  ASSERT_TRUE(talk1->Logout() == true);
-  ASSERT_TRUE(mock->logout_calls == 1);
+  EXPECT_TRUE(talk1->Logout());
+  EXPECT_EQ(1, mock->logout_calls);
 
   // Successive logout calls do nothing.
-  ASSERT_TRUE(talk1->Logout() == false);
-  ASSERT_TRUE(mock->logout_calls == 1);
+  EXPECT_FALSE(talk1->Logout());
+  EXPECT_EQ(1, mock->logout_calls);
 }
 
 TEST_F(TalkMediatorImplTest, SendNotification) {
@@ -115,33 +122,33 @@ TEST_F(TalkMediatorImplTest, SendNotification) {
 
   // Failure due to not being logged in.
   OutgoingNotificationData data;
-  ASSERT_TRUE(talk1->SendNotification(data) == false);
-  ASSERT_TRUE(mock->send_calls == 0);
+  EXPECT_FALSE(talk1->SendNotification(data));
+  EXPECT_EQ(0, mock->send_calls);
 
-  ASSERT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
-                                  "fake_service") == true);
-  ASSERT_TRUE(talk1->Login() == true);
+  EXPECT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
+                                  "fake_service"));
+  EXPECT_TRUE(talk1->Login());
   talk1->OnLogin();
-  ASSERT_TRUE(mock->login_calls == 1);
+  EXPECT_EQ(1, mock->login_calls);
 
   // Failure due to not being subscribed.
-  ASSERT_TRUE(talk1->SendNotification(data) == false);
-  ASSERT_TRUE(mock->send_calls == 0);
+  EXPECT_FALSE(talk1->SendNotification(data));
+  EXPECT_EQ(0, mock->send_calls);
 
   // Fake subscription
   talk1->OnSubscriptionSuccess();
-  ASSERT_TRUE(talk1->state_.subscribed == 1);
-  ASSERT_TRUE(talk1->SendNotification(data) == true);
-  ASSERT_TRUE(mock->send_calls == 1);
-  ASSERT_TRUE(talk1->SendNotification(data) == true);
-  ASSERT_TRUE(mock->send_calls == 2);
+  EXPECT_TRUE(talk1->state_.subscribed);
+  EXPECT_TRUE(talk1->SendNotification(data));
+  EXPECT_EQ(1, mock->send_calls);
+  EXPECT_TRUE(talk1->SendNotification(data));
+  EXPECT_EQ(2, mock->send_calls);
 
-  ASSERT_TRUE(talk1->Logout() == true);
-  ASSERT_TRUE(mock->logout_calls == 1);
+  EXPECT_TRUE(talk1->Logout());
+  EXPECT_EQ(1, mock->logout_calls);
 
   // Failure due to being logged out.
-  ASSERT_TRUE(talk1->SendNotification(data) == false);
-  ASSERT_TRUE(mock->send_calls == 2);
+  EXPECT_FALSE(talk1->SendNotification(data));
+  EXPECT_EQ(2, mock->send_calls);
 }
 
 TEST_F(TalkMediatorImplTest, MediatorThreadCallbacks) {
@@ -149,41 +156,44 @@ TEST_F(TalkMediatorImplTest, MediatorThreadCallbacks) {
   MockMediatorThread* mock = new MockMediatorThread();
   scoped_ptr<TalkMediatorImpl> talk1(new TalkMediatorImpl(mock));
 
-  scoped_ptr<EventListenerHookup> callback(NewEventListenerHookup(
-      talk1->channel(), this, &TalkMediatorImplTest::HandleTalkMediatorEvent));
+  MockTalkMediatorDelegate mock_delegate;
+  EXPECT_CALL(mock_delegate, OnNotificationStateChange(true));
+  EXPECT_CALL(mock_delegate, OnIncomingNotification(_));
+  EXPECT_CALL(mock_delegate, OnOutgoingNotification());
+
+  talk1->SetDelegate(&mock_delegate);
 
-  ASSERT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
-                                  "fake_service") == true);
-  ASSERT_TRUE(talk1->Login() == true);
-  ASSERT_TRUE(mock->login_calls == 1);
+  EXPECT_TRUE(talk1->SetAuthToken("chromium@gmail.com", "token",
+                                  "fake_service"));
+  EXPECT_TRUE(talk1->Login());
+  EXPECT_EQ(1, mock->login_calls);
 
   mock->ChangeState(MediatorThread::MSG_LOGGED_IN);
-  ASSERT_TRUE(last_message_ == TalkMediatorEvent::LOGIN_SUCCEEDED);
 
   // The message triggers calls to listen and subscribe.
-  ASSERT_TRUE(mock->listen_calls == 1);
-  ASSERT_TRUE(mock->subscribe_calls == 1);
-  ASSERT_TRUE(talk1->state_.subscribed == 0);
+  EXPECT_EQ(1, mock->listen_calls);
+  EXPECT_EQ(1, mock->subscribe_calls);
+  EXPECT_FALSE(talk1->state_.subscribed);
 
   mock->ChangeState(MediatorThread::MSG_SUBSCRIPTION_SUCCESS);
-  ASSERT_TRUE(last_message_ == TalkMediatorEvent::SUBSCRIPTIONS_ON);
-  ASSERT_TRUE(talk1->state_.subscribed == 1);
+  EXPECT_TRUE(talk1->state_.subscribed);
 
   // After subscription success is receieved, the talk mediator will allow
   // sending of notifications.
   OutgoingNotificationData outgoing_data;
-  ASSERT_TRUE(talk1->SendNotification(outgoing_data) == true);
-  ASSERT_TRUE(mock->send_calls == 1);
+  EXPECT_TRUE(talk1->SendNotification(outgoing_data));
+  // TODO(akalin): Fix locking issues issues and move this into
+  // MediatorThreadMock.
+  mock->ChangeState(MediatorThread::MSG_NOTIFICATION_SENT);
+  EXPECT_EQ(1, mock->send_calls);
 
   IncomingNotificationData incoming_data;
   incoming_data.service_url = "service_url";
   incoming_data.service_specific_data = "service_data";
   mock->Notify(incoming_data);
-  ASSERT_TRUE(last_message_ == TalkMediatorEvent::NOTIFICATION_RECEIVED);
 
-  // A |TALKMEDIATOR_DESTROYED| message is received during tear down.
   talk1.reset();
-  ASSERT_TRUE(last_message_ == TalkMediatorEvent::TALKMEDIATOR_DESTROYED);
+  EXPECT_EQ(1, mock->logout_calls);
 }
 
 }  // namespace notifier