Summary: | [GTK][WPE] Add WebKitWebView::web-process-terminated signal and deprecate web-process-crashed | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | berto, bugs-noreply, ews-watchlist, gustavo, mcatanzaro | ||||
Priority: | P2 | Keywords: | Gtk | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=180861 | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2017-12-15 03:33:37 PST
Created attachment 329484 [details]
Patch
WebKitWebProcessTerminationReason can be expanded to support ExceededCPULimit if we enable that.
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 Carlos Garcia Campos from comment #1) > WebKitWebProcessTerminationReason can be expanded to support > ExceededCPULimit if we enable that. We should definitely enable that, too. Comment on attachment 329484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329484&action=review I thought about recommending that we adjust the API for the possibility that, in the future, we might want to notify the UI process if other secondary processes terminate. The signal could be SECONDARY_PROCESS_TERMINATED or just PROCESS_TERMINATED instead of WEB_PROCESS_TERMINATED. But I doubt we'll ever need to do that, and this seems clearer. > Source/WebKit/UIProcess/API/gtk/WebKitWebView.h:191 > + * @WEBKIT_WEB_PROCESS_CRAHED: the web process crashed. CRASHED > Source/WebKit/UIProcess/API/wpe/WebKitWebView.h:166 > + WEBKIT_WEB_PROCESS_CRAHED, Ditto. > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebExtensions.cpp:86 > + g_assert_cmpuint(reason, ==, WEBKIT_WEB_PROCESS_CRAHED); You have to look after you Ctrl+V :P (In reply to Michael Catanzaro from comment #3) > (In reply to Carlos Garcia Campos from comment #1) > > WebKitWebProcessTerminationReason can be expanded to support > > ExceededCPULimit if we enable that. > > We should definitely enable that, too. Yes, but maybe we a way to disable it. (In reply to Michael Catanzaro from comment #4) > Comment on attachment 329484 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=329484&action=review > > I thought about recommending that we adjust the API for the possibility > that, in the future, we might want to notify the UI process if other > secondary processes terminate. The signal could be > SECONDARY_PROCESS_TERMINATED or just PROCESS_TERMINATED instead of > WEB_PROCESS_TERMINATED. In the case of other secondary processes, for example the network process, the signal wouldn't belong to the web view, but to the web context. > But I doubt we'll ever need to do that, and this seems clearer. > > > Source/WebKit/UIProcess/API/gtk/WebKitWebView.h:191 > > + * @WEBKIT_WEB_PROCESS_CRAHED: the web process crashed. > > CRASHED > > > Source/WebKit/UIProcess/API/wpe/WebKitWebView.h:166 > > + WEBKIT_WEB_PROCESS_CRAHED, > > Ditto. > > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebExtensions.cpp:86 > > + g_assert_cmpuint(reason, ==, WEBKIT_WEB_PROCESS_CRAHED); > > You have to look after you Ctrl+V :P Indeed Committed r226019: <https://trac.webkit.org/changeset/226019> |