[webkit-dev] JS bindings: Adding EventTarget to the prototype chain

[Sam Weinig]

I don't think we should do this.  EventTarget is really just an abstract interface, and changing its implementation globally is of limited utility.

-Sam

On Jun 8, 2011, at 5:54 PM, Dominic Cooney wrote:

> [If you don't care about JSC or V8 bindings you can safely ignore
> this.]
> 
> TL;DR I want to change the JavaScript bindings to put EventTarget on
> the prototype chain so it is easier to work with event targets from
> JavaScript. What do you think?
> 
> Here is the prototype chain for a button today:
> 
> HTMLButtonElement-HTMLElement-Element-Node-Object
> (add/removeEventListener and dispatchEvent are on Node.)
> 
> Here is how I think we should make it look:
> 
> HTMLButtonElement-HTMLElement-Element-Node-EventTarget-Object
> (addEventListener etc. are on EventTarget.)
> 
> Here’s why I think we should do this:
> 
> - Specs are moving towards specifying EventTarget as living on the
> prototype chain. DOM Core*, Notifications, Indexed DB, SVG and XHR
> already do it this way. (* Editor’s draft.)
> 
> - Frameworks that want to hook add/removeEventListener will be able to
> do it in one place: on EventTarget.prototype. In WebKit today they
> have to hook the prototypes of window, Node, XMLHttpRequest, etc.
> individually (Because WebKit implements EventTarget as a mixin
> everywhere, there are 20+ different kinds of event targets to hook if
> you want to be comprehensive.) If we make this change, it gets easier
> to tell if a given object is an EventTarget; just do
> EventTarget.prototype.isPrototypeOf(something).
> 
> - It will modestly simplify WebKit’s IDLs and bindings. Instead of
> declaring addEventListener in two dozen places in IDLs, it will be in
> one place; instead of calling visitJSEventListeners in dozens of
> places for JSC GC, it will be called in one place; instead of assuming
> that EventTarget parameters are all Node* under the covers for V8 code
> generation, we can treat EventTargets uniformly; instead of
> redundantly specifying EventTarget on Document and Node inheritance
> will do what you want; etc.
> 
> Will doing this break the web? I don’t think so:
> 
> Anyone who calls or hooks addEventListener, etc. will continue to
> work, just their foo.addEventListener might be resolved at one level
> higher up the prototype chain than it is today. To really observe the
> different placement of addEventListener, etc. minutely you need to
> access __proto__ and hasOwnProperty. Other browsers are already differ
> from WebKit in this regard, too: For example, Firefox reports
> addEventListener is an own property on *every* step in the prototype
> chain of DOM objects (until Object.)
> 
> Scripts that squirrel up the prototype chain themselves will see one
> more link (EventTarget’s) but it is towards the top of the chain, past
> most prototypes they care about (every prototype except Object.)
> 
> I tried changing half of the EventTargets in WebKit to put EventTarget
> in the prototype chain, including Node and XHR (but not window) and
> used it to surf the web for a few days. I didn’t notice anything break
> :)
> 
> There is also the possibility that this could hurt performance,
> because accessing addEventListener, etc. will have to search more
> prototypes (usually just one more.) Accessing the properties of Object
> on an event target via the prototype chain will have to squirrel
> through one more prototype (EventTarget’s.) So I prototyped this
> change in the JSC bindings and put EventTarget in the prototype chain
> of a number of event targets in WebKit, including Node. Here are the
> results for Dromaeo’s dom and jslib tests:
> 
> <http://dromaeo.com/?id=141811,141813>
> 
> (141811 on the left is the status quo.)
> 
> I expect the Prototype and jQuery events benchmarks are of particular
> interest, and the result looks particularly bad for Prototype (~3%
> slowdown). So I reran <http://dromaeo.com/?event> half a dozen times,
> and couldn’t produce the poor result for Prototype; on average the
> prototype was 1.0% slower for Prototype and 0.5% slower for jQuery. I
> think Dromaeo is too noisy for measuring something as fine as this.
> 
> So I wrote three microbenchmarks:
> 
> 1. Add/remove click listener on a button.
> 
> 2. Add/remove progress listener on an XHR.
> 
> 3. Test property presence with 'in':
> 
> if ('dispatchEvent' in target)
>  n++;
> // return n outside of loop
> 
> where target is an XMLHttpRequest and n is a local var n = 0.
> 
> Here are the results. A brief note on methodology: release build
> running in Mac Safari, JSC, averaging 500 samples with 1,000,000
> iterations of the inner loop per sample.
> 
> 1. Add/remove on button
> Before (ms): min=409, median=434, mean=434.4, max=472
> After (ms): min=410, median=453.5, mean=452.4, max=497 (mean is 1.04x)
> 
> 2. Add/remove on XHR
> Before (ms): min=286, median=298, mean=298.6, max=315
> After (ms): min=287, median=300.5, mean=300.7, max=320 (mean is 1.01x)
> 
> 3. 'dispatchEvent' in XHR
> Before (ms): min=85, median=88, mean=87.7, max=91
> After (ms): min=89, median=91, mean=91.0, max=95 (mean is 1.04x)
> 
> So this shows that, yes, this is not free, but it is low-single
> digits. Since adding and removing event listeners is a relatively
> infrequent operation, I think this is OK. I want to emphasize that the
> change I’m proposing has no impact on native event *dispatch*, which
> is obviously a lot more performance-sensitive than adding and removing
> event listeners.
> 
> There is the question of maintenance: Making EventTarget an
> Objective-C class is a public API change, so some of the #ifdef-ing
> for Objective-C in IDLs will remain to avoid that. This change doesn’t
> need to touch EventTarget on the C++ side; the existing design works
> well with this new shape of binding.
> 
> As I mentioned above, there are a number of little clean-ups and
> simplifications this makes possible. I can tackle this incrementally,
> although unfortunately most of the clean-up is contingent on the old
> way of mixing in EventTarget being completely replaced.
> 
> What do you think?
> 
> Dominic
> 
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110609/877f8df8/attachment.html>