[scroll-animations] Avoid creating a timeline for every CSSAnimation

Currently each CSSAnimation that references a @scroll-timeline
gets its own CSSScrollTimeline instance (even in cases where the same
animation-timeline name is used). This is a bit excessive, so this CL
adds a mechanism for reusing matching instances.

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

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

@@ -448,22 +448,40 @@ CSSScrollTimeline* CreateCSSScrollTimeline(
   return scroll_timeline;
 }
 
+CSSScrollTimeline* FindMatchingCachedTimeline(
+    Document& document,
+    const AtomicString& name,
+    const CSSScrollTimeline::Options& options) {
+  auto* cached_timeline = DynamicTo<CSSScrollTimeline>(
+      document.GetDocumentAnimations().FindCachedCSSScrollTimeline(name));
+  if (cached_timeline && cached_timeline->Matches(options))
+    return cached_timeline;
+  return nullptr;
+}
+
 AnimationTimeline* ComputeTimeline(Element* element,
                                    const StyleNameOrKeyword& timeline_name,
                                    StyleRuleScrollTimeline* rule,
                                    AnimationTimeline* existing_timeline) {
+  Document& document = element->GetDocument();
   if (timeline_name.IsKeyword()) {
     if (timeline_name.GetKeyword() == CSSValueID::kAuto)
-      return &element->GetDocument().Timeline();
+      return &document.Timeline();
     DCHECK_EQ(timeline_name.GetKeyword(), CSSValueID::kNone);
     return nullptr;
   }
   if (rule) {
     CSSScrollTimeline::Options options(element, *rule);
 
-    // When the incoming options match the existing timeline, we can continue
-    // to use the existing timeline, since creating a new timeline from
-    // the options would just yield an identical timeline.
+    const AtomicString& name = timeline_name.GetName().GetValue();
+    // When multiple animations refer to the same @scroll-timeline, the same
+    // CSSScrollTimeline instance should be shared.
+    if (auto* timeline = FindMatchingCachedTimeline(document, name, options))
+      return timeline;
+    // When the incoming options match the existing timeline (associated with
+    // an existing animation), we can continue to use the existing timeline,
+    // since creating a new timeline from the options would just yield an
+    // identical timeline.
     if (auto* timeline = DynamicTo<CSSScrollTimeline>(existing_timeline)) {
       if (timeline->Matches(options))
         return existing_timeline;

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

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/animation/css/css_scroll_timeline.h"
 
+#include "third_party/blink/renderer/core/animation/document_animations.h"
 #include "third_party/blink/renderer/core/css/css_element_offset_value.h"
 #include "third_party/blink/renderer/core/css/css_id_selector_value.h"
 #include "third_party/blink/renderer/core/css/css_identifier_value.h"
@@ -220,6 +221,11 @@ CSSScrollTimeline::CSSScrollTimeline(Document* document, const Options& options)
       rule_(options.rule_) {
   DCHECK(options.IsValid());
   DCHECK(rule_);
+  document->GetDocumentAnimations().CacheCSSScrollTimeline(*this);
+}
+
+const AtomicString& CSSScrollTimeline::Name() const {
+  return rule_->GetName();
 }
 
 bool CSSScrollTimeline::Matches(const Options& options) const {

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

@@ -40,6 +40,8 @@ class CORE_EXPORT CSSScrollTimeline : public ScrollTimeline {
 
   CSSScrollTimeline(Document*, const Options&);
 
+  const AtomicString& Name() const;
+
   bool Matches(const Options&) const;
 
   // AnimationTimeline implementation.
@@ -47,6 +49,10 @@ class CORE_EXPORT CSSScrollTimeline : public ScrollTimeline {
   void AnimationAttached(Animation*) override;
   void AnimationDetached(Animation*) override;
 
+  // If a CSSScrollTimeline matching |options| already exists, return that
+  // timeline. Otherwise returns nullptr.
+  static CSSScrollTimeline* FindMatchingTimeline(const Options&);
+
   void Trace(Visitor*) const override;
 
  private:

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

@@ -151,4 +151,70 @@ TEST_F(CSSScrollTimelineTest, IdObserverRuleInsertion) {
   EXPECT_FALSE(HasObservers("offset2"));
 }
 
+TEST_F(CSSScrollTimelineTest, SharedTimelines) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      @keyframes anim1 { to { top: 200px; } }
+      @keyframes anim2 { to { left: 200px; } }
+      @keyframes anim3 { to { right: 200px; } }
+      @scroll-timeline timeline1 {
+        source: selector(#scroller);
+        time-range: 10s;
+      }
+      @scroll-timeline timeline2 {
+        source: selector(#scroller);
+        time-range: 10s;
+      }
+      #scroller {
+        height: 100px;
+        overflow: scroll;
+      }
+      #scroller > div {
+        height: 200px;
+      }
+    </style>
+    <div id=scroller><div></div></div>
+    <main id=main></main>
+  )HTML");
+  // #scroller etc is created in a separate lifecycle phase to ensure that
+  // we get a layout box for #scroller before the animations are started.
+
+  Element* main = GetDocument().getElementById("main");
+  ASSERT_TRUE(main);
+  main->setInnerHTML(R"HTML(
+    <style>
+      #element1, #element2 {
+        animation-name: anim1, anim2, anim3;
+        animation-duration: 10s;
+        animation-timeline: timeline1, timeline1, timeline2;
+      }
+    </style>
+    <div id=element1></div>
+    <div id=element2></div>
+  )HTML");
+  UpdateAllLifecyclePhasesForTest();
+
+  Element* element1 = GetDocument().getElementById("element1");
+  Element* element2 = GetDocument().getElementById("element2");
+  ASSERT_TRUE(element1);
+  ASSERT_TRUE(element2);
+  HeapVector<Member<Animation>> animations1 = element1->getAnimations();
+  HeapVector<Member<Animation>> animations2 = element2->getAnimations();
+  EXPECT_EQ(3u, animations1.size());
+  EXPECT_EQ(3u, animations2.size());
+
+  // The animations associated with anim1 and anim2 should share the same
+  // timeline instance, also across elements.
+  EXPECT_EQ(animations1[0]->timeline(), animations1[1]->timeline());
+  EXPECT_EQ(animations1[1]->timeline(), animations2[0]->timeline());
+  EXPECT_EQ(animations2[0]->timeline(), animations2[1]->timeline());
+
+  // The animation associated with anim3 uses a different timeline
+  // from anim1/2.
+  EXPECT_EQ(animations1[2]->timeline(), animations2[2]->timeline());
+
+  EXPECT_NE(animations2[2]->timeline(), animations1[0]->timeline());
+  EXPECT_NE(animations2[2]->timeline(), animations1[1]->timeline());
+}
+
 }  // namespace blink

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

@@ -33,6 +33,7 @@
 #include "cc/animation/animation_host.h"
 #include "third_party/blink/renderer/core/animation/animation_clock.h"
 #include "third_party/blink/renderer/core/animation/animation_timeline.h"
+#include "third_party/blink/renderer/core/animation/css/css_scroll_timeline.h"
 #include "third_party/blink/renderer/core/animation/keyframe_effect.h"
 #include "third_party/blink/renderer/core/animation/pending_animations.h"
 #include "third_party/blink/renderer/core/animation/worklet_animation_controller.h"
@@ -161,9 +162,20 @@ HeapVector<Member<Animation>> DocumentAnimations::getAnimations(
   return animations;
 }
 
+void DocumentAnimations::CacheCSSScrollTimeline(CSSScrollTimeline& timeline) {
+  // We cache the least seen CSSScrollTimeline for a given name.
+  cached_css_timelines_.Set(timeline.Name(), &timeline);
+}
+
+CSSScrollTimeline* DocumentAnimations::FindCachedCSSScrollTimeline(
+    const AtomicString& name) {
+  return To<CSSScrollTimeline>(cached_css_timelines_.at(name));
+}
+
 void DocumentAnimations::Trace(Visitor* visitor) const {
   visitor->Trace(document_);
   visitor->Trace(timelines_);
+  visitor->Trace(cached_css_timelines_);
 }
 
 void DocumentAnimations::GetAnimationsTargetingTreeScope(

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

@@ -38,6 +38,7 @@
 namespace blink {
 
 class AnimationTimeline;
+class CSSScrollTimeline;
 class Document;
 class PaintArtifactCompositor;
 
@@ -68,6 +69,10 @@ class CORE_EXPORT DocumentAnimations final
   void MarkAnimationsCompositorPending();
 
   HeapVector<Member<Animation>> getAnimations(const TreeScope&);
+
+  void CacheCSSScrollTimeline(CSSScrollTimeline&);
+  CSSScrollTimeline* FindCachedCSSScrollTimeline(const AtomicString&);
+
   const HeapHashSet<WeakMember<AnimationTimeline>>& GetTimelinesForTesting()
       const {
     return timelines_;
@@ -83,6 +88,14 @@ class CORE_EXPORT DocumentAnimations final
  private:
   Member<Document> document_;
   HeapHashSet<WeakMember<AnimationTimeline>> timelines_;
+
+  // We cache CSSScrollTimelines by name, such that multiple animations using
+  // the same timeline can use the same CSSScrollTimeline instance.
+  //
+  // Note that timelines present in |cached_css_timelines_| are also present
+  // in |timelines_|.
+  HeapHashMap<AtomicString, WeakMember<AnimationTimeline>>
+      cached_css_timelines_;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_rule.h

@@ -226,7 +226,7 @@ class CORE_EXPORT StyleRuleScrollTimeline : public StyleRuleBase {
 
   void TraceAfterDispatch(blink::Visitor*) const;
 
-  const String& GetName() const { return name_; }
+  const AtomicString& GetName() const { return name_; }
   const CSSValue* GetSource() const { return source_; }
   const CSSValue* GetOrientation() const { return orientation_; }
   const CSSValue* GetStart() const { return start_; }
@@ -234,7 +234,7 @@ class CORE_EXPORT StyleRuleScrollTimeline : public StyleRuleBase {
   const CSSValue* GetTimeRange() const { return time_range_; }
 
  private:
-  String name_;
+  AtomicString name_;
   Member<const CSSValue> source_;
   Member<const CSSValue> orientation_;
   Member<const CSSValue> start_;