DevTools: DevToolsAndroidBridge polling loops

chrome/browser/devtools/device/devtools_android_bridge.cc

Index: chrome/browser/devtools/device/devtools_android_bridge.cc
diff --git a/chrome/browser/devtools/device/devtools_android_bridge.cc b/chrome/browser/devtools/device/devtools_android_bridge.cc
index 5bf7b550be6e656189d5432afd05be2b4f35ff16..7cbe8137573ffabb5b3762b3daae565e444f16d2 100644
--- a/chrome/browser/devtools/device/devtools_android_bridge.cc
+++ b/chrome/browser/devtools/device/devtools_android_bridge.cc
@@ -63,7 +63,7 @@ class DiscoveryRequest : public base::RefCountedThreadSafe<
     DiscoveryRequest,
     BrowserThread::DeleteOnUIThread> {
  public:
-  typedef base::Callback<void(scoped_ptr<DevToolsAndroidBridge::RemoteDevices>)>
+  typedef base::Callback<void(const DevToolsAndroidBridge::RemoteDevices&)>
       DiscoveryCallback;
   typedef AndroidDeviceManager::Device Device;
   typedef AndroidDeviceManager::Devices Devices;
@@ -99,7 +99,7 @@ class DiscoveryRequest : public base::RefCountedThreadSafe<
   DiscoveryCallback callback_;
   Devices devices_;
   DevToolsAndroidBridge::RemoteBrowsers browsers_;
-  scoped_ptr<DevToolsAndroidBridge::RemoteDevices> remote_devices_;
+  DevToolsAndroidBridge::RemoteDevices remote_devices_;
 };
 
 DiscoveryRequest::DiscoveryRequest(
@@ -107,7 +107,6 @@ DiscoveryRequest::DiscoveryRequest(
     const DiscoveryCallback& callback)
     : callback_(callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  remote_devices_.reset(new DevToolsAndroidBridge::RemoteDevices());
 
   device_manager->QueryDevices(
       base::Bind(&DiscoveryRequest::ReceivedDevices, this));
@@ -136,9 +135,9 @@ void DiscoveryRequest::ProcessDevices() {
 void DiscoveryRequest::ReceivedDeviceInfo(
     const AndroidDeviceManager::DeviceInfo& device_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  remote_devices_->push_back(
+  remote_devices_.push_back(
       new DevToolsAndroidBridge::RemoteDevice(current_device(), device_info));
-  browsers_ = remote_devices_->back()->browsers();
+  browsers_ = remote_devices_.back()->browsers();
   ProcessSockets();
 }
 
@@ -212,7 +211,7 @@ void DiscoveryRequest::NextDevice() {
 }
 
 void DiscoveryRequest::Respond() {
-  callback_.Run(remote_devices_.Pass());
+  callback_.Run(remote_devices_);
 }
 
 // ProtocolCommand ------------------------------------------------------------
@@ -780,7 +779,7 @@ void DevToolsAndroidBridge::AddDeviceListListener(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   device_list_listeners_.push_back(listener);
   if (device_list_listeners_.size() == 1)
-    RequestDeviceList();
+    StartDeviceListPolling();
 }
 
 void DevToolsAndroidBridge::RemoveDeviceListListener(
@@ -791,14 +790,17 @@ void DevToolsAndroidBridge::RemoveDeviceListListener(
   DCHECK(it != device_list_listeners_.end());
   device_list_listeners_.erase(it);
   if (device_list_listeners_.empty())
-    devices_.clear();
+    StopDeviceListPolling();
 }
 
 void DevToolsAndroidBridge::AddDeviceCountListener(
     DeviceCountListener* listener) {
   device_count_listeners_.push_back(listener);
-  if (device_count_listeners_.size() == 1)
-    RequestDeviceCount();
+  if (device_count_listeners_.size() == 1) {
+    device_count_callback_.Reset(
+        base::Bind(&DevToolsAndroidBridge::ReceivedDeviceCount, this));
+    RequestDeviceCount(device_count_callback_.callback());

[Vladislav Kaznacheev, 2014/06/02 18:09:43]

I do not see the reason not to extract the 3 lines above into
StartDeviceCountPolling method.

[vkuzkokov, 2014/06/03 09:20:50]

Done.

+  }
 }
 
 void DevToolsAndroidBridge::RemoveDeviceCountListener(
@@ -808,6 +810,8 @@ void DevToolsAndroidBridge::RemoveDeviceCountListener(
       device_count_listeners_.begin(), device_count_listeners_.end(), listener);
   DCHECK(it != device_count_listeners_.end());
   device_count_listeners_.erase(it);
+  if (device_count_listeners_.empty())
+    device_count_callback_.Cancel();

[Vladislav Kaznacheev, 2014/06/02 18:09:43]

Although it is just a single line, it would look cleaner extracted into
StopDeviceCountPolling method.

[vkuzkokov, 2014/06/03 09:20:50]

Done.

 }
 
 // static
@@ -822,62 +826,85 @@ DevToolsAndroidBridge::~DevToolsAndroidBridge() {
   DCHECK(device_count_listeners_.empty());
 }
 
-void DevToolsAndroidBridge::RequestDeviceList() {
+void DevToolsAndroidBridge::StartDeviceListPolling() {
+  device_list_callback_.Reset(
+    base::Bind(&DevToolsAndroidBridge::ReceivedDeviceList, this));
+  RequestDeviceList(device_list_callback_.callback());
+}
+
+void DevToolsAndroidBridge::StopDeviceListPolling() {
+  device_list_callback_.Cancel();
+  devices_.clear();
+}
+
+void DevToolsAndroidBridge::RequestDeviceList(
+    const base::Callback<void(const RemoteDevices&)>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (device_list_listeners_.empty())
+  if (device_list_listeners_.empty() ||
+      !callback.Equals(device_list_callback_.callback()))
     return;
 
-  new DiscoveryRequest(
-      device_manager_.get(),
-      base::Bind(&DevToolsAndroidBridge::ReceivedDeviceList, this));
+  new DiscoveryRequest(device_manager_.get(), callback);
 }
 
-void DevToolsAndroidBridge::ReceivedDeviceList(
-    scoped_ptr<RemoteDevices> devices) {
+void DevToolsAndroidBridge::ReceivedDeviceList(const RemoteDevices& devices) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  DeviceListListeners copy(device_list_listeners_);
+  for (DeviceListListeners::iterator it = copy.begin(); it != copy.end(); ++it)
+    (*it)->DeviceListChanged(devices);
+
   if (device_list_listeners_.empty())
     return;
 
-  devices_ = *devices;
-
-  DeviceListListeners copy(device_list_listeners_);
-  for (DeviceListListeners::iterator it = copy.begin(); it != copy.end(); ++it)
-    (*it)->DeviceListChanged(*devices.get());
+  devices_ = devices;
 
   BrowserThread::PostDelayedTask(
       BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&DevToolsAndroidBridge::RequestDeviceList, this),
+      base::Bind(&DevToolsAndroidBridge::RequestDeviceList,
+                 this, device_list_callback_.callback()),
       base::TimeDelta::FromMilliseconds(kAdbPollingIntervalMs));
 }
 
-void DevToolsAndroidBridge::RequestDeviceCount() {
+void DevToolsAndroidBridge::RequestDeviceCount(
+    const base::Callback<void(int)>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (device_count_listeners_.empty())
+  if (device_count_listeners_.empty() ||
+      !callback.Equals(device_count_callback_.callback()))
     return;
 
-  UsbDeviceProvider::CountDevices(
-      base::Bind(&DevToolsAndroidBridge::ReceivedDeviceCount, this));
+  UsbDeviceProvider::CountDevices(callback);
 }
 
 void DevToolsAndroidBridge::ReceivedDeviceCount(int count) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (device_count_listeners_.empty())
-     return;
-
   DeviceCountListeners copy(device_count_listeners_);
   for (DeviceCountListeners::iterator it = copy.begin(); it != copy.end(); ++it)
     (*it)->DeviceCountChanged(count);
 
-  BrowserThread::PostDelayedTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&DevToolsAndroidBridge::RequestDeviceCount, this),
-      base::TimeDelta::FromMilliseconds(kAdbPollingIntervalMs));
+  if (device_count_listeners_.empty())
+     return;
+
+  ScheduleDeviceCountRequest(
+      base::Bind(&DevToolsAndroidBridge::RequestDeviceCount,
+                 this, device_count_callback_.callback()));
+}
+
+void DevToolsAndroidBridge::ScheduleDeviceCountRequest(
+    const base::Closure& callback) {
+  if (!device_count_scheduler_for_test_.is_null()) {

[Vladislav Kaznacheev, 2014/06/02 18:09:43]

I have an idea:
1. Call this member |task_scheduler_|
2. Initialize it with a shortcut to PostDelayedTask
3. Use it directly here and when scheduling device list request (you are going
to need that for device list test anyway).

[vkuzkokov, 2014/06/03 09:20:50]

Done.

+    device_count_scheduler_for_test_.Run(callback);
+  } else {
+    BrowserThread::PostDelayedTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        callback,
+        base::TimeDelta::FromMilliseconds(kAdbPollingIntervalMs));
+  }
 }
 
 void DevToolsAndroidBridge::CreateDeviceProviders() {
@@ -901,4 +928,8 @@ void DevToolsAndroidBridge::CreateDeviceProviders() {
     device_providers.push_back(new UsbDeviceProvider(profile_));
   }
   device_manager_->SetDeviceProviders(device_providers);
+  if (!device_list_listeners_.empty()) {
+    StopDeviceListPolling();
+    StartDeviceListPolling();
+  }
 }