Bug 21122 – Autogenerate JS event listeners

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Sam Weinig]

A big chuck of the remain code to be autogenerated is the event listener code (ie. window.onload and the like).  Getting it generated would be a great move in the right direction

[Sam Weinig]

Created attachment 23932 [details]
patch 1 - generate EventTargetNode

[Sam Weinig]

Comment on attachment 23932 [details]
patch 1 - generate EventTargetNode

First patch landed in 37090.

[Sam Weinig]

Created attachment 23960 [details]
patch 2 - standardize EventListeners

[Eric Seidel (no email)]

Comment on attachment 23960 [details]
patch 2 - standardize EventListeners

yay!  yay!  yay!

+    if (JSUnprotectedEventListener* listener = static_cast<JSUnprotectedEventListener*>(m_impl->onchecking()))
+        listener->mark();
Should be a static inline function, IMO
markListener(m_impl->onchecking());
Seems such a static-inline probably should be in some JSBinding.h type header since it could be used all over the place.  Then we could later add a type-check in a single place to handle the obj-c bad static cast check.

If there is a way to set any of these via obj-c, it seems all of this code would crash due to bad static_casts. :(

Can a frame ever not have a window object?  Even say, during destruction?  Cause your code seems to assume that toJSDOMWindow() can never return NULL (which may very well be the case).

It seems also given how common:
toJSDOMWindow(frame)->findOrCreateJSUnprotectedEventListener(exec, value, true)
is we might consider making that a static inline function in some JSBinding.h-like header.
findOrCreateJSUnprotectedEventLisener(frame, exec, value) (I guess that only really saves toJSDOMWindow(frame)-> and , true from typing... 

In terms of solving hte "obj-c listeners cause crash case) one could very easily add a toUnprotectedEventListener() function which did the conversion and was used by all the above proposed functions, as well as any case in this existing code where one needs to do that cast.  

Otherwise looks great!

[Darin Adler]

(In reply to comment #4)
> Seems such a static-inline probably should be in some JSBinding.h type header

As I discussed with Sam the other day. If we're putting an inline in a header it should not be marked static. The static gives it internal linkage which is not what we want for inline functions in a header.

[Sam Weinig]

Comment on attachment 23960 [details]
patch 2 - standardize EventListeners

Second patch landed in r37128.

[Sam Weinig]

Created attachment 23968 [details]
Patch 3 - Generation

[Nikolas Zimmermann]

Comment on attachment 23968 [details]
Patch 3 - Generation 

Yay! Another excellent patch in the right direction, r+ r+ r+!

[Alexey Proskuryakov]

Comment on attachment 23968 [details]
Patch 3 - Generation 

r=me

Please don't auto-generate MessagePort listeners yet - they will need to learn how to work when there is no frame or window soon.

XMLHttpRequest will also need to work in worker threads, but that will happen much later, so I think that it's good to auto-generate it now.

[Sam Weinig]

Last patch landed in r37142.