Schedule frame for scroll linked animations only when scrolling changes

Scheduling frame for scroll linked animations regardless of the scroller
updates makes scroll animations inefficient. This change schedules next
frame only when scroll timeline current time changes. The change
assumes that animation timeline scheduling runs (via
DocumentAnimations::UpdateAnimations) at the end of any frame in which
scroll timeline (i.e. its offset, range, overflow) is invalidated.


Bug: 1039750
Change-Id: Ib87a298e6d09cbeb85276cea4c165022b76f3732
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990003
Commit-Queue: Olga Gerchikov <gerchiko@microsoft.com>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732987}

cc/trees/layer_tree_host_impl.cc

@@ -5293,6 +5293,8 @@ bool LayerTreeHostImpl::AnimateLayers(base::TimeTicks monotonic_time,
   // animation_host, so on the last frame of an animation we will
   // still request an extra SetNeedsAnimate here.
   if (animated) {
+    // TODO(crbug.com/1039750): If only scroll animations present, schedule a
+    // frame only if scroll changes.
     SetNeedsOneBeginImplFrame();
     frame_trackers_.StartSequence(
         FrameSequenceTrackerType::kCompositorAnimation);
@@ -5301,7 +5303,7 @@ bool LayerTreeHostImpl::AnimateLayers(base::TimeTicks monotonic_time,
         FrameSequenceTrackerType::kCompositorAnimation);
   }
 
-  // TODO(crbug.com/551138): We could return true only if the animaitons are on
+  // TODO(crbug.com/551138): We could return true only if the animations are on
   // the active tree. There's no need to cause a draw to take place from
   // animations starting/ticking on the pending tree.
   return animated;

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

@@ -143,9 +143,11 @@ ScrollTimeline::InitialStartTimeForAnimations() {
 }
 
 void ScrollTimeline::ScheduleNextService() {
+  DCHECK_EQ(outdated_animation_count_, 0U);
   if (AnimationsNeedingUpdateCount() == 0)
     return;
-  ScheduleServiceOnNextFrame();
+  if (CurrentTimeInternal() != last_current_time_internal_)
+    ScheduleServiceOnNextFrame();
 }
 
 base::Optional<base::TimeDelta> ScrollTimeline::CurrentTimeInternal() {

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

@@ -23,7 +23,7 @@ namespace blink {
 // control the conversion of scroll amount to time output.
 //
 // Spec: https://wicg.github.io/scroll-animations/#scroll-timelines
-class CORE_EXPORT ScrollTimeline final : public AnimationTimeline {
+class CORE_EXPORT ScrollTimeline : public AnimationTimeline {
   DEFINE_WRAPPERTYPEINFO();
 
  public:

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

@@ -7,6 +7,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/animation/keyframe_effect.h"
 #include "third_party/blink/renderer/core/animation/keyframe_effect_model.h"
+#include "third_party/blink/renderer/core/css/css_numeric_literal_value.h"
 #include "third_party/blink/renderer/core/paint/paint_layer.h"
 #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
@@ -22,6 +23,40 @@ class ScrollTimelineTest : public RenderingTest {
   }
 };
 
+class TestScrollTimeline : public ScrollTimeline {
+ public:
+  TestScrollTimeline(
+      Document* document,
+      Element* scroll_source,
+      CSSPrimitiveValue* start_scroll_offset =
+          CSSNumericLiteralValue::Create(10.0,
+                                         CSSPrimitiveValue::UnitType::kPixels),
+      CSSPrimitiveValue* end_scroll_offset =
+          CSSNumericLiteralValue::Create(90.0,
+                                         CSSPrimitiveValue::UnitType::kPixels))
+      : ScrollTimeline(document,
+                       scroll_source,
+                       ScrollTimeline::Vertical,
+                       start_scroll_offset,
+                       end_scroll_offset,
+                       100.0,
+                       Timing::FillMode::NONE),
+        next_service_scheduled_(false) {}
+
+  void ScheduleServiceOnNextFrame() override {
+    ScrollTimeline::ScheduleServiceOnNextFrame();
+    next_service_scheduled_ = true;
+  }
+  void Trace(blink::Visitor* visitor) override {
+    ScrollTimeline::Trace(visitor);
+  }
+  bool NextServiceScheduled() const { return next_service_scheduled_; }
+  void ResetNextServiceScheduled() { next_service_scheduled_ = false; }
+
+ private:
+  bool next_service_scheduled_;
+};
+
 TEST_F(ScrollTimelineTest, CurrentTimeIsNullIfScrollSourceIsNotScrollable) {
   SetBodyInnerHTML(R"HTML(
     <style>#scroller { width: 100px; height: 100px; }</style>
@@ -220,4 +255,102 @@ TEST_F(ScrollTimelineTest, AttachOrDetachAnimationWithNullScrollSource) {
   EXPECT_EQ(0u, scroll_timeline->GetAnimations().size());
 }
 
+TEST_F(ScrollTimelineTest, ScheduleFrameOnlyWhenScrollOffsetChanges) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #scroller { overflow: scroll; width: 100px; height: 100px; }
+      #spacer { width: 200px; height: 200px; }
+    </style>
+    <div id='scroller'>
+      <div id ='spacer'></div>
+    </div>
+  )HTML");
+
+  LayoutBoxModelObject* scroller =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
+  PaintLayerScrollableArea* scrollable_area = scroller->GetScrollableArea();
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 20), kProgrammaticScroll);
+
+  Element* scroller_element = GetElementById("scroller");
+  TestScrollTimeline* scroll_timeline =
+      MakeGarbageCollected<TestScrollTimeline>(&GetDocument(),
+                                               scroller_element);
+
+  NonThrowableExceptionState exception_state;
+  Timing timing;
+  timing.iteration_duration = AnimationTimeDelta::FromSecondsD(30);
+  Animation* scroll_animation =
+      Animation::Create(MakeGarbageCollected<KeyframeEffect>(
+                            nullptr,
+                            MakeGarbageCollected<StringKeyframeEffectModel>(
+                                StringKeyframeVector()),
+                            timing),
+                        scroll_timeline, exception_state);
+  scroll_animation->play();
+  UpdateAllLifecyclePhasesForTest();
+
+  // Validate that no frame is scheduled when there is no scroll change.
+  scroll_timeline->ResetNextServiceScheduled();
+  scroll_timeline->ScheduleNextService();
+  EXPECT_FALSE(scroll_timeline->NextServiceScheduled());
+
+  // Validate that frame is scheduled when scroll changes.
+  scroll_timeline->ResetNextServiceScheduled();
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 30), kProgrammaticScroll);
+  scroll_timeline->ScheduleNextService();
+  EXPECT_TRUE(scroll_timeline->NextServiceScheduled());
+}
+
+// TODO(crbug.com/944449): Currently DCHECK, verifying that animations do not
+// need an update after layout phase, fails. Scroll timeline snapshotting will
+// fix it.
+#if DCHECK_IS_ON()
+#define MAYBE_ScheduleFrameWhenScrollerLayoutChanges \
+  DISABLED_ScheduleFrameWhenScrollerLayoutChanges
+#else  // DCHECK_IS_ON()
+#define MAYBE_ScheduleFrameWhenScrollerLayoutChanges \
+  ScheduleFrameWhenScrollerLayoutChanges
+#endif
+// This test verifies scenario when scroll timeline is updated as a result of
+// layout run. In this case the expectation is that at the end of paint
+// lifecycle phase scroll timeline schedules a new frame that runs animations
+// update.
+TEST_F(ScrollTimelineTest, MAYBE_ScheduleFrameWhenScrollerLayoutChanges) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #scroller { overflow: scroll; width: 100px; height: 100px; }
+      #spacer { width: 200px; height: 200px; }
+    </style>
+    <div id='scroller'>
+      <div id ='spacer'></div>
+    </div>
+  )HTML");
+  LayoutBoxModelObject* scroller =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
+  PaintLayerScrollableArea* scrollable_area = scroller->GetScrollableArea();
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 20), kProgrammaticScroll);
+  Element* scroller_element = GetElementById("scroller");
+  TestScrollTimeline* scroll_timeline =
+      MakeGarbageCollected<TestScrollTimeline>(&GetDocument(), scroller_element,
+                                               nullptr, nullptr);
+  NonThrowableExceptionState exception_state;
+  Timing timing;
+  timing.iteration_duration = AnimationTimeDelta::FromSecondsD(30);
+  Animation* scroll_animation =
+      Animation::Create(MakeGarbageCollected<KeyframeEffect>(
+                            nullptr,
+                            MakeGarbageCollected<StringKeyframeEffectModel>(
+                                StringKeyframeVector()),
+                            timing),
+                        scroll_timeline, exception_state);
+  scroll_animation->play();
+  UpdateAllLifecyclePhasesForTest();
+  // Validate that frame is scheduled when scroller layout changes.
+  Element* spacer_element = GetElementById("spacer");
+  spacer_element->setAttribute(html_names::kStyleAttr, "height:1000px;");
+  scroll_timeline->ResetNextServiceScheduled();
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_TRUE(scroll_timeline->NextServiceScheduled());
+}
+
 }  //  namespace blink

third_party/blink/web_tests/external/wpt/scroll-animations/scroll-animation.html

@@ -70,8 +70,17 @@
     // Now do some scrolling and make sure that the Animation current time is
     // correct.
     scroller.scrollTop = 0.2 * maxScroll;
+    // TODO(crbug.com/944449): After scroll offset snapshotting is implemented,
+    // scroll timeline current time, animation current time and effect local
+    // time will be updated on the same frame (which in this case will be the
+    // next frame).
     assert_equals(animation.currentTime, animation.timeline.currentTime,
       "The current time corresponds to the scroll position of the scroller.");
+    await waitForNextFrame();
+    assert_times_equal(
+      animation.effect.getComputedTiming().localTime,
+      animation.timeline.currentTime,
+      'Effect local time corresponds to the scroll position of the scroller.');
 }, 'Animation start and current times are correct for each animation state.');
 
 promise_test(async t => {