Update USB extension API with suggestions from review.

Update USB extension API with suggestions from review.

BUG=122984
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173433

[Garret Kelly, 2012-12-14 04:21:42]

mpcomplete, asargent: This change is based primarily on your feedback, would you
mind reviewing?

[Matt Perry, 2012-12-14 18:50:16]

lgtm

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/usb.idl (right):

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/usb.idl:9: // technical reasons, maps to
"interface".
That's... odd. We control this API, why can't we just use "interface" instead of
"iface"?

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/usb.idl:126: VoidCallback callback);
Are you planning on passing the error value to the callback as
chrome.extension.lastError?

[Garret Kelly, 2012-12-14 18:56:21]

I also need to fix the tests.

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/usb.idl (right):

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/usb.idl:9: // technical reasons, maps to
"interface".
On 2012/12/14 18:50:16, Matt Perry wrote:
> That's... odd. We control this API, why can't we just use "interface" instead
of
> "iface"?

"interface" is a reserved word in IDL.

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/usb.idl:126: VoidCallback callback);
> Are you planning on passing the error value to the callback as
> chrome.extension.lastError?

Since these operations can't ever fail I'm inclined not to.

[Matt Perry, 2012-12-14 19:05:30]

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/usb.idl (right):

https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/usb.idl:9: // technical reasons, maps to
"interface".
On 2012/12/14 18:56:21, Garret Kelly wrote:
> On 2012/12/14 18:50:16, Matt Perry wrote:
> > That's... odd. We control this API, why can't we just use "interface"
instead
> of
> > "iface"?
> 
> "interface" is a reserved word in IDL.

Oh, lame :(. I wonder if we can fix our IDL parser to allow that in an enum.

Antony, any idea?

[asargent_no_longer_on_chrome, 2012-12-14 20:29:03]

On 2012/12/14 19:05:30, Matt Perry wrote:
>
https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
> File chrome/common/extensions/api/usb.idl (right):
> 
>
https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/...
> chrome/common/extensions/api/usb.idl:9: // technical reasons, maps to
> "interface".
> On 2012/12/14 18:56:21, Garret Kelly wrote:
> > On 2012/12/14 18:50:16, Matt Perry wrote:
> > > That's... odd. We control this API, why can't we just use "interface"
> instead
> > of
> > > "iface"?
> > 
> > "interface" is a reserved word in IDL.
> 
> Oh, lame :(. I wonder if we can fix our IDL parser to allow that in an enum.
> 
> Antony, any idea?

The WebIDL spec has a solution for this problem: you begin the identifier with a
leading underscore. I can't remember if we've added support for this yet, but it
probably would not be too hard. I can take a look at it.


http://dev.w3.org/2006/webapi/WebIDL/#idl-names

"Note
A leading "_" is used to escape an identifier from looking like a reserved word
so that, for example, an interface named “interface” can be defined. The leading
"_" is dropped to unescape the identifier."

[asargent_no_longer_on_chrome, 2012-12-14 23:44:32]

Patch now up for review:

https://codereview.chromium.org/11565053/


On Fri, Dec 14, 2012 at 12:29 PM, <asargent@chromium.org> wrote:

> On 2012/12/14 19:05:30, Matt Perry wrote:
>
> https://codereview.chromium.**org/11577017/diff/1/chrome/**
>
common/extensions/api/usb.idl<https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/usb.idl>
>
>> File chrome/common/extensions/api/**usb.idl (right):
>>
>
>
> https://codereview.chromium.**org/11577017/diff/1/chrome/**
>
common/extensions/api/usb.idl#**newcode9<https://codereview.chromium.org/11577017/diff/1/chrome/common/extensions/api/usb.idl#newcode9>
>
>> chrome/common/extensions/api/**usb.idl:9: // technical reasons, maps to
>> "interface".
>> On 2012/12/14 18:56:21, Garret Kelly wrote:
>> > On 2012/12/14 18:50:16, Matt Perry wrote:
>> > > That's... odd. We control this API, why can't we just use "interface"
>> instead
>> > of
>> > > "iface"?
>> >
>> > "interface" is a reserved word in IDL.
>>
>
>  Oh, lame :(. I wonder if we can fix our IDL parser to allow that in an
>> enum.
>>
>
>  Antony, any idea?
>>
>
> The WebIDL spec has a solution for this problem: you begin the identifier
> with a
> leading underscore. I can't remember if we've added support for this yet,
> but it
> probably would not be too hard. I can take a look at it.
>
>
>
http://dev.w3.org/2006/webapi/**WebIDL/#idl-names<http://dev.w3.org/2006/weba...
>
> "Note
> A leading "_" is used to escape an identifier from looking like a reserved
> word
> so that, for example, an interface named “interface” can be defined. The
> leading
> "_" is dropped to unescape the identifier."
>
>
>
>
https://codereview.chromium.**org/11577017/<https://codereview.chromium.org/1...
>

[commit-bot: I haz the power, 2012-12-17 05:15:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gdk@chromium.org/11577017/12001

[commit-bot: I haz the power, 2012-12-17 09:25:07]

Change committed as 173433