[usb] Support composite devices in the new Windows backend

This change adds basic support for composite devices to the new Windows
USB backend. The set of device paths for each function driver are
collected into a map which can be used instead of the root device path
when trying to open a WinUSB interface handle.

As with the previous patch this still won't work for devices the are
enumerated while Chrome is running because the device nodes for each USB
function are not yet available. This will be addressed next.

Bug: 637404
Change-Id: Ifbda29ef3cbe4dda2adb25fe8454dcc2a7c2036b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2138260
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757524}

services/device/usb/BUILD.gn

@@ -108,6 +108,8 @@ static_library("usb") {
       "setupapi.lib",
       "winusb.lib",
     ]
+
+    deps += [ "//third_party/re2" ]
   }
 
   if (is_android || is_chromeos || is_linux) {

services/device/usb/DEPS

@@ -3,6 +3,6 @@ include_rules = [
   "+chromeos",
   "+dbus",
   "+services/device/usb/jni_headers",
-
   "+third_party/libusb",
+  "+third_party/re2",
 ]

services/device/usb/README.md

@@ -51,3 +51,57 @@ Unit tests using the gadget can be run manually with a hardware "gadget". These
 unit tests all call [UsbTestGadget::Claim].
 
 [UsbTestGadget::Claim]: https://cs.chromium.org/search/?q=UsbTestGadget::Claim&type=cs
+
+
+### Manual Testing
+
+When making changes to platform-specific code the following manual test steps
+should be run to augment automated testing, which is mostly limited to
+platform-independent logic. These tests require an Android phone with USB
+debugging enabled and support for USB tethering. When USB debugging is enabled
+the device creates a vendor-specific interface for the ADB protocol. On
+Windows, with the [OEM USB drivers] installed this interface will have the
+WinUSB.sys driver loaded. When USB tethering is enabled the device creates
+an RNDIS interface for which most operating systems have a built-in driver.
+With both of these features enabled the device will have two interfaces and
+thus be a "composite" device. This is important for testing on Windows as
+composite and non-composite devices must be handled differently.
+
+[OEM USB drivers]: https://developer.android.com/studio/run/oem-usb
+
+#### Steps
+
+1.  [Enable USB debugging] and check that the **USB tethering** option is
+    disabled under **Network & Internet > Hotspot & Tethering** in the
+    phone's setting app.
+2.  Connect phone to the system under test.
+3.  Launch Chrome.
+4.  Load `chrome://usb-internals`.
+5.  Select the **Devices** tab.
+6.  Find the phone in the list. Ensure that the **Manufacturer name**,
+    **Product name** and **Serial number** columns are all populated for this
+    device.
+7.  Click the **Inspect** button next to this device.
+8.  Click the **Get Device Descriptor** button at the bottom of the page.
+9.  Click the **GET** buttons next to **Manufacturer String**,
+    **Product String** and **Serial number** fields.
+10. Check that the values which appear match the ones seen previously.
+11. Load `chrome://inspect` and ensure that the **Discover USB devices**
+    option is checked.
+12. Check that the phone appears as an available device. It may appear as
+    "Offline" until the **Allow** button is tapped on the "Allow USB debugging"
+    prompt which appears on the device. It may take some time for the device to
+    appear.
+13. Launch Chrome on the phone and ensure that the tabs open on the phone are
+    available for inspection.
+14. Enable USB tethering on the phone and repeat steps 4 through 13. This will
+    test hotplugging of a composite device as enabling USB tethering causes
+    the device to reconnect with multiple interfaces.
+15. Disable USB tethering on the phone and repeat steps 4 through 13. This will
+    test hotplugging of a non-composite device as disabling USB tethering
+    causes the device to reconnect with a single interface.
+16. Close Chrome and re-enable USB tethering on the phone.
+17. Repeat steps 3 through 13 for a final time. This will test enumeration of a
+    composite device on Chrome launch.
+
+[Enable USB debugging]: https://developer.android.com/studio/debug/dev-options#enable
\ No newline at end of file

services/device/usb/usb_device_handle_win.cc

@@ -488,9 +488,9 @@ const mojom::UsbInterfaceInfo* UsbDeviceHandleWin::FindInterfaceByEndpoint(
 UsbDeviceHandleWin::UsbDeviceHandleWin(scoped_refptr<UsbDeviceWin> device,
                                        bool composite)
     : device_(std::move(device)),
+      composite_(composite),
       task_runner_(base::SequencedTaskRunnerHandle::Get()),
       blocking_task_runner_(UsbService::CreateBlockingTaskRunner()) {
-  DCHECK(!composite);
   // Windows only supports configuration 1, which therefore must be active.
   DCHECK(device_->GetActiveConfiguration());
 
@@ -504,6 +504,13 @@ UsbDeviceHandleWin::UsbDeviceHandleWin(scoped_refptr<UsbDeviceWin> device,
       interface_info.first_interface = interface->first_interface;
       RegisterEndpoints(
           CombinedInterfaceInfo(interface.get(), alternate.get()));
+
+      if (composite_ &&
+          interface->interface_number == interface->first_interface) {
+        auto it = device_->function_paths().find(interface->interface_number);
+        if (it != device_->function_paths().end())
+          interface_info.function_path = it->second;
+      }
     }
   }
 }
@@ -511,11 +518,12 @@ UsbDeviceHandleWin::UsbDeviceHandleWin(scoped_refptr<UsbDeviceWin> device,
 UsbDeviceHandleWin::UsbDeviceHandleWin(scoped_refptr<UsbDeviceWin> device,
                                        base::win::ScopedHandle handle)
     : device_(std::move(device)),
+      composite_(false),
       hub_handle_(std::move(handle)),
       task_runner_(base::SequencedTaskRunnerHandle::Get()),
       blocking_task_runner_(UsbService::CreateBlockingTaskRunner()) {}
 
-UsbDeviceHandleWin::~UsbDeviceHandleWin() {}
+UsbDeviceHandleWin::~UsbDeviceHandleWin() = default;
 
 bool UsbDeviceHandleWin::OpenInterfaceHandle(Interface* interface) {
   if (interface->handle.IsValid())
@@ -524,12 +532,25 @@ bool UsbDeviceHandleWin::OpenInterfaceHandle(Interface* interface) {
   WINUSB_INTERFACE_HANDLE handle;
   if (interface->first_interface == interface->interface_number) {
     if (!function_handle_.IsValid()) {
+      const base::string16* function_path;
+      if (composite_) {
+        if (interface->function_path.empty()) {
+          USB_LOG(ERROR) << "No WinUSB interface for interface "
+                         << static_cast<int>(interface->interface_number)
+                         << ".";
+          return false;
+        }
+        function_path = &interface->function_path;
+      } else {
+        function_path = &device_->device_path();
+      }
+
       function_handle_.Set(CreateFile(
-          device_->device_path().c_str(), GENERIC_READ | GENERIC_WRITE,
-          FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING,
-          FILE_FLAG_OVERLAPPED, nullptr));
+          function_path->c_str(), GENERIC_READ | GENERIC_WRITE,
+          FILE_SHARE_READ | FILE_SHARE_WRITE, /*lpSecurityAttributes=*/nullptr,
+          OPEN_EXISTING, FILE_FLAG_OVERLAPPED, /*hTemplateFile=*/nullptr));
       if (!function_handle_.IsValid()) {
-        USB_PLOG(ERROR) << "Failed to open " << device_->device_path();
+        USB_PLOG(ERROR) << "Failed to open " << *function_path;
         return false;
       }
     }
@@ -583,6 +604,8 @@ WINUSB_INTERFACE_HANDLE UsbDeviceHandleWin::GetInterfaceForControlTransfer(
     UsbControlTransferRecipient recipient,
     uint16_t index) {
   if (recipient == UsbControlTransferRecipient::ENDPOINT) {
+    // By convention the lower bits of the wIndex field indicate the target
+    // endpoint.
     auto endpoint_it = endpoints_.find(index & 0xff);
     if (endpoint_it == endpoints_.end())
       return INVALID_HANDLE_VALUE;
@@ -592,19 +615,32 @@ WINUSB_INTERFACE_HANDLE UsbDeviceHandleWin::GetInterfaceForControlTransfer(
     index = endpoint_it->second.interface->interface_number;
   }
 
-  Interface* interface;
+  Interface* interface = nullptr;
   if (recipient == UsbControlTransferRecipient::INTERFACE) {
+    // By convention the lower bits of the wIndex field indicate the target
+    // interface.
     auto interface_it = interfaces_.find(index & 0xff);
     if (interface_it == interfaces_.end())
       return INVALID_HANDLE_VALUE;
 
     interface = &interface_it->second;
-  } else {
-    // TODO: To support composite devices a particular function handle must be
-    // chosen, probably arbitrarily.
-    interface = &interfaces_[0];
+  } else if (composite_) {
+    // For all other recipients any interface can be used but if the device
+    // is composite, then a function with the WinUSB driver loaded must be
+    // found.
+    for (auto& map_entry : interfaces_) {
+      if (!map_entry.second.function_path.empty())
+        interface = &map_entry.second;
+    }
+  } else if (!interfaces_.empty()) {
+    // For a non-composite device there is only a single device path to
+    // choose from so just pick the first interface.
+    interface = &interfaces_.begin()->second;
   }
 
+  if (!interface)
+    return INVALID_HANDLE_VALUE;
+
   OpenInterfaceHandle(interface);
   return interface->handle.Get();
 }

services/device/usb/usb_device_handle_win.h

@@ -91,7 +91,15 @@ class UsbDeviceHandleWin : public UsbDeviceHandle {
     ~Interface();
 
     uint8_t interface_number;
+
+    // If this interface is part of a function then this will be the interface
+    // number of the first interface in that function. Otherwise it will be
+    // equal to |interface_number|.
     uint8_t first_interface;
+
+    // In a composite device each function has its own driver and path to open.
+    base::string16 function_path;
+
     ScopedWinUsbHandle handle;
     bool claimed = false;
     uint8_t alternate_setting = 0;
@@ -143,6 +151,7 @@ class UsbDeviceHandleWin : public UsbDeviceHandle {
   SEQUENCE_CHECKER(sequence_checker_);
 
   scoped_refptr<UsbDeviceWin> device_;
+  const bool composite_;
 
   // |hub_handle_| or all the handles for claimed interfaces in |interfaces_|
   // must outlive their associated |requests_| because individual Request

services/device/usb/usb_device_win.cc

@@ -27,14 +27,14 @@ const uint16_t kUsbVersion2_1 = 0x0210;
 UsbDeviceWin::UsbDeviceWin(
     const base::string16& device_path,
     const base::string16& hub_path,
-    const std::vector<base::string16>& child_device_paths,
+    const base::flat_map<int, base::string16>& function_paths,
     uint32_t bus_number,
     uint32_t port_number,
     const base::string16& driver_name)
     : UsbDevice(bus_number, port_number),
       device_path_(device_path),
       hub_path_(hub_path),
-      child_device_paths_(child_device_paths),
+      function_paths_(function_paths),
       driver_name_(driver_name) {}
 
 UsbDeviceWin::~UsbDeviceWin() {}
@@ -44,10 +44,9 @@ void UsbDeviceWin::Open(OpenCallback callback) {
 
   scoped_refptr<UsbDeviceHandle> device_handle;
   if (base::EqualsCaseInsensitiveASCII(driver_name_, L"winusb"))
-    device_handle = new UsbDeviceHandleWin(this, false);
-  // TODO: Support composite devices.
-  // else if (base::EqualsCaseInsensitiveASCII(driver_name_, "usbccgp"))
-  //  device_handle = new UsbDeviceHandleWin(this, true);
+    device_handle = new UsbDeviceHandleWin(this, /*composite=*/false);
+  else if (base::EqualsCaseInsensitiveASCII(driver_name_, L"usbccgp"))
+    device_handle = new UsbDeviceHandleWin(this, /*composite=*/true);
 
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(std::move(callback), device_handle));

services/device/usb/usb_device_win.h

@@ -9,6 +9,7 @@
 #include <memory>
 #include <string>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/sequence_checker.h"
 #include "base/strings/string16.h"
@@ -23,7 +24,7 @@ class UsbDeviceWin : public UsbDevice {
  public:
   UsbDeviceWin(const base::string16& device_path,
                const base::string16& hub_path,
-               const std::vector<base::string16>& child_device_paths,
+               const base::flat_map<int, base::string16>& child_device_paths,
                uint32_t bus_number,
                uint32_t port_number,
                const base::string16& driver_name);
@@ -38,8 +39,8 @@ class UsbDeviceWin : public UsbDevice {
   ~UsbDeviceWin() override;
 
   const base::string16& device_path() const { return device_path_; }
-  const std::vector<base::string16>& child_device_paths() const {
-    return child_device_paths_;
+  const base::flat_map<int, base::string16>& function_paths() const {
+    return function_paths_;
   }
   const base::string16& driver_name() const { return driver_name_; }
 
@@ -76,7 +77,7 @@ class UsbDeviceWin : public UsbDevice {
 
   const base::string16 device_path_;
   const base::string16 hub_path_;
-  const std::vector<base::string16> child_device_paths_;
+  const base::flat_map<int, base::string16> function_paths_;
   const base::string16 driver_name_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbDeviceWin);

services/device/usb/usb_service_win.cc

@@ -19,6 +19,7 @@
 #include "base/scoped_generic.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ -30,6 +31,7 @@
 #include "services/device/usb/usb_descriptors.h"
 #include "services/device/usb/usb_device_handle.h"
 #include "services/device/usb/webusb_descriptors.h"
+#include "third_party/re2/src/re2/re2.h"
 
 namespace device {
 
@@ -188,7 +190,7 @@ bool GetDeviceInterfaceDetails(HDEVINFO dev_info,
     auto result = GetDeviceStringListProperty(dev_info, &dev_info_data,
                                               DEVPKEY_Device_Children);
     if (!result.has_value()) {
-      if (GetLastError() != ERROR_NOT_FOUND) {
+      if (GetLastError() == ERROR_NOT_FOUND) {
         result.emplace();
       } else {
         USB_PLOG(ERROR) << "Failed to get device children";
@@ -238,6 +240,25 @@ base::string16 GetDevicePath(const base::string16& instance_id,
   return device_path;
 }
 
+int GetInterfaceNumber(const base::string16& instance_id) {
+  // According to MSDN the instance IDs for the device nodes created by the
+  // composite driver is in the form "USB\VID_vvvv&PID_dddd&MI_zz" where "zz"
+  // is the interface number.
+  //
+  // https://docs.microsoft.com/en-us/windows-hardware/drivers/install/standard-usb-identifiers#multiple-interface-usb-devices
+  std::string instance_id_ascii = base::UTF16ToASCII(instance_id);
+  std::string interface_number_str;
+  if (!RE2::PartialMatch(instance_id_ascii, "MI_([0-9a-fA-F]{2})",
+                         &interface_number_str)) {
+    return -1;
+  }
+
+  int interface_number;
+  if (!base::HexStringToInt(interface_number_str, &interface_number))
+    return -1;
+  return interface_number;
+}
+
 base::string16 GetWinUsbDevicePath(const base::string16& instance_id) {
   ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr));
   if (!dev_info.is_valid()) {
@@ -381,12 +402,14 @@ class UsbServiceWin::BlockingTaskRunnerHelper {
     // child device notes for each of the device functions. It is these device
     // paths for these children which must be opened in order to communicate
     // with the WinUSB driver.
-    std::vector<base::string16> child_device_paths;
+    std::vector<std::pair<int, base::string16>> function_paths;
     if (base::EqualsCaseInsensitiveASCII(service_name, L"usbccgp")) {
       for (const base::string16& instance_id : child_instance_ids) {
-        base::string16 child_device_path = GetWinUsbDevicePath(instance_id);
-        if (!child_device_path.empty())
-          child_device_paths.push_back(std::move(child_device_path));
+        int interface_number = GetInterfaceNumber(instance_id);
+        if (interface_number != -1) {
+          function_paths.emplace_back(interface_number,
+                                      GetWinUsbDevicePath(instance_id));
+        }
       }
     }
 
@@ -399,8 +422,9 @@ class UsbServiceWin::BlockingTaskRunnerHelper {
 
     service_task_runner_->PostTask(
         FROM_HERE, base::BindOnce(&UsbServiceWin::CreateDeviceObject, service_,
-                                  device_path, hub_path, child_device_paths,
-                                  bus_number, port_number, service_name));
+                                  std::move(device_path), std::move(hub_path),
+                                  std::move(function_paths), bus_number,
+                                  port_number, std::move(service_name)));
   }
 
  private:
@@ -485,7 +509,7 @@ void UsbServiceWin::HelperStarted() {
 void UsbServiceWin::CreateDeviceObject(
     const base::string16& device_path,
     const base::string16& hub_path,
-    const std::vector<base::string16>& child_device_paths,
+    const base::flat_map<int, base::string16>& function_paths,
     uint32_t bus_number,
     uint32_t port_number,
     const base::string16& driver_name) {
@@ -495,9 +519,9 @@ void UsbServiceWin::CreateDeviceObject(
   if (!enumeration_ready())
     ++first_enumeration_countdown_;
 
-  auto device = base::MakeRefCounted<UsbDeviceWin>(
-      device_path, hub_path, child_device_paths, bus_number, port_number,
-      driver_name);
+  auto device =
+      base::MakeRefCounted<UsbDeviceWin>(device_path, hub_path, function_paths,
+                                         bus_number, port_number, driver_name);
   devices_by_path_[device->device_path()] = device;
   device->ReadDescriptors(base::BindOnce(&UsbServiceWin::DeviceReady,
                                          weak_factory_.GetWeakPtr(), device));
@@ -530,9 +554,7 @@ void UsbServiceWin::DeviceReady(scoped_refptr<UsbDeviceWin> device,
                   << "\", product=" << device->product_id() << " \""
                   << device->product_string() << "\", serial=\""
                   << device->serial_number() << "\", driver=\""
-                  << device->driver_name() << "\", children=["
-                  << base::JoinString(device->child_device_paths(), L", ")
-                  << "], guid=" << device->guid();
+                  << device->driver_name() << "\", guid=" << device->guid();
   } else {
     devices_by_path_.erase(it);
   }

services/device/usb/usb_service_win.h

@@ -10,6 +10,7 @@
 #include <list>
 #include <unordered_map>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
@@ -40,12 +41,13 @@ class UsbServiceWin final : public DeviceMonitorWin::Observer,
 
   // Methods called by BlockingThreadHelper
   void HelperStarted();
-  void CreateDeviceObject(const base::string16& device_path,
-                          const base::string16& hub_path,
-                          const std::vector<base::string16>& child_device_paths,
-                          uint32_t bus_number,
-                          uint32_t port_number,
-                          const base::string16& driver_name);
+  void CreateDeviceObject(
+      const base::string16& device_path,
+      const base::string16& hub_path,
+      const base::flat_map<int, base::string16>& function_paths,
+      uint32_t bus_number,
+      uint32_t port_number,
+      const base::string16& driver_name);
 
   void DeviceReady(scoped_refptr<UsbDeviceWin> device, bool success);