[serial] Add monitor for device add/remove events on Windows

This change removes the old serial enumeration code on Windows and
always uses the new enumeration path while listening to window messages
to discover when ports are added and removed from the system.

This will allow us to keep the UI up-to-date in real time when the user
plugs in a device and deliver connection events to web apps.

Bug: 981483
Change-Id: I4126e370b02973458b5c2aac6a230244248b2320
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099238
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751217}

base/win/windows_types.h

@@ -93,6 +93,7 @@ CHROME_DECLARE_HANDLE(HWND);
 typedef LPVOID HINTERNET;
 typedef HINSTANCE HMODULE;
 typedef PVOID LSA_HANDLE;
+typedef PVOID HDEVINFO;
 
 // Forward declare some Windows struct/typedef sets.
 
@@ -114,6 +115,8 @@ typedef struct tagMENUITEMINFOW MENUITEMINFOW, MENUITEMINFO;
 
 typedef struct tagNMHDR NMHDR;
 
+typedef struct _SP_DEVINFO_DATA SP_DEVINFO_DATA;
+
 typedef PVOID PSID;
 
 // Declare Chrome versions of some Windows structures. These are needed for

services/device/serial/serial_device_enumerator.cc

@@ -7,9 +7,32 @@
 #include <utility>
 
 #include "base/unguessable_token.h"
+#include "build/build_config.h"
+
+#if defined(OS_LINUX)
+#include "services/device/serial/serial_device_enumerator_linux.h"
+#elif defined(OS_MACOSX)
+#include "services/device/serial/serial_device_enumerator_mac.h"
+#elif defined(OS_WIN)
+#include "services/device/serial/serial_device_enumerator_win.h"
+#endif
 
 namespace device {
 
+// static
+std::unique_ptr<SerialDeviceEnumerator> SerialDeviceEnumerator::Create(
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
+#if defined(OS_LINUX)
+  return std::make_unique<SerialDeviceEnumeratorLinux>();
+#elif defined(OS_MACOSX)
+  return std::make_unique<SerialDeviceEnumeratorMac>();
+#elif defined(OS_WIN)
+  return std::make_unique<SerialDeviceEnumeratorWin>(std::move(ui_task_runner));
+#else
+#error "No implementation of SerialDeviceEnumerator on this platform."
+#endif
+}
+
 SerialDeviceEnumerator::SerialDeviceEnumerator() = default;
 
 SerialDeviceEnumerator::~SerialDeviceEnumerator() = default;

services/device/serial/serial_device_enumerator.h

@@ -24,7 +24,8 @@ class SerialDeviceEnumerator {
  public:
   using TokenPathMap = std::map<base::UnguessableToken, base::FilePath>;
 
-  static std::unique_ptr<SerialDeviceEnumerator> Create();
+  static std::unique_ptr<SerialDeviceEnumerator> Create(
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
 
   SerialDeviceEnumerator();
   virtual ~SerialDeviceEnumerator();

services/device/serial/serial_device_enumerator_linux.cc

@@ -33,11 +33,6 @@ const char kRfcommMajor[] = "216";
 
 }  // namespace
 
-// static
-std::unique_ptr<SerialDeviceEnumerator> SerialDeviceEnumerator::Create() {
-  return std::make_unique<SerialDeviceEnumeratorLinux>();
-}
-
 SerialDeviceEnumeratorLinux::SerialDeviceEnumeratorLinux() {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 

services/device/serial/serial_device_enumerator_mac.cc

@@ -104,11 +104,6 @@ bool GetUInt16Property(io_service_t service,
 
 }  // namespace
 
-// static
-std::unique_ptr<SerialDeviceEnumerator> SerialDeviceEnumerator::Create() {
-  return std::make_unique<SerialDeviceEnumeratorMac>();
-}
-
 SerialDeviceEnumeratorMac::SerialDeviceEnumeratorMac() {
   notify_port_.reset(IONotificationPortCreate(kIOMasterPortDefault));
   CFRunLoopAddSource(CFRunLoopGetMain(),

services/device/serial/serial_device_enumerator_unittest.cc

@@ -19,7 +19,7 @@ class SerialDeviceEnumeratorTest : public testing::Test {
       : task_environment_(base::test::TaskEnvironment::MainThreadType::IO) {}
   ~SerialDeviceEnumeratorTest() override = default;
 
- private:
+ protected:
   base::test::TaskEnvironment task_environment_;
 };
 
@@ -27,7 +27,8 @@ TEST_F(SerialDeviceEnumeratorTest, GetDevices) {
   // There is no guarantee that a test machine will have a serial device
   // available. The purpose of this test is to ensure that the process of
   // attempting to enumerate devices does not cause a crash.
-  auto enumerator = SerialDeviceEnumerator::Create();
+  auto enumerator = SerialDeviceEnumerator::Create(
+      task_environment_.GetMainThreadTaskRunner());
   ASSERT_TRUE(enumerator);
   std::vector<mojom::SerialPortInfoPtr> devices = enumerator->GetDevices();
 }

services/device/serial/serial_device_enumerator_win.cc

@@ -7,17 +7,19 @@
 #include <windows.h>  // Must be in front of other Windows header files.
 
 #include <devguid.h>
+#include <ntddser.h>
 #include <setupapi.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
 #include <string>
-#include <unordered_set>
 #include <utility>
 
 #include "base/metrics/histogram_functions.h"
 #include "base/numerics/ranges.h"
+#include "base/scoped_generic.h"
+#include "base/sequence_checker.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -30,29 +32,38 @@ namespace device {
 
 namespace {
 
+struct DevInfoScopedTraits {
+  static HDEVINFO InvalidValue() { return INVALID_HANDLE_VALUE; }
+  static void Free(HDEVINFO h) { SetupDiDestroyDeviceInfoList(h); }
+};
+
+using ScopedDevInfo = base::ScopedGeneric<HDEVINFO, DevInfoScopedTraits>;
+
 // Searches the specified device info for a property with the specified key,
 // assigns the result to value, and returns whether the operation was
 // successful.
 bool GetProperty(HDEVINFO dev_info,
-                 SP_DEVINFO_DATA dev_info_data,
+                 SP_DEVINFO_DATA* dev_info_data,
                  const int key,
                  std::string* value) {
   // We don't know how much space the property's value will take up, so we call
   // the property retrieval function once to fetch the size of the required
   // value buffer, then again once we've allocated a sufficiently large buffer.
   DWORD buffer_size = 0;
-  SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data, key, nullptr,
+  SetupDiGetDeviceRegistryProperty(dev_info, dev_info_data, key, nullptr,
                                    nullptr, buffer_size, &buffer_size);
   if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
     return false;
 
-  std::unique_ptr<wchar_t[]> buffer(new wchar_t[buffer_size]);
-  if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data, key, nullptr,
-                                        reinterpret_cast<PBYTE>(buffer.get()),
-                                        buffer_size, nullptr))
+  base::string16 buffer;
+  if (!SetupDiGetDeviceRegistryProperty(
+          dev_info, dev_info_data, key, nullptr,
+          reinterpret_cast<PBYTE>(base::WriteInto(&buffer, buffer_size)),
+          buffer_size, nullptr)) {
     return false;
+  }
 
-  *value = base::WideToUTF8(buffer.get());
+  *value = base::UTF16ToUTF8(buffer);
   return true;
 }
 
@@ -92,40 +103,71 @@ bool GetProductID(const std::string hardware_id, uint32_t* product_id) {
 
 }  // namespace
 
-// static
-std::unique_ptr<SerialDeviceEnumerator> SerialDeviceEnumerator::Create() {
-  return std::make_unique<SerialDeviceEnumeratorWin>();
-}
+class SerialDeviceEnumeratorWin::UiThreadHelper
+    : public DeviceMonitorWin::Observer {
+ public:
+  UiThreadHelper() : task_runner_(base::SequencedTaskRunnerHandle::Get()) {
+    DETACH_FROM_SEQUENCE(sequence_checker_);
+  }
 
-SerialDeviceEnumeratorWin::SerialDeviceEnumeratorWin() {}
+  // Disallow copy and assignment.
+  UiThreadHelper(UiThreadHelper&) = delete;
+  UiThreadHelper& operator=(UiThreadHelper&) = delete;
 
-SerialDeviceEnumeratorWin::~SerialDeviceEnumeratorWin() {}
+  virtual ~UiThreadHelper() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  }
 
-std::vector<mojom::SerialPortInfoPtr> SerialDeviceEnumeratorWin::GetDevices() {
-  std::vector<mojom::SerialPortInfoPtr> devices = GetDevicesNew();
-  std::vector<mojom::SerialPortInfoPtr> old_devices = GetDevicesOld();
-
-  base::UmaHistogramSparse(
-      "Hardware.Serial.NewMinusOldDeviceListSize",
-      base::ClampToRange<int>(devices.size() - old_devices.size(), -10, 10));
-
-  // Add devices found from both the new and old methods of enumeration. If a
-  // device is found using both the new and the old enumeration method, then we
-  // take the device from the new enumeration method because it's able to
-  // collect more information. We do this by inserting the new devices first,
-  // because insertions are ignored if the key already exists.
-  std::unordered_set<base::FilePath> devices_seen;
-  for (const auto& device : devices) {
-    bool inserted = devices_seen.insert(device->path).second;
-    DCHECK(inserted);
+  void Initialize(base::WeakPtr<SerialDeviceEnumeratorWin> enumerator) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    enumerator_ = std::move(enumerator);
+    device_observer_.Add(
+        DeviceMonitorWin::GetForDeviceInterface(GUID_DEVINTERFACE_COMPORT));
+  }
+
+  void OnDeviceAdded(const GUID& class_guid,
+                     const base::string16& device_path) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&SerialDeviceEnumeratorWin::OnPathAdded,
+                                  enumerator_, device_path));
   }
-  for (auto& device : old_devices) {
-    if (devices_seen.insert(device->path).second)
-      devices.push_back(std::move(device));
+
+  void OnDeviceRemoved(const GUID& class_guid,
+                       const base::string16& device_path) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&SerialDeviceEnumeratorWin::OnPathRemoved,
+                                  enumerator_, device_path));
   }
-  return devices;
+
+ private:
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  // Weak reference to the SerialDeviceEnumeratorWin that owns this object.
+  // Calls on |enumerator_| must be posted to |task_runner_|.
+  base::WeakPtr<SerialDeviceEnumeratorWin> enumerator_;
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  ScopedObserver<DeviceMonitorWin, DeviceMonitorWin::Observer> device_observer_{
+      this};
+};
+
+SerialDeviceEnumeratorWin::SerialDeviceEnumeratorWin(
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
+    : helper_(new UiThreadHelper(), base::OnTaskRunnerDeleter(ui_task_runner)) {
+  // Passing a raw pointer to |helper_| is safe here because this task will
+  // reach the UI thread before any task to delete |helper_|.
+  ui_task_runner->PostTask(FROM_HERE,
+                           base::BindOnce(&UiThreadHelper::Initialize,
+                                          base::Unretained(helper_.get()),
+                                          weak_factory_.GetWeakPtr()));
+
+  DoInitialEnumeration();
 }
 
+SerialDeviceEnumeratorWin::~SerialDeviceEnumeratorWin() = default;
+
 // static
 base::Optional<base::FilePath> SerialDeviceEnumeratorWin::GetPath(
     const std::string& friendly_name) {
@@ -136,40 +178,114 @@ base::Optional<base::FilePath> SerialDeviceEnumeratorWin::GetPath(
   return FixUpPortName(com_port);
 }
 
-// Returns an array of devices as retrieved through the new method of
-// enumerating serial devices (SetupDi).  This new method gives more information
-// about the devices than the old method.
-std::vector<mojom::SerialPortInfoPtr>
-SerialDeviceEnumeratorWin::GetDevicesNew() {
-  std::vector<mojom::SerialPortInfoPtr> devices;
+std::vector<mojom::SerialPortInfoPtr> SerialDeviceEnumeratorWin::GetDevices() {
+  std::vector<mojom::SerialPortInfoPtr> ports;
+  ports.reserve(ports_.size());
+  for (const auto& map_entry : ports_)
+    ports.push_back(map_entry.second->Clone());
+  return ports;
+}
+
+base::Optional<base::FilePath> SerialDeviceEnumeratorWin::GetPathFromToken(
+    const base::UnguessableToken& token) {
+  auto it = ports_.find(token);
+  if (it == ports_.end())
+    return base::nullopt;
+  return it->second->path;
+}
+
+void SerialDeviceEnumeratorWin::OnPathAdded(const base::string16& device_path) {
+  ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr));
+  if (!dev_info.is_valid())
+    return;
+
+  if (!SetupDiOpenDeviceInterface(dev_info.get(), device_path.c_str(), 0,
+                                  nullptr)) {
+    return;
+  }
+
+  SP_DEVINFO_DATA dev_info_data = {};
+  dev_info_data.cbSize = sizeof(dev_info_data);
+  if (!SetupDiEnumDeviceInfo(dev_info.get(), 0, &dev_info_data))
+    return;
+
+  EnumeratePort(dev_info.get(), &dev_info_data);
+}
+
+void SerialDeviceEnumeratorWin::OnPathRemoved(
+    const base::string16& device_path) {
+  ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr));
+  if (!dev_info.is_valid())
+    return;
+
+  if (!SetupDiOpenDeviceInterface(dev_info.get(), device_path.c_str(), 0,
+                                  nullptr)) {
+    return;
+  }
+
+  SP_DEVINFO_DATA dev_info_data = {};
+  dev_info_data.cbSize = sizeof(dev_info_data);
+  if (!SetupDiEnumDeviceInfo(dev_info.get(), 0, &dev_info_data))
+    return;
+
+  std::string friendly_name;
+  // SPDRP_FRIENDLYNAME looks like "USB_SERIAL_PORT (COM3)".
+  // In Windows, the COM port is the path used to uniquely identify the
+  // serial device. If the COM can't be found, ignore the device.
+  if (!GetProperty(dev_info.get(), &dev_info_data, SPDRP_FRIENDLYNAME,
+                   &friendly_name)) {
+    return;
+  }
+
+  base::Optional<base::FilePath> path = GetPath(friendly_name);
+  if (!path)
+    return;
+
+  auto it = paths_.find(*path);
+  if (it == paths_.end())
+    return;
 
+  ports_.erase(it->second);
+  paths_.erase(it);
+}
+
+void SerialDeviceEnumeratorWin::DoInitialEnumeration() {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
   // Make a device interface query to find all serial devices.
-  HDEVINFO dev_info =
-      SetupDiGetClassDevs(&GUID_DEVCLASS_PORTS, 0, 0, DIGCF_PRESENT);
-  if (dev_info == INVALID_HANDLE_VALUE)
-    return devices;
-
-  SP_DEVINFO_DATA dev_info_data;
-  dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
-  for (DWORD i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
+  ScopedDevInfo dev_info(
+      SetupDiGetClassDevs(&GUID_DEVINTERFACE_COMPORT, nullptr, 0,
+                          DIGCF_DEVICEINTERFACE | DIGCF_PRESENT));
+  if (!dev_info.is_valid())
+    return;
+
+  SP_DEVINFO_DATA dev_info_data = {};
+  dev_info_data.cbSize = sizeof(dev_info_data);
+  for (DWORD i = 0; SetupDiEnumDeviceInfo(dev_info.get(), i, &dev_info_data);
+       i++) {
+    EnumeratePort(dev_info.get(), &dev_info_data);
+  }
+}
+
+void SerialDeviceEnumeratorWin::EnumeratePort(HDEVINFO dev_info,
+                                              SP_DEVINFO_DATA* dev_info_data) {
   std::string friendly_name;
   // SPDRP_FRIENDLYNAME looks like "USB_SERIAL_PORT (COM3)".
   // In Windows, the COM port is the path used to uniquely identify the
   // serial device. If the COM can't be found, ignore the device.
   if (!GetProperty(dev_info, dev_info_data, SPDRP_FRIENDLYNAME,
                    &friendly_name)) {
-      continue;
+    return;
   }
 
   base::Optional<base::FilePath> path = GetPath(friendly_name);
   if (!path)
-      continue;
+    return;
 
   auto info = mojom::SerialPortInfo::New();
   info->path = *path;
-    info->token = GetTokenFromPath(info->path);
+  base::UnguessableToken token = base::UnguessableToken::Create();
+  info->token = token;
 
   std::string display_name;
   if (GetDisplayName(friendly_name, &display_name))
@@ -189,30 +305,8 @@ SerialDeviceEnumeratorWin::GetDevicesNew() {
     }
   }
 
-    devices.push_back(std::move(info));
-  }
-
-  SetupDiDestroyDeviceInfoList(dev_info);
-  return devices;
-}
-
-// Returns an array of devices as retrieved through the old method of
-// enumerating serial devices (searching the registry). This old method gives
-// less information about the devices than the new method.
-std::vector<mojom::SerialPortInfoPtr>
-SerialDeviceEnumeratorWin::GetDevicesOld() {
-  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
-                                                base::BlockingType::MAY_BLOCK);
-  base::win::RegistryValueIterator iter_key(
-      HKEY_LOCAL_MACHINE, L"HARDWARE\\DEVICEMAP\\SERIALCOMM\\");
-  std::vector<mojom::SerialPortInfoPtr> devices;
-  for (; iter_key.Valid(); ++iter_key) {
-    auto info = mojom::SerialPortInfo::New();
-    info->path = FixUpPortName(base::UTF16ToASCII(iter_key.Value()));
-    info->token = GetTokenFromPath(info->path);
-    devices.push_back(std::move(info));
-  }
-  return devices;
+  ports_[token] = std::move(info);
+  paths_.insert(std::make_pair(*path, token));
 }
 
 }  // namespace device

services/device/serial/serial_device_enumerator_win.h

@@ -5,10 +5,10 @@
 #ifndef SERVICES_DEVICE_SERIAL_SERIAL_DEVICE_ENUMERATOR_WIN_H_
 #define SERVICES_DEVICE_SERIAL_SERIAL_DEVICE_ENUMERATOR_WIN_H_
 
-#include <vector>
-
 #include "base/macros.h"
-#include "base/optional.h"
+#include "base/scoped_observer.h"
+#include "base/win/windows_types.h"
+#include "device/base/device_monitor_win.h"
 #include "services/device/serial/serial_device_enumerator.h"
 
 namespace device {
@@ -16,21 +16,35 @@ namespace device {
 // Discovers and enumerates serial devices available to the host.
 class SerialDeviceEnumeratorWin : public SerialDeviceEnumerator {
  public:
-  SerialDeviceEnumeratorWin();
+  SerialDeviceEnumeratorWin(
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
   ~SerialDeviceEnumeratorWin() override;
 
-  // Implementation for SerialDeviceEnumerator.
-  std::vector<mojom::SerialPortInfoPtr> GetDevices() override;
-
   // Searches for the COM port in the device's friendly name and returns the
   // appropriate device path or nullopt if the input did not contain a valid
   // name.
   static base::Optional<base::FilePath> GetPath(
       const std::string& friendly_name);
 
+  // SerialDeviceEnumerator
+  std::vector<mojom::SerialPortInfoPtr> GetDevices() override;
+  base::Optional<base::FilePath> GetPathFromToken(
+      const base::UnguessableToken& token) override;
+
+  void OnPathAdded(const base::string16& device_path);
+  void OnPathRemoved(const base::string16& device_path);
+
  private:
-  std::vector<mojom::SerialPortInfoPtr> GetDevicesNew();
-  std::vector<mojom::SerialPortInfoPtr> GetDevicesOld();
+  class UiThreadHelper;
+
+  void DoInitialEnumeration();
+  void EnumeratePort(HDEVINFO dev_info, SP_DEVINFO_DATA* dev_info_data);
+
+  std::map<base::UnguessableToken, mojom::SerialPortInfoPtr> ports_;
+  std::map<base::FilePath, base::UnguessableToken> paths_;
+
+  std::unique_ptr<UiThreadHelper, base::OnTaskRunnerDeleter> helper_;
+  base::WeakPtrFactory<SerialDeviceEnumeratorWin> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SerialDeviceEnumeratorWin);
 };

services/device/serial/serial_port_manager_impl.cc

@@ -36,7 +36,7 @@ void SerialPortManagerImpl::SetSerialEnumeratorForTesting(
 
 void SerialPortManagerImpl::GetDevices(GetDevicesCallback callback) {
   if (!enumerator_)
-    enumerator_ = SerialDeviceEnumerator::Create();
+    enumerator_ = SerialDeviceEnumerator::Create(ui_task_runner_);
   std::move(callback).Run(enumerator_->GetDevices());
 }
 
@@ -45,7 +45,7 @@ void SerialPortManagerImpl::GetPort(
     mojo::PendingReceiver<mojom::SerialPort> receiver,
     mojo::PendingRemote<mojom::SerialPortConnectionWatcher> watcher) {
   if (!enumerator_)
-    enumerator_ = SerialDeviceEnumerator::Create();
+    enumerator_ = SerialDeviceEnumerator::Create(ui_task_runner_);
   base::Optional<base::FilePath> path = enumerator_->GetPathFromToken(token);
   if (path) {
     io_task_runner_->PostTask(