[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