2 files changed, 70 insertions, 81 deletions

diff --git a/device/usb/usb_device_impl.cc b/device/usb/usb_device_impl.cc
index b1beeb2..17e9c13 100644
--- a/device/usb/usb_device_impl.cc
+++ b/device/usb/usb_device_impl.cc
@@ -158,14 +158,22 @@ UsbDeviceImpl::UsbDeviceImpl(
       }
 
       value = udev_device_get_sysattr_value(device.get(), "manufacturer");
-      manufacturer_ = value ? value : "";
+      if (value) {
+        manufacturer_ = base::UTF8ToUTF16(value);
+      }
       value = udev_device_get_sysattr_value(device.get(), "product");
-      product_ = value ? value : "";
+      if (value) {
+        product_ = base::UTF8ToUTF16(value);
+      }
       value = udev_device_get_sysattr_value(device.get(), "serial");
-      serial_number_ = value ? value : "";
+      if (value) {
+        serial_number_ = base::UTF8ToUTF16(value);
+      }
       break;
     }
   }
+#else
+  strings_cached_ = false;
 #endif
 }
 
@@ -219,6 +227,7 @@ scoped_refptr<UsbDeviceHandle> UsbDeviceImpl::Open() {
   if (LIBUSB_SUCCESS == rv) {
     GetConfiguration();
     if (!current_configuration_cached_) {
+      libusb_close(handle);
       return NULL;
     }
     scoped_refptr<UsbDeviceHandleImpl> device_handle =
@@ -322,95 +331,40 @@ const UsbConfigDescriptor& UsbDeviceImpl::GetConfiguration() {
 bool UsbDeviceImpl::GetManufacturer(base::string16* manufacturer) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-#if defined(USE_UDEV)
-  if (manufacturer_.empty()) {
-    return false;
-  }
-  *manufacturer = base::UTF8ToUTF16(manufacturer_);
-  return true;
-#else
-  // This is a non-blocking call as libusb has the descriptor in memory.
-  libusb_device_descriptor desc;
-  const int rv = libusb_get_device_descriptor(platform_device_, &desc);
-  if (rv != LIBUSB_SUCCESS) {
-    VLOG(1) << "Failed to read device descriptor: "
-            << ConvertPlatformUsbErrorToString(rv);
-    return false;
-  }
-
-  if (desc.iManufacturer == 0) {
-    return false;
-  }
-
-  scoped_refptr<UsbDeviceHandle> device_handle = Open();
-  if (device_handle.get()) {
-    return device_handle->GetStringDescriptor(desc.iManufacturer, manufacturer);
+#if !defined(USE_UDEV)
+  if (!strings_cached_) {
+    CacheStrings();
   }
-  return false;
 #endif
+
+  *manufacturer = manufacturer_;
+  return !manufacturer_.empty();
 }
 
 bool UsbDeviceImpl::GetProduct(base::string16* product) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-#if defined(USE_UDEV)
-  if (product_.empty()) {
-    return false;
-  }
-  *product = base::UTF8ToUTF16(product_);
-  return true;
-#else
-  // This is a non-blocking call as libusb has the descriptor in memory.
-  libusb_device_descriptor desc;
-  const int rv = libusb_get_device_descriptor(platform_device_, &desc);
-  if (rv != LIBUSB_SUCCESS) {
-    VLOG(1) << "Failed to read device descriptor: "
-            << ConvertPlatformUsbErrorToString(rv);
-    return false;
-  }
-
-  if (desc.iProduct == 0) {
-    return false;
-  }
-
-  scoped_refptr<UsbDeviceHandle> device_handle = Open();
-  if (device_handle.get()) {
-    return device_handle->GetStringDescriptor(desc.iProduct, product);
+#if !defined(USE_UDEV)
+  if (!strings_cached_) {
+    CacheStrings();
   }
-  return false;
 #endif
+
+  *product = product_;
+  return !product_.empty();
 }
 
 bool UsbDeviceImpl::GetSerialNumber(base::string16* serial_number) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-#if defined(USE_UDEV)
-  if (serial_number_.empty()) {
-    return false;
-  }
-  *serial_number = base::UTF8ToUTF16(serial_number_);
-  return true;
-#else
-  // This is a non-blocking call as libusb has the descriptor in memory.
-  libusb_device_descriptor desc;
-  const int rv = libusb_get_device_descriptor(platform_device_, &desc);
-  if (rv != LIBUSB_SUCCESS) {
-    VLOG(1) << "Failed to read device descriptor: "
-            << ConvertPlatformUsbErrorToString(rv);
-    return false;
-  }
-
-  if (desc.iSerialNumber == 0) {
-    return false;
+#if !defined(USE_UDEV)
+  if (!strings_cached_) {
+    CacheStrings();
   }
-
-  scoped_refptr<UsbDeviceHandle> device_handle = Open();
-  if (device_handle.get()) {
-    return device_handle->GetStringDescriptor(desc.iSerialNumber,
-                                              serial_number);
-  }
-  return false;
 #endif
+
+  *serial_number = serial_number_;
+  return !serial_number_.empty();
 }
 
 void UsbDeviceImpl::OnDisconnect() {
@@ -421,4 +375,34 @@ void UsbDeviceImpl::OnDisconnect() {
     (*it)->InternalClose();
 }
 
+#if !defined(USE_UDEV)
+void UsbDeviceImpl::CacheStrings() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // This is a non-blocking call as libusb has the descriptor in memory.
+  libusb_device_descriptor desc;
+  const int rv = libusb_get_device_descriptor(platform_device_, &desc);
+  if (rv == LIBUSB_SUCCESS) {
+    scoped_refptr<UsbDeviceHandle> device_handle = Open();
+    if (device_handle.get()) {
+      if (desc.iManufacturer != 0) {
+        device_handle->GetStringDescriptor(desc.iManufacturer, &manufacturer_);
+      }
+      if (desc.iProduct != 0) {
+        device_handle->GetStringDescriptor(desc.iProduct, &product_);
+      }
+      if (desc.iSerialNumber != 0) {
+        device_handle->GetStringDescriptor(desc.iSerialNumber, &serial_number_);
+      }
+      device_handle->Close();
+    } else {
+      VLOG(1) << "Failed to open device to cache string descriptors.";
+    }
+  } else {
+    VLOG(1) << "Failed to read device descriptor to cache string descriptors: "
+            << ConvertPlatformUsbErrorToString(rv);
+  }
+  strings_cached_ = true;
+}
+#endif  // !defined(USE_UDEV)
+
 }  // namespace device
diff --git a/device/usb/usb_device_impl.h b/device/usb/usb_device_impl.h
index 9a14947..2718998 100644
--- a/device/usb/usb_device_impl.h
+++ b/device/usb/usb_device_impl.h
@@ -56,20 +56,25 @@ class UsbDeviceImpl : public UsbDevice {
 
   virtual ~UsbDeviceImpl();
 
-  // Called only be UsbService.
+  // Called only by UsbService.
   void OnDisconnect();
 
  private:
   base::ThreadChecker thread_checker_;
   PlatformUsbDevice platform_device_;
 
-#if defined(USE_UDEV)
   // On Linux these properties are read from sysfs when the device is enumerated
   // to avoid hitting the permission broker on Chrome OS for a real string
   // descriptor request.
-  std::string manufacturer_;
-  std::string product_;
-  std::string serial_number_;
+  base::string16 manufacturer_;
+  base::string16 product_;
+  base::string16 serial_number_;
+#if !defined(USE_UDEV)
+  // On other platforms the device must be opened in order to cache them. This
+  // should be delayed until the strings are needed to avoid poor interactions
+  // with other applications.
+  void CacheStrings();
+  bool strings_cached_;
 #endif
 
   // The active configuration descriptor is not read immediately but cached for