[Webkit-unassigned] [Bug 123292] [GTK][WK2] Add webview signal for web-process-started
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 30 07:06:29 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=123292
Carlos Garcia Campos <cgarcia at igalia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #215114|review?, commit-queue? |review-, commit-queue-
Flag| |
--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> 2013-10-30 07:05:15 PST ---
(From update of attachment 215114)
View in context: https://bugs.webkit.org/attachment.cgi?id=215114&action=review
Thanks for the patch. You should also add a new unit tests to test the new API.
> Source/WebKit2/UIProcess/API/gtk/WebKitLoaderClient.cpp:198
> + WKContextConnectionClient wkConnectionClient = {
> + kWKContextConnectionClientCurrentVersion,
> + webView, // clientInfo
> + processDidStart
> + };
> + WebKitWebContext* webContext = webkit_web_view_get_context(webView);
> + WKContextSetConnectionClient(toAPI(webkitWebContextGetContext(webContext)), &wkConnectionClient);
Why is this in WebKitLoaderClient? I think this should be in WebKitWebContext, and the context should notify all its web views.
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1408
> + * This signal is emitted when the web process started.
You should explain this a bit more, is this also emitted when the web process is respawned after a crash?
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1409
> + */
Since: 2.4
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:3142
> +void webkitWebViewWebProcessStarted(WebKitWebView* webView, WebConnection* connection)
This would be easier if it receives the pid directly instead of the connection
>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:3148
>> + g_signal_emit(webView, signals[WEB_PROCESS_STARTED], 0, (gpointer) &pid);
>
> You should use a c++-style cast if it is indeed needed (static_cast<gpointer>())
We could avoid this by not passing the pid as a signal parameter, and adding webkit_web_view_get_web_process_identifier(). This way you can also get the pid without having to connect to this signal (if you already know the process has been launched already).
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:252
> + gboolean (* web_process_started) (WebKitWebView *web_view,
This should be void instead of gboolean.
> Source/WebKit2/UIProcess/WebContext.cpp:637
> + result = m_processes[i].get();
> + break;
You can simply return m_processes[i].get() here and you don't need the local variable nor the break.
> Source/WebKit2/UIProcess/WebContext.cpp:640
> + return result;
And return 0 here.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list