Add more generic filters to the chrome.usb.getDevices API.

Instead of specifying each of the USB vendor and product ID pairs an
app has permission to open this API extension allows filtering of
devices by vendor and product IDs as well as USB interface classes, sub-
classes and protocols. The 'vendorId' and 'productId' properties of
EnumerateDevicesOptions are still supported for backwards compatibility.

BUG=

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

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

components/usb_service/usb_device_filter.cc

@@ -6,7 +6,6 @@
 
 #include "base/values.h"
 #include "components/usb_service/usb_device.h"
-#include "components/usb_service/usb_device_handle.h"
 #include "components/usb_service/usb_interface.h"
 
 namespace usb_service {
@@ -57,7 +56,7 @@ void UsbDeviceFilter::SetInterfaceProtocol(uint8 interface_protocol) {
   interface_protocol_ = interface_protocol;
 }
 
-bool UsbDeviceFilter::Matches(scoped_refptr<UsbDevice> device) {
+bool UsbDeviceFilter::Matches(scoped_refptr<UsbDevice> device) const {
   if (vendor_id_set_) {
     if (device->vendor_id() != vendor_id_) {
       return false;
@@ -125,4 +124,17 @@ base::Value* UsbDeviceFilter::ToValue() const {
   return obj.release();
 }
 
+// static
+bool UsbDeviceFilter::MatchesAny(scoped_refptr<UsbDevice> device,
+                                 const std::vector<UsbDeviceFilter>& filters) {
+  for (std::vector<UsbDeviceFilter>::const_iterator i = filters.begin();
+       i != filters.end();
+       ++i) {
+    if (i->Matches(device)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace usb_service

components/usb_service/usb_device_filter.h

@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_USB_SERVICE_USB_DEVICE_FILTER_H_
 #define COMPONENTS_USB_SERVICE_USB_DEVICE_FILTER_H_
 
+#include <vector>
+
 #include "base/memory/ref_counted.h"
 #include "components/usb_service/usb_service_export.h"
 
@@ -27,9 +29,12 @@ class USB_SERVICE_EXPORT UsbDeviceFilter {
   void SetInterfaceSubclass(uint8 interface_subclass);
   void SetInterfaceProtocol(uint8 interface_protocol);
 
-  bool Matches(scoped_refptr<UsbDevice> device);
+  bool Matches(scoped_refptr<UsbDevice> device) const;
   base::Value* ToValue() const;
 
+  static bool MatchesAny(scoped_refptr<UsbDevice> device,
+                         const std::vector<UsbDeviceFilter>& filters);
+
  private:
   uint16 vendor_id_;
   uint16 product_id_;

components/usb_service/usb_device_filter_unittest.cc

@@ -237,3 +237,20 @@ TEST_F(UsbFilterTest, MatchInterfaceProtocolNegative) {
   filter.SetInterfaceProtocol(0x02);
   ASSERT_FALSE(filter.Matches(android_phone_));
 }
+
+TEST_F(UsbFilterTest, MatchAnyEmptyListNegative) {
+  std::vector<UsbDeviceFilter> filters;
+  ASSERT_FALSE(UsbDeviceFilter::MatchesAny(android_phone_, filters));
+}
+
+TEST_F(UsbFilterTest, MatchesAnyVendorId) {
+  std::vector<UsbDeviceFilter> filters(1);
+  filters.back().SetVendorId(0x18d1);
+  ASSERT_TRUE(UsbDeviceFilter::MatchesAny(android_phone_, filters));
+}
+
+TEST_F(UsbFilterTest, MatchesAnyVendorIdNegative) {
+  std::vector<UsbDeviceFilter> filters(1);
+  filters.back().SetVendorId(0x1d6b);
+  ASSERT_FALSE(UsbDeviceFilter::MatchesAny(android_phone_, filters));
+}

extensions/browser/api/usb/usb_api.cc

@@ -52,6 +52,7 @@ using usb::TransferType;
 using usb::UsageType;
 using usb_service::UsbConfigDescriptor;
 using usb_service::UsbDevice;
+using usb_service::UsbDeviceFilter;
 using usb_service::UsbDeviceHandle;
 using usb_service::UsbEndpointDescriptor;
 using usb_service::UsbEndpointDirection;
@@ -417,35 +418,51 @@ bool UsbAsyncApiFunction::Respond() {
   return error_.empty();
 }
 
-scoped_refptr<UsbDevice> UsbAsyncApiFunction::GetDeviceOrOrCompleteWithError(
-    const Device& input_device) {
-  const uint16_t vendor_id = input_device.vendor_id;
-  const uint16_t product_id = input_device.product_id;
-  UsbDevicePermission::CheckParam param(
-      vendor_id, product_id, UsbDevicePermissionData::UNSPECIFIED_INTERFACE);
-  if (!extension()->permissions_data()->CheckAPIPermissionWithParam(
-          APIPermission::kUsbDevice, &param)) {
-    LOG(WARNING) << "Insufficient permissions to access device.";
-    CompleteWithError(kErrorPermissionDenied);
-    return NULL;
+// static
+void UsbAsyncApiFunction::CreateDeviceFilter(const usb::DeviceFilter& input,
+                                             UsbDeviceFilter* output) {
+  if (input.vendor_id) {
+    output->SetVendorId(*input.vendor_id);
+  }
+  if (input.product_id) {
+    output->SetProductId(*input.product_id);
+  }
+  if (input.interface_class) {
+    output->SetInterfaceClass(*input.interface_class);
   }
+  if (input.interface_subclass) {
+    output->SetInterfaceSubclass(*input.interface_subclass);
+  }
+  if (input.interface_protocol) {
+    output->SetInterfaceProtocol(*input.interface_protocol);
+  }
+}
 
+bool UsbAsyncApiFunction::HasDevicePermission(
+    scoped_refptr<usb_service::UsbDevice> device) {
+  UsbDevicePermission::CheckParam param(
+      device->vendor_id(),
+      device->product_id(),
+      UsbDevicePermissionData::UNSPECIFIED_INTERFACE);
+  return extension()->permissions_data()->CheckAPIPermissionWithParam(
+      APIPermission::kUsbDevice, &param);
+}
+
+scoped_refptr<UsbDevice> UsbAsyncApiFunction::GetDeviceOrCompleteWithError(
+    const Device& input_device) {
   UsbService* service = device::DeviceClient::Get()->GetUsbService();
   if (!service) {
     CompleteWithError(kErrorInitService);
     return NULL;
   }
-  scoped_refptr<UsbDevice> device;
-
-  device = service->GetDeviceById(input_device.device);
 
+  scoped_refptr<UsbDevice> device = service->GetDeviceById(input_device.device);
   if (!device.get()) {
     CompleteWithError(kErrorNoDevice);
     return NULL;
   }
 
-  if (device->vendor_id() != input_device.vendor_id ||
-      device->product_id() != input_device.product_id) {
+  if (!HasDevicePermission(device)) {
     // Must act as if there is no such a device.
     // Otherwise can be used to finger print unauthorized devices.
     CompleteWithError(kErrorNoDevice);
@@ -624,17 +641,20 @@ bool UsbGetDevicesFunction::Prepare() {
 }
 
 void UsbGetDevicesFunction::AsyncWorkStart() {
-  scoped_ptr<base::ListValue> result(new base::ListValue());
-
-  const uint16_t vendor_id = parameters_->options.vendor_id;
-  const uint16_t product_id = parameters_->options.product_id;
-  UsbDevicePermission::CheckParam param(
-      vendor_id, product_id, UsbDevicePermissionData::UNSPECIFIED_INTERFACE);
-  if (!extension()->permissions_data()->CheckAPIPermissionWithParam(
-          APIPermission::kUsbDevice, &param)) {
-    LOG(WARNING) << "Insufficient permissions to access device.";
-    CompleteWithError(kErrorPermissionDenied);
-    return;
+  std::vector<UsbDeviceFilter> filters;
+  if (parameters_->options.filters) {
+    filters.resize(parameters_->options.filters->size());
+    for (size_t i = 0; i < parameters_->options.filters->size(); ++i) {
+      CreateDeviceFilter(*parameters_->options.filters->at(i).get(),
+                         &filters[i]);
+    }
+  }
+  if (parameters_->options.vendor_id) {
+    filters.resize(filters.size() + 1);
+    filters.back().SetVendorId(*parameters_->options.vendor_id);
+    if (parameters_->options.product_id) {
+      filters.back().SetProductId(*parameters_->options.product_id);
+    }
   }
 
   UsbService* service = device::DeviceClient::Get()->GetUsbService();
@@ -646,18 +666,15 @@ void UsbGetDevicesFunction::AsyncWorkStart() {
   DeviceVector devices;
   service->GetDevices(&devices);
 
-  for (DeviceVector::iterator it = devices.begin(); it != devices.end();) {
-    if ((*it)->vendor_id() != vendor_id || (*it)->product_id() != product_id) {
-      it = devices.erase(it);
-    } else {
-      ++it;
+  scoped_ptr<base::ListValue> result(new base::ListValue());
+  for (DeviceVector::iterator it = devices.begin(); it != devices.end(); ++it) {
+    scoped_refptr<UsbDevice> device = *it;
+    if ((filters.empty() || UsbDeviceFilter::MatchesAny(device, filters)) &&
+        HasDevicePermission(device)) {
+      result->Append(PopulateDevice(it->get()));
     }
   }
 
-  for (size_t i = 0; i < devices.size(); ++i) {
-    result->Append(PopulateDevice(devices[i].get()));
-  }
-
   SetResult(result.release());
   AsyncWorkCompleted();
 }
@@ -677,7 +694,7 @@ bool UsbRequestAccessFunction::Prepare() {
 void UsbRequestAccessFunction::AsyncWorkStart() {
 #if defined(OS_CHROMEOS)
   scoped_refptr<UsbDevice> device =
-      GetDeviceOrOrCompleteWithError(parameters_->device);
+      GetDeviceOrCompleteWithError(parameters_->device);
   if (!device)
     return;
 
@@ -709,7 +726,7 @@ bool UsbOpenDeviceFunction::Prepare() {
 
 void UsbOpenDeviceFunction::AsyncWorkStart() {
   scoped_refptr<UsbDevice> device =
-      GetDeviceOrOrCompleteWithError(parameters_->device);
+      GetDeviceOrCompleteWithError(parameters_->device);
   if (!device.get())
     return;
 

extensions/browser/api/usb/usb_api.h

@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/usb_service/usb_device.h"
+#include "components/usb_service/usb_device_filter.h"
 #include "components/usb_service/usb_device_handle.h"
 #include "extensions/browser/api/api_resource_manager.h"
 #include "extensions/browser/api/async_api_function.h"
@@ -31,7 +32,13 @@ class UsbAsyncApiFunction : public AsyncApiFunction {
   virtual bool PrePrepare() OVERRIDE;
   virtual bool Respond() OVERRIDE;
 
-  scoped_refptr<usb_service::UsbDevice> GetDeviceOrOrCompleteWithError(
+  static void CreateDeviceFilter(
+      const extensions::core_api::usb::DeviceFilter& input,
+      usb_service::UsbDeviceFilter* output);
+
+  bool HasDevicePermission(scoped_refptr<usb_service::UsbDevice> device);
+
+  scoped_refptr<usb_service::UsbDevice> GetDeviceOrCompleteWithError(
       const extensions::core_api::usb::Device& input_device);
 
   scoped_refptr<usb_service::UsbDeviceHandle>

extensions/browser/api/usb_private/usb_private_api.cc

@@ -16,6 +16,7 @@
 #include "device/usb/usb_ids.h"
 #include "extensions/common/api/usb_private.h"
 
+namespace usb = extensions::core_api::usb;
 namespace usb_private = extensions::core_api::usb_private;
 namespace GetDevices = usb_private::GetDevices;
 namespace GetDeviceInfo = usb_private::GetDeviceInfo;
@@ -26,6 +27,8 @@ using usb_service::UsbDeviceFilter;
 using usb_service::UsbDeviceHandle;
 using usb_service::UsbService;
 
+typedef std::vector<scoped_refptr<UsbDevice> > DeviceVector;
+
 namespace {
 
 const char kErrorInitService[] = "Failed to initialize USB service.";
@@ -58,46 +61,16 @@ void UsbPrivateGetDevicesFunction::AsyncWorkStart() {
   std::vector<UsbDeviceFilter> filters;
   filters.resize(parameters_->filters.size());
   for (size_t i = 0; i < parameters_->filters.size(); ++i) {
-    UsbDeviceFilter& filter = filters[i];
-    const usb_private::DeviceFilter* filter_param =
-        parameters_->filters[i].get();
-
-    if (filter_param->vendor_id) {
-      filter.SetVendorId(*filter_param->vendor_id);
-    }
-    if (filter_param->product_id) {
-      filter.SetProductId(*filter_param->product_id);
-    }
-    if (filter_param->interface_class) {
-      filter.SetInterfaceClass(*filter_param->interface_class);
-    }
-    if (filter_param->interface_subclass) {
-      filter.SetInterfaceSubclass(*filter_param->interface_subclass);
-    }
-    if (filter_param->interface_protocol) {
-      filter.SetInterfaceProtocol(*filter_param->interface_protocol);
-    }
+    CreateDeviceFilter(*parameters_->filters[i].get(), &filters[i]);
   }
 
-  std::vector<scoped_refptr<UsbDevice> > devices;
+  DeviceVector devices;
   service->GetDevices(&devices);
 
   scoped_ptr<base::ListValue> result(new base::ListValue());
-  for (size_t i = 0; i < devices.size(); ++i) {
-    scoped_refptr<UsbDevice> device = devices[i];
-    bool matched = false;
-
-    if (filters.empty()) {
-      matched = true;
-    } else {
-      for (size_t j = 0; !matched && j < filters.size(); ++j) {
-        if (filters[j].Matches(device)) {
-          matched = true;
-        }
-      }
-    }
-
-    if (matched) {
+  for (DeviceVector::iterator it = devices.begin(); it != devices.end(); ++it) {
+    scoped_refptr<UsbDevice> device = *it;
+    if (filters.empty() || UsbDeviceFilter::MatchesAny(device, filters)) {
       result->Append(new base::FundamentalValue((int)device->unique_id()));
     }
   }

extensions/common/api/usb.idl

@@ -5,6 +5,10 @@
 // Use the <code>chrome.usb</code> API to interact with connected USB
 // devices. This API provides access to USB operations from within the context
 // of an app. Using this API, apps can function as drivers for hardware devices.
+//
+// Errors generated by this API are reported by setting
+// $(ref:runtime.lastError) and executing the function's regular callback. The
+// callback's regular parameters will be undefined in this case.
 namespace usb {
 
   // Direction, Recipient, RequestType, and TransferType all map to their
@@ -19,110 +23,107 @@ namespace usb {
   enum SynchronizationType {asynchronous, adaptive, synchronous};
   enum UsageType {data, feedback, explicitFeedback};
 
-  // Returned by |getDevices| to identify a connected USB device.
   dictionary Device {
-    // The id of the USB device. It remains unchanged until the device is
+    // An opaque ID for the USB device. It remains unchanged until the device is
     // unplugged.
     long device;
+    // The device vendor ID.
     long vendorId;
+    // The product ID.
     long productId;
   };
 
-  // Returned by |openDevice| to be used for USB communication.
-  // Every time a device is opened, a new connection handle is created.
-  //
-  // A connection handle represents the underlying data structure that contains
-  // all the data we need to communicate with a USB device, including the status
-  // of interfaces, the pending transfers, the descriptors, and etc. A connectin
-  // handle id is different from a USB device id.
-  //
-  // All connection handles can work together if the device allows it.
-  // The connection handle will be automatically closed when the app is reloaded
-  // or suspended.
-  //
-  // When a connection handle is closed, all the interfaces it claimed will be
-  // released and all the transfers in progress will be canceled immediately.
   dictionary ConnectionHandle {
-    // The id of the USB connection handle.
+    // An opaque handle representing this connection to the USB device and all
+    // associated claimed interfaces and pending transfers. A new handle is
+    // created each time the device is opened. The connection handle is
+    // different from $(ref:Device.device).
     long handle;
+    // The device vendor ID.
     long vendorId;
+    // The product ID.
     long productId;
   };
 
-  dictionary EndpointDescriptor {
+  [noinline_doc] dictionary EndpointDescriptor {
+    // Endpoint address.
     long address;
+    // Transfer type.
     TransferType type;
+    // Transfer direction.
     Direction direction;
+    // Maximum packet size.
     long maximumPacketSize;
-
-    // Used for isochronous mode.
+    // Transfer synchronization mode (isochronous only).
     SynchronizationType? synchronization;
+    // Endpoint usage hint.
     UsageType? usage;
-
-    // If this is an interrupt endpoint, this will be 1-255.
+    // Polling interval (interrupt and isochronous only).
     long? pollingInterval;
   };
 
-  dictionary InterfaceDescriptor {
+  [noinline_doc] dictionary InterfaceDescriptor {
+    // The interface number.
     long interfaceNumber;
+    // The interface alternate setting number (defaults to <code>0</code).
     long alternateSetting;
+    // The USB interface class.
     long interfaceClass;
+    // The USB interface sub-class.
     long interfaceSubclass;
+    // The USB interface protocol.
     long interfaceProtocol;
+    // Description of the interface.
     DOMString? description;
+    // Available endpoints.
     EndpointDescriptor[] endpoints;
   };
 
-  // ControlTransferInfo represents that parameters to a single USB control
-  // transfer.
   dictionary ControlTransferInfo {
-    // The direction of this transfer.
+    // The transfer direction (<code>"in"</code> or <code>"out"</code>).
     Direction direction;
 
-    // The intended recipient for this transfer.
+    // The transfer target. The target given by <code>index</code> must be
+    // claimed if <code>"interface"</code> or <code>"endpoint"</code>.
     Recipient recipient;
 
-    // The type of this request.
+    // The request type.
     RequestType requestType;
 
+    // The <code>bRequest</code> field, see <i>Universal Serial Bus Specification
+    // Revision 1.1</i> &sect; 9.3.
     long request;
+    // The <code>wValue</code> field, see <i>Ibid</i>.
     long value;
+    // The <code>wIndex</code> field, see <i>Ibid</i>.
     long index;
 
-    // If this transfer is an input transfer, then this field must be set to
-    // indicate the expected data length. If this is an output transfer, then
-    // this field is ignored.
+    // The amount of data to receive (required only by input transfers).
     long? length;
 
-    // The data payload carried by this transfer. If this is an output transfer
-    // then this field must be set.
+    // The data to transmit (required only by output transfers).
     ArrayBuffer? data;
   };
 
-  // GenericTransferInfo is used by both bulk and interrupt transfers to
-  // specify the parameters of the transfer.
   dictionary GenericTransferInfo {
-    // The direction of this transfer.
+    // The transfer direction (<code>"in"</code> or <code>"out"</code>).
     Direction direction;
 
+    // The target endpoint address. The interface containing this endpoint must
+    // be claimed.
     long endpoint;
 
-    // If this is an input transfer then this field indicates the size of the
-    // input buffer. If this is an output transfer then this field is ignored.
+    // The amount of data to receive (required only by input transfers).
     long? length;
 
-    // If this is an output transfer then this field must be populated.
-    // Otherwise, it will be ignored.
+    // The data to transmit (required only by output transfers).
     ArrayBuffer? data;
   };
 
-  // IsochronousTransferInfo describes a single multi-packet isochronous
-  // transfer.
   dictionary IsochronousTransferInfo {
-    // All of the normal transfer parameters are encapsulated in the
-    // transferInfo parameters. Note that the data specified in this parameter
-    // block is split along packetLength boundaries to form the individual
-    // packets of the transfer.
+    // Transfer parameters. The transfer length or data buffer specified in this
+    // parameter block is split along <code>packetLength</code> boundaries to
+    // form the individual packets of the transfer.
     GenericTransferInfo transferInfo;
 
     // The total number of packets in this transfer.
@@ -133,174 +134,177 @@ namespace usb {
   };
 
   dictionary TransferResultInfo {
-    // A value of 0 indicates that the transfer was a success. Other values
-    // indicate failure.
+    // A value of <code>0</code> indicates that the transfer was a success.
+    // Other values indicate failure.
     long? resultCode;
 
-    // If the transfer was an input transfer then this field will contain all
-    // of the input data requested.
+    // The data returned by an input transfer. <code>undefined</code> for output
+    // transfers.
     ArrayBuffer? data;
   };
 
-  // Describes the properties of devices which are found via |getDevices|.
+  [noinline_doc] dictionary DeviceFilter {
+    // Device vendor ID.
+    long? vendorId;
+    // Device product ID, checked only if the vendor ID matches.
+    long? productId;
+    // USB interface class, matches any interface on the device.
+    long? interfaceClass;
+    // USB interface sub-class, checked only if the interface class matches.
+    long? interfaceSubclass;
+    // USB interface protocol, checked only if the interface sub-class matches.
+    long? interfaceProtocol;
+  };
+
   dictionary EnumerateDevicesOptions {
-    long vendorId;
-    long productId;
+    [deprecated="Equivalent to setting $(ref:DeviceFilter.vendorId)."]
+    long? vendorId;
+    [deprecated="Equivalent to setting $(ref:DeviceFilter.productId)."]
+    long? productId;
+    // A device matching any given filter will be returned. An empty filter list
+    // will return all devices the app has permission for.
+    DeviceFilter[]? filters;
   };
   
-  // Describes the properties of devices which are found via |findDevices|.
   dictionary EnumerateDevicesAndRequestAccessOptions {
+    // The device vendor ID.
     long vendorId;
+    // The product ID.
     long productId;
-    // The interface id to request access against.
+    // The interface ID to request access to.
     // Only available on ChromeOS. It has no effect on other platforms.
     long? interfaceId;
   };
 
   callback VoidCallback = void ();
   callback GetDevicesCallback = void (Device[] devices);
-  callback RequestAccessCallback = void (boolean sucess);
+  callback RequestAccessCallback = void (boolean success);
   callback OpenDeviceCallback = void (ConnectionHandle handle);
   callback FindDevicesCallback = void (ConnectionHandle[] handles);
   callback ListInterfacesCallback = void (InterfaceDescriptor[] descriptors);
   callback CloseDeviceCallback = void ();
   callback TransferCallback = void (TransferResultInfo info);
-  callback ResetDeviceCallback = void(boolean result);
+  callback ResetDeviceCallback = void(boolean success);
 
   interface Functions {
-    // Lists USB devices specified by vendorId/productId/interfaceId tuple.
+    // Enumerates connected USB devices.
     // |options|: The properties to search for on target devices.
-    // |callback|: Invoked with a list of |Device|s on complete.
     static void getDevices(EnumerateDevicesOptions options,
                            GetDevicesCallback callback);
 
-    // This method is ChromeOS specific. Calling this method on other platforms
-    // will fail.
-    // Requests access from the permission broker to an OS claimed device if the
-    // given interface on the device is not claimed.
+    // Requests access from the permission broker to a device claimed by
+    // ChromeOS if the given interface on the device is not claimed.
+    //
+    // <b>Note:</b> This method is ChromeOS specific. Calling this method on
+    // other platforms will fail.
     //
-    // |device|: The device to request access to.
-    // |interfaceId|: 
+    // |device|: The $(ref:Device) to request access to.
+    // |interfaceId|: The particular interface requested.
     static void requestAccess(Device device,
                               long interfaceId,
                               RequestAccessCallback callback);
 
-    // Opens a USB device returned by |getDevices|.
-    // |device|: The device to open.
-    // |callback|: Invoked with the created ConnectionHandle on complete.
+    // Opens a USB device returned by $(ref:getDevices).
+    // |device|: The $(ref:Device) to open.
     static void openDevice(Device device, OpenDeviceCallback callback);
 
-    // Finds USB devices specified by the vendorId/productId/interfaceId tuple
-    // and, if permissions allow, opens them for use.
+    // Finds USB devices specified by the vendor, product and (optionally)
+    // interface IDs and if permissions allow opens them for use.
     //
-    // On Chrome OS, you can specify the interfaceId. In that case the method
-    // will request access from permission broker in the same way as in
-    // |requestUsbAcess|.
+    // On Chrome OS, you can specify the interface ID. In that case the method
+    // will request access from permission broker in the same way as
+    // $(ref:requestUsbAccess).
     //
-    // If the access request is rejected, or the device is failed to be opened,
-    // its connection handle will not be created or returned.
+    // If the access request is rejected or the device fails to be opened a
+    // connection handle will not be created or returned.
     //
-    // Calling this method is equivalent to calling |getDevices| followed by
-    // a series of |requestAccess| (if it is on ChromeOs) and |openDevice|
-    // calls, and returning all the successfully opened connection handles.
+    // Calling this method is equivalent to calling $(ref:getDevices followed by
+    // $(ref:requestAccess) (if it is on ChromeOS) and $(ref:openDevice) for
+    // each device.
     //
     // |options|: The properties to search for on target devices.
-    // |callback|: Invoked with the opened ConnectionHandle on complete.
     static void findDevices(EnumerateDevicesAndRequestAccessOptions options,
                             FindDevicesCallback callback);
 
-    // Closes a connection handle. Invoking operations on a device after it
-    // has been closed is a safe operation, but causes no action to be taken.
-    // |handle|: The connection handle to close.
-    // |callback|: The callback to invoke once the device is closed.
+    // Closes a connection handle. Invoking operations on a handle after it
+    // has been closed is a safe operation but causes no action to be taken.
+    // |handle|: The $(ref:ConnectionHandle) to close.
     static void closeDevice(ConnectionHandle handle,
                             optional CloseDeviceCallback callback);
 
-    // Lists all the interfaces on the USB device.
-    // |handle|: The device from which the interfaces should be listed.
-    // |callback|: The callback to invoke when the interfaces are enumerated.
+    // Lists all interfaces on a USB device.
+    // |handle|: An open connection to the device.
     static void listInterfaces(ConnectionHandle handle,
                                ListInterfacesCallback callback);
 
-    // Claims an interface on the specified USB device.
-    // Before you can transfer data with endpoints, you must claim their parent
-    // interfaces. Only one connection handle on the same host can claim each
-    // interface. If the interface is already claimed, this call will fail.
+    // Claims an interface on a USB device.
+    // Before data can be transfered to an interface or associated endpoints the
+    // interface must be claimed. Only one connection handle can claim an
+    // interface at any given time. If the interface is already claimed, this
+    // call will fail.
     //
-    // You shall call releaseInterface when the interface is not needed anymore.
+    // $(ref:releaseInterface) should be called when the interface is no longer
+    // needed.
     //
-    // |handle|: The device on which the interface is to be claimed.
-    // |interface|: The interface number to be claimed.
-    // |callback|: The callback to invoke once the interface is claimed.
+    // |handle|: An open connection to the device.
+    // |interfaceNumber|: The interface to be claimed.
     static void claimInterface(ConnectionHandle handle, long interfaceNumber,
                                VoidCallback callback);
 
-    // Releases a claim to an interface on the provided device.
-    // |handle|: The device on which the interface is to be released.
-    // |interface|: The interface number to be released.
-    // |callback|: The callback to invoke once the interface is released.
+    // Releases a claimed interface.
+    // |handle|: An open connection to the device.
+    // |interfaceNumber|: The interface to be released.
     static void releaseInterface(ConnectionHandle handle, long interfaceNumber,
                                  VoidCallback callback);
 
-    // Selects an alternate setting on a previously claimed interface on a
-    // device.
-    // |handle|: The device on which the interface settings are to be set.
-    // |interface|: The interface number to be set.
-    // |alternateSetting|: The alternate setting to set.
-    // |callback|: The callback to invoke once the interface setting is set.
+    // Selects an alternate setting on a previously claimed interface.
+    // |handle|: An open connection to the device where this interface has been
+    //     claimed.
+    // |interfaceNumber|: The interface to configure.
+    // |alternateSetting|: The alternate setting to configure.
     static void setInterfaceAlternateSetting(ConnectionHandle handle,
                                              long interfaceNumber,
                                              long alternateSetting,
                                              VoidCallback callback);
 
-    // Performs a control transfer on the specified device. See the
-    // ControlTransferInfo structure for the parameters required to make a
-    // transfer.
+    // Performs a control transfer on the specified device.
     //
-    // Conceptually control transfer talks to the device itself. You do not need
-    // to claim interface 0 to perform a control transfer.
+    // Control transfers refer to either the device, an interface or an
+    // endpoint. Transfers to an interface or endpoint require the interface to
+    // be claimed.
     //
-    // |handle|: A connection handle to make the transfer on.
-    // |transferInfo|: The parameters to the transfer. See ControlTransferInfo.
-    // |callback|: Invoked once the transfer has completed.
+    // |handle|: An open connection to the device.
     static void controlTransfer(ConnectionHandle handle,
                                 ControlTransferInfo transferInfo,
                                 TransferCallback callback);
 
     // Performs a bulk transfer on the specified device.
-    // |handle|: A connection handle to make the transfer on.
-    // |transferInfo|: The parameters to the transfer. See GenericTransferInfo.
-    // |callback|: Invoked once the transfer has completed.
+    // |handle|: An open connection to the device.
+    // |transferInfo|: The transfer parameters.
     static void bulkTransfer(ConnectionHandle handle,
                              GenericTransferInfo transferInfo,
                              TransferCallback callback);
 
     // Performs an interrupt transfer on the specified device.
-    // |handle|: A connection handle to make the transfer on.
-    // |transferInfo|: The parameters to the transfer. See GenericTransferInfo.
-    // |callback|: Invoked once the transfer has completed.
+    // |handle|: An open connection to the device.
+    // |transferInfo|: The transfer parameters.
     static void interruptTransfer(ConnectionHandle handle,
                                   GenericTransferInfo transferInfo,
                                   TransferCallback callback);
 
     // Performs an isochronous transfer on the specific device.
-    // |handle|: A connection handle to make the transfer on.
-    // |transferInfo|: The parameters to the transfer. See
-    // IsochronousTransferInfo.
-    // |callback|: Invoked once the transfer has been completed.
+    // |handle|: An open connection to the device.
     static void isochronousTransfer(ConnectionHandle handle,
                                     IsochronousTransferInfo transferInfo,
                                     TransferCallback callback);
 
-    // Tries to reset the USB device and restores it to the previous status.
+    // Tries to reset the USB device.
     // If the reset fails, the given connection handle will be closed and the 
     // USB device will appear to be disconnected then reconnected. 
-    // In that case you must call |getDevices| or |findDevices| again to acquire
-    // the device.
+    // In this case $(ref:getDevices) or $(ref:findDevices) must be called again
+    // to acquire the device.
     //
     // |handle|: A connection handle to reset.
-    // |callback|: Invoked once the device is reset with a boolean indicating
-    // whether the reset is completed successfully.
     static void resetDevice(ConnectionHandle handle,
                             ResetDeviceCallback callback);
   };

extensions/common/api/usb_private.idl

@@ -7,22 +7,6 @@
 // API which should only be available to trusted pages.
 namespace usbPrivate {
 
-  // Properties for matching devices. A device matches of any of its interfaces
-  // match the given properties. An empty dictionary matches any device.
-  dictionary DeviceFilter {
-    // Device-level matching criteria:
-    long? vendorId;
-    // Checked only if the vendorId matches.
-    long? productId;
-
-    // Per-interface matching criteria:
-    long? interfaceClass;
-    // Checked only if the interfaceClass matches.
-    long? interfaceSubclass;
-    // Checked only if the interfaceSubclass matches.
-    long? interfaceProtocol;
-  };
-
   dictionary DeviceInfo {
     long vendorId;  // idVendor from the device
     long productId;  // idProduct from the device
@@ -44,7 +28,7 @@ namespace usbPrivate {
     // Lists USB devices matching any of the given filters.
     // |filters|: The properties to search for on target devices.
     // |callback|: Invoked with a list of device IDs on complete.
-    static void getDevices(DeviceFilter[] filters,
+    static void getDevices(usb.DeviceFilter[] filters,
                            GetDevicesCallback callback);
 
     // Gets basic display information about a device.