1208938 - Further tidy ups now that animations are ticked by their timelines

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

In bug 1195180 there were some further clean up patches that we didn't land due to bug 1208385. Once bug 1208385 is fixed, we should land those patches, specifically:

* attachment 8662222 [details] [diff] [review]
* attachment 8662223 [details] [diff] [review]
* attachment 8662781 [details] [diff] [review]

Comment 1

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: part 1 - Rename AnimationCollection::mNeedsRefreshes to mStyleChanging

This patch renames AnimationCollection::mNeedsRefreshes to indicate that it
no longer has any relationship to whether or not we observe the refresh driver.

Comment 2

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: part 2 - Remove Animation::HasEndEventToQueue

Comment 3

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: part 3 - Update pending finishing handling

Animation::Tick contains special handling to cope with pending ready times
that are in the future. This was originally introduced to cope with the
situation where we are called multiple times per refresh-driver tick.

As of bug 1195180, Animation::Tick should no longer be called multiple
times per refresh driver tick. It would seem, therefore, that we no longer
need to check for a future time. However, since introducing this check, the
vsync refresh driver timer has been added which means that we can still have
a recorded time from TimeStamp::Now that is ahead of the vsync time used to
update the refresh driver. In that case, however, rather than waiting for the
next tick, we should simply clamp that pending ready time to the refresh driver
time and finish pending immediately.

Comment 4

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Not putting this up for review yet because I seem to sometimes hit the following error on try on Linux:

dom/animation/test/css-animations/test_animation-reverse.html | reverse() inverts playbackRate - reverse() inverts playbackRate: An attempt was made to use an object that is not, or is no longer, usable

e.g. http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-59049fd644f2/try-linux/try_ubuntu32_vm_test-mochitest-1-bm04-tests1-linux32-build1851.txt.gz

It seems to be triggered by part 3 but I've no idea how that could cause it.

Comment 5

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Brian Birtles (:birtles) from comment #4)
> Not putting this up for review yet because I seem to sometimes hit the
> following error on try on Linux:
> 
> dom/animation/test/css-animations/test_animation-reverse.html | reverse()
> inverts playbackRate - reverse() inverts playbackRate: An attempt was made
> to use an object that is not, or is no longer, usable
> 
> e.g.
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bbirtles@mozilla.
> com-59049fd644f2/try-linux/try_ubuntu32_vm_test-mochitest-1-bm04-tests1-
> linux32-build1851.txt.gz
> 
> It seems to be triggered by part 3 but I've no idea how that could cause it.

I didn't realise this message is simply what we print when an unhandled InvalidStateError exception is shown.

I'm pretty sure this bug is just because we sometimes end up calling reverse() on an infinite animation while currentTime is still 0. That wouldn't happen before since we'd wait longer to start the animation. We simply need to wait an extra frame before reversing.

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: part 3 - Update pending finishing handling

Comment 7

[Brian Birtles (:birtles, Feb 26~ parental leave)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3b28977e41

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89ddaac28785
https://hg.mozilla.org/integration/mozilla-inbound/rev/05f79b5c5ef9
https://hg.mozilla.org/integration/mozilla-inbound/rev/649b90ba9363

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/89ddaac28785
https://hg.mozilla.org/mozilla-central/rev/05f79b5c5ef9
https://hg.mozilla.org/mozilla-central/rev/649b90ba9363