-
Notifications
You must be signed in to change notification settings - Fork 756
Description
The following test fails in a WebKit development branch I'm working on:
promise_test(t => {
const div = createDiv(t);
const animation = div.animate({}, 100 * MS_PER_SEC);
// Setup callback to run if finished promise is resolved
let finishPromiseResolved = false;
animation.finished.then(() => {
finishPromiseResolved = true;
});
return animation.ready.then(() => {
// Jump to mid-way in interval and pause
animation.currentTime = 100 * MS_PER_SEC / 2;
animation.pause();
// Jump to the end
animation.currentTime = 100 * MS_PER_SEC;
return waitForAnimationFrames(2);
}).then(() => {
assert_false(finishPromiseResolved,
'Finished promise should not resolve when pause-pending');
});
}, 'Finished promise does not resolve when pause-pending');The issue that I see is that the playState is no longer paused once we set currentTime to 100ms after the call to pause().
From my reading of the spec, when pause() is called the hold time is set in the pending pause task which is performed asynchronously. So when the call to pause returns, the hold time is unresolved and the start time is resolved.
Then we set the current time to 100ms and start by silently setting the current time. We then have these two options:
Update either animation’s hold time or start time as follows:
If any of the following conditions are true:
animation’s hold time is resolved, or
animation’s start time is unresolved, or
animation has no associated timeline or the associated timeline is inactive, or
animation’s playback rate is 0,
Set animation's hold time to seek time.
Otherwise,
Set animation's start time to the result of evaluatingtimeline time - (seek time / playback rate)where timeline time is the current time value of timeline associated with animation.
So we execute the "Otherwise" clause since none of the conditions of the first clause are true. I think the reason we check whether the hold time is resolved is to identify whether the animation is currently paused. If that is the case, then shouldn't we check its play state instead? This would account for the pending paused task.
As a result, currently, setting the start time will cancel the pending pause task and the finished promise will resolve when the finish notification steps are executed.
Note that letting the pending pause task run before updating the currentTime to 100ms makes the test work as expected:
promise_test(t => {
const div = createDiv(t);
const animation = div.animate({}, 100 * MS_PER_SEC);
// Setup callback to run if finished promise is resolved
let finishPromiseResolved = false;
animation.finished.then(() => {
finishPromiseResolved = true;
});
return animation.ready.then(() => {
// Jump to mid-way in interval and pause
animation.currentTime = 100 * MS_PER_SEC / 2;
animation.pause();
animation.ready.then(() => {
// Jump to the end
animation.currentTime = 100 * MS_PER_SEC;
});
return waitForAnimationFrames(2);
}).then(() => {
assert_false(finishPromiseResolved,
'Finished promise should not resolve when pause-pending');
});
}, 'Finished promise does not resolve when pause-pending');