[webkit-reviews] review granted: [Bug 46525] When animations are paused, play-state can cause them to be unpaused. : [Attachment 72698] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 46525: When animations are paused, play-state can cause them to be
unpaused.
https://bugs.webkit.org/show_bug.cgi?id=46525

Attachment 72698: Patch
https://bugs.webkit.org/attachment.cgi?id=72698&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=72698&action=review

> WebCore/ChangeLog:10
> +	   "running" and not suspended. Likewise pauses animation when either
> +	   flag is in the right state,

There's an incomplete sentence here.

> WebCore/page/animation/AnimationBase.cpp:1214
> +    bool wantToBePaused = playState == AnimPlayStatePaused ||
m_compAnim->suspended();

'wantToBePaused' confuses me. "shouldBePaused", or just "pause" perhaps?

> LayoutTests/animations/play-state-suspend.html:83
> +    runAnimationTest(expectedValues, setTimers, null, true);

Please annotate the parameters, or use variables to make the purposes of 
'null' and 'true' clear.