Bug 1821416 - Drop the element property usage from ScrollTimelineSet. r=emilio

We are doing the following things here:
1. Rename ScrollTimelineSet to ProgressTimelineScheduler because this
   class is used to schedule animations with progress timelines, including
   scroll timelines and view timelines.
2. Drop the element property usage and let ElementAnimationData store
   ProgressTimelineScheduler.
3. We avoid using the generated content in ScrollTimeline::Scroller.
   Instead, we use a pair of Element and PseudoStyleType to represent
   ScrollTimeline::Scroller.

We hit the assertion because the generatd content may change and so we
shouldn't use it as the ScrollTimeline::Scroller.

Differential Revision: https://phabricator.services.mozilla.com/D172610

dom/animation/ElementAnimationData.cpp

@@ -26,6 +26,7 @@ void ElementAnimationData::ClearAllAnimationCollections() {
     data->mAnimations = nullptr;
     data->mTransitions = nullptr;
     data->mScrollTimelines = nullptr;
+    data->mProgressTimelineScheduler = nullptr;
   }
 }
 
@@ -73,6 +74,14 @@ ElementAnimationData::PerElementOrPseudoData::DoEnsureScrollTimelines(
   return *mScrollTimelines;
 }
 
+dom::ProgressTimelineScheduler&
+ElementAnimationData::PerElementOrPseudoData::DoEnsureProgressTimelineScheduler(
+    dom::Element& aOwner, PseudoStyleType aType) {
+  MOZ_ASSERT(!mProgressTimelineScheduler);
+  mProgressTimelineScheduler = MakeUnique<dom::ProgressTimelineScheduler>();
+  return *mProgressTimelineScheduler;
+}
+
 void ElementAnimationData::PerElementOrPseudoData::DoClearEffectSet() {
   MOZ_ASSERT(mEffectSet);
   mEffectSet = nullptr;
@@ -93,4 +102,10 @@ void ElementAnimationData::PerElementOrPseudoData::DoClearScrollTimelines() {
   mScrollTimelines = nullptr;
 }
 
+void ElementAnimationData::PerElementOrPseudoData::
+    DoClearProgressTimelineScheduler() {
+  MOZ_ASSERT(mProgressTimelineScheduler);
+  mProgressTimelineScheduler = nullptr;
+}
+
 }  // namespace mozilla

dom/animation/ElementAnimationData.h

@@ -23,6 +23,7 @@ namespace dom {
 class Element;
 class CSSAnimation;
 class CSSTransition;
+class ProgressTimelineScheduler;
 class ScrollTimeline;
 }  // namespace dom
 using CSSAnimationCollection = AnimationCollection<dom::CSSAnimation>;
@@ -47,6 +48,28 @@ class ElementAnimationData {
     UniquePtr<ScrollTimelineCollection> mScrollTimelines;
     // TODO: Bug 1737920. Add support for ViewTimeline.
 
+    // This is different from |mScrollTimelines|. We use this to schedule all
+    // scroll-driven animations (which use anonymous/named scroll timelines or
+    // anonymous/name view timelines) for a specific scroll source (which is the
+    // element with nsIScrollableFrame).
+    //
+    // TimelineCollection owns and manages the named progress timeline generated
+    // by specifying scroll-timeline-name property and view-timeline-name
+    // property on this element. However, the anonymous progress timelines (e.g.
+    // animation-timeline:scroll()) are owned by Animation objects only.
+    //
+    // Note:
+    // 1. For named scroll timelines. The element which specifies
+    //    scroll-timeline-name is the scroll source. However, for named view
+    //    timelines, the element which specifies view-timeline-name may not be
+    //    the scroll source because we use its nearest scroll container as the
+    //    scroll source.
+    // 2. For anonymous progress timelines, we don't keep their timeline obejcts
+    //    in TimelineCollection.
+    // So, per 1) and 2), we use |mProgressTimelineScheduler| for the scroll
+    // source element to schedule scroll-driven animations.
+    UniquePtr<dom::ProgressTimelineScheduler> mProgressTimelineScheduler;
+
     PerElementOrPseudoData();
     ~PerElementOrPseudoData();
 
@@ -56,10 +79,13 @@ class ElementAnimationData {
     CSSAnimationCollection& DoEnsureAnimations(dom::Element&, PseudoStyleType);
     ScrollTimelineCollection& DoEnsureScrollTimelines(dom::Element&,
                                                       PseudoStyleType);
+    dom::ProgressTimelineScheduler& DoEnsureProgressTimelineScheduler(
+        dom::Element&, PseudoStyleType);
     void DoClearEffectSet();
     void DoClearTransitions();
     void DoClearAnimations();
     void DoClearScrollTimelines();
+    void DoClearProgressTimelineScheduler();
 
     void Traverse(nsCycleCollectionTraversalCallback&);
   };
@@ -181,6 +207,27 @@ class ElementAnimationData {
     return data.DoEnsureScrollTimelines(aOwner, aType);
   }
 
+  dom::ProgressTimelineScheduler* GetProgressTimelineScheduler(
+      PseudoStyleType aType) {
+    return DataFor(aType).mProgressTimelineScheduler.get();
+  }
+
+  void ClearProgressTimelineScheduler(PseudoStyleType aType) {
+    auto& data = DataFor(aType);
+    if (data.mProgressTimelineScheduler) {
+      data.DoClearProgressTimelineScheduler();
+    }
+  }
+
+  dom::ProgressTimelineScheduler& EnsureProgressTimelineScheduler(
+      dom::Element& aOwner, PseudoStyleType aType) {
+    auto& data = DataFor(aType);
+    if (auto* collection = data.mProgressTimelineScheduler.get()) {
+      return *collection;
+    }
+    return data.DoEnsureProgressTimelineScheduler(aOwner, aType);
+  }
+
   ElementAnimationData() = default;
 };
 

dom/animation/ScrollTimeline.cpp

@@ -10,6 +10,7 @@
 #include "mozilla/dom/ElementInlines.h"
 #include "mozilla/AnimationTarget.h"
 #include "mozilla/DisplayPortUtils.h"
+#include "mozilla/ElementAnimationData.h"
 #include "mozilla/PresShell.h"
 #include "nsIFrame.h"
 #include "nsIScrollableFrame.h"
@@ -52,10 +53,14 @@ ScrollTimeline::ScrollTimeline(Document* aDocument, const Scroller& aScroller,
   RegisterWithScrollSource();
 }
 
-static Element* FindNearestScroller(const Element* aSubject) {
+static std::pair<const Element*, PseudoStyleType> FindNearestScroller(
+    Element* aSubject, PseudoStyleType aPseudoType) {
   MOZ_ASSERT(aSubject);
-  Element* curr = aSubject->GetFlattenedTreeParentElement();
+  Element* subject =
-  Element* root = aSubject->OwnerDoc()->GetDocumentElement();
+      AnimationUtils::GetElementForRestyle(aSubject, aPseudoType);
+
+  Element* curr = subject->GetFlattenedTreeParentElement();
+  Element* root = subject->OwnerDoc()->GetDocumentElement();
   while (curr && curr != root) {
     const ComputedStyle* style = Servo_Element_GetMaybeOutOfDateStyle(curr);
     MOZ_ASSERT(style, "The ancestor should be styled.");
@@ -65,7 +70,10 @@ static Element* FindNearestScroller(const Element* aSubject) {
     curr = curr->GetFlattenedTreeParentElement();
   }
   // If there is no scroll container, we use root.
-  return curr ? curr : root;
+  if (!curr) {
+    return {root, PseudoStyleType::NotPseudo};
+  }
+  return AnimationUtils::GetElementPseudoPair(curr);
 }
 
 /* static */
@@ -80,7 +88,9 @@ already_AddRefed<ScrollTimeline> ScrollTimeline::MakeAnonymous(
       break;
 
     case StyleScroller::Nearest: {
-      scroller = Scroller::Nearest(FindNearestScroller(aTarget.mElement));
+      auto [element, pseudo] =
+          FindNearestScroller(aTarget.mElement, aTarget.mPseudoType);
+      scroller = Scroller::Nearest(const_cast<Element*>(element), pseudo);
       break;
     }
   }
@@ -97,10 +107,10 @@ already_AddRefed<ScrollTimeline> ScrollTimeline::MakeAnonymous(
 
 /* static*/ already_AddRefed<ScrollTimeline> ScrollTimeline::MakeNamed(
     Document* aDocument, Element* aReferenceElement,
-    const StyleScrollTimeline& aStyleTimeline) {
+    PseudoStyleType aPseudoType, const StyleScrollTimeline& aStyleTimeline) {
   MOZ_ASSERT(NS_IsMainThread());
 
-  Scroller scroller = Scroller::Named(aReferenceElement);
+  Scroller scroller = Scroller::Named(aReferenceElement, aPseudoType);
   return MakeAndAddRef<ScrollTimeline>(aDocument, std::move(scroller),
                                        aStyleTimeline.GetAxis());
 }
@@ -181,8 +191,10 @@ bool ScrollTimeline::ScrollingDirectionIsAvailable() const {
 }
 
 void ScrollTimeline::ReplacePropertiesWith(const Element* aReferenceElement,
+                                           PseudoStyleType aPseudoType,
                                            const StyleScrollTimeline& aNew) {
-  MOZ_ASSERT(aReferenceElement == mSource.mElement);
+  MOZ_ASSERT(aReferenceElement == mSource.mElement &&
+             aPseudoType == mSource.mPseudoType);
   mAxis = aNew.GetAxis();
 
   for (auto* anim = mAnimationOrder.getFirst(); anim;
@@ -198,10 +210,9 @@ void ScrollTimeline::RegisterWithScrollSource() {
     return;
   }
 
-  if (ScrollTimelineSet* scrollTimelineSet =
+  auto& scheduler =
-          ScrollTimelineSet::GetOrCreateScrollTimelineSet(mSource.mElement)) {
+      ProgressTimelineScheduler::Ensure(mSource.mElement, mSource.mPseudoType);
-    scrollTimelineSet->AddScrollTimeline(this);
+  scheduler.AddTimeline(this);
-  }
 }
 
 void ScrollTimeline::UnregisterFromScrollSource() {
@@ -209,12 +220,15 @@ void ScrollTimeline::UnregisterFromScrollSource() {
     return;
   }
 
-  if (ScrollTimelineSet* scrollTimelineSet =
+  auto* scheduler =
-          ScrollTimelineSet::GetScrollTimelineSet(mSource.mElement)) {
+      ProgressTimelineScheduler::Get(mSource.mElement, mSource.mPseudoType);
-    scrollTimelineSet->RemoveScrollTimeline(this);
+  if (!scheduler) {
-    if (scrollTimelineSet->IsEmpty()) {
+    return;
-      ScrollTimelineSet::DestroyScrollTimelineSet(mSource.mElement);
+  }
-    }
+
+  scheduler->RemoveTimeline(this);
+  if (scheduler->IsEmpty()) {
+    ProgressTimelineScheduler::Destroy(mSource.mElement, mSource.mPseudoType);
   }
 }
 
@@ -239,40 +253,33 @@ const nsIScrollableFrame* ScrollTimeline::GetScrollFrame() const {
   return nullptr;
 }
 
-// ---------------------------------
+// ------------------------------------
-// Methods of ScrollTimelineSet
+// Methods of ProgressTimelineScheduler
-// ---------------------------------
+// ------------------------------------
-
+/* static */ ProgressTimelineScheduler* ProgressTimelineScheduler::Get(
-/* static */ ScrollTimelineSet* ScrollTimelineSet::GetScrollTimelineSet(
+    const Element* aElement, PseudoStyleType aPseudoType) {
-    Element* aElement) {
-  return aElement ? static_cast<ScrollTimelineSet*>(aElement->GetProperty(
-                        nsGkAtoms::scrollTimelinesProperty))
-                  : nullptr;
-}
-
-/* static */ ScrollTimelineSet* ScrollTimelineSet::GetOrCreateScrollTimelineSet(
-    Element* aElement) {
   MOZ_ASSERT(aElement);
-  ScrollTimelineSet* scrollTimelineSet = GetScrollTimelineSet(aElement);
+  auto* data = aElement->GetAnimationData();
-  if (scrollTimelineSet) {
+  if (!data) {
-    return scrollTimelineSet;
-  }
-
-  scrollTimelineSet = new ScrollTimelineSet();
-  nsresult rv = aElement->SetProperty(
-      nsGkAtoms::scrollTimelinesProperty, scrollTimelineSet,
-      nsINode::DeleteProperty<ScrollTimelineSet>, true);
-  if (NS_FAILED(rv)) {
-    NS_WARNING("SetProperty failed");
-    delete scrollTimelineSet;
     return nullptr;
   }
-  return scrollTimelineSet;
+
+  return data->GetProgressTimelineScheduler(aPseudoType);
 }
 
-/* static */ void ScrollTimelineSet::DestroyScrollTimelineSet(
+/* static */ ProgressTimelineScheduler& ProgressTimelineScheduler::Ensure(
-    Element* aElement) {
+    Element* aElement, PseudoStyleType aPseudoType) {
-  aElement->RemoveProperty(nsGkAtoms::scrollTimelinesProperty);
+  MOZ_ASSERT(aElement);
+  return aElement->EnsureAnimationData().EnsureProgressTimelineScheduler(
+      *aElement, aPseudoType);
+}
+
+/* static */
+void ProgressTimelineScheduler::Destroy(const Element* aElement,
+                                        PseudoStyleType aPseudoType) {
+  auto* data = aElement->GetAnimationData();
+  MOZ_ASSERT(data);
+  data->ClearProgressTimelineScheduler(aPseudoType);
 }
 
 }  // namespace mozilla::dom

dom/animation/ScrollTimeline.h

@@ -19,11 +19,9 @@
 class nsIScrollableFrame;
 
 namespace mozilla {
-
+class ElementAnimationData;
 struct NonOwningAnimationTarget;
-
 namespace dom {
-
 class Element;
 
 /**
@@ -70,9 +68,9 @@ class ScrollTimeline final : public AnimationTimeline {
 
  public:
   struct Scroller {
-    // FIXME: Once we support <custom-ident> for <scroller>, we can use
+    // FIXME: Bug 1814444. Add self keyword.
-    // StyleScroller here.
+    // FIXME: Bug 1765211. Perhaps we only need root and a specific element.
-    // https://drafts.csswg.org/scroll-animations-1/#typedef-scroller
+    // This depends on how we fix this bug.
     enum class Type : uint8_t {
       Root,
       Nearest,
@@ -80,6 +78,7 @@ class ScrollTimeline final : public AnimationTimeline {
     };
     Type mType = Type::Root;
     RefPtr<Element> mElement;
+    PseudoStyleType mPseudoType;
 
     // We use the owner doc of the animation target. This may be different from
     // |mDocument| after we implement ScrollTimeline interface for script.
@@ -89,18 +88,22 @@ class ScrollTimeline final : public AnimationTimeline {
       // we always register the ScrollTimeline to the document element (i.e.
       // root element) because the content of the root scroll frame is the root
       // element.
-      return {Type::Root, aOwnerDoc->GetDocumentElement()};
+      return {Type::Root, aOwnerDoc->GetDocumentElement(),
+              PseudoStyleType::NotPseudo};
     }
 
-    static Scroller Nearest(Element* aElement) {
+    static Scroller Nearest(Element* aElement, PseudoStyleType aPseudoType) {
-      return {Type::Nearest, aElement};
+      return {Type::Nearest, aElement, aPseudoType};
     }
 
-    static Scroller Named(Element* aElement) { return {Type::Name, aElement}; }
+    static Scroller Named(Element* aElement, PseudoStyleType aPseudoType) {
+      return {Type::Name, aElement, aPseudoType};
+    }
 
     explicit operator bool() const { return mElement; }
     bool operator==(const Scroller& aOther) const {
-      return mType == aOther.mType && mElement == aOther.mElement;
+      return mType == aOther.mType && mElement == aOther.mElement &&
+             mPseudoType == aOther.mPseudoType;
     }
   };
 
@@ -110,7 +113,7 @@ class ScrollTimeline final : public AnimationTimeline {
 
   static already_AddRefed<ScrollTimeline> MakeNamed(
       Document* aDocument, Element* aReferenceElement,
-      const StyleScrollTimeline& aStyleTimeline);
+      PseudoStyleType aPseudoType, const StyleScrollTimeline& aStyleTimeline);
 
   bool operator==(const ScrollTimeline& aOther) const {
     return mDocument == aOther.mDocument && mSource == aOther.mSource &&
@@ -183,6 +186,7 @@ class ScrollTimeline final : public AnimationTimeline {
   bool ScrollingDirectionIsAvailable() const;
 
   void ReplacePropertiesWith(const Element* aReferenceElement,
+                             PseudoStyleType aPseudoType,
                              const StyleScrollTimeline& aNew);
 
  protected:
@@ -219,45 +223,40 @@ class ScrollTimeline final : public AnimationTimeline {
  * A wrapper around a hashset of ScrollTimeline objects to handle the scheduling
  * of scroll driven animations. This is used for all kinds of progress
  * timelines, i.e. anonymous/named scroll timelines and anonymous/named view
- * timelines.
+ * timelines. And this object is owned by the scroll source (See
- *
+ * ElementAnimationData and nsGfxScrollFrame for the usage).
- * Note:
- * 1. Each ScrollTimeline hooks an dom::Element (as the scroller), and a
- *    dom::Element may be registered by multiple ScrollTimelines.
- * 2. Element holds the ScrollTimelineSet as an element property.
  */
-class ScrollTimelineSet {
+class ProgressTimelineScheduler {
  public:
-  using NonOwningScrollTimelineSet = HashSet<ScrollTimeline*>;
+  ProgressTimelineScheduler() { MOZ_COUNT_CTOR(ProgressTimelineScheduler); }
+  ~ProgressTimelineScheduler() { MOZ_COUNT_DTOR(ProgressTimelineScheduler); }
 
-  ~ScrollTimelineSet() = default;
+  static ProgressTimelineScheduler* Get(const Element* aElement,
+                                        PseudoStyleType aPseudoType);
+  static ProgressTimelineScheduler& Ensure(Element* aElement,
+                                           PseudoStyleType aPseudoType);
+  static void Destroy(const Element* aElement, PseudoStyleType aPseudoType);
 
-  static ScrollTimelineSet* GetScrollTimelineSet(Element* aElement);
+  void AddTimeline(ScrollTimeline* aScrollTimeline) {
-  static ScrollTimelineSet* GetOrCreateScrollTimelineSet(Element* aElement);
+    Unused << mTimelines.put(aScrollTimeline);
-  static void DestroyScrollTimelineSet(Element* aElement);
-
-  void AddScrollTimeline(ScrollTimeline* aScrollTimeline) {
-    Unused << mScrollTimelines.put(aScrollTimeline);
   }
-  void RemoveScrollTimeline(ScrollTimeline* aScrollTimeline) {
+  void RemoveTimeline(ScrollTimeline* aScrollTimeline) {
-    mScrollTimelines.remove(aScrollTimeline);
+    mTimelines.remove(aScrollTimeline);
   }
 
-  bool IsEmpty() const { return mScrollTimelines.empty(); }
+  bool IsEmpty() const { return mTimelines.empty(); }
 
   void ScheduleAnimations() const {
-    for (auto iter = mScrollTimelines.iter(); !iter.done(); iter.next()) {
+    for (auto iter = mTimelines.iter(); !iter.done(); iter.next()) {
       iter.get()->ScheduleAnimations();
     }
   }
 
  private:
-  ScrollTimelineSet() = default;
+  // We let Animations own its scroll timeline or view timeline if it is
-
+  // anonymous. For named progress timelines, they are created and destroyed by
-  // ScrollTimelineSet doesn't own ScrollTimeline. We let Animations own its
+  // TimelineCollection.
-  // scroll timeline if it is anonymous. For named progress timelines, they are
+  HashSet<ScrollTimeline*> mTimelines;
-  // managed by TimelineCollection.
-  NonOwningScrollTimelineSet mScrollTimelines;
 };
 
 }  // namespace dom

layout/generic/nsGfxScrollFrame.cpp

@@ -8488,12 +8488,22 @@ void ScrollFrameHelper::ScheduleScrollAnimations() {
   MOZ_ASSERT(content && content->IsElement(),
              "The nsIScrollableFrame should have the element.");
 
-  const auto* set =
+  const Element* elementOrPseudo = content->AsElement();
-      ScrollTimelineSet::GetScrollTimelineSet(content->AsElement());
+  PseudoStyleType pseudo = elementOrPseudo->GetPseudoElementType();
-  if (!set) {
+  if (pseudo != PseudoStyleType::NotPseudo &&
+      !AnimationUtils::IsSupportedPseudoForAnimations(pseudo)) {
+    // This is not an animatable pseudo element, and so we don't generate
+    // scroll-timeline for it.
+    return;
+  }
+
+  const auto [element, type] =
+      AnimationUtils::GetElementPseudoPair(elementOrPseudo);
+  const auto* scheduler = ProgressTimelineScheduler::Get(element, type);
+  if (!scheduler) {
     // We don't have scroll timelines associated with this frame.
     return;
   }
 
-  set->ScheduleAnimations();
+  scheduler->ScheduleAnimations();
 }

layout/style/TimelineManager.cpp

@@ -6,7 +6,6 @@
 
 #include "TimelineManager.h"
 
-#include "mozilla/AnimationUtils.h"
 #include "mozilla/ElementAnimationData.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/ScrollTimeline.h"
@@ -88,11 +87,11 @@ static auto BuildTimelines(nsPresContext* aPresContext, Element* aElement,
 
     RefPtr<TimelineType> dest =
         PopExistingTimeline(timeline.GetName(), aCollection);
-    Element* e = AnimationUtils::GetElementForRestyle(aElement, aPseudoType);
     if (dest) {
-      dest->ReplacePropertiesWith(e, timeline);
+      dest->ReplacePropertiesWith(aElement, aPseudoType, timeline);
     } else {
-      dest = TimelineType::MakeNamed(aPresContext->Document(), e, timeline);
+      dest = TimelineType::MakeNamed(aPresContext->Document(), aElement,
+                                     aPseudoType, timeline);
     }
     MOZ_ASSERT(dest);
 

layout/style/crashtests/1821416.html

@@ -0,0 +1,21 @@
+<!DOCTYPE HTML>
+<style>
+*, *::after {
+  scroll-timeline: tl;
+}
+</style>
+<script>
+window.addEventListener("load", () => {
+  let a = document.createElement("ul")
+  let b = document.createElement("li")
+  let c = document.createElement("div")
+  let d = document.createElement("del")
+  d.appendChild(document.createElement("q"))
+  c.appendChild(d)
+  b.appendChild(c)
+  a.appendChild(b)
+  document.documentElement.appendChild(a);
+  a.scrollBy(32767, 256);
+  a.type = "square";
+})
+</script>

layout/style/crashtests/crashtests.list

@@ -320,3 +320,4 @@ pref(layout.css.constructable-stylesheets.enabled,true) load 1616407.html
 load 1639533.html
 pref(layout.accessiblecaret.enabled,true) load 1640040.html
 load 1806189-1.html
+pref(layout.css.scroll-driven-animations.enabled,true) load 1821416.html

xpcom/ds/StaticAtoms.py

@@ -2158,7 +2158,6 @@ STATIC_ATOMS = [
     Atom("docLevelNativeAnonymousContent", "docLevelNativeAnonymousContent"),  # bool
     Atom("paintRequestTime", "PaintRequestTime"),
     Atom("pseudoProperty", "PseudoProperty"),  # PseudoStyleType
-    Atom("scrollTimelinesProperty", "SrollTimelinesProperty"),  # ScrollTimelineSet*
     Atom("manualNACProperty", "ManualNACProperty"),  # ManualNAC*
     Atom("markerPseudoProperty", "markerPseudoProperty"),  # nsXMLElement*
     # Languages for lang-specific transforms