Bug 1923344 - r=smaug, a=dsmith

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

dom/animation/AnimationTimeline.cpp

@@ -40,41 +40,33 @@ AnimationTimeline::~AnimationTimeline() { mAnimationOrder.clear(); }
 bool AnimationTimeline::Tick(TickState& aState) {
   bool needsTicks = false;
 
-  nsTArray<Animation*> animationsToRemove;
+  AutoTArray<RefPtr<Animation>, 32> animationsToTick;
-
+  for (Animation* animation : mAnimationOrder) {
-  for (Animation* animation = mAnimationOrder.getFirst(); animation;
-       animation =
-           static_cast<LinkedListElement<Animation>*>(animation)->getNext()) {
     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
+      RemoveAnimation(animation);
-      // animation list of this timeline object!
-      MOZ_ASSERT(!animation->GetTimeline());
-      animationsToRemove.AppendElement(animation);
       continue;
     }
 
     needsTicks |= animation->NeedsTicks();
-    // Even if |animation| doesn't need future ticks, we should still
+    // Even if |animation| doesn't need future ticks, we should still Tick it
-    // Tick it this time around since it might just need a one-off tick in
+    // this time around since it might just need a one-off tick in order to
-    // order to dispatch events.
+    // queue events.
     animation->Tick(aState);
-
     if (!animation->NeedsTicks()) {
-      animationsToRemove.AppendElement(animation);
+      RemoveAnimation(animation);
     }
   }
 
-  for (Animation* animation : animationsToRemove) {
-    RemoveAnimation(animation);
-  }
-
   return needsTicks;
 }
 
@@ -90,11 +82,12 @@ void AnimationTimeline::NotifyAnimationUpdated(Animation& 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);
 }

dom/animation/DocumentTimeline.cpp

@@ -160,7 +160,12 @@ void DocumentTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
 }
 
 void DocumentTimeline::TriggerAllPendingAnimationsNow() {
+  AutoTArray<RefPtr<Animation>, 32> animationsToTrigger;
   for (Animation* animation : mAnimationOrder) {
+    animationsToTrigger.AppendElement(animation);
+  }
+
+  for (Animation* animation : animationsToTrigger) {
     animation->TryTriggerNow();
   }
 }
@@ -188,9 +193,6 @@ void DocumentTimeline::WillRefresh() {
   // of mDocument's PresShell.
   if (nsRefreshDriver* refreshDriver = GetRefreshDriver()) {
     refreshDriver->EnsureAnimationUpdate();
-  } else {
-    MOZ_ASSERT_UNREACHABLE(
-        "Refresh driver should still be valid at end of WillRefresh");
   }
 }
 

dom/animation/ScrollTimelineAnimationTracker.cpp

@@ -13,13 +13,10 @@ namespace mozilla {
 NS_IMPL_CYCLE_COLLECTION(ScrollTimelineAnimationTracker, mPendingSet, mDocument)
 
 void ScrollTimelineAnimationTracker::TriggerPendingAnimations() {
-  for (auto iter = mPendingSet.begin(), end = mPendingSet.end(); iter != end;
+  for (RefPtr<dom::Animation>& animation :
-       ++iter) {
+       ToTArray<AutoTArray<RefPtr<dom::Animation>, 32>>(mPendingSet)) {
-    dom::Animation* animation = *iter;
-
     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
@@ -39,9 +36,7 @@ void ScrollTimelineAnimationTracker::TriggerPendingAnimations() {
       // inactive, and this also matches the current spec definition.
       continue;
     }
-
+    mPendingSet.Remove(animation);
-    // Note: Remove() is legitimately called once per entry during the loop.
-    mPendingSet.Remove(iter);
   }
 }
 

layout/base/nsRefreshDriver.cpp

@@ -2332,8 +2332,15 @@ void nsRefreshDriver::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()) {
@@ -2363,7 +2370,8 @@ void nsRefreshDriver::UpdateAnimationsAndSendEvents() {
     // [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