[Webkit-unassigned] [Bug 77835] [GObject bindings] Make EventTarget interface introspectable

[bugzilla-daemon at webkit.org]

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





--- Comment #68 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-10-24 00:45:38 PST ---
(In reply to comment #67)
> (From update of attachment 214942 [details])
> LGTM!

Thanks for the review.

> View in context: https://bugs.webkit.org/attachment.cgi?id=214942&action=review
> 
> >> Source/WebCore/bindings/gobject/WebKitDOMEventTarget.cpp:49
> >> -void webkit_dom_event_target_dispatch_event(WebKitDOMEventTarget* target, WebKitDOMEvent* event, GError** error)
> >> +gboolean webkit_dom_event_target_dispatch_event(WebKitDOMEventTarget* target, WebKitDOMEvent* event, GError** error)
> > 
> > webkit_dom_event_target_dispatch_event is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
> 
> Can you add exceptions for these checks along with this patch? It should be pretty straight-forward, just add it alongside WebKitDOMCustom in Tools/Scripts/webkitpy/style/checker.py, I believe.

Let me try, yes.

> > Source/WebCore/bindings/gobject/WebKitDOMEventTarget.cpp:65
> > +    GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(handler, userData, 0));
> > +    return WEBKIT_DOM_EVENT_TARGET_GET_IFACE(target)->add_event_listener(target, eventName, closure.get(), useCapture);
> 
> I keep wondering if we should ditch GRefPtr here and do the adoption of the reference for the closure in the GObjectListener constructor. Feels like unnecessary churn but at the same time makes things more straight-forward and obvious I guess…

We don't want to adopt the ref in the case of closure received from add_event_listener_with_closure, so GObjectEventListener should always take a new reference.

> >> Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:64
> >> +WEBKIT_API gboolean  webkit_dom_event_target_dispatch_event(WebKitDOMEventTarget *target,
> > 
> > The parameter name "target" adds no information, so it should be removed.  [readability/parameter_name] [5]
> 
> API break, but ABI stable, no sweat from my side =P

Yes, I explained the situation in the mailing list asking if anybody is using dispatch_event, and nobody replied so far. If someone eventually complains we can just revert this patch before the next stable release.

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