From 7446a70a6b099681ea1b7f8ac03120b8c30d3e97 Mon Sep 17 00:00:00 2001 From: reillyg Date: Fri, 29 Jan 2016 17:06:41 -0800 Subject: 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} --- device/usb/usb_descriptors.cc | 106 ++++++++++++++++++++++++++--- device/usb/usb_descriptors.h | 13 ++++ device/usb/usb_descriptors_unittest.cc | 62 +++++++++++++++++ device/usb/usb_service_impl.cc | 117 ++++++++------------------------- 4 files changed, 199 insertions(+), 99 deletions(-) (limited to 'device') 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 +#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; +using IndexMapPtr = scoped_ptr; + +// 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& callback, + UsbTransferStatus status, + scoped_refptr buffer, + size_t length) { + base::string16 string; + if (status == USB_TRANSFER_COMPLETED && + ParseUsbStringDescriptor( + std::vector(buffer->data(), buffer->data() + length), + &string)) { + callback.Run(string); + } else { + callback.Run(base::string16()); + } +} + +void ReadStringDescriptor( + scoped_refptr device_handle, + uint8_t index, + uint16_t language_id, + const base::Callback& callback) { + scoped_refptr 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 device_handle, + IndexMapPtr index_map, + const base::Callback& 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 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(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& 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(&descriptor[2]), length / 2 - 1); - } + + // The string is returned by the device in UTF-16LE. + *output = base::string16( + reinterpret_cast(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 device_handle, + IndexMapPtr index_map, + const base::Callback& 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 + +#include #include +#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& descriptor, base::string16* output); +void ReadUsbStringDescriptors( + scoped_refptr device_handle, + scoped_ptr> index_map, + const base::Callback>)>& + 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 +#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> 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> string_map( + new std::map()); + (*string_map)[1] = base::string16(); + (*string_map)[2] = base::string16(); + (*string_map)[3] = base::string16(); + + scoped_refptr 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& callback, - UsbTransferStatus status, - scoped_refptr buffer, - size_t length) { - base::string16 string; - if (status == USB_TRANSFER_COMPLETED && - ParseUsbStringDescriptor( - std::vector(buffer->data(), buffer->data() + length), - &string)) { - callback.Run(string); - } else { - callback.Run(base::string16()); - } -} - -void ReadStringDescriptor( - scoped_refptr device_handle, - uint8_t index, - uint16_t language_id, - const base::Callback& callback) { - scoped_refptr 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 device_handle, const base::Closure& continuation) { device_handle->Close(); continuation.Run(); } -void SaveStringAndRunContinuation( - const base::Callback& save_callback, +void SaveStringsAndRunContinuation( + scoped_refptr 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> 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 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 device = - static_cast(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 device_handle, const base::Closure& barrier, scoped_ptr allowed_origins, @@ -227,12 +158,17 @@ void OnDeviceOpenedReadDescriptors( const base::Closure& failure_closure, scoped_refptr device_handle) { if (device_handle) { - int count = 0; + scoped_ptr> string_map( + new std::map()); 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 device = + static_cast(device_handle->GetDevice().get()); + + ReadUsbStringDescriptors( + device_handle, std::move(string_map), + base::Bind(&SaveStringsAndRunContinuation, device, manufacturer, + product, serial_number, barrier)); } if (read_bos_descriptors) { -- cgit v1.1