For the cases where "WEBKIT_INSPECTOR_SERVER" is defined as an envvar. Let's think in a case where we are initializing the UI process (ex: a WebKit launcher) and we have a code like this ``` ... webkit_user_content_filter_store_save() webkit_web_context_new_with_website_data_manager() ... ``` (note that before `webkit_user_content_filter_store_save()` no other WebContext or WebView was initialized nor created). When the webkit_user_content_filter_store_save() calls RemoteInspector.singleton() [WebKit!L72](https://github.com/WebKit/webkit/blame/d2c95d8ce5e68f5c81983dad44085dbf915ca52b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp#L72), the RemoteInspectorServer isn't initialized yet because this action is triggered only by any API method which returns a WebKitWebContext: WebKitWebContext * webkit_web_context_get_default () WebKitWebContext * webkit_web_context_new () WebKitWebContext * webkit_web_context_new_ephemeral () WebKitWebContext * webkit_web_context_new_with_website_data_manager ()  ... so the RemoteInspector object is going to fail trying to connect to the server: "RemoteInspector failed to connect to inspector server at: %s: %s"  The initialization of the RemoteInspectorServer is done in the WebKit::WebProcessPool::platformInitialize() method during the WebProcessPool::create(). And this, as I said, it only can be triggered from the WK API by creating or getting a WebContext. This is because the initialization is done during the WebKit::WebProcessPool::WebProcessPool() creation. My question: Is it really needed the Inspector::RemoteInspectorServer::start() be called only from the WebKit::WebProcessPool::platformInitialize() or the from WebKit::WebProcessPool::WebProcessPool()? Could be possible start the Inspector::RemoteInspectorServer from another place, early?
The issue that I had while working on bug #212002 where I was trying to add API to remote inspect JSCContext instances is probably related: we can have a bare JSCContext without any WebKitWebContext being created, so this makes me think that maybe we should detach the creation of the RemoteInspectorServer singleton from the WebKitWebContext, and instead have some API to start it.
Created attachment 407979 [details] patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
(In reply to Adrian Perez from comment #1) > The issue that I had while working on bug #212002 where I was trying > to add API to remote inspect JSCContext instances is probably related: > we can have a bare JSCContext without any WebKitWebContext being created, > so this makes me think that maybe we should detach the creation of the > RemoteInspectorServer singleton from the WebKitWebContext, and instead > have some API to start it. Maybe. Based on your description, yes, it could be the case.
Pushed a tentative patch to address this. After a conversation with Carlos García, I decided to add a new WebKit/UIProcess/API/glib/WebKitInitialize what implements its own webkitInitialize() method in a similar way that the shared InitializeWebKit2() but exclusive for the UIProcess/API objects. This WebKitInitialize is now the responsible of the initialization of the RemoteInspectorServer instead of WebProcessPoolGLib. The WebKitInitialize is called in the initialization of any of the API objects what could be the first in to be initialized by a UI process: WebKitCookieManager, WebKitFaviconDatabase, WebKitGeolocationManager, WebKitInputMethodContext, WebKitSettings, WebKitUserContentManager, WebKitWebContext, WebKitWebsiteDataManager, WebKitUserContentFilterStore, WebKitWebViewBase.
Comment on attachment 407979 [details] 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.
Created attachment 407983 [details] patch
(In reply to Carlos Garcia Campos from comment #6) > Comment on attachment 407979 [details] > 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. > There is not an specific reason. For coherence, I will modify the patch to put it the call in the beginning of the class_init always.
Created attachment 407990 [details] patch
Comment on attachment 407990 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=407990&action=review > Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:122 > + webkitInitialize(); WebKitCookieManager can only be crated by a WebKitWebsiteDataManager, so at this point the init has already been called for sure. The user can't create a WebKitCookieManager. > Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp:77 > + webkitInitialize(); WebKitFaviconDatabase 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 WebKitFaviconDatabase. > Source/WebKit/UIProcess/API/glib/WebKitInitialize.cpp:35 > +#if !PLATFORM(COCOA) You don't need this, cocoa based ports never build this file. > Source/WebKit/UIProcess/API/glib/WebKitInitialize.h:32 > +}; Trailing ; is not needed here.
Created attachment 408171 [details] patch
I've updated the ticket again.. @cgarcia thanks for your comments.
Created attachment 408178 [details] patch
Comment on attachment 408178 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=408178&action=review > Source/WebKit/ChangeLog:22 > + Reviewed by NOBODY (OOPS!). This usually goes before he description.
Created attachment 408192 [details] patch
(In reply to Carlos Garcia Campos from comment #14) > Comment on attachment 408178 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408178&action=review > > > Source/WebKit/ChangeLog:22 > > + Reviewed by NOBODY (OOPS!). > > This usually goes before he description. OK. Let's do this change too.
Committed r266718: <https://trac.webkit.org/changeset/266718> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408192 [details].