From d692a7c804728a3b83bfe8c40d10d95a6e30cace Mon Sep 17 00:00:00 2001 From: "ikarienator@chromium.org" Date: Mon, 26 Aug 2013 12:40:57 +0000 Subject: Fix the horrible naming of UsbInterfaceDescriptor. Removing unnecessary ref-countings. Review URL: https://chromiumcodereview.appspot.com/22678007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219529 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/devtools/adb/android_usb_device.cc | 13 ++-- chrome/browser/extensions/api/usb/usb_api.cc | 25 ++++---- chrome/browser/extensions/api/usb/usb_api.h | 1 - chrome/browser/extensions/api/usb/usb_apitest.cc | 6 +- chrome/browser/usb/usb_device.cc | 11 ++-- chrome/browser/usb/usb_device.h | 2 +- chrome/browser/usb/usb_device_handle.cc | 13 ++-- chrome/browser/usb/usb_device_handle.h | 5 +- chrome/browser/usb/usb_interface.cc | 49 +++++++-------- chrome/browser/usb/usb_interface.h | 75 +++++++++++++---------- 10 files changed, 106 insertions(+), 94 deletions(-) diff --git a/chrome/browser/devtools/adb/android_usb_device.cc b/chrome/browser/devtools/adb/android_usb_device.cc index f32b665..511e862 100644 --- a/chrome/browser/devtools/adb/android_usb_device.cc +++ b/chrome/browser/devtools/adb/android_usb_device.cc @@ -48,12 +48,12 @@ base::LazyInstance::Leaky g_devices = scoped_refptr ClaimInterface( crypto::RSAPrivateKey* rsa_key, scoped_refptr usb_device, - const UsbInterface* interface) { + scoped_refptr interface) { if (interface->GetNumAltSettings() == 0) return NULL; - scoped_refptr idesc = - interface->GetAltSetting(0).get(); + scoped_refptr idesc = + interface->GetAltSetting(0); if (idesc->GetInterfaceClass() != kAdbClass || idesc->GetInterfaceSubclass() != kAdbSubclass || @@ -68,7 +68,7 @@ scoped_refptr ClaimInterface( for (size_t i = 0; i < idesc->GetNumEndpoints(); ++i) { scoped_refptr edesc = - idesc->GetEndpoint(i).get(); + idesc->GetEndpoint(i); if (edesc->GetTransferType() != USB_TRANSFER_BULK) continue; if (edesc->GetDirection() == USB_DIRECTION_INBOUND) @@ -187,9 +187,8 @@ static void EnumerateOnFileThread(crypto::RSAPrivateKey* rsa_key, if (ContainsKey(claimed_devices, it->get())) continue; - scoped_refptr config = new UsbConfigDescriptor(); - bool success = (*it)->ListInterfaces(config.get()); - if (!success) + scoped_refptr config = (*it)->ListInterfaces(); + if (!config) continue; scoped_refptr usb_device = (*it)->Open(); diff --git a/chrome/browser/extensions/api/usb/usb_api.cc b/chrome/browser/extensions/api/usb/usb_api.cc index 679f7fa..c8ec848 100644 --- a/chrome/browser/extensions/api/usb/usb_api.cc +++ b/chrome/browser/extensions/api/usb/usb_api.cc @@ -489,10 +489,10 @@ void UsbListInterfacesFunction::AsyncWorkStart() { return; } - config_ = new UsbConfigDescriptor(); - bool success = resource->device()->device()->ListInterfaces(config_.get()); + scoped_refptr config = + resource->device()->device()->ListInterfaces(); - if (!success) { + if (!config) { SetError(kErrorCannotListInterfaces); AsyncWorkCompleted(); return; @@ -500,16 +500,17 @@ void UsbListInterfacesFunction::AsyncWorkStart() { result_.reset(new base::ListValue()); - for (size_t i = 0, numInterfaces = config_->GetNumInterfaces(); - i < numInterfaces; ++i) { - scoped_refptr usbInterface(config_->GetInterface(i)); - for (size_t j = 0, numDescriptors = usbInterface->GetNumAltSettings(); - j < numDescriptors; ++j) { - scoped_refptr descriptor - = usbInterface->GetAltSetting(j); + for (size_t i = 0, num_interfaces = config->GetNumInterfaces(); + i < num_interfaces; ++i) { + scoped_refptr + usb_interface(config->GetInterface(i)); + for (size_t j = 0, num_descriptors = usb_interface->GetNumAltSettings(); + j < num_descriptors; ++j) { + scoped_refptr descriptor + = usb_interface->GetAltSetting(j); std::vector > endpoints; - for (size_t k = 0, numEndpoints = descriptor->GetNumEndpoints(); - k < numEndpoints; k++) { + for (size_t k = 0, num_endpoints = descriptor->GetNumEndpoints(); + k < num_endpoints; k++) { scoped_refptr endpoint = descriptor->GetEndpoint(k); linked_ptr endpoint_desc(new EndpointDescriptor()); diff --git a/chrome/browser/extensions/api/usb/usb_api.h b/chrome/browser/extensions/api/usb/usb_api.h index e6317ec..a778503 100644 --- a/chrome/browser/extensions/api/usb/usb_api.h +++ b/chrome/browser/extensions/api/usb/usb_api.h @@ -105,7 +105,6 @@ class UsbListInterfacesFunction : public UsbAsyncApiFunction { extensions::api::usb::UsageType* output); scoped_ptr result_; - scoped_refptr config_; scoped_ptr parameters_; }; diff --git a/chrome/browser/extensions/api/usb/usb_apitest.cc b/chrome/browser/extensions/api/usb/usb_apitest.cc index 3af2e2e..8fa2621 100644 --- a/chrome/browser/extensions/api/usb/usb_apitest.cc +++ b/chrome/browser/extensions/api/usb/usb_apitest.cc @@ -80,7 +80,7 @@ class MockUsbDevice : public UsbDevice { return false; } - MOCK_METHOD1(ListInterfaces, bool(UsbConfigDescriptor* config)); + MOCK_METHOD0(ListInterfaces, scoped_refptr()); private: MockUsbDeviceHandle* mock_handle_; @@ -118,8 +118,8 @@ IN_PROC_BROWSER_TEST_F(UsbApiTest, DeviceHandling) { } IN_PROC_BROWSER_TEST_F(UsbApiTest, ListInterfaces) { - EXPECT_CALL(*mock_device_.get(), ListInterfaces(_)) - .WillOnce(Return(false)); + EXPECT_CALL(*mock_device_.get(), ListInterfaces()) + .WillOnce(Return(scoped_refptr())); EXPECT_CALL(*mock_device_handle_.get(), Close()).Times(AnyNumber()); ASSERT_TRUE(RunExtensionTest("usb/list_interfaces")); } diff --git a/chrome/browser/usb/usb_device.cc b/chrome/browser/usb/usb_device.cc index 6041e68..a488380 100644 --- a/chrome/browser/usb/usb_device.cc +++ b/chrome/browser/usb/usb_device.cc @@ -50,8 +50,11 @@ scoped_refptr UsbDevice::Open() { PlatformUsbDeviceHandle handle; int rv = libusb_open(platform_device_, &handle); if (LIBUSB_SUCCESS == rv) { + scoped_refptr interfaces = ListInterfaces(); + if (!interfaces) + return NULL; scoped_refptr device_handle = - new UsbDeviceHandle(context_, this, handle); + new UsbDeviceHandle(context_, this, handle, interfaces); handles_.push_back(device_handle); return device_handle; } @@ -73,16 +76,16 @@ bool UsbDevice::Close(scoped_refptr handle) { return false; } -bool UsbDevice::ListInterfaces(UsbConfigDescriptor* config) { +scoped_refptr UsbDevice::ListInterfaces() { DCHECK(thread_checker_.CalledOnValidThread()); PlatformUsbConfigDescriptor platform_config; const int list_result = libusb_get_active_config_descriptor(platform_device_, &platform_config); if (list_result == 0) - config->Reset(platform_config); + return new UsbConfigDescriptor(platform_config); - return list_result == 0; + return NULL; } void UsbDevice::OnDisconnect() { diff --git a/chrome/browser/usb/usb_device.h b/chrome/browser/usb/usb_device.h index 524cee3..8891ad0 100644 --- a/chrome/browser/usb/usb_device.h +++ b/chrome/browser/usb/usb_device.h @@ -42,7 +42,7 @@ class UsbDevice : public base::RefCountedThreadSafe { // Lists the interfaces provided by the device and fills the given // UsbConfigDescriptor. // Blocking method. Must be called on FILE thread. - virtual bool ListInterfaces(UsbConfigDescriptor* config); + virtual scoped_refptr ListInterfaces(); protected: friend class UsbService; diff --git a/chrome/browser/usb/usb_device_handle.cc b/chrome/browser/usb/usb_device_handle.cc index 32903ba..6d32be4 100644 --- a/chrome/browser/usb/usb_device_handle.cc +++ b/chrome/browser/usb/usb_device_handle.cc @@ -178,12 +178,15 @@ UsbDeviceHandle::Transfer::~Transfer() {} UsbDeviceHandle::UsbDeviceHandle( scoped_refptr context, UsbDevice* device, - PlatformUsbDeviceHandle handle) - : device_(device), handle_(handle), context_(context) { + PlatformUsbDeviceHandle handle, + scoped_refptr interfaces) + : device_(device), + handle_(handle), + interfaces_(interfaces), + context_(context) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(handle) << "Cannot create device with NULL handle."; - interfaces_ = new UsbConfigDescriptor(); - device->ListInterfaces(interfaces_.get()); + DCHECK(interfaces_) << "Unabled to list interfaces"; } UsbDeviceHandle::UsbDeviceHandle() : device_(NULL), handle_(NULL) { @@ -524,7 +527,7 @@ void UsbDeviceHandle::RefreshEndpointMap() { endpoint_map_.clear(); for (ClaimedInterfaceMap::iterator it = claimed_interfaces_.begin(); it != claimed_interfaces_.end(); ++it) { - scoped_refptr interface_desc = + scoped_refptr interface_desc = interfaces_->GetInterface(it->first)->GetAltSetting( it->second->alternate_setting()); for (size_t i = 0; i < interface_desc->GetNumEndpoints(); i++) { diff --git a/chrome/browser/usb/usb_device_handle.h b/chrome/browser/usb/usb_device_handle.h index f7aa43f..3a5bd47 100644 --- a/chrome/browser/usb/usb_device_handle.h +++ b/chrome/browser/usb/usb_device_handle.h @@ -28,7 +28,7 @@ typedef libusb_transfer* PlatformUsbTransferHandle; class UsbContext; class UsbConfigDescriptor; class UsbDevice; -class UsbInterface; +class UsbInterfaceDescriptor; namespace base { class MessageLoopProxy; @@ -119,7 +119,8 @@ class UsbDeviceHandle : public base::RefCountedThreadSafe { // This constructor is called by UsbDevice. UsbDeviceHandle(scoped_refptr context, - UsbDevice* device, PlatformUsbDeviceHandle handle); + UsbDevice* device, PlatformUsbDeviceHandle handle, + scoped_refptr interfaces); // This constructor variant is for use in testing only. UsbDeviceHandle(); diff --git a/chrome/browser/usb/usb_interface.cc b/chrome/browser/usb/usb_interface.cc index b3ff83c..d5a1e0f 100644 --- a/chrome/browser/usb/usb_interface.cc +++ b/chrome/browser/usb/usb_interface.cc @@ -85,63 +85,63 @@ int UsbEndpointDescriptor::GetPollingInterval() const { return descriptor_->bInterval; } -UsbInterfaceDescriptor::UsbInterfaceDescriptor( +UsbInterfaceAltSettingDescriptor::UsbInterfaceAltSettingDescriptor( scoped_refptr config, PlatformUsbInterfaceDescriptor descriptor) : config_(config), descriptor_(descriptor) { } -UsbInterfaceDescriptor::~UsbInterfaceDescriptor() {} +UsbInterfaceAltSettingDescriptor::~UsbInterfaceAltSettingDescriptor() {} -size_t UsbInterfaceDescriptor::GetNumEndpoints() const { +size_t UsbInterfaceAltSettingDescriptor::GetNumEndpoints() const { return descriptor_->bNumEndpoints; } scoped_refptr - UsbInterfaceDescriptor::GetEndpoint(size_t index) const { - return make_scoped_refptr(new UsbEndpointDescriptor(config_, - &descriptor_->endpoint[index])); + UsbInterfaceAltSettingDescriptor::GetEndpoint(size_t index) const { + return new UsbEndpointDescriptor(config_, &descriptor_->endpoint[index]); } -int UsbInterfaceDescriptor::GetInterfaceNumber() const { +int UsbInterfaceAltSettingDescriptor::GetInterfaceNumber() const { return descriptor_->bInterfaceNumber; } -int UsbInterfaceDescriptor::GetAlternateSetting() const { +int UsbInterfaceAltSettingDescriptor::GetAlternateSetting() const { return descriptor_->bAlternateSetting; } -int UsbInterfaceDescriptor::GetInterfaceClass() const { +int UsbInterfaceAltSettingDescriptor::GetInterfaceClass() const { return descriptor_->bInterfaceClass; } -int UsbInterfaceDescriptor::GetInterfaceSubclass() const { +int UsbInterfaceAltSettingDescriptor::GetInterfaceSubclass() const { return descriptor_->bInterfaceSubClass; } -int UsbInterfaceDescriptor::GetInterfaceProtocol() const { +int UsbInterfaceAltSettingDescriptor::GetInterfaceProtocol() const { return descriptor_->bInterfaceProtocol; } -UsbInterface::UsbInterface(scoped_refptr config, +UsbInterfaceDescriptor::UsbInterfaceDescriptor( + scoped_refptr config, PlatformUsbInterface usbInterface) : config_(config), interface_(usbInterface) { } -UsbInterface::~UsbInterface() {} +UsbInterfaceDescriptor::~UsbInterfaceDescriptor() {} -size_t UsbInterface::GetNumAltSettings() const { +size_t UsbInterfaceDescriptor::GetNumAltSettings() const { return interface_->num_altsetting; } -scoped_refptr - UsbInterface::GetAltSetting(size_t index) const { - return make_scoped_refptr(new UsbInterfaceDescriptor(config_, - &interface_->altsetting[index])); +scoped_refptr + UsbInterfaceDescriptor::GetAltSetting(size_t index) const { + return new UsbInterfaceAltSettingDescriptor(config_, + &interface_->altsetting[index]); } -UsbConfigDescriptor::UsbConfigDescriptor() - : config_(NULL) { +UsbConfigDescriptor::UsbConfigDescriptor(PlatformUsbConfigDescriptor config) + : config_(config) { } UsbConfigDescriptor::~UsbConfigDescriptor() { @@ -151,16 +151,11 @@ UsbConfigDescriptor::~UsbConfigDescriptor() { } } -void UsbConfigDescriptor::Reset(PlatformUsbConfigDescriptor config) { - config_ = config; -} - size_t UsbConfigDescriptor::GetNumInterfaces() const { return config_->bNumInterfaces; } -scoped_refptr +scoped_refptr UsbConfigDescriptor::GetInterface(size_t index) const { - return make_scoped_refptr(new UsbInterface(make_scoped_refptr(this), - &config_->interface[index])); + return new UsbInterfaceDescriptor(this, &config_->interface[index]); } diff --git a/chrome/browser/usb/usb_interface.h b/chrome/browser/usb/usb_interface.h index ca01210..e73b044 100644 --- a/chrome/browser/usb/usb_interface.h +++ b/chrome/browser/usb/usb_interface.h @@ -6,7 +6,6 @@ #define CHROME_BROWSER_USB_USB_INTERFACE_H_ #include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" struct libusb_config_descriptor; struct libusb_endpoint_descriptor; @@ -18,8 +17,6 @@ typedef const libusb_endpoint_descriptor* PlatformUsbEndpointDescriptor; typedef const libusb_interface* PlatformUsbInterface; typedef const libusb_interface_descriptor* PlatformUsbInterfaceDescriptor; -class UsbDeviceHandle; - enum UsbTransferType { USB_TRANSFER_CONTROL = 0, USB_TRANSFER_ISOCHRONOUS, @@ -45,13 +42,14 @@ enum UsbUsageType { USB_USAGE_EXPLICIT_FEEDBACK }; +class UsbDevice; class UsbConfigDescriptor; +class UsbInterfaceDescriptor; +class UsbInterfaceAltSettingDescriptor; -class UsbEndpointDescriptor : public base::RefCounted { +class UsbEndpointDescriptor + : public base::RefCounted { public: - UsbEndpointDescriptor(scoped_refptr config, - PlatformUsbEndpointDescriptor descriptor); - int GetAddress() const; UsbEndpointDirection GetDirection() const; int GetMaximumPacketSize() const; @@ -61,22 +59,25 @@ class UsbEndpointDescriptor : public base::RefCounted { int GetPollingInterval() const; private: - friend class base::RefCounted; + friend class base::RefCounted; + friend class UsbInterfaceAltSettingDescriptor; + + UsbEndpointDescriptor( + scoped_refptr config, + PlatformUsbEndpointDescriptor descriptor); ~UsbEndpointDescriptor(); scoped_refptr config_; PlatformUsbEndpointDescriptor descriptor_; + + DISALLOW_COPY_AND_ASSIGN(UsbEndpointDescriptor); }; -class UsbInterfaceDescriptor - : public base::RefCounted { +class UsbInterfaceAltSettingDescriptor + : public base::RefCounted { public: - UsbInterfaceDescriptor(scoped_refptr config, - PlatformUsbInterfaceDescriptor descriptor); - size_t GetNumEndpoints() const; - scoped_refptr - GetEndpoint(size_t index) const; + scoped_refptr GetEndpoint(size_t index) const; int GetInterfaceNumber() const; int GetAlternateSetting() const; @@ -85,46 +86,56 @@ class UsbInterfaceDescriptor int GetInterfaceProtocol() const; private: - friend class base::RefCounted; - ~UsbInterfaceDescriptor(); + friend class base::RefCounted; + friend class UsbInterfaceDescriptor; + + UsbInterfaceAltSettingDescriptor( + scoped_refptr config, + PlatformUsbInterfaceDescriptor descriptor); + ~UsbInterfaceAltSettingDescriptor(); scoped_refptr config_; PlatformUsbInterfaceDescriptor descriptor_; + + DISALLOW_COPY_AND_ASSIGN(UsbInterfaceAltSettingDescriptor); }; -class UsbInterface : public base::RefCounted { +class UsbInterfaceDescriptor + : public base::RefCounted { public: - UsbInterface(scoped_refptr config, - PlatformUsbInterface usbInterface); - size_t GetNumAltSettings() const; - scoped_refptr - GetAltSetting(size_t index) const; + scoped_refptr GetAltSetting( + size_t index) const; private: - friend class base::RefCounted; - ~UsbInterface(); + friend class base::RefCounted; + friend class UsbConfigDescriptor; + + UsbInterfaceDescriptor(scoped_refptr config, + PlatformUsbInterface usbInterface); + ~UsbInterfaceDescriptor(); scoped_refptr config_; PlatformUsbInterface interface_; + + DISALLOW_COPY_AND_ASSIGN(UsbInterfaceDescriptor); }; class UsbConfigDescriptor : public base::RefCounted { public: - UsbConfigDescriptor(); - - void Reset(PlatformUsbConfigDescriptor config); - size_t GetNumInterfaces() const; - - scoped_refptr - GetInterface(size_t index) const; + scoped_refptr GetInterface(size_t index) const; private: friend class base::RefCounted; + friend class UsbDevice; + + explicit UsbConfigDescriptor(PlatformUsbConfigDescriptor config); ~UsbConfigDescriptor(); PlatformUsbConfigDescriptor config_; + + DISALLOW_COPY_AND_ASSIGN(UsbConfigDescriptor); }; #endif // CHROME_BROWSER_USB_USB_INTERFACE_H_ -- cgit v1.1