275574 - [FIX]Crash [@ nsFrame::Destroy] with div:hover {display:-moz-box} with inside an image and some other stuff

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Martijn Wargers (dead)]

The testcase that I'll attach crashes my trunk build, when hovering over the image.

Talkback ID's:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB2675680Z
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB2675895K

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

Comment 2

[Martijn Wargers (dead)]

Output from one of the Talkback ID's:

nsFrame::Destroy 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 640]
nsTextFrame::Destroy 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsTextFrame.cpp,
line 830]
nsPropertyTable::SetProperty 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/content/base/src/nsPropertyTable.cpp,
line 172]
nsContainerFrame::SetOverflowFrames 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsContainerFrame.cpp,
line 1178]
nsInlineFrame::ReflowFrames 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsInlineFrame.cpp,
line 538]
nsInlineFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsInlineFrame.cpp,
line 452]
nsFrame::BoxReflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 5351]
nsFrame::DoLayout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 5091]
nsIFrame::Layout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsBox.cpp,
line 805]
nsBoxFrame::DoLayout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 1099]
nsLineLayout::ReflowFrame 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsLineLayout.cpp,
line 1002]
nsBlockFrame::ReflowInlineFrame 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 4119]
nsBlockFrame::DoReflowInlineFrames 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 3809]
nsBlockFrame::ReflowInlineFrames 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 3698]
nsBlockFrame::ReflowLine 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 2723]
nsBlockFrame::ReflowDirtyLines 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 2234]
nsBlockFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 835]
nsBlockReflowContext::ReflowBlock 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockReflowContext.cpp,
line 547]
nsBlockFrame::ReflowBlockFrame 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 3427]
nsBlockFrame::ReflowLine 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 2604]
nsBlockFrame::ReflowDirtyLines 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 2234]
nsBlockFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsBlockFrame.cpp,
line 835]
nsContainerFrame::ReflowChild 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsContainerFrame.cpp,
line 972]
CanvasFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsHTMLFrame.cpp,
line 549]
nsFrame::BoxReflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 5351]
nsFrame::DoLayout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 5091]
nsIFrame::Layout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsBox.cpp,
line 805]
nsIFrame::Layout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsBox.cpp,
line 805]
nsGfxScrollFrameInner::LayoutBox 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 1666]
nsHTMLScrollFrame::DoLayout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 1083]
nsIFrame::Layout 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsBox.cpp,
line 805]
nsXULScrollFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 1031]
nsContainerFrame::ReflowChild 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsContainerFrame.cpp,
line 972]
ViewportFrame::Reflow 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/generic/nsViewportFrame.cpp,
line 249]
IncrementalReflow::Dispatch 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 908]
PresShell::ProcessReflowCommands 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 6254]
PresShell::FlushPendingNotifications 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 4972]
nsEventStateManager::FlushPendingEvents 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 4421]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 5914]
PresShell::HandleEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 5772]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2402]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2127]
HandleEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp, line
174]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1102]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5387]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5638]
nsWindow::WindowProc 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1383]
USER32.dll + 0x1ef0 (0x77e11ef0)
USER32.dll + 0x204c (0x77e1204c)
USER32.dll + 0x21af (0x77e121af)
nsAppStartup::Run 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 156]
main 
[c:/builds/tinderbox/firefox/WINNT_5.0_Clobber/mozilla/browser/app/nsBrowserApp.cpp,
line 60]
KERNEL32.DLL + 0x2893d (0x7c59893d)

Comment 3

[Mats Palmgren (inactive)]

(gdb) fr 8
#8  0x41ddedb8 in nsFrame::Destroy(nsPresContext*) (this=0x86bef58,
aPresContext=0x0) at nsFrame.cpp:640
640       nsIPresShell *shell = aPresContext->GetPresShell();
(gdb) p aPresContext
$2 = (class nsPresContext *) 0x0

Comment 4

[Boris Zbarsky [:bzbarsky]]

Over to bryner.  The problem is that nsPropertyTable::SetProperty is buggy. 
What I'm not sure about is why this doesn't bite us all the time.

To be precise, the aPropDtorData pointer passed to SetProperty is completely
ignored.  So propertyList->mDtorData is always null, and this is what gets
passed as the dtor data argument to propertyList->mDtorFunc.

In this particular case, mDtorFunc is DestroyOverflowFrames, which wants a
prescontext as the destructor arg, so it can pass it to the frames' Destroy()
method.  But it gets a null prescontext.

What's odd is that I'd expect us to crash the moment we try to destroy an
overflow list (eg any time we destroy a frame that has overflow).  I'm not sure
why that's not happening...

In any case, we need to store the destructor arg somewhere.  Given that it can
apparently be different on a per-object basis, I'm not sure storing it on the
PropertyList like we try to now will work.

Comment 5

[Boris Zbarsky [:bzbarsky]]

It really worries me that this is not crashing, btw...  It suggests that we just
forget to clear out the overflow from the property table a lot of the time or
something.

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, so it looks like we pull overflow frames out to the next-in-flow with
UnsetProperty, which doesn't have to call the destructor... So not crashing all
over is fine.

It also looks like each prescontext has its own property table, so it should be
ok to continue storing the destructor arg in the list.  Just have to bail out if
we don't match args with what's already stored.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Maybe like so?

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 171111 [details] [diff] [review]
Maybe like so?

sr=jst

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED with the testcase at
https://bugzilla.mozilla.org/attachment.cgi?id=169305 using build Mozilla/5.0
(Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050123 on Windows XP
(Seamonkey nightly trunk).