Move USB string descriptor reading out of usb_service_impl.cc.

As I did for WebUSB descriptor reading this allows this process to be
unit tested with a MockUsbDeviceHandle.

BUG=None

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

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

4 files changed, 199 insertions, 99 deletions

diff --git a/device/usb/usb_descriptors.cc b/device/usb/usb_descriptors.cc
index a7f8ad4..840eb5d 100644
--- a/device/usb/usb_descriptors.cc
+++ b/device/usb/usb_descriptors.cc
@@ -8,12 +8,84 @@
 
 #include <algorithm>
 
+#include "base/barrier_closure.h"
+#include "base/bind.h"
+#include "device/usb/usb_device_handle.h"
+#include "net/base/io_buffer.h"
+
 namespace device {
 
 namespace {
+
+using IndexMap = std::map<uint8_t, base::string16>;
+using IndexMapPtr = scoped_ptr<IndexMap>;
+
+// Standard USB requests and descriptor types:
+const uint8_t kGetDescriptorRequest = 0x06;
 const uint8_t kStringDescriptorType = 0x03;
+
+const int kControlTransferTimeout = 60000;  // 1 minute
+
+void StoreStringDescriptor(IndexMap::iterator it,
+                           const base::Closure& callback,
+                           const base::string16& string) {
+  it->second = string;
+  callback.Run();
+}
+
+void OnReadStringDescriptor(
+    const base::Callback<void(const base::string16&)>& callback,
+    UsbTransferStatus status,
+    scoped_refptr<net::IOBuffer> buffer,
+    size_t length) {
+  base::string16 string;
+  if (status == USB_TRANSFER_COMPLETED &&
+      ParseUsbStringDescriptor(
+          std::vector<uint8_t>(buffer->data(), buffer->data() + length),
+          &string)) {
+    callback.Run(string);
+  } else {
+    callback.Run(base::string16());
+  }
+}
+
+void ReadStringDescriptor(
+    scoped_refptr<UsbDeviceHandle> device_handle,
+    uint8_t index,
+    uint16_t language_id,
+    const base::Callback<void(const base::string16&)>& callback) {
+  scoped_refptr<net::IOBufferWithSize> buffer = new net::IOBufferWithSize(255);
+  device_handle->ControlTransfer(
+      USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD, UsbDeviceHandle::DEVICE,
+      kGetDescriptorRequest, kStringDescriptorType << 8 | index, language_id,
+      buffer, buffer->size(), kControlTransferTimeout,
+      base::Bind(&OnReadStringDescriptor, callback));
+}
+
+void OnReadLanguageIds(scoped_refptr<UsbDeviceHandle> device_handle,
+                       IndexMapPtr index_map,
+                       const base::Callback<void(IndexMapPtr)>& callback,
+                       const base::string16& languages) {
+  // Default to English unless the device provides a language and then just pick
+  // the first one.
+  uint16_t language_id = languages.empty() ? 0x0409 : languages[0];
+
+  std::map<uint8_t, IndexMap::iterator> iterator_map;
+  for (auto it = index_map->begin(); it != index_map->end(); ++it)
+    iterator_map[it->first] = it;
+
+  base::Closure barrier =
+      base::BarrierClosure(static_cast<int>(iterator_map.size()),
+                           base::Bind(callback, base::Passed(&index_map)));
+  for (const auto& map_entry : iterator_map) {
+    ReadStringDescriptor(
+        device_handle, map_entry.first, language_id,
+        base::Bind(&StoreStringDescriptor, map_entry.second, barrier));
+  }
 }
 
+}  // namespace
+
 UsbEndpointDescriptor::UsbEndpointDescriptor()
     : address(0),
       direction(USB_DIRECTION_INBOUND),
@@ -50,23 +122,37 @@ UsbConfigDescriptor::~UsbConfigDescriptor() {
 
 bool ParseUsbStringDescriptor(const std::vector<uint8_t>& descriptor,
                               base::string16* output) {
-  if (descriptor.size() < 2 || descriptor[1] != kStringDescriptorType) {
+  if (descriptor.size() < 2 || descriptor[1] != kStringDescriptorType)
     return false;
-  }
+
   // Let the device return a buffer larger than the actual string but prefer the
   // length reported inside the descriptor.
   size_t length = descriptor[0];
   length = std::min(length, descriptor.size());
-  if (length < 2) {
+  if (length < 2)
     return false;
-  } else if (length == 2) {
-    // Special case to avoid indexing beyond the end of |descriptor|.
-    *output = base::string16();
-  } else {
-    *output = base::string16(
-        reinterpret_cast<const base::char16*>(&descriptor[2]), length / 2 - 1);
-  }
+
+  // The string is returned by the device in UTF-16LE.
+  *output = base::string16(
+      reinterpret_cast<const base::char16*>(descriptor.data() + 2),
+      length / 2 - 1);
   return true;
 }
 
+// For each key in |index_map| this function reads that string descriptor from
+// |device_handle| and updates the value in in |index_map|.
+void ReadUsbStringDescriptors(
+    scoped_refptr<UsbDeviceHandle> device_handle,
+    IndexMapPtr index_map,
+    const base::Callback<void(IndexMapPtr)>& callback) {
+  if (index_map->empty()) {
+    callback.Run(std::move(index_map));
+    return;
+  }
+
+  ReadStringDescriptor(device_handle, 0, 0,
+                       base::Bind(&OnReadLanguageIds, device_handle,
+                                  base::Passed(&index_map), callback));
+}
+
 }  // namespace device
diff --git a/device/usb/usb_descriptors.h b/device/usb/usb_descriptors.h
index adaa0ab..cd8490a 100644
--- a/device/usb/usb_descriptors.h
+++ b/device/usb/usb_descriptors.h
@@ -6,12 +6,19 @@
 #define DEVICE_USB_USB_DESCRIPTORS_H_
 
 #include <stdint.h>
+
+#include <map>
 #include <vector>
 
+#include "base/callback_forward.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 
 namespace device {
 
+class UsbDeviceHandle;
+
 // A Java counterpart will be generated for this enum.
 // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.device.usb
 enum UsbTransferType {
@@ -83,6 +90,12 @@ struct UsbConfigDescriptor {
 bool ParseUsbStringDescriptor(const std::vector<uint8_t>& descriptor,
                               base::string16* output);
 
+void ReadUsbStringDescriptors(
+    scoped_refptr<UsbDeviceHandle> device_handle,
+    scoped_ptr<std::map<uint8_t, base::string16>> index_map,
+    const base::Callback<void(scoped_ptr<std::map<uint8_t, base::string16>>)>&
+        callback);
+
 }  // namespace device
 
 #endif  // DEVICE_USB_USB_DESCRIPTORS_H_
diff --git a/device/usb/usb_descriptors_unittest.cc b/device/usb/usb_descriptors_unittest.cc
index 03533ae..402304f 100644
--- a/device/usb/usb_descriptors_unittest.cc
+++ b/device/usb/usb_descriptors_unittest.cc
@@ -4,14 +4,32 @@
 
 #include <stdint.h>
 
+#include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
+#include "device/usb/mock_usb_device_handle.h"
 #include "device/usb/usb_descriptors.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+
 namespace device {
 
 namespace {
 
+ACTION_P2(InvokeCallback, data, length) {
+  size_t transferred_length = std::min(length, arg7);
+  memcpy(arg6->data(), data, transferred_length);
+  arg9.Run(USB_TRANSFER_COMPLETED, arg6, transferred_length);
+}
+
+void ExpectStringDescriptors(
+    scoped_ptr<std::map<uint8_t, base::string16>> string_map) {
+  EXPECT_EQ(3u, string_map->size());
+  EXPECT_EQ(base::ASCIIToUTF16("String 1"), (*string_map)[1]);
+  EXPECT_EQ(base::ASCIIToUTF16("String 2"), (*string_map)[2]);
+  EXPECT_EQ(base::ASCIIToUTF16("String 3"), (*string_map)[3]);
+}
+
 class UsbDescriptorsTest : public ::testing::Test {};
 
 TEST_F(UsbDescriptorsTest, StringDescriptor) {
@@ -68,6 +86,50 @@ TEST_F(UsbDescriptorsTest, OneByteStringDescriptor) {
   EXPECT_EQ(base::string16(), string);
 }
 
+TEST_F(UsbDescriptorsTest, ReadStringDescriptors) {
+  scoped_ptr<std::map<uint8_t, base::string16>> string_map(
+      new std::map<uint8_t, base::string16>());
+  (*string_map)[1] = base::string16();
+  (*string_map)[2] = base::string16();
+  (*string_map)[3] = base::string16();
+
+  scoped_refptr<MockUsbDeviceHandle> device_handle(
+      new MockUsbDeviceHandle(nullptr));
+  static const uint8_t kStringDescriptor0[] = {0x04, 0x03, 0x21, 0x43};
+  EXPECT_CALL(*device_handle,
+              ControlTransfer(USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD,
+                              UsbDeviceHandle::DEVICE, 0x06, 0x0300, 0x0000, _,
+                              _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor0, sizeof(kStringDescriptor0)));
+  static const uint8_t kStringDescriptor1[] = {0x12, 0x03, 'S', 0, 't', 0,
+                                               'r',  0,    'i', 0, 'n', 0,
+                                               'g',  0,    ' ', 0, '1', 0};
+  EXPECT_CALL(*device_handle,
+              ControlTransfer(USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD,
+                              UsbDeviceHandle::DEVICE, 0x06, 0x0301, 0x4321, _,
+                              _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor1, sizeof(kStringDescriptor1)));
+  static const uint8_t kStringDescriptor2[] = {0x12, 0x03, 'S', 0, 't', 0,
+                                               'r',  0,    'i', 0, 'n', 0,
+                                               'g',  0,    ' ', 0, '2', 0};
+  EXPECT_CALL(*device_handle,
+              ControlTransfer(USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD,
+                              UsbDeviceHandle::DEVICE, 0x06, 0x0302, 0x4321, _,
+                              _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor2, sizeof(kStringDescriptor2)));
+  static const uint8_t kStringDescriptor3[] = {0x12, 0x03, 'S', 0, 't', 0,
+                                               'r',  0,    'i', 0, 'n', 0,
+                                               'g',  0,    ' ', 0, '3', 0};
+  EXPECT_CALL(*device_handle,
+              ControlTransfer(USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD,
+                              UsbDeviceHandle::DEVICE, 0x06, 0x0303, 0x4321, _,
+                              _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor3, sizeof(kStringDescriptor3)));
+
+  ReadUsbStringDescriptors(device_handle, std::move(string_map),
+                           base::Bind(&ExpectStringDescriptors));
+}
+
 }  // namespace
 
 }  // namespace device
diff --git a/device/usb/usb_service_impl.cc b/device/usb/usb_service_impl.cc
index 759f33c..b72ce52 100644
--- a/device/usb/usb_service_impl.cc
+++ b/device/usb/usb_service_impl.cc
@@ -46,10 +46,6 @@ namespace {
 
 // Standard USB requests and descriptor types:
 const uint16_t kUsbVersion2_1 = 0x0210;
-const uint8_t kGetDescriptorRequest = 0x06;
-const uint8_t kStringDescriptorType = 0x03;
-
-const int kControlTransferTimeout = 60000;  // 1 minute
 
 #if defined(OS_WIN)
 
@@ -116,93 +112,28 @@ void GetDeviceListOnBlockingThread(
                         base::Bind(callback, platform_devices, device_count));
 }
 
-void OnReadStringDescriptor(
-    const base::Callback<void(const base::string16&)>& callback,
-    UsbTransferStatus status,
-    scoped_refptr<net::IOBuffer> buffer,
-    size_t length) {
-  base::string16 string;
-  if (status == USB_TRANSFER_COMPLETED &&
-      ParseUsbStringDescriptor(
-          std::vector<uint8_t>(buffer->data(), buffer->data() + length),
-          &string)) {
-    callback.Run(string);
-  } else {
-    callback.Run(base::string16());
-  }
-}
-
-void ReadStringDescriptor(
-    scoped_refptr<UsbDeviceHandle> device_handle,
-    uint8_t index,
-    uint16_t language_id,
-    const base::Callback<void(const base::string16&)>& callback) {
-  scoped_refptr<IOBufferWithSize> buffer = new IOBufferWithSize(255);
-  device_handle->ControlTransfer(
-      USB_DIRECTION_INBOUND, UsbDeviceHandle::STANDARD, UsbDeviceHandle::DEVICE,
-      kGetDescriptorRequest, kStringDescriptorType << 8 | index, language_id,
-      buffer, buffer->size(), kControlTransferTimeout,
-      base::Bind(&OnReadStringDescriptor, callback));
-}
-
 void CloseHandleAndRunContinuation(scoped_refptr<UsbDeviceHandle> device_handle,
                                    const base::Closure& continuation) {
   device_handle->Close();
   continuation.Run();
 }
 
-void SaveStringAndRunContinuation(
-    const base::Callback<void(const base::string16&)>& save_callback,
+void SaveStringsAndRunContinuation(
+    scoped_refptr<UsbDeviceImpl> device,
+    uint8_t manufacturer,
+    uint8_t product,
+    uint8_t serial_number,
     const base::Closure& continuation,
-    const base::string16& value) {
-  if (!value.empty()) {
-    save_callback.Run(value);
-  }
+    scoped_ptr<std::map<uint8_t, base::string16>> string_map) {
+  if (manufacturer != 0)
+    device->set_manufacturer_string((*string_map)[manufacturer]);
+  if (product != 0)
+    device->set_product_string((*string_map)[product]);
+  if (serial_number != 0)
+    device->set_serial_number((*string_map)[serial_number]);
   continuation.Run();
 }
 
-// This function runs |barrier| once for every string it tries to read.
-void OnReadLanguageIds(scoped_refptr<UsbDeviceHandle> device_handle,
-                       uint8_t manufacturer,
-                       uint8_t product,
-                       uint8_t serial_number,
-                       const base::Closure& barrier,
-                       const base::string16& languages) {
-  // Default to English unless the device provides a language and then just pick
-  // the first one.
-  uint16_t language_id = 0x0409;
-  if (!languages.empty()) {
-    language_id = languages[0];
-  }
-
-  scoped_refptr<UsbDeviceImpl> device =
-      static_cast<UsbDeviceImpl*>(device_handle->GetDevice().get());
-
-  if (manufacturer != 0) {
-    ReadStringDescriptor(
-        device_handle, manufacturer, language_id,
-        base::Bind(&SaveStringAndRunContinuation,
-                   base::Bind(&UsbDeviceImpl::set_manufacturer_string, device),
-                   barrier));
-  }
-
-  if (product != 0) {
-    ReadStringDescriptor(
-        device_handle, product, language_id,
-        base::Bind(&SaveStringAndRunContinuation,
-                   base::Bind(&UsbDeviceImpl::set_product_string, device),
-                   barrier));
-  }
-
-  if (serial_number != 0) {
-    ReadStringDescriptor(
-        device_handle, serial_number, language_id,
-        base::Bind(&SaveStringAndRunContinuation,
-                   base::Bind(&UsbDeviceImpl::set_serial_number, device),
-                   barrier));
-  }
-}
-
 void OnReadBosDescriptor(scoped_refptr<UsbDeviceHandle> device_handle,
                          const base::Closure& barrier,
                          scoped_ptr<WebUsbAllowedOrigins> allowed_origins,
@@ -227,12 +158,17 @@ void OnDeviceOpenedReadDescriptors(
     const base::Closure& failure_closure,
     scoped_refptr<UsbDeviceHandle> device_handle) {
   if (device_handle) {
-    int count = 0;
+    scoped_ptr<std::map<uint8_t, base::string16>> string_map(
+        new std::map<uint8_t, base::string16>());
     if (manufacturer != 0)
-      count++;
+      (*string_map)[manufacturer] = base::string16();
     if (product != 0)
-      count++;
+      (*string_map)[product] = base::string16();
     if (serial_number != 0)
+      (*string_map)[serial_number] = base::string16();
+
+    int count = 0;
+    if (!string_map->empty())
       count++;
     if (read_bos_descriptors)
       count++;
@@ -242,11 +178,14 @@ void OnDeviceOpenedReadDescriptors(
         base::BarrierClosure(count, base::Bind(&CloseHandleAndRunContinuation,
                                                device_handle, success_closure));
 
-    if (manufacturer != 0 || product != 0 || serial_number != 0) {
-      ReadStringDescriptor(
-          device_handle, 0, 0,
-          base::Bind(&OnReadLanguageIds, device_handle, manufacturer, product,
-                     serial_number, barrier));
+    if (!string_map->empty()) {
+      scoped_refptr<UsbDeviceImpl> device =
+          static_cast<UsbDeviceImpl*>(device_handle->GetDevice().get());
+
+      ReadUsbStringDescriptors(
+          device_handle, std::move(string_map),
+          base::Bind(&SaveStringsAndRunContinuation, device, manufacturer,
+                     product, serial_number, barrier));
     }
 
     if (read_bos_descriptors) {