Bug 25109 - Eliminate CompositeAnimationPrivate
Summary: Eliminate CompositeAnimationPrivate
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
Depends on:
Reported: 2009-04-08 22:28 PDT by Simon Fraser (smfr)
Modified: 2009-04-09 09:30 PDT (History)
2 users (show)

See Also:

Patch, changelog (24.72 KB, patch)
2009-04-08 22:52 PDT, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-04-08 22:28:30 PDT
CompositeAnimationPrivate, in the CSS animation code, really doesn't provide any useful layering, and should be removed.
Comment 1 Simon Fraser (smfr) 2009-04-08 22:52:04 PDT
Created attachment 29359 [details]
Patch, changelog
Comment 2 Darin Adler 2009-04-09 08:53:43 PDT
Comment on attachment 29359 [details]
Patch, changelog

> +    m_compAnim->animationControllerPriv()->removeFromStyleAvailableWaitList(this);
> +    m_compAnim->animationControllerPriv()->removeFromStartTimeResponseWaitList(this);

The function name animationControllerPriv() is both long and abbreviated. And I find it ugly. Can you give it a shorter name without abbreviations? Or I guess you could use our standard name for such things, impl(), which I also find ugly.

Aside from this new function that needs a better name, r=me

There are quite a few things here that aren't using standard WebKit style, using "get" for example for simple getters, whereas we use nouns for getters and "get" only for getters that use out arguments. And this design with the private object that you get to explicitly is also pretty awkward.
Comment 3 Simon Fraser (smfr) 2009-04-09 09:08:42 PDT
I'd like to do the rename of getAnimatedStyle() (to computeAnimtedStyle() perhaps) in another patch, because that rename should extend outside of AnimationController.

I renamed animationControllerPriv() to just animationController(). I know that the normal term is just animation(), but that's very confusing in this context.
Comment 4 Simon Fraser (smfr) 2009-04-09 09:30:37 PDT