Move string descriptor getters from UsbDeviceHandle to UsbDevice.

The common string descriptors: iManufacturer, iProduct and iSerialNumber
should be accessible without opening the device. On Linux these can be
read out of sysfs without having access to the usbfs device node. This
is critical on Chrome OS because otherwise the permission broker needs
to be asked for permission to access the device. On other platforms we
fall back to opening the device temporarily. This will stop being the
case on OS X when libusb is no longer used and on Windows this will be
part of the initial enumeration process.

BUG=
TBR=dgozman@chromium.org

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

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

apps/saved_devices_service.cc

@@ -158,11 +158,7 @@ bool SavedDevicesService::SavedDevices::IsRegistered(
       continue;
     }
     if (!have_serial_number) {
-      scoped_refptr<UsbDeviceHandle> device_handle = device->Open();
-      if (!device_handle.get()) {
-        break;
-      }
-      if (!device_handle->GetSerial(&serial_number)) {
+      if (!device->GetSerialNumber(&serial_number)) {
         break;
       }
       have_serial_number = true;

apps/saved_devices_service_unittest.cc

@@ -25,88 +25,27 @@ using device::UsbEndpointDirection;
 using device::UsbTransferCallback;
 using testing::Return;
 
-class MockUsbDeviceHandle : public UsbDeviceHandle {
- public:
-  MockUsbDeviceHandle(const std::string& serial_number)
-      : UsbDeviceHandle(), serial_number_(serial_number) {}
-
-  MOCK_CONST_METHOD0(GetDevice, scoped_refptr<UsbDevice>());
-  MOCK_METHOD0(Close, void());
-
-  MOCK_METHOD10(ControlTransfer,
-                void(UsbEndpointDirection direction,
-                     TransferRequestType request_type,
-                     TransferRecipient recipient,
-                     uint8 request,
-                     uint16 value,
-                     uint16 index,
-                     net::IOBuffer* buffer,
-                     size_t length,
-                     unsigned int timeout,
-                     const UsbTransferCallback& callback));
-
-  MOCK_METHOD6(BulkTransfer,
-               void(UsbEndpointDirection direction,
-                    uint8 endpoint,
-                    net::IOBuffer* buffer,
-                    size_t length,
-                    unsigned int timeout,
-                    const UsbTransferCallback& callback));
-
-  MOCK_METHOD6(InterruptTransfer,
-               void(UsbEndpointDirection direction,
-                    uint8 endpoint,
-                    net::IOBuffer* buffer,
-                    size_t length,
-                    unsigned int timeout,
-                    const UsbTransferCallback& callback));
-
-  MOCK_METHOD8(IsochronousTransfer,
-               void(UsbEndpointDirection direction,
-                    uint8 endpoint,
-                    net::IOBuffer* buffer,
-                    size_t length,
-                    unsigned int packets,
-                    unsigned int packet_length,
-                    unsigned int timeout,
-                    const UsbTransferCallback& callback));
-
-  MOCK_METHOD0(ResetDevice, bool());
-  MOCK_METHOD1(ClaimInterface, bool(int interface_number));
-  MOCK_METHOD1(ReleaseInterface, bool(int interface_number));
-  MOCK_METHOD2(SetInterfaceAlternateSetting,
-               bool(int interface_number, int alternate_setting));
-  MOCK_METHOD1(GetManufacturer, bool(base::string16* manufacturer));
-  MOCK_METHOD1(GetProduct, bool(base::string16* product));
-
-  bool GetSerial(base::string16* serial) OVERRIDE {
-    if (serial_number_.empty()) {
-      return false;
-    }
-
-    *serial = base::UTF8ToUTF16(serial_number_);
-    return true;
-  }
-
- private:
-  virtual ~MockUsbDeviceHandle() {}
-
-  const std::string serial_number_;
-};
-
 class MockUsbDevice : public UsbDevice {
  public:
   MockUsbDevice(const std::string& serial_number, uint32 unique_id)
       : UsbDevice(0, 0, unique_id), serial_number_(serial_number) {}
 
+  MOCK_METHOD0(Open, scoped_refptr<UsbDeviceHandle>());
   MOCK_METHOD1(Close, bool(scoped_refptr<UsbDeviceHandle>));
 #if defined(OS_CHROMEOS)
   MOCK_METHOD2(RequestUsbAccess, void(int, const base::Callback<void(bool)>&));
 #endif
   MOCK_METHOD0(GetConfiguration, const device::UsbConfigDescriptor&());
+  MOCK_METHOD1(GetManufacturer, bool(base::string16*));
+  MOCK_METHOD1(GetProduct, bool(base::string16*));
+
+  bool GetSerialNumber(base::string16* serial) OVERRIDE {
+    if (serial_number_.empty()) {
+      return false;
+    }
 
-  scoped_refptr<UsbDeviceHandle> Open() OVERRIDE {
-    return new MockUsbDeviceHandle(serial_number_);
+    *serial = base::UTF8ToUTF16(serial_number_);
+    return true;
   }
 
   void NotifyDisconnect() { UsbDevice::NotifyDisconnect(); }

chrome/browser/devtools/device/usb/android_usb_browsertest.cc

@@ -144,20 +144,8 @@ class MockUsbDeviceHandle : public UsbDeviceHandle {
   }
 
   virtual bool ResetDevice() OVERRIDE { return true; }
-
-  virtual bool GetManufacturer(base::string16* manufacturer) OVERRIDE {
-    *manufacturer = base::UTF8ToUTF16(kDeviceManufacturer);
-    return true;
-  }
-
-  virtual bool GetProduct(base::string16* product) OVERRIDE {
-    *product = base::UTF8ToUTF16(kDeviceModel);
-    return true;
-  }
-
-  virtual bool GetSerial(base::string16* serial) OVERRIDE {
-    *serial = base::UTF8ToUTF16(kDeviceSerial);
-    return true;
+  virtual bool GetStringDescriptor(uint8_t string_id, base::string16* content) {
+    return false;
   }
 
   // Async IO. Can be called on any thread.
@@ -379,6 +367,21 @@ class MockUsbDevice : public UsbDevice {
     return config_desc_;
   }
 
+  virtual bool GetManufacturer(base::string16* manufacturer) OVERRIDE {
+    *manufacturer = base::UTF8ToUTF16(kDeviceManufacturer);
+    return true;
+  }
+
+  virtual bool GetProduct(base::string16* product) OVERRIDE {
+    *product = base::UTF8ToUTF16(kDeviceModel);
+    return true;
+  }
+
+  virtual bool GetSerialNumber(base::string16* serial) OVERRIDE {
+    *serial = base::UTF8ToUTF16(kDeviceSerial);
+    return true;
+  }
+
   virtual bool Close(scoped_refptr<UsbDeviceHandle> handle) OVERRIDE {
     return true;
   }

chrome/browser/devtools/device/usb/android_usb_device.cc

@@ -96,7 +96,7 @@ scoped_refptr<AndroidUsbDevice> ClaimInterface(
     return NULL;
 
   base::string16 serial;
-  if (!usb_handle->GetSerial(&serial) || serial.empty())
+  if (!usb_handle->GetDevice()->GetSerialNumber(&serial) || serial.empty())
     return NULL;
 
   return new AndroidUsbDevice(rsa_key,

device/hid/hid_connection_unittest.cc

@@ -100,7 +100,7 @@ class HidConnectionTest : public testing::Test {
     for (std::vector<HidDeviceInfo>::iterator it = devices.begin();
          it != devices.end();
          ++it) {
-      if (it->serial_number == test_gadget_->GetSerial()) {
+      if (it->serial_number == test_gadget_->GetSerialNumber()) {
         device_id_ = it->device_id;
         break;
       }

device/test/usb_test_gadget.h

@@ -35,7 +35,7 @@ class UsbTestGadget {
   virtual bool SetType(Type type) = 0;
 
   virtual UsbDevice* GetDevice() const = 0;
-  virtual std::string GetSerial() const = 0;
+  virtual std::string GetSerialNumber() const = 0;
 
  protected:
   UsbTestGadget() {}

device/test/usb_test_gadget_impl.cc

@@ -71,7 +71,7 @@ class UsbTestGadgetImpl : public UsbTestGadget {
   virtual bool Reconnect() OVERRIDE;
   virtual bool SetType(Type type) OVERRIDE;
   virtual UsbDevice* GetDevice() const OVERRIDE;
-  virtual std::string GetSerial() const OVERRIDE;
+  virtual std::string GetSerialNumber() const OVERRIDE;
 
  protected:
   UsbTestGadgetImpl();
@@ -166,7 +166,7 @@ UsbDevice* UsbTestGadgetImpl::GetDevice() const {
   return device_.get();
 }
 
-std::string UsbTestGadgetImpl::GetSerial() const {
+std::string UsbTestGadgetImpl::GetSerialNumber() const {
   return device_address_;
 }
 
@@ -205,13 +205,8 @@ bool UsbTestGadgetImpl::FindUnclaimed() {
          devices.begin(); iter != devices.end(); ++iter) {
     const scoped_refptr<UsbDevice> &device = *iter;
     if (device->vendor_id() == 0x18D1 && device->product_id() == 0x58F0) {
-      scoped_refptr<UsbDeviceHandle> handle = device->Open();
-      if (handle.get() == NULL) {
-        continue;
-      }
-
       base::string16 serial_utf16;
-      if (!handle->GetSerial(&serial_utf16)) {
+      if (!device->GetSerialNumber(&serial_utf16)) {
         continue;
       }
 
@@ -340,7 +335,7 @@ bool UsbTestGadgetImpl::Update() {
 bool UsbTestGadgetImpl::FindClaimed() {
   CHECK(!device_.get());
 
-  std::string expected_serial = GetSerial();
+  std::string expected_serial = GetSerialNumber();
 
   std::vector<scoped_refptr<UsbDevice> > devices;
   usb_service_->GetDevices(&devices);
@@ -362,13 +357,8 @@ bool UsbTestGadgetImpl::FindClaimed() {
         continue;
       }
 
-      scoped_refptr<UsbDeviceHandle> handle(device->Open());
-      if (handle.get() == NULL) {
-        continue;
-      }
-
       base::string16 serial_utf16;
-      if (!handle->GetSerial(&serial_utf16)) {
+      if (!device->GetSerialNumber(&serial_utf16)) {
         continue;
       }
 

device/usb/usb_device.h

@@ -9,6 +9,7 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
+#include "base/strings/string16.h"
 
 namespace device {
 
@@ -54,6 +55,18 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   // Blocking method. Must be called on FILE thread.
   virtual const UsbConfigDescriptor& GetConfiguration() = 0;
 
+  // Gets the manufacturer string of the device, or returns false.
+  // Blocking method. Must be called on FILE thread.
+  virtual bool GetManufacturer(base::string16* manufacturer) = 0;
+
+  // Gets the product string of the device, or returns false.
+  // Blocking method. Must be called on FILE thread.
+  virtual bool GetProduct(base::string16* product) = 0;
+
+  // Gets the serial number string of the device, or returns false.
+  // Blocking method. Must be called on FILE thread.
+  virtual bool GetSerialNumber(base::string16* serial) = 0;
+
   void AddObserver(Observer* obs) { observer_list_.AddObserver(obs); }
   void RemoveObserver(Observer* obs) { observer_list_.RemoveObserver(obs); }
 

device/usb/usb_device_filter_unittest.cc

@@ -30,6 +30,9 @@ class MockUsbDevice : public UsbDevice {
   MOCK_METHOD2(RequestUsbAccess, void(int, const base::Callback<void(bool)>&));
 #endif
   MOCK_METHOD0(GetConfiguration, const UsbConfigDescriptor&());
+  MOCK_METHOD1(GetManufacturer, bool(base::string16*));
+  MOCK_METHOD1(GetProduct, bool(base::string16*));
+  MOCK_METHOD1(GetSerialNumber, bool(base::string16*));
 
  private:
   virtual ~MockUsbDevice() {}

device/usb/usb_device_handle.h

@@ -56,9 +56,10 @@ class UsbDeviceHandle : public base::RefCountedThreadSafe<UsbDeviceHandle> {
   virtual bool SetInterfaceAlternateSetting(int interface_number,
                                             int alternate_setting) = 0;
   virtual bool ResetDevice() = 0;
-  virtual bool GetManufacturer(base::string16* manufacturer) = 0;
-  virtual bool GetProduct(base::string16* product) = 0;
-  virtual bool GetSerial(base::string16* serial) = 0;
+
+  // Gets the string descriptor with the given index from the device, or returns
+  // false. This method is blocking and must be called on the FILE thread.
+  virtual bool GetStringDescriptor(uint8 string_id, base::string16* string) = 0;
 
   // Async IO. Can be called on any thread.
   virtual void ControlTransfer(UsbEndpointDirection direction,