Bug 1291413 - Fix assertion when resuming an Animation with both a start time and hold time; r?hiro
When we set the playback rate to zero on a play-pending animation that is
resuming from an aborted pause we can arrive in a state where both the start
time and hold time are resolved. However, we previously added an assertion that
only one of these is ever set at a time.
Part of the assertion is warranted since that method contains the following
code:
if (mStartTime.IsNull()) {
mStartTime = StartTimeFromReadyTime(aReadyTime);
if (mPlaybackRate != 0) {
mHoldTime.SetNull();
}
}
Here StartTimeFromReadyTime requires a non-null hold time. So either mStartTime
or mHoldTime needs to be non-null. The requirement that only one or the other be
non-null, however, is not in the spec and not necessary (as the test cases in
this bug show).
What this assertion does bring to light, however, is that in the case where we
have *both* the start time and the hold time, we need to consider whether to use
the start time as-is, or calculate it from the hold time.
I have filed the following spec issue for this:
https://github.com/w3c/web-animations/issues/200
MozReview-Commit-ID: CTCT7Up1E5n
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -1183,19 +1183,19 @@ Animation::PauseNoUpdate(ErrorResult& aR
void
Animation::ResumeAt(const TimeDuration& aReadyTime)
{
// This method is only expected to be called for an animation that is
// waiting to play. We can easily adapt it to handle other states
// but it's currently not necessary.
MOZ_ASSERT(mPendingState == PendingState::PlayPending,
"Expected to resume a play-pending animation");
- MOZ_ASSERT(mHoldTime.IsNull() != mStartTime.IsNull(),
+ MOZ_ASSERT(!mHoldTime.IsNull() || !mStartTime.IsNull(),
"An animation in the play-pending state should have either a"
- " resolved hold time or resolved start time (but not both)");
+ " resolved hold time or resolved start time");
// If we aborted a pending pause operation we will already have a start time
// we should use. In all other cases, we resolve it from the ready time.
if (mStartTime.IsNull()) {
mStartTime = StartTimeFromReadyTime(aReadyTime);
if (mPlaybackRate != 0) {
mHoldTime.SetNull();
}
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1291413-1.html
@@ -0,0 +1,20 @@
+<!doctype html>
+<html class=reftest-wait>
+<script>
+window.onload = () => {
+ const div = document.createElement('div');
+
+ document.documentElement.appendChild(div);
+ const anim = div.animate([], 1000);
+
+ anim.ready.then(() => {
+ anim.pause();
+ anim.reverse();
+ anim.playbackRate = 0;
+ anim.ready.then(() => {
+ document.documentElement.className = '';
+ });
+ });
+};
+</script>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1291413-2.html
@@ -0,0 +1,21 @@
+<!doctype html>
+<html class=reftest-wait>
+<script>
+window.onload = () => {
+ const div = document.createElement('div');
+
+ document.documentElement.appendChild(div);
+ const anim = div.animate([], 1000);
+
+ anim.ready.then(() => {
+ anim.pause();
+ anim.reverse();
+ anim.playbackRate = 0;
+ anim.playbackRate = 1;
+ anim.ready.then(() => {
+ document.documentElement.className = '';
+ });
+ });
+};
+</script>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -5,16 +5,18 @@ pref(dom.animations-api.core.enabled,tru
pref(dom.animations-api.core.enabled,true) load 1216842-3.html
pref(dom.animations-api.core.enabled,true) load 1216842-4.html
pref(dom.animations-api.core.enabled,true) load 1216842-5.html
pref(dom.animations-api.core.enabled,true) load 1216842-6.html
pref(dom.animations-api.core.enabled,true) load 1272475-1.html
pref(dom.animations-api.core.enabled,true) load 1272475-2.html
pref(dom.animations-api.core.enabled,true) load 1278485-1.html
pref(dom.animations-api.core.enabled,true) load 1277272-1.html
+pref(dom.animations-api.core.enabled,true) load 1291413-1.html
+pref(dom.animations-api.core.enabled,true) load 1291413-2.html
pref(dom.animations-api.core.enabled,true) load 1304886-1.html
pref(dom.animations-api.core.enabled,true) load 1322382-1.html
pref(dom.animations-api.core.enabled,true) load 1322291-1.html
pref(dom.animations-api.core.enabled,true) load 1322291-2.html
pref(dom.animations-api.core.enabled,true) load 1323114-1.html
pref(dom.animations-api.core.enabled,true) load 1323114-2.html
pref(dom.animations-api.core.enabled,true) load 1324554-1.html
pref(dom.animations-api.core.enabled,true) load 1325193-1.html
