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. STR: 1. Load http://itunes.apple.com/us/app/track-8/id519363162?mt=8 2. Drag the scrollbar to the right to scroll the screenshots. 3. Crash 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: https://crash-stats.mozilla.com/report/list?signature=nsContentSubtreeIterator%3A%3AInit%28nsIDOMRange*%29
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.
5 years ago
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 Try: https://tbpl.mozilla.org/?tree=Try&rev=b2311999e756 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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3284f32a16b https://hg.mozilla.org/integration/mozilla-inbound/rev/e9a9bcd5e1b4 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) > https://hg.mozilla.org/mozilla-central/rev/f3284f32a16b > https://hg.mozilla.org/mozilla-central/rev/e9a9bcd5e1b4 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?