150082 2015-10-13 00:17:09 -0700 [GTK] Use GUniquePtr for GtkIconInfo 2015-10-13 04:31:26 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 changseok changseok bugs-noreply cgarcia commit-queue esprehn+autocc glenn kondapallykalyan oldest_to_newest 1132713 0 changseok 2015-10-13 00:17:09 -0700 SSIA 1132716 1 262977 changseok 2015-10-13 00:21:54 -0700 Created attachment 262977 Patch 1132730 2 262977 cgarcia 2015-10-13 01:21:11 -0700 Comment on attachment 262977 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262977&action=review Thanks for the patch. This is not enough, though. With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > Source/WebCore/rendering/RenderThemeGtk.cpp:242 > GdkPixbuf* icon = 0; Please change this to nullptr. > Source/WebCore/rendering/RenderThemeGtk.cpp:244 > + icon = gtk_icon_info_load_symbolic_for_context(info.get(), context, 0, 0); nullptr, nullptr 1132738 3 changseok 2015-10-13 02:48:51 -0700 (In reply to comment #2) > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. Thanks for your review. =) I am not sure of this comment. We already have the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used in ImageGtk.cpp 1132739 4 cgarcia 2015-10-13 02:52:59 -0700 (In reply to comment #3) > (In reply to comment #2) > > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > Thanks for your review. =) I am not sure of this comment. We already have > the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used > in ImageGtk.cpp ImageGtk.cpp includes GUniquePtrGtk.h, but RenderThemeGtk.cpp doesn't. 1132741 5 262981 changseok 2015-10-13 03:34:05 -0700 Created attachment 262981 Patch 1132742 6 changseok 2015-10-13 03:37:01 -0700 (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > > Thanks for your review. =) I am not sure of this comment. We already have > > the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used > > in ImageGtk.cpp > > ImageGtk.cpp includes GUniquePtrGtk.h, but RenderThemeGtk.cpp doesn't. Oh yeah. You are right. 1132743 7 262981 commit-queue 2015-10-13 04:31:23 -0700 Comment on attachment 262981 Patch Clearing flags on attachment: 262981 Committed r190970: <http://trac.webkit.org/changeset/190970> 1132744 8 commit-queue 2015-10-13 04:31:26 -0700 All reviewed patches have been landed. Closing bug. 262977 2015-10-13 00:21:54 -0700 2015-10-13 03:33:59 -0700 Patch bug-150082-20151013082118.patch text/plain 2109 changseok U3VidmVyc2lvbiBSZXZpc2lvbjogMTkwODkwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZThlMDAzMzFhYWJhMTEw Y2Q1NzliNGNjMzU3MzZkNDM0NTBmN2JjNy4uMjE1MjJmNzRjMTA4MjRhZGY3YWZiM2VlYTliZGU2 ZTgwNGQwMmI1OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE1LTEwLTEzICBDaGFu Z1Nlb2sgT2ggIDxjaGFuZ3Nlb2sub2hAY29sbGFib3JhLmNvbT4KKworICAgICAgICBbR1RLXSBV c2UgR1VuaXF1ZVB0ciBmb3IgR3RrSWNvbkluZm8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTE1MDA4MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEd0a0ljb25JbmZvIGNhYiBiZSB3cmFwcGVkIGJ5IGEgc21h cnQgcG9pbnRlciwgR1VuaXF1ZXB0ci4KKworICAgICAgICBObyBuZXcgdGVzdCByZXF1aXJlZCBz aW5jZSBubyBmdW5jdGlvbmFsaXR5IGNoYW5nZWQuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVu ZGVyVGhlbWVHdGsuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Z2V0U3RvY2tTeW1ib2xpY0ljb25G b3JXaWRnZXRUeXBlKToKKwogMjAxNS0xMC0xMiAgQW50b2luZSBRdWludCAgPGdyYW91dHNAYXBw bGUuY29tPgogCiAgICAgICAgIFtTVkddIEhhbmRsZSBlbmRFdmVudCBmb3Igc3ZnIGFuaW1hdGlv bnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUd0ay5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVHdGsuY3BwCmluZGV4IDdh MmFlOWJmNTI3ZDcwNmQxMmZjYzRiMGE3MzExM2U3YWIyNzRmZTguLjg2MjM3NmNlNjRhNGI4YTAz NmRjMmFkNzc4YzAxMTFlYWRhNDE1NmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJUaGVtZUd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlclRoZW1lR3RrLmNwcApAQCAtMjM2LDEzICsyMzYsMTIgQEAgc3RhdGljIEdSZWZQdHI8R2Rr UGl4YnVmPiBnZXRTdG9ja1N5bWJvbGljSWNvbkZvcldpZGdldFR5cGUoR1R5cGUgd2lkZ2V0VHlw ZSwgY28KIAogICAgIGd0a19zdHlsZV9jb250ZXh0X3NldF9zdGF0ZShjb250ZXh0LCBzdGF0aWNf Y2FzdDxHdGtTdGF0ZUZsYWdzPihmbGFncykpOwogICAgIGd0a19zdHlsZV9jb250ZXh0X3NldF9k aXJlY3Rpb24oY29udGV4dCwgc3RhdGljX2Nhc3Q8R3RrVGV4dERpcmVjdGlvbj4oZGlyZWN0aW9u KSk7Ci0gICAgR3RrSWNvbkluZm8qIGluZm8gPSBndGtfaWNvbl90aGVtZV9sb29rdXBfaWNvbihn dGtfaWNvbl90aGVtZV9nZXRfZGVmYXVsdCgpLCBzeW1ib2xpY0ljb25OYW1lLCBpY29uU2l6ZSwK LSAgICAgICAgc3RhdGljX2Nhc3Q8R3RrSWNvbkxvb2t1cEZsYWdzPihHVEtfSUNPTl9MT09LVVBf Rk9SQ0VfU1ZHIHwgR1RLX0lDT05fTE9PS1VQX0ZPUkNFX1NJWkUpKTsKKworICAgIEdVbmlxdWVQ dHI8R3RrSWNvbkluZm8+IGluZm8oZ3RrX2ljb25fdGhlbWVfbG9va3VwX2ljb24oZ3RrX2ljb25f dGhlbWVfZ2V0X2RlZmF1bHQoKSwgc3ltYm9saWNJY29uTmFtZSwgaWNvblNpemUsCisgICAgICAg IHN0YXRpY19jYXN0PEd0a0ljb25Mb29rdXBGbGFncz4oR1RLX0lDT05fTE9PS1VQX0ZPUkNFX1NW RyB8IEdUS19JQ09OX0xPT0tVUF9GT1JDRV9TSVpFKSkpOwogICAgIEdka1BpeGJ1ZiogaWNvbiA9 IDA7Ci0gICAgaWYgKGluZm8pIHsKLSAgICAgICAgaWNvbiA9IGd0a19pY29uX2luZm9fbG9hZF9z eW1ib2xpY19mb3JfY29udGV4dChpbmZvLCBjb250ZXh0LCAwLCAwKTsKLSAgICAgICAgZ3RrX2lj b25faW5mb19mcmVlKGluZm8pOwotICAgIH0KKyAgICBpZiAoaW5mbykKKyAgICAgICAgaWNvbiA9 IGd0a19pY29uX2luZm9fbG9hZF9zeW1ib2xpY19mb3JfY29udGV4dChpbmZvLmdldCgpLCBjb250 ZXh0LCAwLCAwKTsKIAogICAgIGd0a19zdHlsZV9jb250ZXh0X3Jlc3RvcmUoY29udGV4dCk7CiAK 262981 2015-10-13 03:34:05 -0700 2015-10-13 04:31:23 -0700 Patch bug-150082-20151013113328.patch text/plain 2358 changseok U3VidmVyc2lvbiBSZXZpc2lvbjogMTkwODkwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZThlMDAzMzFhYWJhMTEw Y2Q1NzliNGNjMzU3MzZkNDM0NTBmN2JjNy4uMjE1MjJmNzRjMTA4MjRhZGY3YWZiM2VlYTliZGU2 ZTgwNGQwMmI1OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE1LTEwLTEzICBDaGFu Z1Nlb2sgT2ggIDxjaGFuZ3Nlb2sub2hAY29sbGFib3JhLmNvbT4KKworICAgICAgICBbR1RLXSBV c2UgR1VuaXF1ZVB0ciBmb3IgR3RrSWNvbkluZm8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTE1MDA4MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEd0a0ljb25JbmZvIGNhYiBiZSB3cmFwcGVkIGJ5IGEgc21h cnQgcG9pbnRlciwgR1VuaXF1ZXB0ci4KKworICAgICAgICBObyBuZXcgdGVzdCByZXF1aXJlZCBz aW5jZSBubyBmdW5jdGlvbmFsaXR5IGNoYW5nZWQuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVu ZGVyVGhlbWVHdGsuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Z2V0U3RvY2tTeW1ib2xpY0ljb25G b3JXaWRnZXRUeXBlKToKKwogMjAxNS0xMC0xMiAgQW50b2luZSBRdWludCAgPGdyYW91dHNAYXBw bGUuY29tPgogCiAgICAgICAgIFtTVkddIEhhbmRsZSBlbmRFdmVudCBmb3Igc3ZnIGFuaW1hdGlv bnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUd0ay5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVHdGsuY3BwCmluZGV4IDdh MmFlOWJmNTI3ZDcwNmQxMmZjYzRiMGE3MzExM2U3YWIyNzRmZTguLmE0ZDhmOGZkMGUwNDM2OWUw N2FkOGU5ZTY1ZDM3NWEyMGYyZDg2YWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJUaGVtZUd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlclRoZW1lR3RrLmNwcApAQCAtMzAsNiArMzAsNyBAQAogI2luY2x1ZGUgIkZpbGVMaXN0Lmgi CiAjaW5jbHVkZSAiRmlsZVN5c3RlbS5oIgogI2luY2x1ZGUgIkZvbnREZXNjcmlwdGlvbi5oIgor I2luY2x1ZGUgIkdVbmlxdWVQdHJHdGsuaCIKICNpbmNsdWRlICJHcmFkaWVudC5oIgogI2luY2x1 ZGUgIkdyYXBoaWNzQ29udGV4dC5oIgogI2luY2x1ZGUgIkd0a1ZlcnNpb25pbmcuaCIKQEAgLTIz NiwxMyArMjM3LDEyIEBAIHN0YXRpYyBHUmVmUHRyPEdka1BpeGJ1Zj4gZ2V0U3RvY2tTeW1ib2xp Y0ljb25Gb3JXaWRnZXRUeXBlKEdUeXBlIHdpZGdldFR5cGUsIGNvCiAKICAgICBndGtfc3R5bGVf Y29udGV4dF9zZXRfc3RhdGUoY29udGV4dCwgc3RhdGljX2Nhc3Q8R3RrU3RhdGVGbGFncz4oZmxh Z3MpKTsKICAgICBndGtfc3R5bGVfY29udGV4dF9zZXRfZGlyZWN0aW9uKGNvbnRleHQsIHN0YXRp Y19jYXN0PEd0a1RleHREaXJlY3Rpb24+KGRpcmVjdGlvbikpOwotICAgIEd0a0ljb25JbmZvKiBp bmZvID0gZ3RrX2ljb25fdGhlbWVfbG9va3VwX2ljb24oZ3RrX2ljb25fdGhlbWVfZ2V0X2RlZmF1 bHQoKSwgc3ltYm9saWNJY29uTmFtZSwgaWNvblNpemUsCi0gICAgICAgIHN0YXRpY19jYXN0PEd0 a0ljb25Mb29rdXBGbGFncz4oR1RLX0lDT05fTE9PS1VQX0ZPUkNFX1NWRyB8IEdUS19JQ09OX0xP T0tVUF9GT1JDRV9TSVpFKSk7Ci0gICAgR2RrUGl4YnVmKiBpY29uID0gMDsKLSAgICBpZiAoaW5m bykgewotICAgICAgICBpY29uID0gZ3RrX2ljb25faW5mb19sb2FkX3N5bWJvbGljX2Zvcl9jb250 ZXh0KGluZm8sIGNvbnRleHQsIDAsIDApOwotICAgICAgICBndGtfaWNvbl9pbmZvX2ZyZWUoaW5m byk7Ci0gICAgfQorCisgICAgR1VuaXF1ZVB0cjxHdGtJY29uSW5mbz4gaW5mbyhndGtfaWNvbl90 aGVtZV9sb29rdXBfaWNvbihndGtfaWNvbl90aGVtZV9nZXRfZGVmYXVsdCgpLCBzeW1ib2xpY0lj b25OYW1lLCBpY29uU2l6ZSwKKyAgICAgICAgc3RhdGljX2Nhc3Q8R3RrSWNvbkxvb2t1cEZsYWdz PihHVEtfSUNPTl9MT09LVVBfRk9SQ0VfU1ZHIHwgR1RLX0lDT05fTE9PS1VQX0ZPUkNFX1NJWkUp KSk7CisgICAgR2RrUGl4YnVmKiBpY29uID0gbnVsbHB0cjsKKyAgICBpZiAoaW5mbykKKyAgICAg ICAgaWNvbiA9IGd0a19pY29uX2luZm9fbG9hZF9zeW1ib2xpY19mb3JfY29udGV4dChpbmZvLmdl dCgpLCBjb250ZXh0LCBudWxscHRyLCBudWxscHRyKTsKIAogICAgIGd0a19zdHlsZV9jb250ZXh0 X3Jlc3RvcmUoY29udGV4dCk7CiAK