[hid] Merge top-level collections on Windows

The HID spec allows a device to group its functionality into
separate top-level collections within the HID report
descriptor. On Windows, each top-level collection is
represented in the system registry as a separate logical
device node.

This CL identifies when a device exposes multiple top-level
collections and merges them into a single HIDDevice instance.

Bug: 1009715
Change-Id: I0524fee04b5e530dfa5ff724b57d2442187946be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592149
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839601}

device/gamepad/nintendo_data_fetcher_unittest.cc

@@ -97,11 +97,17 @@ class NintendoDataFetcherTest : public DeviceServiceTestBase {
 
 TEST_F(NintendoDataFetcherTest, UnsupportedDeviceIsIgnored) {
   // Simulate an unsupported, non-Nintendo HID device.
+  HidDeviceInfo::PlatformDeviceIdMap platform_device_id_map;
+  platform_device_id_map.emplace_back(base::flat_set<uint8_t>{0},
+                                      kTestDeviceId);
+  std::vector<mojom::HidCollectionInfoPtr> collections;
   auto collection = mojom::HidCollectionInfo::New();
   collection->usage = mojom::HidUsageAndPage::New(0, 0);
+  collections.push_back(std::move(collection));
   scoped_refptr<HidDeviceInfo> device_info(new HidDeviceInfo(
-      kTestDeviceId, kPhysicalDeviceId, 0x1234, 0xabcd, "Invalipad", "",
-      mojom::HidBusType::kHIDBusTypeUSB, std::move(collection), 0, 0, 0));
+      std::move(platform_device_id_map), kPhysicalDeviceId, 0x1234, 0xabcd,
+      "Invalipad", "", mojom::HidBusType::kHIDBusTypeUSB,
+      std::move(collections), 0, 0, 0));
 
   // Add the device to the mock HID service. The HID service should notify the
   // data fetcher.
@@ -118,11 +124,17 @@ TEST_F(NintendoDataFetcherTest, UnsupportedDeviceIsIgnored) {
 
 TEST_F(NintendoDataFetcherTest, AddAndRemoveSwitchPro) {
   // Simulate a Switch Pro over USB.
+  HidDeviceInfo::PlatformDeviceIdMap platform_device_id_map;
+  platform_device_id_map.emplace_back(base::flat_set<uint8_t>{0},
+                                      kTestDeviceId);
+  std::vector<mojom::HidCollectionInfoPtr> collections;
   auto collection = mojom::HidCollectionInfo::New();
   collection->usage = mojom::HidUsageAndPage::New(0, 0);
+  collections.push_back(std::move(collection));
   scoped_refptr<HidDeviceInfo> device_info(new HidDeviceInfo(
-      kTestDeviceId, kPhysicalDeviceId, 0x057e, 0x2009, "Switch Pro Controller",
-      "", mojom::HidBusType::kHIDBusTypeUSB, std::move(collection), 0, 63, 0));
+      std::move(platform_device_id_map), kPhysicalDeviceId, 0x057e, 0x2009,
+      "Switch Pro Controller", "", mojom::HidBusType::kHIDBusTypeUSB,
+      std::move(collections), 0, 63, 0));
 
   // Add the device to the mock HID service. The HID service should notify the
   // data fetcher.

services/device/hid/hid_connection_impl_unittest.cc

@@ -175,13 +175,18 @@ class HidConnectionImplTest : public DeviceServiceTestBase {
   }
 
   scoped_refptr<HidDeviceInfo> CreateTestDevice() {
+    HidDeviceInfo::PlatformDeviceIdMap platform_device_id_map;
+    platform_device_id_map.emplace_back(base::flat_set<uint8_t>{0},
+                                        kTestDeviceId);
+    std::vector<mojom::HidCollectionInfoPtr> collections;
     auto hid_collection_info = mojom::HidCollectionInfo::New();
     hid_collection_info->usage = mojom::HidUsageAndPage::New(0, 0);
     hid_collection_info->report_ids.push_back(kTestReportId);
+    collections.push_back(std::move(hid_collection_info));
     return base::MakeRefCounted<HidDeviceInfo>(
-        kTestDeviceId, "1", 0x1234, 0xabcd, "product name", "serial number",
-        mojom::HidBusType::kHIDBusTypeUSB, std::move(hid_collection_info),
-        kMaxReportSizeBytes, kMaxReportSizeBytes, 0);
+        std::move(platform_device_id_map), "1", 0x1234, 0xabcd, "product name",
+        "serial number", mojom::HidBusType::kHIDBusTypeUSB,
+        std::move(collections), kMaxReportSizeBytes, kMaxReportSizeBytes, 0);
   }
 
   std::vector<uint8_t> CreateTestReportBuffer(uint8_t report_id, size_t size) {

services/device/hid/hid_connection_win.cc

@@ -29,6 +29,21 @@ extern "C" {
 
 namespace device {
 
+namespace {
+
+bool IsValidHandle(HANDLE handle) {
+  return handle != nullptr && handle != INVALID_HANDLE_VALUE;
+}
+
+}  // namespace
+
+HidConnectionWin::HidDeviceEntry::HidDeviceEntry(
+    base::flat_set<uint8_t> report_ids,
+    base::win::ScopedHandle file_handle)
+    : report_ids(std::move(report_ids)), file_handle(std::move(file_handle)) {}
+
+HidConnectionWin::HidDeviceEntry::~HidDeviceEntry() = default;
+
 class PendingHidTransfer : public base::win::ObjectWatcher::Delegate {
  public:
   typedef base::OnceCallback<void(PendingHidTransfer*, bool)> Callback;
@@ -77,7 +92,7 @@ void PendingHidTransfer::TakeResultFromWindowsAPI(BOOL result) {
   } else if (GetLastError() == ERROR_IO_PENDING) {
     watcher_.StartWatchingOnce(event_.Get(), this);
   } else {
-    HID_PLOG(EVENT) << "HID transfer failed";
+    HID_PLOG(DEBUG) << "HID transfer failed";
     std::move(callback_).Run(this, false);
   }
 }
@@ -89,28 +104,33 @@ void PendingHidTransfer::OnObjectSignaled(HANDLE event_handle) {
 // static
 scoped_refptr<HidConnection> HidConnectionWin::Create(
     scoped_refptr<HidDeviceInfo> device_info,
-    base::win::ScopedHandle file,
+    std::vector<std::unique_ptr<HidDeviceEntry>> file_handles,
     bool allow_protected_reports) {
-  scoped_refptr<HidConnectionWin> connection(new HidConnectionWin(
-      std::move(device_info), std::move(file), allow_protected_reports));
-  connection->ReadNextInputReport();
+  scoped_refptr<HidConnectionWin> connection(
+      new HidConnectionWin(std::move(device_info), std::move(file_handles),
+                           allow_protected_reports));
+  connection->SetUpInitialReads();
   return std::move(connection);
 }
 
-HidConnectionWin::HidConnectionWin(scoped_refptr<HidDeviceInfo> device_info,
-                                   base::win::ScopedHandle file,
+HidConnectionWin::HidConnectionWin(
+    scoped_refptr<HidDeviceInfo> device_info,
+    std::vector<std::unique_ptr<HidDeviceEntry>> file_handles,
     bool allow_protected_reports)
     : HidConnection(std::move(device_info), allow_protected_reports),
-      file_(std::move(file)) {}
+      file_handles_(std::move(file_handles)) {}
 
 HidConnectionWin::~HidConnectionWin() {
-  DCHECK(!file_.IsValid());
+  DCHECK(file_handles_.empty());
   DCHECK(transfers_.empty());
 }
 
 void HidConnectionWin::PlatformClose() {
-  CancelIo(file_.Get());
-  file_.Close();
+  for (auto& entry : file_handles_) {
+    CancelIo(entry->file_handle.Get());
+    entry->file_handle.Close();
+  }
+  file_handles_.clear();
   transfers_.clear();
 }
 
@@ -124,11 +144,18 @@ void HidConnectionWin::PlatformWrite(
   DCHECK(buffer->size() <= expected_size);
   buffer->data().resize(expected_size);
 
+  uint8_t report_id = buffer->data()[0];
+  HANDLE file_handle = GetHandleForReportId(report_id);
+  if (!IsValidHandle(file_handle)) {
+    HID_LOG(DEBUG) << "HID write failed due to invalid handle.";
+    std::move(callback).Run(false);
+  }
+
   transfers_.push_back(std::make_unique<PendingHidTransfer>(
       buffer, base::BindOnce(&HidConnectionWin::OnWriteComplete, this,
-                             std::move(callback))));
+                             file_handle, std::move(callback))));
   transfers_.back()->TakeResultFromWindowsAPI(WriteFile(
-      file_.Get(), buffer->front(), static_cast<DWORD>(buffer->size()), NULL,
+      file_handle, buffer->front(), static_cast<DWORD>(buffer->size()), NULL,
       transfers_.back()->GetOverlapped()));
 }
 
@@ -139,11 +166,17 @@ void HidConnectionWin::PlatformGetFeatureReport(uint8_t report_id,
       device_info()->max_feature_report_size() + 1);
   buffer->data()[0] = report_id;
 
+  HANDLE file_handle = GetHandleForReportId(report_id);
+  if (!IsValidHandle(file_handle)) {
+    HID_LOG(DEBUG) << "HID read failed due to invalid handle.";
+    std::move(callback).Run(false, nullptr, 0);
+  }
+
   transfers_.push_back(std::make_unique<PendingHidTransfer>(
       buffer, base::BindOnce(&HidConnectionWin::OnReadFeatureComplete, this,
-                             buffer, std::move(callback))));
+                             file_handle, buffer, std::move(callback))));
   transfers_.back()->TakeResultFromWindowsAPI(
-      DeviceIoControl(file_.Get(), IOCTL_HID_GET_FEATURE, NULL, 0,
+      DeviceIoControl(file_handle, IOCTL_HID_GET_FEATURE, NULL, 0,
                       buffer->front(), static_cast<DWORD>(buffer->size()), NULL,
                       transfers_.back()->GetOverlapped()));
 }
@@ -151,54 +184,65 @@ void HidConnectionWin::PlatformGetFeatureReport(uint8_t report_id,
 void HidConnectionWin::PlatformSendFeatureReport(
     scoped_refptr<base::RefCountedBytes> buffer,
     WriteCallback callback) {
+  uint8_t report_id = buffer->data()[0];
+  HANDLE file_handle = GetHandleForReportId(report_id);
+  if (!IsValidHandle(file_handle)) {
+    HID_LOG(DEBUG) << "HID write failed due to invalid handle.";
+    std::move(callback).Run(false);
+    return;
+  }
+
   // The Windows API always wants either a report ID (if supported) or
   // zero at the front of every output report.
   transfers_.push_back(std::make_unique<PendingHidTransfer>(
       buffer, base::BindOnce(&HidConnectionWin::OnWriteComplete, this,
-                             std::move(callback))));
+                             file_handle, std::move(callback))));
   transfers_.back()->TakeResultFromWindowsAPI(
-      DeviceIoControl(file_.Get(), IOCTL_HID_SET_FEATURE, buffer->front(),
+      DeviceIoControl(file_handle, IOCTL_HID_SET_FEATURE, buffer->front(),
                       static_cast<DWORD>(buffer->size()), NULL, 0, NULL,
                       transfers_.back()->GetOverlapped()));
 }
 
-void HidConnectionWin::ReadNextInputReport() {
+void HidConnectionWin::SetUpInitialReads() {
+  for (const auto& entry : file_handles_)
+    ReadNextInputReportOnHandle(entry->file_handle.Get());
+}
+
+void HidConnectionWin::ReadNextInputReportOnHandle(HANDLE file_handle) {
   // Windows will always include the report ID (including zero if report IDs
   // are not in use) in the buffer.
   auto buffer = base::MakeRefCounted<base::RefCountedBytes>(
       device_info()->max_input_report_size() + 1);
 
   transfers_.push_back(std::make_unique<PendingHidTransfer>(
-      buffer,
-      base::BindOnce(&HidConnectionWin::OnReadInputReport, this, buffer)));
+      buffer, base::BindOnce(&HidConnectionWin::OnReadInputReport, this,
+                             file_handle, buffer)));
   transfers_.back()->TakeResultFromWindowsAPI(
-      ReadFile(file_.Get(), buffer->front(), static_cast<DWORD>(buffer->size()),
+      ReadFile(file_handle, buffer->front(), static_cast<DWORD>(buffer->size()),
                NULL, transfers_.back()->GetOverlapped()));
 }
 
 void HidConnectionWin::OnReadInputReport(
+    HANDLE file_handle,
     scoped_refptr<base::RefCountedBytes> buffer,
     PendingHidTransfer* transfer_raw,
     bool signaled) {
-  if (!file_.IsValid())
-    return;
-
   std::unique_ptr<PendingHidTransfer> transfer = UnlinkTransfer(transfer_raw);
   DWORD bytes_transferred;
-  if (!signaled || !GetOverlappedResult(file_.Get(), transfer->GetOverlapped(),
+  if (!signaled || !GetOverlappedResult(file_handle, transfer->GetOverlapped(),
                                         &bytes_transferred, FALSE)) {
-    HID_PLOG(EVENT) << "HID read failed";
+    HID_PLOG(DEBUG) << "HID read failed";
     return;
   }
 
   if (bytes_transferred < 1) {
-    HID_LOG(EVENT) << "HID read too short.";
+    HID_LOG(DEBUG) << "HID read too short.";
     return;
   }
 
   uint8_t report_id = buffer->data()[0];
   if (IsReportIdProtected(report_id, HidReportType::kInput)) {
-    ReadNextInputReport();
+    ReadNextInputReportOnHandle(file_handle);
     return;
   }
 
@@ -207,46 +251,38 @@ void HidConnectionWin::OnReadInputReport(
   scoped_refptr<HidConnection> self(this);
   ProcessInputReport(buffer, bytes_transferred);
 
-  ReadNextInputReport();
+  ReadNextInputReportOnHandle(file_handle);
 }
 
 void HidConnectionWin::OnReadFeatureComplete(
+    HANDLE file_handle,
     scoped_refptr<base::RefCountedBytes> buffer,
     ReadCallback callback,
     PendingHidTransfer* transfer_raw,
     bool signaled) {
-  if (!file_.IsValid()) {
-    std::move(callback).Run(false, nullptr, 0);
-    return;
-  }
-
   std::unique_ptr<PendingHidTransfer> transfer = UnlinkTransfer(transfer_raw);
   DWORD bytes_transferred;
-  if (signaled && GetOverlappedResult(file_.Get(), transfer->GetOverlapped(),
+  if (signaled && GetOverlappedResult(file_handle, transfer->GetOverlapped(),
                                       &bytes_transferred, FALSE)) {
     DCHECK_LE(bytes_transferred, buffer->size());
     std::move(callback).Run(true, buffer, bytes_transferred);
   } else {
-    HID_PLOG(EVENT) << "HID read failed";
+    HID_PLOG(DEBUG) << "HID read failed";
     std::move(callback).Run(false, nullptr, 0);
   }
 }
 
-void HidConnectionWin::OnWriteComplete(WriteCallback callback,
+void HidConnectionWin::OnWriteComplete(HANDLE file_handle,
+                                       WriteCallback callback,
                                        PendingHidTransfer* transfer_raw,
                                        bool signaled) {
-  if (!file_.IsValid()) {
-    std::move(callback).Run(false);
-    return;
-  }
-
   std::unique_ptr<PendingHidTransfer> transfer = UnlinkTransfer(transfer_raw);
   DWORD bytes_transferred;
-  if (signaled && GetOverlappedResult(file_.Get(), transfer->GetOverlapped(),
+  if (signaled && GetOverlappedResult(file_handle, transfer->GetOverlapped(),
                                       &bytes_transferred, FALSE)) {
     std::move(callback).Run(true);
   } else {
-    HID_PLOG(EVENT) << "HID write failed";
+    HID_PLOG(DEBUG) << "HID write failed";
     std::move(callback).Run(false);
   }
 }
@@ -264,4 +300,12 @@ std::unique_ptr<PendingHidTransfer> HidConnectionWin::UnlinkTransfer(
   return saved_transfer;
 }
 
+HANDLE HidConnectionWin::GetHandleForReportId(uint8_t report_id) const {
+  for (const auto& entry : file_handles_) {
+    if (base::Contains(entry->report_ids, report_id))
+      return entry->file_handle.Get();
+  }
+  return nullptr;
+}
+
 }  // namespace device

services/device/hid/hid_connection_win.h

@@ -11,6 +11,7 @@
 
 #include <list>
 
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "base/win/scoped_handle.h"
 #include "services/device/hid/hid_connection.h"
@@ -21,9 +22,25 @@ class PendingHidTransfer;
 
 class HidConnectionWin : public HidConnection {
  public:
+  // On Windows, a single HID interface may be represented by multiple file
+  // handles where each file handle represents one top-level HID collection.
+  // Maintain a mapping of report IDs to open file handles so that the correct
+  // handle is used for each report supported by the device.
+  struct HidDeviceEntry {
+    HidDeviceEntry(base::flat_set<uint8_t> report_ids,
+                   base::win::ScopedHandle file_handle);
+    ~HidDeviceEntry();
+
+    // Reports with these IDs will be routed to |file_handle|.
+    base::flat_set<uint8_t> report_ids;
+
+    // An open file handle representing a HID top-level collection.
+    base::win::ScopedHandle file_handle;
+  };
+
   static scoped_refptr<HidConnection> Create(
       scoped_refptr<HidDeviceInfo> device_info,
-      base::win::ScopedHandle file,
+      std::vector<std::unique_ptr<HidDeviceEntry>> file_handles,
       bool allow_protected_reports);
 
  private:
@@ -31,7 +48,7 @@ class HidConnectionWin : public HidConnection {
   friend class PendingHidTransfer;
 
   HidConnectionWin(scoped_refptr<HidDeviceInfo> device_info,
-                   base::win::ScopedHandle file,
+                   std::vector<std::unique_ptr<HidDeviceEntry>> file_handles,
                    bool allow_protected_reports);
   ~HidConnectionWin() override;
 
@@ -44,22 +61,31 @@ class HidConnectionWin : public HidConnection {
   void PlatformSendFeatureReport(scoped_refptr<base::RefCountedBytes> buffer,
                                  WriteCallback callback) override;
 
-  void ReadNextInputReport();
-  void OnReadInputReport(scoped_refptr<base::RefCountedBytes> buffer,
+  // Start listening for input reports from all devices in |file_handles_|.
+  void SetUpInitialReads();
+
+  // Listen for the next input report from |file_handle|.
+  void ReadNextInputReportOnHandle(HANDLE file_handle);
+
+  void OnReadInputReport(HANDLE file_handle,
+                         scoped_refptr<base::RefCountedBytes> buffer,
                          PendingHidTransfer* transfer,
                          bool signaled);
-  void OnReadFeatureComplete(scoped_refptr<base::RefCountedBytes> buffer,
+  void OnReadFeatureComplete(HANDLE file_handle,
+                             scoped_refptr<base::RefCountedBytes> buffer,
                              ReadCallback callback,
                              PendingHidTransfer* transfer,
                              bool signaled);
-  void OnWriteComplete(WriteCallback callback,
+  void OnWriteComplete(HANDLE file_handle,
+                       WriteCallback callback,
                        PendingHidTransfer* transfer,
                        bool signaled);
 
   std::unique_ptr<PendingHidTransfer> UnlinkTransfer(
       PendingHidTransfer* transfer);
+  HANDLE GetHandleForReportId(uint8_t report_id) const;
 
-  base::win::ScopedHandle file_;
+  std::vector<std::unique_ptr<HidDeviceEntry>> file_handles_;
 
   std::list<std::unique_ptr<PendingHidTransfer>> transfers_;
 

services/device/hid/hid_device_info.cc

@@ -11,7 +11,22 @@
 
 namespace device {
 
-HidDeviceInfo::HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
+HidDeviceInfo::PlatformDeviceIdEntry::PlatformDeviceIdEntry(
+    base::flat_set<uint8_t> report_ids,
+    HidPlatformDeviceId platform_device_id)
+    : report_ids(std::move(report_ids)),
+      platform_device_id(platform_device_id) {}
+
+HidDeviceInfo::PlatformDeviceIdEntry::PlatformDeviceIdEntry(
+    const PlatformDeviceIdEntry& entry) = default;
+
+HidDeviceInfo::PlatformDeviceIdEntry&
+HidDeviceInfo::PlatformDeviceIdEntry::operator=(
+    const PlatformDeviceIdEntry& entry) = default;
+
+HidDeviceInfo::PlatformDeviceIdEntry::~PlatformDeviceIdEntry() = default;
+
+HidDeviceInfo::HidDeviceInfo(HidPlatformDeviceId platform_device_id,
                              const std::string& physical_device_id,
                              uint16_t vendor_id,
                              uint16_t product_id,
@@ -19,8 +34,7 @@ HidDeviceInfo::HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
                              const std::string& serial_number,
                              mojom::HidBusType bus_type,
                              const std::vector<uint8_t> report_descriptor,
-                             std::string device_node)
-    : platform_device_id_(platform_device_id) {
+                             std::string device_node) {
   std::vector<mojom::HidCollectionInfoPtr> collections;
   bool has_report_id;
   size_t max_input_report_size;
@@ -39,6 +53,17 @@ HidDeviceInfo::HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
   auto protected_feature_report_ids = HidBlocklist::Get().GetProtectedReportIds(
       HidBlocklist::kReportTypeFeature, vendor_id, product_id, collections);
 
+  std::vector<uint8_t> report_ids;
+  if (has_report_id) {
+    for (const auto& collection : collections) {
+      report_ids.insert(report_ids.end(), collection->report_ids.begin(),
+                        collection->report_ids.end());
+    }
+  } else {
+    report_ids.push_back(0);
+  }
+  platform_device_id_map_.emplace_back(report_ids, platform_device_id);
+
   device_ = mojom::HidDeviceInfo::New(
       base::GenerateGUID(), physical_device_id, vendor_id, product_id,
       product_name, serial_number, bus_type, report_descriptor,
@@ -48,21 +73,26 @@ HidDeviceInfo::HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
       protected_feature_report_ids);
 }
 
-HidDeviceInfo::HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
+HidDeviceInfo::HidDeviceInfo(
+    PlatformDeviceIdMap platform_device_id_map,
     const std::string& physical_device_id,
     uint16_t vendor_id,
     uint16_t product_id,
     const std::string& product_name,
     const std::string& serial_number,
     mojom::HidBusType bus_type,
-                             mojom::HidCollectionInfoPtr collection,
+    std::vector<mojom::HidCollectionInfoPtr> collections,
     size_t max_input_report_size,
     size_t max_output_report_size,
     size_t max_feature_report_size)
-    : platform_device_id_(platform_device_id) {
-  std::vector<mojom::HidCollectionInfoPtr> collections;
-  bool has_report_id = !collection->report_ids.empty();
-  collections.push_back(std::move(collection));
+    : platform_device_id_map_(std::move(platform_device_id_map)) {
+  bool has_report_id = false;
+  for (const auto& collection : collections) {
+    if (!collection->report_ids.empty()) {
+      has_report_id = true;
+      break;
+    }
+  }
 
   auto protected_input_report_ids = HidBlocklist::Get().GetProtectedReportIds(
       HidBlocklist::kReportTypeInput, vendor_id, product_id, collections);

services/device/hid/hid_device_info.h

@@ -11,6 +11,7 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
@@ -29,7 +30,21 @@ typedef std::string HidPlatformDeviceId;
 
 class HidDeviceInfo : public base::RefCountedThreadSafe<HidDeviceInfo> {
  public:
-  HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
+  // PlatformDeviceIdMap defines a mapping from report IDs to the
+  // HidPlatformDeviceId responsible for handling those reports.
+  struct PlatformDeviceIdEntry {
+    PlatformDeviceIdEntry(base::flat_set<uint8_t> report_ids,
+                          HidPlatformDeviceId platform_device_id);
+    PlatformDeviceIdEntry(const PlatformDeviceIdEntry& entry);
+    PlatformDeviceIdEntry& operator=(const PlatformDeviceIdEntry& entry);
+    ~PlatformDeviceIdEntry();
+
+    base::flat_set<uint8_t> report_ids;
+    HidPlatformDeviceId platform_device_id;
+  };
+  using PlatformDeviceIdMap = std::vector<PlatformDeviceIdEntry>;
+
+  HidDeviceInfo(HidPlatformDeviceId platform_device_id,
                 const std::string& physical_device_id,
                 uint16_t vendor_id,
                 uint16_t product_id,
@@ -39,14 +54,14 @@ class HidDeviceInfo : public base::RefCountedThreadSafe<HidDeviceInfo> {
                 const std::vector<uint8_t> report_descriptor,
                 std::string device_node = "");
 
-  HidDeviceInfo(const HidPlatformDeviceId& platform_device_id,
+  HidDeviceInfo(PlatformDeviceIdMap platform_device_id_map,
                 const std::string& physical_device_id,
                 uint16_t vendor_id,
                 uint16_t product_id,
                 const std::string& product_name,
                 const std::string& serial_number,
                 mojom::HidBusType bus_type,
-                mojom::HidCollectionInfoPtr collection,
+                std::vector<mojom::HidCollectionInfoPtr> collections,
                 size_t max_input_report_size,
                 size_t max_output_report_size,
                 size_t max_feature_report_size);
@@ -55,8 +70,8 @@ class HidDeviceInfo : public base::RefCountedThreadSafe<HidDeviceInfo> {
 
   // Device identification.
   const std::string& device_guid() const { return device_->guid; }
-  const HidPlatformDeviceId& platform_device_id() const {
-    return platform_device_id_;
+  const PlatformDeviceIdMap& platform_device_id_map() const {
+    return platform_device_id_map_;
   }
   const std::string& physical_device_id() const {
     return device_->physical_device_id;
@@ -94,7 +109,7 @@ class HidDeviceInfo : public base::RefCountedThreadSafe<HidDeviceInfo> {
  private:
   friend class base::RefCountedThreadSafe<HidDeviceInfo>;
 
-  HidPlatformDeviceId platform_device_id_;
+  PlatformDeviceIdMap platform_device_id_map_;
   mojom::HidDeviceInfoPtr device_;
 
   DISALLOW_COPY_AND_ASSIGN(HidDeviceInfo);

services/device/hid/hid_manager_unittest.cc

@@ -163,12 +163,17 @@ class HidManagerTest : public DeviceServiceTestBase {
   scoped_refptr<HidDeviceInfo> AddTestDeviceWithTopLevelCollection() {
     // Construct a HidDeviceInfo with a top-level collection. The collection has
     // a usage ID from the FIDO usage page.
+    HidDeviceInfo::PlatformDeviceIdMap platform_device_id_map;
+    platform_device_id_map.emplace_back(base::flat_set<uint8_t>{0},
+                                        kTestDeviceIds[2]);
+    std::vector<mojom::HidCollectionInfoPtr> collections;
     auto collection_info = mojom::HidCollectionInfo::New();
     collection_info->usage = mojom::HidUsageAndPage::New(1, 0xf1d0);
+    collections.push_back(std::move(collection_info));
     auto device_info = base::MakeRefCounted<HidDeviceInfo>(
-        kTestDeviceIds[2], "physical id 2", /*vendor_id=*/0, /*product_id=*/0,
-        "Hid Service Unit Test", "HidDevice-2",
-        mojom::HidBusType::kHIDBusTypeUSB, std::move(collection_info),
+        std::move(platform_device_id_map), "physical id 2", /*vendor_id=*/0,
+        /*product_id=*/0, "Hid Service Unit Test", "HidDevice-2",
+        mojom::HidBusType::kHIDBusTypeUSB, std::move(collections),
         /*max_input_report_size=*/64, /*max_output_report_size=*/64,
         /*max_feature_report_size=*/64);
     mock_hid_service_->AddDevice(device_info);

services/device/hid/hid_service.cc

@@ -4,6 +4,8 @@
 
 #include "services/device/hid/hid_service.h"
 
+#include <sstream>
+
 #include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/containers/contains.h"
@@ -24,6 +26,26 @@
 
 namespace device {
 
+namespace {
+
+// Formats the platform device IDs in |platform_device_id_map| into a
+// comma-separated list for logging. The report IDs are not logged.
+std::string PlatformDeviceIdsToString(
+    const HidDeviceInfo::PlatformDeviceIdMap& platform_device_id_map) {
+  std::ostringstream buf("'");
+  bool first = true;
+  for (const auto& entry : platform_device_id_map) {
+    if (!first)
+      buf << "', '";
+    first = false;
+    buf << entry.platform_device_id;
+  }
+  buf << "'";
+  return buf.str();
+}
+
+}  // namespace
+
 void HidService::Observer::OnDeviceAdded(mojom::HidDeviceInfoPtr device_info) {}
 
 void HidService::Observer::OnDeviceRemoved(
@@ -74,9 +96,12 @@ HidService::~HidService() {
 void HidService::AddDevice(scoped_refptr<HidDeviceInfo> device_info) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::string device_guid =
-      FindDeviceIdByPlatformDeviceId(device_info->platform_device_id());
-  if (device_guid.empty()) {
+  base::Optional<std::string> found_guid = base::nullopt;
+  for (const auto& entry : device_info->platform_device_id_map()) {
+    if ((found_guid = FindDeviceGuidInDeviceMap(entry.platform_device_id)))
+      break;
+  }
+  if (!found_guid) {
     devices_[device_info->device_guid()] = device_info;
 
     HID_LOG(USER) << "HID device "
@@ -84,8 +109,10 @@ void HidService::AddDevice(scoped_refptr<HidDeviceInfo> device_info) {
                   << ": vendorId=" << device_info->vendor_id()
                   << ", productId=" << device_info->product_id() << ", name='"
                   << device_info->product_name() << "', serial='"
-                  << device_info->serial_number() << "', deviceId='"
-                  << device_info->platform_device_id() << "'";
+                  << device_info->serial_number() << "', deviceIds=["
+                  << PlatformDeviceIdsToString(
+                         device_info->platform_device_id_map())
+                  << "]";
 
     if (enumeration_ready_) {
       for (auto& observer : observer_list_)
@@ -97,18 +124,18 @@ void HidService::AddDevice(scoped_refptr<HidDeviceInfo> device_info) {
 void HidService::RemoveDevice(const HidPlatformDeviceId& platform_device_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::string device_guid = FindDeviceIdByPlatformDeviceId(platform_device_id);
-  if (!device_guid.empty()) {
+  auto found_guid = FindDeviceGuidInDeviceMap(platform_device_id);
+  if (found_guid) {
     HID_LOG(USER) << "HID device removed: deviceId='" << platform_device_id
                   << "'";
-    DCHECK(base::Contains(devices_, device_guid));
+    DCHECK(base::Contains(devices_, *found_guid));
 
-    scoped_refptr<HidDeviceInfo> device_info = devices_[device_guid];
+    scoped_refptr<HidDeviceInfo> device_info = devices_[*found_guid];
     if (enumeration_ready_) {
       for (auto& observer : observer_list_)
         observer.OnDeviceRemoved(device_info->device()->Clone());
     }
-    devices_.erase(device_guid);
+    devices_.erase(*found_guid);
   }
 }
 
@@ -135,14 +162,17 @@ void HidService::FirstEnumerationComplete() {
   }
 }
 
-std::string HidService::FindDeviceIdByPlatformDeviceId(
+base::Optional<std::string> HidService::FindDeviceGuidInDeviceMap(
     const HidPlatformDeviceId& platform_device_id) {
-  for (const auto& map_entry : devices_) {
-    if (map_entry.second->platform_device_id() == platform_device_id) {
-      return map_entry.first;
+  for (const auto& device_entry : devices_) {
+    const auto& platform_device_map =
+        device_entry.second->platform_device_id_map();
+    for (const auto& platform_device_entry : platform_device_map) {
+      if (platform_device_entry.platform_device_id == platform_device_id)
+        return device_entry.first;
     }
   }
-  return std::string();
+  return base::nullopt;
 }
 
 }  // namespace device

services/device/hid/hid_service.h

@@ -89,7 +89,7 @@ class HidService {
 
  private:
   void RunPendingEnumerations();
-  std::string FindDeviceIdByPlatformDeviceId(
+  base::Optional<std::string> FindDeviceGuidInDeviceMap(
       const HidPlatformDeviceId& platform_device_id);
 
   DeviceMap devices_;

services/device/hid/hid_service_linux.cc

@@ -446,7 +446,7 @@ void HidServiceLinux::FinishOpen(std::unique_ptr<ConnectParams> params) {
   DCHECK(params->fd.is_valid());
 
   if (!base::SetNonBlocking(params->fd.get())) {
-    HID_PLOG(ERROR) << "Failed to set the non-blocking flag on the device fd";
+    HID_PLOG(DEBUG) << "Failed to set the non-blocking flag on the device fd";
     std::move(params->callback).Run(nullptr);
     return;
   }

services/device/hid/hid_service_mac.cc

@@ -105,7 +105,7 @@ HidServiceMac::HidServiceMac() : weak_factory_(this) {
       IOServiceMatching(kIOHIDDeviceKey), FirstMatchCallback, this,
       devices_added_iterator_.InitializeInto());
   if (result != kIOReturnSuccess) {
-    HID_LOG(ERROR) << "Failed to listen for device arrival: "
+    HID_LOG(DEBUG) << "Failed to listen for device arrival: "
                    << HexErrorCode(result);
     return;
   }
@@ -118,7 +118,7 @@ HidServiceMac::HidServiceMac() : weak_factory_(this) {
       IOServiceMatching(kIOHIDDeviceKey), TerminatedCallback, this,
       devices_removed_iterator_.InitializeInto());
   if (result != kIOReturnSuccess) {
-    HID_LOG(ERROR) << "Failed to listen for device removal: "
+    HID_LOG(DEBUG) << "Failed to listen for device removal: "
                    << HexErrorCode(result);
     return;
   }
@@ -157,11 +157,14 @@ base::WeakPtr<HidService> HidServiceMac::GetWeakPtr() {
 // static
 base::ScopedCFTypeRef<IOHIDDeviceRef> HidServiceMac::OpenOnBlockingThread(
     scoped_refptr<HidDeviceInfo> device_info) {
+  DCHECK_EQ(device_info->platform_device_id_map().size(), 1u);
+  const auto& platform_device_id =
+      device_info->platform_device_id_map().front().platform_device_id;
   base::ScopedCFTypeRef<CFDictionaryRef> matching_dict(
-      IORegistryEntryIDMatching(device_info->platform_device_id()));
+      IORegistryEntryIDMatching(platform_device_id));
   if (!matching_dict.get()) {
-    HID_LOG(EVENT) << "Failed to create matching dictionary for ID: "
-                   << device_info->platform_device_id();
+    HID_LOG(DEBUG) << "Failed to create matching dictionary for ID: "
+                   << platform_device_id;
     return base::ScopedCFTypeRef<IOHIDDeviceRef>();
   }
 
@@ -170,21 +173,20 @@ base::ScopedCFTypeRef<IOHIDDeviceRef> HidServiceMac::OpenOnBlockingThread(
   base::mac::ScopedIOObject<io_service_t> service(IOServiceGetMatchingService(
       kIOMasterPortDefault, matching_dict.release()));
   if (!service.get()) {
-    HID_LOG(EVENT) << "IOService not found for ID: "
-                   << device_info->platform_device_id();
+    HID_LOG(DEBUG) << "IOService not found for ID: " << platform_device_id;
     return base::ScopedCFTypeRef<IOHIDDeviceRef>();
   }
 
   base::ScopedCFTypeRef<IOHIDDeviceRef> hid_device(
       IOHIDDeviceCreate(kCFAllocatorDefault, service));
   if (!hid_device) {
-    HID_LOG(EVENT) << "Unable to create IOHIDDevice object.";
+    HID_LOG(DEBUG) << "Unable to create IOHIDDevice object.";
     return base::ScopedCFTypeRef<IOHIDDeviceRef>();
   }
 
   IOReturn result = IOHIDDeviceOpen(hid_device, kIOHIDOptionsTypeNone);
   if (result != kIOReturnSuccess) {
-    HID_LOG(EVENT) << "Failed to open device: " << HexErrorCode(result);
+    HID_LOG(DEBUG) << "Failed to open device: " << HexErrorCode(result);
     return base::ScopedCFTypeRef<IOHIDDeviceRef>();
   }
 

services/device/hid/hid_service_win.cc

@@ -25,7 +25,9 @@
 #include "base/location.h"
 #include "base/memory/free_deleter.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/thread_pool.h"
 #include "base/threading/sequenced_task_runner_handle.h"
@@ -67,23 +69,72 @@ void UnpackBitField(uint16_t bit_field, mojom::HidReportItem* item) {
   item->is_buffered_bytes = bit_field & kBitFieldFlagBufferedBytes;
 }
 
+// Looks up the value of a string list device property specified by
+// |property_key| for the device described by |device_info_data|. On success,
+// returns the property value as a string vector. Returns base::nullopt if the
+// property is not present or has a different type.
+base::Optional<std::vector<std::wstring>> GetDeviceStringListProperty(
+    HDEVINFO device_info_set,
+    SP_DEVINFO_DATA& device_info_data,
+    const DEVPROPKEY& property_key) {
+  DEVPROPTYPE property_type;
+  DWORD required_size;
+  if (SetupDiGetDeviceProperty(device_info_set, &device_info_data,
+                               &property_key, &property_type,
+                               /*PropertyBuffer=*/nullptr,
+                               /*PropertyBufferSize=*/0, &required_size,
+                               /*Flags=*/0)) {
+    HID_LOG(DEBUG) << "SetupDiGetDeviceProperty unexpectedly succeeded.";
+    return base::nullopt;
+  }
+
+  DWORD last_error = GetLastError();
+  if (last_error == ERROR_NOT_FOUND)
+    return base::nullopt;
+
+  if (last_error != ERROR_INSUFFICIENT_BUFFER) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceProperty failed";
+    return base::nullopt;
+  }
+
+  if (property_type != DEVPROP_TYPE_STRING_LIST)
+    return base::nullopt;
+
+  base::string16 buffer16;
+  if (!SetupDiGetDeviceProperty(
+          device_info_set, &device_info_data, &property_key, &property_type,
+          reinterpret_cast<PBYTE>(base::WriteInto(&buffer16, required_size)),
+          required_size, /*RequiredSize=*/nullptr, /*Flags=*/0)) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceProperty failed";
+    return base::nullopt;
+  }
+
+  return base::SplitString(buffer16, base::WStringPiece(L"\0", 1),
+                           base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+}
+
 // Looks up the value of a GUID-type device property specified by |property| for
-// the device described by |device_info_data|. On success, returns true and sets
-// |property_buffer| to the property value. Returns false if the property is not
-// present or has a different type.
-bool GetDeviceGuidProperty(HDEVINFO device_info_set,
+// the device described by |device_info_data|. On success, returns the property
+// value as a string. Returns base::nullopt if the property is not present or
+// has a different type.
+base::Optional<std::string> GetDeviceGuidProperty(
+    HDEVINFO device_info_set,
     SP_DEVINFO_DATA& device_info_data,
-                           const DEVPROPKEY& property,
-                           GUID* property_buffer) {
+    const DEVPROPKEY& property_key) {
   DEVPROPTYPE property_type;
+  GUID property_buffer;
   if (!SetupDiGetDeviceProperty(
-          device_info_set, &device_info_data, &property, &property_type,
-          reinterpret_cast<PBYTE>(property_buffer), sizeof(*property_buffer),
-          /*RequiredSize=*/nullptr, /*Flags=*/0) ||
-      property_type != DEVPROP_TYPE_GUID) {
-    return false;
+          device_info_set, &device_info_data, &property_key, &property_type,
+          reinterpret_cast<PBYTE>(&property_buffer), sizeof(property_buffer),
+          /*RequiredSize=*/nullptr, /*Flags=*/0)) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceProperty failed";
+    return base::nullopt;
   }
-  return true;
+
+  if (property_type != DEVPROP_TYPE_GUID)
+    return base::nullopt;
+
+  return base::SysWideToUTF8(base::win::WStringFromGUID(property_buffer));
 }
 
 // Looks up information about the device described by |device_interface_data|
@@ -103,8 +154,13 @@ bool GetDeviceInfoAndPathFromInterface(
                                       /*DeviceInterfaceDetailData=*/nullptr,
                                       /*DeviceInterfaceDetailSize=*/0,
                                       &required_size,
-                                      /*DeviceInfoData=*/nullptr) ||
-      GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+                                      /*DeviceInfoData=*/nullptr)) {
+    HID_LOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail unexpectedly succeeded.";
+    return false;
+  }
+
+  if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail failed";
     return false;
   }
 
@@ -119,6 +175,7 @@ bool GetDeviceInfoAndPathFromInterface(
                                        device_interface_detail_data.get(),
                                        required_size, /*RequiredSize=*/nullptr,
                                        device_info_data)) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail failed";
     return false;
   }
 
@@ -130,22 +187,83 @@ bool GetDeviceInfoAndPathFromInterface(
   return true;
 }
 
+// Returns a device path for the HID device described by |instance_id|, or
+// base::nullopt if an error occurred.
+base::Optional<std::wstring> GetDevicePathFromInstanceId(
+    const std::wstring& instance_id) {
+  base::win::ScopedDevInfo device_info_set(SetupDiGetClassDevs(
+      &GUID_DEVINTERFACE_HID, instance_id.c_str(),
+      /*hwndParent=*/0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT));
+  if (!device_info_set.is_valid()) {
+    HID_PLOG(DEBUG) << "SetupDiGetClassDevsA failed";
+    return base::nullopt;
+  }
+
+  // Assume there is at most one matching device.
+  SP_DEVICE_INTERFACE_DATA device_interface_data;
+  device_interface_data.cbSize = sizeof(device_interface_data);
+  if (!SetupDiEnumDeviceInterfaces(device_info_set.get(),
+                                   /*DeviceInfoData=*/nullptr,
+                                   &GUID_DEVINTERFACE_HID,
+                                   /*MemberIndex=*/0, &device_interface_data)) {
+    HID_PLOG(DEBUG) << "SetupDiEnumDeviceInterfaces failed";
+    return base::nullopt;
+  }
+
+  // Determine the required size of the detail struct.
+  DWORD required_size = 0;
+  if (SetupDiGetDeviceInterfaceDetail(
+          device_info_set.get(), &device_interface_data,
+          /*DeviceInterfaceDetailData=*/nullptr,
+          /*DeviceInterfaceDetailDataSize=*/0, &required_size,
+          /*DeviceInfoData=*/nullptr)) {
+    HID_LOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail unexpectedly succeeded.";
+    return base::nullopt;
+  }
+
+  if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail failed";
+    return base::nullopt;
+  }
+
+  std::unique_ptr<SP_DEVICE_INTERFACE_DETAIL_DATA, base::FreeDeleter>
+      device_interface_detail_data(
+          static_cast<SP_DEVICE_INTERFACE_DETAIL_DATA*>(malloc(required_size)));
+  device_interface_detail_data->cbSize = sizeof(*device_interface_detail_data);
+
+  // Get the detailed data for this device.
+  if (!SetupDiGetDeviceInterfaceDetail(
+          device_info_set.get(), &device_interface_data,
+          device_interface_detail_data.get(), required_size,
+          /*RequiredSize=*/nullptr,
+          /*DeviceInfoData=*/nullptr)) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceInterfaceDetail failed";
+    return base::nullopt;
+  }
+
+  DCHECK(base::IsStringASCII(device_interface_detail_data->DevicePath));
+  return base::ToLowerASCII(device_interface_detail_data->DevicePath);
+}
+
 // Returns a device info set containing only the device described by
 // |device_path|, or an invalid ScopedDevInfo if there was an error while
 // creating the device set. The device info is returned in |device_info_data|.
-base::win::ScopedDevInfo GetDeviceInfoFromPath(
+base::win::ScopedDevInfo GetDeviceInfoSetFromDevicePath(
     const std::wstring& device_path,
     SP_DEVINFO_DATA* device_info_data) {
   base::win::ScopedDevInfo device_info_set(SetupDiGetClassDevs(
       &GUID_DEVINTERFACE_HID, /*Enumerator=*/nullptr,
       /*hwndParent=*/0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT));
-  if (!device_info_set.is_valid())
+  if (!device_info_set.is_valid()) {
+    HID_PLOG(DEBUG) << "SetupDiGetClassDevs failed";
     return base::win::ScopedDevInfo();
+  }
 
   SP_DEVICE_INTERFACE_DATA device_interface_data;
   device_interface_data.cbSize = sizeof(device_interface_data);
   if (!SetupDiOpenDeviceInterface(device_info_set.get(), device_path.c_str(),
                                   /*OpenFlags=*/0, &device_interface_data)) {
+    HID_PLOG(DEBUG) << "SetupDiOpenDeviceInterface failed";
     return base::win::ScopedDevInfo();
   }
 
@@ -157,6 +275,83 @@ base::win::ScopedDevInfo GetDeviceInfoFromPath(
   return device_info_set;
 }
 
+// Looks up the device instance ID for the device described by
+// |device_info_data|. On success, returns the property value as a string
+// vector. Returns base::nullopt if the property is not present or has a
+// different type.
+base::Optional<std::wstring> GetDeviceInstanceId(
+    HDEVINFO device_info_set,
+    SP_DEVINFO_DATA& device_info_data) {
+  DEVPROPTYPE property_type;
+  DWORD required_size;
+  if (SetupDiGetDeviceProperty(device_info_set, &device_info_data,
+                               &DEVPKEY_Device_InstanceId, &property_type,
+                               /*PropertyBuffer=*/nullptr,
+                               /*PropertyBufferSize=*/0, &required_size,
+                               /*Flags=*/0)) {
+    HID_LOG(DEBUG) << "SetupDiGetDeviceProperty unexpectedly succeeded.";
+    return base::nullopt;
+  }
+
+  DWORD last_error = GetLastError();
+  if (last_error == ERROR_NOT_FOUND)
+    return base::nullopt;
+
+  if (last_error != ERROR_INSUFFICIENT_BUFFER) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceProperty failed";
+    return base::nullopt;
+  }
+
+  if (property_type != DEVPROP_TYPE_STRING)
+    return base::nullopt;
+
+  std::wstring instance_id;
+  if (!SetupDiGetDeviceProperty(
+          device_info_set, &device_info_data, &DEVPKEY_Device_InstanceId,
+          &property_type,
+          reinterpret_cast<PBYTE>(base::WriteInto(&instance_id, required_size)),
+          required_size, /*RequiredSize=*/nullptr, /*Flags=*/0)) {
+    HID_PLOG(DEBUG) << "SetupDiGetDeviceProperty failed";
+    return base::nullopt;
+  }
+
+  // Canonicalize the instance ID.
+  DCHECK(base::IsStringASCII(instance_id));
+  instance_id = base::ToLowerASCII(instance_id);
+  // Removing trailing NUL bytes.
+  return std::wstring(base::TrimString(
+      instance_id, base::WStringPiece(L"\0", 1), base::TRIM_TRAILING));
+}
+
+// Returns a vector of instance IDs for all siblings of the device described by
+// |device_interface_data| in |device_info_set|. Returns an empty vector if the
+// instance IDs could not be retrieved.
+std::vector<std::wstring> GetSiblingInstanceIds(
+    HDEVINFO device_info_set,
+    SP_DEVICE_INTERFACE_DATA& device_interface_data) {
+  // Get device info for |device_interface_data|.
+  SP_DEVINFO_DATA device_info_data = {0};
+  device_info_data.cbSize = sizeof(device_info_data);
+  std::wstring device_path;
+  if (!GetDeviceInfoAndPathFromInterface(device_info_set, device_interface_data,
+                                         &device_info_data, &device_path)) {
+    return {};
+  }
+
+  // Get the sibling instance IDs.
+  auto instance_ids = GetDeviceStringListProperty(
+      device_info_set, device_info_data, DEVPKEY_Device_Siblings);
+  if (!instance_ids)
+    return {};
+
+  // Canonicalize the instance IDs.
+  for (auto& instance_id : *instance_ids) {
+    DCHECK(base::IsStringASCII(instance_id));
+    instance_id = base::ToLowerASCII(instance_id);
+  }
+  return *instance_ids;
+}
+
 mojom::HidReportItemPtr CreateHidReportItem(
     const HidServiceWin::PreparsedData::ReportItem& item) {
   auto hid_report_item = mojom::HidReportItem::New();
@@ -364,18 +559,31 @@ void HidServiceWin::Connect(const std::string& device_guid,
   }
   scoped_refptr<HidDeviceInfo> device_info = map_entry->second;
 
-  base::win::ScopedHandle file(OpenDevice(device_info->platform_device_id()));
-  if (!file.IsValid()) {
-    HID_PLOG(EVENT) << "Failed to open device";
+  const auto& platform_device_id_map = device_info->platform_device_id_map();
+  std::vector<std::unique_ptr<HidConnectionWin::HidDeviceEntry>> file_handles;
+  for (const auto& entry : platform_device_id_map) {
+    base::win::ScopedHandle file_handle(OpenDevice(entry.platform_device_id));
+    if (!file_handle.IsValid()) {
+      HID_PLOG(DEBUG) << "Failed to open device with deviceId='"
+                      << entry.platform_device_id << "'";
+      continue;
+    }
+
+    file_handles.push_back(std::make_unique<HidConnectionWin::HidDeviceEntry>(
+        entry.report_ids, std::move(file_handle)));
+  }
+
+  if (file_handles.empty()) {
+    // Report failure if none of the file handles could be opened.
     task_runner_->PostTask(FROM_HERE,
                            base::BindOnce(std::move(callback), nullptr));
     return;
   }
 
   task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(std::move(callback),
-                     HidConnectionWin::Create(device_info, std::move(file),
+      FROM_HERE, base::BindOnce(std::move(callback),
+                                HidConnectionWin::Create(
+                                    device_info, std::move(file_handles),
                                     allow_protected_reports)));
 }
 
@@ -387,37 +595,61 @@ base::WeakPtr<HidService> HidServiceWin::GetWeakPtr() {
 void HidServiceWin::EnumerateBlocking(
     base::WeakPtr<HidServiceWin> service,
     scoped_refptr<base::SequencedTaskRunner> task_runner) {
-  base::win::ScopedDevInfo dev_info(SetupDiGetClassDevs(
+  base::win::ScopedDevInfo device_info_set(SetupDiGetClassDevs(
       &GUID_DEVINTERFACE_HID, /*Enumerator=*/nullptr,
       /*hwndParent=*/nullptr, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE));
 
-  if (dev_info.is_valid()) {
+  // Remember the instance IDs of devices that have already been enumerated.
+  base::flat_set<std::wstring> seen_instance_ids;
+
+  if (device_info_set.is_valid()) {
     SP_DEVICE_INTERFACE_DATA device_interface_data = {0};
     device_interface_data.cbSize = sizeof(device_interface_data);
 
     for (int device_index = 0; SetupDiEnumDeviceInterfaces(
-             dev_info.get(), /*DeviceInfoData=*/nullptr, &GUID_DEVINTERFACE_HID,
-             device_index, &device_interface_data);
+             device_info_set.get(), /*DeviceInfoData=*/nullptr,
+             &GUID_DEVINTERFACE_HID, device_index, &device_interface_data);
          ++device_index) {
-      SP_DEVINFO_DATA dev_info_data = {0};
-      dev_info_data.cbSize = sizeof(dev_info_data);
+      SP_DEVINFO_DATA device_info_data = {0};
+      device_info_data.cbSize = sizeof(device_info_data);
       std::wstring device_path;
-      if (!GetDeviceInfoAndPathFromInterface(dev_info.get(),
+      if (!GetDeviceInfoAndPathFromInterface(device_info_set.get(),
                                              device_interface_data,
-                                             &dev_info_data, &device_path)) {
+                                             &device_info_data, &device_path)) {
         continue;
       }
 
+      // Get the instance ID. Skip this device if it was already enumerated as a
+      // sibling of another device.
+      auto instance_id =
+          GetDeviceInstanceId(device_info_set.get(), device_info_data);
+      if (!instance_id || base::Contains(seen_instance_ids, *instance_id))
+        continue;
+      seen_instance_ids.insert(*instance_id);
+
       // Get the container ID for the physical device.
-      GUID container_id;
-      if (!GetDeviceGuidProperty(dev_info.get(), dev_info_data,
-                                 DEVPKEY_Device_ContainerId, &container_id)) {
+      auto physical_device_id = GetDeviceGuidProperty(
+          device_info_set.get(), device_info_data, DEVPKEY_Device_ContainerId);
+      if (!physical_device_id)
         continue;
+
+      // Get the instance IDs for any siblings of the added device. Siblings of
+      // a HID device node represent other top-level collections generated from
+      // the same HID interface.
+      auto sibling_instance_ids =
+          GetSiblingInstanceIds(device_info_set.get(), device_interface_data);
+
+      // Get the device path for each sibling from its instance ID.
+      std::vector<std::wstring> device_paths = {device_path};
+      for (auto sibling_instance_id : sibling_instance_ids) {
+        seen_instance_ids.insert(sibling_instance_id);
+        auto sibling_path = GetDevicePathFromInstanceId(sibling_instance_id);
+        if (sibling_path)
+          device_paths.push_back(std::move(*sibling_path));
       }
-      std::string physical_device_id =
-          base::WideToUTF8(base::win::WStringFromGUID(container_id));
 
-      AddDeviceBlocking(service, task_runner, device_path, physical_device_id);
+      AddDeviceBlocking(service, task_runner, device_paths,
+                        *physical_device_id);
     }
   }
 
@@ -430,51 +662,84 @@ void HidServiceWin::EnumerateBlocking(
 void HidServiceWin::AddDeviceBlocking(
     base::WeakPtr<HidServiceWin> service,
     scoped_refptr<base::SequencedTaskRunner> task_runner,
-    const std::wstring& device_path,
+    const std::vector<std::wstring>& device_paths,
     const std::string& physical_device_id) {
+  // On Windows, HID interfaces with multiple top-level collections are split
+  // into separate device nodes. Merge these top-level collections into a single
+  // HidDeviceInfo object.
+  uint16_t vendor_id = 0;
+  uint16_t product_id = 0;
+  std::string product_name;
+  std::string serial_number;
+  HidDeviceInfo::PlatformDeviceIdMap platform_device_id_map;
+  std::vector<mojom::HidCollectionInfoPtr> collections;
+  uint16_t max_input_report_size = 0;
+  uint16_t max_output_report_size = 0;
+  uint16_t max_feature_report_size = 0;
+  for (const auto& device_path : device_paths) {
     base::win::ScopedHandle device_handle(OpenDevice(device_path));
-  if (!device_handle.IsValid()) {
-    return;
-  }
+    if (!device_handle.IsValid())
+      continue;
+
+    auto preparsed_data = HidPreparsedData::Create(device_handle.Get());
+    if (!preparsed_data)
+      continue;
 
+    // USB-level device properties should not differ for device nodes that are
+    // part of the same physical device. Only read these properties once.
+    if (collections.empty()) {
       HIDD_ATTRIBUTES attrib = {0};
       attrib.Size = sizeof(attrib);
       if (!HidD_GetAttributes(device_handle.Get(), &attrib)) {
-    HID_LOG(EVENT) << "Failed to get device attributes.";
-    return;
+        HID_LOG(DEBUG) << "Failed to get device attributes.";
+        continue;
       }
+      vendor_id = attrib.VendorID;
+      product_id = attrib.ProductID;
 
-  auto preparsed_data = HidPreparsedData::Create(device_handle.Get());
-  if (!preparsed_data)
-    return;
-
-  // 1023 characters plus NULL terminator is more than enough for a USB string
-  // descriptor which is limited to 126 characters.
+      // 1023 characters plus NULL terminator is more than enough for a USB
+      // string descriptor which is limited to 126 characters.
       base::char16 buffer[1024];
-  std::string product_name;
-  if (HidD_GetProductString(device_handle.Get(), &buffer[0], sizeof(buffer))) {
+      if (HidD_GetProductString(device_handle.Get(), &buffer[0],
+                                sizeof(buffer))) {
         // NULL termination guaranteed by the API.
         product_name = base::UTF16ToUTF8(buffer);
       }
-  std::string serial_number;
       if (HidD_GetSerialNumberString(device_handle.Get(), &buffer[0],
                                      sizeof(buffer))) {
         // NULL termination guaranteed by the API.
         serial_number = base::UTF16ToUTF8(buffer);
       }
+    }
+
+    // Create a HidCollectionInfo for |device_path| and update the relevant
+    // HidDeviceInfo properties.
+    auto collection = preparsed_data->CreateHidCollectionInfo();
+    platform_device_id_map.emplace_back(collection->report_ids, device_path);
+    collections.push_back(std::move(collection));
+    max_input_report_size = std::max(
+        max_input_report_size, preparsed_data->GetReportByteLength(HidP_Input));
+    max_output_report_size =
+        std::max(max_output_report_size,
+                 preparsed_data->GetReportByteLength(HidP_Output));
+    max_feature_report_size =
+        std::max(max_feature_report_size,
+                 preparsed_data->GetReportByteLength(HidP_Feature));
+  }
+
+  // Only add the device if we were able to create a HidCollectionInfo for at
+  // least one of the paths in |device_paths|.
+  if (collections.empty())
+    return;
 
   // This populates the HidDeviceInfo instance without a raw report descriptor.
-  // The descriptor is unavailable on Windows because HID devices are exposed to
-  // user-space as individual top-level collections.
-  scoped_refptr<HidDeviceInfo> device_info(
-      new HidDeviceInfo(device_path, physical_device_id, attrib.VendorID,
-                        attrib.ProductID, product_name, serial_number,
+  // The descriptor is unavailable on Windows.
+  scoped_refptr<HidDeviceInfo> device_info(new HidDeviceInfo(
+      std::move(platform_device_id_map), physical_device_id, vendor_id,
+      product_id, product_name, serial_number,
       // TODO(crbug.com/443335): Detect Bluetooth.
-                        mojom::HidBusType::kHIDBusTypeUSB,
-                        preparsed_data->CreateHidCollectionInfo(),
-                        preparsed_data->GetReportByteLength(HidP_Input),
-                        preparsed_data->GetReportByteLength(HidP_Output),
-                        preparsed_data->GetReportByteLength(HidP_Feature)));
+      mojom::HidBusType::kHIDBusTypeUSB, std::move(collections),
+      max_input_report_size, max_output_report_size, max_feature_report_size));
 
   task_runner->PostTask(FROM_HERE, base::BindOnce(&HidServiceWin::AddDevice,
                                                   service, device_info));
@@ -484,22 +749,50 @@ void HidServiceWin::OnDeviceAdded(const GUID& class_guid,
                                   const std::wstring& device_path) {
   SP_DEVINFO_DATA device_info_data = {0};
   device_info_data.cbSize = sizeof(device_info_data);
-  auto device_info_set = GetDeviceInfoFromPath(device_path, &device_info_data);
+  auto device_info_set =
+      GetDeviceInfoSetFromDevicePath(device_path, &device_info_data);
   if (!device_info_set.is_valid())
     return;
 
-  GUID container_id;
-  if (!GetDeviceGuidProperty(device_info_set.get(), device_info_data,
-                             DEVPKEY_Device_ContainerId, &container_id)) {
+  // Assume there is at most one matching device.
+  SP_DEVICE_INTERFACE_DATA device_interface_data;
+  device_interface_data.cbSize = sizeof(device_interface_data);
+  if (!SetupDiEnumDeviceInterfaces(device_info_set.get(), &device_info_data,
+                                   &GUID_DEVINTERFACE_HID,
+                                   /*MemberIndex=*/0, &device_interface_data)) {
+    HID_PLOG(DEBUG) << "SetupDiEnumDeviceInterfaces failed";
     return;
   }
-  std::string physical_device_id =
-      base::WideToUTF8(base::win::WStringFromGUID(container_id));
+
+  // Get the container ID for the physical device.
+  auto physical_device_id = GetDeviceGuidProperty(
+      device_info_set.get(), device_info_data, DEVPKEY_Device_ContainerId);
+  if (!physical_device_id)
+    return;
+
+  // Get the instance IDs for any siblings of the added device. Siblings of a
+  // HID device node represent other top-level collections generated from the
+  // same HID interface.
+  //
+  // It is expected that OnDeviceAdded will be called again for each of the
+  // siblings, causing some of the work here to be duplicated. We assume that
+  // all sibling devices have been added to the device registry by the time
+  // OnDeviceAdded is called.
+  auto sibling_instance_ids =
+      GetSiblingInstanceIds(device_info_set.get(), device_interface_data);
+
+  // Get the device path for each sibling from its instance ID.
+  std::vector<std::wstring> device_paths = {device_path};
+  for (auto sibling_instance_id : sibling_instance_ids) {
+    auto sibling_path = GetDevicePathFromInstanceId(sibling_instance_id);
+    if (sibling_path)
+      device_paths.push_back(std::move(*sibling_path));
+  }
 
   blocking_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&HidServiceWin::AddDeviceBlocking,
                                 weak_factory_.GetWeakPtr(), task_runner_,
-                                device_path, physical_device_id));
+                                std::move(device_paths), *physical_device_id));
 }
 
 void HidServiceWin::OnDeviceRemoved(const GUID& class_guid,

services/device/hid/hid_service_win.h

@@ -141,7 +141,7 @@ class HidServiceWin : public HidService, public DeviceMonitorWin::Observer {
   static void AddDeviceBlocking(
       base::WeakPtr<HidServiceWin> service,
       scoped_refptr<base::SequencedTaskRunner> task_runner,
-      const std::wstring& device_path,
+      const std::vector<std::wstring>& device_paths,
       const std::string& physical_device_id);
 
   // DeviceMonitorWin::Observer implementation:

third_party/blink/web_tests/wpt_internal/hid/hidDevice_collections-manual.https.html

@@ -62,15 +62,21 @@
                       'Incorrect bit width for usage 0x' + hex32(usage) + '.');
       };
 
+      // Returns the first top-level collection in |device.collections| with a
+      // usage matching |usagePage| and |usage|, or undefined if no matching
+      // collection was found.
+      const getCollectionByUsage = (device, usagePage, usage) => {
+        return device.collections.find(c => {
+            return c.usagePage == usagePage && c.usage == usage;});
+      }
+
       // Returns the first device in |devices| with a top-level collection
       // matching |usagePage| and |usage|, or undefined if no matching device
       // was found.
       const getDeviceByCollectionUsage = (devices, usagePage, usage) => {
         return devices.find(d => {
-          return d.collections.find(c => {
-            return c.usagePage == usagePage && c.usage == usage;
+          return getCollectionByUsage(d, usagePage, usage);
         }) !== undefined;
-        });
       }
 
       // Returns the first report in |devices| with matching |reportType| and
@@ -103,6 +109,12 @@
       };
 
       const checkReportMapDualshock4 = devices => {
+        // Expect one device with one top-level collection.
+        assert_equals(devices.length, 1, 'device count');
+        assert_equals(devices[0].collections.length, 1, 'collection count');
+        const collection = getCollectionByUsage(devices[0], 0x0001, 0x0005);
+        assert_not_equals(collection, undefined, 'game pad collection');
+
         // Input report
         const input01 = getReport(devices, 'input', 0x01);
         assert_not_equals(input01, undefined, 'input report 0x01');
@@ -282,6 +294,12 @@
       };
 
       checkReportMapDualSense = devices => {
+        // Expect one device with one top-level collection.
+        assert_equals(devices.length, 1, 'device count');
+        assert_equals(devices[0].collections.length, 1, 'collection count');
+        const collection = getCollectionByUsage(devices[0], 0x0001, 0x0005);
+        assert_not_equals(collection, undefined, 'game pad collection');
+
         // Input report
         const input01 = getReport(devices, 'input', 0x01);
         assert_not_equals(input01, undefined, 'input report 0x01');
@@ -380,6 +398,12 @@
       };
 
       checkReportMapSwitchPro = devices => {
+        // Expect one device with one top-level collection.
+        assert_equals(devices.length, 1, 'device count');
+        assert_equals(devices[0].collections.length, 1, 'collection count');
+        const collection = getCollectionByUsage(devices[0], 0x0001, 0x0004);
+        assert_not_equals(collection, undefined, 'joystick collection');
+
         // Input reports
         const input30 = getReport(devices, 'input', 0x30);
         assert_not_equals(input30, undefined, 'input report 0x30');
@@ -437,12 +461,19 @@
         // Speech Mike exposes five HID interfaces, two of which are blocked by
         // WebHID. None of the interfaces use report IDs. Distinguish the
         // interfaces by their top-level collection usage information.
+        assert_equals(devices.length, 3, 'device count');
         const device0 = getDeviceByCollectionUsage(devices, 0xffa0, 0x0001);
         assert_not_equals(device0, undefined, 'vendor device');
+        assert_equals(device0.collections.length, 1,
+                      'vendor device collection count');
         const device1 = getDeviceByCollectionUsage(devices, 0x000c, 0x0001);
         assert_not_equals(device1, undefined, 'consumer device');
+        assert_equals(device1.collections.length, 1,
+                      'consumer device collection count');
         const device2 = getDeviceByCollectionUsage(devices, 0x0001, 0x0004);
         assert_not_equals(device2, undefined, 'joystick device');
+        assert_equals(device2.collections.length, 1,
+                      'joystick device collection count');
 
         // These devices should be blocked by WebHID.
         const device3 = getDeviceByCollectionUsage(devices, 0x0001, 0x0002);
@@ -492,6 +523,16 @@
       };
 
       checkReportMapEvolveLink = devices => {
+        // Expect one device with three top-level collections.
+        assert_equals(devices.length, 1, 'device count');
+        assert_equals(devices[0].collections.length, 3, 'collection count');
+        const collection0 = getCollectionByUsage(devices[0], 0x000b, 0x0005);
+        assert_not_equals(collection0, undefined, 'headset collection');
+        const collection1 = getCollectionByUsage(devices[0], 0xff00, 0x0005);
+        assert_not_equals(collection1, undefined, 'vendor collection');
+        const collection2 = getCollectionByUsage(devices[0], 0x000c, 0x0001);
+        assert_not_equals(collection2, undefined, 'consumer collection');
+
         // Input reports
         const input01 = getReport(devices, 'input', 0x01);
         assert_not_equals(input01, undefined, 'input report 0x01');
@@ -609,6 +650,12 @@
       };
 
       checkReportMapStadiaController = devices => {
+        // Expect one device with one top-level collection.
+        assert_equals(devices.length, 1, 'device count');
+        assert_equals(devices[0].collections.length, 1, 'collection count');
+        const collection = getCollectionByUsage(devices[0], 0x0001, 0x0005);
+        assert_not_equals(collection, undefined, 'game pad collection');
+
         // Input report 0x03
         const input03 = getReport(devices, 'input', 0x03);
         assert_not_equals(input03, undefined, 'input report 0x03');