1810234 - Crash in [@ nsIContent::GetPrimaryFrame] and others from nsNumberControlFrame::SpinnerStateChanged()

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/2f908798-253a-4128-a644-2b8120230111

Reason: SIGSEGV / SEGV_MAPERR

Top 10 frames of crashing thread:

0  libxul.so  nsIContent::GetPrimaryFrame const  dom/base/nsIContent.h:547
0  libxul.so  nsNumberControlFrame::SpinnerStateChanged const  layout/forms/nsNumberControlFrame.cpp:143
1  libxul.so  mozilla::dom::HTMLInputElement::GetEventTargetParent  dom/html/HTMLInputElement.cpp
2  libxul.so  mozilla::EventTargetChainItem::GetEventTargetParent  dom/events/EventDispatcher.cpp:416
2  libxul.so  mozilla::EventDispatcher::Dispatch  dom/events/EventDispatcher.cpp:974
3  libxul.so  mozilla::PresShell::EventHandler::DispatchEventToDOM  layout/base/PresShell.cpp:8734
4  libxul.so  mozilla::PresShell::EventHandler::DispatchEvent  layout/base/PresShell.cpp:8306
5  libxul.so  mozilla::PresShell::EventHandler::HandleEventWithCurrentEventInfo  layout/base/PresShell.cpp:8238
6  libxul.so  mozilla::PresShell::EventHandler::HandleEventUsingCoordinates  layout/base/PresShell.cpp:7187
7  libxul.so  mozilla::PresShell::EventHandler::HandleEvent  layout/base/PresShell.cpp:6990

This shows up under a few other signatures, too.

[@ nsNumberControlFrame::SpinnerStateChanged ] in ESR: bp-0da07860-4449-484a-bee7-9777c0230113

SpinnerStateChanged has an assertion that mSpinUp and mSpinDown are non-null, but I'm not sure exactly what ensures it. It looks like mSpinUp (and presumably mSpinDown) are null sometimes, so maybe we could add a check for it?

I found 136 crashes in the last month with SpinnerStateChanged in the proto signature and the ones I looked at all looked kind of like this.

Comment 1

[Andrew McCreight [:mccr8]]

A third signature it shows up fairly often under is [@ nsINode::GetBoolFlag ]: bp-3d576fe7-fa94-4168-99ca-fa0a10230112

Comment 2

[Andrew McCreight [:mccr8]]

GetBoolFlag is fairly generic so I don't think it is worth adding as a signature to this bug.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1810234 - Null-check in nsNumberControlFrame::SpinnerStateChanged. r=#layout

I don't have a repro, but it doesn't seem too unbelievable that if the
appearance changes mid-click we could end up with no spinners but a call
here.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74a8d4dbc831
Null-check in nsNumberControlFrame::SpinnerStateChanged. r=layout-reviewers,jfkthame

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/74a8d4dbc831

Comment 6

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox110 to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9314230 [details]
Bug 1810234 - Null-check in nsNumberControlFrame::SpinnerStateChanged. r=#layout

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): null-check
• String changes made/needed: none
• Is Android affected?: Yes

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9314230 [details]
Bug 1810234 - Null-check in nsNumberControlFrame::SpinnerStateChanged. r=#layout

Approved for 110 beta 8, thanks.

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/bdef914120fe