Add getUserSelectedDevices to the USB extensions API.

Add getUserSelectedDevices to the USB extensions API.

chrome.usb.getUserSelectedDevices allows an app to prompt the user for
one or more USB devices for it to access. This is an alternative to
statically declaring the list of devices an app can access in the
application manifest. This is useful for device classes such as
ADB-enabled Android devices which may have a great number of vendor and
product ID pairs but all match a well-known interface protocol.

If a selected device has a serial number then the app can retain access
to it until explicitly revoked (as is the case for most Android phones)
while access to a device without a serial number is revoked when the app
is unloaded or the device is disconnected.

BUG=346953
Committed: https://crrev.com/042073be6f85951049a6a3a4f84499ab85109566
Cr-Commit-Position: refs/heads/master@{#300141}

[Reilly Grant (use Gerrit), 2014-10-15 00:17:56]

Patchset #2 (id:20001) has been deleted

[Reilly Grant (use Gerrit), 2014-10-15 00:18:18]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2014-10-15 00:19:39]

reillyg@chromium.org changed reviewers:
+ finnur@chromium.org

[Reilly Grant (use Gerrit), 2014-10-15 00:20:58]

Please take a look. This is the new API itself.

[Finnur, 2014-10-15 09:33:08]

I'm probably not the ideal candidate to review this. Is there nobody on the
extension team more familiar with the USB API?

I notice also there's been no action on flipping the flags on the Feature bug
(security, privacy, UI, etc). What's the plan here?

One question about the file picker. I assume the list of USB_ID:description in
that picker is manifest provided? If so, what is preventing me from supplying a
pair like:
<sensitiveUSBDeviceID>:"Harmless debugging device"?

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
File extensions/browser/api/usb/usb_api.cc (right):

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.cc:479: }
Nit: No braces for single-line if clauses.

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.cc:730: }
nit: No braces.

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.cc:735: }
nit: No braces.

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.cc:750: }
nit: No braces.

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
File extensions/browser/api/usb/usb_api.h (right):

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.h:121: const
std::vector<scoped_refptr<device::UsbDevice>>& devices) override;
style: s/>>/> >/

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.h:124:
std::vector<scoped_refptr<device::UsbDevice>> devices_;
style: s/>>/> >/

[Finnur, 2014-10-15 09:34:14]

> One question about the file picker.

Just noticed the other CL.

[Reilly Grant (use Gerrit), 2014-10-15 19:12:27]

On 2014/10/15 09:33:08, Finnur wrote:
> I'm probably not the ideal candidate to review this. Is there nobody on the
> extension team more familiar with the USB API?

My bad, I believed that you were in one of the more specific OWNERs files
relevant to this change.

> I notice also there's been no action on flipping the flags on the Feature bug
> (security, privacy, UI, etc). What's the plan here?

Security has already signed off on the design doc. I'll poke them again on
flipping the bit. Privacy and UI review are in progress.

> One question about the file picker. I assume the list of USB_ID:description in
> that picker is manifest provided? If so, what is preventing me from supplying
a
> pair like:
> <sensitiveUSBDeviceID>:"Harmless debugging device"?

The human-readable names come from a mapping file that's built into Chrome, or
from the device itself.

[Reilly Grant (use Gerrit), 2014-10-15 19:12:43]

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
File extensions/browser/api/usb/usb_api.cc (right):

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.cc:479: }
On 2014/10/15 09:33:08, Finnur wrote:
> Nit: No braces for single-line if clauses.

Style Guide: "In general, curly braces are not required for single-line
statements, but they are allowed if you like them[.]"

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
File extensions/browser/api/usb/usb_api.h (right):

https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.h:121: const
std::vector<scoped_refptr<device::UsbDevice>>& devices) override;
On 2014/10/15 09:33:08, Finnur wrote:
> style: s/>>/> >/

clang-format has been updated to prefer the no-spaces style.

[Finnur, 2014-10-16 09:15:23]

> Style Guide: "In general, curly braces are not required for single-line
> statements, but they are allowed if you like them[.]"

I forget if the style guide used to require it (and now it doesn't) or if we
mandated this as a project-wide Chromium style rule, but my reading of the
Chromium and the C++ style guide now is at least clear that this is allowed. I
always felt it was a losing battle anyway, given how many people are used to the
braces from other projects, including Chrome on Android and such. Oh well.

> 
>
https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
> File extensions/browser/api/usb/usb_api.h (right):
> 
>
https://codereview.chromium.org/599303004/diff/40001/extensions/browser/api/u...
> extensions/browser/api/usb/usb_api.h:121: const
> std::vector<scoped_refptr<device::UsbDevice>>& devices) override;
> On 2014/10/15 09:33:08, Finnur wrote:
> > style: s/>>/> >/
> 
> clang-format has been updated to prefer the no-spaces style.

I'm actually happy to hear that; I never liked enforcing this rule. :)

LGTM

[Reilly Grant (use Gerrit), 2014-10-16 17:32:23]

reillyg@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Reilly Grant (use Gerrit), 2014-10-16 17:32:23]

Alexei, please take a look at the extension function histogram changes.

[Alexei Svitkine (slow), 2014-10-16 17:34:14]

lgtm

[Reilly Grant (use Gerrit), 2014-10-16 19:04:16]

reillyg@chromium.org changed reviewers:
+ rockot@chromium.org

[Reilly Grant (use Gerrit), 2014-10-16 19:05:09]

Ken, please take a look at the latest diff. I've updated the API to use an
options dictionary for the parameters in order to allow for future expansion.
Also fixed a leak of the DevicePermissionsPrompt object.

[Ken Rockot(use gerrit already), 2014-10-16 19:15:23]

lgtm

https://codereview.chromium.org/599303004/diff/60001/extensions/browser/api/u...
File extensions/browser/api/usb/usb_api.h (right):

https://codereview.chromium.org/599303004/diff/60001/extensions/browser/api/u...
extensions/browser/api/usb/usb_api.h:120: virtual void UsbDevicesChosen(
optional nit: I kind of wish this interface were named OnUsbDevicesChosen (or
OnUsbDevicesSelected) if you agree and feel like changing
DevicePermissionsPrompt::Delegate.

[Reilly Grant (use Gerrit), 2014-10-17 17:52:22]

Ken, please take a look at the ExtensionsAPIClient redirection I've added to fix
the app_shell build.

[Ken Rockot(use gerrit already), 2014-10-17 18:28:37]

Nice. LGTM

[Reilly Grant (use Gerrit), 2014-10-17 18:32:15]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2014-10-17 18:33:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/599303004/140001

[commit-bot: I haz the power, 2014-10-17 19:20:52]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2014-10-17 19:22:01]

Patchset 6 (id:??) landed as
https://crrev.com/042073be6f85951049a6a3a4f84499ab85109566
Cr-Commit-Position: refs/heads/master@{#300141}