# HG changeset patch
# User Emilio Cobos Álvarez <[email protected]>
# Date 1728404712 0
# Node ID e0c969a3bfc0a23219384269e5b36a589c8f6cc5
# Parent  9a327e036cdce4d976424e729db47a5d56defc4f
Bug 1923344 - r=smaug, a=dsmith

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

diff --git a/dom/animation/AnimationTimeline.cpp b/dom/animation/AnimationTimeline.cpp
--- a/dom/animation/AnimationTimeline.cpp
+++ b/dom/animation/AnimationTimeline.cpp
@@ -35,71 +35,64 @@ AnimationTimeline::AnimationTimeline(nsI
   MOZ_ASSERT(mWindow);
 }
 
 AnimationTimeline::~AnimationTimeline() { mAnimationOrder.clear(); }
 
 bool AnimationTimeline::Tick(TickState& aState) {
   bool needsTicks = false;
 
-  nsTArray<Animation*> animationsToRemove;
-
-  for (Animation* animation = mAnimationOrder.getFirst(); animation;
-       animation =
-           static_cast<LinkedListElement<Animation>*>(animation)->getNext()) {
+  AutoTArray<RefPtr<Animation>, 32> animationsToTick;
+  for (Animation* animation : mAnimationOrder) {
     MOZ_ASSERT(mAnimations.Contains(animation),
                "The sampling order list should be a subset of the hashset");
     MOZ_ASSERT(!animation->IsHiddenByContentVisibility(),
                "The sampling order list should not contain any animations "
                "that are hidden by content-visibility");
+    animationsToTick.AppendElement(animation);
+  }
 
+  for (Animation* animation : animationsToTick) {
     // Skip any animations that are longer need associated with this timeline.
     if (animation->GetTimeline() != this) {
-      // If animation has some other timeline, it better not be also in the
-      // animation list of this timeline object!
-      MOZ_ASSERT(!animation->GetTimeline());
-      animationsToRemove.AppendElement(animation);
+      RemoveAnimation(animation);
       continue;
     }
 
     needsTicks |= animation->NeedsTicks();
-    // Even if |animation| doesn't need future ticks, we should still
-    // Tick it this time around since it might just need a one-off tick in
-    // order to dispatch events.
+    // Even if |animation| doesn't need future ticks, we should still Tick it
+    // this time around since it might just need a one-off tick in order to
+    // queue events.
     animation->Tick(aState);
-
     if (!animation->NeedsTicks()) {
-      animationsToRemove.AppendElement(animation);
+      RemoveAnimation(animation);
     }
   }
 
-  for (Animation* animation : animationsToRemove) {
-    RemoveAnimation(animation);
-  }
-
   return needsTicks;
 }
 
 void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
   if (mAnimations.EnsureInserted(&aAnimation)) {
     if (aAnimation.GetTimeline() && aAnimation.GetTimeline() != this) {
       aAnimation.GetTimeline()->RemoveAnimation(&aAnimation);
     }
     if (!aAnimation.IsHiddenByContentVisibility()) {
       mAnimationOrder.insertBack(&aAnimation);
     }
   }
 }
 
 void AnimationTimeline::RemoveAnimation(Animation* aAnimation) {
-  MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this);
-  if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList()) {
+  if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList() &&
+      MOZ_LIKELY(!aAnimation->GetTimeline() ||
+                 aAnimation->GetTimeline() == this)) {
+    static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
     MOZ_ASSERT(mAnimations.Contains(aAnimation),
                "The sampling order list should be a subset of the hashset");
-    static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
   }
   mAnimations.Remove(aAnimation);
 }
 
 void AnimationTimeline::NotifyAnimationContentVisibilityChanged(
     Animation* aAnimation, bool aIsVisible) {
   bool inList =
       static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList();
diff --git a/dom/animation/DocumentTimeline.cpp b/dom/animation/DocumentTimeline.cpp
--- a/dom/animation/DocumentTimeline.cpp
+++ b/dom/animation/DocumentTimeline.cpp
@@ -155,17 +155,22 @@ void DocumentTimeline::NotifyAnimationUp
                  "We should not register with the refresh driver if we are not"
                  " in the document's list of timelines");
       refreshDriver->EnsureAnimationUpdate();
     }
   }
 }
 
 void DocumentTimeline::TriggerAllPendingAnimationsNow() {
+  AutoTArray<RefPtr<Animation>, 32> animationsToTrigger;
   for (Animation* animation : mAnimationOrder) {
+    animationsToTrigger.AppendElement(animation);
+  }
+
+  for (Animation* animation : animationsToTrigger) {
     animation->TryTriggerNow();
   }
 }
 
 void DocumentTimeline::WillRefresh() {
   if (!mDocument->GetPresShell()) {
     // If we're not displayed, don't tick animations.
     return;
@@ -183,19 +188,16 @@ void DocumentTimeline::WillRefresh() {
   }
   // We already assert that GetRefreshDriver() is non-null at the beginning
   // of this function but we check it again here to be sure that ticking
   // animations does not have any side effects that cause us to lose the
   // connection with the refresh driver, such as triggering the destruction
   // of mDocument's PresShell.
   if (nsRefreshDriver* refreshDriver = GetRefreshDriver()) {
     refreshDriver->EnsureAnimationUpdate();
-  } else {
-    MOZ_ASSERT_UNREACHABLE(
-        "Refresh driver should still be valid at end of WillRefresh");
   }
 }
 
 void DocumentTimeline::RemoveAnimation(Animation* aAnimation) {
   AnimationTimeline::RemoveAnimation(aAnimation);
 }
 
 void DocumentTimeline::NotifyAnimationContentVisibilityChanged(
diff --git a/dom/animation/ScrollTimelineAnimationTracker.cpp b/dom/animation/ScrollTimelineAnimationTracker.cpp
--- a/dom/animation/ScrollTimelineAnimationTracker.cpp
+++ b/dom/animation/ScrollTimelineAnimationTracker.cpp
@@ -8,23 +8,20 @@
 
 #include "mozilla/dom/Document.h"
 
 namespace mozilla {
 
 NS_IMPL_CYCLE_COLLECTION(ScrollTimelineAnimationTracker, mPendingSet, mDocument)
 
 void ScrollTimelineAnimationTracker::TriggerPendingAnimations() {
-  for (auto iter = mPendingSet.begin(), end = mPendingSet.end(); iter != end;
-       ++iter) {
-    dom::Animation* animation = *iter;
-
+  for (RefPtr<dom::Animation>& animation :
+       ToTArray<AutoTArray<RefPtr<dom::Animation>, 32>>(mPendingSet)) {
     MOZ_ASSERT(animation->GetTimeline() &&
                !animation->GetTimeline()->IsMonotonicallyIncreasing());
-
     // FIXME: Trigger now may not be correct because the spec says:
     // If a user agent determines that animation is immediately ready, it may
     // schedule the task (i.e. ResumeAt()) as a microtask such that it runs at
     // the next microtask checkpoint, but it must not perform the task
     // synchronously.
     // Note: So, for now, we put the animation into the tracker, and trigger
     // them immediately until the frames are ready. Using TriggerOnNextTick()
     // for scroll-driven animations may have issues because we don't tick if
@@ -34,15 +31,13 @@ void ScrollTimelineAnimationTracker::Tri
       // inactive. It's pretty hard to tell its future status, for example, it's
       // possible that the scroll container is in display:none subtree but the
       // animating element isn't the subtree, then we need to keep tracking the
       // situation until the scroll container gets framed. so in general we make
       // this animation be pending (i.e. not ready) if its scroll-timeline is
       // inactive, and this also matches the current spec definition.
       continue;
     }
-
-    // Note: Remove() is legitimately called once per entry during the loop.
-    mPendingSet.Remove(iter);
+    mPendingSet.Remove(animation);
   }
 }
 
 }  // namespace mozilla
diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -2327,18 +2327,25 @@ void nsRefreshDriver::DetermineProximity
                                                        ShouldCollect);
 
   for (const RefPtr<Document>& doc : documents) {
     MOZ_KnownLive(doc)->DetermineProximityToViewportAndNotifyResizeObservers();
   }
 }
 
 static CallState UpdateAndReduceAnimations(Document& aDocument) {
-  for (DocumentTimeline* timeline : aDocument.Timelines()) {
-    timeline->WillRefresh();
+  {
+    AutoTArray<RefPtr<DocumentTimeline>, 32> timelinesToTick;
+    for (DocumentTimeline* timeline : aDocument.Timelines()) {
+      timelinesToTick.AppendElement(timeline);
+    }
+
+    for (DocumentTimeline* tl : timelinesToTick) {
+      tl->WillRefresh();
+    }
   }
 
   if (nsPresContext* pc = aDocument.GetPresContext()) {
     if (pc->EffectCompositor()->NeedsReducing()) {
       pc->EffectCompositor()->ReduceAnimations();
     }
   }
   aDocument.EnumerateSubDocuments(UpdateAndReduceAnimations);
@@ -2358,17 +2365,18 @@ void nsRefreshDriver::UpdateAnimationsAn
     // run these, however, until we have fully updated the animation state. As
     // per the "update animations and send events" procedure[1], we should
     // remove replaced animations and then run these microtasks before
     // dispatching the corresponding animation events.
     //
     // [1]:
     // https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events
     nsAutoMicroTask mt;
-    UpdateAndReduceAnimations(*mPresContext->Document());
+    RefPtr doc = mPresContext->Document();
+    UpdateAndReduceAnimations(*doc);
   }
 
   // Hold all AnimationEventDispatcher in mAnimationEventFlushObservers as
   // a RefPtr<> array since each AnimationEventDispatcher might be destroyed
   // during processing the previous dispatcher.
   AutoTArray<RefPtr<AnimationEventDispatcher>, 16> dispatchers;
   dispatchers.AppendElements(mAnimationEventFlushObservers);
   mAnimationEventFlushObservers.Clear();