Bug 129783 – [GTK] Close the page when the view is disposed instead of when finalized

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 225956 [details]
Patch

[WebKit Commit Bot]

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 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]

(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]

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 on attachment 225971 [details]
Updated patch including a unit test

Much better now.

[Carlos Garcia Campos]

Committed r165182: <http://trac.webkit.org/changeset/165182>