IDL: Support extended attributes on iterable<>, maplike<> and setlike<>

IDL: Support extended attributes on iterable<>, maplike<> and setlike<>

Such extended attributes are copied over to and affect the methods that
are automatically added by those definitions, e.g. keys() and values().

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

[Jens Widell, 2015-01-14 11:13:17]

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

[Jens Widell, 2015-01-14 11:13:17]

PTAL

[haraken, 2015-01-14 11:38:47]

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

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:345: use_extended_attributes = {}

used_extended_attributes ?

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:349: elif interface.maplike:

elif => if ?

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:351: elif interface.setlike:

elif => if ?

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:357: use_extended_attributes['CallWith']
= 'ScriptState'

This rule looks a bit hacky. Do we need to support other than ScriptState?
(ScriptState is the most general and modern (?) way to pass script information
to the Blink side.)

https://codereview.chromium.org/792903004/diff/1/Source/bindings/templates/in...
File Source/bindings/templates/interface_base.cpp (right):

https://codereview.chromium.org/792903004/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface_base.cpp:382: {% filter
conditional(iterator_method.conditional_string) %}

If we don't have any use case for [Conditional] on iterable<>, we might want to
drop this. [Conditional] is deprecated and we should remove it rather than
supporting it in a new feature.

[Jens Widell, 2015-01-14 11:50:45]

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

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:345: use_extended_attributes = {}
On 2015/01/14 11:38:47, haraken wrote:
> 
> used_extended_attributes ?

Works too. :-)

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:349: elif interface.maplike:
On 2015/01/14 11:38:47, haraken wrote:
> 
> elif => if ?

Wouldn't change behavior, since there can only be one of iterable, maplike and
setlike. It would give the impression that there could be, though, so I think I
prefer elif here.

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:357: use_extended_attributes['CallWith']
= 'ScriptState'
On 2015/01/14 11:38:47, haraken wrote:
> 
> This rule looks a bit hacky. Do we need to support other than ScriptState?
> (ScriptState is the most general and modern (?) way to pass script information
> to the Blink side.)

We don't need it right now, for sure, since there's hardly any code that uses
this. And I suppose there's no compelling reason to support it at all given that
ScriptState covers all the other candidates rather well.

I could change this code to just check that RaisesException and CallWith aren't
included in the extended attributes (since they have fixed values) and then
unconditionally override them.

[Jens Widell, 2015-01-14 12:00:02]

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

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:345: use_extended_attributes = {}
On 2015/01/14 11:50:45, Jens Widell wrote:
> On 2015/01/14 11:38:47, haraken wrote:
> > 
> > used_extended_attributes ?
> 
> Works too. :-)

Done.

https://codereview.chromium.org/792903004/diff/1/Source/bindings/templates/in...
File Source/bindings/templates/interface_base.cpp (right):

https://codereview.chromium.org/792903004/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface_base.cpp:382: {% filter
conditional(iterator_method.conditional_string) %}
On 2015/01/14 11:38:47, haraken wrote:
> 
> If we don't have any use case for [Conditional] on iterable<>, we might want
to
> drop this. [Conditional] is deprecated and we should remove it rather than
> supporting it in a new feature.

Done.

[haraken, 2015-01-14 12:53:33]

LGTM

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

https://codereview.chromium.org/792903004/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:349: elif interface.maplike:
On 2015/01/14 11:50:45, Jens Widell wrote:
> On 2015/01/14 11:38:47, haraken wrote:
> > 
> > elif => if ?
> 
> Wouldn't change behavior, since there can only be one of iterable, maplike and
> setlike. It would give the impression that there could be, though, so I think
I
> prefer elif here.

Makes sense.

[Jens Widell, 2015-01-14 13:16:26]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2015-01-14 13:17:30]

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

[commit-bot: I haz the power, 2015-01-14 15:47:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-14 15:47:15]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/4...)

[Jens Widell, 2015-01-14 15:48:00]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2015-01-14 15:48:29]

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

[commit-bot: I haz the power, 2015-01-14 18:36:59]

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