402713 - [FIX]bug 402649 testcase crashes [@ nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame]

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Daniel Veditz [:dveditz]]

The "joke" version of the testcase in bug 402649 crashes Minefield. Filing as a separate bug just in case it represents an unrelated problem that won't be fixed by whatever fixes that timing issue. I think it needs to be fixed first or else fixing bug 402649 might mask this crash while leaving the problem untouched.

I first get the assertion

###!!! ASSERTION: Should be in an update while destroying frames: 'mUpdateCount != 0', file c:/dev/fftrunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9572

And then crashes referencing a deleted aFrame in nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame()

 	dddddddd	
>	nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame() Line 11229	C++
 	nsCSSFrameConstructor::ContentRemoved() Line 9617	C++
 	PresShell::ContentRemoved() Line 4631	C++
 	nsBindingManager::ContentRemoved() Line 1498	C++
 	nsNodeUtils::ContentRemoved() Line 166	C++
 	nsHTMLDocument::OpenCommon() Line 2214	C++
 	nsHTMLDocument::Open() Line 2311	C++
 	nsHTMLDocumentSH::DocumentOpen() Line 7658	C++
 	XPC_XOW_FunctionWrapper() Line 324	C++
 	js_Invoke() Line 1386	C
 	js_Interpret() Line 4146	C
 	js_Invoke() Line 1406	C
 	js_InternalInvoke() Line 1462	C
 	JS_CallFunctionValue() Line 4882	C
 	nsJSContext::CallEventHandler() Line 1904	C++
 	nsJSEventListener::HandleEvent() Line 235	C++
 	nsEventListenerManager::HandleEventSubType() Line 1097	C++
 	nsEventListenerManager::HandleEvent() Line 1217	C++
 	nsEventTargetChainItem::HandleEvent() Line 206	C++
 	nsEventTargetChainItem::HandleEventTargetChain() Line 266	C++
 	nsEventDispatcher::Dispatch() Line 479	C++
 	PresShell::HandleEventInternal() Line 5776	C++
 	PresShell::HandleEventWithTarget() Line 5682	C++
 	nsEventStateManager::CheckForAndDispatchClick() Line 3296	C++
 	nsEventStateManager::PostHandleEvent() Line 2377	C++
 	PresShell::HandleEventInternal() Line 5797	C++
 	PresShell::HandlePositionedEvent() Line 5665	C++
 	PresShell::HandleEvent() Line 5508	C++
 	nsViewManager::HandleEvent() Line 1296	C++
 	nsViewManager::DispatchEvent() Line 1252	C++
 	HandleEvent() Line 168	C++
 	nsWindow::DispatchEvent() Line 1051	C++
 	nsWindow::DispatchWindowEvent() Line 1071	C++
 	nsWindow::DispatchMouseEvent() Line 5941	C++
 	ChildWindow::DispatchMouseEvent() Line 6123	C++
 	nsWindow::ProcessMessage() Line 4403	C++
 	nsWindow::WindowProc() Line 1264	C++
 	7e418724	
 	7e418806	
 	7e41c92f	
 	7e4189bd	
 	7e4193f2	
 	7e418a00	
 	nsAppShell::ProcessNextNativeEvent() Line 148	C++
 	nsBaseAppShell::DoProcessNextNativeEvent() Line 137	C++
 	nsBaseAppShell::OnProcessNextEvent() Line 247	C++
 	nsThread::ProcessNextEvent() Line 477	C++
 	NS_ProcessNextEvent_P() Line 227	C++
 	nsBaseAppShell::Run() Line 154	C++
 	nsAppStartup::Run() Line 170	C++
 	XRE_main() Line 3142	C++
 	main() Line 153	C++
 	mainCRTStartup() Line 398	C
 	7c816fd7

Comment 1

[Boris Zbarsky [:bzbarsky]]

So there are a few things going on here.  I have patches for two of them, but the third is that InvalidateCanvasAsNeeded is called at unsafe times during frame construction, leading to this stack:

#20 0xb6743870 in PresShell::DoFlushPendingNotifications (this=0x8ba0198, 
    aType=Flush_Layout, aInterruptibleReflow=1)
    at ../../../mozilla/layout/base/nsPresShell.cpp:4447
#21 0xb6747bd4 in PresShell::WillPaint (this=0x8ba0198)
    at ../../../mozilla/layout/base/nsPresShell.cpp:5861
#22 0xb6c134e8 in nsViewManager::FlushPendingInvalidates (this=0x8b8d350)
    at ../../../mozilla/view/src/nsViewManager.cpp:2128
...
#25 0xb6c12c5d in nsViewManager::EndUpdateViewBatch (this=0x89b8d98, aUpdateFlags=0)
    at ../../../mozilla/view/src/nsViewManager.cpp:1870
#26 0xb66de74d in ApplyRenderingChangeToTree (aPresContext=0x89b8898, aFrame=0x8c01560, 
    aChange=nsChangeHint_RepaintFrame)
    at ../../../mozilla/layout/base/nsCSSFrameConstructor.cpp:9880
#27 0xb66de8da in InvalidateCanvasIfNeeded (aFrame=0x8e3b46c)
    at ../../../mozilla/layout/base/nsCSSFrameConstructor.cpp:9937

That is, the InvalidateCanvasIfNeeded call happens in the subframe, and the parent document has a pending style change to set the iframe to display:none.  The above stack processes this style change, after which the frame tree in the kid is dead.  The relevant code in nsCSSFrameConstructor::ContentRemoved is:

    InvalidateCanvasIfNeeded(childFrame);
    if (!aInReinsertContent &&
        MaybeRecreateContainerForIBSplitterFrame(childFrame, &rv)) {
      return rv;
    }

In this case, that first call kills off childFrame.  Note that the IsSafeToFlush() stuff doesn't help, because it's the _parent_ getting flushed.

roc, could we make this invalidate DEFERRED?  Or would that cause flicker?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I have no reason to believe it would cause flicker.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 4

[Boris Zbarsky [:bzbarsky]]

Fixed.