712937 - crash in nsTypedSelection::selectFrames

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-bb7f7db3-9398-4501-885b-6355d2111222 .
============================================================= 

Crash stack:
0 	xul.dll 	nsTypedSelection::selectFrames 	layout/generic/nsSelection.cpp:4369
1 	xul.dll 	nsWindowRoot::AddRef 	content/base/src/nsInProcessTabChildGlobal.cpp:168
2 	xul.dll 	nsTypedSelection::ReplaceAnchorFocusRange 	layout/generic/nsSelection.cpp:5075
3 	xul.dll 	nsFrameSelection::AdjustForMaintainedSelection 	layout/generic/nsSelection.cpp:1634
4 	xul.dll 	nsIFrame::GetContentOffsetsFromPoint 	layout/generic/nsFrame.cpp:3358
5 	xul.dll 	nsFrameSelection::HandleClick 	layout/generic/nsSelection.cpp:1673
6 	xul.dll 	nsFrameSelection::HandleDrag 	layout/generic/nsSelection.cpp:1749
7 	xul.dll 	nsLayoutUtils::GetEventCoordinatesRelativeTo 	layout/base/nsLayoutUtils.cpp:983
8 	xul.dll 	nsFrame::DisplaySelection 	layout/generic/nsFrame.cpp:1147
9 	xul.dll 	nsCOMPtr<nsIDOMStorage>::StartAssignment 	obj-firefox/dist/include/nsCOMPtr.h:843
10 	xul.dll 	nsFrame::HandleDrag 	
11 	xul.dll 	nsJSContext::Release 	dom/base/nsJSEnvironment.cpp:1166
12 	xul.dll 	nsFrame::HandleEvent 	layout/generic/nsFrame.cpp:2146
13 	xul.dll 	nsPresShellEventCB::HandleEvent 	layout/base/nsPresShell.cpp:760
14 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:393
15 	xul.dll 	nsGlobalChromeWindow::AddRef 	dom/base/nsGlobalWindow.cpp:1402
16 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:681


More reports are listed at https://crash-stats.mozilla.com/report/list?signature=nsTypedSelection::selectFrames(nsPresContext*,%20nsIRange*,%20bool)

This started in the 2011-12-20 builds on 12.0a1 trunk and had over 30 crashes both on that and the -21 build so far.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Almost certainly a regression from bug 619273.

Comment 2

[Mats Palmgren (inactive)]

It looks like a null-pointer crash at line 4369.
http://hg.mozilla.org/mozilla-central/annotate/cd921d073b22/layout/generic/nsSelection.cpp#l4369
and indeed I inadvertently removed a null check on aRange (at 1.343):
http://hg.mozilla.org/mozilla-central/diff/e3766ee732cc/layout/generic/nsSelection.cpp#l1.323

I don't quite understand why it's null though since AdjustForMaintainedSelection
does check for a null 'mMaintainRange':
http://hg.mozilla.org/mozilla-central/annotate/cd921d073b22/layout/generic/nsSelection.cpp#l1600

I should add back that null-check in selectFrames() anyway...

Comment 3

[Mats Palmgren (inactive)]

Attachment: fix

Comment 4

[Mats Palmgren (inactive)]

Ah, now I see that ReplaceAnchorFocusRange() calls selectFrames() 
for mAnchorFocusRange, not aRange, so that could explain it.
http://hg.mozilla.org/mozilla-central/annotate/cd921d073b22/layout/generic/nsSelection.cpp#l5070

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 583819 [details] [diff] [review]
fix

r=me

Comment 7

[Marcia Knous [:marcia]]

Adding Mac version of the signature.

Comment 8

[Mats Palmgren (inactive)]

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

Comment 9

[Scoobidiver (away)]

It's #4 top crasher in 11.0a2.

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 583819 [details] [diff] [review]
fix

Adds back a missing null-check.  Zero risk.

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 583819 [details] [diff] [review]
fix

[Triage Comment]
Top crasher for FF11. Approved for Aurora.

Comment 12

[Mats Palmgren (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a39673dc41bc

Comment 13

[u279076]

Is there something QA can do to verify this fix, apart from checking crashstats?

Comment 14

[Virgil Dicu [:virgil] [QA]]

Still about 10 crashes since the fix landed on Aurora (2012-01-06), if I'm reading this correctly. 

https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=nsTypedSelection%3A%3AselectFrames%28nsPresContext%2A%2C%20nsIRange%2A%2C%20bool%29

Comment 15

[Scoobidiver (away)]

(In reply to Virgil Dicu [:virgil] [QA] from comment #14)
> Still about 10 crashes since the fix landed on Aurora (2012-01-06), if I'm
> reading this correctly.
I see one crash in 11.0b1 but with a different stack: bp-fb5f61b5-7763-423e-b5c4-5dfb12120207.
The latest crashes on 11.0 happened in 11.0a2/20120106 and on 12.0 in 12.0a1/20111223.