Content process crashes in ActiveScrolledRoot::AssertDepthInvariant when a position: sticky element is painted inside a CSS-transformed scroll container
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr140 | --- | unaffected |
| firefox151 | --- | wontfix |
| firefox152 | --- | wontfix |
| firefox153 | --- | fixed |
People
(Reporter: pronge, Assigned: tnikkel)
References
(Depends on 1 open bug, Regression)
Details
(Keywords: regression)
Crash Data
Attachments
(3 files, 1 obsolete file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:152.0) Gecko/20100101 Firefox/152.0
Steps to reproduce:
- Open a page that contains a draggable/responsive grid layout. Each grid item is positioned via
transform: translate(...) and contains an overflow: auto scroll container with a position: sticky
child inside it. - Let the page issue an XHR that, when it resolves, causes the grid items to re-render with new
content. - Wait for the XHR to complete.
Reproduces 100% of the time, also in a private window. Firefox 152.0b2 on macOS 15.
Actual results:
The tab crashed at the next paint after the XHR completed.
Crash ID: bp-b9507e22-1d3a-40cd-a2bd-863170260526
Signature: mozilla::ActiveScrolledRoot::AssertDepthInvariant
MOZ_CRASH reason: MOZ_DIAGNOSTIC_ASSERT(mDepth == (mParent ? mParent->mDepth + 1 : 1)) at
layout/painting/nsDisplayList.cpp:377
Top frames:
ActiveScrolledRoot::AssertDepthInvariant nsDisplayList.cpp:377
ActiveScrolledRoot::LowestCommonAncestor nsDisplayList.cpp:292
WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList :2123
nsDisplayWrapList::CreateWebRenderCommandsNewClipListOption :4762
nsDisplayOwnLayer::CreateWebRenderCommands :5439
nsDisplayStickyPosition::CreateWebRenderCommands :5880
nsDisplayTransform::CreateWebRenderCommands :6869
nsDisplayList::PaintRoot :2353
Not OOM (~520 MB available, no JSLargeAllocationFailure).
Expected results:
The page should repaint without the content process aborting. The combination of a transformed
ancestor, an overflow: auto scroll container inside it, and a position: sticky descendant is valid CSS
and shouldn't trip an ASR depth assertion during WebRender display-list construction.
Comment 1•2 months ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::CSS Parsing and Computation' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
| Reporter | ||
Updated•2 months ago
|
| Assignee | ||
Updated•2 months ago
|
| Assignee | ||
Comment 2•2 months ago
|
||
This appears to be another useful crash report for this bug
https://crash-stats.mozilla.org/report/index/df4eea6b-71d6-4f0f-a60f-be5800260526
| Assignee | ||
Comment 3•2 months ago
|
||
| Assignee | ||
Comment 4•2 months ago
|
||
Thanks for reporting!
Based on your instructions I tried to create a testcase but I was unable to reproduce a problem. I uploaded my first attempt, but after this there are 5 more attempts that also didn't reproduce.
Would you be able to upload a testcase? Or link to one? You could use a site like https://jsfiddle.net/ or https://codepen.io/
Or provide more detailed steps to find a testcase? The framework you are using?
| Assignee | ||
Updated•2 months ago
|
| Reporter | ||
Comment 5•2 months ago
|
||
I didn't manage to reproduce it in a public testcase, despite a few attempts (plain HTML, then a Vite project using the same libraries as the real app: @tanstack/react-table + @dnd-kit/sortable + Radix ScrollArea, scaled up to 20 widgets × 200 rows × 24 cols). None crash.
Bisecting in the real app got me somewhere though. The crash needs a specific table component that wraps @tanstack/react-table in @dnd-kit/sortable: one <DndContext>, one <SortableContext> per header row, then one <SortableContext> per cell (around a useSortable'd <td>). A 25 × 8 table mounts about 200 SortableContexts and 225 useSortable registrations. Swapping that table for a plain <table> rendering the same rows via flexRender stops the crash.
Versions if it matters: React 18.3.1, @dnd-kit/core 6.3.1, @dnd-kit/sortable 10.0.0, @dnd-kit/utilities 3.2.2, @dnd-kit/modifiers 9.0.0, @tanstack/react-table 8.20.5, @radix-ui/react-scroll-area 1.2.x. Firefox 152.0b4 on macOS.
The more important finding: the crash isn't from the table being rendered, it's from the table being inserted into a parent that already exists. Two paths through the app produce the same final DOM but only one crashes.
-
Open the page at a small viewport. The widget shows a plain list, not the table. The fetch finishes silently. Then resize the window wider, the widget switches to the table component, which mounts fresh with the data already present. No crash.
-
Open the page at a wide viewport so the table is rendered from the start. It mounts in a loading state with skeletons inside an outer scroll container. The page sits there for a couple of seconds with a spinner animating while the network request is in flight. The request finishes, the loading state flips, and React inserts a new wrapper (
<div style="overflow-x: auto"><table>...</table></div>) into the already-mounted parent. Crash on the next paint, 100%.
So it's specifically splicing a sticky/scroll subtree into a parent that's already been painted. Building the same tree in one initial render is fine. That probably explains why every reduction I tried failed: a setTimeout insertion in a fiddle is still "fresh enough" from Firefox's perspective. The real app sits on a stable, animating, painted parent for several seconds before the insertion.
Sorry this is a bit vague without a public testcase.
Profile from a crash run: https://share.firefox.dev/4vcWPqH
| Assignee | ||
Comment 6•2 months ago
|
||
Would sharing a usernane/password to a private site that reproduces with a small set of Firefox developers be a possibility?
Comment 7•2 months ago
|
||
The bug has a crash signature, thus the bug will be considered confirmed.
| Reporter | ||
Comment 8•2 months ago
|
||
I can see if I can find some credentials you could use. I'll get back to you on that.
I did clone the Firefox source and let an AI agent go through it that had the bug context. I'm not a browser engineer, so I can't make anything useful out of the response. It might provide you with some valuable context though. Hopefully it's not just AI-bloat.
Setup: nsDisplayListBuilder::mActiveScrolledRoots gets cleared every paint in EndFrame (nsDisplayList.cpp:911), but ASRs are also cached on the frame itself via frame properties (ActiveScrolledRootCache, StickyActiveScrolledRootCache). In GetOrCreateASRForFrame and GetOrCreateASRForStickyFrame (nsDisplayList.cpp:168 and :205), aActiveScrolledRoots.AppendElement(asr) only runs inside the if (!asr) branch, so cache hits return an ASR but don't re-append it to the per-paint array.
InsertScrollFrame (nsDisplayList.cpp:603) iterates mActiveScrolledRoots[mDescendantsStartIndex..end). The comment on mDescendantsStartIndex in the header (nsDisplayList.h:1264) says this is intentional, to skip ASRs created in the past. The subtle bit is that "ASRs created in the past" effectively means "ASRs that were cache-hit this paint", not "ASRs created before this scope logically started".
Failure walkthrough:
-
Paint N. A sticky element gets an ASR
SviaGetOrCreateASRForStickyFrame. Cache miss,Sis appended and cached on the frame.S.parent = X,S.depth = X.depth + 1. End of paint clears the array.Spersists on the frame property. -
Paint N+1. Same sticky is processed. Cache hit,
Sis returned but NOT re-appended.S.parentandS.depthare reassigned to the same values. ThenSetCurrentActiveScrolledRoot(S)is called, so descendants of the sticky seeSas their current ASR. -
Inside the sticky's descendants, a new ASR-altering element is encountered for the first time this run. A new ASR
Zis created withZ.parent = S.Zis appended to the array.Z.depth = S.depth + 1 = X.depth + 2. -
Later, a sibling scroll frame causes
ShouldForceLayerForScrollParentto return true on the enclosing scroll, andInsertScrollFrameruns. New ASRYis created for that scroll, withY.parent = X. The loop iterates new ASRs in the range. It reachesZ.IsAncestor(X, Z)walksZ → S → Xand returns true.Z.IncrementDepth()runs.Z.parent (S) != X, so no re-parent.Z.depthis nowX.depth + 3. -
Swas a cache hit, so it's not in the loop range.S.depthis unchanged atX.depth + 1. -
After the loop:
Z.depth = X.depth + 3andZ.parent = SwithS.depth = X.depth + 1.Z.depth != S.depth + 1. The invariant is broken. The new assertion at line 627 doesn't fire because it only asserts on the same range. Later,LowestCommonAncestorwalks throughZand callsAssertDepthInvarianton it, and it fires.
One possible fix: in the loop, replace descendantASR->IncrementDepth() with descendantASR->mDepth = descendantASR->mParent->mDepth + 1 after the re-parent step. That recomputes depth from the (possibly updated) parent instead of blindly adding one, which would handle the case where the parent's depth didn't move. Probably has its own caveats.
Comment 9•2 months ago
|
||
Tentatively triaging as S3, since this doesn't seem widespread. It seems fairly severe for those who experience it, though, so this might go up if this is experienced by a wide set of people.
I'll try to reproduce this, too, once a reproducible example is provided from OP.
| Reporter | ||
Comment 10•2 months ago
|
||
Where can I forward you some login details for the site? The crash happens instantly after logging in, so that would probably be the easiest path forward
Comment 12•2 months ago
•
|
||
After some debugging together via Matrix, Patric determined that the issue only seemed to be reproducing while using a Lenovo ThinkPad Universal USB-C Dock (40AY) from an M3 MacBook Pro with Tahoe 26.5. Otherwise, this reproduced the same between Beta and Nightly.
AIUI, this sort of dock may have separate hardware that might be handled differently in the rendering pipeline. I'm not sure if that (if true) would change the fundamental behavior in causing this crash, but it seems likely.
Comment 13•2 months ago
|
||
(In reply to Erich Gubler [:ErichDonGubler] (he/him) from comment #9)
It seems fairly severe for those who experience it, though, so this might go up if this is experienced by a wide set of people.
Note that the assertion that's failing is a MOZ_DIAGNOSTIC_ASSERT. I guess that's enabled on the beta channel these days, but it shouldn't be enabled on the release channel.
| Assignee | ||
Comment 14•2 months ago
|
||
MOZ_DIAGNOSTIC_ASSERT is enabled in early beta (the first week or two of the beta cycle) and nightly.
| Assignee | ||
Comment 15•2 months ago
|
||
What was the prompts and input that you gave the AI to generate that info? This would help me understand it better
(In reply to Patric Ronge from comment #8)
Setup:
nsDisplayListBuilder::mActiveScrolledRootsgets cleared every paint inEndFrame(nsDisplayList.cpp:911), but ASRs are also cached on the frame itself via frame properties (ActiveScrolledRootCache,StickyActiveScrolledRootCache). InGetOrCreateASRForFrameandGetOrCreateASRForStickyFrame(nsDisplayList.cpp:168and:205),aActiveScrolledRoots.AppendElement(asr)only runs inside theif (!asr)branch, so cache hits return an ASR but don't re-append it to the per-paint array.
InsertScrollFrame(nsDisplayList.cpp:603) iteratesmActiveScrolledRoots[mDescendantsStartIndex..end). The comment onmDescendantsStartIndexin the header (nsDisplayList.h:1264) says this is intentional, to skip ASRs created in the past. The subtle bit is that "ASRs created in the past" effectively means "ASRs that were cache-hit this paint", not "ASRs created before this scope logically started".
InsertScrollFrame should never be called on your machine. In fact, you can see here
that we insert a critical note into any crashes that might happen if it is ever called, and I looked in your crash reports and there is no such note. So that means it either wasn't called, or ShouldActivateAllScrollFrames is false in your build. Which is very unexpected, but possible. If that was the case we would want to look into why that happened.
Can you provide the contents of about:support from the machine that is affected when it is in the affected configuration (ie in the dock)? That would probably tell us if anything looks wrong that would change the value of ShouldActivateAllScrollFrames.
Erich shared with me the repro site and login info, but I have so far been unable to reproduce the issue. But I can look into the site content for more clues.
| Assignee | ||
Comment 16•2 months ago
|
||
Also, running https://mozilla.github.io/mozregression/ in the affected configuration to see exactly what changed caused this to start failing would be quite helpful.
| Assignee | ||
Comment 17•2 months ago
•
|
||
One possible reason the dock is needed is that the you have a mouse connected to the dock and that can affect the type of scrollbars you get on macos. In the mac system settings, search for "scroll bar", and then find the setting "Show scroll bars" that has options "Automatically based on mouse or trackpad" "When scrolling" "Always" and report what your setting is.
| Assignee | ||
Comment 18•2 months ago
•
|
||
I also put a printf in the CSS anchor pos functions that muck with ASRs and it was not hit when I loaded the reproducing website privately provided by the reported. So that means the CSS anchor pos ASR stuff should not come into play, which simplifies the debugging and what could be going wrong.
| Assignee | ||
Comment 19•2 months ago
|
||
And also no sticky ASRs created either.
| Assignee | ||
Comment 20•2 months ago
|
||
I can reproduce with the private site now, I needed to increase the width of my window.
| Assignee | ||
Comment 21•2 months ago
|
||
Comment 22•2 months ago
|
||
Set release status flags based on info from the regressing bug 2001862
| Reporter | ||
Comment 23•2 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #17)
One possible reason the dock is needed is that the you have a mouse connected to the dock and that can affect the type of scrollbars you get on macos. In the mac system settings, search for "scroll bar", and then find the setting "Show scroll bars" that has options "Automatically based on mouse or trackpad" "When scrolling" "Always" and report what your setting is.
Ah yes. I had Auto selected. I'm able to replicate the crash 100% of the time in my app when changing that setting from "Automatically based on mouse or trackpad" to "Always", with a viewport of >1600px
No external monitor or dock needed. Reproduced on just my macbook
| Assignee | ||
Comment 24•2 months ago
|
||
Got a small reproducing testcase.
| Assignee | ||
Updated•2 months ago
|
| Assignee | ||
Comment 25•2 months ago
|
||
And this is a testcase that is similar but triggers the same assert without using sticky ASRs at all (which changed wholesale in Nov). So this likely shows the bug has existed for a long time (not confirmed yet).
Updated•2 months ago
|
| Assignee | ||
Comment 26•2 months ago
|
||
Take an outer scroll container A that is async-inactive because it has zero scroll range. It contains various active scroll frames and active nested scroll frames. Some content inside A (but not related to any of the active descendant scroll frames) is modified so that the size of the scrolled content increases making the scroll range non-zero and now WantAsyncScroll() is true. The size increase triggers a retained display list update. None of the descendant already-active scroll frames are marked modified, so the display list build does not visit the every active descendant scroll frame (even if we were to visit every direct ASR child of A). Visiting the frame of an ASR is the only way that we can update it to changes in the ASR tree, so some ASRs do not get updated, so the depth values get out of sync and we hit the depth invariant assert.
The most obvious way to fix this is to check after reflow if a scroll frame has changed scroll range that changed the return value of WantAsyncScroll and mark it modified. But this is too large of a hammer because the extra display list building the modified frames will trigger, but also because WantAsyncScroll is relatively heavy. As such we limit the check as narrowly as we can. We use a bool that tracks scroll frames that were inactive during the previous paint but that have active scroll frame descendants. Further, previous work in bug 2044231 reduced the cost of WantAsyncScroll by caching GetScrolledRect during ReflowFinished. The main cost of WantAsyncScroll will then be the memory load of fetching the overflow values from the style display struct which might not be cached at this point (even though they are fetched at the start of the reflow of this scroll frame) because this happens after the reflow of the entire frame tree. Lower down in ReflowFinished we call GetScrollRangeForUserInputEvents() inside |if (mVScrollbarBox || mHScrollbarBox)| which will also fetch those values. So hopefully this should not add many memory fetches.
I also checked that anything else that can change WantAsyncScroll's return value should not have this problem.
Updated•2 months ago
|
Comment 27•2 months ago
|
||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/60389 for changes under testing/web-platform/tests
Comment 29•2 months ago
|
||
Comment 30•2 months ago
|
||
Backed out for causing wpt failures @scrollend-event-fires-to-iframe-window.html.
Upstream PR was closed without merging
| Assignee | ||
Comment 32•2 months ago
|
||
The failure was not actually caused by anything in this patch, but because a field was not init'ed, bug 2042439.
Comment 33•2 months ago
|
||
Comment 34•2 months ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
Comment 36•2 months ago
|
||
The patch landed in nightly and beta is affected.
:tnikkel, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
- If no, please set
status-firefox152towontfix.
For more information, please visit BugBot documentation.
| Assignee | ||
Updated•2 months ago
|
Updated•2 months ago
|
Description
•