Bug 161552 - [Mac] RetainPtr misuse, AnimationController leaks
Summary: [Mac] RetainPtr misuse, AnimationController leaks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-02 15:12 PDT by Joseph Pecoraro
Modified: 2016-09-04 23:45 PDT (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (3.58 KB, patch)
2016-09-02 15:13 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-09-02 15:12:06 PDT
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.
Comment 1 Joseph Pecoraro 2016-09-02 15:13:04 PDT
Created attachment 287824 [details]
[PATCH] Proposed Fix
Comment 2 WebKit Commit Bot 2016-09-02 15:46:09 PDT
Comment on attachment 287824 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 287824

Committed r205378: <http://trac.webkit.org/changeset/205378>
Comment 3 WebKit Commit Bot 2016-09-02 15:46:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Darin Adler 2016-09-03 07:29:35 PDT
Comment on attachment 287824 [details]
[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.
Comment 5 Joseph Pecoraro 2016-09-04 23:45:04 PDT
Yeah, that was dumb! Dropped RetainPtr with:
https://trac.webkit.org/changeset/205427