[webkit-reviews] review granted: [Bug 73746] [GTK] Don't log document events in DRT : [Attachment 117766] Patch proposal

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 73746: [GTK] Don't log document events in DRT
https://bugs.webkit.org/show_bug.cgi?id=73746

Attachment 117766: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=117766&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=117766&action=review


Seems good to me, but I have included a few minor style issue below.

> Source/WebKit/gtk/ChangeLog:21
> +	   (updateLoadingEventsResult): New, updates a global variable to
> +	   allow checking later on that the right signals were emmitted.
> +	   (documentLoadingEventCb): New, global listener for load-complete
> +	   and reload signals over the document object (the web area).
> +	   (testWebkitAtkDocumentLoadingEvents): New unit test, globally
> +	   connects to document-related signals and check they are properly
> +	   emitted when reloading the web view.
> +	   (main): Added new test.

This is nice.

> Source/WebKit/gtk/tests/testatk.c:495
> +    if (!ATK_IS_OBJECT(axObject) || !signalName)

Is this an error if it doesn't happen. If so, would it be better to assert here
instead?

> Source/WebKit/gtk/tests/testatk.c:506
> +
> +static gboolean documentLoadingEventCb(GSignalInvocationHint *signalHint,
> +					  guint numParamValues,
> +					  const GValue *paramValues,
> +					  gpointer data)

Please put these on one line to match WebKit style. The function should also be
named documentLoadingEventCallback.

> Source/WebKit/gtk/tests/testatk.c:516
> +    GSignalQuery signal_query;

signal_query -> signalQuery.