[scroll-animations] Handle element-based offsets with null targets

Currently we assume that element-based offsets always have non-null
targets ResolveOffset-time, since we enforce this during the creation
of the (non-CSS) ScrollTimelines.

However, for CSSScrollTimelines, we can't invalidate the rule this way,
since we're referring to an element indirectly via an ID, not with a
pointer to a specific element.

Checking whether or not the target is null during ResolveOffset fixes
this, and it's also what we're supposed to be doing per spec.

This fixes a crash when using @scroll-timelines with element-based
offsets that reference non-existent IDs.

Also removed a nearby comment that's outdated (but not strictly
related to the change in this CL).

Bug: 1074052, 1102788
Change-Id: I4e138b55c3ef8cb122c58755a72a586e94c97e69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2498463
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821682}

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

@@ -129,15 +129,8 @@ base::Optional<double> ScrollTimelineOffset::ResolveOffset(
 
     return resolved;
   } else if (element_based_offset_) {
-    // We assume that the root is the target's ancestor in layout tree. Under
-    // this assumption |target.LocalToAncestorRect()| returns the targets's
-    // position relative to the root's border box, while ignoring scroll offset.
-    //
-    // TODO(majidvp): We need to validate this assumption and deal with cases
-    // where it is not true. See the spec discussion here:
-    // https://github.com/w3c/csswg-drafts/issues/4337#issuecomment-610989843
-
-    DCHECK(element_based_offset_->hasTarget());
+    if (!element_based_offset_->hasTarget())
+      return base::nullopt;
     Element* target = element_based_offset_->target();
     const LayoutBox* target_box = target->GetLayoutBox();
 

third_party/blink/web_tests/external/wpt/scroll-animations/css/at-scroll-timeline-element-offsets.html

@@ -77,6 +77,12 @@
     start: selector(#offset_display_none);
     end: auto;
   }
+  @scroll-timeline timeline_null_target {
+    source: selector(#scroller);
+    time-range: 10s;
+    start: selector(#no_such_id);
+    end: selector(#no_such_id);
+  }
 
   #container > div {
     width: 0px;
@@ -97,6 +103,7 @@
   #element_50px_end { animation-timeline: timeline_50px_end; }
   #element_outside { animation-timeline: timeline_outside; }
   #element_display_none { animation-timeline: timeline_display_none; }
+  #element_null_target { animation-timeline: timeline_null_target; }
 </style>
 <div id=scroller>
   <div id=contents>
@@ -119,6 +126,7 @@
   <div id=element_50px_end></div>
   <div id=element_outside></div>
   <div id=element_display_none></div>
+  <div id=element_null_target></div>
 </div>
 <script>
 
@@ -238,4 +246,12 @@
   test_width_at_scroll_top(element_display_none, 400, '0px');
   test_width_at_scroll_top(element_display_none, 450, '0px');
 
+  // Target of element-based offset is null (=> no effect value)
+  test_width_at_scroll_top(element_null_target, 0, '0px');
+  test_width_at_scroll_top(element_null_target, 100, '0px');
+  test_width_at_scroll_top(element_null_target, 200, '0px');
+  test_width_at_scroll_top(element_null_target, 300, '0px');
+  test_width_at_scroll_top(element_null_target, 400, '0px');
+  test_width_at_scroll_top(element_null_target, 450, '0px');
+
 </script>