Summary: | Autogenerate JS event listeners | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||||
Component: | WebCore JavaScript | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, zimmermann | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Sam Weinig
2008-09-25 15:29:37 PDT
Created attachment 23932 [details]
patch 1 - generate EventTargetNode
Comment on attachment 23932 [details]
patch 1 - generate EventTargetNode
First patch landed in 37090.
Created attachment 23960 [details]
patch 2 - standardize EventListeners
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!
(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. Comment on attachment 23960 [details] patch 2 - standardize EventListeners Second patch landed in r37128. Created attachment 23968 [details]
Patch 3 - Generation
Comment on attachment 23968 [details]
Patch 3 - Generation
Yay! Another excellent patch in the right direction, r+ r+ r+!
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.
Last patch landed in r37142. |