Commit 83ed3e32 authored by Kevin Ellis's avatar Kevin Ellis Committed by Commit Bot

Fix firing of finished event for animations with an end delay.

If an animation has a positive-valued end delay, we need an additional
tick at the end time to ensure that the finished event is delivered.
Otherwise we stop ticking the animation at the end of the active phase
and the limit is not reached until triggered by another event such as
a mouse move|click.

Bug: 840637
Change-Id: Id811c787833a34f8739b2c10dc2af5b19fa4d17b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634387Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665135}
parent 38d4182b
...@@ -755,45 +755,55 @@ TEST_F(AnimationAnimationTest, AnimationsReturnTimeToNextEffect) { ...@@ -755,45 +755,55 @@ TEST_F(AnimationAnimationTest, AnimationsReturnTimeToNextEffect) {
animation = timeline->Play(keyframe_effect); animation = timeline->Play(keyframe_effect);
animation->setStartTime(0, false); animation->setStartTime(0, false);
// Next effect change at end of start delay.
SimulateFrame(0); SimulateFrame(0);
EXPECT_EQ(1, animation->TimeToEffectChange()); EXPECT_EQ(1, animation->TimeToEffectChange());
// Next effect change at end of start delay.
SimulateFrame(0.5); SimulateFrame(0.5);
EXPECT_EQ(0.5, animation->TimeToEffectChange()); EXPECT_EQ(0.5, animation->TimeToEffectChange());
// Start of active phase.
SimulateFrame(1); SimulateFrame(1);
EXPECT_EQ(0, animation->TimeToEffectChange()); EXPECT_EQ(0, animation->TimeToEffectChange());
// Still in active phase.
SimulateFrame(1.5); SimulateFrame(1.5);
EXPECT_EQ(0, animation->TimeToEffectChange()); EXPECT_EQ(0, animation->TimeToEffectChange());
// Start of the after phase. Next effect change at end of after phase.
SimulateFrame(2); SimulateFrame(2);
EXPECT_EQ(std::numeric_limits<double>::infinity(), EXPECT_EQ(1, animation->TimeToEffectChange());
animation->TimeToEffectChange());
// Still in effect if fillmode = forward|both.
SimulateFrame(3); SimulateFrame(3);
EXPECT_EQ(std::numeric_limits<double>::infinity(), EXPECT_EQ(std::numeric_limits<double>::infinity(),
animation->TimeToEffectChange()); animation->TimeToEffectChange());
// Reset to start of animation. Next effect at the end of the start delay.
animation->SetCurrentTimeInternal(0); animation->SetCurrentTimeInternal(0);
SimulateFrame(3); SimulateFrame(3);
EXPECT_EQ(1, animation->TimeToEffectChange()); EXPECT_EQ(1, animation->TimeToEffectChange());
// Start delay is scaled by playback rate.
animation->setPlaybackRate(2); animation->setPlaybackRate(2);
SimulateFrame(3); SimulateFrame(3);
EXPECT_EQ(0.5, animation->TimeToEffectChange()); EXPECT_EQ(0.5, animation->TimeToEffectChange());
// Effectively a paused animation.
animation->setPlaybackRate(0); animation->setPlaybackRate(0);
animation->Update(kTimingUpdateOnDemand); animation->Update(kTimingUpdateOnDemand);
EXPECT_EQ(std::numeric_limits<double>::infinity(), EXPECT_EQ(std::numeric_limits<double>::infinity(),
animation->TimeToEffectChange()); animation->TimeToEffectChange());
// Reversed animation from end time. Next effect after end delay.
animation->SetCurrentTimeInternal(3); animation->SetCurrentTimeInternal(3);
animation->setPlaybackRate(-1); animation->setPlaybackRate(-1);
animation->Update(kTimingUpdateOnDemand); animation->Update(kTimingUpdateOnDemand);
SimulateFrame(3); SimulateFrame(3);
EXPECT_EQ(1, animation->TimeToEffectChange()); EXPECT_EQ(1, animation->TimeToEffectChange());
// End delay is scaled by playback rate.
animation->setPlaybackRate(-2); animation->setPlaybackRate(-2);
animation->Update(kTimingUpdateOnDemand); animation->Update(kTimingUpdateOnDemand);
SimulateFrame(3); SimulateFrame(3);
......
...@@ -460,11 +460,14 @@ double KeyframeEffect::CalculateTimeToEffectChange( ...@@ -460,11 +460,14 @@ double KeyframeEffect::CalculateTimeToEffectChange(
return 0; return 0;
case kPhaseAfter: case kPhaseAfter:
DCHECK_GE(local_time, after_time); DCHECK_GE(local_time, after_time);
// If this KeyframeEffect is still in effect then it will need to update if (forwards) {
// when its parent goes out of effect. We have no way of knowing when // If an animation has a positive-valued end delay, we need an
// that will be, however, so the parent will need to supply it. // additional tick at the end time to ensure that the finished event is
return forwards ? std::numeric_limits<double>::infinity() // delivered.
: local_time - after_time; return end_time > local_time ? end_time - local_time
: std::numeric_limits<double>::infinity();
}
return local_time - after_time;
default: default:
NOTREACHED(); NOTREACHED();
return std::numeric_limits<double>::infinity(); return std::numeric_limits<double>::infinity();
......
...@@ -346,22 +346,26 @@ TEST_F(KeyframeEffectTest, TimeToEffectChange) { ...@@ -346,22 +346,26 @@ TEST_F(KeyframeEffectTest, TimeToEffectChange) {
Animation* animation = GetDocument().Timeline().Play(keyframe_effect); Animation* animation = GetDocument().Timeline().Play(keyframe_effect);
double inf = std::numeric_limits<double>::infinity(); double inf = std::numeric_limits<double>::infinity();
// Beginning of the animation.
EXPECT_EQ(100, keyframe_effect->TimeToForwardsEffectChange()); EXPECT_EQ(100, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(inf, keyframe_effect->TimeToReverseEffectChange()); EXPECT_EQ(inf, keyframe_effect->TimeToReverseEffectChange());
// End of the before phase.
animation->SetCurrentTimeInternal(100); animation->SetCurrentTimeInternal(100);
EXPECT_EQ(100, keyframe_effect->TimeToForwardsEffectChange()); EXPECT_EQ(100, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange()); EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange());
// Nearing the end of the active phase.
animation->SetCurrentTimeInternal(199); animation->SetCurrentTimeInternal(199);
EXPECT_EQ(1, keyframe_effect->TimeToForwardsEffectChange()); EXPECT_EQ(1, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange()); EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange());
// End of the active phase.
animation->SetCurrentTimeInternal(200); animation->SetCurrentTimeInternal(200);
// End-exclusive. EXPECT_EQ(100, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(inf, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange()); EXPECT_EQ(0, keyframe_effect->TimeToReverseEffectChange());
// End of the animation.
animation->SetCurrentTimeInternal(300); animation->SetCurrentTimeInternal(300);
EXPECT_EQ(inf, keyframe_effect->TimeToForwardsEffectChange()); EXPECT_EQ(inf, keyframe_effect->TimeToForwardsEffectChange());
EXPECT_EQ(100, keyframe_effect->TimeToReverseEffectChange()); EXPECT_EQ(100, keyframe_effect->TimeToReverseEffectChange());
......
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