Further tidy ups now that animations are ticked by their timelines

RESOLVED FIXED in Firefox 44

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: birtles, Assigned: birtles)

Tracking

Trunk
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

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]
This patch renames AnimationCollection::mNeedsRefreshes to indicate that it
no longer has any relationship to whether or not we observe the refresh driver.
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
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.
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.
(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.
Attachment #8668840 - Flags: review?(cam)
Attachment #8668841 - Flags: review?(cam)
Attachment #8668842 - Attachment is obsolete: true
Attachment #8668840 - Flags: review?(cam) → review+
Attachment #8668841 - Flags: review?(cam) → review+
Attachment #8669536 - Flags: review?(cam) → review+
You need to log in before you can comment on or make changes to this bug.