This bug was filed from the Socorro interface and is
report bp-23c182f1-2e02-4428-999e-eff7a2120703 .
Seen while looking at trunk crash stats. Small volume Mac crash - https://crash-stats.mozilla.com/report/list?signature=nsRange::GetCommonAncestor which first started showing up using 2012070203 build.
1. Load http://itunes.apple.com/us/app/track-8/id519363162?mt=8
2. Drag the scrollbar to the right to scroll the screenshots.
Possible regression range based on crash stats: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d9d61d199b11&tochange=a31fc9052840
Frame Module Signature Source
0 XUL nsRange::GetCommonAncestor nsRange.cpp:748
1 XUL nsContentSubtreeIterator::Init nsContentIterator.cpp:1195
2 XUL mozilla::Selection::selectFrames nsSelection.cpp:4007
3 XUL mozilla::Selection::ReplaceAnchorFocusRange nsSelection.cpp:4708
4 XUL nsFrameSelection::HandleClick nsSelection.cpp:1387
5 XUL nsFrameSelection::HandleDrag nsSelection.cpp:1500
6 XUL nsFrame::HandleDrag nsFrame.cpp:2887
7 XUL nsEventTargetChainItem::HandleEventTargetChain nsEventDispatcher.cpp:362
8 XUL nsEventDispatcher::Dispatch nsEventDispatcher.cpp:639
9 XUL PresShell::HandleEventInternal nsPresShell.cpp:6477
10 XUL PresShell::HandlePositionedEvent nsPresShell.cpp:6208
11 XUL PresShell::HandleEvent nsPresShell.cpp:6010
12 XUL PresShell::HandleEvent nsPresShell.cpp:5747
13 XUL nsViewManager::DispatchEvent nsViewManager.cpp:893
14 XUL HandleEvent nsView.cpp:127
15 XUL nsChildView::DispatchEvent nsChildView.mm:1486
16 XUL nsChildView::DispatchWindowEvent nsChildView.mm:1496
17 XUL -[ChildView mouseDragged:] nsChildView.mm:3511
18 AppKit AppKit@0xd57ab
19 libsystem_c.dylib libsystem_c.dylib@0x4d46f
20 libsystem_c.dylib libsystem_c.dylib@0x4d6aa
21 AppKit AppKit@0x98f8bf
22 AppKit AppKit@0x98f8bf
23 Foundation Foundation@0x8405
24 AppKit AppKit@0x6bda3
25 AppKit AppKit@0x6bda3
26 AppKit AppKit@0x6ca73
27 libobjc.A.dylib libobjc.A.dylib@0xb0ff
28 XUL -[ToolbarWindow sendEvent:] nsCocoaWindow.mm:2721
29 AppKit AppKit@0x6ea54
30 libobjc.A.dylib libobjc.A.dylib@0xd2c5
31 libobjc.A.dylib libobjc.A.dylib@0xd254
32 CoreFoundation CoreFoundation@0x309a8
33 AppKit AppKit@0x6d0df
34 AppKit AppKit@0x8aa4
35 XUL nsHTMLBodyElement::GetOnload nsEventNameList.h:354
36 XUL -[GeckoNSApplication sendEvent:] nsAppShell.mm:157
37 AppKit AppKit@0x50c5
38 XUL nsAppShell::Run nsAppShell.mm:756
39 XUL nsAppStartup::Run nsAppStartup.cpp:256
40 XUL XREMain::XRE_mainRun nsAppRunner.cpp:3786
41 XUL XREMain::XRE_main nsAppRunner.cpp:3863
42 XUL XRE_main nsAppRunner.cpp:3939
43 firefox main nsBrowserApp.cpp:160
44 firefox firefox@0x14f3
Crashes affect 10.7 and 10.8 only so far - no 10.6 crashes.
Here are Windows crashes:
There's a good chance it's a regression from bug 767169.
Looks like in some cases aRange is null. This is checked only by MOZ_ASSERT, so it will crash if it's false. This is probably due to bug 767169, because previously selectFrames would return an error if aRange was null and this changed it to MOZ_ASSERT. It looks like it's being called by ReplaceAnchorFocusRange, which is sometimes called when mAnchorFocusRange is null and calls selectFrames to deselect the old range.
I think the right thing to do here is to add a null check to ReplaceAnchorFocusRange. Calling selectFrames on a null range looks bogus and we probably want to know about it.
Created attachment 639284 [details] [diff] [review]
Patch part 1 -- Clarify Selection::selectFrames parameter
It's named "aFlags", which makes no sense. The name in the .h file is "aSelect", which does make sense. So change that and improve the comment a bit.
Created attachment 639285 [details] [diff] [review]
Patch part 2 -- Fix crash due to removed null check in Selection::selectFrames
There are two issues with this patch:
1) It has no test. I reproduced the crash from comment #0 and verified that the patch fixes it, but I don't understand selection code well enough to figure out how to trigger the crash in an automated fashion.
2) I don't know if it's correct. Prior to bug 767169, selectFrames and SetAnchorFocusToRange both would return an error in this case if mAnchorFocusRange was null, which ReplaceAnchorFocusRange would ignore, so it would silently do nothing in that case. This restores that behavior, but using NS_ENSURE_TRUE, so it spams lots of warnings when you try to execute the test case. What's actually happening in this case, and how should we behave? I don't understand what the caller (nsFrameSelection::AdjustForMaintainedSelection) is trying to do -- I see no code documentation for anything related to maintaining selections, and it's not obvious from looking. Probably we don't want a warning here; I'm guessing we want to do a check further up the call chain. But I don't know where.
So this is good for a quick fix for the crash, but I don't know that it's a proper solution.
With combined signatures, it's #6 top browser crasher over the last 3 days.
If someone wants to make sure this gets to m-c before the next nightly is built, please do -- I can't necessarily stick around to watch the tree.
(In reply to comment #9)
A nightly was triggered on this...
Verified fixed using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/16.0 Firefox/16.0. No crash using the reproducible steps in Comment 0.
Not sure what happened with the tracking flags in the past few changes, but I'm untracking this since it is fixed in 16.
It should still be tracking+ in case it gets backed out for example, right?