Bug 129783 - [GTK] Close the page when the view is disposed instead of when finalized
Summary: [GTK] Close the page when the view is disposed instead of when finalized
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2014-03-05 23:25 PST by Carlos Garcia Campos
Modified: 2014-03-06 04:06 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.97 KB, patch)
2014-03-05 23:27 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch including a unit test (4.70 KB, patch)
2014-03-06 02:10 PST, Carlos Garcia Campos
svillar: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2014-03-05 23:25:28 PST
When a GtkWidget is destroyed, the GObject is disposed, but not finalized if it has additional references. When the destroyed widget is leaked by the application, we leak the page proxy and what is more important the web process in multi-process mode.
Comment 1 Carlos Garcia Campos 2014-03-05 23:27:16 PST
Created attachment 225956 [details]
Patch
Comment 2 WebKit Commit Bot 2014-03-05 23:29:23 PST
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
Comment 3 Sergio Villar Senin 2014-03-06 01:06:59 PST
Comment on attachment 225956 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:391
> +    webView->priv->pageProxy->close();

Hmm, dispose() might be called multiple times, what would happen to the pageProxy if we call close() many times?.

I'd better add some condition there.
Comment 4 Carlos Garcia Campos 2014-03-06 01:08:53 PST
(In reply to comment #3)
> (From update of attachment 225956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225956&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:391
> > +    webView->priv->pageProxy->close();
> 
> Hmm, dispose() might be called multiple times, what would happen to the pageProxy if we call close() many times?.

Nothing, there's an early return in WebPageProxy:close();

> I'd better add some condition there.

It's not needed.
Comment 5 Carlos Garcia Campos 2014-03-06 02:10:00 PST
Created attachment 225971 [details]
Updated patch including a unit test

The test is disabled though, because it's affected by bug #129684
Comment 6 Sergio Villar Senin 2014-03-06 03:14:28 PST
Comment on attachment 225971 [details]
Updated patch including a unit test

Much better now.
Comment 7 Carlos Garcia Campos 2014-03-06 04:06:45 PST
Committed r165182: <http://trac.webkit.org/changeset/165182>