[webkit-reviews] review requested: [Bug 77835] [GObject bindings] Missing scope annotations for add/remove_event_listener : [Attachment 131002] Implements EventListeners by leveraging existing CodeGenerator

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 9 00:46:16 PST 2012


C Anthony <anthony at xtfx.me> has asked  for review:
Bug 77835: [GObject bindings] Missing scope annotations for
add/remove_event_listener
https://bugs.webkit.org/show_bug.cgi?id=77835

Attachment 131002: Implements EventListeners by leveraging existing
CodeGenerator
https://bugs.webkit.org/attachment.cgi?id=131002&action=review

------- Additional Comments from C Anthony <anthony at xtfx.me>
^^^^^ by "soon" i meant 3 weeks of "carefully crafting with love [and tears]".

(apologies if somewhat terse ... 2am, want slee... will elaborate as needed
later)

this patch implements WebKitEventListener via the existing
GObjectCodeGenerator; with the exception of one small tweak to generate the
correct signature for `handleEvent` no other changes to the generator were
required (others were made for unrelated reasons, however).

patch is complete, save:

) changing the comments as needed
) making the test cases work instead of commenting them out

patch still against 1.6.3 because i've been in a tight feedback loop, and my
ccache is blisteringly hot. if it won't apply let me know, i will pull ... (any
method to avoid 1GiB+ download??)

side note: there are continuous leaks when using *any* _create() method (any
method annotated with `ReturnsNew` in the IDL files).  these functions must be
marked `(transfer full)` otherwise they get an extra ref and live forever.  The
WebKitEventListener will handle this properly.

DOM spec doesn't allow listeners to be enumerated -- so for example, in JS, if
you do something like this:

window.addEventListener('click', function(){console.log("whoopsie daisy")},
false)

... you get in a situation where the browser own the only ref, and there is *no
way* to stop it.  while the same can happen with GObject, i took a slightly
different "lazy unbind" approach -- if the GObject half is destroyed (from
python, unref(), etc) *without* properly removing itself from bound
EventTargets, the WebCore half will "silently" (CRIT warn) drop incoming events
and remove itself from each EventTarget as they come.  best case, they all
fire, and the WebCore half is properly destroyed.  worst case, uhm, idk, it
gets destroyed maybe when the DOM gets released? beats me :-) better than
nothing.

i implemented this several different ways on my quest for supreme
understanding. at one point i was using a single array of GValues -- it worked
well -- they know how to destroy their boxed objects additionally create clean
separation from C++ objects.  i borrowed some 3 line static functions from
Telepathy to allocate GValues for the array ... i'm rambling now, but if there
were any more than two GObjects, a hashmap (datalist?) keyed on the address
might worthwhile, but since its only `m_listener` and `m_handler` ... meh.

^^^^^ i went that path trying to support GCallback and GClosure in a stupidly
generic and "optimized" way.  if not obvious from the patch, API supports
GClosure only (for good reason IMO).

thanks guys! look forward to review.


More information about the webkit-reviews mailing list