1879441 - Crash in [@ nsTArray_base<T>::Length | mozilla::dom::Selection::StyledRanges::Length]

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/50404a86-f211-495d-868b-7e51e0240128

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>::Length const  xpcom/ds/nsTArray.h:409
0  xul.dll  mozilla::dom::Selection::StyledRanges::Length const  dom/base/Selection.cpp:2062
0  xul.dll  mozilla::dom::Selection::RangeCount const  dom/base/Selection.h:372
0  xul.dll  nsINode::IsSelected const  dom/base/nsINode.cpp:374
1  xul.dll  nsTextFrame::IsFrameSelected const  layout/generic/nsTextFrame.cpp:7447
2  xul.dll  nsIFrame::IsSelected const  xpcom/ds/nsMathUtils.h:68
2  xul.dll  nsTextFrame::UnionAdditionalOverflow  layout/generic/nsTextFrame.cpp:5384
2  xul.dll  nsTextFrame::ReflowText  layout/generic/nsTextFrame.cpp:9775
3  xul.dll  nsLineLayout::ReflowFrame  layout/generic/nsLineLayout.cpp:894
4  xul.dll  nsInlineFrame::ReflowInlineFrame  layout/generic/nsInlineFrame.cpp:667

It looks like there's a possible regression on this signature in 124, as null derefs started showing up. I'm not sure if that's a problem in the DOM selection code or something higher up.

Comment 1

[Henri Sivonen (:hsivonen)]

jjaschke, can you guess a regressor?

Comment 2

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1879441: Added null check for selections in nsINode::IsSelected(). r=smaug,#dom-core

Selections are stored here as WeakPtr to save refcount overhead, which means that a selection can be null.

Comment 3

[Jan Jaeschke [:jjaschke]]

It looks like this crash is caused by nsINode::IsSelected() not checking the ancestorSelections (which are stored as WeakPtr) for being null. Accessing a null Selection would lead to this crash signature.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2e3ebe0ebad
Added null check for selections in nsINode::IsSelected(). r=smaug

Comment 5

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1879441: Added null check for selections in nsINode::IsSelected(). r=smaug,#dom-core

Selections are stored here as WeakPtr to save refcount overhead, which means that a selection can be null.

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

Comment 6

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: This is just a nullptr check.
• Risk associated with taking this patch: None.
• User impact if declined: Occasional (few) crashes
• String changes made/needed: -
• Steps to reproduce for manual QE testing: -
• Fix verified in Nightly: no
• Is Android affected?: yes
• Needs manual QE test: no
• Code covered by automated testing: no

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b2e3ebe0ebad

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/88a4eac14afe