regression(111639): Issue with simultaneous CSS animations

[Simon Fraser (smfr)]

When there's an ongoing CSS animation with its iteration loop set to "infinite", adding a second CSS animation with "webkit-animation-fill-mode" set to "forwards" will always wait for the first animation's cycle to finish before starting to animate.

Steps to Reproduce
I have created a page that reproduces the issue here: http://samvermette.s3.amazonaws.com/safari.html

Expected Results
The blue square should always animate as soon as I hover the link.

Actual results
Except for the first time you hover the link, the blue square will always wait for the red square to finish its animation cycle before starting to animate.

Regression
Removing the "webkit-animation-fill-mode" property seems to fix the issue.

[Simon Fraser (smfr)]

<rdar://problem/11402993>

[Simon Fraser (smfr)]

Igor, please take a look.

[Dean Jackson]

Regressed with http://trac.webkit.org/changeset/111639/

[Dean Jackson]

Created attachment 140814 [details]
Testcase

[Igor Trindade Oliveira]

Looking

[Igor Trindade Oliveira]

After some time debugging i am removing the regression title because:

* I reverted http://trac.webkit.org/changeset/111639/ and the bug still there
* The test works when i run the test *without* accelerated composite;
* I tried to find one changeset where the bug does not happen, so i reverted my local branch for 3 months ago, and the bug already was there.

[Igor Trindade Oliveira]

My bad. It is a regression, however in Qt and GTK, AC had a bug(fixed in the changeset http://trac.webkit.org/changeset/116899) and reverting the changeset http://trac.webkit.org/changeset/111639/, the bug still was there.

[Igor Trindade Oliveira]

Created attachment 141827 [details]
Patch V1.

Proposed patch.

[Dean Jackson]

Comment on attachment 141827 [details]
Patch V1.

View in context: https://bugs.webkit.org/attachment.cgi?id=141827&action=review

> LayoutTests/animations/fill-mode-forwards.html:12
> +    function addPopAnimation () {

Space between function name and ()

> LayoutTests/animations/fill-mode-forwards.html:15
> +        popAnimationCounter = popAnimationCounter + 1;

Just popAnimationCounter++

> LayoutTests/animations/fill-mode-forwards.html:18
> +    function removePopAnimation () {

Ditto on the space.

> LayoutTests/animations/fill-mode-forwards.html:93
> +  <div id="pulsing-square" onwebkitanimationiteration="setTimeout(addPopAnimation(), 50);"
> +       onwebkitanimationend="pulsingSquareEnded()"
> +       ></div>

I think you can move the ></div> to the previous line.

> Source/WebCore/ChangeLog:10
> +        Currently, previousTimeToNextService is just saving the previous CompositeAnimation::timeToNextService
> +        for AnimationControllerPrivate::updateAnimationTimerForRenderer, however CompositeAnimation::timeToNextService
> +        is also called and used by AnimationControllerPrivate::updateAnimationTimer and we
> +        are not saving the previousTimeToNextService for AnimationControllerPrivate::updateAnimationTimer
> +        doing AnimationController have unexpected behavior.

This last line is not correct English. I think you could probably just change the bit after "used by AnimationControllerPrivate::updateAnimationTimer" to be "updateAnimationTimer. Make sure we save the existing timeToNextService from both places, updateAnimationTimerForRenderer and updateAnimationTimer." I don't think you need the class names (e.g. AnimationControllerPrivate::) here.

[Igor Trindade Oliveira]

Created attachment 142038 [details]
Final patch

[WebKit Review Bot]

Comment on attachment 142038 [details]
Final patch

Clearing flags on attachment: 142038

Committed r117216: <http://trac.webkit.org/changeset/117216>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.