IDL: Support iterable<>, maplike<> and setlike<> syntax

IDL: Support iterable<>, maplike<> and setlike<> syntax

This adds support for the basic syntax (in idl_definitions.py) but no
handling of the additional parsed information beyond what the [Iterable]
extended attribute already does. Extended attributes are currently not
supported on the iterable<>, maplike<> or setlike<> definitions.

In existing interfaces, replace [Iterable] with what the specification
has, or in the case of MIDIInputMap and MIDIOutputMap, what seems to be
the intention. In the Iterator interface, [Iterable] is still used,
since it's somewhat unclear what it ought to be replaced with.

This results in no changes in generated code.

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

[Jens Widell, 2015-01-08 11:44:37]

jl@opera.com changed reviewers:
+ haraken@chromium.org, jsbell@chromium.org, yhirano@chromium.org

[Jens Widell, 2015-01-08 11:44:38]

haraken@, yhirano@: PTAL
jsbell@: FYI

[haraken, 2015-01-08 15:52:10]

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

[haraken, 2015-01-08 15:52:11]

bashi-san: Would you take a first look on this?

[jsbell, 2015-01-08 17:02:40]

Thanks for tackling this!

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:139:
iterator_operation.extended_attributes['RaisesException'] = None
Will it be possible (not in this CL, but w/o significant rework) to support
blink-specific extended attributes on iterable<> (etc) annotations, e.g.
[RuntimeEnabled=...], that get applied to the
iterator_operation.extended_attributes as well?

[Jens Widell, 2015-01-08 17:12:04]

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:139:
iterator_operation.extended_attributes['RaisesException'] = None
On 2015/01/08 17:02:40, jsbell wrote:
> Will it be possible (not in this CL, but w/o significant rework) to support
> blink-specific extended attributes on iterable<> (etc) annotations, e.g.
> [RuntimeEnabled=...], that get applied to the
> iterator_operation.extended_attributes as well?

Supporting extended attributes, and simply transferring them to the iterator
operation and the other implied operations (keys(), values(), et al, once we
support them) wouldn't be hard.

A potentially trickier problem to solve would be to support applying extended
attributes to only one of the implied operations. For instance supporting
[ImplementedAs=...] to override the individual C++ method names.

(I guess I shouldn't really say "full support" in the description, since I
actually don't support extended attributes on these definitions. Or,
alternatively, I should add support for extended attributes on these defitions.)

[jsbell, 2015-01-08 18:24:50]

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:139:
iterator_operation.extended_attributes['RaisesException'] = None
On 2015/01/08 17:12:04, Jens Widell wrote:
> A potentially trickier problem to solve would be to support applying extended
> attributes to only one of the implied operations. For instance supporting
> [ImplementedAs=...] to override the individual C++ method names.

Noted. 

FWIW, applying the same attribs to all of the implied operations is fine for the
scenario I'm thinking about - experimentally adding iterable<> to existing
interfaces like FormData.

[bashi, 2015-01-08 23:46:15]

bashi@chromium.org changed reviewers:
+ toyoshim@chromium.org

[bashi, 2015-01-08 23:46:15]

LGTM, but I'd like to see yhirano@'s approval.

toyoshim@ - FYI

https://codereview.chromium.org/841973002/diff/1/Source/bindings/tests/idls/c...
File Source/bindings/tests/idls/core/TestInterface.idl (left):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/tests/idls/c...
Source/bindings/tests/idls/core/TestInterface.idl:40: Iterable,
Shouldn't we keep having a test for "Iterable" extended attribute? We still
support it.

[haraken, 2015-01-09 02:01:00]

LGTM

[yhirano, 2015-01-09 02:30:01]

lgtm

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
File Source/bindings/scripts/idl_definitions.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
Source/bindings/scripts/idl_definitions.py:671: self.is_read_only =
node.GetProperty('READONLY') or False
bool(node.GetProperty('READONLY'))

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
Source/bindings/scripts/idl_definitions.py:684: self.is_read_only =
node.GetProperty('READONLY') or False
ditto

[yhirano, 2015-01-09 02:31:06]

And, thank you for working on this!

[Jens Widell, 2015-01-09 07:15:04]

Thanks for reviews, all!

PS#2 addresses comments, and adds FIXME comments about lacking extended
attribute support (which I'll fix in a follow-up).

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
File Source/bindings/scripts/idl_definitions.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
Source/bindings/scripts/idl_definitions.py:671: self.is_read_only =
node.GetProperty('READONLY') or False
On 2015/01/09 02:30:01, yhirano wrote:
> bool(node.GetProperty('READONLY'))

Done.

I had simply copied this from IdlAttribute, and it matched how all other boolean
properties were accessed, but since I agree bool() is better, I changed this and
all other boolean property accesses to use bool().

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/idl_...
Source/bindings/scripts/idl_definitions.py:684: self.is_read_only =
node.GetProperty('READONLY') or False
On 2015/01/09 02:30:01, yhirano wrote:
> ditto

Done.

https://codereview.chromium.org/841973002/diff/1/Source/bindings/tests/idls/c...
File Source/bindings/tests/idls/core/TestInterface.idl (left):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/tests/idls/c...
Source/bindings/tests/idls/core/TestInterface.idl:40: Iterable,
On 2015/01/08 23:46:15, bashi1 wrote:
> Shouldn't we keep having a test for "Iterable" extended attribute? We still
> support it.

Done.

I've reverted the change to this file, and instead added iterable<any> to
TestInterface3.

[Jens Widell, 2015-01-09 10:10:42]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2015-01-09 10:11:40]

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

[commit-bot: I haz the power, 2015-01-09 10:15:31]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188135

[Jens Widell, 2015-01-13 10:26:26]

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/841973002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:139:
iterator_operation.extended_attributes['RaisesException'] = None
On 2015/01/08 18:24:49, jsbell wrote:
> On 2015/01/08 17:12:04, Jens Widell wrote:
> > A potentially trickier problem to solve would be to support applying
extended
> > attributes to only one of the implied operations. For instance supporting
> > [ImplementedAs=...] to override the individual C++ method names.
> 
> Noted. 
> 
> FWIW, applying the same attribs to all of the implied operations is fine for
the
> scenario I'm thinking about - experimentally adding iterable<> to existing
> interfaces like FormData.

Having thought some more about this, one IMO rather agreeable way to support
extended attributes (and perhaps other modifications) on individual implied
methods would be to allow the IDL file to explicitly define some or all of the
implied methods (checking only that they are correct in terms of arguments and
return type) and if so let the explicitly defined methods win.

WebIDL doesn't allow that, but that's not really something we need to follow in
our internal IDL files if we don't want to, as long as the observable behavior
is the intended.

IOW, you could for instance have

  interface FormData {
    [RuntimeEnabled=...] Iterator values(); // not web compatible
    iterable<...>;
  };

We wouldn't need to implement this unless an actual use-case emerges, of course.