DevTools: Partially redesigned DevToolsAndroidBridge and AndroidDeviceManager

chrome/browser/devtools/device/devtools_android_bridge.cc

 // Copyright 2014 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 "chrome/browser/devtools/device/devtools_android_bridge.h"
 
 #include <map>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 212 matching lines @@
   }
   return socket_to_pid;
 }
 
 // DiscoveryRequest -----------------------------------------------------
 
 class DiscoveryRequest : public base::RefCountedThreadSafe<
     DiscoveryRequest,
     BrowserThread::DeleteOnUIThread> {
  public:
-  typedef base::Callback<void(DevToolsAndroidBridge::RemoteDevices*)> Callback;
+  typedef base::Callback<void(DevToolsAndroidBridge::RemoteDevices*,
+      DevToolsAndroidBridge::DeviceHandleMap*)> Callback;
 
   DiscoveryRequest(
       scoped_refptr<DevToolsAndroidBridge> android_bridge,
+      const DevToolsAndroidBridge::DeviceHandleMap& device_handles,
       AndroidDeviceManager* device_manager,
       base::MessageLoop* device_message_loop,
       const AndroidDeviceManager::DeviceProviders& device_providers,
       const Callback& callback);
 
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<DiscoveryRequest>;
 
   virtual ~DiscoveryRequest();
@@ skipping 27 matching lines @@
 
   void Respond();
 
   void CreateBrowsers(const std::string& packages_response,
                       const std::string& processes_response,
                       const std::string& sockets_response);
 
   void ParseDumpsysResponse(const std::string& response);
   void ParseScreenSize(const std::string& str);
 
+  scoped_refptr<DevToolsAndroidBridge::DeviceHandle>
+  FindDeviceHandle(const std::string& serial);
+
   scoped_refptr<DevToolsAndroidBridge> android_bridge_;
   AndroidDeviceManager* device_manager_;
   base::MessageLoop* device_message_loop_;
   Callback callback_;
   std::vector<std::string> serials_;
   DevToolsAndroidBridge::RemoteBrowsers browsers_;
   scoped_ptr<DevToolsAndroidBridge::RemoteDevices> remote_devices_;
+  DevToolsAndroidBridge::DeviceHandleMap device_handles_;
+  scoped_ptr<DevToolsAndroidBridge::DeviceHandleMap> used_device_handles_;
 };
 
 DiscoveryRequest::DiscoveryRequest(
     scoped_refptr<DevToolsAndroidBridge> android_bridge,
+    const DevToolsAndroidBridge::DeviceHandleMap& device_handles,
     AndroidDeviceManager* device_manager,
     base::MessageLoop* device_message_loop,
     const AndroidDeviceManager::DeviceProviders& device_providers,
     const Callback& callback)
     : android_bridge_(android_bridge),
       device_manager_(device_manager),
       device_message_loop_(device_message_loop),
-      callback_(callback) {
+      callback_(callback),
+      device_handles_(device_handles) {

[Vladislav Kaznacheev, 2014/05/19 10:32:11]

So there is a scenario where DeviceHandle instances are released on different
threads, right?

[vkuzkokov, 2014/05/20 12:34:29]

Yep. One possible solution would be to extend DiscoveryRequest::Callback to
accept 3 parameters: RemoteDevices, DeviceHandleMap, and a vector of
DeviceHandles that need to be freed.

   remote_devices_.reset(new DevToolsAndroidBridge::RemoteDevices());
+  used_device_handles_.reset(new DevToolsAndroidBridge::DeviceHandleMap());
 
   device_message_loop_->PostTask(
       FROM_HERE, base::Bind(
           &AndroidDeviceManager::QueryDevices,
           device_manager_,
           device_providers,
           base::Bind(&DiscoveryRequest::ReceivedSerials, this)));
 }
 
 DiscoveryRequest::~DiscoveryRequest() {
@@ skipping 14 matching lines @@
         BrowserThread::UI, FROM_HERE,
         base::Bind(&DiscoveryRequest::Respond, this));
     return;
   }
 
   if (device_manager_->IsConnected(current_serial())) {
     device_manager_->RunCommand(current_serial(), kDeviceModelCommand,
         base::Bind(&DiscoveryRequest::ReceivedModel, this));
   } else {
     remote_devices_->push_back(new DevToolsAndroidBridge::RemoteDevice(
-        android_bridge_, current_serial(), kUnknownModel, false));
+        FindDeviceHandle(current_serial()), kUnknownModel, false));
     NextDevice();
   }
 }
 
 void DiscoveryRequest::ReceivedModel(int result, const std::string& response) {
   DCHECK_EQ(device_message_loop_, base::MessageLoop::current());
   if (result < 0) {
     NextDevice();
     return;
   }
   remote_devices_->push_back(new DevToolsAndroidBridge::RemoteDevice(
-      android_bridge_, current_serial(), response, true));
+      FindDeviceHandle(current_serial()), response, true));
   device_manager_->RunCommand(current_serial(), kDumpsysCommand,
       base::Bind(&DiscoveryRequest::ReceivedDumpsys, this));
 }
 
 void DiscoveryRequest::ReceivedDumpsys(int result,
                                       const std::string& response) {
   DCHECK_EQ(device_message_loop_, base::MessageLoop::current());
   if (result >= 0)
     ParseDumpsysResponse(response);
 
@@ skipping 112 matching lines @@
   browsers_.pop_back();
   ProcessSockets();
 }
 
 void DiscoveryRequest::NextDevice() {
   serials_.pop_back();
   ProcessSerials();
 }
 
 void DiscoveryRequest::Respond() {
-  callback_.Run(remote_devices_.release());
+  callback_.Run(remote_devices_.release(), used_device_handles_.release());
 }
 
 void DiscoveryRequest::CreateBrowsers(
     const std::string& packages_response,
     const std::string& processes_response,
     const std::string& sockets_response) {
   DescriptorMap package_to_descriptor =
       FindInstalledBrowserPackages(packages_response);
 
   StringMap pid_to_package;
@@ skipping 12 matching lines @@
   // Create RemoteBrowser instances.
   BrowserMap package_to_running_browser;
   BrowserMap socket_to_unnamed_browser;
   for (StringMap::iterator it = socket_to_pid.begin();
       it != socket_to_pid.end(); ++it) {
     std::string socket = it->first;
     std::string pid = it->second;
 
     scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser =
         new DevToolsAndroidBridge::RemoteBrowser(
-            android_bridge_, current_serial(), socket);
+            FindDeviceHandle(current_serial()),
+            socket);
 
     StringMap::iterator pit = pid_to_package.find(pid);
     if (pit != pid_to_package.end()) {
       std::string package = pit->second;
       package_to_running_browser[package] = browser;
       const BrowserDescriptor* descriptor = FindBrowserDescriptor(package);
       if (descriptor) {
         browser->set_display_name(descriptor->display_name);
       } else if (socket.find(kWebViewSocketPrefix) == 0) {
         browser->set_display_name(
@@ skipping 75 matching lines @@
   std::vector<std::string> numbers;
   Tokenize(pairs[1].substr(1, pairs[1].size() - 2), ",", &numbers);
   if (numbers.size() != 2 ||
       !base::StringToInt(numbers[0], &width) ||
       !base::StringToInt(numbers[1], &height))
     return;
 
   remote_devices_->back()->set_screen_size(gfx::Size(width, height));
 }
 
+scoped_refptr<DevToolsAndroidBridge::DeviceHandle>
+DiscoveryRequest::FindDeviceHandle(const std::string& serial) {
+  DevToolsAndroidBridge::DeviceHandleMap::iterator it =
+      device_handles_.find(serial);
+  scoped_refptr<DevToolsAndroidBridge::DeviceHandle> device_handle;
+  if (it != device_handles_.end()) {
+    device_handle = it->second;
+  } else {
+    device_handle = new DevToolsAndroidBridge::DeviceHandle(android_bridge_,
+                                                            serial);
+    device_handles_[serial] = device_handle;
+  }
+  (*used_device_handles_)[serial] = device_handle;
+  return device_handle;
+}
 
 // ProtocolCommand ------------------------------------------------------------
 
 class ProtocolCommand
     : public DevToolsAndroidBridge::AndroidWebSocket::Delegate {
  public:
   ProtocolCommand(
       scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
       const std::string& debug_url,
       const std::string& command,
@@ skipping 320 matching lines @@
                                 base::Closure callback) const {
   base::DictionaryValue params;
   params.SetString(kUrlParam, url);
   browser_->SendProtocolCommand(debug_url_, kPageNavigateCommand, &params,
                                 callback);
 }
 
 // DevToolsAndroidBridge::RemoteBrowser ---------------------------------------
 
 DevToolsAndroidBridge::RemoteBrowser::RemoteBrowser(
-    scoped_refptr<DevToolsAndroidBridge> android_bridge,
-    const std::string& serial,
+    scoped_refptr<DeviceHandle> device_handle,
     const std::string& socket)
-    : android_bridge_(android_bridge),
-      serial_(serial),
+    : device_handle_(device_handle),
       socket_(socket),
       page_descriptors_(new base::ListValue()) {
 }
 
 bool DevToolsAndroidBridge::RemoteBrowser::IsChrome() const {
   return socket_.find(kChromeDefaultSocket) == 0;
 }
 
 DevToolsAndroidBridge::RemoteBrowser::ParsedVersion
 DevToolsAndroidBridge::RemoteBrowser::GetParsedVersion() const {
@@ skipping 35 matching lines @@
     const std::string& response) {
   if (callback.is_null())
     return;
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE, base::Bind(callback, result, response));
 }
 
 void DevToolsAndroidBridge::RemoteBrowser::SendJsonRequest(
     const std::string& request, const JsonRequestCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  android_bridge_->device_message_loop()->PostTask(
+  device_handle_->android_bridge()->device_message_loop()->PostTask(

[Vladislav Kaznacheev, 2014/05/19 10:32:11]

Lets consider moving it into DeviceHandle as well.

[vkuzkokov, 2014/05/20 12:34:29]

Done.

       FROM_HERE,
       base::Bind(&AndroidDeviceManager::HttpQuery,
-                 android_bridge_->device_manager(), serial_, socket_, request,
+                 device_handle_->android_bridge()->device_manager(),
+                 device_handle_->serial(),
+                 socket_,
+                 request,
                  base::Bind(&RespondOnUIThread, callback)));
 }
 
 void DevToolsAndroidBridge::RemoteBrowser::SendProtocolCommand(
     const std::string& debug_url,
     const std::string& method,
     base::DictionaryValue* params,
     const base::Closure callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (debug_url.empty())
@@ skipping 84 matching lines @@
   }
 }
 
 DevToolsAndroidBridge::RemoteBrowser::~RemoteBrowser() {
 }
 
 
 // DevToolsAndroidBridge::RemoteDevice ----------------------------------------
 
 DevToolsAndroidBridge::RemoteDevice::RemoteDevice(
-    scoped_refptr<DevToolsAndroidBridge> android_bridge,
-    const std::string& serial,
+    scoped_refptr<DeviceHandle> device_handle,
     const std::string& model,
     bool connected)
-    : android_bridge_(android_bridge),
-      serial_(serial),
+    : device_handle_(device_handle),
       model_(model),
       connected_(connected) {
 }
 
 void DevToolsAndroidBridge::RemoteDevice::AddBrowser(
     scoped_refptr<RemoteBrowser> browser) {
   browsers_.push_back(browser);
 }
 
 void DevToolsAndroidBridge::RemoteDevice::OpenSocket(
     const std::string& socket_name,
     const AndroidDeviceManager::SocketCallback& callback) {
-  android_bridge_->device_message_loop()->PostTask(FROM_HERE,
+  device_handle_->android_bridge()->device_message_loop()->PostTask(FROM_HERE,

[Vladislav Kaznacheev, 2014/05/19 10:32:11]

Ditto

[vkuzkokov, 2014/05/20 12:34:29]

Done.

       base::Bind(&AndroidDeviceManager::OpenSocket,
-                 android_bridge_->device_manager(),
-                 serial_,
+                 device_handle_->android_bridge()->device_manager(),
+                 device_handle_->serial(),
                  socket_name,
                  callback));
 }
 
 DevToolsAndroidBridge::RemoteDevice::~RemoteDevice() {
 }
 
 // DevToolsAndroidBridge::HandlerThread ---------------------------------
 
 const char kDevToolsAdbBridgeThreadName[] = "Chrome_DevToolsADBThread";
@@ skipping 35 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   instance_ = NULL;
   if (!thread_)
     return;
   // Shut down thread on FILE thread to join into IO.
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&HandlerThread::StopThread, thread_));
 }
 
+// DevToolsAndroidBridge::DeviceHandle ----------------------------------------
+
+DevToolsAndroidBridge::DeviceHandle::DeviceHandle(
+    scoped_refptr<DevToolsAndroidBridge> android_bridge,
+    const std::string& serial)
+    : android_bridge_(android_bridge),
+      serial_(serial) {
+}
+
+DevToolsAndroidBridge::DeviceHandle::~DeviceHandle() {
+  android_bridge_->device_message_loop()->PostTask(FROM_HERE,
+      base::Bind(&AndroidDeviceManager::ReleaseDevice,
+                 android_bridge_->device_manager(),
+                 serial_));
+}
+
 // DevToolsAndroidBridge ------------------------------------------------------
 
 DevToolsAndroidBridge::DevToolsAndroidBridge(Profile* profile)
     : profile_(profile),
       handler_thread_(HandlerThread::GetInstance()) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(prefs::kDevToolsDiscoverUsbDevicesEnabled,
       base::Bind(&DevToolsAndroidBridge::CreateDeviceProviders,
                  base::Unretained(this)));
@@ skipping 13 matching lines @@
     RequestDeviceList();
 }
 
 void DevToolsAndroidBridge::RemoveDeviceListListener(
     DeviceListListener* listener) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DeviceListListeners::iterator it = std::find(
       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() && device_manager_) {
-    device_message_loop()->PostTask(FROM_HERE,
-        base::Bind(&AndroidDeviceManager::Stop, device_manager_));
-  }
+  if (device_list_listeners_.empty() && device_manager_)
+    device_handles_.clear();
 }
 
 void DevToolsAndroidBridge::AddDeviceCountListener(
     DeviceCountListener* listener) {
   device_count_listeners_.push_back(listener);
   if (device_count_listeners_.size() == 1 && device_manager_)
     RequestDeviceCount();
 }
 
 void DevToolsAndroidBridge::RemoveDeviceCountListener(
     DeviceCountListener* listener) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DeviceCountListeners::iterator it = std::find(
       device_count_listeners_.begin(), device_count_listeners_.end(), listener);
   DCHECK(it != device_count_listeners_.end());
   device_count_listeners_.erase(it);
 }
 
 // static
 bool DevToolsAndroidBridge::HasDevToolsWindow(const std::string& agent_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return g_host_delegates.Get().find(agent_id) != g_host_delegates.Get().end();
 }
 
+void DevToolsAndroidBridge::IsConnectedForTest(const std::string& serial,
+    const base::Callback<void(bool)>& callback) {
+  base::PostTaskAndReplyWithResult(
+      device_message_loop()->message_loop_proxy(),
+      FROM_HERE,
+      base::Bind(&AndroidDeviceManager::IsConnected, device_manager_, serial),
+      callback);
+}
+
 DevToolsAndroidBridge::~DevToolsAndroidBridge() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(device_list_listeners_.empty());
   DCHECK(device_count_listeners_.empty());
-  if (device_manager_)
-    device_message_loop()->PostTask(FROM_HERE,
-        base::Bind(&AndroidDeviceManager::Stop, device_manager_));
 }
 
 void DevToolsAndroidBridge::RequestDeviceList() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(device_manager_);
 
   if (device_list_listeners_.empty())
     return;
 
   new DiscoveryRequest(
       this,
+      device_handles_,
       device_manager(),
       device_message_loop(),
       device_providers_,
       base::Bind(&DevToolsAndroidBridge::ReceivedDeviceList, this));
 }
 
 void DevToolsAndroidBridge::CreatedDeviceManager(
     scoped_refptr<AndroidDeviceManager> device_manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   device_manager_ = device_manager;
   if (!device_list_listeners_.empty())
     RequestDeviceList();
   if (!device_count_listeners_.empty())
     RequestDeviceCount();
 }
 
-void DevToolsAndroidBridge::ReceivedDeviceList(RemoteDevices* devices_ptr) {
+void DevToolsAndroidBridge::ReceivedDeviceList(RemoteDevices* devices_ptr,
+    DeviceHandleMap* device_handles_ptr) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   scoped_ptr<RemoteDevices> devices(devices_ptr);
+  scoped_ptr<DeviceHandleMap> device_handles(device_handles_ptr);
 
   if (device_list_listeners_.empty())
     return;
 
+  device_handles_ = *device_handles.get();
+
   DeviceListListeners copy(device_list_listeners_);
   for (DeviceListListeners::iterator it = copy.begin(); it != copy.end(); ++it)
     (*it)->DeviceListChanged(*devices.get());
 
   BrowserThread::PostDelayedTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&DevToolsAndroidBridge::RequestDeviceList, this),
       base::TimeDelta::FromMilliseconds(kAdbPollingIntervalMs));
 }
@@ skipping 40 matching lines @@
   PrefService* service = profile_->GetPrefs();
   const PrefService::Preference* pref =
       service->FindPreference(prefs::kDevToolsDiscoverUsbDevicesEnabled);
   const base::Value* pref_value = pref->GetValue();
 
   bool enabled;
   if (pref_value->GetAsBoolean(&enabled) && enabled) {
     device_providers_.push_back(new UsbDeviceProvider(profile_));
   }
 }