Add WebUSB bindings and client interface [part 2]

Add WebUSB bindings and client interface [part 2]

This is part 2 of a multipart change set to add WebUSB
bindings to Blink. In this CL, device enumeration logic
is implemented and some additional getters are added
to the USBDevice interface.

The original full CL is here:
https://codereview.chromium.org/1245363002/

The WebUSB draft spec is here:
http://reillyeon.github.io/webusb/

Part 1 landed as
https://src.chromium.org/viewvc/blink?revision=199777&view=revision

BUG=492204
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199921

[Ken Rockot(use gerrit already), 2015-07-31 03:05:16]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2015-07-31 03:06:39]

Patchset #1 (id:20001) has been deleted

[Ken Rockot(use gerrit already), 2015-07-31 03:12:20]

Patchset #1 (id:40001) has been deleted

[Ken Rockot(use gerrit already), 2015-07-31 03:13:17]

rockot@chromium.org changed reviewers:
+ dcheng@chromium.org, haraken@chromium.org

[Ken Rockot(use gerrit already), 2015-07-31 03:13:17]

haraken@ please review Source/
dcheng@ please review public/

Thank you!

[haraken, 2015-07-31 08:58:24]

haraken@chromium.org changed reviewers:
+ yhirano@chromium.org

[haraken, 2015-07-31 08:58:25]

Source/ LGTM

I have to have yhirano@ to take a look at the CallbackPromiseAdapter part.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
File Source/modules/webusb/USB.cpp (right):

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USB.cpp:56: WTF_MAKE_NONCOPYABLE(DeviceArray);

You can use STATIC_ONLY().

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USB.cpp:69: DeviceArray() = delete;

Then you can remove this.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
File Source/modules/webusb/USBError.h (right):

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USBError.h:20: WTF_MAKE_NONCOPYABLE(USBError);

Can we use STATIC_ONLY()?

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USBError.h:27: USBError() = delete;

Then we can remove this.

[yhirano, 2015-07-31 09:41:58]

Looks good, but if you can wait for a while, you will be able to use cleaner
interface.

Please see
https://codereview.chromium.org/1234603003/
https://codereview.chromium.org/1235083006/
https://codereview.chromium.org/1240763002/

[Ken Rockot(use gerrit already), 2015-07-31 16:19:26]

Thanks. It does look like a cleaner interface, but it would be very unfortunate
for us to block all of our work for over a week on those changes.

For now how about I instead refrain from using CallbackPromiseAdapter (done).
There are several follow-up CLs that add getter-only interfaces, so we can
parallelize our work while waiting for your CPA changes to land.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
File Source/modules/webusb/USB.cpp (right):

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USB.cpp:56: WTF_MAKE_NONCOPYABLE(DeviceArray);
On 2015/07/31 08:58:25, haraken wrote:
> 
> You can use STATIC_ONLY().

Done.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USB.cpp:69: DeviceArray() = delete;
On 2015/07/31 08:58:25, haraken wrote:
> 
> Then you can remove this.

Done.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
File Source/modules/webusb/USBError.h (right):

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USBError.h:20: WTF_MAKE_NONCOPYABLE(USBError);
On 2015/07/31 08:58:25, haraken wrote:
> 
> Can we use STATIC_ONLY()?
> 
> 

Done.

https://codereview.chromium.org/1264483005/diff/60001/Source/modules/webusb/U...
Source/modules/webusb/USBError.h:27: USBError() = delete;
On 2015/07/31 08:58:25, haraken wrote:
> 
> Then we can remove this.

Done.

[dcheng, 2015-07-31 18:58:25]

Just one quick thought (I'm OOO for most of the rest of today, but I'll take a
look in more depth tonight)

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
File public/platform/modules/webusb/WebUSBDevice.h (right):

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:31: enum TransferDirection {
enum class

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:36: enum RequestType {
enum class

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:37: RequestTypeStandard,
Then you can just call this "Standard", "Class", "Vendor"

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:42: enum RequestRecipient {
enum class

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
File public/platform/modules/webusb/WebUSBError.h (right):

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBError.h:15: enum ErrorType {
enum class here as well. You can probably just name this:

enum class Error {
    Device,
    Security,
    Service,
    Transfer,
};

[Ken Rockot(use gerrit already), 2015-07-31 19:14:28]

Thanks

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
File public/platform/modules/webusb/WebUSBDevice.h (right):

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:31: enum TransferDirection {
On 2015/07/31 18:58:25, dcheng wrote:
> enum class

Done.

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:36: enum RequestType {
On 2015/07/31 18:58:24, dcheng wrote:
> enum class

Done.

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:37: RequestTypeStandard,
On 2015/07/31 18:58:24, dcheng wrote:
> Then you can just call this "Standard", "Class", "Vendor"

Done.

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:42: enum RequestRecipient {
On 2015/07/31 18:58:24, dcheng wrote:
> enum class

Done.

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
File public/platform/modules/webusb/WebUSBError.h (right):

https://codereview.chromium.org/1264483005/diff/100001/public/platform/module...
public/platform/modules/webusb/WebUSBError.h:15: enum ErrorType {
On 2015/07/31 18:58:25, dcheng wrote:
> enum class here as well. You can probably just name this:
> 
> enum class Error {
>     Device,
>     Security,
>     Service,
>     Transfer,
> };

Done.

[yhirano, 2015-08-03 05:28:52]

> For now how about I instead refrain from using CallbackPromiseAdapter (done).
> There are several follow-up CLs that add getter-only interfaces, so we can
> parallelize our work while waiting for your CPA changes to land.

Thank you.

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
File public/platform/modules/webusb/WebUSBDevice.h (right):

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:17: using
WebUSBDeviceOpenCallbacks = WebCallbacks<void, WebUSBError*>;
const WebUSBError&

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:25: using
WebUSBDeviceControlTransferCallbacks = WebCallbacks<WebUSBTransferInfo*,
WebUSBError*>;
Is WebUSBTransferInfo defined anywhere?

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:25: using
WebUSBDeviceControlTransferCallbacks = WebCallbacks<WebUSBTransferInfo*,
WebUSBError*>;
Please consider using |WebPassOwnPtr<WebTransferInfo>| or |const
WebTransferInfo&|.

[Ken Rockot(use gerrit already), 2015-08-03 21:49:44]

thanks - comments addresed

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
File public/platform/modules/webusb/WebUSBDevice.h (right):

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:17: using
WebUSBDeviceOpenCallbacks = WebCallbacks<void, WebUSBError*>;
On 2015/08/03 05:28:51, yhirano wrote:
> const WebUSBError&

Done.

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:25: using
WebUSBDeviceControlTransferCallbacks = WebCallbacks<WebUSBTransferInfo*,
WebUSBError*>;
On 2015/08/03 05:28:51, yhirano wrote:
> Please consider using |WebPassOwnPtr<WebTransferInfo>| or |const
> WebTransferInfo&|.

Done.

https://codereview.chromium.org/1264483005/diff/120001/public/platform/module...
public/platform/modules/webusb/WebUSBDevice.h:25: using
WebUSBDeviceControlTransferCallbacks = WebCallbacks<WebUSBTransferInfo*,
WebUSBError*>;
On 2015/08/03 05:28:52, yhirano wrote:
> Is WebUSBTransferInfo defined anywhere?

Oops - Added the header to this CL.

[dcheng, 2015-08-03 21:59:18]

lgtm

FWIW, I don't think you technically need my OWNERS stamp anymore, since you
added one in your previous patch for public/platform/modules/webusb.

https://codereview.chromium.org/1264483005/diff/140001/public/platform/module...
File public/platform/modules/webusb/WebUSBTransferInfo.h (right):

https://codereview.chromium.org/1264483005/diff/140001/public/platform/module...
public/platform/modules/webusb/WebUSBTransferInfo.h:13: enum Status {
enum class here for consistency

[Ken Rockot(use gerrit already), 2015-08-03 22:01:16]

https://codereview.chromium.org/1264483005/diff/140001/public/platform/module...
File public/platform/modules/webusb/WebUSBTransferInfo.h (right):

https://codereview.chromium.org/1264483005/diff/140001/public/platform/module...
public/platform/modules/webusb/WebUSBTransferInfo.h:13: enum Status {
On 2015/08/03 21:59:18, dcheng wrote:
> enum class here for consistency

Oops! Done.

[Ken Rockot(use gerrit already), 2015-08-03 22:01:25]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2015-08-03 22:01:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1264483005/#ps160001
(title: "enum class")

[commit-bot: I haz the power, 2015-08-03 22:01:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1264483005/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1264483005/160001

[commit-bot: I haz the power, 2015-08-03 23:16:02]

Committed patchset #6 (id:160001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199921

[ortuno, 2015-10-29 01:51:01]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org

[ortuno, 2015-10-29 01:51:02]

https://codereview.chromium.org/1264483005/diff/100001/Source/modules/webusb/...
File Source/modules/webusb/USB.cpp (right):

https://codereview.chromium.org/1264483005/diff/100001/Source/modules/webusb/...
Source/modules/webusb/USB.cpp:56: WTF_MAKE_NONCOPYABLE(DeviceArray);
question: why did this end up not being STATIC_ONLY? same for USBError.

[haraken, 2015-10-29 04:24:57]

https://codereview.chromium.org/1264483005/diff/100001/Source/modules/webusb/...
File Source/modules/webusb/USB.cpp (right):

https://codereview.chromium.org/1264483005/diff/100001/Source/modules/webusb/...
Source/modules/webusb/USB.cpp:56: WTF_MAKE_NONCOPYABLE(DeviceArray);
On 2015/10/29 01:51:02, ortuno wrote:
> question: why did this end up not being STATIC_ONLY? same for USBError.

This should be STATIC_ONLY. And should remove the below "DeviceArray() =
delete;".

We should have a clang plugin support to detect this error...