From 0375bee4e0212ca8d882ac15f058153a7a8d2da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 8 Oct 2024 16:25:12 +0000 Subject: [PATCH] Bug 1923344 - r=smaug, a=dsmith Differential Revision: https://phabricator.services.mozilla.com/D224958 --- dom/animation/AnimationTimeline.cpp | 35 ++++++++----------- dom/animation/DocumentTimeline.cpp | 8 +++-- .../ScrollTimelineAnimationTracker.cpp | 11 ++---- layout/base/nsRefreshDriver.cpp | 14 ++++++-- 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/dom/animation/AnimationTimeline.cpp b/dom/animation/AnimationTimeline.cpp index a684cd60cfeea..9870dfac48c89 100644 --- a/dom/animation/AnimationTimeline.cpp +++ b/dom/animation/AnimationTimeline.cpp @@ -40,41 +40,33 @@ AnimationTimeline::~AnimationTimeline() { mAnimationOrder.clear(); } bool AnimationTimeline::Tick(TickState& aState) { bool needsTicks = false; - nsTArray animationsToRemove; - - for (Animation* animation = mAnimationOrder.getFirst(); animation; - animation = - static_cast*>(animation)->getNext()) { + AutoTArray, 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; } @@ -90,11 +82,12 @@ void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) { } void AnimationTimeline::RemoveAnimation(Animation* aAnimation) { - MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this); - if (static_cast*>(aAnimation)->isInList()) { + if (static_cast*>(aAnimation)->isInList() && + MOZ_LIKELY(!aAnimation->GetTimeline() || + aAnimation->GetTimeline() == this)) { + static_cast*>(aAnimation)->remove(); MOZ_ASSERT(mAnimations.Contains(aAnimation), "The sampling order list should be a subset of the hashset"); - static_cast*>(aAnimation)->remove(); } mAnimations.Remove(aAnimation); } diff --git a/dom/animation/DocumentTimeline.cpp b/dom/animation/DocumentTimeline.cpp index 81fc2bcb4c4ce..2684195b88772 100644 --- a/dom/animation/DocumentTimeline.cpp +++ b/dom/animation/DocumentTimeline.cpp @@ -160,7 +160,12 @@ void DocumentTimeline::NotifyAnimationUpdated(Animation& aAnimation) { } void DocumentTimeline::TriggerAllPendingAnimationsNow() { + AutoTArray, 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"); } } diff --git a/dom/animation/ScrollTimelineAnimationTracker.cpp b/dom/animation/ScrollTimelineAnimationTracker.cpp index 48cad8cedd258..8ae26781e079d 100644 --- a/dom/animation/ScrollTimelineAnimationTracker.cpp +++ b/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; - ++iter) { - dom::Animation* animation = *iter; - + for (RefPtr& animation : + ToTArray, 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 @@ -39,9 +36,7 @@ void ScrollTimelineAnimationTracker::TriggerPendingAnimations() { // 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); } } diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index eca84ca4de3c0..9838f6e790519 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -2332,8 +2332,15 @@ void nsRefreshDriver::DetermineProximityToViewportAndNotifyResizeObservers() { } static CallState UpdateAndReduceAnimations(Document& aDocument) { - for (DocumentTimeline* timeline : aDocument.Timelines()) { - timeline->WillRefresh(); + { + AutoTArray, 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