129783 2014-03-05 23:25:28 -0800 [GTK] Close the page when the view is disposed instead of when finalized 2014-03-06 04:06:45 -0800 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Gtk P2 Normal --- 1 cgarcia webkit-unassigned berto commit-queue csaavedra gustavo mrobinson svillar oldest_to_newest 987576 0 cgarcia 2014-03-05 23:25:28 -0800 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. 987577 1 225956 cgarcia 2014-03-05 23:27:16 -0800 Created attachment 225956 Patch 987579 2 commit-queue 2014-03-05 23:29:23 -0800 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 987610 3 225956 svillar 2014-03-06 01:06:59 -0800 Comment on attachment 225956 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. 987611 4 cgarcia 2014-03-06 01:08:53 -0800 (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. 987629 5 225971 cgarcia 2014-03-06 02:10:00 -0800 Created attachment 225971 Updated patch including a unit test The test is disabled though, because it's affected by bug #129684 987639 6 225971 svillar 2014-03-06 03:14:28 -0800 Comment on attachment 225971 Updated patch including a unit test Much better now. 987646 7 cgarcia 2014-03-06 04:06:45 -0800 Committed r165182: <http://trac.webkit.org/changeset/165182> 225956 2014-03-05 23:27:16 -0800 2014-03-06 02:10:00 -0800 Patch wk2-close-on-dispose.diff text/plain 2019 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzMWNlNzY5Li4zYWE4MDk2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTQtMDMtMDUgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtHVEtdIENsb3NlIHRoZSBwYWdlIHdoZW4gdGhlIHZpZXcgaXMgZGlzcG9z ZWQgaW5zdGVhZCBvZiB3aGVuIGZpbmFsaXplZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI5NzgzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgV2hlbiBhIEd0a1dpZGdldCBpcyBkZXN0cm95ZWQsIHRoZSBH T2JqZWN0IGlzIGRpc3Bvc2VkLCBidXQgbm90CisgICAgICAgIGZpbmFsaXplZCBpZiBpdCBoYXMg YWRkaXRpb25hbCByZWZlcmVuY2VzLiBXaGVuIHRoZSBkZXN0cm95ZWQKKyAgICAgICAgd2lkZ2V0 IGlzIGxlYWtlZCBieSB0aGUgYXBwbGljYXRpb24sIHdlIGxlYWsgdGhlIHBhZ2UgcHJveHkgYW5k CisgICAgICAgIHdoYXQgaXMgbW9yZSBpbXBvcnRhbnQgdGhlIHdlYiBwcm9jZXNzIGluIG11bHRp LXByb2Nlc3MgbW9kZS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdFdlYlZp ZXdCYXNlLmNwcDoKKyAgICAgICAgKHdlYmtpdFdlYlZpZXdCYXNlRGlzcG9zZSk6IENsb3NlIHRo ZSBXZWJQYWdlUHJveHkuCisKIDIwMTQtMDMtMDMgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dh cmNpYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtHVEtdIFdlYktpdDJXZWJFeHRlbnNpb24gR0lS IGNhbid0IGJlIHVzZWQgaW4gdmFsYQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwCmluZGV4IGVhMDI4YTYuLmJmNWUwNDIg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdFdlYlZp ZXdCYXNlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRX ZWJWaWV3QmFzZS5jcHAKQEAgLTg3LDExICs4Nyw2IEBAIHZvaWQgcmVkaXJlY3RlZFdpbmRvd0Rh bWFnZWRDYWxsYmFjayh2b2lkKiBkYXRhKTsKICNlbmRpZgogCiBzdHJ1Y3QgX1dlYktpdFdlYlZp ZXdCYXNlUHJpdmF0ZSB7Ci0gICAgfl9XZWJLaXRXZWJWaWV3QmFzZVByaXZhdGUoKQotICAgIHsK LSAgICAgICAgcGFnZVByb3h5LT5jbG9zZSgpOwotICAgIH0KLQogICAgIFdlYktpdFdlYlZpZXdD aGlsZHJlbk1hcCBjaGlsZHJlbjsKICAgICBPd25QdHI8UGFnZUNsaWVudEltcGw+IHBhZ2VDbGll bnQ7CiAgICAgUmVmUHRyPFdlYlBhZ2VQcm94eT4gcGFnZVByb3h5OwpAQCAtMzkxLDcgKzM4Niw5 IEBAIHZvaWQgd2Via2l0V2ViVmlld0Jhc2VDaGlsZE1vdmVSZXNpemUoV2ViS2l0V2ViVmlld0Jh c2UqIHdlYlZpZXcsIEd0a1dpZGdldCogY2hpCiAKIHN0YXRpYyB2b2lkIHdlYmtpdFdlYlZpZXdC YXNlRGlzcG9zZShHT2JqZWN0KiBnb2JqZWN0KQogewotICAgIHdlYmtpdFdlYlZpZXdCYXNlU2V0 VG9wbGV2ZWxPblNjcmVlbldpbmRvdyhXRUJLSVRfV0VCX1ZJRVdfQkFTRShnb2JqZWN0KSwgMCk7 CisgICAgV2ViS2l0V2ViVmlld0Jhc2UqIHdlYlZpZXcgPSBXRUJLSVRfV0VCX1ZJRVdfQkFTRShn b2JqZWN0KTsKKyAgICB3ZWJraXRXZWJWaWV3QmFzZVNldFRvcGxldmVsT25TY3JlZW5XaW5kb3co d2ViVmlldywgbnVsbHB0cik7CisgICAgd2ViVmlldy0+cHJpdi0+cGFnZVByb3h5LT5jbG9zZSgp OwogICAgIEdfT0JKRUNUX0NMQVNTKHdlYmtpdF93ZWJfdmlld19iYXNlX3BhcmVudF9jbGFzcykt PmRpc3Bvc2UoZ29iamVjdCk7CiB9CiAK 225971 2014-03-06 02:10:00 -0800 2014-03-06 03:14:27 -0800 Updated patch including a unit test wk2-close-on-dispose2.diff text/plain 4812 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzMWNlNzY5Li4zYWE4MDk2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTQtMDMtMDUgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtHVEtdIENsb3NlIHRoZSBwYWdlIHdoZW4gdGhlIHZpZXcgaXMgZGlzcG9z ZWQgaW5zdGVhZCBvZiB3aGVuIGZpbmFsaXplZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI5NzgzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgV2hlbiBhIEd0a1dpZGdldCBpcyBkZXN0cm95ZWQsIHRoZSBH T2JqZWN0IGlzIGRpc3Bvc2VkLCBidXQgbm90CisgICAgICAgIGZpbmFsaXplZCBpZiBpdCBoYXMg YWRkaXRpb25hbCByZWZlcmVuY2VzLiBXaGVuIHRoZSBkZXN0cm95ZWQKKyAgICAgICAgd2lkZ2V0 IGlzIGxlYWtlZCBieSB0aGUgYXBwbGljYXRpb24sIHdlIGxlYWsgdGhlIHBhZ2UgcHJveHkgYW5k CisgICAgICAgIHdoYXQgaXMgbW9yZSBpbXBvcnRhbnQgdGhlIHdlYiBwcm9jZXNzIGluIG11bHRp LXByb2Nlc3MgbW9kZS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdFdlYlZp ZXdCYXNlLmNwcDoKKyAgICAgICAgKHdlYmtpdFdlYlZpZXdCYXNlRGlzcG9zZSk6IENsb3NlIHRo ZSBXZWJQYWdlUHJveHkuCisKIDIwMTQtMDMtMDMgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dh cmNpYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtHVEtdIFdlYktpdDJXZWJFeHRlbnNpb24gR0lS IGNhbid0IGJlIHVzZWQgaW4gdmFsYQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwCmluZGV4IGVhMDI4YTYuLmJmNWUwNDIg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdFdlYlZp ZXdCYXNlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRX ZWJWaWV3QmFzZS5jcHAKQEAgLTg3LDExICs4Nyw2IEBAIHZvaWQgcmVkaXJlY3RlZFdpbmRvd0Rh bWFnZWRDYWxsYmFjayh2b2lkKiBkYXRhKTsKICNlbmRpZgogCiBzdHJ1Y3QgX1dlYktpdFdlYlZp ZXdCYXNlUHJpdmF0ZSB7Ci0gICAgfl9XZWJLaXRXZWJWaWV3QmFzZVByaXZhdGUoKQotICAgIHsK LSAgICAgICAgcGFnZVByb3h5LT5jbG9zZSgpOwotICAgIH0KLQogICAgIFdlYktpdFdlYlZpZXdD aGlsZHJlbk1hcCBjaGlsZHJlbjsKICAgICBPd25QdHI8UGFnZUNsaWVudEltcGw+IHBhZ2VDbGll bnQ7CiAgICAgUmVmUHRyPFdlYlBhZ2VQcm94eT4gcGFnZVByb3h5OwpAQCAtMzkxLDcgKzM4Niw5 IEBAIHZvaWQgd2Via2l0V2ViVmlld0Jhc2VDaGlsZE1vdmVSZXNpemUoV2ViS2l0V2ViVmlld0Jh c2UqIHdlYlZpZXcsIEd0a1dpZGdldCogY2hpCiAKIHN0YXRpYyB2b2lkIHdlYmtpdFdlYlZpZXdC YXNlRGlzcG9zZShHT2JqZWN0KiBnb2JqZWN0KQogewotICAgIHdlYmtpdFdlYlZpZXdCYXNlU2V0 VG9wbGV2ZWxPblNjcmVlbldpbmRvdyhXRUJLSVRfV0VCX1ZJRVdfQkFTRShnb2JqZWN0KSwgMCk7 CisgICAgV2ViS2l0V2ViVmlld0Jhc2UqIHdlYlZpZXcgPSBXRUJLSVRfV0VCX1ZJRVdfQkFTRShn b2JqZWN0KTsKKyAgICB3ZWJraXRXZWJWaWV3QmFzZVNldFRvcGxldmVsT25TY3JlZW5XaW5kb3co d2ViVmlldywgbnVsbHB0cik7CisgICAgd2ViVmlldy0+cHJpdi0+cGFnZVByb3h5LT5jbG9zZSgp OwogICAgIEdfT0JKRUNUX0NMQVNTKHdlYmtpdF93ZWJfdmlld19iYXNlX3BhcmVudF9jbGFzcykt PmRpc3Bvc2UoZ29iamVjdCk7CiB9CiAKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rv b2xzL0NoYW5nZUxvZwppbmRleCBkOWI3MzhhZC4uMjhhMTg0YSAxMDA2NDQKLS0tIGEvVG9vbHMv Q2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE0LTAz LTA2ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KKworICAgICAg ICBbR1RLXSBDbG9zZSB0aGUgcGFnZSB3aGVuIHRoZSB2aWV3IGlzIGRpc3Bvc2VkIGluc3RlYWQg b2Ygd2hlbiBmaW5hbGl6ZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEyOTc4MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIEFkZCB0ZXN0IGNhc2UgYnV0IGRpc2FibGVkIGZvciBub3cgYmVjYXVzZSBpdCdz IGFmZmV0ZWN0ZWQgYnkgYnVnICMxMjk2ODQuCisKKyAgICAgICAgKiBUZXN0V2ViS2l0QVBJL1Rl c3RzL1dlYktpdDJHdGsvVGVzdE11bHRpcHJvY2Vzcy5jcHA6CisgICAgICAgICh0ZXN0UHJvY2Vz c1BlcldlYlZpZXcpOgorCiAyMDE0LTAzLTA1ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJj aWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1RLXSBNaW5pQnJvd3NlciBjcmFzaGVzIHdoZW4g b3BlbmluZyBzZXZlcmFsIHVybHMgcGFzc2VkIGFzIGNvbW1hbmQgbGluZSBvcHRpb25zCmRpZmYg LS1naXQgYS9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdDJHdGsvVGVzdE11bHRpcHJv Y2Vzcy5jcHAgYi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdDJHdGsvVGVzdE11bHRp cHJvY2Vzcy5jcHAKaW5kZXggNGFiNWVkZi4uZDgwZTY4OSAxMDA2NDQKLS0tIGEvVG9vbHMvVGVz dFdlYktpdEFQSS9UZXN0cy9XZWJLaXQyR3RrL1Rlc3RNdWx0aXByb2Nlc3MuY3BwCisrKyBiL1Rv b2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Mkd0ay9UZXN0TXVsdGlwcm9jZXNzLmNwcApA QCAtODEsNiArODEsMjYgQEAgcHVibGljOgogICAgICAgICByZXR1cm4gaWRlbnRpZmllcjsKICAg ICB9CiAKKyAgICBzdGF0aWMgdm9pZCBuYW1lVmFuaXNoZWRDYWxsYmFjayhHREJ1c0Nvbm5lY3Rp b24qIGNvbm5lY3Rpb24sIGNvbnN0IGdjaGFyKiBuYW1lLCBncG9pbnRlciB1c2VyRGF0YSkKKyAg ICB7CisgICAgICAgIGdfbWFpbl9sb29wX3F1aXQoc3RhdGljX2Nhc3Q8R01haW5Mb29wKj4odXNl ckRhdGEpKTsKKyAgICB9CisKKyAgICB2b2lkIGRlc3Ryb3lXZWJWaWV3QW5kV2FpdFVudGlsV2Vi UHJvY2Vzc0ZpbmlzaGVzKHVuc2lnbmVkIGluZGV4KQorICAgIHsKKyAgICAgICAgLy8gRklYTUU6 IFRoaXMgdGVzdCBpcyBkaXNhYmxlZCBiZWNhdXNlIHRoZSB3ZWIgcHJvY2Vzc2VkIGRvbid0IGFj dHVhbGx5IGRpZQorICAgICAgICAvLyBkdWUgdG8gYnVnIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMjk2ODQuCisjaWYgMAorICAgICAgICBnX2Fzc2VydF9jbXB1aW50 KGluZGV4LCA8LCBudW1WaWV3cyk7CisKKyAgICAgICAgdW5zaWduZWQgd2F0Y2hlcklEID0gZ19i dXNfd2F0Y2hfbmFtZV9vbl9jb25uZWN0aW9uKGJ1cy0+Y29ubmVjdGlvbigpLCBtX3dlYlZpZXdC dXNOYW1lc1tpbmRleF0uZ2V0KCksIEdfQlVTX05BTUVfV0FUQ0hFUl9GTEFHU19OT05FLAorICAg ICAgICAgICAgbnVsbHB0ciwgbmFtZVZhbmlzaGVkQ2FsbGJhY2ssIG1fbWFpbkxvb3AsIG51bGxw dHIpOworICAgICAgICBndGtfd2lkZ2V0X2Rlc3Ryb3koR1RLX1dJREdFVChtX3dlYlZpZXdzW2lu ZGV4XS5nZXQoKSkpOworICAgICAgICBnX21haW5fbG9vcF9ydW4obV9tYWluTG9vcCk7CisgICAg ICAgIGdfYnVzX3Vud2F0Y2hfbmFtZSh3YXRjaGVySUQpOworI2VuZGlmCisgICAgfQorCiAgICAg R01haW5Mb29wKiBtX21haW5Mb29wOwogICAgIFZlY3RvcjxHVW5pcXVlUHRyPGNoYXI+LCBudW1W aWV3cz4gbV93ZWJWaWV3QnVzTmFtZXM7CiAgICAgVmVjdG9yPEdSZWZQdHI8V2ViS2l0V2ViVmll dz4sIG51bVZpZXdzPiBtX3dlYlZpZXdzOwpAQCAtMTAzLDYgKzEyMywxMyBAQCBzdGF0aWMgdm9p ZCB0ZXN0UHJvY2Vzc1BlcldlYlZpZXcoTXVsdGlwcm9jZXNzVGVzdCogdGVzdCwgZ2NvbnN0cG9p bnRlcikKICAgICBnX2Fzc2VydF9jbXB1aW50KGluaXRpYWxpemVXZWJFeHRlbnNpb25zU2lnbmFs Q291bnQsID09LCBudW1WaWV3cyk7CiAgICAgZ19hc3NlcnRfY21wc3RyKHRlc3QtPm1fd2ViVmll d0J1c05hbWVzWzBdLmdldCgpLCAhPSwgdGVzdC0+bV93ZWJWaWV3QnVzTmFtZXNbMV0uZ2V0KCkp OwogICAgIGdfYXNzZXJ0X2NtcHVpbnQodGVzdC0+d2ViUHJvY2Vzc1BpZCgwKSwgIT0sIHRlc3Qt PndlYlByb2Nlc3NQaWQoMSkpOworCisgICAgLy8gQ2hlY2sgdGhhdCB3ZWIgcHJvY2Vzc2VzIGZp bmlzaCB3aGVuIHRoZSB3ZWIgdmlldyBpcyBkZXN0cm95ZWQgZXZlbiB3aGVuIGl0J3Mgbm90IGZp bmFsaXplZC4KKyAgICAvLyBTZWUgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEyOTc4My4KKyAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbnVtVmlld3M7IGkrKykg eworICAgICAgICBHUmVmUHRyPFdlYktpdFdlYlZpZXc+IHdlYlZpZXcgPSB0ZXN0LT5tX3dlYlZp ZXdzW2ldOworICAgICAgICB0ZXN0LT5kZXN0cm95V2ViVmlld0FuZFdhaXRVbnRpbFdlYlByb2Nl c3NGaW5pc2hlcyhpKTsKKyAgICB9CiB9CiAKIGNsYXNzIFVJQ2xpZW50TXVsdGlwcm9jZXNzVGVz dDogcHVibGljIFdlYlZpZXdUZXN0IHsK