Commit 7ac4f74b authored by Findit's avatar Findit

Revert "[animation-worklet] Basic pause implementation"

This reverts commit 94b5b84b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 625711 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vOTRiNWI4NGIwNTg2MDljY2YxZDU4ZjUxNzVjMDkzMWI2M2I5ZjBmMgw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/WebKit%20Linux%20Trusty%20Leak/29257

Sample Failed Step: webkit_layout_tests

Sample Flaky Test: external/wpt/animation-worklet/worklet-animation-pause.https.html

Original change's description:
> [animation-worklet] Basic pause implementation
> 
> Pausing worklet animation now holds the time. This works as expected for
> main thread animations. Implementing this for composited worklet
> animations will be done in a follow up patch.
> 
> Major changes:
>  - Add and expose pause() method pausing the animation.
>  - Introduce hold_time that is used when animation is paused.
>  - Rework how current time is computed, it is now closer to
>    regular animations i.e., we either compute it based on
>    "start time and timeline.currentTime" or use "hold time".
>  - Instead of setting start time we now set the current time
>    which then works backward to compute either the start time
>    or the hold time based on the animation state.
>  - When transitioning animation play state, we now always set
>    the current time. Previously this was adhoc and inconsistent.
>  - Introduce has_started_ to differentiate when playing an
>    animation for the first time vs playing it from pause.
> 
> 
> TEST:
>   * wpt/animation-worklet/worklet-animation-pause.https.html: js test for basic current time calculations
>   * wpt/animation-worklet/worklet-animation-pause-immediately.https.html: reftest for basic pause
>   * wpt/animation-worklet/worklet-animation-pause-result.https.html: reftest for pause/resume.
>   * WorkletAnimationTest.PausePlay: unit test for basic state transition and time calc
> 
> Bug: 821910
> 
> Change-Id: Ie4b00129398159b3b5b83212bb63c43f2ce8bf4e
> Reviewed-on: https://chromium-review.googlesource.com/c/1383298
> Commit-Queue: Majid Valipour <majidvp@chromium.org>
> Reviewed-by: Majid Valipour <majidvp@chromium.org>
> Reviewed-by: Yi Gu <yigu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#625711}

Change-Id: Ibb3c50f772493f1761cee657aaa618f357348188
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 821910
Reviewed-on: https://chromium-review.googlesource.com/c/1434815
Cr-Commit-Position: refs/heads/master@{#625747}
parent 2a51528a
......@@ -150,34 +150,6 @@ double ToMilliseconds(base::Optional<base::TimeDelta> time) {
: std::numeric_limits<double>::quiet_NaN();
}
// Calculates start time backwards from the current time and
// timeline.currentTime.
//
// If this is a scroll-linked animation, we always consider start time to be
// zero (i.e., scroll origin). This means the computed start time post this
// calculation may not match the expected current time that was given as input.
//
// Changing this is under consideration here:
// https://github.com/w3c/csswg-drafts/issues/2075
base::Optional<base::TimeDelta> CalculateStartTime(
base::TimeDelta current_time,
AnimationTimeline& timeline) {
if (timeline.IsScrollTimeline())
return base::TimeDelta();
bool is_null;
double time_ms = timeline.currentTime(is_null);
// TODO(majidvp): Make it so that inactive timelines do not reach here
// i.e., we should instead "hold" when timeline is inactive.
// https://crbug.com/924159
if (is_null)
return base::nullopt;
auto timeline_time = base::TimeDelta::FromMillisecondsD(time_ms);
// TODO(majidvp): Divide the current time by playback rate once that is
// implemented. https://crbug.com/852475
return timeline_time - current_time;
}
} // namespace
WorkletAnimation* WorkletAnimation::Create(
......@@ -284,17 +256,6 @@ void WorkletAnimation::play(ExceptionState& exception_state) {
if (play_state_ == Animation::kPending)
return;
if (play_state_ == Animation::kPaused) {
// If we have ever started before then just unpause otherwise we need to
// start the animation.
if (has_started_) {
SetPlayState(Animation::kPending);
SetCurrentTime(CurrentTime());
InvalidateCompositingState();
return;
}
}
String failure_message;
if (!CheckCanStart(&failure_message)) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
......@@ -303,11 +264,7 @@ void WorkletAnimation::play(ExceptionState& exception_state) {
}
document_->GetWorkletAnimationController().AttachAnimation(*this);
// While animation is pending, it hold time at Zero, see:
// https://drafts.csswg.org/web-animations-1/#playing-an-animation-section
SetPlayState(Animation::kPending);
SetCurrentTime(base::TimeDelta());
has_started_ = true;
for (auto& effect : effects_) {
Element* target = effect->target();
......@@ -328,37 +285,19 @@ double WorkletAnimation::currentTime(bool& is_null) {
return ToMilliseconds(current_time);
}
void WorkletAnimation::pause(ExceptionState& exception_state) {
DCHECK(IsMainThread());
if (play_state_ == Animation::kPaused)
return;
// If animation is pending it means we have not sent an update to
// compositor. Since we are pausing, immediately start the animation
// which updates start time and marks animation as main thread.
// This ensures we have a valid current time to hold.
if (play_state_ == Animation::kPending)
StartOnMain();
// If animation is playing then we should hold the current time
// otherwise hold zero.
base::TimeDelta new_current_time =
Playing() ? CurrentTime().value() : base::TimeDelta();
SetPlayState(Animation::kPaused);
SetCurrentTime(new_current_time);
}
void WorkletAnimation::cancel() {
DCHECK(IsMainThread());
if (play_state_ == Animation::kIdle)
return;
document_->GetWorkletAnimationController().DetachAnimation(*this);
if (compositor_animation_) {
GetEffect()->CancelAnimationOnCompositor(compositor_animation_.get());
DestroyCompositorAnimation();
}
has_started_ = false;
local_times_.Fill(base::nullopt);
start_time_ = base::nullopt;
running_on_main_thread_ = false;
// TODO(yigu): Because this animation has been detached and will not receive
// updates anymore, we have to update its value upon cancel. Similar to
......@@ -369,7 +308,6 @@ void WorkletAnimation::cancel() {
effect->UpdateInheritedTime(NullValue(), kTimingUpdateOnDemand);
}
SetPlayState(Animation::kIdle);
SetCurrentTime(base::nullopt);
for (auto& effect : effects_) {
Element* target = effect->target();
......@@ -399,7 +337,11 @@ void WorkletAnimation::EffectInvalidated() {
}
void WorkletAnimation::Update(TimingUpdateReason reason) {
if (play_state_ != Animation::kRunning && play_state_ != Animation::kPaused)
if (play_state_ != Animation::kRunning)
return;
// ScrollTimeline animation doesn't require start_time_ to be set.
if (!start_time_ && !timeline_->IsScrollTimeline())
return;
DCHECK_EQ(effects_.size(), local_times_.size());
......@@ -422,24 +364,12 @@ bool WorkletAnimation::CheckCanStart(String* failure_message) {
return true;
}
void WorkletAnimation::SetCurrentTime(
base::Optional<base::TimeDelta> seek_time) {
DCHECK(timeline_);
// The procedure either:
// 1) updates the hold time (for paused animations, non-existent timeline)
// 2) updates the start time (for playing animations)
bool should_hold = play_state_ == Animation::kPaused || !seek_time;
if (should_hold) {
start_time_ = base::nullopt;
hold_time_ = seek_time;
} else {
// TODO(majidvp): Currently when timeline is inactive (e.g., scroll timeline
// whose source is not scrolling), we will end up here but probably we
// should not. In those cases we should hold instead.
// https://crbug.com/924159
start_time_ = CalculateStartTime(seek_time.value(), *timeline_);
hold_time_ = base::nullopt;
}
void WorkletAnimation::SetStartTimeToNow() {
DCHECK(!start_time_);
bool is_null;
double time_ms = timeline_->currentTime(is_null);
if (!is_null)
start_time_ = base::TimeDelta::FromMillisecondsD(time_ms);
}
void WorkletAnimation::UpdateCompositingState() {
......@@ -472,10 +402,8 @@ void WorkletAnimation::InvalidateCompositingState() {
void WorkletAnimation::StartOnMain() {
running_on_main_thread_ = true;
// Start from existing current time in case one exists or zero.
base::TimeDelta current_time = CurrentTime().value_or(base::TimeDelta());
SetStartTimeToNow();
SetPlayState(Animation::kRunning);
SetCurrentTime(current_time);
}
bool WorkletAnimation::StartOnCompositor() {
......@@ -505,7 +433,6 @@ bool WorkletAnimation::StartOnCompositor() {
if (!failure_code.Ok()) {
SetPlayState(Animation::kIdle);
SetCurrentTime(base::nullopt);
return false;
}
......@@ -536,7 +463,12 @@ bool WorkletAnimation::StartOnCompositor() {
// TODO(smcgruer): We need to start all of the effects, not just the first.
StartEffectOnCompositor(compositor_animation_.get(), GetEffect());
SetPlayState(Animation::kRunning);
SetCurrentTime(base::TimeDelta());
bool is_null;
double time_ms = timeline_->currentTime(is_null);
if (!is_null)
start_time_ = base::TimeDelta::FromMillisecondsD(time_ms);
return true;
}
......@@ -603,8 +535,11 @@ base::Optional<base::TimeDelta> WorkletAnimation::CurrentTime() const {
if (play_state_ == Animation::kIdle || play_state_ == Animation::kUnset)
return base::nullopt;
if (hold_time_)
return hold_time_.value();
// TODO(majidvp): Animation has a hold time while it waits for animation
// to truly start and returns that instead. Replace with with hold time
// once pause logic is implemented.
if (play_state_ == Animation::kPending)
return base::TimeDelta();
bool is_null;
double timeline_time_ms = timeline_->currentTime(is_null);
......@@ -613,6 +548,8 @@ base::Optional<base::TimeDelta> WorkletAnimation::CurrentTime() const {
base::TimeDelta timeline_time =
base::TimeDelta::FromMillisecondsD(timeline_time_ms);
if (timeline_->IsScrollTimeline())
return timeline_time;
DCHECK(start_time_);
return timeline_time - start_time_.value();
}
......@@ -650,6 +587,8 @@ void WorkletAnimation::UpdateInputState(
bool was_active = IsActive(last_play_state_);
bool is_active = IsActive(play_state_);
// ScrollTimeline animation doesn't require start_time_ to be set.
DCHECK(start_time_ || timeline_->IsScrollTimeline());
base::Optional<base::TimeDelta> current_time = CurrentTime();
double current_time_ms = ToMilliseconds(current_time);
......
......@@ -74,7 +74,6 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
String playState();
double currentTime(bool& is_null);
void play(ExceptionState& exception_state);
void pause(ExceptionState& exception_state);
void cancel();
// AnimationEffectOwner implementation:
......@@ -141,28 +140,7 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
bool StartOnCompositor();
void StartOnMain();
bool CheckCanStart(String* failure_message);
// Sets the current time for the animation.
//
// Note that the current time of the animation is a computed value that
// depends on either the start time (for playing animations) or the hold time
// (for pending, paused, or idle animations). So this procedure updates either
// the start time or the hold time so that the computed current time is
// matched.
//
// The exception to this are scroll-linked animations whose start time is not
// modifiable (always zero) in which case the post setting the current time,
// the computed current time may not match it.
//
// Generally, when an animation play state transitions, we expect to see the
// current time is set. Here are some interesting examples of this:
// - when transitioning to play, the current time is either set to
// zero (first time) or the last current time (when resuming from pause).
// - when transitioning to idle or cancel, the current time is set to
// "null".
// - when transitioning to pause, the current time is set to the last
// current time for holding.
void SetCurrentTime(base::Optional<base::TimeDelta> current_time);
void SetStartTimeToNow();
// Updates a running animation on the compositor side.
void UpdateOnCompositor();
......@@ -171,7 +149,6 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
return options_ ? options_->Clone() : nullptr;
}
Animation::AnimationPlayState PlayState() const { return play_state_; }
void SetPlayState(const Animation::AnimationPlayState& state) {
play_state_ = state;
}
......@@ -186,9 +163,6 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
Animation::AnimationPlayState last_play_state_;
base::Optional<base::TimeDelta> start_time_;
Vector<base::Optional<base::TimeDelta>> local_times_;
// Hold time is used when animation is paused.
// TODO(majidvp): Replace base::TimeDelta usage with AnimationTimeDelta.
base::Optional<base::TimeDelta> hold_time_;
// We use this to skip updating if current time has not changed since last
// update.
base::Optional<base::TimeDelta> last_current_time_;
......@@ -203,14 +177,12 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
std::unique_ptr<CompositorAnimation> compositor_animation_;
bool running_on_main_thread_;
bool has_started_;
// Tracks whether any KeyframeEffect associated with this WorkletAnimation has
// been invalidated and needs to be restarted. Used to avoid unnecessarily
// restarting the effect on the compositor. When true, a call to
// |UpdateOnCompositor| will update the effect on the compositor.
bool effect_needs_restart_;
FRIEND_TEST_ALL_PREFIXES(WorkletAnimationTest, PausePlay);
};
} // namespace blink
......
......@@ -20,6 +20,5 @@
readonly attribute AnimationPlayState playState;
readonly attribute double? currentTime;
[RaisesException] void play();
[RaisesException] void pause();
void cancel();
};
......@@ -56,13 +56,10 @@ WorkletAnimation* CreateWorkletAnimation(
scoped_refptr<SerializedScriptValue> options;
ScriptState::Scope scope(script_state);
DummyExceptionStateForTesting exception_state;
return WorkletAnimation::Create(script_state, animator_name, effects,
timeline, std::move(options),
ASSERT_NO_EXCEPTION);
}
base::TimeDelta ToTimeDelta(double milliseconds) {
return base::TimeDelta::FromMillisecondsD(milliseconds);
exception_state);
}
} // namespace
......@@ -85,14 +82,6 @@ class WorkletAnimationTest : public RenderingTest {
CreateWorkletAnimation(GetScriptState(), element_, animator_name_);
}
void SimulateFrame(double milliseconds) {
base::TimeTicks tick = base::TimeTicks() + ToTimeDelta(milliseconds);
GetDocument().GetAnimationClock().ResetTimeForTesting(tick);
GetDocument().GetWorkletAnimationController().UpdateAnimationStates();
GetDocument().GetWorkletAnimationController().UpdateAnimationTimings(
kTimingUpdateForAnimationFrame);
}
ScriptState* GetScriptState() {
return ToScriptStateForMainWorld(&GetFrame());
}
......@@ -103,7 +92,8 @@ class WorkletAnimationTest : public RenderingTest {
};
TEST_F(WorkletAnimationTest, WorkletAnimationInElementAnimations) {
worklet_animation_->play(ASSERT_NO_EXCEPTION);
DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
EXPECT_EQ(1u,
element_->EnsureElementAnimations().GetWorkletAnimations().size());
worklet_animation_->cancel();
......@@ -115,7 +105,8 @@ TEST_F(WorkletAnimationTest, StyleHasCurrentAnimation) {
scoped_refptr<ComputedStyle> style =
GetDocument().EnsureStyleResolver().StyleForElement(element_).get();
EXPECT_EQ(false, style->HasCurrentOpacityAnimation());
worklet_animation_->play(ASSERT_NO_EXCEPTION);
DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
element_->EnsureElementAnimations().UpdateAnimationFlags(*style);
EXPECT_EQ(true, style->HasCurrentOpacityAnimation());
}
......@@ -134,7 +125,8 @@ TEST_F(WorkletAnimationTest,
base::TimeTicks() + base::TimeDelta::FromMillisecondsD(111 + 123.4);
GetDocument().GetAnimationClock().ResetTimeForTesting(first_ticks);
worklet_animation_->play(ASSERT_NO_EXCEPTION);
DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
worklet_animation_->UpdateCompositingState();
std::unique_ptr<AnimationWorkletDispatcherInput> state =
......@@ -181,7 +173,8 @@ TEST_F(WorkletAnimationTest,
GetScriptState(), element_, animator_name_, scroll_timeline);
WorkletAnimationId id = worklet_animation->GetWorkletAnimationId();
worklet_animation->play(ASSERT_NO_EXCEPTION);
DummyExceptionStateForTesting exception_state;
worklet_animation->play(exception_state);
worklet_animation->UpdateCompositingState();
// Only expect precision up to 1 microsecond with an additional smaller
......@@ -212,7 +205,8 @@ TEST_F(WorkletAnimationTest, MainThreadSendsPeekRequestTest) {
base::TimeTicks() + base::TimeDelta::FromMillisecondsD(111 + 123.4);
GetDocument().GetAnimationClock().ResetTimeForTesting(first_ticks);
worklet_animation_->play(ASSERT_NO_EXCEPTION);
DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
worklet_animation_->UpdateCompositingState();
// Only peek if animation is running on compositor.
......@@ -262,31 +256,4 @@ TEST_F(WorkletAnimationTest, MainThreadSendsPeekRequestTest) {
state.reset(new AnimationWorkletDispatcherInput);
}
TEST_F(WorkletAnimationTest, PausePlay) {
SimulateFrame(0);
worklet_animation_->play(ASSERT_NO_EXCEPTION);
EXPECT_EQ(Animation::kPending, worklet_animation_->PlayState());
SimulateFrame(0);
EXPECT_EQ(Animation::kRunning, worklet_animation_->PlayState());
EXPECT_TRUE(worklet_animation_->Playing());
EXPECT_EQ(ToTimeDelta(0), worklet_animation_->CurrentTime().value());
SimulateFrame(10);
worklet_animation_->pause(ASSERT_NO_EXCEPTION);
EXPECT_EQ(Animation::kPaused, worklet_animation_->PlayState());
EXPECT_FALSE(worklet_animation_->Playing());
EXPECT_EQ(ToTimeDelta(10), worklet_animation_->CurrentTime().value());
SimulateFrame(20);
EXPECT_EQ(Animation::kPaused, worklet_animation_->PlayState());
EXPECT_EQ(ToTimeDelta(10), worklet_animation_->CurrentTime().value());
worklet_animation_->play(ASSERT_NO_EXCEPTION);
EXPECT_EQ(Animation::kPending, worklet_animation_->PlayState());
SimulateFrame(20);
EXPECT_EQ(Animation::kRunning, worklet_animation_->PlayState());
EXPECT_TRUE(worklet_animation_->Playing());
EXPECT_EQ(ToTimeDelta(10), worklet_animation_->CurrentTime().value());
SimulateFrame(30);
EXPECT_EQ(Animation::kRunning, worklet_animation_->PlayState());
EXPECT_EQ(ToTimeDelta(20), worklet_animation_->CurrentTime().value());
}
} // namespace blink
......@@ -30,9 +30,3 @@ function waitForAsyncAnimationFrames(count) {
// AnimationWorklet.
return waitForAnimationFrames(count + 1);
}
async function waitForAnimationFrameWithCondition(condition) {
do {
await new Promise(window.requestAnimationFrame);
} while (!condition())
};
<!DOCTYPE html>
<style>
#box {
width: 100px;
height: 100px;
transform: translateY(100px);
background-color: green;
}
</style>
<div id="box"></div>
<!DOCTYPE html>
<html class="reftest-wait">
<title>Verify that calling pause immediately after playing works as expected</title>
<link rel="help" href="https://drafts.css-houdini.org/css-animationworklet/">
<link rel="match" href="references/translated-box-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="/web-animations/testcommon.js"></script>
<script src="common.js"></script>
<style>
#box {
width: 100px;
height: 100px;
background-color: green;
}
</style>
<div id="box"></div>
<script>
registerPassthroughAnimator().then(async _ => {
const box = document.getElementById('box');
const effect = new KeyframeEffect(box,
{ transform: ['translateY(100px)', 'translateY(200px)'] },
{ duration: 100, iterations: 1 }
);
const animation = new WorkletAnimation('passthrough', effect);
animation.play();
// Immediately pausing animation should freeze the current time at 0.
animation.pause();
// Wait at least one frame to ensure a paused animation actually freezes.
await waitForAsyncAnimationFrames(1);
takeScreenshot();
});
</script>
</html>
<!DOCTYPE html>
<html class="reftest-wait">
<title>Verify that calling pause immediately after playing works as expected</title>
<link rel="help" href="https://drafts.css-houdini.org/css-animationworklet/">
<link rel="match" href="references/translated-box-ref.html">
<script src="/common/reftest-wait.js"></script>
<script src="common.js"></script>
<style>
#box {
width: 100px;
height: 100px;
background-color: green;
}
</style>
<div id="box"></div>
<script>
registerPassthroughAnimator().then(async _ => {
const duration = 18; // a bit longer than a frame
const box = document.getElementById('box');
const effect = new KeyframeEffect(box,
{ transform: ['translateY(0px)', 'translateY(100px)'] },
{ duration: duration, iterations: 1, fill: 'forwards'}
);
const animation = new WorkletAnimation('passthrough', effect);
// Immediately pausing animation should freeze the current time at 0.
animation.pause();
// Playing should cause animation to resume.
animation.play();
// Wait until we ensure animation has reached completion.
await waitForAnimationFrameWithCondition( _ => {
return animation.currentTime >= duration;
});
takeScreenshot();
});
</script>
</html>
<!DOCTYPE html>
<title>Verify that currentTime and playState are correct when animation is paused</title>
<link rel="help" href="https://drafts.css-houdini.org/css-animationworklet/">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/web-animations/testcommon.js"></script>
<script src="common.js"></script>
<div id="box"></div>
<script>
setup(setupAndRegisterTests, {explicit_done: true});
function createAnimation() {
const box = document.getElementById('box');
const effect = new KeyframeEffect(box,
{ transform: ['translateY(100px)', 'translateY(200px)'] },
{ duration: 100, iterations: 1 }
);
return new WorkletAnimation('passthrough', effect);
}
async function setupAndRegisterTests() {
await registerPassthroughAnimator();
promise_test(async t => {
const animation = createAnimation();
animation.play();
// Immediately pausing animation should freeze the current time at 0.
animation.pause();
assert_equals(animation.currentTime, 0);
assert_equals(animation.playState, "paused");
// Wait some time to ensure a paused animation actually freezes.
await waitForNextFrame();
assert_equals(animation.currentTime, 0);
assert_equals(animation.playState, "paused");
}, 'pausing an animation freezes its current time');
promise_test(async t => {
const animation = createAnimation();
const startTime = document.timeline.currentTime;
animation.pause();
animation.play();
await waitForNextFrame();
const timelineTime = document.timeline.currentTime;
assert_equals(animation.playState, "running");
assert_greater_than(animation.currentTime, 0);
assert_times_equal(animation.currentTime, (timelineTime - startTime));
}, 'playing a paused animation should resume it');
done();
}
</script>
......@@ -10248,7 +10248,6 @@ interface WorkletAnimation
getter timeline
method cancel
method constructor
method pause
method play
interface WritableStream
attribute @@toStringTag
......
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