161552 2016-09-02 15:12:06 -0700 [Mac] RetainPtr misuse, AnimationController leaks 2016-09-04 23:45:04 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck joepeck bdakin commit-queue joepeck thorton oldest_to_newest 1225983 0 joepeck 2016-09-02 15:12:06 -0700 RetainPtr misuse, Web/WKAnimationController leaks: $ ack 'RetainPtr.*?alloc' | grep -v adopt WebKit/mac/WebView/WebImmediateActionController.mm:260: RetainPtr<WebAnimationController> dummyController = [[WebAnimationController alloc] init]; WebKit2/UIProcess/mac/WKImmediateActionController.mm:274: RetainPtr<WKAnimationController> dummyController = [[WKAnimationController alloc] init]; These should be adopted on allocation. 1225984 1 287824 joepeck 2016-09-02 15:13:04 -0700 Created attachment 287824 [PATCH] Proposed Fix 1226001 2 287824 commit-queue 2016-09-02 15:46:09 -0700 Comment on attachment 287824 [PATCH] Proposed Fix Clearing flags on attachment: 287824 Committed r205378: <http://trac.webkit.org/changeset/205378> 1226002 3 commit-queue 2016-09-02 15:46:12 -0700 All reviewed patches have been landed. Closing bug. 1226120 4 287824 darin 2016-09-03 07:29:35 -0700 Comment on attachment 287824 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=287824&action=review > Source/WebKit/mac/WebView/WebImmediateActionController.mm:260 > - RetainPtr<WebAnimationController> dummyController = [[WebAnimationController alloc] init]; > - return dummyController.get(); > + RetainPtr<WebAnimationController> dummyController = adoptNS([[WebAnimationController alloc] init]); > + return dummyController.autorelease(); This should just be: return [[[WebAnimationController alloc] init] autorelease]; The RetainPtr does us no good at all. > Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm:274 > - RetainPtr<WKAnimationController> dummyController = [[WKAnimationController alloc] init]; > - return dummyController.get(); > + RetainPtr<WKAnimationController> dummyController = adoptNS([[WKAnimationController alloc] init]); > + return dummyController.autorelease(); Ditto. 1226360 5 joepeck 2016-09-04 23:45:04 -0700 Yeah, that was dumb! Dropped RetainPtr with: https://trac.webkit.org/changeset/205427 287824 2016-09-02 15:13:04 -0700 2016-09-02 15:46:09 -0700 [PATCH] Proposed Fix animation-controller-leaks-1.patch text/plain 3663 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZwppbmRleCBmY2M4YjhmLi40YTIxMDg1IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L21hYy9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTMgQEAKKzIwMTYtMDktMDIgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFw cGxlLmNvbT4KKworICAgICAgICBbTWFjXSBSZXRhaW5QdHIgbWlzdXNlLCBBbmltYXRpb25Db250 cm9sbGVyIGxlYWtzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNjE1NTIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIFdlYlZpZXcvV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlci5tbToKKyAgICAgICAg KC1bV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlciBfZGVmYXVsdEFuaW1hdGlvbkNvbnRyb2xs ZXJdKToKKwogMjAxNi0wNy0wOCAgSmVyIE5vYmxlICA8amVyLm5vYmxlQGFwcGxlLmNvbT4KIAog ICAgICAgICBSZWZhY3RvciBXZWJQbGF5YmFja1Nlc3Npb25Nb2RlbE1lZGlhRWxlbWVudCB0byBi ZSBjbGllbnQgYmFzZWQuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dl YkltbWVkaWF0ZUFjdGlvbkNvbnRyb2xsZXIubW0gYi9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3 L1dlYkltbWVkaWF0ZUFjdGlvbkNvbnRyb2xsZXIubW0KaW5kZXggY2I0ZjM3Yy4uOWQyNTA2ZiAx MDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJJbW1lZGlhdGVBY3Rpb25D b250cm9sbGVyLm1tCisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL1dlYlZpZXcvV2ViSW1tZWRpYXRl QWN0aW9uQ29udHJvbGxlci5tbQpAQCAtNjMsOCArNjMsNyBAQCBTT0ZUX0xJTktfQ0xBU1MoUXVp Y2tMb29rVUksIFFMUHJldmlld01lbnVJdGVtKQogQGludGVyZmFjZSBXZWJJbW1lZGlhdGVBY3Rp b25Db250cm9sbGVyICgpIDxRTFByZXZpZXdNZW51SXRlbURlbGVnYXRlPgogQGVuZAogCi1AaW50 ZXJmYWNlIFdlYkFuaW1hdGlvbkNvbnRyb2xsZXIgOiBOU09iamVjdCA8TlNJbW1lZGlhdGVBY3Rp b25BbmltYXRpb25Db250cm9sbGVyPiB7Ci19CitAaW50ZXJmYWNlIFdlYkFuaW1hdGlvbkNvbnRy b2xsZXIgOiBOU09iamVjdCA8TlNJbW1lZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9sbGVyPgog QGVuZAogCiBAaW1wbGVtZW50YXRpb24gV2ViQW5pbWF0aW9uQ29udHJvbGxlcgpAQCAtMjU3LDgg KzI1Niw4IEBAIHVzaW5nIG5hbWVzcGFjZSBXZWJDb3JlOwogLSAoaWQgPE5TSW1tZWRpYXRlQWN0 aW9uQW5pbWF0aW9uQ29udHJvbGxlcj4pX2RlZmF1bHRBbmltYXRpb25Db250cm9sbGVyCiB7CiAg ICAgaWYgKF9jb250ZW50UHJldmVudHNEZWZhdWx0KSB7Ci0gICAgICAgIFJldGFpblB0cjxXZWJB bmltYXRpb25Db250cm9sbGVyPiBkdW1teUNvbnRyb2xsZXIgPSBbW1dlYkFuaW1hdGlvbkNvbnRy b2xsZXIgYWxsb2NdIGluaXRdOwotICAgICAgICByZXR1cm4gZHVtbXlDb250cm9sbGVyLmdldCgp OworICAgICAgICBSZXRhaW5QdHI8V2ViQW5pbWF0aW9uQ29udHJvbGxlcj4gZHVtbXlDb250cm9s bGVyID0gYWRvcHROUyhbW1dlYkFuaW1hdGlvbkNvbnRyb2xsZXIgYWxsb2NdIGluaXRdKTsKKyAg ICAgICAgcmV0dXJuIGR1bW15Q29udHJvbGxlci5hdXRvcmVsZWFzZSgpOwogICAgIH0KIAogICAg IE5TVVJMICp1cmwgPSBfaGl0VGVzdFJlc3VsdC5hYnNvbHV0ZUxpbmtVUkwoKTsKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpp bmRleCBiMGFkMDlkLi4wM2NkYWZhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VM b2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTUgQEAKIDIwMTYt MDktMDIgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAorICAgICAgICBb TWFjXSBSZXRhaW5QdHIgbWlzdXNlLCBBbmltYXRpb25Db250cm9sbGVyIGxlYWtzCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjE1NTIKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9tYWMv V0tJbW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tOgorICAgICAgICAoLVtXS0ltbWVkaWF0ZUFj dGlvbkNvbnRyb2xsZXIgX2RlZmF1bHRBbmltYXRpb25Db250cm9sbGVyXSk6CisKKzIwMTYtMDkt MDIgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKwogICAgICAgICBbTWFj XSBSZXRhaW5QdHIgbWlzdXNlLCBEREFjdGlvbkNvbnRleHQgbGVha3MKICAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2MTU1MQogCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1dLSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlci5t bSBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV0tJbW1lZGlhdGVBY3Rpb25Db250cm9s bGVyLm1tCmluZGV4IGYyZDdhOTIuLjBiMzc5YjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQy L1VJUHJvY2Vzcy9tYWMvV0tJbW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tCisrKyBiL1NvdXJj ZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV0tJbW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tCkBA IC01NSw4ICs1NSw3IEBAIHVzaW5nIG5hbWVzcGFjZSBXZWJLaXQ7CiBAaW50ZXJmYWNlIFdLSW1t ZWRpYXRlQWN0aW9uQ29udHJvbGxlciAoKSA8UUxQcmV2aWV3TWVudUl0ZW1EZWxlZ2F0ZT4KIEBl bmQKIAotQGludGVyZmFjZSBXS0FuaW1hdGlvbkNvbnRyb2xsZXIgOiBOU09iamVjdCA8TlNJbW1l ZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9sbGVyPiB7Ci19CitAaW50ZXJmYWNlIFdLQW5pbWF0 aW9uQ29udHJvbGxlciA6IE5TT2JqZWN0IDxOU0ltbWVkaWF0ZUFjdGlvbkFuaW1hdGlvbkNvbnRy b2xsZXI+CiBAZW5kCiAKIEBpbXBsZW1lbnRhdGlvbiBXS0FuaW1hdGlvbkNvbnRyb2xsZXIKQEAg LTI3MSw4ICsyNzAsOCBAQCB1c2luZyBuYW1lc3BhY2UgV2ViS2l0OwogLSAoaWQgPE5TSW1tZWRp YXRlQWN0aW9uQW5pbWF0aW9uQ29udHJvbGxlcj4pX2RlZmF1bHRBbmltYXRpb25Db250cm9sbGVy CiB7CiAgICAgaWYgKF9jb250ZW50UHJldmVudHNEZWZhdWx0KSB7Ci0gICAgICAgIFJldGFpblB0 cjxXS0FuaW1hdGlvbkNvbnRyb2xsZXI+IGR1bW15Q29udHJvbGxlciA9IFtbV0tBbmltYXRpb25D b250cm9sbGVyIGFsbG9jXSBpbml0XTsKLSAgICAgICAgcmV0dXJuIGR1bW15Q29udHJvbGxlci5n ZXQoKTsKKyAgICAgICAgUmV0YWluUHRyPFdLQW5pbWF0aW9uQ29udHJvbGxlcj4gZHVtbXlDb250 cm9sbGVyID0gYWRvcHROUyhbW1dLQW5pbWF0aW9uQ29udHJvbGxlciBhbGxvY10gaW5pdF0pOwor ICAgICAgICByZXR1cm4gZHVtbXlDb250cm9sbGVyLmF1dG9yZWxlYXNlKCk7CiAgICAgfQogCiAg ICAgUmVmUHRyPEFQSTo6SGl0VGVzdFJlc3VsdD4gaGl0VGVzdFJlc3VsdCA9IFtzZWxmIF93ZWJI aXRUZXN0UmVzdWx0XTsK