[Webkit-unassigned] [Bug 64851] [chromium]Optimize AnimationController::updateAnimations

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64851





--- Comment #4 from Dean Jackson <dino at apple.com>  2011-07-21 08:23:07 PST ---
(From update of attachment 101557)
View in context: https://bugs.webkit.org/attachment.cgi?id=101557&action=review

In general I don't like that this adds the parameter to setAnimatableStyle. Is there way the RenderObject can know if it is attached to the tree so that we don't have to pass the flag around? From a brief investigation this path is only hit when the RenderObject is not in the render tree (it shouldn't have a parent) - is this correct?

> Source/WebCore/ChangeLog:3
> +        [Bug 64851][chromium]Optimize AnimationController::updateAnimations

You should give this a better title. There is no need for [Bug 64851] and this isn't specific to [chromium].

> Source/WebCore/ChangeLog:7
> +

Similarly, you should put a description here of what the problem was and what you fixed.

> Source/WebCore/page/animation/AnimationController.cpp:479
> +PassRefPtr<RenderStyle> AnimationController::updateAnimations(RenderObject* renderer, RenderStyle* newStyle, bool InAttach)

The parameter name is pretty unclear to me. It should describe what it is doing within the function itself, not what the caller is using it for. Does that make sense? I mean that it should be something like timerShouldUpdate because that's how the updateAnimations function is using it.

Also, a style issue, the parameter name should not start with an uppercase character.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.