[cc] Have WorkletAnimation pause all of its KeyframeEffects

Conceptually, this is somewhat close to what WorkletAnimation does; the
KeyframeEffect skips (seeks) from point to point driven by the script-provided
local time. More importantly, it means that WorkletAnimations do not terminate
after the duration has expired (which they are not meant to do), and allows us
to remove the ActiveTimeProvider hack (in favour of this one!).

This CL also removes PushPropertiesTo from WorkletAnimation. This method used
to push the local_time_ from cc-main to impl. Doing so was incorrect as cc-main
never gets the output of the mutate call, so always has a zero local time, which
then started to interfere with the other changes made in this CL.

Bug: 779189
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ibb0bdd60753bd24264112e10a1f829d394db27e2
Reviewed-on: https://chromium-review.googlesource.com/1037766
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556087}

cc/animation/animation.cc

@@ -243,7 +243,7 @@ void Animation::PushPropertiesTo(Animation* animation_impl) {
 void Animation::Tick(base::TimeTicks monotonic_time) {
   DCHECK(!monotonic_time.is_null());
   for (auto& keyframe_effect : keyframe_effects_)
-    keyframe_effect->Tick(monotonic_time, nullptr);
+    keyframe_effect->Tick(monotonic_time);
 }
 
 void Animation::UpdateState(bool start_ready_animations,

cc/animation/animation.h

@@ -90,7 +90,7 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> {
   void AbortKeyframeModels(TargetProperty::Type target_property,
                            bool needs_completion);
 
-  virtual void PushPropertiesTo(Animation* animation_impl);
+  void PushPropertiesTo(Animation* animation_impl);
 
   void UpdateState(bool start_ready_keyframe_models, AnimationEvents* events);
   virtual void Tick(base::TimeTicks monotonic_time);

cc/animation/keyframe_effect.cc

@@ -104,8 +104,7 @@ void KeyframeEffect::DetachElement() {
   element_id_ = ElementId();
 }
 
-void KeyframeEffect::Tick(base::TimeTicks monotonic_time,
-                          const AnimationTimeProvider* tick_provider) {
+void KeyframeEffect::Tick(base::TimeTicks monotonic_time) {
   DCHECK(has_bound_element_animations());
   if (!element_animations_->has_element_in_any_list())
     return;
@@ -113,12 +112,8 @@ void KeyframeEffect::Tick(base::TimeTicks monotonic_time,
   if (needs_to_start_keyframe_models_)
     StartKeyframeModels(monotonic_time);
 
-  base::TimeTicks tick_time = monotonic_time;
   for (auto& keyframe_model : keyframe_models_) {
-    if (tick_provider)
-      tick_time = tick_provider->GetTimeForKeyframeModel(*keyframe_model);
-
-    TickKeyframeModel(tick_time, keyframe_model.get(),
+    TickKeyframeModel(monotonic_time, keyframe_model.get(),
                       element_animations_.get());
   }
 
@@ -225,6 +220,19 @@ void KeyframeEffect::UpdateTickingState(UpdateTickingType type) {
   }
 }
 
+void KeyframeEffect::Pause(base::TimeDelta pause_offset) {
+  for (auto& keyframe_model : keyframe_models_) {
+    base::TimeTicks pause_time = keyframe_model->time_offset() +
+                                 keyframe_model->start_time() + pause_offset;
+    keyframe_model->SetRunState(KeyframeModel::PAUSED, pause_time);
+  }
+
+  if (has_bound_element_animations()) {
+    animation_->SetNeedsCommit();
+    SetNeedsPushProperties();
+  }
+}
+
 void KeyframeEffect::AddKeyframeModel(
     std::unique_ptr<KeyframeModel> keyframe_model) {
   AnimationHost* animation_host = animation_->animation_host();

cc/animation/keyframe_effect.h

@@ -41,12 +41,6 @@ typedef size_t KeyframeEffectId;
 // given target.
 class CC_ANIMATION_EXPORT KeyframeEffect {
  public:
-  class AnimationTimeProvider {
-   public:
-    virtual base::TimeTicks GetTimeForKeyframeModel(
-        const KeyframeModel&) const = 0;
-  };
-
   explicit KeyframeEffect(KeyframeEffectId id);
   ~KeyframeEffect();
 
@@ -87,8 +81,7 @@ class CC_ANIMATION_EXPORT KeyframeEffect {
   void AttachElement(ElementId element_id);
   void DetachElement();
 
-  void Tick(base::TimeTicks monotonic_time,
-            const AnimationTimeProvider* tick_provider);
+  void Tick(base::TimeTicks monotonic_time);
   static void TickKeyframeModel(base::TimeTicks monotonic_time,
                                 KeyframeModel* keyframe_model,
                                 AnimationTarget* target);
@@ -98,6 +91,8 @@ class CC_ANIMATION_EXPORT KeyframeEffect {
   void UpdateState(bool start_ready_keyframe_models, AnimationEvents* events);
   void UpdateTickingState(UpdateTickingType type);
 
+  void Pause(base::TimeDelta pause_offset);
+
   void AddKeyframeModel(std::unique_ptr<KeyframeModel> keyframe_model);
   void PauseKeyframeModel(int keyframe_model_id, double time_offset);
   void RemoveKeyframeModel(int keyframe_model_id);

cc/animation/keyframe_model.cc

@@ -160,21 +160,6 @@ bool KeyframeModel::InEffect(base::TimeTicks monotonic_time) const {
          (fill_mode_ == FillMode::BOTH || fill_mode_ == FillMode::BACKWARDS);
 }
 
-base::TimeTicks KeyframeModel::ConvertLocalTimeToMonotonicTime(
-    base::TimeDelta local_time) const {
-  // When waiting on receiving a start time, then our global clock is 'stuck' at
-  // the initial state.
-  if ((run_state_ == STARTING && !has_set_start_time()) ||
-      needs_synchronized_start_time())
-    return base::TimeTicks();
-
-  // If we're paused, time is 'stuck' at the pause time.
-  if (run_state_ == PAUSED)
-    return pause_time_;
-
-  return local_time + start_time_ + total_paused_time_;
-}
-
 base::TimeDelta KeyframeModel::ConvertToActiveTime(
     base::TimeTicks monotonic_time) const {
   // If we're just starting or we're waiting on receiving a start time,

cc/animation/keyframe_model.h

@@ -140,9 +140,6 @@ class CC_ANIMATION_EXPORT KeyframeModel {
   base::TimeDelta TrimTimeToCurrentIteration(
       base::TimeTicks monotonic_time) const;
 
-  base::TimeTicks ConvertLocalTimeToMonotonicTime(
-      base::TimeDelta local_time) const;
-
   void set_is_controlling_instance_for_test(bool is_controlling_instance) {
     is_controlling_instance_ = is_controlling_instance;
   }

cc/animation/worklet_animation.cc

@@ -42,7 +42,13 @@ void WorkletAnimation::SetLocalTime(base::TimeDelta local_time) {
 }
 
 void WorkletAnimation::Tick(base::TimeTicks monotonic_time) {
-  keyframe_effect()->Tick(monotonic_time, this);
+  // As the output of a WorkletAnimation is driven by a script-provided local
+  // time, we don't want the underlying effect to participate in the normal
+  // animations lifecycle. To avoid this we pause the underlying keyframe effect
+  // at the local time obtained from the user script - essentially turning each
+  // call to |WorkletAnimation::Tick| into a seek in the effect.
+  keyframe_effect()->Pause(local_time_);
+  keyframe_effect()->Tick(monotonic_time);
 }
 
 // TODO(crbug.com/780151): The current time returned should be an offset against
@@ -66,19 +72,6 @@ bool WorkletAnimation::NeedsUpdate(base::TimeTicks monotonic_time,
   return needs_update;
 }
 
-base::TimeTicks WorkletAnimation::GetTimeForKeyframeModel(
-    const KeyframeModel& keyframe_model) const {
-  // The local time set by the script has to be converted to monotonic time;
-  // largely this means it is offset from the start time and includes any time
-  // the animation spent paused.
-  return keyframe_model.ConvertLocalTimeToMonotonicTime(local_time_);
-}
-
-void WorkletAnimation::PushPropertiesTo(Animation* animation_impl) {
-  SingleKeyframeEffectAnimation::PushPropertiesTo(animation_impl);
-  static_cast<WorkletAnimation*>(animation_impl)->SetLocalTime(local_time_);
-}
-
 bool WorkletAnimation::IsWorkletAnimation() const {
   return true;
 }

cc/animation/worklet_animation.h

@@ -19,8 +19,7 @@ class ScrollTimeline;
 // timing to be controlled by an animator instance that is running in a
 // AnimationWorkletGlobalScope.
 class CC_ANIMATION_EXPORT WorkletAnimation final
-    : public SingleKeyframeEffectAnimation,
-      KeyframeEffect::AnimationTimeProvider {
+    : public SingleKeyframeEffectAnimation {
  public:
   WorkletAnimation(int id,
                    const std::string& name,
@@ -51,12 +50,6 @@ class CC_ANIMATION_EXPORT WorkletAnimation final
   bool NeedsUpdate(base::TimeTicks monotonic_time,
                    const ScrollTree& scroll_tree);
 
-  // KeyframeEffect::AnimationTimeProvider:
-  base::TimeTicks GetTimeForKeyframeModel(
-      const KeyframeModel& keyframe_model) const override;
-
-  void PushPropertiesTo(Animation* animation_impl) override;
-
  private:
   ~WorkletAnimation() override;
 

cc/animation/worklet_animation_unittest.cc

@@ -36,9 +36,15 @@ class WorkletAnimationTest : public AnimationTimelinesTest {
     worklet_animation_->AttachElement(element_id_);
     host_->AddAnimationTimeline(timeline_);
     timeline_->AttachAnimation(worklet_animation_);
+
+    host_->PushPropertiesTo(host_impl_);
+    timeline_impl_ = host_impl_->GetTimelineById(timeline_id_);
+    worklet_animation_impl_ = static_cast<WorkletAnimation*>(
+        timeline_impl_->GetAnimationById(worklet_animation_id_));
   }
 
   scoped_refptr<WorkletAnimation> worklet_animation_;
+  scoped_refptr<WorkletAnimation> worklet_animation_impl_;
   int worklet_animation_id_ = 11;
 };
 
@@ -61,26 +67,14 @@ TEST_F(WorkletAnimationTest, LocalTimeIsUsedWithAnimations) {
   AddOpacityTransitionToAnimation(worklet_animation_.get(), duration,
                                   start_opacity, end_opacity, true);
 
+  // Push the opacity animation to the impl thread.
   host_->PushPropertiesTo(host_impl_);
   host_impl_->ActivateAnimations();
 
-  // TODO(majidvp): At the moment the animation does not use the local time when
-  // it is starting. This is because KeyframeModel::ConvertToActiveTime always
-  // returns the time_offset when starting. We need to change this.
-  base::TimeTicks time;
-  time += base::TimeDelta::FromSecondsD(0.1);
-  TickAnimationsTransferEvents(time, 1u);
-
   base::TimeDelta local_time = base::TimeDelta::FromSecondsD(duration / 2);
-  worklet_animation_->SetLocalTime(local_time);
-
-  host_->PushPropertiesTo(host_impl_);
-
-  TickAnimationsTransferEvents(time, 0u);
-
-  client_.ExpectOpacityPropertyMutated(element_id_, ElementListType::ACTIVE,
-                                       expected_opacity);
+  worklet_animation_impl_->SetLocalTime(local_time);
 
+  TickAnimationsTransferEvents(base::TimeTicks(), 0u);
   client_impl_.ExpectOpacityPropertyMutated(
       element_id_, ElementListType::ACTIVE, expected_opacity);
 }
@@ -109,7 +103,7 @@ TEST_F(WorkletAnimationTest, LayerTreeMutatorsIsMutatedWithCorrectInputState) {
 
   base::TimeTicks time;
   time += base::TimeDelta::FromSecondsD(0.1);
-  TickAnimationsTransferEvents(time, 1u);
+  TickAnimationsTransferEvents(time, 0u);
 
   Mock::VerifyAndClearExpectations(mock_mutator);
 }
@@ -136,7 +130,7 @@ TEST_F(WorkletAnimationTest, LayerTreeMutatorsIsMutatedOnlyWhenInputChanges) {
 
   base::TimeTicks time;
   time += base::TimeDelta::FromSecondsD(0.1);
-  TickAnimationsTransferEvents(time, 1u);
+  TickAnimationsTransferEvents(time, 0u);
 
   // The time has not changed which means worklet animation input is the same.
   // Ticking animations again should not result in mutator being asked to

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-duration-expected.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+  will-change: transform;
+  transform: translateY(100px);
+  opacity: 0.8;
+}
+</style>
+
+<div id="box"></div>

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-duration.html

@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+}
+</style>
+
+<div id="box"></div>
+
+<script id="visual_update"  type="text/worklet">
+registerAnimator("test_animator", class {
+  animate(currentTime, effect) {
+    effect.localTime = 0.5;
+  }
+});
+</script>
+
+<script src="resources/animation-worklet-tests.js"></script>
+
+<script>
+if (window.testRunner) {
+  testRunner.waitUntilDone();
+}
+
+runInAnimationWorklet(
+  document.getElementById('visual_update').textContent
+).then(() => {
+  const box = document.getElementById('box');
+  const effect = new KeyframeEffect(box,
+    [
+     { transform: 'translateY(0px)' },
+     { transform: 'translateY(200px)' }
+    ], {
+      duration: 1,
+    }
+  );
+
+  const animation = new WorkletAnimation('test_animator', effect, document.timeline, {});
+  animation.play();
+
+  // The WorkletAnimation should continue to be in effect forever, even if its
+  // duration is passed.
+  //
+  // The animation is specified to last for 1 millisecond, but we wait for 500ms
+  // to be safe. This is definitely risking flake, but until we start forwarding
+  // style changes to the main thread there's not much else we can do here.
+  setTimeout(function() {
+    // TODO(crbug.com/829926): At the moment a worklet animation ending does not
+    // cause a commit from main to impl which means impl continues to have its
+    // last animated value even after cancelling. This is certainly a bug! But
+    // for now force a commit by manually updating the opacity.
+    //
+    // Note that this is only required in the case where this test would *fail*.
+    // We want to be sure we spot the WorkletAnimation incorrectly ending early,
+    // so we need to force a redraw.
+    box.style.opacity = 0.8;
+
+    if (window.testRunner) {
+      waitTwoAnimationFrames(_ => {
+        testRunner.notifyDone();
+      });
+    }
+  }, 500);
+});
+</script>

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-local-time-after-duration-expected.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+  will-change: transform;
+  transform: translateY(200px);
+}
+</style>
+
+<div id="box"></div>

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-local-time-after-duration.html

@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+}
+</style>
+
+<div id="box"></div>
+
+<script id="visual_update"  type="text/worklet">
+registerAnimator("test_animator", class {
+  animate(currentTime, effect) {
+    // The local time should be trimmed by the duration, e.g. this is equivalent
+    // to effect.localTime = 1000;
+    effect.localTime = 5000;
+  }
+});
+</script>
+
+<script src="resources/animation-worklet-tests.js"></script>
+
+<script>
+if (window.testRunner) {
+  testRunner.waitUntilDone();
+}
+
+runInAnimationWorklet(
+  document.getElementById('visual_update').textContent
+).then(() => {
+  const box = document.getElementById('box');
+  const effect = new KeyframeEffect(box,
+    [
+     { transform: 'translateY(0px)' },
+     { transform: 'translateY(200px)' }
+    ], {
+      duration: 1000,
+    }
+  );
+
+  const animation = new WorkletAnimation('test_animator', effect, document.timeline, {});
+  animation.play();
+
+  if (window.testRunner) {
+    waitTwoAnimationFrames(_ => {
+      testRunner.notifyDone();
+    });
+  }
+});
+</script>

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-local-time-before-start-expected.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+  will-change: transform;
+  transform: translateY(200px);
+}
+</style>
+
+<div id="box"></div>

third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-local-time-before-start.html

@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<style>
+#box {
+  width: 100px;
+  height: 100px;
+  background-color: #00ff00;
+}
+</style>
+
+<div id="box"></div>
+
+<script id="visual_update"  type="text/worklet">
+registerAnimator("test_animator", class {
+  animate(currentTime, effect) {
+    // The local time should be trimmed by the duration, e.g. this is equivalent
+    // to effect.localTime = 0;
+    effect.localTime = -500;
+  }
+});
+</script>
+
+<script src="resources/animation-worklet-tests.js"></script>
+
+<script>
+if (window.testRunner) {
+  testRunner.waitUntilDone();
+}
+
+runInAnimationWorklet(
+  document.getElementById('visual_update').textContent
+).then(() => {
+  const box = document.getElementById('box');
+  const effect = new KeyframeEffect(box,
+    [
+     { transform: 'translateY(200px)' },
+     { transform: 'translateY(0px)' }
+    ], {
+      duration: 1000,
+    }
+  );
+
+  const animation = new WorkletAnimation('test_animator', effect, document.timeline, {});
+  animation.play();
+
+  if (window.testRunner) {
+    waitTwoAnimationFrames(_ => {
+      testRunner.notifyDone();
+    });
+  }
+});
+</script>