62163 2011-06-06 15:59:09 -0700 [chromium] PlatformContextSkia leaks SkDeviceFactory 2011-06-07 13:29:11 -0700 1 1 1 Unclassified WebKit Canvas 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 alokp alokp mdelaney7 reed senorblanco vangelis webkit.review.bot oldest_to_newest 416085 0 alokp 2011-06-06 15:59:09 -0700 I found the leak while debugging a crash in PlatformContextSkia::setSharedGraphicsContext3D. Note that the factory created here: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp?rev=86182#L745 is never unref'd. chromium bug: http://code.google.com/p/chromium/issues/detail?id=84758 416099 1 96146 alokp 2011-06-06 16:09:07 -0700 Created attachment 96146 proposed patch 416437 2 reed 2011-06-07 04:55:20 -0700 Seems ok, though I think a 1-liner fix could also just be from m_canvas->setDeviceFactory(factory); to m_canvas->setDeviceFactory(factory)->unref(); This matches the skia pattern, and matches the line above for the device. 416503 3 alokp 2011-06-07 07:53:31 -0700 (In reply to comment #2) > Seems ok, though I think a 1-liner fix could also just be > > from > m_canvas->setDeviceFactory(factory); > to > m_canvas->setDeviceFactory(factory)->unref(); > > This matches the skia pattern, and matches the line above for the device. Either way is fine with me. I used auto-ptr because there was a bunch of TODOs in the code to that effect and also the skia pattern does not work in case of GrTexture. 416720 4 96280 alokp 2011-06-07 12:18:30 -0700 Created attachment 96280 proposed patch Went with the one-line fix. This is probably not the right time make unnecessary changes. 416727 5 96280 senorblanco 2011-06-07 12:20:50 -0700 Comment on attachment 96280 proposed patch Looks good. R=me. 416756 6 96280 webkit.review.bot 2011-06-07 13:29:06 -0700 Comment on attachment 96280 proposed patch Clearing flags on attachment: 96280 Committed r88258: <http://trac.webkit.org/changeset/88258> 416757 7 webkit.review.bot 2011-06-07 13:29:11 -0700 All reviewed patches have been landed. Closing bug. 96146 2011-06-06 16:09:07 -0700 2011-06-07 12:18:30 -0700 proposed patch leak.patch text/plain 2819 alokp SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4MTk4KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMDYgIEFsb2sgUHJp eWFkYXJzaGkgIDxhbG9rcEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgW2Nocm9taXVtXSBQbGF0Zm9ybUNvbnRleHRTa2lhIGxl YWtzIFNrRGV2aWNlRmFjdG9yeQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NjIxNjMKKworICAgICAgICBObyBuZXcgdGVzdHMuIChPT1BTISkKKworICAg ICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3NraWEvUGxhdGZvcm1Db250ZXh0U2tpYS5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpQbGF0Zm9ybUNvbnRleHRTa2lhOjpzZXRTaGFyZWRHcmFwaGljc0Nv bnRleHQzRCk6CisKIDIwMTEtMDYtMDYgIEVtaWwgQSBFa2x1bmQgIDxlYWVAY2hyb21pdW0ub3Jn PgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpJbmRleDogU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9QbGF0Zm9ybUNvbnRleHRTa2lhLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lhL1BsYXRmb3JtQ29u dGV4dFNraWEuY3BwCShyZXZpc2lvbiA4ODE4OCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL3NraWEvUGxhdGZvcm1Db250ZXh0U2tpYS5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTcyMyw2ICs3MjMsNyBAQCB2b2lkIFBsYXRmb3JtQ29udGV4dFNraWE6OnNldFNoYXJlZEdyYXBo CiAgICAgbV9hY2NlbGVyYXRpb25Nb2RlID0gTm9BY2NlbGVyYXRpb247CiAjaWYgRU5BQkxFKEFD Q0VMRVJBVEVEXzJEX0NBTlZBUykKICAgICBpZiAoY29udGV4dCAmJiBkcmF3aW5nQnVmZmVyKSB7 CisgICAgICAgIEFTU0VSVChzaXplID09IGRyYXdpbmdCdWZmZXItPnNpemUoKSk7CiAgICAgICAg IG1fZ3B1Q2FudmFzID0gYWRvcHRQdHIobmV3IEdyYXBoaWNzQ29udGV4dEdQVShjb250ZXh0LCBk cmF3aW5nQnVmZmVyLCBzaXplKSk7CiAgICAgICAgIG1fdXBsb2FkVGV4dHVyZS5jbGVhcigpOwog ICAgICAgICBkcmF3aW5nQnVmZmVyLT5zZXRXaWxsUHVibGlzaENhbGxiYWNrKFdpbGxQdWJsaXNo Q2FsbGJhY2tJbXBsOjpjcmVhdGUodGhpcykpOwpAQCAtNzQwLDE3ICs3NDEsMTIgQEAgdm9pZCBQ bGF0Zm9ybUNvbnRleHRTa2lhOjpzZXRTaGFyZWRHcmFwaAogCiAgICAgICAgICAgICBHclBsYXRm b3JtU3VyZmFjZURlc2MgZHJhd0J1ZkRlc2M7CiAgICAgICAgICAgICBkcmF3aW5nQnVmZmVyLT5n ZXRHclBsYXRmb3JtU3VyZmFjZURlc2MoJmRyYXdCdWZEZXNjKTsKLSAgICAgICAgICAgIEdyVGV4 dHVyZSogZHJhd0J1ZlRleCA9IHN0YXRpY19jYXN0PEdyVGV4dHVyZSo+KGdyLT5jcmVhdGVQbGF0 Zm9ybVN1cmZhY2UoZHJhd0J1ZkRlc2MpKTsKLSAgICAgICAgICAgIC8vIEZJWE1FOiBUaGlzIHNo b3VsZCB1c2UgYSBzbWFydCBwb2ludGVyLgotICAgICAgICAgICAgU2tEZXZpY2VGYWN0b3J5KiBm YWN0b3J5ID0gbmV3IFNrR3B1RGV2aWNlRmFjdG9yeShnciwgZHJhd0J1ZlRleCk7Ci0gICAgICAg ICAgICAvLyBGSVhNRTogVGhpcyBzaG91bGQgdXNlIGEgc21hcnQgcG9pbnRlci4KLSAgICAgICAg ICAgIGRyYXdCdWZUZXgtPnVucmVmKCk7CisgICAgICAgICAgICBTa0F1dG9UVW5yZWY8R3JUZXh0 dXJlPiBkcmF3QnVmVGV4KHN0YXRpY19jYXN0PEdyVGV4dHVyZSo+KGdyLT5jcmVhdGVQbGF0Zm9y bVN1cmZhY2UoZHJhd0J1ZkRlc2MpKSk7CisgICAgICAgICAgICBTa0F1dG9UVW5yZWY8U2tEZXZp Y2VGYWN0b3J5PiBmYWN0b3J5KG5ldyBTa0dwdURldmljZUZhY3RvcnkoZ3IsIGRyYXdCdWZUZXgu Z2V0KCkpKTsKKyAgICAgICAgICAgIFNrQXV0b1RVbnJlZjxTa0RldmljZT4gZGV2aWNlKGZhY3Rv cnkuZ2V0KCktPm5ld0RldmljZSgwLCBTa0JpdG1hcDo6a0FSR0JfODg4OF9Db25maWcsIHNpemUu d2lkdGgoKSwgc2l6ZS5oZWlnaHQoKSwgZmFsc2UsIGZhbHNlKSk7CiAKLSAgICAgICAgICAgIC8v IEZJWE1FOiBUaGlzIHNob3VsZCB1c2UgYSBzbWFydCBwb2ludGVyLgotICAgICAgICAgICAgU2tE ZXZpY2UqIGRldmljZSA9IGZhY3RvcnktPm5ld0RldmljZShtX2NhbnZhcywgU2tCaXRtYXA6OmtB UkdCXzg4ODhfQ29uZmlnLCBkcmF3aW5nQnVmZmVyLT5zaXplKCkud2lkdGgoKSwgZHJhd2luZ0J1 ZmZlci0+c2l6ZSgpLmhlaWdodCgpLCBmYWxzZSwgZmFsc2UpOwotICAgICAgICAgICAgLy8gRklY TUU6IFRoaXMgc2hvdWxkIHVzZSBhIHNtYXJ0IHBvaW50ZXIuCi0gICAgICAgICAgICBtX2NhbnZh cy0+c2V0RGV2aWNlKGRldmljZSktPnVucmVmKCk7Ci0gICAgICAgICAgICBtX2NhbnZhcy0+c2V0 RGV2aWNlRmFjdG9yeShmYWN0b3J5KTsKKyAgICAgICAgICAgIG1fY2FudmFzLT5zZXREZXZpY2Uo ZGV2aWNlLmdldCgpKTsKKyAgICAgICAgICAgIG1fY2FudmFzLT5zZXREZXZpY2VGYWN0b3J5KGZh Y3RvcnkuZ2V0KCkpOwogICAgICAgICB9IGVsc2UKICAgICAgICAgICAgIG1fYWNjZWxlcmF0aW9u TW9kZSA9IEdQVTsKICAgICB9IGVsc2Ugewo= 96280 2011-06-07 12:18:30 -0700 2011-06-07 13:29:06 -0700 proposed patch leak.patch text/plain 1498 alokp SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4MjUyKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMDcgIEFsb2sgUHJp eWFkYXJzaGkgIDxhbG9rcEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgW2Nocm9taXVtXSBQbGF0Zm9ybUNvbnRleHRTa2lhIGxl YWtzIFNrRGV2aWNlRmFjdG9yeQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NjIxNjMKKworICAgICAgICBWYWxncmluZCB0ZXN0cyBzaG91bGQgaGF2ZSBj YXVnaHQgdGhpcy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3NraWEvUGxhdGZvcm1D b250ZXh0U2tpYS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQbGF0Zm9ybUNvbnRleHRTa2lhOjpz ZXRTaGFyZWRHcmFwaGljc0NvbnRleHQzRCk6CisKIDIwMTEtMDYtMDcgIEVtaWwgQSBFa2x1bmQg IDxlYWVAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpJ bmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9QbGF0Zm9ybUNvbnRl eHRTa2lhLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9za2lhL1BsYXRmb3JtQ29udGV4dFNraWEuY3BwCShyZXZpc2lvbiA4ODE4OCkKKysrIFNvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvUGxhdGZvcm1Db250ZXh0U2tpYS5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTc1MCw3ICs3NTAsNyBAQCB2b2lkIFBsYXRmb3JtQ29udGV4dFNr aWE6OnNldFNoYXJlZEdyYXBoCiAgICAgICAgICAgICBTa0RldmljZSogZGV2aWNlID0gZmFjdG9y eS0+bmV3RGV2aWNlKG1fY2FudmFzLCBTa0JpdG1hcDo6a0FSR0JfODg4OF9Db25maWcsIGRyYXdp bmdCdWZmZXItPnNpemUoKS53aWR0aCgpLCBkcmF3aW5nQnVmZmVyLT5zaXplKCkuaGVpZ2h0KCks IGZhbHNlLCBmYWxzZSk7CiAgICAgICAgICAgICAvLyBGSVhNRTogVGhpcyBzaG91bGQgdXNlIGEg c21hcnQgcG9pbnRlci4KICAgICAgICAgICAgIG1fY2FudmFzLT5zZXREZXZpY2UoZGV2aWNlKS0+ dW5yZWYoKTsKLSAgICAgICAgICAgIG1fY2FudmFzLT5zZXREZXZpY2VGYWN0b3J5KGZhY3Rvcnkp OworICAgICAgICAgICAgbV9jYW52YXMtPnNldERldmljZUZhY3RvcnkoZmFjdG9yeSktPnVucmVm KCk7CiAgICAgICAgIH0gZWxzZQogICAgICAgICAgICAgbV9hY2NlbGVyYXRpb25Nb2RlID0gR1BV OwogICAgIH0gZWxzZSB7Cg==