Use mojo type UsbDeviceInfo in UsbDevice and UsbDeviceDescriptor.

This CL uses mojo type UsbDeviceInfo in UsbDevice instead of the
custom type UsbConfigDescriptor and adjusted related code.

TBR=thestig@chromium.org

Bug: 716557, 699790
Change-Id: Ibade21adacc1c12fbebedad8271ea162bc6ef0f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1703473
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678566}

chrome/browser/chromeos/printing/usb_printer_util.cc

@@ -157,17 +157,6 @@ void MD5UpdateString16(base::MD5Context* ctx, const base::string16& str) {
   base::MD5Update(ctx, base::StringPiece(tmp.data(), tmp.size()));
 }
 
-uint16_t GetUsbVersion(const UsbDeviceInfo& device_info) {
-  return device_info.usb_version_major << 8 |
-         device_info.usb_version_minor << 4 | device_info.usb_version_subminor;
-}
-
-uint16_t GetDeviceVersion(const UsbDeviceInfo& device_info) {
-  return device_info.device_version_major << 8 |
-         device_info.device_version_minor << 4 |
-         device_info.device_version_subminor;
-}
-
 // Get the usb printer id for |device|.  This is used both as the identifier for
 // the printer in the user's PrintersManager and as the name of the printer in
 // CUPS, so it has to satisfy the naming restrictions of both.  CUPS in
@@ -186,7 +175,7 @@ std::string CreateUsbPrinterId(const UsbDeviceInfo& device_info) {
                 "Protocol size changed");
   static_assert(sizeof(device_info.vendor_id) == 2, "Vendor id size changed");
   static_assert(sizeof(device_info.product_id) == 2, "Product id size changed");
-  static_assert(sizeof(GetDeviceVersion(device_info)) == 2,
+  static_assert(sizeof(device::GetDeviceVersion(device_info)) == 2,
                 "Version size changed");
 
   base::MD5Context ctx;
@@ -196,7 +185,7 @@ std::string CreateUsbPrinterId(const UsbDeviceInfo& device_info) {
   MD5UpdateBigEndian(&ctx, device_info.protocol_code);
   MD5UpdateBigEndian(&ctx, device_info.vendor_id);
   MD5UpdateBigEndian(&ctx, device_info.product_id);
-  MD5UpdateBigEndian(&ctx, GetDeviceVersion(device_info));
+  MD5UpdateBigEndian(&ctx, device::GetDeviceVersion(device_info));
   MD5UpdateString16(&ctx, GetManufacturerName(device_info));
   MD5UpdateString16(&ctx, GetProductName(device_info));
   MD5UpdateString16(&ctx, GetSerialNumber(device_info));
@@ -239,10 +228,10 @@ std::string UsbPrinterDeviceDetailsAsString(const UsbDeviceInfo& device_info) {
       " manufacturer string: %s\n"
       " product string:      %s\n"
       " serial number:       %s",
-      device_info.guid.c_str(), GetUsbVersion(device_info),
+      device_info.guid.c_str(), device::GetUsbVersion(device_info),
       device_info.class_code, device_info.subclass_code,
       device_info.protocol_code, device_info.vendor_id, device_info.product_id,
-      GetDeviceVersion(device_info),
+      device::GetDeviceVersion(device_info),
       base::UTF16ToUTF8(GetManufacturerName(device_info)).c_str(),
       base::UTF16ToUTF8(GetProductName(device_info)).c_str(),
       base::UTF16ToUTF8(GetSerialNumber(device_info)).c_str());

services/device/public/cpp/usb/usb_utils.cc

@@ -89,4 +89,15 @@ uint8_t ConvertEndpointNumberToAddress(
                                         mojo_endpoint.direction);
 }
 
+uint16_t GetUsbVersion(const mojom::UsbDeviceInfo& device_info) {
+  return device_info.usb_version_major << 8 |
+         device_info.usb_version_minor << 4 | device_info.usb_version_subminor;
+}
+
+uint16_t GetDeviceVersion(const mojom::UsbDeviceInfo& device_info) {
+  return device_info.device_version_major << 8 |
+         device_info.device_version_minor << 4 |
+         device_info.device_version_subminor;
+}
+
 }  // namespace device

services/device/public/cpp/usb/usb_utils.h

@@ -31,6 +31,10 @@ uint8_t ConvertEndpointNumberToAddress(uint8_t endpoint_number,
 uint8_t ConvertEndpointNumberToAddress(
     const mojom::UsbEndpointInfo& mojo_endpoint);
 
+uint16_t GetUsbVersion(const mojom::UsbDeviceInfo& device_info);
+
+uint16_t GetDeviceVersion(const mojom::UsbDeviceInfo& device_info);
+
 }  // namespace device
 
 #endif  // SERVICES_DEVICE_PUBLIC_CPP_USB_USB_UTILS_H_

services/device/usb/mock_usb_device.cc

@@ -4,6 +4,8 @@
 
 #include "services/device/usb/mock_usb_device.h"
 
+#include <utility>
+
 #include "base/strings/utf_string_conversions.h"
 
 namespace device {
@@ -32,7 +34,7 @@ MockUsbDevice::MockUsbDevice(uint16_t vendor_id,
 MockUsbDevice::~MockUsbDevice() = default;
 
 void MockUsbDevice::AddMockConfig(mojom::UsbConfigurationInfoPtr config) {
-  descriptor_.configurations.push_back(std::move(config));
+  device_info_->configurations.push_back(std::move(config));
 }
 
 void MockUsbDevice::ActiveConfigurationChanged(int configuration_value) {

services/device/usb/mojo/device_manager_impl.cc

@@ -82,7 +82,7 @@ void DeviceManagerImpl::RefreshDeviceInfo(const std::string& guid,
   }
 
   if (device->permission_granted()) {
-    std::move(callback).Run(mojom::UsbDeviceInfo::From(*device));
+    std::move(callback).Run(device->device_info().Clone());
     return;
   }
 
@@ -101,7 +101,7 @@ void DeviceManagerImpl::OnPermissionGrantedToRefresh(
     return;
   }
 
-  std::move(callback).Run(mojom::UsbDeviceInfo::From(*device));
+  std::move(callback).Run(device->device_info().Clone());
 }
 #endif  // defined(OS_ANDROID)
 
@@ -174,9 +174,8 @@ void DeviceManagerImpl::OnGetDevices(
 
   std::vector<mojom::UsbDeviceInfoPtr> device_infos;
   for (const auto& device : devices) {
-    mojom::UsbDeviceInfoPtr device_info = mojom::UsbDeviceInfo::From(*device);
-    if (UsbDeviceFilterMatchesAny(filters, *device_info)) {
-      device_infos.push_back(std::move(device_info));
+    if (UsbDeviceFilterMatchesAny(filters, device->device_info())) {
+      device_infos.push_back(device->device_info().Clone());
     }
   }
 
@@ -187,18 +186,14 @@ void DeviceManagerImpl::OnGetDevices(
 }
 
 void DeviceManagerImpl::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
-  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
-  DCHECK(device_info);
-  clients_.ForAllPtrs([&device_info](mojom::UsbDeviceManagerClient* client) {
-    client->OnDeviceAdded(device_info->Clone());
+  clients_.ForAllPtrs([&device](mojom::UsbDeviceManagerClient* client) {
+    client->OnDeviceAdded(device->device_info().Clone());
   });
 }
 
 void DeviceManagerImpl::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
-  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
-  DCHECK(device_info);
-  clients_.ForAllPtrs([&device_info](mojom::UsbDeviceManagerClient* client) {
-    client->OnDeviceRemoved(device_info->Clone());
+  clients_.ForAllPtrs([&device](mojom::UsbDeviceManagerClient* client) {
+    client->OnDeviceRemoved(device->device_info().Clone());
   });
 }
 

services/device/usb/mojo/device_manager_test.cc

@@ -46,7 +46,7 @@ TestUsbDevice::TestUsbDevice(const std::string& name,
                 base::UTF8ToUTF16(serial_number),
                 0,
                 0) {
-  webusb_landing_page_ = landing_page;
+  device_info_->webusb_landing_page = landing_page;
 }
 
 void TestUsbDevice::Open(OpenCallback callback) {

services/device/usb/mojo/type_converters.cc

@@ -10,46 +10,8 @@
 #include <map>
 #include <utility>
 
-#include "services/device/public/cpp/usb/usb_utils.h"
-#include "services/device/usb/usb_descriptors.h"
-#include "services/device/usb/usb_device.h"
-
 namespace mojo {
 
-// static
-device::mojom::UsbDeviceInfoPtr
-TypeConverter<device::mojom::UsbDeviceInfoPtr, device::UsbDevice>::Convert(
-    const device::UsbDevice& device) {
-  auto info = device::mojom::UsbDeviceInfo::New();
-  info->guid = device.guid();
-  info->usb_version_major = device.usb_version() >> 8;
-  info->usb_version_minor = device.usb_version() >> 4 & 0xf;
-  info->usb_version_subminor = device.usb_version() & 0xf;
-  info->class_code = device.device_class();
-  info->subclass_code = device.device_subclass();
-  info->protocol_code = device.device_protocol();
-  info->bus_number = device.bus_number();
-  info->port_number = device.port_number();
-  info->vendor_id = device.vendor_id();
-  info->product_id = device.product_id();
-  info->device_version_major = device.device_version() >> 8;
-  info->device_version_minor = device.device_version() >> 4 & 0xf;
-  info->device_version_subminor = device.device_version() & 0xf;
-  info->manufacturer_name = device.manufacturer_string();
-  info->product_name = device.product_string();
-  info->serial_number = device.serial_number();
-  info->webusb_landing_page = device.webusb_landing_page();
-  const device::mojom::UsbConfigurationInfo* config =
-      device.active_configuration();
-  info->active_configuration = config ? config->configuration_value : 0;
-
-  info->configurations.reserve(device.configurations().size());
-  for (const auto& config : device.configurations()) {
-    info->configurations.push_back(config->Clone());
-  }
-  return info;
-}
-
 // static
 device::mojom::UsbIsochronousPacketPtr
 TypeConverter<device::mojom::UsbIsochronousPacketPtr,

services/device/usb/mojo/type_converters.h

@@ -16,18 +16,8 @@
 // public Mojo interface data types. This must be included by any source file
 // that uses these conversions explicitly or implicitly.
 
-namespace device {
-class UsbDevice;
-}  // namespace device
-
 namespace mojo {
 
-template <>
-struct TypeConverter<device::mojom::UsbDeviceInfoPtr, device::UsbDevice> {
-  static device::mojom::UsbDeviceInfoPtr Convert(
-      const device::UsbDevice& device);
-};
-
 template <>
 struct TypeConverter<device::mojom::UsbIsochronousPacketPtr,
                      device::UsbDeviceHandle::IsochronousPacket> {

services/device/usb/usb_descriptors.cc

@@ -81,12 +81,12 @@ void OnDoneReadingConfigDescriptors(
     scoped_refptr<UsbDeviceHandle> device_handle,
     std::unique_ptr<UsbDeviceDescriptor> desc,
     base::OnceCallback<void(std::unique_ptr<UsbDeviceDescriptor>)> callback) {
-  if (desc->num_configurations == desc->configurations.size()) {
+  if (desc->num_configurations == desc->device_info->configurations.size()) {
     std::move(callback).Run(std::move(desc));
   } else {
     LOG(ERROR) << "Failed to read all configuration descriptors. Expected "
                << static_cast<int>(desc->num_configurations) << ", got "
-               << desc->configurations.size() << ".";
+               << desc->device_info->configurations.size() << ".";
     std::move(callback).Run(nullptr);
   }
 }
@@ -244,46 +244,8 @@ bool CombinedInterfaceInfo::IsValid() const {
   return interface && alternate;
 }
 
-UsbDeviceDescriptor::UsbDeviceDescriptor() = default;
-
-// Do a deep copy especially for |configurations|.
-UsbDeviceDescriptor::UsbDeviceDescriptor(const UsbDeviceDescriptor& other) {
-  usb_version = other.usb_version;
-  device_class = other.device_class;
-  device_subclass = other.device_subclass;
-  device_protocol = other.device_protocol;
-  vendor_id = other.vendor_id;
-  product_id = other.product_id;
-  device_version = other.device_version;
-  i_manufacturer = other.i_manufacturer;
-  i_product = other.i_product;
-  i_serial_number = other.i_serial_number;
-  num_configurations = other.num_configurations;
-
-  for (const auto& config : other.configurations) {
-    configurations.push_back(config->Clone());
-  }
-}
-
-UsbDeviceDescriptor::UsbDeviceDescriptor(UsbDeviceDescriptor&& other) = default;
-
-UsbDeviceDescriptor& UsbDeviceDescriptor::operator=(
-    UsbDeviceDescriptor&& other) {
-  usb_version = other.usb_version;
-  device_class = other.device_class;
-  device_subclass = other.device_subclass;
-  device_protocol = other.device_protocol;
-  vendor_id = other.vendor_id;
-  product_id = other.product_id;
-  device_version = other.device_version;
-  i_manufacturer = other.i_manufacturer;
-  i_product = other.i_product;
-  i_serial_number = other.i_serial_number;
-  num_configurations = other.num_configurations;
-  configurations.swap(other.configurations);
-
-  return *this;
-}
+UsbDeviceDescriptor::UsbDeviceDescriptor()
+    : device_info(mojom::UsbDeviceInfo::New()) {}
 
 UsbDeviceDescriptor::~UsbDeviceDescriptor() = default;
 
@@ -302,15 +264,21 @@ bool UsbDeviceDescriptor::Parse(const std::vector<uint8_t>& buffer) {
 
     switch (data[1] /* bDescriptorType */) {
       case kDeviceDescriptorType:
-        if (configurations.size() > 0 || length < kDeviceDescriptorLength)
+        if (device_info->configurations.size() > 0 ||
+            length < kDeviceDescriptorLength) {
           return false;
-        usb_version = data[2] | data[3] << 8;
-        device_class = data[4];
-        device_subclass = data[5];
-        device_protocol = data[6];
-        vendor_id = data[8] | data[9] << 8;
-        product_id = data[10] | data[11] << 8;
-        device_version = data[12] | data[13] << 8;
+        }
+        device_info->usb_version_minor = data[2] >> 4 & 0xf;
+        device_info->usb_version_subminor = data[2] & 0xf;
+        device_info->usb_version_major = data[3];
+        device_info->class_code = data[4];
+        device_info->subclass_code = data[5];
+        device_info->protocol_code = data[6];
+        device_info->vendor_id = data[8] | data[9] << 8;
+        device_info->product_id = data[10] | data[11] << 8;
+        device_info->device_version_minor = data[12] >> 4 & 0xf;
+        device_info->device_version_subminor = data[12] & 0xf;
+        device_info->device_version_major = data[13];
         i_manufacturer = data[14];
         i_product = data[15];
         i_serial_number = data[16];
@@ -323,8 +291,9 @@ bool UsbDeviceDescriptor::Parse(const std::vector<uint8_t>& buffer) {
           AssignFirstInterfaceNumbers(last_config);
           AggregateInterfacesForConfig(last_config);
         }
-        configurations.push_back(BuildUsbConfigurationInfoPtr(data));
-        last_config = configurations.back().get();
+        device_info->configurations.push_back(
+            BuildUsbConfigurationInfoPtr(data));
+        last_config = device_info->configurations.back().get();
         last_interface = nullptr;
         last_endpoint = nullptr;
         break;

services/device/usb/usb_descriptors.h

@@ -28,6 +28,7 @@ using UsbEndpointInfoPtr = mojom::UsbEndpointInfoPtr;
 using UsbAlternateInterfaceInfoPtr = mojom::UsbAlternateInterfaceInfoPtr;
 using UsbInterfaceInfoPtr = mojom::UsbInterfaceInfoPtr;
 using UsbConfigurationInfoPtr = mojom::UsbConfigurationInfoPtr;
+using UsbDeviceInfoPtr = mojom::UsbDeviceInfoPtr;
 
 struct CombinedInterfaceInfo {
   CombinedInterfaceInfo() = default;
@@ -42,12 +43,9 @@ struct CombinedInterfaceInfo {
 
 struct UsbDeviceDescriptor {
   UsbDeviceDescriptor();
-  UsbDeviceDescriptor(const UsbDeviceDescriptor& other);
-  UsbDeviceDescriptor(UsbDeviceDescriptor&& other);
+  UsbDeviceDescriptor(const UsbDeviceDescriptor& other) = delete;
   ~UsbDeviceDescriptor();
 
-  UsbDeviceDescriptor& operator=(UsbDeviceDescriptor&& other);
-
   // Parses |buffer| for USB descriptors. Any configuration descriptors found
   // will be added to |configurations|. If a device descriptor is found it will
   // be used to populate this struct's fields. This function may be called more
@@ -55,18 +53,11 @@ struct UsbDeviceDescriptor {
   // each).
   bool Parse(const std::vector<uint8_t>& buffer);
 
-  uint16_t usb_version = 0;
-  uint8_t device_class = 0;
-  uint8_t device_subclass = 0;
-  uint8_t device_protocol = 0;
-  uint16_t vendor_id = 0;
-  uint16_t product_id = 0;
-  uint16_t device_version = 0;
   uint8_t i_manufacturer = 0;
   uint8_t i_product = 0;
   uint8_t i_serial_number = 0;
   uint8_t num_configurations = 0;
-  std::vector<UsbConfigurationInfoPtr> configurations;
+  UsbDeviceInfoPtr device_info;
 };
 
 void ReadUsbDescriptors(

services/device/usb/usb_descriptors_unittest.cc

@@ -183,16 +183,20 @@ void ExpectConfig2Info(const mojom::UsbConfigurationInfo& config) {
 
 void ExpectDeviceDescriptor(const UsbDeviceDescriptor& descriptor) {
   // Device
-  EXPECT_EQ(0x0310, descriptor.usb_version);
-  EXPECT_EQ(0xFF, descriptor.device_class);
-  EXPECT_EQ(0xFF, descriptor.device_subclass);
-  EXPECT_EQ(0xFF, descriptor.device_protocol);
-  EXPECT_EQ(0x1234, descriptor.vendor_id);
-  EXPECT_EQ(0x5678, descriptor.product_id);
-  EXPECT_EQ(0x0100, descriptor.device_version);
-  ASSERT_EQ(2u, descriptor.configurations.size());
-  ExpectConfig1Info(*descriptor.configurations[0]);
-  ExpectConfig2Info(*descriptor.configurations[1]);
+  EXPECT_EQ(0x03, descriptor.device_info->usb_version_major);
+  EXPECT_EQ(0x01, descriptor.device_info->usb_version_minor);
+  EXPECT_EQ(0x00, descriptor.device_info->usb_version_subminor);
+  EXPECT_EQ(0xFF, descriptor.device_info->class_code);
+  EXPECT_EQ(0xFF, descriptor.device_info->subclass_code);
+  EXPECT_EQ(0xFF, descriptor.device_info->protocol_code);
+  EXPECT_EQ(0x1234, descriptor.device_info->vendor_id);
+  EXPECT_EQ(0x5678, descriptor.device_info->product_id);
+  EXPECT_EQ(0x01, descriptor.device_info->device_version_major);
+  EXPECT_EQ(0x00, descriptor.device_info->device_version_minor);
+  EXPECT_EQ(0x00, descriptor.device_info->device_version_subminor);
+  ASSERT_EQ(2u, descriptor.device_info->configurations.size());
+  ExpectConfig1Info(*descriptor.device_info->configurations[0]);
+  ExpectConfig2Info(*descriptor.device_info->configurations[1]);
 }
 
 void OnReadDescriptors(std::unique_ptr<UsbDeviceDescriptor> descriptor) {

services/device/usb/usb_device.cc

@@ -4,7 +4,10 @@
 
 #include "services/device/usb/usb_device.h"
 
+#include <utility>
+
 #include "base/guid.h"
+#include "services/device/public/cpp/usb/usb_utils.h"
 #include "services/device/usb/usb_device_handle.h"
 #include "services/device/usb/webusb_descriptors.h"
 
@@ -14,24 +17,19 @@ UsbDevice::Observer::~Observer() = default;
 
 void UsbDevice::Observer::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {}
 
-UsbDevice::UsbDevice(uint32_t bus_number, uint32_t port_number)
-    : bus_number_(bus_number),
-      port_number_(port_number),
-      guid_(base::GenerateGUID()) {}
+UsbDevice::UsbDevice(uint32_t bus_number, uint32_t port_number) {
+  device_info_ = mojom::UsbDeviceInfo::New();
+  device_info_->guid = base::GenerateGUID();
+  device_info_->bus_number = bus_number;
+  device_info_->port_number = port_number;
+}
 
-UsbDevice::UsbDevice(const UsbDeviceDescriptor& descriptor,
-                     const base::string16& manufacturer_string,
-                     const base::string16& product_string,
-                     const base::string16& serial_number,
-                     uint32_t bus_number,
-                     uint32_t port_number)
-    : descriptor_(descriptor),
-      manufacturer_string_(manufacturer_string),
-      product_string_(product_string),
-      serial_number_(serial_number),
-      bus_number_(bus_number),
-      port_number_(port_number),
-      guid_(base::GenerateGUID()) {}
+UsbDevice::UsbDevice(mojom::UsbDeviceInfoPtr device_info)
+    : device_info_(std::move(device_info)) {
+  DCHECK(device_info_);
+  device_info_->guid = base::GenerateGUID();
+  ActiveConfigurationChanged(device_info_->active_configuration);
+}
 
 UsbDevice::UsbDevice(uint16_t usb_version,
                      uint8_t device_class,
@@ -44,24 +42,37 @@ UsbDevice::UsbDevice(uint16_t usb_version,
                      const base::string16& product_string,
                      const base::string16& serial_number,
                      uint32_t bus_number,
-                     uint32_t port_number)
-    : manufacturer_string_(manufacturer_string),
-      product_string_(product_string),
-      serial_number_(serial_number),
-      bus_number_(bus_number),
-      port_number_(port_number),
-      guid_(base::GenerateGUID()) {
-  descriptor_.usb_version = usb_version;
-  descriptor_.device_class = device_class;
-  descriptor_.device_subclass = device_subclass;
-  descriptor_.device_protocol = device_protocol;
-  descriptor_.vendor_id = vendor_id;
-  descriptor_.product_id = product_id;
-  descriptor_.device_version = device_version;
+                     uint32_t port_number) {
+  device_info_ = mojom::UsbDeviceInfo::New();
+  device_info_->guid = base::GenerateGUID();
+  device_info_->usb_version_major = usb_version >> 8;
+  device_info_->usb_version_minor = usb_version >> 4 & 0xf;
+  device_info_->usb_version_subminor = usb_version & 0xf;
+  device_info_->class_code = device_class;
+  device_info_->subclass_code = device_subclass;
+  device_info_->protocol_code = device_protocol;
+  device_info_->vendor_id = vendor_id;
+  device_info_->product_id = product_id;
+  device_info_->device_version_major = device_version >> 8;
+  device_info_->device_version_minor = device_version >> 4 & 0xf;
+  device_info_->device_version_subminor = device_version & 0xf;
+  device_info_->manufacturer_name = manufacturer_string;
+  device_info_->product_name = product_string;
+  device_info_->serial_number = serial_number;
+  device_info_->bus_number = bus_number;
+  device_info_->port_number = port_number;
 }
 
 UsbDevice::~UsbDevice() = default;
 
+uint16_t UsbDevice::usb_version() const {
+  return GetUsbVersion(*device_info_);
+}
+
+uint16_t UsbDevice::device_version() const {
+  return GetDeviceVersion(*device_info_);
+}
+
 void UsbDevice::CheckUsbAccess(ResultCallback callback) {
   // By default assume that access to the device is allowed. This is implemented
   // on Chrome OS by checking with permission_broker.

services/device/usb/usb_device.h

@@ -46,28 +46,51 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
     virtual void OnDeviceRemoved(scoped_refptr<UsbDevice> device);
   };
 
+  const mojom::UsbDeviceInfo& device_info() const { return *device_info_; }
+
   // A unique identifier which remains stable for the lifetime of this device
   // object (i.e., until the device is unplugged or the USB service dies.)
-  const std::string& guid() const { return guid_; }
+  const std::string& guid() const { return device_info_->guid; }
 
   // Accessors to basic information.
-  uint32_t bus_number() const { return bus_number_; }
-  uint32_t port_number() const { return port_number_; }
-  uint16_t usb_version() const { return descriptor_.usb_version; }
-  uint8_t device_class() const { return descriptor_.device_class; }
-  uint8_t device_subclass() const { return descriptor_.device_subclass; }
-  uint8_t device_protocol() const { return descriptor_.device_protocol; }
-  uint16_t vendor_id() const { return descriptor_.vendor_id; }
-  uint16_t product_id() const { return descriptor_.product_id; }
-  uint16_t device_version() const { return descriptor_.device_version; }
+  uint32_t bus_number() const { return device_info_->bus_number; }
+  uint32_t port_number() const { return device_info_->port_number; }
+  uint8_t device_class() const { return device_info_->class_code; }
+  uint8_t device_subclass() const { return device_info_->subclass_code; }
+  uint8_t device_protocol() const { return device_info_->protocol_code; }
+  uint16_t vendor_id() const { return device_info_->vendor_id; }
+  uint16_t product_id() const { return device_info_->product_id; }
+
+  uint16_t usb_version() const;
+  uint16_t device_version() const;
+
   const base::string16& manufacturer_string() const {
-    return manufacturer_string_;
+    if (device_info_->manufacturer_name)
+      return *device_info_->manufacturer_name;
+
+    return base::EmptyString16();
+  }
+  const base::string16& product_string() const {
+    if (device_info_->product_name)
+      return *device_info_->product_name;
+
+    return base::EmptyString16();
+  }
+  const base::string16& serial_number() const {
+    if (device_info_->serial_number)
+      return *device_info_->serial_number;
+
+    return base::EmptyString16();
+  }
+  const GURL& webusb_landing_page() const {
+    if (device_info_->webusb_landing_page)
+      return *device_info_->webusb_landing_page;
+
+    return GURL::EmptyGURL();
   }
-  const base::string16& product_string() const { return product_string_; }
-  const base::string16& serial_number() const { return serial_number_; }
-  const GURL& webusb_landing_page() const { return webusb_landing_page_; }
+
   const std::vector<mojom::UsbConfigurationInfoPtr>& configurations() const {
-    return descriptor_.configurations;
+    return device_info_->configurations;
   }
   const mojom::UsbConfigurationInfo* active_configuration() const {
     return active_configuration_;
@@ -95,12 +118,7 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   friend class UsbService;
 
   UsbDevice(uint32_t bus_number, uint32_t port_number);
-  UsbDevice(const UsbDeviceDescriptor& descriptor,
-            const base::string16& manufacturer_string,
-            const base::string16& product_string,
-            const base::string16& serial_number,
-            uint32_t bus_number,
-            uint32_t port_number);
+  explicit UsbDevice(mojom::UsbDeviceInfoPtr device_info);
   UsbDevice(uint16_t usb_version,
             uint8_t device_class,
             uint8_t device_subclass,
@@ -120,14 +138,10 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
 
   std::list<UsbDeviceHandle*>& handles() { return handles_; }
 
-  // These members must be mutable by subclasses as necessary during device
+  // This member must be mutable by subclasses as necessary during device
   // enumeration. To preserve the thread safety of this object they must remain
   // constant afterwards.
-  UsbDeviceDescriptor descriptor_;
-  base::string16 manufacturer_string_;
-  base::string16 product_string_;
-  base::string16 serial_number_;
-  GURL webusb_landing_page_;
+  mojom::UsbDeviceInfoPtr device_info_;
 
  private:
   friend class base::RefCountedThreadSafe<UsbDevice>;
@@ -142,11 +156,6 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   void OnDisconnect();
   void HandleClosed(UsbDeviceHandle* handle);
 
-  const uint32_t bus_number_;
-  const uint32_t port_number_;
-
-  const std::string guid_;
-
   // The current device configuration descriptor. May be null if the device is
   // in an unconfigured state; if not null, it is a pointer to one of the
   // items in |descriptor_.configurations|.

services/device/usb/usb_device_android.cc

@@ -4,6 +4,10 @@
 
 #include "services/device/usb/usb_device_android.h"
 
+#include <list>
+#include <memory>
+#include <utility>
+
 #include "base/android/build_info.h"
 #include "base/android/jni_string.h"
 #include "base/bind.h"
@@ -134,9 +138,9 @@ UsbDeviceAndroid::UsbDeviceAndroid(JNIEnv* env,
       base::android::SDK_VERSION_LOLLIPOP) {
     JavaObjectArrayReader<jobject> configurations(
         Java_ChromeUsbDevice_getConfigurations(env, j_object_));
-    descriptor_.configurations.reserve(configurations.size());
+    device_info_->configurations.reserve(configurations.size());
     for (auto config : configurations) {
-      descriptor_.configurations.push_back(
+      device_info_->configurations.push_back(
           UsbConfigurationAndroid::Convert(env, config));
     }
   } else {
@@ -156,7 +160,7 @@ UsbDeviceAndroid::UsbDeviceAndroid(JNIEnv* env,
           UsbInterfaceAndroid::Convert(env, interface));
     }
     AggregateInterfacesForConfig(config.get());
-    descriptor_.configurations.push_back(std::move(config));
+    device_info_->configurations.push_back(std::move(config));
   }
 
   if (configurations().size() > 0)
@@ -174,7 +178,7 @@ void UsbDeviceAndroid::PermissionGranted(JNIEnv* env, bool granted) {
   ScopedJavaLocalRef<jstring> serial_jstring =
       Java_ChromeUsbDevice_getSerialNumber(env, j_object_);
   if (!serial_jstring.is_null())
-    serial_number_ = ConvertJavaStringToUTF16(env, serial_jstring);
+    device_info_->serial_number = ConvertJavaStringToUTF16(env, serial_jstring);
 
   Open(
       base::BindOnce(&UsbDeviceAndroid::OnDeviceOpenedToReadDescriptors, this));
@@ -209,7 +213,16 @@ void UsbDeviceAndroid::OnReadDescriptors(
     return;
   }
 
-  descriptor_ = std::move(*descriptor);
+  // Keep following members in original |device_info_| because they will
+  // not be updated in the new |descriptor->device_info|:
+  //   |bus_number|, |port_number|,
+  //   |manufacturer_string|, |product_string|, |serial_number|.
+  descriptor->device_info->bus_number = device_info_->bus_number,
+  descriptor->device_info->port_number = device_info_->port_number,
+  descriptor->device_info->manufacturer_name = device_info_->manufacturer_name,
+  descriptor->device_info->product_name = device_info_->product_name,
+  descriptor->device_info->serial_number = device_info_->serial_number,
+  device_info_ = std::move(descriptor->device_info);
 
   if (usb_version() >= 0x0210) {
     ReadWebUsbDescriptors(
@@ -226,7 +239,7 @@ void UsbDeviceAndroid::OnReadWebUsbDescriptors(
     scoped_refptr<UsbDeviceHandle> device_handle,
     const GURL& landing_page) {
   if (landing_page.is_valid())
-    webusb_landing_page_ = landing_page;
+    device_info_->webusb_landing_page = landing_page;
 
   device_handle->Close();
   CallRequestPermissionCallbacks(true);

services/device/usb/usb_device_impl.cc

@@ -68,6 +68,7 @@ void UsbDeviceImpl::ReadAllConfigurations() {
   libusb_device_descriptor device_descriptor;
   int rv = libusb_get_device_descriptor(platform_device(), &device_descriptor);
   if (rv == LIBUSB_SUCCESS) {
+    UsbDeviceDescriptor usb_descriptor;
     for (uint8_t i = 0; i < device_descriptor.bNumConfigurations; ++i) {
       unsigned char* buffer;
       rv = libusb_get_raw_config_descriptor(platform_device(), i, &buffer);
@@ -77,9 +78,13 @@ void UsbDeviceImpl::ReadAllConfigurations() {
         continue;
       }
 
-      if (!descriptor_.Parse(std::vector<uint8_t>(buffer, buffer + rv)))
+      if (!usb_descriptor.Parse(std::vector<uint8_t>(buffer, buffer + rv)))
         USB_LOG(EVENT) << "Config descriptor index " << i << " was corrupt.";
       free(buffer);
+
+      // Update the configurations.
+      device_info_->configurations =
+          std::move(usb_descriptor.device_info->configurations);
     }
   } else {
     USB_LOG(EVENT) << "Failed to get device descriptor: "

services/device/usb/usb_device_impl.h

@@ -43,15 +43,17 @@ class UsbDeviceImpl : public UsbDevice {
   // These functions are used during enumeration only. The values must not
   // change during the object's lifetime.
   void set_manufacturer_string(const base::string16& value) {
-    manufacturer_string_ = value;
+    device_info_->manufacturer_name = value;
   }
   void set_product_string(const base::string16& value) {
-    product_string_ = value;
+    device_info_->product_name = value;
   }
   void set_serial_number(const base::string16& value) {
-    serial_number_ = value;
+    device_info_->serial_number = value;
+  }
+  void set_webusb_landing_page(const GURL& url) {
+    device_info_->webusb_landing_page = url;
   }
-  void set_webusb_landing_page(const GURL& url) { webusb_landing_page_ = url; }
 
   libusb_device* platform_device() const { return platform_device_.get(); }
 

services/device/usb/usb_device_linux.cc

@@ -27,22 +27,9 @@
 namespace device {
 
 UsbDeviceLinux::UsbDeviceLinux(const std::string& device_path,
-                               const UsbDeviceDescriptor& descriptor,
-                               const std::string& manufacturer_string,
-                               const std::string& product_string,
-                               const std::string& serial_number,
-                               uint8_t active_configuration,
-                               uint32_t bus_number,
-                               uint32_t port_number)
-    : UsbDevice(descriptor,
-                base::UTF8ToUTF16(manufacturer_string),
-                base::UTF8ToUTF16(product_string),
-                base::UTF8ToUTF16(serial_number),
-                bus_number,
-                port_number),
-      device_path_(device_path) {
-  ActiveConfigurationChanged(active_configuration);
-}
+                               std::unique_ptr<UsbDeviceDescriptor> descriptor)
+    : UsbDevice(std::move(descriptor->device_info)),
+      device_path_(device_path) {}
 
 UsbDeviceLinux::~UsbDeviceLinux() = default;
 

services/device/usb/usb_device_linux.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <utility>
 
@@ -36,20 +37,16 @@ class UsbDeviceLinux : public UsbDevice {
 
   // These functions are used during enumeration only. The values must not
   // change during the object's lifetime.
-  void set_webusb_landing_page(const GURL& url) { webusb_landing_page_ = url; }
+  void set_webusb_landing_page(const GURL& url) {
+    device_info_->webusb_landing_page = url;
+  }
 
  protected:
   friend class UsbServiceLinux;
 
   // Called by UsbServiceLinux only.
   UsbDeviceLinux(const std::string& device_path,
-                 const UsbDeviceDescriptor& descriptor,
-                 const std::string& manufacturer_string,
-                 const std::string& product_string,
-                 const std::string& serial_number,
-                 uint8_t active_configuration,
-                 uint32_t bus_number,
-                 uint32_t port_number);
+                 std::unique_ptr<UsbDeviceDescriptor> descriptor);
 
   ~UsbDeviceLinux() override;
 

services/device/usb/usb_device_win.cc

@@ -4,6 +4,10 @@
 
 #include "services/device/usb/usb_device_win.h"
 
+#include <windows.h>
+
+#include <utility>
+
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/sequenced_task_runner.h"
@@ -14,8 +18,6 @@
 #include "services/device/usb/usb_device_handle_win.h"
 #include "services/device/usb/webusb_descriptors.h"
 
-#include <windows.h>
-
 namespace device {
 
 namespace {
@@ -81,39 +83,47 @@ void UsbDeviceWin::OnReadDescriptors(
     return;
   }
 
-  descriptor_ = std::move(*descriptor);
+  // Keep |bus_number| and |port_number| before updating the |device_info_|.
+  descriptor->device_info->bus_number = device_info_->bus_number,
+  descriptor->device_info->port_number = device_info_->port_number,
+  device_info_ = std::move(descriptor->device_info);
 
   // WinUSB only supports the configuration 1.
   ActiveConfigurationChanged(1);
 
   auto string_map = std::make_unique<std::map<uint8_t, base::string16>>();
-  if (descriptor_.i_manufacturer)
-    (*string_map)[descriptor_.i_manufacturer] = base::string16();
-  if (descriptor_.i_product)
-    (*string_map)[descriptor_.i_product] = base::string16();
-  if (descriptor_.i_serial_number)
-    (*string_map)[descriptor_.i_serial_number] = base::string16();
+  if (descriptor->i_manufacturer)
+    (*string_map)[descriptor->i_manufacturer] = base::string16();
+  if (descriptor->i_product)
+    (*string_map)[descriptor->i_product] = base::string16();
+  if (descriptor->i_serial_number)
+    (*string_map)[descriptor->i_serial_number] = base::string16();
 
   ReadUsbStringDescriptors(
       device_handle, std::move(string_map),
       base::BindOnce(&UsbDeviceWin::OnReadStringDescriptors, this,
-                     std::move(callback), device_handle));
+                     std::move(callback), device_handle,
+                     descriptor->i_manufacturer, descriptor->i_product,
+                     descriptor->i_serial_number));
 }
 
 void UsbDeviceWin::OnReadStringDescriptors(
     base::OnceCallback<void(bool)> callback,
     scoped_refptr<UsbDeviceHandle> device_handle,
+    uint8_t i_manufacturer,
+    uint8_t i_product,
+    uint8_t i_serial_number,
     std::unique_ptr<std::map<uint8_t, base::string16>> string_map) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   device_handle->Close();
 
-  if (descriptor_.i_manufacturer)
-    manufacturer_string_ = (*string_map)[descriptor_.i_manufacturer];
-  if (descriptor_.i_product)
-    product_string_ = (*string_map)[descriptor_.i_product];
-  if (descriptor_.i_serial_number)
-    serial_number_ = (*string_map)[descriptor_.i_serial_number];
+  if (i_manufacturer)
+    device_info_->manufacturer_name = (*string_map)[i_manufacturer];
+  if (i_product)
+    device_info_->product_name = (*string_map)[i_product];
+  if (i_serial_number)
+    device_info_->serial_number = (*string_map)[i_serial_number];
 
   if (usb_version() >= kUsbVersion2_1) {
     Open(base::BindOnce(&UsbDeviceWin::OnOpenedToReadWebUsbDescriptors, this,
@@ -146,7 +156,7 @@ void UsbDeviceWin::OnReadWebUsbDescriptors(
     const GURL& landing_page) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  webusb_landing_page_ = landing_page;
+  device_info_->webusb_landing_page = landing_page;
 
   device_handle->Close();
   std::move(callback).Run(true);

services/device/usb/usb_device_win.h

@@ -5,6 +5,8 @@
 #ifndef SERVICES_DEVICE_USB_USB_DEVICE_WIN_H_
 #define SERVICES_DEVICE_USB_USB_DEVICE_WIN_H_
 
+#include <map>
+#include <memory>
 #include <string>
 
 #include "base/macros.h"
@@ -47,6 +49,9 @@ class UsbDeviceWin : public UsbDevice {
   void OnReadStringDescriptors(
       base::OnceCallback<void(bool)> callback,
       scoped_refptr<UsbDeviceHandle> device_handle,
+      uint8_t i_manufacturer,
+      uint8_t i_product,
+      uint8_t i_serial_number,
       std::unique_ptr<std::map<uint8_t, base::string16>> string_map);
   void OnOpenedToReadWebUsbDescriptors(
       base::OnceCallback<void(bool)> callback,

services/device/usb/usb_service_linux.cc

@@ -7,6 +7,7 @@
 #include <stdint.h>
 
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/files/file.h"
@@ -67,7 +68,7 @@ void OnDeviceOpenedToReadDescriptors(
 
 class UsbServiceLinux::BlockingTaskRunnerHelper : public UdevWatcher::Observer {
  public:
-  BlockingTaskRunnerHelper(base::WeakPtr<UsbServiceLinux> service);
+  explicit BlockingTaskRunnerHelper(base::WeakPtr<UsbServiceLinux> service);
   ~BlockingTaskRunnerHelper() override;
 
   void Start();
@@ -141,52 +142,50 @@ void UsbServiceLinux::BlockingTaskRunnerHelper::OnDeviceAdded(
   if (!base::ReadFileToString(descriptors_path, &descriptors_str))
     return;
 
-  UsbDeviceDescriptor descriptor;
-  if (!descriptor.Parse(std::vector<uint8_t>(descriptors_str.begin(),
-                                             descriptors_str.end()))) {
+  std::unique_ptr<UsbDeviceDescriptor> descriptor(new UsbDeviceDescriptor());
+  if (!descriptor->Parse(std::vector<uint8_t>(descriptors_str.begin(),
+                                              descriptors_str.end()))) {
     return;
   }
 
-  if (descriptor.device_class == kDeviceClassHub) {
+  if (descriptor->device_info->class_code == kDeviceClassHub) {
     // Don't try to enumerate hubs. We never want to connect to a hub.
     return;
   }
 
-  std::string manufacturer;
   value = udev_device_get_sysattr_value(device.get(), "manufacturer");
   if (value)
-    manufacturer = value;
+    descriptor->device_info->manufacturer_name = base::UTF8ToUTF16(value);
 
-  std::string product;
   value = udev_device_get_sysattr_value(device.get(), "product");
   if (value)
-    product = value;
+    descriptor->device_info->product_name = base::UTF8ToUTF16(value);
 
-  std::string serial_number;
   value = udev_device_get_sysattr_value(device.get(), "serial");
   if (value)
-    serial_number = value;
+    descriptor->device_info->serial_number = base::UTF8ToUTF16(value);
 
   unsigned active_configuration = 0;
   value = udev_device_get_sysattr_value(device.get(), "bConfigurationValue");
   if (value)
     base::StringToUint(value, &active_configuration);
+  descriptor->device_info->active_configuration = active_configuration;
 
   unsigned bus_number = 0;
   value = udev_device_get_sysattr_value(device.get(), "busnum");
   if (value)
     base::StringToUint(value, &bus_number);
+  descriptor->device_info->bus_number = bus_number;
 
   unsigned port_number = 0;
   value = udev_device_get_sysattr_value(device.get(), "devnum");
   if (value)
     base::StringToUint(value, &port_number);
+  descriptor->device_info->port_number = port_number;
 
   task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UsbServiceLinux::OnDeviceAdded, service_, device_path,
-                     descriptor, manufacturer, product, serial_number,
-                     active_configuration, bus_number, port_number));
+      FROM_HERE, base::BindOnce(&UsbServiceLinux::OnDeviceAdded, service_,
+                                device_path, std::move(descriptor)));
 }
 
 void UsbServiceLinux::BlockingTaskRunnerHelper::OnDeviceRemoved(
@@ -228,14 +227,9 @@ void UsbServiceLinux::GetDevices(const GetDevicesCallback& callback) {
     enumeration_callbacks_.push_back(callback);
 }
 
-void UsbServiceLinux::OnDeviceAdded(const std::string& device_path,
-                                    const UsbDeviceDescriptor& descriptor,
-                                    const std::string& manufacturer,
-                                    const std::string& product,
-                                    const std::string& serial_number,
-                                    uint8_t active_configuration,
-                                    uint32_t bus_number,
-                                    uint32_t port_number) {
+void UsbServiceLinux::OnDeviceAdded(
+    const std::string& device_path,
+    std::unique_ptr<UsbDeviceDescriptor> descriptor) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (base::Contains(devices_by_path_, device_path)) {
@@ -249,9 +243,8 @@ void UsbServiceLinux::OnDeviceAdded(const std::string& device_path,
   if (!enumeration_ready())
     ++first_enumeration_countdown_;
 
-  scoped_refptr<UsbDeviceLinux> device(new UsbDeviceLinux(
-      device_path, descriptor, manufacturer, product, serial_number,
-      active_configuration, bus_number, port_number));
+  scoped_refptr<UsbDeviceLinux> device(
+      new UsbDeviceLinux(device_path, std::move(descriptor)));
   devices_by_path_[device->device_path()] = device;
   if (device->usb_version() >= kUsbVersion2_1) {
     device->Open(

services/device/usb/usb_service_linux.h

@@ -7,6 +7,7 @@
 
 #include <list>
 #include <memory>
+#include <string>
 #include <unordered_map>
 
 #include "base/macros.h"
@@ -34,13 +35,7 @@ class UsbServiceLinux final : public UsbService {
   class BlockingTaskRunnerHelper;
 
   void OnDeviceAdded(const std::string& device_path,
-                     const UsbDeviceDescriptor& descriptor,
-                     const std::string& manufacturer,
-                     const std::string& product,
-                     const std::string& serial_number,
-                     uint8_t active_configuration,
-                     uint32_t bus_number,
-                     uint32_t port_number);
+                     std::unique_ptr<UsbDeviceDescriptor> descriptor);
   void DeviceReady(scoped_refptr<UsbDeviceLinux> device, bool success);
   void OnDeviceRemoved(const std::string& device_path);
   void HelperStarted();