[Webkit-unassigned] [Bug 77835] [GObject bindings] EventListeners as first-class GObjects: WebKitDOMEventListener

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=77835





--- Comment #34 from C Anthony <anthony at xtfx.me>  2012-03-13 12:11:29 PST ---
(In reply to comment #33)
> I'll just post some concerns about the API for starters.
> 
> > b/Source/WebCore/bindings/gobject/WebKitDOMCustom.h:49
> > +WEBKIT_API WebKitDOMEventListener* webkit_dom_document_create_event_listener(WebKitDOMDocument* self, GClosure* handler);
> 
> Event listener is in no way tied to a document in the DOM spec, meaning creating it should be done through a standalone function - webkit_dom_event_listener_create.

ok ok ok, i did that mainly because people would already be aware of _document_create_event ... so adding _listener is a natural extension of that.

however, i'm mostly impartial, and will change it as suggested unless otherwise stated.

> I'm reiterating ideas previously written in comment #22 - webkit_dom_event_listener_create would take in a WebKitDOMEventListenerFunc, gpointer to a data and a GDestroyNotify callback. As far as I am concerned, this is the most optimal solution. Using a GClosure is just another unnecessary step that should be avoided if possible.

isn't GClosure specifically designed for exactly this? it simplifies things for bindings because an explicit type is not needed and the arguments are variable; additionally, the same [single] object can be used for C/Python/whatever, and maintains it's own ref/destroy.  i want to say it's faster for bindings too, but i could be making that up :-X

i discussed on IRC previously, and tomeu agreed with the GClosure approach, and added it would not affect performance.  one should note that the WebKitDOMEventListener will sink() the closure and assume ownership -- no need for the caller to manage it.

however, i admittedly moved to GClosure while implementing an alternative way, and never changed it back -- it's no longer a hard requirement -- if consensus is for callback/data/destroy signature i'll change it to that (i will anyway for time being)

> > b/Source/WebCore/bindings/gobject/WebKitDOMCustom.h:58
> > +WEBKIT_API void webkit_dom_event_listener_set_handler(WebKitDOMEventListener* self, GClosure* handler);
> 
> The DOM spec does not allow these sorts of manipulations on an event listener, and I am of a very strong opinion that we shouldn't neither.

i'm very much the opposite -- the DOM spec doesn't specify anything really, and EventListener is an technically only an interface ... i mean not even _create() would be permissible then, except by necessity.  by enabling the *real* handler to be swapped dynamically, at no cost or code complexity, add a LOT of power and flexibility -- isn't this useful?  imagine 200 Targets that all shared a Listener, but with varying event types ... not simple/fast to update, and expensive to update from a binding (marshall back/forth hundreds of times).  by exposing a this we can change the backing handler in a single operation from any language ...

you can think of it as a detail being exposed to the DOM implementation, NOT the DOM spec (spec doesn't undestand GCallback/etc, eg. _create(), setHandler()), so it doesn't violate anything IMO.

how about a compromise: you get callback/data/destroy sig, and i get setHandler? :-D fair?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.