Replace custom EventTarget toNative call with [ToNativeArg] extended attribute

Replace custom EventTarget toNative call with [ToNativeArg] extended attribute

EventTarget has custom bindings code for the V8 -> C++ conversion.
This is just to handle which argument to pass to
V8EventListenerList::getEventListener
|ListenerFindOrCreate| or |ListenerFindOnly|.
This replaces the custom bindings with an extended attribute,
[ToNativeArg], which specifies exactly such an argument.

This fixes the main remaining ugliness in EventTarget bindings!

Documentation:
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-ToNativeArg-p-

Followup to:
Simplify EventTarget bindings generation (2)
https://codereview.chromium.org/203143002/

R=haraken
BUG=345503

[Nils Barth (inactive), 2014-03-19 04:31:14]

Other cleanup.

[haraken, 2014-03-19 04:44:17]

Is [ToNativeArg] going to be used in
EventTarget::addEventListener/removeEventListener? If so, I'm not really happy
to add a new IDL attribute.

How about generating different methods for addEventListener and
removeEventListener? For example, V8EventListenerList::ensureEventListener() for
addEventListener() and V8EventListenerList::existingEventListener() for
removeEventListener()?

https://codereview.chromium.org/204213002/diff/1/Source/bindings/tests/result...
File Source/bindings/tests/results/V8TestObjectPython.cpp (right):

https://codereview.chromium.org/204213002/diff/1/Source/bindings/tests/result...
Source/bindings/tests/results/V8TestObjectPython.cpp:6437: static void
voidMethodToNativeArgEventListenerArgMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)

I cannot find any ListenerFindOrCreate/ListenerFindOnly nor
V8EventListenerList::getEventListener() in this generated code, but why?

[Nils Barth (inactive), 2014-03-19 06:12:48]

Turns out there's a subtler problem:
EventListener is a callback interface,
but it has static bindings, so the IDL file was missing
from the GYP dependencies.
This was picked up by r-b-t, but not the full build.

This means I need to rework this; will get back to you on this.

https://codereview.chromium.org/204213002/diff/1/Source/bindings/tests/result...
File Source/bindings/tests/results/V8TestObjectPython.cpp (right):

https://codereview.chromium.org/204213002/diff/1/Source/bindings/tests/result...
Source/bindings/tests/results/V8TestObjectPython.cpp:6437: static void
voidMethodToNativeArgEventListenerArgMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
On 2014/03/19 04:44:17, haraken wrote:
> I cannot find any ListenerFindOrCreate/ListenerFindOnly nor
> V8EventListenerList::getEventListener() in this generated code, but why?

Ah, good catch!

I looked into this, and it's a subtle problem:
We didn't build EventListener bindings, so the actual build
wasn't detecting that this was a callback interface,
but the test cases did (correctly) detect that it was a callback interface,
so these took different code paths.

[Nils Barth (inactive), 2014-03-19 06:16:27]

On 2014/03/19 04:44:17, haraken wrote:
> Is [ToNativeArg] going to be used in
> EventTarget::addEventListener/removeEventListener?
> If so, I'm not really happy to add a new IDL attribute.

I'm reworking this CL, since it's actually the *callback* interface code path
that I need to modify,
but yes, I proposed adding an extended attribute only for these two methods.

I *think* we can make it simpler; I'll see.

> How about generating different methods for addEventListener and
> removeEventListener? For example, V8EventListenerList::ensureEventListener()
for
> addEventListener() and V8EventListenerList::existingEventListener() for
> removeEventListener()?

This wouldn't work.
The problem is that we need to override the *type handling* code
(convert EventListener from JS to C++), which only depends on the *type*
(and whether it's being used as an argument), but otherwise doesn't
depend on the *attribute* or *method*.
(That's why the other template code was so disgusting.)

However, since it's actually different code (callback interface),
I don't think we need to modify the type code at all.

[Nils Barth (inactive), 2014-03-19 09:33:17]

On 2014/03/19 04:44:17, haraken wrote:
> How about generating different methods for addEventListener and
> removeEventListener? For example, V8EventListenerList::ensureEventListener()
for
> addEventListener() and V8EventListenerList::existingEventListener() for
> removeEventListener()?

I think we can *partly* do this,
and I don't think we'll need a new extended attribute!

We still need to special-case removeEventListener somehow.
The problem is that the initializer for the local variable for callback
interfaces usually
*creates* one.

Thus we can remove the special case for addEventListener by adding a new method,
V8EventListener::create
so that the usual call:
V8EventListener::create(v8::Handle<v8::Function>::Cast(info[1]),
currentExecutionContext(info.GetIsolate())
goes through and calls:
V8EventListenerList::getEventListener(info[1], false, ListenerFindOrCreate);
(note the 'OrCreate')

However, for removeEventListener we'll need some way to have it call some other
function, call it
V8EventListener::find
...which calls:
V8EventListenerList::getEventListener(info[1], false, ListenerFindOnly);
(note the 'FindOnly').

I'll see if we can make this simpler.

[haraken, 2014-03-19 09:36:35]

I've not looked at details, but I agree that the current EventListener code is
not well organized. It's worth doing some clean-up.