Return result from HidService::GetDevices asynchronously.

On Linux HID enumeration happens asynchronously. This change updates
HidService and HidDeviceManager to track an asynchronous enumeration
and not return any results to API clients until the first enumeration
has been completed.

In preparation for broadcasting device add/remove events to apps this
change also switches HidDeviceManager to maintaining its resource ID
mappings using a HidService::Observer. This also made the switch to an
asynchronous GetApiDevices API easier.

BUG=376719

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

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

device/device_tests.gyp

@@ -54,7 +54,6 @@
         'hid/hid_connection_unittest.cc',
         'hid/hid_device_filter_unittest.cc',
         'hid/hid_report_descriptor_unittest.cc',
-        'hid/hid_service_unittest.cc',
         'hid/input_service_linux_unittest.cc',
         'serial/data_sink_unittest.cc',
         'serial/data_source_unittest.cc',

device/hid/hid_connection_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
+#include "base/scoped_observer.h"
 #include "device/hid/hid_connection.h"
 #include "device/hid/hid_service.h"
 #include "device/test/usb_test_gadget.h"
@@ -21,6 +22,54 @@ namespace {
 
 using net::IOBufferWithSize;
 
+// Helper class that can be used to block until a HID device with a particular
+// serial number is available. Example usage:
+//
+//   DeviceCatcher device_catcher("ABC123");
+//   HidDeviceId device_id = device_catcher.WaitForDevice();
+//   /* Call HidService::Connect(device_id) to open the device. */
+//
+class DeviceCatcher : HidService::Observer {
+ public:
+  DeviceCatcher(const std::string& serial_number)
+      : serial_number_(serial_number), observer_(this) {
+    HidService* hid_service = HidService::GetInstance(
+        base::MessageLoop::current()->message_loop_proxy());
+    observer_.Add(hid_service);
+    hid_service->GetDevices(base::Bind(&DeviceCatcher::OnEnumerationComplete,
+                                       base::Unretained(this)));
+  }
+
+  const HidDeviceId& WaitForDevice() {
+    run_loop_.Run();
+    observer_.RemoveAll();
+    return device_id_;
+  }
+
+ private:
+  void OnEnumerationComplete(const std::vector<HidDeviceInfo>& devices) {
+    for (const HidDeviceInfo& device_info : devices) {
+      if (device_info.serial_number == serial_number_) {
+        device_id_ = device_info.device_id;
+        run_loop_.Quit();
+        break;
+      }
+    }
+  }
+
+  void OnDeviceAdded(const HidDeviceInfo& device_info) override {
+    if (device_info.serial_number == serial_number_) {
+      device_id_ = device_info.device_id;
+      run_loop_.Quit();
+    }
+  }
+
+  std::string serial_number_;
+  ScopedObserver<device::HidService, device::HidService::Observer> observer_;
+  base::RunLoop run_loop_;
+  HidDeviceId device_id_;
+};
+
 class TestConnectCallback {
  public:
   TestConnectCallback()
@@ -105,43 +154,11 @@ class HidConnectionTest : public testing::Test {
     ASSERT_TRUE(test_gadget_);
     ASSERT_TRUE(test_gadget_->SetType(UsbTestGadget::HID_ECHO));
 
-    device_id_ = kInvalidHidDeviceId;
-
-    base::RunLoop run_loop;
-    message_loop_->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&HidConnectionTest::FindDevice,
-                   base::Unretained(this), run_loop.QuitClosure(), 5),
-        base::TimeDelta::FromMilliseconds(250));
-    run_loop.Run();
-
+    DeviceCatcher device_catcher(test_gadget_->GetSerialNumber());
+    device_id_ = device_catcher.WaitForDevice();
     ASSERT_NE(device_id_, kInvalidHidDeviceId);
   }
 
-  void FindDevice(const base::Closure& done, int retries) {
-    std::vector<HidDeviceInfo> devices;
-    service_->GetDevices(&devices);
-
-    for (std::vector<HidDeviceInfo>::iterator it = devices.begin();
-         it != devices.end();
-         ++it) {
-      if (it->serial_number == test_gadget_->GetSerialNumber()) {
-        device_id_ = it->device_id;
-        break;
-      }
-    }
-
-    if (device_id_ == kInvalidHidDeviceId && --retries > 0) {
-      message_loop_->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&HidConnectionTest::FindDevice, base::Unretained(this),
-                     done, retries),
-          base::TimeDelta::FromMilliseconds(10));
-    } else {
-      message_loop_->PostTask(FROM_HERE, done);
-    }
-  }
-
   scoped_ptr<base::MessageLoopForIO> message_loop_;
   HidService* service_;
   scoped_ptr<UsbTestGadget> test_gadget_;

device/hid/hid_service.cc

@@ -4,6 +4,7 @@
 
 #include "device/hid/hid_service.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
@@ -71,13 +72,17 @@ HidService::~HidService() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
-void HidService::GetDevices(std::vector<HidDeviceInfo>* devices) {
+void HidService::GetDevices(const GetDevicesCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  STLClearObject(devices);
-  for (DeviceMap::iterator it = devices_.begin();
-      it != devices_.end();
-      ++it) {
-    devices->push_back(it->second);
+  if (enumeration_ready_) {
+    std::vector<HidDeviceInfo> devices;
+    for (const auto& map_entry : devices_) {
+      devices.push_back(map_entry.second);
+    }
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+                                           base::Bind(callback, devices));
+  } else {
+    pending_enumerations_.push_back(callback);
   }
 }
 
@@ -100,23 +105,44 @@ bool HidService::GetDeviceInfo(const HidDeviceId& device_id,
   return true;
 }
 
-HidService::HidService() {
+HidService::HidService() : enumeration_ready_(false) {
 }
 
 void HidService::AddDevice(const HidDeviceInfo& info) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!ContainsKey(devices_, info.device_id)) {
     devices_[info.device_id] = info;
-    FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceAdded(info));
+
+    if (enumeration_ready_) {
+      FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceAdded(info));
+    }
   }
 }
 
 void HidService::RemoveDevice(const HidDeviceId& device_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DeviceMap::iterator it = devices_.find(device_id);
   if (it != devices_.end()) {
+    if (enumeration_ready_) {
+      FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(it->second));
+    }
     devices_.erase(it);
-    FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device_id));
+  }
+}
+
+void HidService::FirstEnumerationComplete() {
+  enumeration_ready_ = true;
+
+  if (!pending_enumerations_.empty()) {
+    std::vector<HidDeviceInfo> devices;
+    for (const auto& map_entry : devices_) {
+      devices.push_back(map_entry.second);
+    }
+
+    for (const GetDevicesCallback& callback : pending_enumerations_) {
+      callback.Run(devices);
+    }
+    pending_enumerations_.clear();
   }
 }
 

device/hid/hid_service.h

@@ -19,24 +19,32 @@ namespace device {
 
 class HidConnection;
 
+// The HidService keeps track of human interface devices connected to the
+// system. Call HidService::GetInstance to get the singleton instance.
 class HidService {
  public:
   class Observer {
    public:
     virtual void OnDeviceAdded(const HidDeviceInfo& info) {}
-    virtual void OnDeviceRemoved(const HidDeviceId& device_id) {}
+    virtual void OnDeviceRemoved(const HidDeviceInfo& info) {}
   };
 
+  typedef base::Callback<void(const std::vector<HidDeviceInfo>&)>
+      GetDevicesCallback;
   typedef base::Callback<void(scoped_refptr<HidConnection> connection)>
       ConnectCallback;
 
+  // Gets a pointer to the HidService singleton. This function should be called
+  // on a thread with a MessageLoopForUI and be passed the task runner for a
+  // thread with a MessageLoopForIO.
   static HidService* GetInstance(
       scoped_refptr<base::SingleThreadTaskRunner> file_task_runner);
 
   static void SetInstanceForTest(HidService* instance);
 
-  // Enumerates and returns a list of device identifiers.
-  virtual void GetDevices(std::vector<HidDeviceInfo>* devices);
+  // Enumerates available devices. The provided callback will always be posted
+  // to the calling thread's task runner.
+  virtual void GetDevices(const GetDevicesCallback& callback);
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -60,6 +68,7 @@ class HidService {
 
   void AddDevice(const HidDeviceInfo& info);
   void RemoveDevice(const HidDeviceId& device_id);
+  void FirstEnumerationComplete();
 
   const DeviceMap& devices() const { return devices_; }
 
@@ -69,6 +78,8 @@ class HidService {
   class Destroyer;
 
   DeviceMap devices_;
+  bool enumeration_ready_;
+  std::vector<GetDevicesCallback> pending_enumerations_;
   ObserverList<Observer> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(HidService);

device/hid/hid_service_linux.cc

@@ -70,6 +70,9 @@ class HidServiceLinux::Helper : public DeviceMonitorLinux::Observer,
     observer_.Add(monitor);
     monitor->Enumerate(
         base::Bind(&Helper::OnDeviceAdded, base::Unretained(this)));
+    task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(&HidServiceLinux::FirstEnumerationComplete, service_));
   }
 
   virtual ~Helper() {

device/hid/hid_service_mac.cc

@@ -213,6 +213,7 @@ HidServiceMac::HidServiceMac(
   // Drain devices_added_iterator_ to arm the notification.
   devices_removed_iterator_.reset(iterator);
   RemoveDevices();
+  FirstEnumerationComplete();
 }
 
 HidServiceMac::~HidServiceMac() {

device/hid/hid_service_win.cc

@@ -152,6 +152,8 @@ void HidServiceWin::DoInitialEnumeration() {
       PlatformAddDevice(device_path);
     }
   }
+
+  FirstEnumerationComplete();
 }
 
 // static

extensions/browser/api/hid/hid_api.cc

@@ -89,8 +89,15 @@ ExtensionFunction::ResponseAction HidGetDevicesFunction::Run() {
     filters.push_back(legacy_filter);
   }
 
-  return RespondNow(OneArgument(
-      device_manager->GetApiDevices(extension(), filters).release()));
+  device_manager->GetApiDevices(
+      extension(), filters,
+      base::Bind(&HidGetDevicesFunction::OnEnumerationComplete, this));
+  return RespondLater();
+}
+
+void HidGetDevicesFunction::OnEnumerationComplete(
+    scoped_ptr<base::ListValue> devices) {
+  Respond(OneArgument(devices.release()));
 }
 
 HidConnectFunction::HidConnectFunction() : connection_manager_(nullptr) {

extensions/browser/api/hid/hid_api.h

@@ -38,6 +38,8 @@ class HidGetDevicesFunction : public UIThreadExtensionFunction {
   // ExtensionFunction:
   ResponseAction Run() override;
 
+  void OnEnumerationComplete(scoped_ptr<base::ListValue> devices);
+
   DISALLOW_COPY_AND_ASSIGN(HidGetDevicesFunction);
 };