RESOLVED FIXED 129783
[GTK] Close the page when the view is disposed instead of when finalized
https://bugs.webkit.org/show_bug.cgi?id=129783
Summary [GTK] Close the page when the view is disposed instead of when finalized
Carlos Garcia Campos
Reported 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.
Attachments
Patch (1.97 KB, patch)
2014-03-05 23:27 PST, Carlos Garcia Campos
no flags
Updated patch including a unit test (4.70 KB, patch)
2014-03-06 02:10 PST, Carlos Garcia Campos
svillar: review+
Carlos Garcia Campos
Comment 1 2014-03-05 23:27:16 PST
WebKit Commit Bot
Comment 2 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
Sergio Villar Senin
Comment 3 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.
Carlos Garcia Campos
Comment 4 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.
Carlos Garcia Campos
Comment 5 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
Sergio Villar Senin
Comment 6 2014-03-06 03:14:28 PST
Comment on attachment 225971 [details] Updated patch including a unit test Much better now.
Carlos Garcia Campos
Comment 7 2014-03-06 04:06:45 PST
Note You need to log in before you can comment on or make changes to this bug.