[ScrollTimeline] Update compositor timeline from blink timeline

Previously CompositorTimeline was updated on Animation level which
introduced a reverse dependency. This patch moves the update logic up
to AnimationTimeline level to fix this issue.

Bug: 921031
Change-Id: Iaaa92686003c741cad424eb9867a0993193c05c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225853Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774351}

cc/animation/animation.cc

@@ -282,12 +282,4 @@ void Animation::NotifyKeyframeModelFinishedForTesting(
   DispatchAndDelegateAnimationEvent(event);
 }
 
-void Animation::UpdateScrollTimeline(base::Optional<ElementId> scroller_id,
-                                     base::Optional<double> start_scroll_offset,
-                                     base::Optional<double> end_scroll_offset) {
-  ToScrollTimeline(animation_timeline_)
-      ->UpdateScrollerIdAndScrollOffsets(scroller_id, start_scroll_offset,
-                                         end_scroll_offset);
-}
-
 }  // namespace cc

cc/animation/animation.h

@@ -65,17 +65,6 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> {
   }
   void SetAnimationTimeline(AnimationTimeline* timeline);
 
-  // TODO(yigu): There is a reverse dependency between AnimationTimeline and
-  // Animation. ScrollTimeline update should be handled by AnimationHost instead
-  // of Animation. This could be fixed once the snapshotting in blink is
-  // implemented. https://crbug.com/1023508.
-
-  // Should be called when the ScrollTimeline attached to this animation has a
-  // change, such as when the scroll source changes ElementId.
-  void UpdateScrollTimeline(base::Optional<ElementId> scroller_id,
-                            base::Optional<double> start_scroll_offset,
-                            base::Optional<double> end_scroll_offset);
-
   scoped_refptr<ElementAnimations> element_animations() const;
 
   void set_animation_delegate(AnimationDelegate* delegate) {

cc/animation/worklet_animation_unittest.cc

@@ -555,20 +555,6 @@ TEST_F(WorkletAnimationTest, SkipLockedAnimations) {
   EXPECT_EQ(input->updated_animations.size(), 1u);
 }
 
-TEST_F(WorkletAnimationTest, UpdateScrollTimelineScrollerId) {
-  auto scroll_timeline = base::WrapRefCounted(new MockScrollTimeline());
-  EXPECT_EQ(scroll_timeline->GetPendingIdForTest(), ElementId());
-
-  scoped_refptr<WorkletAnimation> worklet_animation = WorkletAnimation::Create(
-      worklet_animation_id_, "test_name", 1, nullptr, nullptr);
-  host_->AddAnimationTimeline(scroll_timeline);
-  scroll_timeline->AttachAnimation(worklet_animation);
-  ElementId scroller_id = ElementId(1);
-  worklet_animation->UpdateScrollTimeline(scroller_id, base::nullopt,
-                                          base::nullopt);
-  EXPECT_EQ(scroll_timeline->GetPendingIdForTest(), scroller_id);
-}
-
 }  // namespace
 
 }  // namespace cc

third_party/blink/renderer/core/animation/animation.cc

@@ -2027,19 +2027,6 @@ void Animation::DetachCompositorTimeline() {
   compositor_timeline->AnimationDestroyed(*this);
 }
 
-void Animation::UpdateCompositorScrollTimeline() {
-  if (!compositor_animation_ || !timeline_)
-    return;
-  auto& timeline = To<ScrollTimeline>(*timeline_);
-  Node* scroll_source = timeline.ResolvedScrollSource();
-  auto start_scroll_offset = timeline.GetResolvedStartScrollOffset();
-  auto end_scroll_offset = timeline.GetResolvedEndScrollOffset();
-
-  compositor_animation_->GetAnimation()->UpdateScrollTimeline(
-      scroll_timeline_util::GetCompositorScrollElementId(scroll_source),
-      start_scroll_offset, end_scroll_offset);
-}
-
 void Animation::AttachCompositedLayers() {
   if (!compositor_animation_)
     return;

third_party/blink/renderer/core/animation/animation.h

@@ -286,11 +286,6 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   // depends on computed values.
   virtual void FlushPendingUpdates() const {}
 
-  // TODO(yigu): This is a reverse dependency between AnimationTimeline and
-  // Animation. We should move the update logic once snapshotting is
-  // implemented. https://crbug.com/1060578.
-  void UpdateCompositorScrollTimeline();
-
  protected:
   DispatchEventResult DispatchEventInternal(Event&) override;
   void AddedEventListener(const AtomicString& event_type,

third_party/blink/renderer/core/animation/animation_timeline.cc

@@ -85,11 +85,10 @@ void AnimationTimeline::ServiceAnimations(TimingUpdateReason reason) {
   TRACE_EVENT0("blink", "AnimationTimeline::serviceAnimations");
 
   auto current_phase_and_time = CurrentPhaseAndTime();
-  bool maybe_update_compositor_scroll_timeline = false;
 
   if (IsScrollTimeline() &&
       last_current_phase_and_time_ != current_phase_and_time) {
-    maybe_update_compositor_scroll_timeline = true;
+    UpdateCompositorTimeline();
   }
 
   last_current_phase_and_time_ = current_phase_and_time;
@@ -102,12 +101,8 @@ void AnimationTimeline::ServiceAnimations(TimingUpdateReason reason) {
   std::sort(animations.begin(), animations.end(), CompareAnimations);
 
   for (Animation* animation : animations) {
-    if (!animation->Update(reason)) {
+    if (!animation->Update(reason))
       animations_needing_update_.erase(animation);
-      continue;
-    }
-    if (maybe_update_compositor_scroll_timeline)
-      animation->UpdateCompositorScrollTimeline();
   }
 
   DCHECK_EQ(outdated_animation_count_, 0U);

third_party/blink/renderer/core/animation/animation_timeline.h

@@ -89,6 +89,7 @@ class CORE_EXPORT AnimationTimeline : public ScriptWrappable {
     return compositor_timeline_.get();
   }
   virtual CompositorAnimationTimeline* EnsureCompositorTimeline() = 0;
+  virtual void UpdateCompositorTimeline() {}
 
   void MarkAnimationsCompositorPending(bool source_changed = false);
 

third_party/blink/renderer/core/animation/scroll_timeline.cc

@@ -440,4 +440,13 @@ CompositorAnimationTimeline* ScrollTimeline::EnsureCompositorTimeline() {
   return compositor_timeline_.get();
 }
 
+void ScrollTimeline::UpdateCompositorTimeline() {
+  if (!compositor_timeline_)
+    return;
+  compositor_timeline_->UpdateCompositorTimeline(
+      scroll_timeline_util::GetCompositorScrollElementId(
+          resolved_scroll_source_),
+      GetResolvedStartScrollOffset(), GetResolvedEndScrollOffset());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/animation/scroll_timeline.h

@@ -97,6 +97,8 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline {
   virtual void Invalidate();
 
   CompositorAnimationTimeline* EnsureCompositorTimeline() override;
+  void UpdateCompositorTimeline() override;
+
   // TODO(crbug.com/896249): These methods are temporary and currently required
   // to support worklet animations. Once worklet animations become animations
   // these methods will not be longer needed. They are used to keep track of

third_party/blink/renderer/modules/animationworklet/worklet_animation.cc

@@ -668,16 +668,9 @@ bool WorkletAnimation::UpdateOnCompositor() {
     StartEffectOnCompositor(compositor_animation_.get(), GetEffect());
   }
 
-  if (timeline_->IsScrollTimeline()) {
-    auto& timeline = To<ScrollTimeline>(*timeline_);
-    Node* scroll_source = timeline.ResolvedScrollSource();
-    auto start_scroll_offset = timeline.GetResolvedStartScrollOffset();
-    auto end_scroll_offset = timeline.GetResolvedEndScrollOffset();
-
-    compositor_animation_->UpdateScrollTimeline(
-        scroll_timeline_util::GetCompositorScrollElementId(scroll_source),
-        start_scroll_offset, end_scroll_offset);
-  }
+  if (timeline_->IsScrollTimeline())
+    timeline_->UpdateCompositorTimeline();
+
   compositor_animation_->UpdatePlaybackRate(playback_rate_);
   return true;
 }

third_party/blink/renderer/platform/animation/compositor_animation.cc

@@ -80,14 +80,6 @@ void CompositorAnimation::AbortKeyframeModel(int keyframe_model_id) {
   animation_->AbortKeyframeModel(keyframe_model_id);
 }
 
-void CompositorAnimation::UpdateScrollTimeline(
-    base::Optional<cc::ElementId> element_id,
-    base::Optional<double> start_scroll_offset,
-    base::Optional<double> end_scroll_offset) {
-  animation_->UpdateScrollTimeline(element_id, start_scroll_offset,
-                                   end_scroll_offset);
-}
-
 void CompositorAnimation::UpdatePlaybackRate(double playback_rate) {
   cc::ToWorkletAnimation(animation_.get())->UpdatePlaybackRate(playback_rate);
 }

third_party/blink/renderer/platform/animation/compositor_animation.h

@@ -57,9 +57,6 @@ class PLATFORM_EXPORT CompositorAnimation : public cc::AnimationDelegate {
   void PauseKeyframeModel(int keyframe_model_id, base::TimeDelta time_offset);
   void AbortKeyframeModel(int keyframe_model_id);
 
-  void UpdateScrollTimeline(base::Optional<cc::ElementId>,
-                            base::Optional<double> start_scroll_offset,
-                            base::Optional<double> end_scroll_offset);
   void UpdatePlaybackRate(double playback_rate);
 
  private:

third_party/blink/renderer/platform/animation/compositor_animation_timeline.cc

@@ -6,6 +6,7 @@
 
 #include "cc/animation/animation_host.h"
 #include "cc/animation/animation_id_provider.h"
+#include "cc/animation/scroll_timeline.h"
 #include "third_party/blink/renderer/platform/animation/compositor_animation.h"
 #include "third_party/blink/renderer/platform/animation/compositor_animation_client.h"
 
@@ -32,6 +33,15 @@ cc::AnimationTimeline* CompositorAnimationTimeline::GetAnimationTimeline()
   return animation_timeline_.get();
 }
 
+void CompositorAnimationTimeline::UpdateCompositorTimeline(
+    base::Optional<CompositorElementId> pending_id,
+    base::Optional<double> start_scroll_offset,
+    base::Optional<double> end_scroll_offset) {
+  ToScrollTimeline(animation_timeline_.get())
+      ->UpdateScrollerIdAndScrollOffsets(pending_id, start_scroll_offset,
+                                         end_scroll_offset);
+}
+
 void CompositorAnimationTimeline::AnimationAttached(
     const blink::CompositorAnimationClient& client) {
   if (client.GetCompositorAnimation()) {

third_party/blink/renderer/platform/animation/compositor_animation_timeline.h

@@ -10,7 +10,9 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "cc/animation/animation_timeline.h"
+#include "third_party/blink/renderer/platform/graphics/compositor_element_id.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
@@ -28,6 +30,9 @@ class PLATFORM_EXPORT CompositorAnimationTimeline {
   ~CompositorAnimationTimeline();
 
   cc::AnimationTimeline* GetAnimationTimeline() const;
+  void UpdateCompositorTimeline(base::Optional<CompositorElementId> pending_id,
+                                base::Optional<double> start_scroll_offset,
+                                base::Optional<double> end_scroll_offset);
 
   void AnimationAttached(const CompositorAnimationClient&);
   void AnimationDestroyed(const CompositorAnimationClient&);

third_party/blink/web_tests/external/wpt/scroll-animations/layout-changes-on-percentage-based-timeline.html

@@ -34,7 +34,7 @@ layout changes on percentage-based scroll offset">
   }
 
   #spacer {
-    height: 400px;
+    height: 80px;
   }
 
   .invisible {
@@ -70,15 +70,18 @@ layout changes on percentage-based scroll offset">
   const animation = new Animation(effect, timeline);
   animation.play();
 
-  // Moves the scroller to the end point (400px).
+  // Moves the scroller to the end point (240px).
   const maxScroll = scroller.scrollHeight - scroller.clientHeight;
-  scroller.scrollTop = maxScroll;
-
-  // Adds 400px to scroll height which pushes scroll progress back to 50%.
-  const spacer = document.getElementById('spacer');
-  spacer.classList.remove('invisible');
+  scroller.scrollTop = maxScroll * 0.6;
 
+  // Makes sure that the animation runs on compositor with current scroll offset
   waitForAnimationFrames(2).then(_ => {
-    takeScreenshot();
+    // Adds 80px to scroll height which pushes scroll progress back to 50%.
+    const spacer = document.getElementById('spacer');
+    spacer.classList.remove('invisible');
+    // Makes sure that the change is propagated to the compositor.
+    waitForAnimationFrames(2).then(_ => {
+      takeScreenshot();
+    });
   });
 </script>
\ No newline at end of file