[Sync] Replace TalkMediator*/MediatorThread* with PushClient

Streamline methods of PushClient and its Observer subclass.

Remove sync/protocol/service_constants.h and consolidate use of each
constant in one place.  Remove duplicate constant in cloud print code.

BUG=76764
TEST=
TBR=estade@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10398051

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

6 files changed, 103 insertions, 105 deletions

diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc
index ed16e1c..56ee668e 100644
--- a/sync/notifier/invalidation_notifier.cc
+++ b/sync/notifier/invalidation_notifier.cc
@@ -10,7 +10,6 @@
 #include "jingle/notifier/base/notifier_options_util.h"
 #include "net/url_request/url_request_context.h"
 #include "sync/notifier/sync_notifier_observer.h"
-#include "sync/protocol/service_constants.h"
 #include "sync/syncable/model_type_payload_map.h"
 #include "talk/xmpp/jid.h"
 #include "talk/xmpp/xmppclientsettings.h"
@@ -69,8 +68,7 @@ void InvalidationNotifier::UpdateCredentials(
   CHECK(!invalidation_client_id_.empty());
   DVLOG(1) << "Updating credentials for " << email;
   buzz::XmppClientSettings xmpp_client_settings =
-      notifier::MakeXmppClientSettings(notifier_options_,
-                                       email, token, SYNC_SERVICE_NAME);
+      notifier::MakeXmppClientSettings(notifier_options_, email, token);
   if (state_ >= CONNECTING) {
     login_->UpdateXmppSettings(xmpp_client_settings);
   } else {
diff --git a/sync/notifier/invalidation_notifier.h b/sync/notifier/invalidation_notifier.h
index 385fbfc..bd30eed5 100644
--- a/sync/notifier/invalidation_notifier.h
+++ b/sync/notifier/invalidation_notifier.h
@@ -110,6 +110,7 @@ class InvalidationNotifier
   std::string invalidation_state_;
 
   // The XMPP connection manager.
+  // TODO(akalin): Use PushClient instead.
   scoped_ptr<notifier::Login> login_;
 
   // The invalidation client.
diff --git a/sync/notifier/p2p_notifier.cc b/sync/notifier/p2p_notifier.cc
index 49462ce..3e9def1 100644
--- a/sync/notifier/p2p_notifier.cc
+++ b/sync/notifier/p2p_notifier.cc
@@ -12,7 +12,6 @@
 #include "base/message_loop_proxy.h"
 #include "base/values.h"
 #include "sync/notifier/sync_notifier_observer.h"
-#include "sync/protocol/service_constants.h"
 #include "sync/syncable/model_type_payload_map.h"
 
 namespace sync_notifier {
@@ -141,21 +140,21 @@ bool P2PNotificationData::ResetFromString(const std::string& str) {
   return true;
 }
 
-P2PNotifier::P2PNotifier(notifier::TalkMediator* talk_mediator,
+P2PNotifier::P2PNotifier(const notifier::NotifierOptions& notifier_options,
                          P2PNotificationTarget send_notification_target)
-    : talk_mediator_(talk_mediator),
+    : push_client_(notifier_options),
       logged_in_(false),
       notifications_enabled_(false),
       send_notification_target_(send_notification_target),
-      parent_message_loop_proxy_(
-          base::MessageLoopProxy::current()) {
+      parent_message_loop_proxy_(base::MessageLoopProxy::current()) {
   DCHECK(send_notification_target_ == NOTIFY_OTHERS ||
          send_notification_target_ == NOTIFY_ALL);
-  talk_mediator_->SetDelegate(this);
+  push_client_.AddObserver(this);
 }
 
 P2PNotifier::~P2PNotifier() {
   DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
+  push_client_.RemoveObserver(this);
 }
 
 void P2PNotifier::AddObserver(SyncNotifierObserver* observer) {
@@ -163,18 +162,9 @@ void P2PNotifier::AddObserver(SyncNotifierObserver* observer) {
   observer_list_.AddObserver(observer);
 }
 
-// Note: Since we need to shutdown TalkMediator on the method_thread, we are
-// calling Logout on TalkMediator when the last observer is removed.
-// Users will need to call UpdateCredentials again to use the same object.
-// TODO(akalin): Think of a better solution to fix this.
 void P2PNotifier::RemoveObserver(SyncNotifierObserver* observer) {
   DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
   observer_list_.RemoveObserver(observer);
-
-  // Logout after the last observer is removed.
-  if (observer_list_.size() == 0) {
-   talk_mediator_->Logout();
-  }
 }
 
 void P2PNotifier::SetUniqueId(const std::string& unique_id) {
@@ -189,25 +179,19 @@ void P2PNotifier::SetState(const std::string& state) {
 void P2PNotifier::UpdateCredentials(
     const std::string& email, const std::string& token) {
   DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
+  notifier::Subscription subscription;
+  subscription.channel = kSyncP2PNotificationChannel;
+  // There may be some subtle issues around case sensitivity of the
+  // from field, but it doesn't matter too much since this is only
+  // used in p2p mode (which is only used in testing).
+  subscription.from = email;
+  push_client_.UpdateSubscriptions(
+      notifier::SubscriptionList(1, subscription));
+
   // If already logged in, the new credentials will take effect on the
   // next reconnection.
-  talk_mediator_->SetAuthToken(email, token, SYNC_SERVICE_NAME);
-  if (!logged_in_) {
-    if (!talk_mediator_->Login()) {
-      LOG(DFATAL) << "Could not login for " << email;
-      return;
-    }
-
-    notifier::Subscription subscription;
-    subscription.channel = kSyncP2PNotificationChannel;
-    // There may be some subtle issues around case sensitivity of the
-    // from field, but it doesn't matter too much since this is only
-    // used in p2p mode (which is only used in testing).
-    subscription.from = email;
-    talk_mediator_->AddSubscription(subscription);
-
-    logged_in_ = true;
-  }
+  push_client_.UpdateCredentials(email, token);
+  logged_in_ = true;
 }
 
 void P2PNotifier::UpdateEnabledTypes(
@@ -281,10 +265,20 @@ void P2PNotifier::OnIncomingNotification(
                     OnIncomingNotification(type_payloads, REMOTE_NOTIFICATION));
 }
 
-void P2PNotifier::OnOutgoingNotification() {}
+void P2PNotifier::SimulateConnectForTest(
+    base::WeakPtr<buzz::XmppTaskParentInterface> base_task) {
+  DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
+  push_client_.SimulateConnectAndSubscribeForTest(base_task);
+}
+
+void P2PNotifier::ReflectSentNotificationsForTest() {
+  DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
+  push_client_.ReflectSentNotificationsForTest();
+}
 
 void P2PNotifier::SendNotificationDataForTest(
     const P2PNotificationData& notification_data) {
+  DCHECK(parent_message_loop_proxy_->BelongsToCurrentThread());
   SendNotificationData(notification_data);
 }
 
@@ -295,7 +289,7 @@ void P2PNotifier::SendNotificationData(
   notification.channel = kSyncP2PNotificationChannel;
   notification.data = notification_data.ToString();
   DVLOG(1) << "Sending XMPP notification: " << notification.ToString();
-  talk_mediator_->SendNotification(notification);
+  push_client_.SendNotification(notification);
 }
 
 }  // namespace sync_notifier
diff --git a/sync/notifier/p2p_notifier.h b/sync/notifier/p2p_notifier.h
index 093d023..24b0ab3 100644
--- a/sync/notifier/p2p_notifier.h
+++ b/sync/notifier/p2p_notifier.h
@@ -13,8 +13,10 @@
 #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/observer_list.h"
-#include "jingle/notifier/listener/talk_mediator.h"
+#include "jingle/notifier/base/notifier_options.h"
+#include "jingle/notifier/listener/push_client.h"
 #include "sync/notifier/sync_notifier.h"
 #include "sync/syncable/model_type.h"
 
@@ -22,6 +24,9 @@ namespace base {
 class MessageLoopProxy;
 }
 
+namespace buzz {
+class XmppTaskParentInterface;
+}  // namespace buzz
 
 namespace sync_notifier {
 
@@ -81,17 +86,14 @@ class P2PNotificationData {
 
 class P2PNotifier
     : public SyncNotifier,
-      public notifier::TalkMediator::Delegate {
+      public notifier::PushClient::Observer {
  public:
-  // Takes ownership of |talk_mediator|, but it is guaranteed that
-  // |talk_mediator| is destroyed only when this object is destroyed.
-  //
   // The |send_notification_target| parameter was added to allow us to send
   // self-notifications in some cases, but not others.  The value should be
   // either NOTIFY_ALL to send notifications to all clients, or NOTIFY_OTHERS
-  // to send notificaitons to all clients except for the one that triggered the
+  // to send notifications to all clients except for the one that triggered the
   // notification.  See crbug.com/97780.
-  P2PNotifier(notifier::TalkMediator* talk_mediator,
+  P2PNotifier(const notifier::NotifierOptions& notifier_options,
               P2PNotificationTarget send_notification_target);
 
   virtual ~P2PNotifier();
@@ -108,13 +110,20 @@ class P2PNotifier
   virtual void SendNotification(
       syncable::ModelTypeSet changed_types) OVERRIDE;
 
-  // TalkMediator::Delegate implementation.
+  // PushClient::Delegate implementation.
   virtual void OnNotificationStateChange(bool notifications_enabled) OVERRIDE;
   virtual void OnIncomingNotification(
       const notifier::Notification& notification) OVERRIDE;
-  virtual void OnOutgoingNotification() OVERRIDE;
 
   // For testing.
+
+  void SimulateConnectForTest(
+      base::WeakPtr<buzz::XmppTaskParentInterface> base_task);
+
+  // Any notifications sent after this is called will be reflected,
+  // i.e. will be treated as an incoming notification also.
+  void ReflectSentNotificationsForTest();
+
   void SendNotificationDataForTest(
       const P2PNotificationData& notification_data);
 
@@ -123,13 +132,13 @@ class P2PNotifier
 
   ObserverList<SyncNotifierObserver> observer_list_;
 
-  // The actual notification listener.
-  scoped_ptr<notifier::TalkMediator> talk_mediator_;
+  // The XMPP push client.
+  notifier::PushClient push_client_;
   // Our unique ID.
   std::string unique_id_;
-  // Whether we called Login() on |talk_mediator_| yet.
+  // Whether we have called UpdateCredentials() yet.
   bool logged_in_;
-  // Whether |talk_mediator_| has notified us that notifications are
+  // Whether |push_client_| has notified us that notifications are
   // enabled.
   bool notifications_enabled_;
   // Which set of clients should be sent notifications.
diff --git a/sync/notifier/p2p_notifier_unittest.cc b/sync/notifier/p2p_notifier_unittest.cc
index a9f8db9..ea85433 100644
--- a/sync/notifier/p2p_notifier_unittest.cc
+++ b/sync/notifier/p2p_notifier_unittest.cc
@@ -9,6 +9,10 @@
 #include "base/compiler_specific.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/push_client.h"
+#include "net/url_request/url_request_test_util.h"
 #include "sync/notifier/mock_sync_notifier_observer.h"
 #include "sync/syncable/model_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -21,57 +25,25 @@ using ::testing::_;
 using ::testing::Mock;
 using ::testing::StrictMock;
 
-class FakeTalkMediator : public notifier::TalkMediator {
- public:
-  FakeTalkMediator() : delegate_(NULL) {}
-  virtual ~FakeTalkMediator() {}
-
-  // notifier::TalkMediator implementation.
-  virtual void SetDelegate(Delegate* delegate) OVERRIDE {
-    delegate_ = delegate;
-  }
-  virtual void SetAuthToken(const std::string& email,
-                            const std::string& token,
-                            const std::string& token_service) OVERRIDE {}
-  virtual bool Login() OVERRIDE {
-    if (delegate_) {
-      delegate_->OnNotificationStateChange(true /* notifications_enabled */);
-    }
-    return true;
-  }
-  virtual bool Logout() OVERRIDE {
-    if (delegate_) {
-      delegate_->OnNotificationStateChange(false /* notifiations_enabled */);
-    }
-    return true;
-  }
-  virtual void SendNotification(const notifier::Notification& data) OVERRIDE {
-    if (delegate_) {
-      delegate_->OnOutgoingNotification();
-      delegate_->OnIncomingNotification(data);
-    }
-  }
-  virtual void AddSubscription(
-      const notifier::Subscription& subscription) OVERRIDE {}
-
- private:
-  Delegate* delegate_;
-};
-
 class P2PNotifierTest : public testing::Test {
  protected:
-  P2PNotifierTest() : talk_mediator_(NULL) {}
+  P2PNotifierTest() {
+    notifier_options_.request_context_getter =
+        new TestURLRequestContextGetter(message_loop_.message_loop_proxy());
+  }
+
+  virtual ~P2PNotifierTest() {}
 
-  virtual void SetUp() {
-    talk_mediator_ = new FakeTalkMediator();
-    p2p_notifier_.reset(new P2PNotifier(talk_mediator_, NOTIFY_OTHERS));
+  virtual void SetUp() OVERRIDE {
+    p2p_notifier_.reset(new P2PNotifier(notifier_options_, NOTIFY_OTHERS));
     p2p_notifier_->AddObserver(&mock_observer_);
   }
 
-  virtual void TearDown() {
+  virtual void TearDown() OVERRIDE {
+    message_loop_.RunAllPending();
     p2p_notifier_->RemoveObserver(&mock_observer_);
     p2p_notifier_.reset();
-    talk_mediator_ = NULL;
+    message_loop_.RunAllPending();
   }
 
   syncable::ModelTypePayloadMap MakePayloadMap(
@@ -79,11 +51,12 @@ class P2PNotifierTest : public testing::Test {
     return syncable::ModelTypePayloadMapFromEnumSet(types, "");
   }
 
-  MessageLoop message_loop_;
-  // Owned by |p2p_notifier_|.
-  notifier::TalkMediator* talk_mediator_;
+  // The sockets created by the XMPP code expect an IO loop.
+  MessageLoopForIO message_loop_;
+  notifier::NotifierOptions notifier_options_;
   scoped_ptr<P2PNotifier> p2p_notifier_;
   StrictMock<MockSyncNotifierObserver> mock_observer_;
+  notifier::FakeBaseTask fake_base_task_;
 };
 
 TEST_F(P2PNotifierTest, P2PNotificationTarget) {
@@ -166,9 +139,14 @@ TEST_F(P2PNotifierTest, NotificationsBasic) {
               OnIncomingNotification(MakePayloadMap(enabled_types),
                                      REMOTE_NOTIFICATION));
 
+  p2p_notifier_->ReflectSentNotificationsForTest();
+
   p2p_notifier_->SetUniqueId("sender");
   p2p_notifier_->UpdateCredentials("foo@bar.com", "fake_token");
   p2p_notifier_->UpdateEnabledTypes(enabled_types);
+
+  p2p_notifier_->SimulateConnectForTest(fake_base_task_.AsWeakPtr());
+
   // Sent with target NOTIFY_OTHERS so should not be propagated to
   // |mock_observer_|.
   {
@@ -193,14 +171,22 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
               OnIncomingNotification(MakePayloadMap(enabled_types),
                                      REMOTE_NOTIFICATION));
 
+  p2p_notifier_->ReflectSentNotificationsForTest();
+
   p2p_notifier_->SetUniqueId("sender");
   p2p_notifier_->UpdateCredentials("foo@bar.com", "fake_token");
   p2p_notifier_->UpdateEnabledTypes(enabled_types);
 
+  p2p_notifier_->SimulateConnectForTest(fake_base_task_.AsWeakPtr());
+
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   p2p_notifier_->SendNotificationDataForTest(P2PNotificationData());
 
+  message_loop_.RunAllPending();
+
   // Should be propagated.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map,
@@ -208,20 +194,28 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender", NOTIFY_SELF, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender2", NOTIFY_SELF, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender", NOTIFY_SELF, syncable::ModelTypeSet()));
 
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender", NOTIFY_OTHERS, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be propagated.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map,
@@ -229,11 +223,15 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender2", NOTIFY_OTHERS, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender2", NOTIFY_OTHERS, syncable::ModelTypeSet()));
 
+  message_loop_.RunAllPending();
+
   // Should be propagated.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map,
@@ -241,6 +239,8 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender", NOTIFY_ALL, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be propagated.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   EXPECT_CALL(mock_observer_, OnIncomingNotification(changed_payload_map,
@@ -248,10 +248,14 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender2", NOTIFY_ALL, changed_types));
 
+  message_loop_.RunAllPending();
+
   // Should be dropped.
   Mock::VerifyAndClearExpectations(&mock_observer_);
   p2p_notifier_->SendNotificationDataForTest(
       P2PNotificationData("sender2", NOTIFY_ALL, syncable::ModelTypeSet()));
+
+  message_loop_.RunAllPending();
 }
 
 }  // namespace
diff --git a/sync/notifier/sync_notifier_factory.cc b/sync/notifier/sync_notifier_factory.cc
index eaaf2ef..1062106 100644
--- a/sync/notifier/sync_notifier_factory.cc
+++ b/sync/notifier/sync_notifier_factory.cc
@@ -7,8 +7,6 @@
 #include <string>
 
 #include "base/logging.h"
-#include "jingle/notifier/listener/mediator_thread_impl.h"
-#include "jingle/notifier/listener/talk_mediator_impl.h"
 #include "sync/notifier/non_blocking_invalidation_notifier.h"
 #include "sync/notifier/p2p_notifier.h"
 #include "sync/notifier/sync_notifier.h"
@@ -23,17 +21,11 @@ SyncNotifier* CreateDefaultSyncNotifier(
         invalidation_version_tracker,
     const std::string& client_info) {
   if (notifier_options.notification_method == notifier::NOTIFICATION_P2P) {
-    notifier::TalkMediator* const talk_mediator =
-        new notifier::TalkMediatorImpl(
-            new notifier::MediatorThreadImpl(notifier_options),
-            notifier_options);
     // TODO(rlarocque): Ideally, the notification target would be
     // NOTIFY_OTHERS.  There's no good reason to notify ourselves of our own
     // commits.  We self-notify for now only because the integration tests rely
     // on this behaviour.  See crbug.com/97780.
-    //
-    // Takes ownership of |talk_mediator|.
-    return new P2PNotifier(talk_mediator, NOTIFY_ALL);
+    return new P2PNotifier(notifier_options, NOTIFY_ALL);
   }
 
   return new NonBlockingInvalidationNotifier(