Commit c48e746a authored by Kevin Ellis's avatar Kevin Ellis Committed by Commit Bot

Fix timing of animation finished event and promise.

Previously a finished animation would fire the finished event on the next animation tick.  This is not to specification, as the finished event should trigger immediately (https://drafts.csswg.org/web-animations/#finishing-an-animation-section).

The discrepancy with the spec manifested as test failures when the state of an animation is updated after finishing. In some cases, the animation no longer registers as finished at the time of the next tick and the finished event is dropped.

Bug: 772060, 772048, 771977
Change-Id: Ie4161face4e6d5ed9b07bee86fef68b718233d41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1572803Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653584}
parent 03c47f84
...@@ -648,7 +648,11 @@ void Animation::reverse(ExceptionState& exception_state) { ...@@ -648,7 +648,11 @@ void Animation::reverse(ExceptionState& exception_state) {
play(exception_state); play(exception_state);
} }
// https://drafts.csswg.org/web-animations/#finishing-an-animation-section
void Animation::finish(ExceptionState& exception_state) { void Animation::finish(ExceptionState& exception_state) {
// Force resolution of PlayStateUpdateScope to enable immediate queuing of
// the finished event.
{
PlayStateUpdateScope update_scope(*this, kTimingUpdateOnDemand); PlayStateUpdateScope update_scope(*this, kTimingUpdateOnDemand);
if (!playback_rate_) { if (!playback_rate_) {
...@@ -675,7 +679,9 @@ void Animation::finish(ExceptionState& exception_state) { ...@@ -675,7 +679,9 @@ void Animation::finish(ExceptionState& exception_state) {
current_time_pending_ = false; current_time_pending_ = false;
start_time_ = CalculateStartTime(new_current_time); start_time_ = CalculateStartTime(new_current_time);
play_state_ = kFinished; play_state_ = kFinished;
ForceServiceOnNextFrame(); }
// Resolve finished event immediately.
QueueFinishedEvent();
} }
ScriptPromise Animation::finished(ScriptState* script_state) { ScriptPromise Animation::finished(ScriptState* script_state) {
...@@ -979,24 +985,26 @@ bool Animation::Update(TimingUpdateReason reason) { ...@@ -979,24 +985,26 @@ bool Animation::Update(TimingUpdateReason reason) {
pending_cancelled_event_); pending_cancelled_event_);
} }
} else { } else {
QueueFinishedEvent();
}
finished_ = true;
}
}
DCHECK(!outdated_);
return !finished_ || std::isfinite(TimeToEffectChange());
}
void Animation::QueueFinishedEvent() {
const AtomicString& event_type = event_type_names::kFinish; const AtomicString& event_type = event_type_names::kFinish;
if (GetExecutionContext() && HasEventListeners(event_type)) { if (GetExecutionContext() && HasEventListeners(event_type)) {
double event_current_time = CurrentTimeInternal() * 1000; double event_current_time = CurrentTimeInternal() * 1000;
pending_finished_event_ = pending_finished_event_ = MakeGarbageCollected<AnimationPlaybackEvent>(
MakeGarbageCollected<AnimationPlaybackEvent>( event_type, event_current_time, TimelineInternal()->currentTime());
event_type, event_current_time,
TimelineInternal()->currentTime());
pending_finished_event_->SetTarget(this); pending_finished_event_->SetTarget(this);
pending_finished_event_->SetCurrentTarget(this); pending_finished_event_->SetCurrentTarget(this);
timeline_->GetDocument()->EnqueueAnimationFrameEvent( timeline_->GetDocument()->EnqueueAnimationFrameEvent(
pending_finished_event_); pending_finished_event_);
} }
}
finished_ = true;
}
}
DCHECK(!outdated_);
return !finished_ || std::isfinite(TimeToEffectChange());
} }
void Animation::UpdateIfNecessary() { void Animation::UpdateIfNecessary() {
......
...@@ -272,6 +272,8 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData, ...@@ -272,6 +272,8 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData,
void RejectAndResetPromise(AnimationPromise*); void RejectAndResetPromise(AnimationPromise*);
void RejectAndResetPromiseMaybeAsync(AnimationPromise*); void RejectAndResetPromiseMaybeAsync(AnimationPromise*);
void QueueFinishedEvent();
String id_; String id_;
AnimationPlayState play_state_; AnimationPlayState play_state_;
......
...@@ -2729,8 +2729,6 @@ crbug.com/453002 [ Win ] fast/text/justify-ideograph-vertical.html [ Failure Pas ...@@ -2729,8 +2729,6 @@ crbug.com/453002 [ Win ] fast/text/justify-ideograph-vertical.html [ Failure Pas
crbug.com/453002 [ Win ] fast/text/orientation-sideways.html [ Failure Pass ] crbug.com/453002 [ Win ] fast/text/orientation-sideways.html [ Failure Pass ]
crbug.com/600248 external/wpt/web-animations/interfaces/Animation/oncancel.html [ Pass Failure ] crbug.com/600248 external/wpt/web-animations/interfaces/Animation/oncancel.html [ Pass Failure ]
crbug.com/771977 external/wpt/web-animations/interfaces/Animation/onfinish.html [ Failure ]
crbug.com/772048 crbug.com/772060 external/wpt/web-animations/timing-model/animations/updating-the-finished-state.html [ Timeout ]
crbug.com/771722 external/wpt/web-animations/timing-model/animations/the-current-time-of-an-animation.html [ Failure ] crbug.com/771722 external/wpt/web-animations/timing-model/animations/the-current-time-of-an-animation.html [ Failure ]
crbug.com/771722 crbug.com/771751 crbug.com/772060 external/wpt/web-animations/timing-model/animations/setting-the-start-time-of-an-animation.html [ Failure ] crbug.com/771722 crbug.com/771751 crbug.com/772060 external/wpt/web-animations/timing-model/animations/setting-the-start-time-of-an-animation.html [ Failure ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment