Cache important USB string descriptors all at once.

On platforms without udev support we must open a libusb device handle
in order to read string descriptors from the device. Rapidly opening
and closing the handle could cause problems with the platform's USB
stack so this patch caches the manufacturer, product and serial number
strings all at once. It also prevents a leak of the device handle by
explicitly closing it.

BUG=346953

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

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

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