1990034 - Crash in [@ mozilla::StyleGenericSize<T>::AsLengthPercentage]

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Crash report: https://crash-stats.mozilla.org/report/index/335e62af-a2c2-43bb-a0ec-4a9b80250922

MOZ_CRASH Reason:

MOZ_DIAGNOSTIC_ASSERT(IsLengthPercentage())

Top 10 frames:

0  xul.dll  AnnotateMozCrashReason(char const*)  mfbt/Assertions.h:59
0  xul.dll  mozilla::StyleGenericSize<mozilla::StyleLengthPercentageUnion>::AsLengthPerce...  layout/style/ServoStyleConsts.h:14122
0  xul.dll  nsLayoutUtils::ComputeBSizeValueHandlingStretch(int, int, int, int, mozilla::...  layout/base/nsLayoutUtils.h:1615
0  xul.dll  nsIFrame::ComputeBSizeValueAsPercentageBasis(mozilla::StyleGenericSize<mozill...  layout/generic/nsIFrame.cpp:6991
0  xul.dll  nsIFrame::ComputeISizeValue(gfxContext*, const mozilla::WritingMode, mozilla:...  layout/generic/nsIFrame.cpp:7345
1  xul.dll  nsIFrame::ComputeISizeValue(gfxContext*, const mozilla::WritingMode, mozilla:...  layout/generic/nsIFrame.h:5054
1  xul.dll  mozilla::SizeComputationInput::ComputeISizeValue(mozilla::LogicalSize const&,...  layout/generic/ReflowInput.cpp:367
1  xul.dll  mozilla::ReflowInput::ComputeMinMaxValues(mozilla::LogicalSize const&)  layout/generic/ReflowInput.cpp:3093
1  xul.dll  mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::Maybe<mozilla:...  layout/generic/ReflowInput.cpp:2346
1  xul.dll  mozilla::ReflowInput::Init(nsPresContext*, mozilla::Maybe<mozilla::LogicalSiz...  layout/generic/ReflowInput.cpp:490

There are actually 3 crash reports from what looks like the same user (based on hardware) submitted within ~2 minutes, likely attempting to load the same site again and then giving up:
bp-335e62af-a2c2-43bb-a0ec-4a9b80250922
bp-6099fa6a-3892-41bf-8ffa-032d90250922
bp-d455247d-b8b4-425f-bcee-5bfe80250922

There's no URL or comment included in the crash report, but the backtrace & source links might be enough to go on...

Note that this is the same crash signature as bug 1989368 (and likely same regressor, which added a new non-LengthPercentage type of size); but that bug is specific to table layout, whereas this one does not seem to be, based on the backtrace. Hence, filing separately.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1988938

Comment 3

[Daniel Holbert [:dholbert]]

Looks like every crash report here involves nsIFrame::ComputeISizeValue working with its aStyleBSize param.
https://searchfox.org/firefox-main/rev/fd531836ca5e48e18a5afc171418bcbcb9d268e2/layout/generic/nsIFrame.cpp#7305

• in bp-73c1dd54-1f69-4ce4-a5e6-7ad760250926, it's the AsLengthPercentage here

Maybe<nscoord> iSizeFromAspectRatio = [&]() -> Maybe<nscoord> {
...
  return Some(ComputeISizeValueFromAspectRatio(
      aWM, aCBSize, aContentEdgeToBoxSizing, aStyleBSize.AsLengthPercentage(),
      aAspectRatio));

https://searchfox.org/firefox-main/rev/fd531836ca5e48e18a5afc171418bcbcb9d268e2/layout/generic/nsIFrame.cpp#7333-7334
(This is a case where aStyleBSize is -webkit-fill-available and we have a constrained aCBSize.BSize(aWM), so we don't take the IsAutoBSize return on the previous line.)

• in bp-952788e2-a23d-4d83-97f1-7bfdf0250925, it's a AsLengthPercentage buried a bit here:

const nscoord bSize = ComputeBSizeValueAsPercentageBasis(
    aStyleBSize, *stylePos->MinBSize(aWM, anchorResolutionParams),
    *stylePos->MaxBSize(aWM, anchorResolutionParams), aCBSize.BSize(aWM),
    aContentEdgeToBoxSizing.BSize(aWM));

https://searchfox.org/firefox-main/rev/fd531836ca5e48e18a5afc171418bcbcb9d268e2/layout/generic/nsIFrame.cpp#7340-7343

That^ ComputeBSizeValueAsPercentageBasis invocation calls ComputeBSizeValueHandlingStretch, which crashes on the final statement which implies that the bsize there is something other than -webkit-fill-available (since -webkit-fill-available would take the aSize.BehavesLikeStretchOnBlockAxis()` early-return).
https://hg-edge.mozilla.org/mozilla-central/file/e0c3190130f016554a7c41fee5b70832d74327c1/layout/base/nsLayoutUtils.h#l1615

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (warning, may crash your content process)

Here's a testcase that triggers the crash-flavor described in the first bullet-point in comment 3.

Comment 5

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: crash, regression in 145, likely not hard to fix. (I was hoping to fix during this Nightly cycle but here we are at soft-freeze). Want to be sure we don't ship to release, so I want to fix early in the next cycle and uplift to beta.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox145 (nightly). However, the bug still has low severity.

:fgriffith, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 content process crashes on beta

For more information, please visit BugBot documentation.

Comment 8

[Daniel Holbert [:dholbert]]

We switched this feature off on beta145 (bug 1988938 comment 18), so there's no need to track for that release anymore.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Based on the topcrash criteria, the crash signature linked to this bug is not a topcrash signature anymore.

For more information, please visit BugBot documentation.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: Bug 1990034: Properly handle 'stretch'-like block-size values in nsIFrame::ComputeISizeValue and several functions that end up indirectly calling it. r?#layout

We need to do this in order to avoid hitting nsIFrame::ComputeISizeValue's call
to 'AsLengthPercentage()' with 'stretch' (which is not a LengthPercentage);
that triggers an abort right now.

The new code in nsIFrame::ComputeISizeValue is the proximal band-aid here, but
that isn't sufficient for correctness, because it relies on knowing the
computed margin, border, and padding; and we might not yet have the correct
values stashed on the frame, as mentioned in a code-comment in this patch. We
do have the resolved margin/border/padding available up the stack in the
callsite in ReflowInput.cpp, though (the callsite in this bug's backtrace) --
so this patch eagerly resolves 'stretch' at that point, so that
nsIFrame::ComputeISizeValue doesn't need to bother handling it.

This patch also updates nsContainerFrame::ComputeSize to match the
corresponding chunk of nsIFrame::ComputeSize (just adding some code to handle
'stretch' there). This is needed to get the correct answer on a few of the
cases in the included WPT. (We had a code-comment noting that we should do
this, but we didn't previously have any tests where it would make a
difference. Now we do - hooray!)

Comment 11

[Daniel Holbert [:dholbert]]

I went through the last 6 months of crash volume with this crash signature, and, for the crash reports that contain a (privately-included) URL that I can actually load (it's not behind a login wall etc), I loaded the URL and confirmed that I don't crash (and don't see any obvious brokenness) in a build with this bug's patch and bug 1989368's patch.

So I think these two bugs' patches address all or nearly-all of the crash volume under this signature.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/41d71b9a9826
https://hg.mozilla.org/integration/autoland/rev/69c8e4489245
Properly handle 'stretch'-like block-size values in nsIFrame::ComputeISizeValue and several functions that end up indirectly calling it. r=layout-reviewers,TYLin

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/69c8e4489245

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/56057 for changes under testing/web-platform/tests

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:dholbert, 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-firefox146 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Daniel Holbert [:dholbert]]

I just tested Nightly 147.0a1 (2025-11-17) (64-bit) with the attached testcase and with a known-to-have-been-affected site, and I confirmed that I can load correctly with no crash.

(There are other sites/testcases that still crash with the same signature and a different backtrace, which I'm looking at; see bug 1989368.)

--> Calling this bug VERIFIED FIXED

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #15)

The patch landed in nightly and beta is affected.
:dholbert, is this bug important enough to require an uplift?

Yes, I'll nominate now.

Comment 18

[Daniel Holbert [:dholbert]]

Attachment: Bug 1990034: Properly handle 'stretch'-like block-size values in nsIFrame::ComputeISizeValue and several functions that end up indirectly calling it.

We need to do this in order to avoid hitting nsIFrame::ComputeISizeValue's call
to 'AsLengthPercentage()' with 'stretch' (which is not a LengthPercentage);
that triggers an abort right now.

The new code in nsIFrame::ComputeISizeValue is the proximal band-aid here, but
that isn't sufficient for correctness, because it relies on knowing the
computed margin, border, and padding; and we might not yet have the correct
values stashed on the frame, as mentioned in a code-comment in this patch. We
do have the resolved margin/border/padding available up the stack in the
callsite in ReflowInput.cpp, though (the callsite in this bug's backtrace) --
so this patch eagerly resolves 'stretch' at that point, so that
nsIFrame::ComputeISizeValue doesn't need to bother handling it.

This patch also updates nsContainerFrame::ComputeSize to match the
corresponding chunk of nsIFrame::ComputeSize (just adding some code to handle
'stretch' there). This is needed to get the correct answer on a few of the
cases in the included WPT. (We had a code-comment noting that we should do
this, but we didn't previously have any tests where it would make a
difference. Now we do - hooray!)

Original Revision: https://phabricator.services.mozilla.com/D272730

Comment 19

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: This is a fix for a crash that we're seeing users hitting in the wild (in builds with support for -webkit-fill-available enabled, which right now is 146 and 147).
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: 1. Load attached testcase ( https://bugzilla.mozilla.org/attachment.cgi?id=9516561 )

Correct/expected results: purple square.
Buggy results: crash.

• Risk associated with taking this patch: low
• Explanation of risk level: The patch is narrowly targeted to only affect situations where we would have been at-risk-of-crashing.
• String changes made/needed: None
• Is Android affected?: yes

Comment 20

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/c9321c6197f1
https://hg.mozilla.org/releases/mozilla-beta/rev/ed822df375d2

Comment 21

[Bogdan Maris, Desktop Test Engineering]

Reproduced the crash using both the testcase attached to this bug and the https://aprendeconalf.es/ website using an old Nightly from 2025-09-22 on Windows 11 before the fix.
https://crash-stats.mozilla.org/report/index/23f47b86-a652-45c7-8213-825ac0251119
https://crash-stats.mozilla.org/report/index/549c9ede-0d11-411e-ac38-c8ed00251119

Verified that I don't get any more crashes using both the testcase and https://aprendeconalf.es/ website using the latest Nightly 147.0a1 and Firefox Beta 146.0b5 on Windows 11.

Comment 22

[Daniel Holbert [:dholbert]]

Thanks!