Closed Bug 671319 Opened 8 years ago Closed 8 years ago

Crash [@ nsFrame::ExpandSelectionByMouseMove] part 2

Categories

(Core :: Selection, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
mozilla9

People

(Reporter: martijn.martijn, Assigned: masayuki)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(2 files)

I mentioned in bug 670058, comment 16, that I was still seeing this particular crash with a certain testcase.
I managed to minized that testcase considerably, see zipped attachment.
Open the filed named "parentframe - Copy (2).htm".
Normally, it crashes within 10 seconds or so.

Prior to the crash I'm seeing these assertions in a debug build:
###!!! ASSERTION: aFrameSelection must be handling current drag for selection: '
draggingFrameSelection && draggingFrameSelection == GetConstFrameSelection()', f
ile c:/mozilla-build/firefox_trunk_debug/layout/generic/nsFrame.cpp, line 2576
WARNING: NS_ENSURE_TRUE(contentRoot) failed: file c:/mozilla-build/firefox_trunk
_debug/layout/generic/nsSelection.cpp, line 923
###!!! ASSERTION: aFrame must be in dragging nsFrameSelection: '!draggingFrameSe
lection || draggingFrameSelection == aFrame->GetConstFrameSelection()', file c:/
mozilla-build/firefox_trunk_debug/layout/generic/nsFrame.cpp, line 2118


https://crash-stats.mozilla.com/report/index/bp-397f7a53-c124-4490-a693-7b21a2110713
0 	xul.dll 	nsFrame::ExpandSelectionByMouseMove 	layout/generic/nsFrame.cpp:2622
1 	xul.dll 	nsFrame::HandleDrag 	
2 	xul.dll 	nsFrame::HandleEvent 	layout/generic/nsFrame.cpp:1836
3 	xul.dll 	nsPresShellEventCB::HandleEvent 	layout/base/nsPresShell.cpp:1503
4 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:389
5 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:672
6 	xul.dll 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:7093
7 	xul.dll 	PresShell::HandlePositionedEvent 	layout/base/nsPresShell.cpp:6923
8 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:6758
9 	xul.dll 	PresShell::HandleEvent 	
10 	xul.dll 	nsViewManager::HandleEvent 	view/src/nsViewManager.cpp:1053
11 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1031
12 	xul.dll 	AttachedHandleEvent 	view/src/nsView.cpp:192
13 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:3549
14 	xul.dll 	nsDOMWindowUtils::SendMouseEventCommon 	dom/base/nsDOMWindowUtils.cpp:470
15 	xul.dll 	nsDOMWindowUtils::SendMouseEvent 	dom/base/nsDOMWindowUtils.cpp:371
16 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
17 	xul.dll 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1592
18 		@0x8874b78 
etc..
Attachment 545682 [details] crashes Linux as well:
bp-eb4d9ce1-4ff1-4bd2-bc4f-f32622110713
OS: Windows 7 → All
Hardware: x86 → All
> WARNING: NS_ENSURE_TRUE(contentRoot) failed: file c:/mozilla-build/firefox_trunk
_debug/layout/generic/nsSelection.cpp, line 923

Hmm, this looks bad. I guess that when a document is destroying, it doesn't cancel to dragging.
Thanks, then this is a regression from bug 552707, also.
Blocks: 552707
Hmm, the testcase tests very strange case. Looks like it doesn't dispatch mouseup events but dispatches mousedown and mousemove event on another frameselection content during another dragging transaction. My patch didn't assume such case, so, it could be the cause.

Can you reproduce this bug on actual web pages?
> but dispatches mousedown and mousemove event on another frameselection content during another dragging transaction.

Um, this may be wrong since nsIDOMWindowUtils::SendMouseEvent() dispatches the events via widget. So, the contents to receive the events should be corrected by mouse capture...
Okay, I see the cause. If mousedown event is dispatched (not matter the button), constructor of nsAutoHandlingUserInputStatePusher releases the capturing forcibly. By aborting dragging transaction in nsIPresShell::SetCapturingContent(), I succeeded to fix this bug.

I'm trying to write more possible testcase.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attached patch Patch v1.0Splinter Review
When nsIPresShell::SetCapture() is called, we should abort the dragging transaction.

Even if capturing is canceled by some reasons, current trunk build doesn't cancel the dragging transaction at same time. Therefore, there can be a strange situation that nobody is capturing mouse events but drag for selection isn't canceled.

In most cases, any elements doesn't recapture mouse events. However, if someone which has independent selection and is in another document captures mouse events again, we see this crash.
Attachment #546751 - Flags: review?(Olli.Pettay)
Comment on attachment 546751 [details] [diff] [review]
Patch v1.0

Have you went through all the cases when SetCapturingContent is called to
make sure this is ok?
I think so, the method is used for two ways. One is releasing capture. At this time, we cannot keep the dragging transaction. Therefore, we must abort it. The other is capturing content for something. It's typically called by mousedown event handling. So, dragging shouldn't be doing at that time. Even if such case actually happened, it means the user uses two or more mice at same time. In such case, we don't need to continue the transaction for preventing unexpected behavior.

And also even if web pages called setCapture(), it's refused because setCapture() does nothing if another element has been already capturing.
Comment on attachment 546751 [details] [diff] [review]
Patch v1.0


> nsIPresShell::SetCapturingContent(nsIContent* aContent, PRUint8 aFlags)
> {
>+  // If SetCapturingContent() is called during dragging mouse for selection,
>+  // we should abort current transaction.
>+  nsFrameSelection* fs = nsFrameSelection::GetMouseDownFrameSelection();
nsRefPtr, since AbortDragForSelection may end up deleting fs via some selection
listener.
Attachment #546751 - Flags: review?(Olli.Pettay) → review+
http://hg.mozilla.org/mozilla-central/rev/be5295604fd0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Depends on: 673315
Depends on: 673585
Temporarily, backed out for risk management of mozilla8, see bug 675865.
Resolution: FIXED → WORKSFORME
Target Milestone: mozilla8 → mozilla9
You need to log in before you can comment on or make changes to this bug.