[webkit-reviews] review granted: [Bug 224359] [GTK][WPE] Add a property to the WebKitWebView indicating whether the web process is responsive : [Attachment 425592] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 9 05:57:49 PDT 2021


Adrian Perez <aperez at igalia.com> has granted Miguel Gomez
<magomez at igalia.com>'s request for review:
Bug 224359: [GTK][WPE] Add a property to the WebKitWebView indicating whether
the web process is responsive
https://bugs.webkit.org/show_bug.cgi?id=224359

Attachment 425592: Patch

https://bugs.webkit.org/attachment.cgi?id=425592&action=review




--- Comment #4 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 425592
  --> https://bugs.webkit.org/attachment.cgi?id=425592
Patch

The API addition looks good to me. Please take a look at the comments
below before landing, tho.

View in context: https://bugs.webkit.org/attachment.cgi?id=425592&action=review

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:814
> +    priv->isWebProcessResponsive = true;

This line is not needed, the default value is set to “TRUE” in the call to
“g_param_spec_boolean()” inside the “webkit_web_view_class_init()” below,
which will trigger setting the initial value of the property when during
instantiation.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:1360
> +	       _("Whether the current web process is responsive"),

Nit: which process is the “current web process”? I think it would more precise
to
write this as “Whether the web process associated with the web view is
responsive”
(or similar).

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:4759
> +    g_object_notify(G_OBJECT(webView), "is-web-process-responsive");

It always annoys me a tiny little bit that every time we notify of object
property
changes in the WebKit{GTK,WPE} code we don't use “g_object_notify_by_pspec()”
to
avoid the property lookup… so I filed bug #224366 to maybe do that change at
some
point later on ��️

> Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h:123
> +

Why the extra empty space? Please remove it before landing.


More information about the webkit-reviews mailing list