Bug 208689

Summary: [WPE] MiniBrowser: Close child WebViews when exiting
Product: WebKit Reporter: Lauro Moura <lmoura>
Component: WPE WebKitAssignee: Lauro Moura <lmoura>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, zan
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 207529    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

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.