Bug 208689 - [WPE] MiniBrowser: Close child WebViews when exiting
Summary: [WPE] MiniBrowser: Close child WebViews when exiting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lauro Moura
URL:
Keywords:
Depends on: 207529
Blocks:
  Show dependency treegraph
 
Reported: 2020-03-05 19:43 PST by Lauro Moura
Modified: 2020-03-10 09:11 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.17 KB, patch)
2020-03-08 22:17 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff
Patch (3.25 KB, patch)
2020-03-09 18:12 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff
Patch (3.41 KB, patch)
2020-03-09 20:32 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff
Patch (3.37 KB, patch)
2020-03-10 08:26 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lauro Moura 2020-03-05 19:43:51 PST
During the discussion in bug207529, it was noted that WPE's MiniBrowser only cleans up the main webview when exiting, while ideally it should also clean up any child view created.
Comment 1 Lauro Moura 2020-03-08 22:17:16 PDT
Created attachment 393008 [details]
Patch
Comment 2 Zan Dobersek 2020-03-09 01:06:47 PDT
Comment on attachment 393008 [details]
Patch

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

> Tools/MiniBrowser/wpe/main.cpp:308
> +    g_signal_connect(webView, "close", G_CALLBACK(+[](WebKitWebView *view, gpointer) {

That + is redundant.
Comment 3 Carlos Garcia Campos 2020-03-09 01:10:47 PDT
Comment on attachment 393008 [details]
Patch

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

>> Tools/MiniBrowser/wpe/main.cpp:308
>> +    g_signal_connect(webView, "close", G_CALLBACK(+[](WebKitWebView *view, gpointer) {
> 
> That + is redundant.

I think you can use G_CALLBACK(g_object_unref) directly. Maybe it's easier to add the main web view to the hash table and you don't need to handle it differently. You could use a weak ref to remove the web views from the hash table when they are destroyed, too.
Comment 4 Lauro Moura 2020-03-09 18:12:59 PDT
Created attachment 393105 [details]
Patch
Comment 5 Lauro Moura 2020-03-09 20:32:54 PDT
Created attachment 393119 [details]
Patch
Comment 6 Carlos Garcia Campos 2020-03-10 00:41:05 PDT
Comment on attachment 393119 [details]
Patch

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

> Tools/MiniBrowser/wpe/main.cpp:154
> +    g_hash_table_remove(openViews, (gpointer*)webView);

This is cpp file, use C++ cast.

> Tools/MiniBrowser/wpe/main.cpp:174
> +    g_hash_table_add(openViews, (gpointer*)newWebView);

Ditto.

> Tools/MiniBrowser/wpe/main.cpp:303
> +    openViews = g_hash_table_new_full(NULL, NULL, g_object_unref, NULL);

And nullptr instead of NULL.
Comment 7 Lauro Moura 2020-03-10 08:26:51 PDT
Created attachment 393153 [details]
Patch
Comment 8 WebKit Commit Bot 2020-03-10 09:11:29 PDT
Comment on attachment 393153 [details]
Patch

Clearing flags on attachment: 393153

Committed r258211: <https://trac.webkit.org/changeset/258211>
Comment 9 WebKit Commit Bot 2020-03-10 09:11:31 PDT
All reviewed patches have been landed.  Closing bug.