commit 58ae5b2ad37a45d4edb1245fe810d3a0b942b3bc
parent aa50a239e23842c526b42721560a33a1fd0b6c7f
Author: Timothy Nikkel <tnikkel@gmail.com>
Date: Sat, 22 Nov 2025 14:48:51 +0000
Bug 2001613. Don't put ASRs into mActiveScrolledRoots multiple times. r=layout-reviewers,emilio
This doesn't cause problems, but we don't need to do it, and we can clean up these functions a bit to be more clear.
Besides the only functional change of this patch (the first line of the commit message) this patch also renames the functions to GetOrCreate so it is more clear what they do.
Differential Revision: https://phabricator.services.mozilla.com/D273572
Diffstat:
3 files changed, 34 insertions(+), 58 deletions(-)
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
@@ -3388,7 +3388,7 @@ void nsIFrame::BuildDisplayListForStackingContext(
if (shouldFlattenStickyItem) {
stickyASR = aBuilder->CurrentActiveScrolledRoot();
} else {
- stickyASR = aBuilder->AllocateActiveScrolledRootForSticky(
+ stickyASR = aBuilder->GetOrCreateActiveScrolledRootForSticky(
aBuilder->CurrentActiveScrolledRoot(), this);
asrSetter.SetCurrentActiveScrolledRoot(stickyASR);
}
diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
@@ -166,22 +166,18 @@ void InitializeHitTestInfo(nsDisplayListBuilder* aBuilder,
}
/* static */
-already_AddRefed<ActiveScrolledRoot> ActiveScrolledRoot::CreateASRForFrame(
+already_AddRefed<ActiveScrolledRoot> ActiveScrolledRoot::GetOrCreateASRForFrame(
const ActiveScrolledRoot* aParent,
- ScrollContainerFrame* aScrollContainerFrame
-#ifdef DEBUG
- ,
- const nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots
-#endif
-) {
+ ScrollContainerFrame* aScrollContainerFrame,
+ nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots) {
RefPtr<ActiveScrolledRoot> asr =
aScrollContainerFrame->GetProperty(ActiveScrolledRootCache());
#ifdef DEBUG
if (asr && aActiveScrolledRoots.Contains(asr)) {
- // This is the second time we are "creating" this ASR in this paint. Assert
- // that we aren't changing any of the values. (The values can change
- // *between* paints, but not during one paint.)
+ // If this is the second time we are called for this frame in this same
+ // paint, assert that we aren't changing any of the values. (The values can
+ // change *between* paints, but not during one paint.)
MOZ_ASSERT(asr->mParent == aParent);
MOZ_ASSERT(asr->mFrame == aScrollContainerFrame);
MOZ_ASSERT(asr->mKind == ASRKind::Scroll);
@@ -195,6 +191,7 @@ already_AddRefed<ActiveScrolledRoot> ActiveScrolledRoot::CreateASRForFrame(
RefPtr<ActiveScrolledRoot> ref = asr;
aScrollContainerFrame->SetProperty(ActiveScrolledRootCache(),
ref.forget().take());
+ aActiveScrolledRoots.AppendElement(asr);
}
asr->mParent = aParent;
asr->mFrame = aScrollContainerFrame;
@@ -206,13 +203,9 @@ already_AddRefed<ActiveScrolledRoot> ActiveScrolledRoot::CreateASRForFrame(
/* static */
already_AddRefed<ActiveScrolledRoot>
-ActiveScrolledRoot::CreateASRForStickyFrame(
- const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame
-#ifdef DEBUG
- ,
- const nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots
-#endif
-) {
+ActiveScrolledRoot::GetOrCreateASRForStickyFrame(
+ const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame,
+ nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots) {
aStickyFrame = aStickyFrame->FirstContinuation();
RefPtr<ActiveScrolledRoot> asr =
@@ -220,9 +213,9 @@ ActiveScrolledRoot::CreateASRForStickyFrame(
#ifdef DEBUG
if (asr && aActiveScrolledRoots.Contains(asr)) {
- // This is the second time we are "creating" this ASR in this paint. Assert
- // that we aren't changing any of the values. (The values can change
- // *between* paints, but not during one paint.)
+ // If this is the second time we are called for this frame in this same
+ // paint, assert that we aren't changing any of the values. (The values can
+ // change *between* paints, but not during one paint.)
MOZ_ASSERT(asr->mParent == aParent);
MOZ_ASSERT(asr->mFrame == aStickyFrame);
MOZ_ASSERT(asr->mKind == ASRKind::Sticky);
@@ -236,6 +229,7 @@ ActiveScrolledRoot::CreateASRForStickyFrame(
RefPtr<ActiveScrolledRoot> ref = asr;
aStickyFrame->SetProperty(StickyActiveScrolledRootCache(),
ref.forget().take());
+ aActiveScrolledRoots.AppendElement(asr);
}
asr->mParent = aParent;
@@ -575,7 +569,7 @@ void nsDisplayListBuilder::AutoCurrentActiveScrolledRootSetter::
size_t descendantsEndIndex = mBuilder->mActiveScrolledRoots.Length();
const ActiveScrolledRoot* parentASR = mBuilder->mCurrentActiveScrolledRoot;
const ActiveScrolledRoot* asr =
- mBuilder->AllocateActiveScrolledRoot(parentASR, aScrollContainerFrame);
+ mBuilder->GetOrCreateActiveScrolledRoot(parentASR, aScrollContainerFrame);
mBuilder->mCurrentActiveScrolledRoot = asr;
// All child ASRs of parentASR that were created while this
@@ -1537,30 +1531,20 @@ void nsDisplayListBuilder::MarkPreserve3DFramesForDisplayList(
}
}
-ActiveScrolledRoot* nsDisplayListBuilder::AllocateActiveScrolledRoot(
+ActiveScrolledRoot* nsDisplayListBuilder::GetOrCreateActiveScrolledRoot(
const ActiveScrolledRoot* aParent,
ScrollContainerFrame* aScrollContainerFrame) {
- RefPtr<ActiveScrolledRoot> asr =
- ActiveScrolledRoot::CreateASRForFrame(aParent, aScrollContainerFrame
-#ifdef DEBUG
- ,
- mActiveScrolledRoots
-#endif
- );
- mActiveScrolledRoots.AppendElement(asr);
+ RefPtr<ActiveScrolledRoot> asr = ActiveScrolledRoot::GetOrCreateASRForFrame(
+ aParent, aScrollContainerFrame, mActiveScrolledRoots);
return asr;
}
-ActiveScrolledRoot* nsDisplayListBuilder::AllocateActiveScrolledRootForSticky(
+ActiveScrolledRoot*
+nsDisplayListBuilder::GetOrCreateActiveScrolledRootForSticky(
const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame) {
RefPtr<ActiveScrolledRoot> asr =
- ActiveScrolledRoot::CreateASRForStickyFrame(aParent, aStickyFrame
-#ifdef DEBUG
- ,
- mActiveScrolledRoots
-#endif
- );
- mActiveScrolledRoots.AppendElement(asr);
+ ActiveScrolledRoot::GetOrCreateASRForStickyFrame(aParent, aStickyFrame,
+ mActiveScrolledRoots);
return asr;
}
diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h
@@ -185,21 +185,13 @@ LazyLogModule& GetLoggerByProcess();
*/
struct ActiveScrolledRoot {
// TODO: Just have one function with an extra ASRKind parameter
- static already_AddRefed<ActiveScrolledRoot> CreateASRForFrame(
+ static already_AddRefed<ActiveScrolledRoot> GetOrCreateASRForFrame(
const ActiveScrolledRoot* aParent,
- ScrollContainerFrame* aScrollContainerFrame
-#ifdef DEBUG
- ,
- const nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots
-#endif
- );
- static already_AddRefed<ActiveScrolledRoot> CreateASRForStickyFrame(
- const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame
-#ifdef DEBUG
- ,
- const nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots
-#endif
- );
+ ScrollContainerFrame* aScrollContainerFrame,
+ nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots);
+ static already_AddRefed<ActiveScrolledRoot> GetOrCreateASRForStickyFrame(
+ const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame,
+ nsTArray<RefPtr<ActiveScrolledRoot>>& aActiveScrolledRoots);
static const ActiveScrolledRoot* PickAncestor(
const ActiveScrolledRoot* aOne, const ActiveScrolledRoot* aTwo) {
@@ -985,13 +977,13 @@ class nsDisplayListBuilder {
}
/**
- * Allocate a new ActiveScrolledRoot in the arena. Will be cleaned up
- * automatically when the arena goes away.
+ * Get an existing or allocate a new ActiveScrolledRoot in the arena. Will be
+ * cleaned up automatically when the arena goes away.
*/
- ActiveScrolledRoot* AllocateActiveScrolledRoot(
+ ActiveScrolledRoot* GetOrCreateActiveScrolledRoot(
const ActiveScrolledRoot* aParent,
ScrollContainerFrame* aScrollContainerFrame);
- ActiveScrolledRoot* AllocateActiveScrolledRootForSticky(
+ ActiveScrolledRoot* GetOrCreateActiveScrolledRootForSticky(
const ActiveScrolledRoot* aParent, nsIFrame* aStickyFrame);
/**
@@ -1245,7 +1237,7 @@ class nsDisplayListBuilder {
void EnterScrollFrame(ScrollContainerFrame* aScrollContainerFrame) {
MOZ_ASSERT(!mUsed);
- ActiveScrolledRoot* asr = mBuilder->AllocateActiveScrolledRoot(
+ ActiveScrolledRoot* asr = mBuilder->GetOrCreateActiveScrolledRoot(
mBuilder->mCurrentActiveScrolledRoot, aScrollContainerFrame);
mBuilder->mCurrentActiveScrolledRoot = asr;
mUsed = true;
