Skip to content

Commit

Permalink
Reland: Convert device::UsbConfigDescriptor and friends to structs.
Browse files Browse the repository at this point in the history 
These classes do not need to be classes and expecially don't need to be
abstract classes as this leads to a complicated implementation and
complicated tests. All USB devices no matter the platform will have the
same descriptor data.

This change follows the model of device::HidDeviceInfo.

Relanding this patch (reverted in crrev.com/294640) after restoring the
call to libusb_free_config_descriptor that was removed during
refactoring and caused failures in the Linux ASan LSan tests.

BUG=

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

Cr-Commit-Position: refs/heads/master@{#294734}
  • Loading branch information
@reillyeon
reillyeon authored and Commit bot committed Sep 13, 2014
1 parent 9cc6363 commit 2191196
Show file tree
Hide file tree
Showing 20 changed files with 461 additions and 843 deletions.
154 changes: 30 additions & 124 deletions chrome/browser/devtools/device/usb/android_usb_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_utils.h"
#include "device/usb/usb_descriptors.h"
#include "device/usb/usb_device.h"
#include "device/usb/usb_device_handle.h"
#include "device/usb/usb_interface.h"
#include "device/usb/usb_service.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -25,7 +25,6 @@ using device::UsbDeviceHandle;
using device::UsbEndpointDescriptor;
using device::UsbEndpointDirection;
using device::UsbInterfaceDescriptor;
using device::UsbInterfaceAltSettingDescriptor;
using device::UsbService;
using device::UsbSynchronizationType;
using device::UsbTransferCallback;
Expand Down Expand Up @@ -104,125 +103,6 @@ const char* GetMockShellResponse(std::string command) {
return "";
}

class MockUsbEndpointDescriptor : public UsbEndpointDescriptor {
public:
virtual int GetAddress() const OVERRIDE { return address_; }

virtual UsbEndpointDirection GetDirection() const OVERRIDE {
return direction_;
}

virtual int GetMaximumPacketSize() const OVERRIDE {
return maximum_packet_size_;
}

virtual UsbSynchronizationType GetSynchronizationType() const OVERRIDE {
return usb_synchronization_type_;
}

virtual UsbTransferType GetTransferType() const OVERRIDE {
return usb_transfer_type_;
}
virtual UsbUsageType GetUsageType() const OVERRIDE { return usb_usage_type_; }

virtual int GetPollingInterval() const OVERRIDE { return polling_interval_; }

int address_;
UsbEndpointDirection direction_;
int maximum_packet_size_;
UsbSynchronizationType usb_synchronization_type_;
UsbTransferType usb_transfer_type_;
UsbUsageType usb_usage_type_;
int polling_interval_;

private:
virtual ~MockUsbEndpointDescriptor() {}
};

template <class T>
class MockUsbInterfaceAltSettingDescriptor
: public UsbInterfaceAltSettingDescriptor {
public:
MockUsbInterfaceAltSettingDescriptor(int interface_number,
int alternate_setting)
: interface_number_(interface_number),
alternate_setting_(alternate_setting) {}

virtual size_t GetNumEndpoints() const OVERRIDE {
// See IsAndroidInterface function in android_usb_device.cc
return 2;
}

virtual scoped_refptr<const UsbEndpointDescriptor> GetEndpoint(
size_t index) const OVERRIDE {
EXPECT_GT(static_cast<size_t>(2), index);
MockUsbEndpointDescriptor* result = new MockUsbEndpointDescriptor();
result->address_ = index + 1;
result->usb_transfer_type_ = device::USB_TRANSFER_BULK;
result->direction_ = ((index == 0) ? device::USB_DIRECTION_INBOUND
: device::USB_DIRECTION_OUTBOUND);
result->maximum_packet_size_ = 1 << 20; // 1Mb maximum packet size
return result;
}

virtual int GetInterfaceNumber() const OVERRIDE { return interface_number_; }

virtual int GetAlternateSetting() const OVERRIDE {
return alternate_setting_;
}

virtual int GetInterfaceClass() const OVERRIDE { return T::kClass; }

virtual int GetInterfaceSubclass() const OVERRIDE { return T::kSubclass; }

virtual int GetInterfaceProtocol() const OVERRIDE { return T::kProtocol; }

protected:
virtual ~MockUsbInterfaceAltSettingDescriptor() {};

private:
const int interface_number_;
const int alternate_setting_;
};

template <class T>
class MockUsbInterfaceDescriptor : public UsbInterfaceDescriptor {
public:
explicit MockUsbInterfaceDescriptor(int interface_number)
: interface_number_(interface_number) {}

virtual size_t GetNumAltSettings() const OVERRIDE {
// See IsAndroidInterface function in android_usb_device.cc
return 1;
}
virtual scoped_refptr<const UsbInterfaceAltSettingDescriptor> GetAltSetting(
size_t index) const OVERRIDE {
EXPECT_EQ(static_cast<size_t>(0), index);
return new MockUsbInterfaceAltSettingDescriptor<T>(interface_number_, 0);
}

protected:
const int interface_number_;
virtual ~MockUsbInterfaceDescriptor() {}
};

template <class T>
class MockUsbConfigDescriptor : public UsbConfigDescriptor {
public:
MockUsbConfigDescriptor() {}

virtual size_t GetNumInterfaces() const OVERRIDE { return 1; }

virtual scoped_refptr<const UsbInterfaceDescriptor> GetInterface(
size_t index) const OVERRIDE {
EXPECT_EQ(static_cast<size_t>(0), index);
return new MockUsbInterfaceDescriptor<T>(index);
}

protected:
virtual ~MockUsbConfigDescriptor() {};
};

template <class T>
class MockUsbDevice;

Expand Down Expand Up @@ -467,14 +347,37 @@ class MockUsbDeviceHandle : public UsbDeviceHandle {
template <class T>
class MockUsbDevice : public UsbDevice {
public:
MockUsbDevice() : UsbDevice(0, 0, 0) {}
MockUsbDevice() : UsbDevice(0, 0, 0) {
UsbEndpointDescriptor bulk_in;
bulk_in.address = 0x81;
bulk_in.direction = device::USB_DIRECTION_INBOUND;
bulk_in.maximum_packet_size = 512;
bulk_in.transfer_type = device::USB_TRANSFER_BULK;

UsbEndpointDescriptor bulk_out;
bulk_out.address = 0x01;
bulk_out.direction = device::USB_DIRECTION_OUTBOUND;
bulk_out.maximum_packet_size = 512;
bulk_out.transfer_type = device::USB_TRANSFER_BULK;

UsbInterfaceDescriptor interface_desc;
interface_desc.interface_number = 0;
interface_desc.alternate_setting = 0;
interface_desc.interface_class = T::kClass;
interface_desc.interface_subclass = T::kSubclass;
interface_desc.interface_protocol = T::kProtocol;
interface_desc.endpoints.push_back(bulk_in);
interface_desc.endpoints.push_back(bulk_out);

config_desc_.interfaces.push_back(interface_desc);
}

virtual scoped_refptr<UsbDeviceHandle> Open() OVERRIDE {
return new MockUsbDeviceHandle<T>(this);
}

virtual scoped_refptr<UsbConfigDescriptor> ListInterfaces() OVERRIDE {
return new MockUsbConfigDescriptor<T>();
virtual const UsbConfigDescriptor& GetConfiguration() OVERRIDE {
return config_desc_;
}

virtual bool Close(scoped_refptr<UsbDeviceHandle> handle) OVERRIDE {
Expand All @@ -497,6 +400,9 @@ class MockUsbDevice : public UsbDevice {

protected:
virtual ~MockUsbDevice() {}

private:
UsbConfigDescriptor config_desc_;
};

class MockUsbService : public UsbService {
Expand Down
88 changes: 40 additions & 48 deletions chrome/browser/devtools/device/usb/android_usb_device.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#include "content/public/browser/browser_thread.h"
#include "crypto/rsa_private_key.h"
#include "device/core/device_client.h"
#include "device/usb/usb_descriptors.h"
#include "device/usb/usb_device.h"
#include "device/usb/usb_interface.h"
#include "device/usb/usb_service.h"
#include "net/base/ip_endpoint.h"
#include "net/base/net_errors.h"
Expand All @@ -29,7 +29,6 @@
using device::UsbConfigDescriptor;
using device::UsbDevice;
using device::UsbDeviceHandle;
using device::UsbInterfaceAltSettingDescriptor;
using device::UsbInterfaceDescriptor;
using device::UsbEndpointDescriptor;
using device::UsbService;
Expand Down Expand Up @@ -59,17 +58,12 @@ typedef std::set<scoped_refptr<UsbDevice> > UsbDeviceSet;
base::LazyInstance<std::vector<AndroidUsbDevice*> >::Leaky g_devices =
LAZY_INSTANCE_INITIALIZER;

bool IsAndroidInterface(scoped_refptr<const UsbInterfaceDescriptor> interface) {
if (interface->GetNumAltSettings() == 0)
return false;

scoped_refptr<const UsbInterfaceAltSettingDescriptor> idesc =
interface->GetAltSetting(0);

if (idesc->GetInterfaceClass() != kAdbClass ||
idesc->GetInterfaceSubclass() != kAdbSubclass ||
idesc->GetInterfaceProtocol() != kAdbProtocol ||
idesc->GetNumEndpoints() != 2) {
bool IsAndroidInterface(const UsbInterfaceDescriptor& interface) {
if (interface.alternate_setting != 0 ||
interface.interface_class != kAdbClass ||
interface.interface_subclass != kAdbSubclass ||
interface.interface_protocol != kAdbProtocol ||
interface.endpoints.size() != 2) {
return false;
}
return true;
Expand All @@ -78,40 +72,40 @@ bool IsAndroidInterface(scoped_refptr<const UsbInterfaceDescriptor> interface) {
scoped_refptr<AndroidUsbDevice> ClaimInterface(
crypto::RSAPrivateKey* rsa_key,
scoped_refptr<UsbDeviceHandle> usb_handle,
scoped_refptr<const UsbInterfaceDescriptor> interface,
int interface_id) {
scoped_refptr<const UsbInterfaceAltSettingDescriptor> idesc =
interface->GetAltSetting(0);

const UsbInterfaceDescriptor& interface) {
int inbound_address = 0;
int outbound_address = 0;
int zero_mask = 0;

for (size_t i = 0; i < idesc->GetNumEndpoints(); ++i) {
scoped_refptr<const UsbEndpointDescriptor> edesc =
idesc->GetEndpoint(i);
if (edesc->GetTransferType() != device::USB_TRANSFER_BULK)
for (UsbEndpointDescriptor::Iterator endpointIt = interface.endpoints.begin();
endpointIt != interface.endpoints.end();
++endpointIt) {
if (endpointIt->transfer_type != device::USB_TRANSFER_BULK)
continue;
if (edesc->GetDirection() == device::USB_DIRECTION_INBOUND)
inbound_address = edesc->GetAddress();
if (endpointIt->direction == device::USB_DIRECTION_INBOUND)
inbound_address = endpointIt->address;
else
outbound_address = edesc->GetAddress();
zero_mask = edesc->GetMaximumPacketSize() - 1;
outbound_address = endpointIt->address;
zero_mask = endpointIt->maximum_packet_size - 1;
}

if (inbound_address == 0 || outbound_address == 0)
return NULL;

if (!usb_handle->ClaimInterface(interface_id))
if (!usb_handle->ClaimInterface(interface.interface_number))
return NULL;

base::string16 serial;
if (!usb_handle->GetSerial(&serial) || serial.empty())
return NULL;

return new AndroidUsbDevice(rsa_key, usb_handle, base::UTF16ToASCII(serial),
inbound_address, outbound_address, zero_mask,
interface_id);
return new AndroidUsbDevice(rsa_key,
usb_handle,
base::UTF16ToASCII(serial),
inbound_address,
outbound_address,
zero_mask,
interface.interface_number);
}

uint32 Checksum(const std::string& data) {
Expand Down Expand Up @@ -207,12 +201,11 @@ static void OpenAndroidDeviceOnFileThread(
bool success) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
if (success) {
scoped_refptr<UsbConfigDescriptor> config = device->ListInterfaces();
const UsbConfigDescriptor& config = device->GetConfiguration();
scoped_refptr<UsbDeviceHandle> usb_handle = device->Open();
if (usb_handle.get()) {
scoped_refptr<AndroidUsbDevice> android_device =
ClaimInterface(rsa_key, usb_handle, config->GetInterface(interface_id),
interface_id);
ClaimInterface(rsa_key, usb_handle, config.interfaces[interface_id]);
if (android_device.get())
devices->push_back(android_device.get());
else
Expand All @@ -229,15 +222,17 @@ static int CountOnFileThread() {
if (service != NULL)
service->GetDevices(&usb_devices);
int device_count = 0;
for (UsbDevices::iterator it = usb_devices.begin(); it != usb_devices.end();
++it) {
scoped_refptr<UsbConfigDescriptor> config = (*it)->ListInterfaces();
if (!config.get())
continue;

for (size_t j = 0; j < config->GetNumInterfaces(); ++j) {
if (IsAndroidInterface(config->GetInterface(j)))
for (UsbDevices::iterator deviceIt = usb_devices.begin();
deviceIt != usb_devices.end();
++deviceIt) {
const UsbConfigDescriptor& config = (*deviceIt)->GetConfiguration();

for (UsbInterfaceDescriptor::Iterator ifaceIt = config.interfaces.begin();
ifaceIt != config.interfaces.end();
++ifaceIt) {
if (IsAndroidInterface(*ifaceIt)) {
++device_count;
}
}
}
return device_count;
Expand All @@ -264,16 +259,13 @@ static void EnumerateOnFileThread(

for (UsbDevices::iterator it = usb_devices.begin(); it != usb_devices.end();
++it) {
scoped_refptr<UsbConfigDescriptor> config = (*it)->ListInterfaces();
if (!config.get()) {
barrier.Run();
continue;
}
const UsbConfigDescriptor& config = (*it)->GetConfiguration();

bool has_android_interface = false;
for (size_t j = 0; j < config->GetNumInterfaces(); ++j) {
if (!IsAndroidInterface(config->GetInterface(j)))
for (size_t j = 0; j < config.interfaces.size(); ++j) {
if (!IsAndroidInterface(config.interfaces[j])) {
continue;
}

// Request permission on Chrome OS.
#if defined(OS_CHROMEOS)
Expand Down
5 changes: 2 additions & 3 deletions device/usb/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ source_set("usb") {
sources = [
"usb_context.cc",
"usb_context.h",
"usb_descriptors.cc",
"usb_descriptors.h",
"usb_device_impl.cc",
"usb_device_impl.h",
"usb_device.h",
Expand All @@ -21,9 +23,6 @@ source_set("usb") {
"usb_error.h",
"usb_ids.cc",
"usb_ids.h",
"usb_interface.h",
"usb_interface_impl.cc",
"usb_interface_impl.h",
"usb_service.h",
"usb_service_impl.cc",
generated_ids,
Expand Down
5 changes: 2 additions & 3 deletions device/usb/usb.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
'sources': [
'usb_context.cc',
'usb_context.h',
'usb_descriptors.cc',
'usb_descriptors.h',
'usb_device_impl.cc',
'usb_device_impl.h',
'usb_device.h',
Expand All @@ -31,9 +33,6 @@
'usb_error.h',
'usb_ids.cc',
'usb_ids.h',
'usb_interface.h',
'usb_interface_impl.cc',
'usb_interface_impl.h',
'usb_service.h',
'usb_service_impl.cc',
],
Expand Down
Loading

0 comments on commit 2191196

Please sign in to comment.