Commit 4e433f61 authored by Kevin Ellis's avatar Kevin Ellis Committed by Commit Bot

Web-animation: Update CalculateAnimationPlayState to align with the spec.

Previously, the paused_ flag was used to signal that the animation was
in a paused state.  This flag is not part of the spec and is being
phased out.  Instead, we look for pending pause or play tasks and
whether there is a start time to determine if the animation is paused in
alignment with the spec.

Note that fixing play state calculation entailed fixing the handling of
setting a zero playback rate.  Previously, setting the playback rate to
zero would clear the start time; however, this has the unwanted side
effect of forced the animation into the paused state, which is not to
spec.  A running or finished animation should have a start time (if not
a pending change) even if the playback rate is zero.

https://drafts.csswg.org/web-animations/#play-states

Bug: 960944

Change-Id: I1fd43315d3b1c42b22302bf730c450a03836861a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862258
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706628}
parent c37e0fff
...@@ -344,9 +344,9 @@ void Animation::SetCurrentTimeInternal(double new_current_time, ...@@ -344,9 +344,9 @@ void Animation::SetCurrentTimeInternal(double new_current_time,
if (!hold_time_ || hold_time_ != new_current_time) if (!hold_time_ || hold_time_ != new_current_time)
outdated = true; outdated = true;
hold_time_ = new_current_time; hold_time_ = new_current_time;
if (paused_ || !playback_rate_) { if (paused_) {
start_time_ = base::nullopt; start_time_ = base::nullopt;
} else if (is_limited && !start_time_ && } else if (is_limited && !start_time_ && playback_rate_ &&
reason == kTimingUpdateForAnimationFrame) { reason == kTimingUpdateForAnimationFrame) {
start_time_ = CalculateStartTime(new_current_time); start_time_ = CalculateStartTime(new_current_time);
} }
...@@ -762,11 +762,7 @@ Animation::AnimationPlayState Animation::CalculateAnimationPlayState() const { ...@@ -762,11 +762,7 @@ Animation::AnimationPlayState Animation::CalculateAnimationPlayState() const {
// * both the start time of animation is unresolved and it does not have a // * both the start time of animation is unresolved and it does not have a
// pending play task, // pending play task,
// then paused. // then paused.
// TODO(crbug.com/958433): Presently using a paused_ flag that tracks an if (pending_pause_ || (!start_time_ && !pending_play_))
// animation being in a paused state (including a pending pause). The above
// rules do not yet work verbatim due to subtle timing discrepancies on when
// start_time gets resolved.
if (paused_)
return kPaused; return kPaused;
// 3. For animation, current time is resolved and either of the following // 3. For animation, current time is resolved and either of the following
......
...@@ -437,7 +437,8 @@ TEST_F(AnimationAnimationTestNoCompositing, StartTimeWithZeroPlaybackRate) { ...@@ -437,7 +437,8 @@ TEST_F(AnimationAnimationTestNoCompositing, StartTimeWithZeroPlaybackRate) {
animation->setPlaybackRate(0); animation->setPlaybackRate(0);
EXPECT_EQ("running", animation->playState()); EXPECT_EQ("running", animation->playState());
SimulateAwaitReady(); SimulateAwaitReady();
EXPECT_FALSE(animation->startTime()); EXPECT_EQ("running", animation->playState());
EXPECT_EQ(0, animation->startTime());
SimulateFrame(10000); SimulateFrame(10000);
EXPECT_EQ("running", animation->playState()); EXPECT_EQ("running", animation->playState());
......
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