WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
52904
PlatformCAAnimationWin is leaky and inefficient
https://bugs.webkit.org/show_bug.cgi?id=52904
Summary
PlatformCAAnimationWin is leaky and inefficient
Adam Roben (:aroben)
Reported
2011-01-21 09:38:58 PST
PlatformCAAnimationWin is leaky and inefficient
Attachments
Clean up PlatformCAAnimationWin
(7.60 KB, patch)
2011-01-21 09:40 PST
,
Adam Roben (:aroben)
simon.fraser
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1
2011-01-21 09:40:28 PST
Created
attachment 79752
[details]
Clean up PlatformCAAnimationWin
Simon Fraser (smfr)
Comment 2
2011-01-21 10:02:50 PST
Comment on
attachment 79752
[details]
Clean up PlatformCAAnimationWin View in context:
https://bugs.webkit.org/attachment.cgi?id=79752&action=review
> Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:125 > -static CACFTimingFunctionRef toCACFTimingFunction(const TimingFunction* timingFunction) > +static RetainPtr<CACFTimingFunctionRef> toCACFTimingFunction(const TimingFunction* timingFunction)
Do we use the create moniker for things that return RetainPtrs?
Adam Roben (:aroben)
Comment 3
2011-01-21 10:09:06 PST
Comment on
attachment 79752
[details]
Clean up PlatformCAAnimationWin View in context:
https://bugs.webkit.org/attachment.cgi?id=79752&action=review
>> Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:125 >> +static RetainPtr<CACFTimingFunctionRef> toCACFTimingFunction(const TimingFunction* timingFunction) > > Do we use the create moniker for things that return RetainPtrs?
I don't think we do. Since there's no way to incorrectly handle the lifetime when a RetainPtr is returned, there's no need to use the "Create" pattern.
Darin Adler
Comment 4
2011-01-21 10:32:20 PST
(In reply to
comment #3
)
> >> +static RetainPtr<CACFTimingFunctionRef> toCACFTimingFunction(const TimingFunction* timingFunction) > > > > Do we use the create moniker for things that return RetainPtrs? > > I don't think we do.
I think sometimes we do. I’d have to look to find examples, but I believe they exist.
> Since there's no way to incorrectly handle the lifetime when a RetainPtr is returned, there's no need to use the "Create" pattern.
I agree that you don’t need Create/Copy just to avoid basic reference counting mistakes with smart pointers, but I do still think a name that alludes to the fact that it’s a new object and so can be costly for memory might be helpful. So I’d use Create but not Copy in names. And it’s not going to be as precise as the “this gives you a reference you need to balance with release or adopt” rule.
Adam Roben (:aroben)
Comment 5
2011-01-21 10:32:32 PST
Committed
r76358
: <
http://trac.webkit.org/changeset/76358
>
Adam Roben (:aroben)
Comment 6
2011-01-21 10:34:40 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > Since there's no way to incorrectly handle the lifetime when a RetainPtr is returned, there's no need to use the "Create" pattern. > > I agree that you don’t need Create/Copy just to avoid basic reference counting mistakes with smart pointers, but I do still think a name that alludes to the fact that it’s a new object and so can be costly for memory might be helpful. So I’d use Create but not Copy in names. And it’s not going to be as precise as the “this gives you a reference you need to balance with release or adopt” rule.
This function sometimes creates a new object and sometimes returns an existing object, but we could probably still come up with an accurate name.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug