[webkit-reviews] review denied: [Bug 216120] [GLIB] RemoteInspectorServer is not started if WebKitWebContext is not created already created : [Attachment 407979] patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Pablo Saavedra
<psaavedra at igalia.com>'s request for review:
Bug 216120: [GLIB] RemoteInspectorServer is not started if WebKitWebContext is
not created already created
https://bugs.webkit.org/show_bug.cgi?id=216120

Attachment 407979: patch

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




--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 407979
  --> https://bugs.webkit.org/attachment.cgi?id=407979
patch

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

> Source/WebKit/ChangeLog:9
> +	   Added a WebKit/UIProcess/API/glib/WebKitInitialize what implements
> +	   its own webkitInitialize() to ensure the correct initialization
> +	   of WebKit during the class init of the more relevant objects
> +	   exposed by the API.

Not to ensure the correct initialization of WebKit, because that's already done
by InitializeWebKit2, but to ensure the inspector server is initialized as
early as possible, before other api calls that would depend on the inspector
server being running.

> Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp:100
> +    webkitInitialize();

It doesn't really matter but I wonder why you added this at the beginning of
class_init in some cases and the end in others.

> Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:339
> +    webkitInitialize();
> +

WebKitGeolocationManager can only be crated by a WebKitWebContext, so at this
point the init has already been called for sure. The user can't create a
WebKitGeolocationManager.

> Source/WebKit/UIProcess/API/glib/WebKitInitialize.cpp:67
> +    if (const char* address = g_getenv("WEBKIT_INSPECTOR_SERVER"))
> +	   initializeRemoteInspectorServer(address);

webkitInitialize() is going to be called multiple times, better use std::once
to ensure the inspector is only initialized once.

> Source/WebKit/UIProcess/API/glib/WebKitInputMethodContext.cpp:211
> +    webkitInitialize();

This file doesn't have a using namespace WebKit, so you need to either add it,
or use WebKit:: here.