DevTools: Make port forwarding part of DevToolsAndroidBridge.

Rather than being a subscriber to DTAB port forwarding becomes part of it so that DTAB could stop discovering new devices when chrome://inspect is closed.

BUG=387067

Review URL: https://codereview.chromium.org/500373004

Cr-Commit-Position: refs/heads/master@{#293143}

chrome/browser/devtools/device/devtools_android_bridge.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/devtools/browser_list_tabcontents_provider.h"
 #include "chrome/browser/devtools/device/adb/adb_device_info_query.h"
 #include "chrome/browser/devtools/device/adb/adb_device_provider.h"
+#include "chrome/browser/devtools/device/port_forwarding_controller.h"
 #include "chrome/browser/devtools/device/self_device_provider.h"
 #include "chrome/browser/devtools/device/usb/usb_device_provider.h"
 #include "chrome/browser/devtools/devtools_protocol.h"
@@ -733,7 +734,8 @@ DevToolsAndroidBridge::RemoteDevice::~RemoteDevice() {
 DevToolsAndroidBridge::DevToolsAndroidBridge(Profile* profile)
     : profile_(profile),
       device_manager_(AndroidDeviceManager::Create()),
-      task_scheduler_(base::Bind(&DevToolsAndroidBridge::ScheduleTaskDefault)) {
+      task_scheduler_(base::Bind(&DevToolsAndroidBridge::ScheduleTaskDefault)),
+      port_forwarding_controller_(new PortForwardingController(profile)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(prefs::kDevToolsDiscoverUsbDevicesEnabled,
@@ -745,8 +747,9 @@ DevToolsAndroidBridge::DevToolsAndroidBridge(Profile* profile)
 void DevToolsAndroidBridge::AddDeviceListListener(
     DeviceListListener* listener) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  bool polling_was_off = !NeedsDeviceListPolling();
   device_list_listeners_.push_back(listener);
-  if (device_list_listeners_.size() == 1)
+  if (polling_was_off)
     StartDeviceListPolling();
 }
 
@@ -757,7 +760,7 @@ void DevToolsAndroidBridge::RemoveDeviceListListener(
       device_list_listeners_.begin(), device_list_listeners_.end(), listener);
   DCHECK(it != device_list_listeners_.end());
   device_list_listeners_.erase(it);
-  if (device_list_listeners_.empty())
+  if (!NeedsDeviceListPolling())
     StopDeviceListPolling();
 }
 
@@ -779,6 +782,26 @@ void DevToolsAndroidBridge::RemoveDeviceCountListener(
     StopDeviceCountPolling();
 }
 
+void DevToolsAndroidBridge::AddPortForwardingListener(
+    PortForwardingListener* listener) {
+  bool polling_was_off = !NeedsDeviceListPolling();
+  port_forwarding_listeners_.push_back(listener);
+  if (polling_was_off)
+    StartDeviceListPolling();
+}
+
+void DevToolsAndroidBridge::RemovePortForwardingListener(
+    PortForwardingListener* listener) {
+  PortForwardingListeners::iterator it = std::find(
+      port_forwarding_listeners_.begin(),
+      port_forwarding_listeners_.end(),
+      listener);
+  DCHECK(it != port_forwarding_listeners_.end());
+  port_forwarding_listeners_.erase(it);
+  if (!NeedsDeviceListPolling())
+    StopDeviceListPolling();
+}
+
 // static
 bool DevToolsAndroidBridge::HasDevToolsWindow(const std::string& agent_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -789,6 +812,7 @@ DevToolsAndroidBridge::~DevToolsAndroidBridge() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(device_list_listeners_.empty());
   DCHECK(device_count_listeners_.empty());
+  DCHECK(port_forwarding_listeners_.empty());
 }
 
 void DevToolsAndroidBridge::StartDeviceListPolling() {
@@ -802,11 +826,15 @@ void DevToolsAndroidBridge::StopDeviceListPolling() {
   devices_.clear();
 }
 
+bool DevToolsAndroidBridge::NeedsDeviceListPolling() {
+  return !device_list_listeners_.empty() || !port_forwarding_listeners_.empty();
+}
+
 void DevToolsAndroidBridge::RequestDeviceList(
     const base::Callback<void(const RemoteDevices&)>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (device_list_listeners_.empty() ||
+  if (!NeedsDeviceListPolling() ||
       !callback.Equals(device_list_callback_.callback()))
     return;
 
@@ -820,7 +848,15 @@ void DevToolsAndroidBridge::ReceivedDeviceList(const RemoteDevices& devices) {
   for (DeviceListListeners::iterator it = copy.begin(); it != copy.end(); ++it)
     (*it)->DeviceListChanged(devices);
 
-  if (device_list_listeners_.empty())
+  DevicesStatus status =
+      port_forwarding_controller_->DeviceListChanged(devices);
+  PortForwardingListeners forwarding_listeners(port_forwarding_listeners_);
+  for (PortForwardingListeners::iterator it = forwarding_listeners.begin();
+       it != forwarding_listeners.end(); ++it) {
+    (*it)->PortStatusChanged(status);
+  }
+
+  if (!NeedsDeviceListPolling())
     return;
 
   devices_ = devices;
@@ -896,7 +932,7 @@ void DevToolsAndroidBridge::CreateDeviceProviders() {
     device_providers.push_back(new UsbDeviceProvider(profile_));
   }
   device_manager_->SetDeviceProviders(device_providers);
-  if (!device_list_listeners_.empty()) {
+  if (NeedsDeviceListPolling()) {
     StopDeviceListPolling();
     StartDeviceListPolling();
   }

chrome/browser/devtools/device/devtools_android_bridge.h

@@ -33,6 +33,7 @@ class BrowserContext;
 }
 
 class DevToolsTargetImpl;
+class PortForwardingController;
 class Profile;
 
 class DevToolsAndroidBridge
@@ -208,6 +209,23 @@ class DevToolsAndroidBridge
   void AddDeviceCountListener(DeviceCountListener* listener);
   void RemoveDeviceCountListener(DeviceCountListener* listener);
 
+  typedef int PortStatus;
+  typedef std::map<int, PortStatus> PortStatusMap;
+  typedef std::map<std::string, PortStatusMap> DevicesStatus;
+
+  class PortForwardingListener {
+   public:
+    typedef DevToolsAndroidBridge::PortStatusMap PortStatusMap;
+    typedef DevToolsAndroidBridge::DevicesStatus DevicesStatus;
+
+    virtual void PortStatusChanged(const DevicesStatus&) = 0;
+   protected:
+    virtual ~PortForwardingListener() {}
+  };
+
+  void AddPortForwardingListener(PortForwardingListener* listener);
+  void RemovePortForwardingListener(PortForwardingListener* listener);
+
   void set_device_providers_for_test(
       const AndroidDeviceManager::DeviceProviders& device_providers) {
     device_manager_->SetDeviceProviders(device_providers);
@@ -229,6 +247,7 @@ class DevToolsAndroidBridge
 
   void StartDeviceListPolling();
   void StopDeviceListPolling();
+  bool NeedsDeviceListPolling();
   void RequestDeviceList(
       const base::Callback<void(const RemoteDevices&)>& callback);
   void ReceivedDeviceList(const RemoteDevices& devices);
@@ -254,6 +273,10 @@ class DevToolsAndroidBridge
   base::CancelableCallback<void(int)> device_count_callback_;
   base::Callback<void(const base::Closure&)> task_scheduler_;
 
+  typedef std::vector<PortForwardingListener*> PortForwardingListeners;
+  PortForwardingListeners port_forwarding_listeners_;
+  scoped_ptr<PortForwardingController> port_forwarding_controller_;
+
   PrefChangeRegistrar pref_change_registrar_;
   DISALLOW_COPY_AND_ASSIGN(DevToolsAndroidBridge);
 };

chrome/browser/devtools/device/port_forwarding_browsertest.cc

@@ -7,7 +7,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/devtools/browser_list_tabcontents_provider.h"
-#include "chrome/browser/devtools/device/port_forwarding_controller.h"
+#include "chrome/browser/devtools/device/devtools_android_bridge.h"
 #include "chrome/browser/devtools/device/self_device_provider.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -35,18 +35,18 @@ class PortForwardingTest: public InProcessBrowserTest {
   }
 
  protected:
-  class Listener : public PortForwardingController::Listener {
+  class Listener : public DevToolsAndroidBridge::PortForwardingListener {
    public:
     explicit Listener(Profile* profile)
         : profile_(profile),
           skip_empty_devices_(true) {
-      PortForwardingController::Factory::GetForProfile(profile_)->
-          AddListener(this);
+      DevToolsAndroidBridge::Factory::GetForProfile(profile_)->
+          AddPortForwardingListener(this);
     }
 
     virtual ~Listener() {
-      PortForwardingController::Factory::GetForProfile(profile_)->
-          RemoveListener(this);
+      DevToolsAndroidBridge::Factory::GetForProfile(profile_)->
+          RemovePortForwardingListener(this);
     }
 
     virtual void PortStatusChanged(const DevicesStatus& status) OVERRIDE {

chrome/browser/devtools/device/port_forwarding_controller.cc

@@ -531,8 +531,7 @@ void PortForwardingController::Connection::OnFrameRead(
 
 PortForwardingController::PortForwardingController(Profile* profile)
     : profile_(profile),
-      pref_service_(profile->GetPrefs()),
-      listening_(false) {
+      pref_service_(profile->GetPrefs()) {
   pref_change_registrar_.Init(pref_service_);
   base::Closure callback = base::Bind(
       &PortForwardingController::OnPrefsChange, base::Unretained(this));
@@ -541,29 +540,14 @@ PortForwardingController::PortForwardingController(Profile* profile)
   OnPrefsChange();
 }
 
-
 PortForwardingController::~PortForwardingController() {}
 
-void PortForwardingController::Shutdown() {
-  // Existing connection will not be shut down. This might be confusing for
-  // some users, but the opposite is more confusing.
-  StopListening();
-}
-
-void PortForwardingController::AddListener(Listener* listener) {
-  listeners_.push_back(listener);
-}
-
-void PortForwardingController::RemoveListener(Listener* listener) {
-  Listeners::iterator it =
-      std::find(listeners_.begin(), listeners_.end(), listener);
-  DCHECK(it != listeners_.end());
-  listeners_.erase(it);
-}
-
-void PortForwardingController::DeviceListChanged(
+PortForwardingController::DevicesStatus
+PortForwardingController::DeviceListChanged(
     const DevToolsAndroidBridge::RemoteDevices& devices) {
   DevicesStatus status;
+  if (forwarding_map_.empty())
+    return status;
 
   for (DevToolsAndroidBridge::RemoteDevices::const_iterator it =
       devices.begin(); it != devices.end(); ++it) {
@@ -582,7 +566,7 @@ void PortForwardingController::DeviceListChanged(
     }
   }
 
-  NotifyListeners(status);
+  return status;
 }
 
 void PortForwardingController::OnPrefsChange() {
@@ -602,36 +586,12 @@ void PortForwardingController::OnPrefsChange() {
   }
 
   if (!forwarding_map_.empty()) {
-    StartListening();
     UpdateConnections();
   } else {
-    StopListening();
     ShutdownConnections();
-    NotifyListeners(DevicesStatus());
   }
 }
 
-void PortForwardingController::StartListening() {
-  if (listening_)
-    return;
-  listening_ = true;
-  DevToolsAndroidBridge* android_bridge =
-      DevToolsAndroidBridge::Factory::GetForProfile(profile_);
-  if (android_bridge)
-    android_bridge->AddDeviceListListener(this);
-
-}
-
-void PortForwardingController::StopListening() {
-  if (!listening_)
-    return;
-  listening_ = false;
-  DevToolsAndroidBridge* android_bridge =
-      DevToolsAndroidBridge::Factory::GetForProfile(profile_);
-  if (android_bridge)
-    android_bridge->RemoveDeviceListListener(this);
-}
-
 void PortForwardingController::UpdateConnections() {
   for (Registry::iterator it = registry_.begin(); it != registry_.end(); ++it)
     it->second->UpdateForwardingMap(forwarding_map_);
@@ -642,36 +602,3 @@ void PortForwardingController::ShutdownConnections() {
     it->second->Shutdown();
   registry_.clear();
 }
-
-void PortForwardingController::NotifyListeners(
-    const DevicesStatus& status) const {
-  Listeners copy(listeners_);  // Iterate over copy.
-  for (Listeners::const_iterator it = copy.begin(); it != copy.end(); ++it)
-    (*it)->PortStatusChanged(status);
-}
-
-// static
-PortForwardingController::Factory*
-PortForwardingController::Factory::GetInstance() {
-  return Singleton<PortForwardingController::Factory>::get();
-}
-
-// static
-PortForwardingController* PortForwardingController::Factory::GetForProfile(
-    Profile* profile) {
-  return static_cast<PortForwardingController*>(GetInstance()->
-          GetServiceForBrowserContext(profile, true));
-}
-
-PortForwardingController::Factory::Factory()
-    : BrowserContextKeyedServiceFactory(
-          "PortForwardingController",
-          BrowserContextDependencyManager::GetInstance()) {}
-
-PortForwardingController::Factory::~Factory() {}
-
-KeyedService* PortForwardingController::Factory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  Profile* profile = Profile::FromBrowserContext(context);
-  return new PortForwardingController(profile);
-}

chrome/browser/devtools/device/port_forwarding_controller.h

@@ -15,81 +15,33 @@
 class PrefService;
 class Profile;
 
-class PortForwardingController
-    : private KeyedService,
-      private DevToolsAndroidBridge::DeviceListListener {
+class PortForwardingController {
  public:
+  typedef DevToolsAndroidBridge::PortStatus PortStatus;
+  typedef DevToolsAndroidBridge::PortStatusMap PortStatusMap;
+  typedef DevToolsAndroidBridge::DevicesStatus DevicesStatus;
+
   explicit PortForwardingController(Profile* profile);
 
   virtual ~PortForwardingController();
 
-  // KeyedService implementation.
-  virtual void Shutdown() OVERRIDE;
-
-  class Factory : public BrowserContextKeyedServiceFactory {
-   public:
-    // Returns singleton instance of Factory.
-    static Factory* GetInstance();
-
-    // Returns PortForwardingController associated with |profile|.
-    static PortForwardingController* GetForProfile(Profile* profile);
-
-   private:
-    friend struct DefaultSingletonTraits<Factory>;
-
-    Factory();
-    virtual ~Factory();
-
-    // BrowserContextKeyedServiceFactory overrides:
-    virtual KeyedService* BuildServiceInstanceFor(
-        content::BrowserContext* context) const OVERRIDE;
-    DISALLOW_COPY_AND_ASSIGN(Factory);
-  };
-
-  typedef int PortStatus;
-  typedef std::map<int, PortStatus> PortStatusMap;
-  typedef std::map<std::string, PortStatusMap> DevicesStatus;
-
-  class Listener {
-   public:
-    typedef PortForwardingController::PortStatusMap PortStatusMap;
-    typedef PortForwardingController::DevicesStatus DevicesStatus;
-
-    virtual void PortStatusChanged(const DevicesStatus&) = 0;
-   protected:
-    virtual ~Listener() {}
-  };
-
-  void AddListener(Listener* listener);
-  void RemoveListener(Listener* listener);
+  DevicesStatus DeviceListChanged(
+      const DevToolsAndroidBridge::RemoteDevices& devices);
 
  private:
   class Connection;
   typedef std::map<std::string, Connection*> Registry;
 
-  // DevToolsAndroidBridge::Listener implementation.
-  virtual void DeviceListChanged(
-      const DevToolsAndroidBridge::RemoteDevices& devices) OVERRIDE;
-
   void OnPrefsChange();
 
-  void StartListening();
-  void StopListening();
-
   void UpdateConnections();
   void ShutdownConnections();
 
-  void NotifyListeners(const DevicesStatus& status) const;
-
   Profile* profile_;
   PrefService* pref_service_;
   PrefChangeRegistrar pref_change_registrar_;
   Registry registry_;
 
-  typedef std::vector<Listener*> Listeners;
-  Listeners listeners_;
-  bool listening_;
-
   typedef std::map<int, std::string> ForwardingMap;
   ForwardingMap forwarding_map_;
 

chrome/browser/devtools/devtools_targets_ui.cc

@@ -515,17 +515,17 @@ PortForwardingStatusSerializer::PortForwardingStatusSerializer(
     const Callback& callback, Profile* profile)
       : callback_(callback),
         profile_(profile) {
-  PortForwardingController* port_forwarding_controller =
-      PortForwardingController::Factory::GetForProfile(profile_);
-  if (port_forwarding_controller)
-    port_forwarding_controller->AddListener(this);
+  DevToolsAndroidBridge* android_bridge =
+      DevToolsAndroidBridge::Factory::GetForProfile(profile_);
+  if (android_bridge)
+    android_bridge->AddPortForwardingListener(this);
 }
 
 PortForwardingStatusSerializer::~PortForwardingStatusSerializer() {
-  PortForwardingController* port_forwarding_controller =
-      PortForwardingController::Factory::GetForProfile(profile_);
-  if (port_forwarding_controller)
-    port_forwarding_controller->RemoveListener(this);
+  DevToolsAndroidBridge* android_bridge =
+      DevToolsAndroidBridge::Factory::GetForProfile(profile_);
+  if (android_bridge)
+    android_bridge->RemovePortForwardingListener(this);
 }
 
 void PortForwardingStatusSerializer::PortStatusChanged(

chrome/browser/devtools/devtools_targets_ui.h

@@ -10,7 +10,7 @@
 
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/devtools/device/port_forwarding_controller.h"
+#include "chrome/browser/devtools/device/devtools_android_bridge.h"
 
 namespace base {
 class ListValue;
@@ -64,7 +64,7 @@ class DevToolsTargetsUIHandler {
 };
 
 class PortForwardingStatusSerializer
-    : private PortForwardingController::Listener {
+    : private DevToolsAndroidBridge::PortForwardingListener {
  public:
   typedef base::Callback<void(const base::Value&)> Callback;