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+
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
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.