tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 6cbbb9bc81e4e55a4a00101cc25cd44686f3255b
parent ba75c753ec99f324d372141fa9c02de0e45d8469
Author: Timothy Nikkel <tnikkel@gmail.com>
Date:   Fri, 14 Nov 2025 04:15:57 +0000

Bug 1995759. If CSS anchor pos has been seen then activate all scroll frames. r=botond,hiro,emilio,jwatt

We cache it on the builder because getting the root presshell can be a bit expensive, but for the cases where it is called outside of a paint it is not called very often, only after the display port expiry timer (I think 10 seconds).

I haven't yet thought through every possible case here. For example, say we activate a scroll frame, then anchor pos appears so ShouldActivateAllScrollFrames flips to true, then we want to make sure the display port expiry code all makes sense, but it seems like it would be okay.

The current situation is that we activate all scroll frames when fission is turned on. Fission is turned on on desktop, but not android. So this is only necessary on android. Turning on activating all scroll frames on Android (regardless of fission) is about a 0.3% regression on speedometer 3. We are not supposed to regress sp3. This solution shouldn't regress sp3 and is easier to do then finding offseting optimizations to get back the regression. When we turn on fission on android in the future we will still have to reckon with this potential regression.

The overall reason why we need to activate all scroll frames with anchor pos. With CSS anchor pos we want to assign the ASR of the anchor to the anchored content so they async scroll together (because they will main thread scroll together too). Due to the way that anchors are defined in the spec we may call BuildDisplayList on the anchor either before or after BuildDisplayList on the anchored content. The anchored content may or may not contain an active scroll frame, if it does that would force the nearest scroll frame to the anchor to be active (because the anchored content is a child of nearest scroll frame to the anchor in the ASR tree). So if we have anchored content that contains an active scroll frame, and we've already called BuildDisplayList on the anchor and there was nothing that made it active at that time and we are now calling BuildDisplayList on the anchored content we would need to activate the nearest scrollframe to the anchor content, but now it's too late to do that as we've already built the display list for that content. The presshell tracks whenever anchor pos is used, so we can just activate all scroll frames when anchor pos is observed.

(In actuality I plan to just have the code always assume that anchored content contains active scroll frames to make the code easier to understand and work with. And there will be code that activates any containing scrollframes of the anchor in the opposite case to above in which we would call BuildDisplayList on the anchored content before the anchor.)

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

Diffstat:
Mlayout/generic/ScrollContainerFrame.cpp | 28+++++++++++++++++++---------
Mlayout/generic/ScrollContainerFrame.h | 6+++++-
Mlayout/painting/nsDisplayList.cpp | 9+++++++++
Mlayout/painting/nsDisplayList.h | 7+++++++
4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/layout/generic/ScrollContainerFrame.cpp b/layout/generic/ScrollContainerFrame.cpp @@ -2619,7 +2619,7 @@ void ScrollContainerFrame::RemoveDisplayPortCallback(nsITimer* aTimer, nsIContent* content = sf->GetContent(); - if (ScrollContainerFrame::ShouldActivateAllScrollFrames()) { + if (ScrollContainerFrame::ShouldActivateAllScrollFrames(nullptr, sf)) { // If we are activating all scroll frames then we only want to remove the // regular display port and downgrade to a minimal display port. MOZ_ASSERT(!content->GetProperty(nsGkAtoms::MinimalDisplayPort)); @@ -2705,7 +2705,7 @@ bool ScrollContainerFrame::AllowDisplayPortExpiration() { return false; } - if (ShouldActivateAllScrollFrames() && + if (ShouldActivateAllScrollFrames(nullptr, this) && GetContent()->GetProperty(nsGkAtoms::MinimalDisplayPort)) { return false; } @@ -4061,7 +4061,7 @@ void ScrollContainerFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, if (aBuilder->IsPaintingToWindow()) { mIsParentToActiveScrollFrames = - ShouldActivateAllScrollFrames() + ShouldActivateAllScrollFrames(aBuilder, this) ? asrSetter.GetContainsNonMinimalDisplayPort() : asrSetter.ShouldForceLayerForScrollParent(); } @@ -4095,7 +4095,7 @@ void ScrollContainerFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, #ifndef MOZ_WIDGET_ANDROID gfxCriticalNoteOnce << "inserted scroll frame"; #endif - MOZ_ASSERT(!ShouldActivateAllScrollFrames()); + MOZ_ASSERT(!ShouldActivateAllScrollFrames(aBuilder, this)); asrSetter.InsertScrollFrame(this); aBuilder->SetDisablePartialUpdates(true); } @@ -4327,10 +4327,20 @@ nsRect ScrollContainerFrame::RestrictToRootDisplayPort( return aDisplayportBase.Intersect(rootDisplayPort); } -/* static */ bool ScrollContainerFrame::ShouldActivateAllScrollFrames() { - return (StaticPrefs::apz_wr_activate_all_scroll_frames() || - (StaticPrefs::apz_wr_activate_all_scroll_frames_when_fission() && - FissionAutostart())); +/* static */ bool ScrollContainerFrame::ShouldActivateAllScrollFrames( + nsDisplayListBuilder* aBuilder, nsIFrame* aFrame) { + if (aBuilder) { + return aBuilder->ShouldActivateAllScrollFrames(); + } + MOZ_ASSERT(aFrame); + if (StaticPrefs::apz_wr_activate_all_scroll_frames()) { + return true; + } + if (StaticPrefs::apz_wr_activate_all_scroll_frames_when_fission() && + FissionAutostart()) { + return true; + } + return aFrame->PresShell()->GetRootPresShell()->HasSeenAnchorPos(); } bool ScrollContainerFrame::DecideScrollableLayer( @@ -4350,7 +4360,7 @@ bool ScrollContainerFrame::DecideScrollableLayer( // do this when aSetBase is true because we only want to do this the first // time this function is called for the same scroll frame.) if (aSetBase && !hasDisplayPort && aBuilder->IsPaintingToWindow() && - ShouldActivateAllScrollFrames() && + ShouldActivateAllScrollFrames(aBuilder, this) && nsLayoutUtils::AsyncPanZoomEnabled(this) && WantAsyncScroll()) { // SetDisplayPortMargins calls TriggerDisplayPortExpiration which starts a // display port expiry timer for display ports that do expire. However diff --git a/layout/generic/ScrollContainerFrame.h b/layout/generic/ScrollContainerFrame.h @@ -984,7 +984,11 @@ class ScrollContainerFrame : public nsContainerFrame, bool UsesOverlayScrollbars() const; bool IsLastSnappedTarget(const nsIFrame* aFrame) const; - static bool ShouldActivateAllScrollFrames(); + // If aBuilder is non-null, returns the value cached on aBuilder. Pass null + // for aBuilder to get the correct value to cache on a new builder or new + // frame of painting, or if you need the correct value outside of paint time. + static bool ShouldActivateAllScrollFrames(nsDisplayListBuilder* aBuilder, + nsIFrame* aFrame); nsRect RestrictToRootDisplayPort(const nsRect& aDisplayportBase); bool DecideScrollableLayer(nsDisplayListBuilder* aBuilder, nsRect* aVisibleRect, nsRect* aDirtyRect, diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp @@ -951,6 +951,11 @@ bool nsDisplayListBuilder::ShouldRebuildDisplayListDueToPrefChange() { mAlwaysLayerizeScrollbars = StaticPrefs::layout_scrollbars_always_layerize_track(); + bool oldShouldActivateAllScrollFrames = mShouldActivateAllScrollFrames; + mShouldActivateAllScrollFrames = + ScrollContainerFrame::ShouldActivateAllScrollFrames(nullptr, + mReferenceFrame); + if (didBuildAsyncZoomContainer != mBuildAsyncZoomContainer) { return true; } @@ -963,6 +968,10 @@ bool nsDisplayListBuilder::ShouldRebuildDisplayListDueToPrefChange() { return true; } + if (oldShouldActivateAllScrollFrames != mShouldActivateAllScrollFrames) { + return true; + } + return false; } diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h @@ -1696,6 +1696,10 @@ class nsDisplayListBuilder { bool ShouldRebuildDisplayListDueToPrefChange(); + bool ShouldActivateAllScrollFrames() const { + return mShouldActivateAllScrollFrames; + } + /** * Represents a region composed of frame/rect pairs. * WeakFrames are used to track whether a rect still belongs to the region. @@ -2026,6 +2030,9 @@ class nsDisplayListBuilder { // display items. bool mAvoidBuildingDuplicateOofs = false; + // Cached copy so we don't have to get the root presshell repeatedly. + bool mShouldActivateAllScrollFrames = false; + Maybe<layers::ScrollDirection> mCurrentScrollbarDirection; };