[animation worklet] Do not set start_time_ if scroll timeline has unresolved current time

Currently when scroll timeline has unresolved current time, e.g. the
scroll source is not scrollable, we set WorkletAnimation::start_time_ to
base::TimeDelta::FromSecondsD(NaN) which is incorrect.

Other changes in the patch:
1. By spec [1], unresolved time values are represented by null. Update
tests to match that.
2. For WorkletAnimation with ScrollTimeline, it's legit not to set
start_time_. Update DCHECKs for this behavior.

[1] https://drafts.csswg.org/web-animations/#time-values-in-the-programming-interface

Bug: 905358
Change-Id: Ifde3890a6a4ad9e27dda8d4a78c5b6dcbf24ef54
Reviewed-on: https://chromium-review.googlesource.com/c/1336055
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608841}

cc/animation/worklet_animation.cc

@@ -108,6 +108,9 @@ void WorkletAnimation::UpdateInputState(MutatorInputState* input_state,
 
   double current_time =
       CurrentTime(monotonic_time, scroll_tree, is_active_tree);
+  // TODO(yigu): If current_time becomes newly unresolved and last_current_time_
+  // is resolved, we apply the last current time to the animation if the scroll
+  // timeline becomes newly inactive. See https://crbug.com/906050.
   last_current_time_ = current_time;
 
   switch (state_) {

third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime-nan.html

@@ -24,8 +24,8 @@ test(function() {
   const scrollTimeline = new ScrollTimeline(
       { scrollSource: scroller, timeRange: 100, orientation: 'block' });
 
-  assert_equals(scrollTimeline.currentTime, NaN);
-}, 'currentTime returns NaN for a display: inline scrollSource');
+  assert_equals(scrollTimeline.currentTime, null);
+}, 'currentTime should be null for a display: inline scrollSource');
 </script>
 
 <div id='displayNoneScroller' class='scroller' style='display: none;'>
@@ -37,8 +37,8 @@ test(function() {
   const scrollTimeline = new ScrollTimeline(
       { scrollSource: scroller, timeRange: 100, orientation: 'block' });
 
-  assert_equals(scrollTimeline.currentTime, NaN);
-}, 'currentTime returns NaN for a display: none scrollSource');
+  assert_equals(scrollTimeline.currentTime, null);
+}, 'currentTime should be null for a display: none scrollSource');
 </script>
 
 <script>
@@ -57,8 +57,8 @@ test(function() {
   const scrollTimeline = new ScrollTimeline(
       { scrollSource: scroller, timeRange: 100, orientation: 'block' });
 
-  assert_equals(scrollTimeline.currentTime, NaN);
-}, 'currentTime returns NaN for an unattached scrollSource');
+  assert_equals(scrollTimeline.currentTime, null);
+}, 'currentTime should be null for an unattached scrollSource');
 </script>
 
 <div id='notAScroller' class='scroller' style='overflow: visible;'>
@@ -70,6 +70,6 @@ test(function() {
   const scrollTimeline = new ScrollTimeline(
       { scrollSource: scroller, timeRange: 100, orientation: 'block' });
 
-  assert_equals(scrollTimeline.currentTime, NaN);
-}, 'currentTime returns NaN when the scrollSource is not a scroller');
+  assert_equals(scrollTimeline.currentTime, null);
+}, 'currentTime should be null when the scrollSource is not a scroller');
 </script>

third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime.html

@@ -266,13 +266,13 @@ test(function() {
 
   // Unscrolled, all timelines should read a current time of unresolved, since
   // the current offset (0) will be less than the startScrollOffset.
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
+  assert_equals(percentageScrollTimeline.currentTime, null);
+  assert_equals(calcScrollTimeline.currentTime, null);
 
   // Check the length-based ScrollTimeline.
   scroller.scrollTop = 19;
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
   scroller.scrollTop = 20;
   assert_equals(lengthScrollTimeline.currentTime, 0);
   scroller.scrollTop = 50;
@@ -286,7 +286,7 @@ test(function() {
 
   // Check the percentage-based ScrollTimeline.
   scroller.scrollTop = 0.19 * scrollerSize;
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
+  assert_equals(percentageScrollTimeline.currentTime, null);
   scroller.scrollTop = 0.20 * scrollerSize;
   assert_equals(percentageScrollTimeline.currentTime, 0);
   scroller.scrollTop = 0.50 * scrollerSize;
@@ -297,7 +297,7 @@ test(function() {
 
   // Check the calc-based ScrollTimeline.
   scroller.scrollTop = 0.2 * scrollerSize - 10;
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(calcScrollTimeline.currentTime, null);
   scroller.scrollTop = 0.2 * scrollerSize - 5;
   assert_equals(calcScrollTimeline.currentTime, 0);
   scroller.scrollTop = 0.2 * scrollerSize;
@@ -343,9 +343,9 @@ test(function() {
 
   // Unscrolled, all timelines should read a current time of unresolved, since
   // the current offset (0) will be less than the startScrollOffset.
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
+  assert_equals(percentageScrollTimeline.currentTime, null);
+  assert_equals(calcScrollTimeline.currentTime, null);
 
   // With direction rtl offsets are inverted, such that scrollLeft ==
   // scrollerSize is the 'zero' point for currentTime. However the
@@ -354,7 +354,7 @@ test(function() {
 
   // Check the length-based ScrollTimeline.
   scroller.scrollLeft = scrollerSize;
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
   scroller.scrollLeft = scrollerSize - 20;
   assert_equals(lengthScrollTimeline.currentTime, 0);
   scroller.scrollLeft = scrollerSize - 50;
@@ -368,7 +368,7 @@ test(function() {
 
   // Check the percentage-based ScrollTimeline.
   scroller.scrollLeft = scrollerSize - (0.19 * scrollerSize);
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
+  assert_equals(percentageScrollTimeline.currentTime, null);
   scroller.scrollLeft = scrollerSize - (0.20 * scrollerSize);
   assert_equals(percentageScrollTimeline.currentTime, 0);
   scroller.scrollLeft = scrollerSize - (0.4 * scrollerSize);
@@ -379,7 +379,7 @@ test(function() {
 
   // Check the calc-based ScrollTimeline.
   scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize - 10);
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(calcScrollTimeline.currentTime, null);
   scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize - 5);
   assert_equals(calcScrollTimeline.currentTime, 0);
   scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize);
@@ -424,7 +424,7 @@ test(function() {
 
   // Check the length-based ScrollTimeline.
   scroller.scrollTop = scrollerSize;
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
   scroller.scrollTop = scrollerSize - 20;
   assert_equals(
       lengthScrollTimeline.currentTime,
@@ -443,7 +443,7 @@ test(function() {
 
   // Check the percentage-based ScrollTimeline.
   scroller.scrollTop = 0.81 * scrollerSize;
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
+  assert_equals(percentageScrollTimeline.currentTime, null);
   scroller.scrollTop = 0.80 * scrollerSize;
   assert_equals(
       percentageScrollTimeline.currentTime,
@@ -457,7 +457,7 @@ test(function() {
 
   // Check the calc-based ScrollTimeline.
   scroller.scrollTop = 0.8 * scrollerSize + 6;
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(calcScrollTimeline.currentTime, null);
   scroller.scrollTop = 0.8 * scrollerSize + 5;
   assert_equals(
       calcScrollTimeline.currentTime,
@@ -504,7 +504,7 @@ test(function() {
 
   // Check the length-based ScrollTimeline.
   scroller.scrollLeft = 0;
-  assert_equals(lengthScrollTimeline.currentTime, NaN);
+  assert_equals(lengthScrollTimeline.currentTime, null);
   scroller.scrollLeft = 20;
   assert_equals(
        lengthScrollTimeline.currentTime,
@@ -523,7 +523,7 @@ test(function() {
 
   // Check the percentage-based ScrollTimeline.
   scroller.scrollLeft = 0.19 * scrollerSize;
-  assert_equals(percentageScrollTimeline.currentTime, NaN);
+  assert_equals(percentageScrollTimeline.currentTime, null);
   scroller.scrollLeft = 0.20 * scrollerSize;
   assert_equals(
       percentageScrollTimeline.currentTime,
@@ -537,7 +537,7 @@ test(function() {
 
   // Check the calc-based ScrollTimeline. 80% + 5px
   scroller.scrollLeft = 0.2 * scrollerSize - 10;
-  assert_equals(calcScrollTimeline.currentTime, NaN);
+  assert_equals(calcScrollTimeline.currentTime, null);
   scroller.scrollLeft = 0.2 * scrollerSize - 5;
   assert_equals(
       calcScrollTimeline.currentTime,
@@ -592,7 +592,7 @@ test(function() {
   });
 
   scroller.scrollTop = 150;
-  assert_equals(scrollTimeline.currentTime, NaN);
+  assert_equals(scrollTimeline.currentTime, null);
 }, 'currentTime handles startScrollOffset == endScrollOffset correctly');
 
 test(function() {
@@ -611,6 +611,6 @@ test(function() {
   });
 
   scroller.scrollTop = 150;
-  assert_equals(scrollTimeline.currentTime, NaN);
+  assert_equals(scrollTimeline.currentTime, null);
 }, 'currentTime handles startScrollOffset > endScrollOffset correctly');
 </script>

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

@@ -116,11 +116,11 @@ ScrollTimeline::ScrollTimeline(Element* scroll_source,
 }
 
 double ScrollTimeline::currentTime(bool& is_null) {
+  is_null = true;
   // 1. If scrollSource does not currently have a CSS layout box, or if its
   // layout box is not a scroll container, return an unresolved time value.
   LayoutBox* layout_box = ResolvedScrollSource()->GetLayoutBox();
   if (!layout_box || !layout_box->HasOverflowClip()) {
-    is_null = false;
     return std::numeric_limits<double>::quiet_NaN();
   }
 

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

@@ -6,6 +6,7 @@
 
 #include "testing/gtest/include/gtest/gtest.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"
 #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
@@ -58,4 +59,104 @@ TEST_F(ScrollTimelineTest,
   EXPECT_EQ(scroller->Layer()->GetCompositingState(), kNotComposited);
 }
 
+TEST_F(ScrollTimelineTest, CurrentTimeIsNullIfScrollSourceIsNotScrollable) {
+  SetBodyInnerHTML(R"HTML(
+    <style>#scroller { width: 100px; height: 100px; }</style>
+    <div id='scroller'></div>
+  )HTML");
+
+  LayoutBoxModelObject* scroller =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
+  ASSERT_TRUE(scroller);
+
+  ScrollTimelineOptions* options = ScrollTimelineOptions::Create();
+  DoubleOrScrollTimelineAutoKeyword time_range =
+      DoubleOrScrollTimelineAutoKeyword::FromDouble(100);
+  options->setTimeRange(time_range);
+  options->setScrollSource(GetElementById("scroller"));
+  ScrollTimeline* scroll_timeline =
+      ScrollTimeline::Create(GetDocument(), options, ASSERT_NO_EXCEPTION);
+
+  bool current_time_is_null = false;
+  scroll_timeline->currentTime(current_time_is_null);
+  EXPECT_TRUE(current_time_is_null);
+}
+
+TEST_F(ScrollTimelineTest,
+       CurrentTimeIsNullIfScrollOffsetIsBeyondStartAndEndScrollOffset) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #scroller { overflow: scroll; width: 100px; height: 100px; }
+      #spacer { height: 1000px; }
+    </style>
+    <div id='scroller'>
+      <div id ='spacer'></div>
+    </div>
+  )HTML");
+
+  LayoutBoxModelObject* scroller =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
+  ASSERT_TRUE(scroller);
+  ASSERT_TRUE(scroller->HasOverflowClip());
+  PaintLayerScrollableArea* scrollable_area = scroller->GetScrollableArea();
+  ASSERT_TRUE(scrollable_area);
+  ScrollTimelineOptions* options = ScrollTimelineOptions::Create();
+  DoubleOrScrollTimelineAutoKeyword time_range =
+      DoubleOrScrollTimelineAutoKeyword::FromDouble(100);
+  options->setTimeRange(time_range);
+  options->setScrollSource(GetElementById("scroller"));
+  options->setStartScrollOffset("10px");
+  options->setEndScrollOffset("90px");
+  ScrollTimeline* scroll_timeline =
+      ScrollTimeline::Create(GetDocument(), options, ASSERT_NO_EXCEPTION);
+
+  bool current_time_is_null = false;
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 5), kProgrammaticScroll);
+  scroll_timeline->currentTime(current_time_is_null);
+  EXPECT_TRUE(current_time_is_null);
+
+  current_time_is_null = true;
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 50), kProgrammaticScroll);
+  scroll_timeline->currentTime(current_time_is_null);
+  EXPECT_FALSE(current_time_is_null);
+
+  current_time_is_null = false;
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 100), kProgrammaticScroll);
+  scroll_timeline->currentTime(current_time_is_null);
+  EXPECT_TRUE(current_time_is_null);
+}
+
+TEST_F(ScrollTimelineTest,
+       CurrentTimeIsNullIfEndScrollOffsetIsLessThanStartScrollOffset) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #scroller { overflow: scroll; width: 100px; height: 100px; }
+      #spacer { height: 1000px; }
+    </style>
+    <div id='scroller'>
+      <div id ='spacer'></div>
+    </div>
+  )HTML");
+
+  LayoutBoxModelObject* scroller =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
+  ASSERT_TRUE(scroller);
+  ASSERT_TRUE(scroller->HasOverflowClip());
+  PaintLayerScrollableArea* scrollable_area = scroller->GetScrollableArea();
+  ASSERT_TRUE(scrollable_area);
+  ScrollTimelineOptions* options = ScrollTimelineOptions::Create();
+  DoubleOrScrollTimelineAutoKeyword time_range =
+      DoubleOrScrollTimelineAutoKeyword::FromDouble(100);
+  options->setTimeRange(time_range);
+  options->setScrollSource(GetElementById("scroller"));
+  options->setStartScrollOffset("80px");
+  options->setEndScrollOffset("40px");
+  ScrollTimeline* scroll_timeline =
+      ScrollTimeline::Create(GetDocument(), options, ASSERT_NO_EXCEPTION);
+
+  bool current_time_is_null = false;
+  scrollable_area->SetScrollOffset(ScrollOffset(0, 50), kProgrammaticScroll);
+  scroll_timeline->currentTime(current_time_is_null);
+  EXPECT_TRUE(current_time_is_null);
+}
 }  //  namespace blink

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

@@ -404,7 +404,8 @@ void WorkletAnimation::Update(TimingUpdateReason reason) {
   if (play_state_ != Animation::kRunning)
     return;
 
-  if (!start_time_)
+  // ScrollTimeline animation doesn't require start_time_ to be set.
+  if (!start_time_ && !timeline_->IsScrollTimeline())
     return;
 
   DCHECK_EQ(effects_.size(), local_times_.size());
@@ -599,16 +600,22 @@ void WorkletAnimation::UpdateInputState(
   bool was_active = IsActive(last_play_state_);
   bool is_active = IsActive(play_state_);
 
-  DCHECK(start_time_);
+  // ScrollTimeline animation doesn't require start_time_ to be set.
+  DCHECK(start_time_ || timeline_->IsScrollTimeline());
   DCHECK(last_current_time_ || !was_active);
   bool is_null;
   double current_time = timeline_->currentTime(is_null);
 
   bool did_time_change =
       !last_current_time_ || current_time != last_current_time_->InSecondsF();
+  // TODO(yigu): If current_time becomes newly unresolved and last_current_time_
+  // is resolved, we apply the last current time to the animation if the scroll
+  // timeline becomes newly inactive. See https://crbug.com/906050.
   last_current_time_ = base::TimeDelta::FromSecondsD(current_time);
 
   if (!was_active && is_active) {
+    // TODO(yigu): current_time should be offset by start_time_ for animations
+    // with document timeline. https://crbug.com/905405.
     input_state->Add(
         {id_,
          std::string(animator_name_.Ascii().data(), animator_name_.length()),