Closed Bug 531175 Opened 10 years ago Closed 10 years ago

Crash [@ nsPresShellEventCB::HandleEvent(nsEventChainPostVisitor&)]

Categories

(Core :: Layout, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta5-fixed

People

(Reporter: mats, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: crash, Whiteboard: [fixed by bug 530880])

Crash Data

Attachments

(1 file)

Crash [@ nsPresShellEventCB::HandleEvent(nsEventChainPostVisitor&)]
It's #196 in the frame poisoning list in bug 526587:
https://bugzilla.mozilla.org/attachment.cgi?id=414317

It's relatively rare, only 31 crashes in 4 weeks (29 Windows, 2 OSX).
There are crash reports for Firefox 3.0x 3.5x 3.6x and 3.7x
http://crash-stats.mozilla.com/report/list?query_search=signature&query_type=exact&query=nsPresShellEventCB%3A%3AHandleEvent%28nsEventChainPostVisitor%26%29&date=&range_value=4&range_unit=weeks&do_query=1&signature=nsPresShellEventCB%3A%3AHandleEvent%28nsEventChainPostVisitor%26%29

Stack for bp-5c9708a6-063f-4de4-8b27-013ef2091122:
@0x12f837	
nsPresShellEventCB::HandleEvent	layout/base/nsPresShell.cpp:1375
nsEventTargetChainItem::HandleEventTargetChain	content/events/src/nsEventDispatcher.cpp:346
nsEventDispatcher::Dispatch	content/events/src/nsEventDispatcher.cpp:514
PresShell::HandleEventInternal	layout/base/nsPresShell.cpp:6323
PresShell::HandlePositionedEvent	layout/base/nsPresShell.cpp:6211
PresShell::HandleEvent	layout/base/nsPresShell.cpp:6071
nsViewManager::HandleEvent	view/src/nsViewManager.cpp:1400
nsViewManager::DispatchEvent	view/src/nsViewManager.cpp:1359
HandleEvent	view/src/nsView.cpp:168
nsWindow::DispatchEvent	widget/src/windows/nsWindow.cpp:1051
nsWindow::DispatchWindowEvent	widget/src/windows/nsWindow.cpp:1071
nsWindow::DispatchMouseEvent	widget/src/windows/nsWindow.cpp:6614
ChildWindow::DispatchMouseEvent	widget/src/windows/nsWindow.cpp:6761
nsWindow::ProcessMessage	widget/src/windows/nsWindow.cpp:4618
nsWindow::WindowProc	widget/src/windows/nsWindow.cpp:1267
InternalCallWinProc	
UserCallWinProcCheckWow	
DispatchMessageWorker	
DispatchMessageW	
nsAppShell::ProcessNextNativeEvent	widget/src/windows/nsAppShell.cpp:165
nsBaseAppShell::OnProcessNextEvent	widget/src/xpwidgets/nsBaseAppShell.cpp:278
nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:508
nsBaseAppShell::Run	widget/src/xpwidgets/nsBaseAppShell.cpp:170
nsAppStartup::Run	toolkit/components/startup/src/nsAppStartup.cpp:193
PR_GetEnv	
wmain	toolkit/xre/nsWindowsWMain.cpp:110
__tmainCRTStartup	obj-firefox/memory/jemalloc/src/crtexe.c:591
BaseProcessStart
The problem appears to be that nsPresShell::mCurrentEventFrame points
to a destroyed frame:
http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/35bb84e06502/layout/base/nsPresShell.cpp#l1321

We reset mCurrentEventFrame and all the frames in mCurrentEventFrameStack
(if it's the same as the destroyed frame) in PresShell::ClearFrameRefs()
but not in PresShell::NotifyDestroyingFrame(), and ClearFrameRefs() is only
called for some frames:

417 nsFrame::Destroy()
...
441   shell->NotifyDestroyingFrame(this);
442 
443   if ((mState & NS_FRAME_EXTERNAL_REFERENCE) ||
444       (mState & NS_FRAME_SELECTED_CONTENT)) {
445     shell->ClearFrameRefs(this);
446   }

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#417

I can't find any code that adds NS_FRAME_EXTERNAL_REFERENCE for frames
referenced by mCurrentEventFrame or mCurrentEventFrameStack.

The straight-forward fix is to move the reset code from ClearFrameRefs()
to NotifyDestroyingFrame(). (nsFrame::Destroy() is the only consumer
of these methods on all branches AFAICT)

Or, change the type s/nsIFrame*/nsWeakFrame/ for mCurrentEventFrame and mCurrentEventFrameStack - which would add the NS_FRAME_EXTERNAL_REFERENCE bit
to those frames (and thus reach ClearFrameRefs()).
http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/35bb84e06502/layout/base/nsPresShell.cpp#l1566

Patch coming up...
Attached patch Patch rev. 1Splinter Review
Looks like we (theoretically) have the same problem for mDrawEventTargetFrame
(NS_DEBUG only though) and mFramesToDirty, so I moved the code clearing
those also.  They are now under the !mIgnoreFrameDestruction condition so
I added clearing them in PresShell::Destroy() like for mCurrentEventFrame/
mCurrentEventFrameStack.
Attachment #414682 - Flags: review?(roc)
Yes, that's essentially the same fix, without the assertions I added
for the mIgnoreFrameDestruction case.
Fixed by bug 530880.
Status: NEW → RESOLVED
Closed: 10 years ago
Depends on: 530880
Resolution: --- → FIXED
Whiteboard: [fixed by bug 530880]
Attachment #414682 - Flags: review?(roc)
Crash Signature: [@ nsPresShellEventCB::HandleEvent(nsEventChainPostVisitor&)]
Group: core-security
You need to log in before you can comment on or make changes to this bug.