[remoting] Remove HostChangeNotificationListener

The old HostChangeNotificationListener listens to host deletion events
from XMPP, which will go away with the gTalk turn down. New hosts rely
on FtlHostChangeNotificationListener for getting host deletion events
through FTL. This CL deletes the old HostChangeNotificationListener.

Bug: 983282
Change-Id: I0d9acb261d3702391f8345a228853d9c4e1af5c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700472
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677081}

remoting/host/BUILD.gn

@@ -167,8 +167,6 @@ static_library("common") {
     "heartbeat_sender.h",
     "host_attributes.cc",
     "host_attributes.h",
-    "host_change_notification_listener.cc",
-    "host_change_notification_listener.h",
     "host_config.cc",
     "host_config.h",
     "host_config_upgrader.cc",
@@ -489,7 +487,6 @@ source_set("unit_tests") {
     "gcd_state_updater_unittest.cc",
     "heartbeat_sender_unittest.cc",
     "host_attributes_unittest.cc",
-    "host_change_notification_listener_unittest.cc",
     "host_config_unittest.cc",
     "host_experiment_session_plugin_unittest.cc",
     "host_extension_session_manager_unittest.cc",

remoting/host/host_change_notification_listener.cc

-// Copyright (c) 2013 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 "remoting/host/host_change_notification_listener.h"
-
-#include "base/bind.h"
-#include "base/location.h"
-#include "base/logging.h"
-#include "base/single_thread_task_runner.h"
-#include "base/threading/thread_task_runner_handle.h"
-#include "remoting/base/constants.h"
-#include "remoting/protocol/jingle_messages.h"
-#include "third_party/libjingle_xmpp/xmllite/xmlelement.h"
-#include "third_party/libjingle_xmpp/xmpp/constants.h"
-
-using jingle_xmpp::QName;
-using jingle_xmpp::XmlElement;
-
-namespace remoting {
-
-HostChangeNotificationListener::HostChangeNotificationListener(
-    Listener* listener,
-    const std::string& host_id,
-    SignalStrategy* signal_strategy,
-    const std::string& directory_bot_jid)
-    : listener_(listener),
-      host_id_(host_id),
-      signal_strategy_(signal_strategy),
-      directory_bot_jid_(directory_bot_jid),
-      weak_factory_(this) {
-  DCHECK(signal_strategy_);
-
-  signal_strategy_->AddListener(this);
-}
-
-HostChangeNotificationListener::~HostChangeNotificationListener() {
-  signal_strategy_->RemoveListener(this);
-}
-
-void HostChangeNotificationListener::OnSignalStrategyStateChange(
-    SignalStrategy::State state) {
-}
-
-bool HostChangeNotificationListener::OnSignalStrategyIncomingStanza(
-    const jingle_xmpp::XmlElement* stanza) {
-  if (stanza->Name() != jingle_xmpp::QN_IQ || stanza->Attr(jingle_xmpp::QN_TYPE) != "set")
-    return false;
-
-  const XmlElement* host_changed_element =
-      stanza->FirstNamed(QName(kChromotingXmlNamespace, "host-changed"));
-  if (!host_changed_element)
-    return false;
-
-  const std::string& host_id =
-      host_changed_element->Attr(QName(kChromotingXmlNamespace, "hostid"));
-  const std::string& from = stanza->Attr(jingle_xmpp::QN_FROM);
-
-  std::string to_error;
-  SignalingAddress to =
-      SignalingAddress::Parse(stanza, SignalingAddress::TO, &to_error);
-
-  if (host_id == host_id_ && from == directory_bot_jid_ &&
-      to == signal_strategy_->GetLocalAddress()) {
-    const std::string& operation =
-        host_changed_element->Attr(QName(kChromotingXmlNamespace, "operation"));
-    if (operation == "delete") {
-      // OnHostDeleted() may want delete |signal_strategy_|, but SignalStrategy
-      // objects cannot be deleted from a Listener callback, so OnHostDeleted()
-      // has to be invoked later.
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE,
-          base::BindOnce(&HostChangeNotificationListener::OnHostDeleted,
-                         weak_factory_.GetWeakPtr()));
-    }
-  } else {
-    LOG(ERROR) << "Invalid host-changed message received: " << stanza->Str();
-  }
-  return true;
-}
-
-void HostChangeNotificationListener::OnHostDeleted() {
-  listener_->OnHostDeleted();
-}
-
-}  // namespace remoting

remoting/host/host_change_notification_listener.h

-// Copyright (c) 2013 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 REMOTING_HOST_HOST_CHANGE_NOTIFICATION_LISTENER_H
-#define REMOTING_HOST_HOST_CHANGE_NOTIFICATION_LISTENER_H
-
-#include <memory>
-#include <string>
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "base/memory/weak_ptr.h"
-#include "remoting/signaling/signal_strategy.h"
-
-namespace jingle_xmpp {
-class XmlElement;
-}  // namespace jingle_xmpp
-
-namespace remoting {
-
-// HostChangeNotificationListener listens for messages from the remoting bot
-// indicating that its host entry has been changed in the directory.
-// If a message is received indicating that the host was deleted, it uses the
-// OnHostDeleted callback to shut down the host.
-class HostChangeNotificationListener : public SignalStrategy::Listener {
- public:
-  class Listener {
-   protected:
-    virtual ~Listener() {}
-    // Invoked when a notification that the host was deleted is received.
-   public:
-    virtual void OnHostDeleted() = 0;
-  };
-
-  // Both listener and signal_strategy are expected to outlive this object.
-  HostChangeNotificationListener(Listener* listener,
-                                 const std::string& host_id,
-                                 SignalStrategy* signal_strategy,
-                                 const std::string& directory_bot_jid);
-  ~HostChangeNotificationListener() override;
-
-  // SignalStrategy::Listener interface.
-  void OnSignalStrategyStateChange(SignalStrategy::State state) override;
-  bool OnSignalStrategyIncomingStanza(const jingle_xmpp::XmlElement* stanza) override;
-
- private:
-  void OnHostDeleted();
-
-  Listener* listener_;
-  std::string host_id_;
-  SignalStrategy* signal_strategy_;
-  std::string directory_bot_jid_;
-  base::WeakPtrFactory<HostChangeNotificationListener> weak_factory_;
-  DISALLOW_COPY_AND_ASSIGN(HostChangeNotificationListener);
-};
-
-}  // namespace remoting
-
-#endif  // REMOTING_HOST_HOST_CHANGE_NOTIFICATION_LISTENER_H

remoting/host/host_change_notification_listener_unittest.cc

-// Copyright (c) 2013 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 "remoting/host/host_change_notification_listener.h"
-
-#include <set>
-
-#include "base/bind.h"
-#include "base/memory/ref_counted.h"
-#include "base/run_loop.h"
-#include "base/single_thread_task_runner.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/test/scoped_task_environment.h"
-#include "remoting/base/constants.h"
-#include "remoting/signaling/mock_signal_strategy.h"
-#include "remoting/signaling/signaling_address.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/libjingle_xmpp/xmllite/xmlelement.h"
-#include "third_party/libjingle_xmpp/xmpp/constants.h"
-
-using jingle_xmpp::QName;
-using jingle_xmpp::XmlElement;
-
-using testing::NotNull;
-using testing::Return;
-
-namespace remoting {
-
-namespace {
-const char kHostId[] = "0";
-const char kTestJid[] = "user@gmail.com/chromoting123";
-const char kTestBotJid[] = "remotingunittest@bot.talk.google.com";
-}  // namespace
-
-ACTION_P(AddListener, list) {
-  list->insert(arg0);
-}
-
-ACTION_P(RemoveListener, list) {
-  EXPECT_TRUE(list->find(arg0) != list->end());
-  list->erase(arg0);
-}
-
-
-class HostChangeNotificationListenerTest : public testing::Test {
- protected:
-  HostChangeNotificationListenerTest()
-      : signal_strategy_(SignalingAddress(kTestJid)) {}
-  class MockListener : public HostChangeNotificationListener::Listener {
-   public:
-    MOCK_METHOD0(OnHostDeleted, void());
-  };
-
-  void SetUp() override {
-    EXPECT_CALL(signal_strategy_, AddListener(NotNull()))
-        .WillRepeatedly(AddListener(&signal_strategy_listeners_));
-    EXPECT_CALL(signal_strategy_, RemoveListener(NotNull()))
-        .WillRepeatedly(RemoveListener(&signal_strategy_listeners_));
-
-    host_change_notification_listener_.reset(new HostChangeNotificationListener(
-        &mock_listener_, kHostId, &signal_strategy_, kTestBotJid));
-  }
-
-  void TearDown() override {
-    host_change_notification_listener_.reset();
-    EXPECT_TRUE(signal_strategy_listeners_.empty());
-  }
-
-  std::unique_ptr<XmlElement> GetNotificationStanza(std::string operation,
-                                                    std::string hostId,
-                                                    std::string botJid) {
-    std::unique_ptr<XmlElement> stanza(new XmlElement(jingle_xmpp::QN_IQ));
-    stanza->AddAttr(QName(std::string(), "type"), "set");
-    XmlElement* host_changed =
-        new XmlElement(QName(kChromotingXmlNamespace, "host-changed"));
-    host_changed->AddAttr(QName(kChromotingXmlNamespace, "operation"),
-                          operation);
-    host_changed->AddAttr(QName(kChromotingXmlNamespace, "hostid"), hostId);
-    stanza->AddElement(host_changed);
-    stanza->AddAttr(jingle_xmpp::QN_FROM, botJid);
-    stanza->AddAttr(jingle_xmpp::QN_TO, kTestJid);
-    return stanza;
-  }
-
-  MockListener mock_listener_;
-  MockSignalStrategy signal_strategy_;
-  std::set<SignalStrategy::Listener*> signal_strategy_listeners_;
-  std::unique_ptr<HostChangeNotificationListener>
-      host_change_notification_listener_;
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
-};
-
-TEST_F(HostChangeNotificationListenerTest, ReceiveValidNotification) {
-  EXPECT_CALL(mock_listener_, OnHostDeleted())
-      .WillOnce(Return());
-  std::unique_ptr<XmlElement> stanza =
-      GetNotificationStanza("delete", kHostId, kTestBotJid);
-  host_change_notification_listener_->OnSignalStrategyIncomingStanza(
-      stanza.get());
-  scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
-  base::RunLoop().Run();
-}
-
-TEST_F(HostChangeNotificationListenerTest, ReceiveNotificationBeforeDelete) {
-  EXPECT_CALL(mock_listener_, OnHostDeleted())
-      .Times(0);
-  std::unique_ptr<XmlElement> stanza =
-      GetNotificationStanza("delete", kHostId, kTestBotJid);
-  host_change_notification_listener_->OnSignalStrategyIncomingStanza(
-      stanza.get());
-  host_change_notification_listener_.reset();
-  scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
-  base::RunLoop().Run();
-}
-
-
-TEST_F(HostChangeNotificationListenerTest, ReceiveInvalidHostIdNotification) {
-  EXPECT_CALL(mock_listener_, OnHostDeleted())
-      .Times(0);
-  std::unique_ptr<XmlElement> stanza =
-      GetNotificationStanza("delete", "1", kTestBotJid);
-  host_change_notification_listener_->OnSignalStrategyIncomingStanza(
-      stanza.get());
-  scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
-  base::RunLoop().Run();
-}
-
-TEST_F(HostChangeNotificationListenerTest, ReceiveInvalidBotJidNotification) {
-  EXPECT_CALL(mock_listener_, OnHostDeleted())
-      .Times(0);
-  std::unique_ptr<XmlElement> stanza = GetNotificationStanza(
-      "delete", kHostId, "notremotingbot@bot.talk.google.com");
-  host_change_notification_listener_->OnSignalStrategyIncomingStanza(
-      stanza.get());
-  scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
-  base::RunLoop().Run();
-}
-
-TEST_F(HostChangeNotificationListenerTest, ReceiveNonDeleteNotification) {
-  EXPECT_CALL(mock_listener_, OnHostDeleted())
-      .Times(0);
-  std::unique_ptr<XmlElement> stanza =
-      GetNotificationStanza("update", kHostId, kTestBotJid);
-  host_change_notification_listener_->OnSignalStrategyIncomingStanza(
-      stanza.get());
-  scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
-  base::RunLoop().Run();
-}
-
-}  // namespace remoting

remoting/host/remoting_me2me_host.cc

@@ -64,7 +64,6 @@
 #include "remoting/host/gcd_rest_client.h"
 #include "remoting/host/gcd_state_updater.h"
 #include "remoting/host/heartbeat_sender.h"
-#include "remoting/host/host_change_notification_listener.h"
 #include "remoting/host/host_config.h"
 #include "remoting/host/host_event_logger.h"
 #include "remoting/host/host_exit_codes.h"
@@ -212,7 +211,6 @@ namespace remoting {
 
 class HostProcess : public ConfigWatcher::Delegate,
                     public FtlHostChangeNotificationListener::Listener,
-                    public HostChangeNotificationListener::Listener,
                     public IPC::Listener,
                     public base::RefCountedThreadSafe<HostProcess> {
  public:
@@ -231,7 +229,7 @@ class HostProcess : public ConfigWatcher::Delegate,
   bool OnMessageReceived(const IPC::Message& message) override;
   void OnChannelError() override;
 
-  // HostChangeNotificationListener::Listener overrides.
+  // FtlHostChangeNotificationListener::Listener overrides.
   void OnHostDeleted() override;
 
   // Handler of the ChromotingDaemonNetworkMsg_InitializePairingRegistry IPC
@@ -439,8 +437,6 @@ class HostProcess : public ConfigWatcher::Delegate,
   std::unique_ptr<PushNotificationSubscriber> gcd_subscriber_;
 #endif  // defined(USE_GCD)
 
-  std::unique_ptr<HostChangeNotificationListener>
-      host_change_notification_listener_;
   std::unique_ptr<HostStatusLogger> host_status_logger_;
   std::unique_ptr<HostEventLogger> host_event_logger_;
   std::unique_ptr<HostPowerSaveBlocker> power_save_blocker_;
@@ -1505,12 +1501,6 @@ void HostProcess::InitializeSignaling() {
           this, muxing_signal_strategy->ftl_signal_strategy());
   signal_strategy_ = std::move(muxing_signal_strategy);
 
-  // XMPP based HostChangeNotificationListener is still needed by double
-  // signaling hosts so that they can handle host deletion triggered by an old
-  // client talking to the legacy directory backend.
-  host_change_notification_listener_.reset(new HostChangeNotificationListener(
-      this, host_id_, unowned_xmpp_signal_strategy, directory_bot_jid_));
-
 #if defined(USE_GCD)
   // Create objects to manage GCD state.
   ServiceUrls* service_urls = ServiceUrls::GetInstance();
@@ -1697,7 +1687,6 @@ void HostProcess::GoOffline(const std::string& host_offline_reason) {
   host_status_logger_.reset();
   power_save_blocker_.reset();
   ftl_host_change_notification_listener_.reset();
-  host_change_notification_listener_.reset();
 
   // Before shutting down HostSignalingManager, send the |host_offline_reason|
   // if possible (i.e. if we have the config).