[DevTools] Port forwarding doesn't keep USB connection

That means that when chrome://inspect is closed device is released.

BUG=387067

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

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

chrome/browser/devtools/device/android_device_manager.cc

@@ -326,7 +326,7 @@ AndroidDeviceManager::DeviceProvider::~DeviceProvider() {
 
 void AndroidDeviceManager::Device::QueryDeviceInfo(
     const DeviceInfoCallback& callback) {
-  device_message_loop_->PostTask(
+  message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&DeviceProvider::QueryDeviceInfo,
                  provider_,
@@ -338,7 +338,7 @@ void AndroidDeviceManager::Device::QueryDeviceInfo(
 
 void AndroidDeviceManager::Device::OpenSocket(const std::string& socket_name,
                                               const SocketCallback& callback) {
-  device_message_loop_->PostTask(
+  message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&DeviceProvider::OpenSocket,
                  provider_,
@@ -351,7 +351,7 @@ void AndroidDeviceManager::Device::SendJsonRequest(
     const std::string& socket_name,
     const std::string& request,
     const CommandCallback& callback) {
-  device_message_loop_->PostTask(
+  message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&DeviceProvider::SendJsonRequest,
                  provider_,
@@ -366,7 +366,7 @@ void AndroidDeviceManager::Device::SendJsonRequest(
 void AndroidDeviceManager::Device::HttpUpgrade(const std::string& socket_name,
                                                const std::string& url,
                                                const SocketCallback& callback) {
-  device_message_loop_->PostTask(
+  message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&DeviceProvider::HttpUpgrade,
                  provider_,
@@ -382,17 +382,21 @@ AndroidDeviceManager::Device::Device(
     scoped_refptr<base::MessageLoopProxy> device_message_loop,
     scoped_refptr<DeviceProvider> provider,
     const std::string& serial)
-    : device_message_loop_(device_message_loop),
+    : message_loop_proxy_(device_message_loop),
       provider_(provider),
       serial_(serial),
       weak_factory_(this) {
 }
 
 AndroidDeviceManager::Device::~Device() {
+  std::set<AndroidWebSocket*> sockets_copy(sockets_);
+  for (AndroidWebSocket* socket : sockets_copy)
+    socket->OnSocketClosed();
+
   provider_->AddRef();
   DeviceProvider* raw_ptr = provider_.get();
   provider_ = NULL;
-  device_message_loop_->PostTask(
+  message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&ReleaseDeviceAndProvider,
                  base::Unretained(raw_ptr),

chrome/browser/devtools/device/android_device_manager.h

@@ -83,8 +83,9 @@ class AndroidDeviceManager
     void Connected(int result, scoped_ptr<net::StreamSocket> socket);
     void OnFrameRead(const std::string& message);
     void OnSocketClosed();
+    void Terminate();
 
-    scoped_refptr<Device> device_;
+    Device* device_;
     WebSocketImpl* socket_impl_;
     Delegate* delegate_;
     base::WeakPtrFactory<AndroidWebSocket> weak_factory_;
@@ -126,9 +127,10 @@ class AndroidDeviceManager
 
     virtual ~Device();
 
-    scoped_refptr<base::MessageLoopProxy> device_message_loop_;
+    scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
     scoped_refptr<DeviceProvider> provider_;
     std::string serial_;
+    std::set<AndroidWebSocket*> sockets_;
     base::WeakPtrFactory<Device> weak_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(Device);

chrome/browser/devtools/device/android_web_socket.cc

@@ -132,12 +132,14 @@ AndroidDeviceManager::AndroidWebSocket::AndroidWebSocket(
     const std::string& socket_name,
     const std::string& url,
     Delegate* delegate)
-    : device_(device),
+    : device_(device.get()),
       socket_impl_(nullptr),
       delegate_(delegate),
       weak_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(delegate_);
+  DCHECK(device_);
+  device_->sockets_.insert(this);
   device_->HttpUpgrade(
       socket_name, url,
       base::Bind(&AndroidWebSocket::Connected, weak_factory_.GetWeakPtr()));
@@ -145,15 +147,15 @@ AndroidDeviceManager::AndroidWebSocket::AndroidWebSocket(
 
 AndroidDeviceManager::AndroidWebSocket::~AndroidWebSocket() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (socket_impl_)
-    device_->device_message_loop_->DeleteSoon(FROM_HERE, socket_impl_);
+  Terminate();
 }
 
 void AndroidDeviceManager::AndroidWebSocket::SendFrame(
     const std::string& message) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(socket_impl_);
-  device_->device_message_loop_->PostTask(
+  DCHECK(device_);
+  device_->message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&WebSocketImpl::SendFrame,
                  base::Unretained(socket_impl_), message));
@@ -170,7 +172,7 @@ void AndroidDeviceManager::AndroidWebSocket::Connected(
   socket_impl_ = new WebSocketImpl(base::MessageLoopProxy::current(),
                                    weak_factory_.GetWeakPtr(),
                                    socket.Pass());
-  device_->device_message_loop_->PostTask(
+  device_->message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&WebSocketImpl::StartListening,
                  base::Unretained(socket_impl_)));
@@ -185,13 +187,27 @@ void AndroidDeviceManager::AndroidWebSocket::OnFrameRead(
 
 void AndroidDeviceManager::AndroidWebSocket::OnSocketClosed() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  Terminate();
   delegate_->OnSocketClosed();
 }
 
+void AndroidDeviceManager::AndroidWebSocket::Terminate() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (socket_impl_) {
+    DCHECK(device_);
+    device_->message_loop_proxy_->DeleteSoon(FROM_HERE, socket_impl_);
+    socket_impl_ = nullptr;
+  }
+  if (device_) {
+    device_->sockets_.erase(this);
+    device_ = nullptr;
+  }
+}
+
 AndroidDeviceManager::AndroidWebSocket*
 AndroidDeviceManager::Device::CreateWebSocket(
-    const std::string& socket,
+    const std::string& socket_name,
     const std::string& url,
     AndroidWebSocket::Delegate* delegate) {
-  return new AndroidWebSocket(this, socket, url, delegate);
+  return new AndroidWebSocket(this, socket_name, url, delegate);
 }

chrome/browser/devtools/device/devtools_android_bridge.cc

@@ -308,6 +308,7 @@ class DevToolsAndroidBridge::AgentHostDelegate
   bool socket_opened_;
   bool is_web_view_;
   std::vector<std::string> pending_messages_;
+  scoped_refptr<AndroidDeviceManager::Device> device_;
   scoped_ptr<AndroidDeviceManager::AndroidWebSocket> web_socket_;
   content::DevToolsAgentHost* agent_host_;
   content::DevToolsExternalAgentProxy* proxy_;
@@ -361,12 +362,19 @@ void DevToolsAndroidBridge::AgentHostDelegate::Attach(
   proxy_ = proxy;
   content::RecordAction(base::UserMetricsAction(is_web_view_ ?
       "DevTools_InspectAndroidWebView" : "DevTools_InspectAndroidPage"));
+
+  // Retain the device so it's not released until AgentHost is detached.
+  device_ = bridge_->FindDevice(browser_id_.first);
+  if (!device_.get())
+    return;
+
   web_socket_.reset(
-      bridge_->CreateWebSocket(browser_id_, debug_url_, this));
+      device_->CreateWebSocket(browser_id_.second, debug_url_, this));
 }
 
 void DevToolsAndroidBridge::AgentHostDelegate::Detach() {
   web_socket_.reset();
+  device_ = nullptr;
 }
 
 void DevToolsAndroidBridge::AgentHostDelegate::SendMessageToBackend(
@@ -594,12 +602,13 @@ void DevToolsAndroidBridge::SendJsonRequest(
     const BrowserId& browser_id,
     const std::string& request,
     const JsonRequestCallback& callback) {
-  DeviceMap::iterator it = device_map_.find(browser_id.first);
-  if (it == device_map_.end()) {
+  scoped_refptr<AndroidDeviceManager::Device> device(
+      FindDevice(browser_id.first));
+  if (!device.get()) {
     callback.Run(net::ERR_FAILED, std::string());
     return;
   }
-  it->second->SendJsonRequest(browser_id.second, request, callback);
+  device->SendJsonRequest(browser_id.second, request, callback);
 }
 
 void DevToolsAndroidBridge::SendProtocolCommand(
@@ -611,13 +620,14 @@ void DevToolsAndroidBridge::SendProtocolCommand(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (debug_url.empty())
     return;
-  DeviceMap::iterator it = device_map_.find(browser_id.first);
-  if (it == device_map_.end()) {
+  scoped_refptr<AndroidDeviceManager::Device> device(
+      FindDevice(browser_id.first));
+  if (!device.get()) {
     callback.Run();
     return;
   }
   DevToolsProtocol::Command command(1, method, params);
-  new ProtocolCommand(it->second, browser_id.second, debug_url,
+  new ProtocolCommand(device, browser_id.second, debug_url,
                       command.Serialize(), callback);
 }
 
@@ -632,16 +642,10 @@ DevToolsAndroidBridge::GetBrowserAgentHost(
       browser->IsWebView());
 }
 
-AndroidDeviceManager::AndroidWebSocket*
-DevToolsAndroidBridge::CreateWebSocket(
-    const BrowserId& browser_id,
-    const std::string& url,
-    AndroidDeviceManager::AndroidWebSocket::Delegate* delegate) {
-  DeviceMap::iterator it = device_map_.find(browser_id.first);
-  if (it == device_map_.end())
-    return NULL;
-
-  return it->second->CreateWebSocket(browser_id.second, url, delegate);
+scoped_refptr<AndroidDeviceManager::Device> DevToolsAndroidBridge::FindDevice(
+    const std::string& serial) {
+  DeviceMap::iterator it = device_map_.find(serial);
+  return it == device_map_.end() ? nullptr : it->second;
 }
 
 void DevToolsAndroidBridge::RespondToOpenOnUIThread(
@@ -760,7 +764,7 @@ DevToolsAndroidBridge::DevToolsAndroidBridge(Profile* profile)
     : profile_(profile),
       device_manager_(AndroidDeviceManager::Create()),
       task_scheduler_(base::Bind(&DevToolsAndroidBridge::ScheduleTaskDefault)),
-      port_forwarding_controller_(new PortForwardingController(profile)) {
+      port_forwarding_controller_(new PortForwardingController(profile, this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(prefs::kDevToolsDiscoverUsbDevicesEnabled,
@@ -881,7 +885,7 @@ void DevToolsAndroidBridge::ReceivedDeviceList(
     (*it)->DeviceListChanged(remote_devices);
 
   ForwardingStatus status =
-      port_forwarding_controller_->DeviceListChanged(complete_devices);
+      port_forwarding_controller_->DeviceListChanged(remote_devices);
   PortForwardingListeners forwarding_listeners(port_forwarding_listeners_);
   for (PortForwardingListeners::iterator it = forwarding_listeners.begin();
        it != forwarding_listeners.end(); ++it) {

chrome/browser/devtools/device/devtools_android_bridge.h

@@ -226,17 +226,13 @@ class DevToolsAndroidBridge
 
   scoped_refptr<content::DevToolsAgentHost> GetBrowserAgentHost(
       scoped_refptr<RemoteBrowser> browser);
-
-  typedef std::pair<scoped_refptr<AndroidDeviceManager::Device>,
-                    scoped_refptr<RemoteDevice>> CompleteDevice;
-  typedef std::vector<CompleteDevice> CompleteDevices;
-  typedef base::Callback<void(const CompleteDevices&)> DeviceListCallback;
-
  private:
   friend struct content::BrowserThread::DeleteOnThread<
       content::BrowserThread::UI>;
   friend class base::DeleteHelper<DevToolsAndroidBridge>;
 
+  friend class PortForwardingController;
+
   class AgentHostDelegate;
   class DiscoveryRequest;
   class RemotePageTarget;
@@ -247,8 +243,12 @@ class DevToolsAndroidBridge
   void StopDeviceListPolling();
   bool NeedsDeviceListPolling();
 
-  void RequestDeviceList(const DeviceListCallback& callback);
+  typedef std::pair<scoped_refptr<AndroidDeviceManager::Device>,
+                    scoped_refptr<RemoteDevice>> CompleteDevice;
+  typedef std::vector<CompleteDevice> CompleteDevices;
+  typedef base::Callback<void(const CompleteDevices&)> DeviceListCallback;
 
+  void RequestDeviceList(const DeviceListCallback& callback);
   void ReceivedDeviceList(const CompleteDevices& complete_devices);
 
   void StartDeviceCountPolling();
@@ -270,10 +270,8 @@ class DevToolsAndroidBridge
                            base::DictionaryValue* params,
                            const base::Closure callback);
 
-  AndroidDeviceManager::AndroidWebSocket* CreateWebSocket(
-      const BrowserId& browser_id,
-      const std::string& url,
-      AndroidDeviceManager::AndroidWebSocket::Delegate* delegate);
+  scoped_refptr<AndroidDeviceManager::Device> FindDevice(
+      const std::string& serial);
 
   void PageCreatedOnUIThread(scoped_refptr<RemoteBrowser> browser,
                              const RemotePageCallback& callback,

chrome/browser/devtools/device/port_forwarding_controller.cc

@@ -254,8 +254,7 @@ FindBestBrowserForTethering(
 class PortForwardingController::Connection
     : public AndroidDeviceManager::AndroidWebSocket::Delegate {
  public:
-  Connection(Registry* registry,
-             scoped_refptr<AndroidDeviceManager::Device> device,
+  Connection(PortForwardingController* controller,
              scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
              const ForwardingMap& forwarding_map);
   ~Connection() override;
@@ -273,9 +272,7 @@ class PortForwardingController::Connection
       content::BrowserThread::UI>;
   friend class base::DeleteHelper<Connection>;
 
-
   typedef std::map<int, std::string> ForwardingMap;
-
   typedef base::Callback<void(PortStatus)> CommandCallback;
   typedef std::map<int, CommandCallback> CommandCallbackMap;
 
@@ -298,8 +295,7 @@ class PortForwardingController::Connection
   void OnFrameRead(const std::string& message) override;
   void OnSocketClosed() override;
 
-  PortForwardingController::Registry* registry_;
-  scoped_refptr<AndroidDeviceManager::Device> device_;
+  PortForwardingController* controller_;
   scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser_;
   scoped_ptr<AndroidDeviceManager::AndroidWebSocket> web_socket_;
   int command_id_;
@@ -313,29 +309,30 @@ class PortForwardingController::Connection
 };
 
 PortForwardingController::Connection::Connection(
-    Registry* registry,
-    scoped_refptr<AndroidDeviceManager::Device> device,
+    PortForwardingController* controller,
     scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
     const ForwardingMap& forwarding_map)
-    : registry_(registry),
-      device_(device),
+    : controller_(controller),
       browser_(browser),
       command_id_(0),
       connected_(false),
       forwarding_map_(forwarding_map),
       weak_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  (*registry_)[device_->serial()] = this;
+  controller_->registry_[browser->serial()] = this;
+  scoped_refptr<AndroidDeviceManager::Device> device(
+      controller_->bridge_->FindDevice(browser->serial()));
+  DCHECK(device.get());
   web_socket_.reset(
-      device_->CreateWebSocket(browser->socket(),
-                               kDevToolsRemoteBrowserTarget, this));
+      device->CreateWebSocket(browser->socket(),
+                              kDevToolsRemoteBrowserTarget, this));
 }
 
 PortForwardingController::Connection::~Connection() {
-
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(registry_->find(device_->serial()) != registry_->end());
-  registry_->erase(device_->serial());
+  DCHECK(controller_->registry_.find(browser_->serial()) !=
+         controller_->registry_.end());
+  controller_->registry_.erase(browser_->serial());
 }
 
 void PortForwardingController::Connection::UpdateForwardingMap(
@@ -515,7 +512,10 @@ void PortForwardingController::Connection::OnFrameRead(
       base::Bind(&Connection::UpdateSocketCountOnHandlerThread,
                  weak_factory_.GetWeakPtr(), port);
 
-  device_->OpenSocket(
+  scoped_refptr<AndroidDeviceManager::Device> device(
+      controller_->bridge_->FindDevice(browser_->serial()));
+  DCHECK(device.get());
+  device->OpenSocket(
       connection_id.c_str(),
       base::Bind(&SocketTunnel::StartTunnel,
                  destination_host,
@@ -523,8 +523,11 @@ void PortForwardingController::Connection::OnFrameRead(
                  callback));
 }
 
-PortForwardingController::PortForwardingController(Profile* profile)
+PortForwardingController::PortForwardingController(
+    Profile* profile,
+    DevToolsAndroidBridge* bridge)
     : profile_(profile),
+      bridge_(bridge),
       pref_service_(profile->GetPrefs()) {
   pref_change_registrar_.Init(pref_service_);
   base::Closure callback = base::Bind(
@@ -538,23 +541,20 @@ PortForwardingController::~PortForwardingController() {}
 
 PortForwardingController::ForwardingStatus
 PortForwardingController::DeviceListChanged(
-    const DevToolsAndroidBridge::CompleteDevices& complete_devices) {
+    const DevToolsAndroidBridge::RemoteDevices& devices) {
   ForwardingStatus status;
   if (forwarding_map_.empty())
     return status;
 
-  for (const auto& pair : complete_devices) {
-    scoped_refptr<AndroidDeviceManager::Device> device(pair.first);
-    scoped_refptr<DevToolsAndroidBridge::RemoteDevice> remote_device(
-        pair.second);
-    if (!remote_device->is_connected())
+  for (const auto& device : devices) {
+    if (!device->is_connected())
       continue;
-    Registry::iterator rit = registry_.find(remote_device->serial());
+    Registry::iterator rit = registry_.find(device->serial());
     if (rit == registry_.end()) {
       scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser(
-          FindBestBrowserForTethering(remote_device->browsers()));
+          FindBestBrowserForTethering(device->browsers()));
       if (browser.get())
-        new Connection(&registry_, device, browser, forwarding_map_);
+        new Connection(this, browser, forwarding_map_);
     } else {
       status.push_back(std::make_pair(rit->second->browser(),
                                       rit->second->GetPortStatusMap()));

chrome/browser/devtools/device/port_forwarding_controller.h

@@ -22,12 +22,13 @@ class PortForwardingController {
   typedef DevToolsAndroidBridge::BrowserStatus BrowserStatus;
   typedef DevToolsAndroidBridge::ForwardingStatus ForwardingStatus;
 
-  explicit PortForwardingController(Profile* profile);
+  explicit PortForwardingController(Profile* profile,
+                                    DevToolsAndroidBridge* bridge);
 
   virtual ~PortForwardingController();
 
   ForwardingStatus DeviceListChanged(
-      const DevToolsAndroidBridge::CompleteDevices& complete_devices);
+      const DevToolsAndroidBridge::RemoteDevices& devices);
 
  private:
   class Connection;
@@ -38,6 +39,7 @@ class PortForwardingController {
   void UpdateConnections();
 
   Profile* profile_;
+  DevToolsAndroidBridge* bridge_;
   PrefService* pref_service_;
   PrefChangeRegistrar pref_change_registrar_;
   Registry registry_;