Generate eventListenerCacheIndex if an EventListener is present

Generate eventListenerCacheIndex if an EventListener is present

Update the bindings generator to generate the eventListenerCacheIndex
internal property if the IDL interface has an EventListener attribute.
This is one step towards removing the [EventTarget] extended
attribute.

This change was made for the Performance interface which is currently
an exception: it has an EventListener attribute but does not inherit
EventTarget according to its specification:
https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/ResourceTiming/Overview.html#extensions-performance-interface
http://www.w3.org/TR/navigation-timing/#performance

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

[haraken, 2013-07-09 12:01:23]

> This change was made for the Performance interface which is currently 
> an exception: it has an EventListener attribute but does not inherit 
> EventTarget according to its specification:

Wouldn't this be a bug of the spec?

[do-not-use, 2013-07-09 12:14:36]

On 2013/07/09 12:01:23, haraken wrote:
> > This change was made for the Performance interface which is currently 
> > an exception: it has an EventListener attribute but does not inherit 
> > EventTarget according to its specification:
> 
> Wouldn't this be a bug of the spec?

I'm not sure, maybe arv would know? :)

The spec uses a Function object for that attribute and not an EventListener. In
practice, the only difference is that the EventTarget API is not exposed to
JavaScript side. However, it would still be usable by assigning your callback to
this attribute. I don't know if this is done on purpose or if the spec hasn't
not updated yet.

[arv (Not doing code reviews), 2013-07-09 13:40:01]

This looks like a bug in our implementation. It should be a Function and
not an EventListener. This should be observable since EventListeners get an
Event object as the argument.

window.performance.onwebkitresourcetimingbufferfull = function(e) {
  assert(e === undefined);
};


On Tue, Jul 9, 2013 at 8:14 AM, <ch.dumez@sisa.samsung.com> wrote:

> On 2013/07/09 12:01:23, haraken wrote:
>
>> > This change was made for the Performance interface which is currently
>> > an exception: it has an EventListener attribute but does not inherit
>> > EventTarget according to its specification:
>>
>
>  Wouldn't this be a bug of the spec?
>>
>
> I'm not sure, maybe arv would know? :)
>
> The spec uses a Function object for that attribute and not an
> EventListener. In
> practice, the only difference is that the EventTarget API is not exposed to
> JavaScript side. However, it would still be usable by assigning your
> callback to
> this attribute. I don't know if this is done on purpose or if the spec
> hasn't
> not updated yet.
>
>
https://codereview.chromium.**org/18897002/<https://codereview.chromium.org/1...
>



-- 
erik

[do-not-use, 2013-07-09 14:02:33]

On 2013/07/09 13:40:01, arv wrote:
> This looks like a bug in our implementation. It should be a Function and
> not an EventListener. This should be observable since EventListeners get an
> Event object as the argument.
> 
> window.performance.onwebkitresourcetimingbufferfull = function(e) {
>   assert(e === undefined);
> };
> 
> 
> On Tue, Jul 9, 2013 at 8:14 AM, <mailto:ch.dumez@sisa.samsung.com> wrote:
> 
> > On 2013/07/09 12:01:23, haraken wrote:
> >
> >> > This change was made for the Performance interface which is currently
> >> > an exception: it has an EventListener attribute but does not inherit
> >> > EventTarget according to its specification:
> >>
> >
> >  Wouldn't this be a bug of the spec?
> >>
> >
> > I'm not sure, maybe arv would know? :)
> >
> > The spec uses a Function object for that attribute and not an
> > EventListener. In
> > practice, the only difference is that the EventTarget API is not exposed to
> > JavaScript side. However, it would still be usable by assigning your
> > callback to
> > this attribute. I don't know if this is done on purpose or if the spec
> > hasn't
> > not updated yet.
> >
> >
>
https://codereview.chromium.**org/18897002/%3Chttps://codereview.chromium.org...>
> >
> 
> 
> 
> -- 
> erik

The 'Function' type is not supported by our bindings generator.

[arv (Not doing code reviews), 2013-07-09 14:09:36]

On Tue, Jul 9, 2013 at 10:02 AM, <ch.dumez@sisa.samsung.com> wrote:

>
> The 'Function' type is not supported by our bindings generator.
>

Filed https://code.google.com/p/chromium/issues/detail?id=258452

-- 
erik

[arv (Not doing code reviews), 2013-07-09 14:10:49]

LGTM so that this does not block us from removing [EventTarget].

[do-not-use, 2013-07-09 18:00:42]

On 2013/07/09 14:10:49, arv wrote:
> LGTM so that this does not block us from removing [EventTarget].

I tried to fix this properly in:
https://codereview.chromium.org/18916003/

Let's see if the approach is the other issue correct, in which case we could
drop this CL.

[do-not-use, 2013-07-10 06:56:03]

On 2013/07/09 18:00:42, Christophe Dumez wrote:
> On 2013/07/09 14:10:49, arv wrote:
> > LGTM so that this does not block us from removing [EventTarget].
> 
> I tried to fix this properly in:
> https://codereview.chromium.org/18916003/
> 
> Let's see if the approach is the other issue correct, in which case we could
> drop this CL.

It seems my other CL is unlikely to land soon as the patch needs refactoring and
there is still discussion whether it is the spec that should be updated or not.

I would like land this patch for now to unlock my work as the change is minor
and can easily be removed later on when the IDL gets fixed.
Haraken, are you OK with this?

[haraken, 2013-07-10 07:05:48]

> Haraken, are you OK with this?

Sure!

[do-not-use, 2013-07-10 07:18:01]

On 2013/07/10 07:05:48, haraken wrote:
> > Haraken, are you OK with this?
> 
> Sure!

Then could you please LGTM? I need it for the bindings part.

[haraken, 2013-07-10 07:18:24]

LGTM

[commit-bot: I haz the power, 2013-07-10 07:23:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/18897002/1

[commit-bot: I haz the power, 2013-07-10 09:01:15]

Change committed as 153872