Martijn Wargers (dead) Reporter
	Description • 16 years ago

See testcase, which crashes current trunk build within 300ms.

http://crash-stats.mozilla.com/report/index/6a84f51e-203a-11dd-b77f-001a4bd46e84
0  	xul.dll  	nsGlobalWindow::EnsureReflowFlushAndPaint  	 mozilla/dom/src/base/nsGlobalWindow.cpp:3883
1 	xul.dll 	nsGlobalWindow::ShowModalDialog 	mozilla/dom/src/base/nsGlobalWindow.cpp:6071
2 	xul.dll 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
3 	xul.dll 	XPCWrappedNative::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:1984

This probably started crashing when bug 194404 was fixed (which implemented the method).

	Doug Turner (:dougt) Assignee
	Comment 2 • 16 years ago

#0  0x131d3d93 in nsGlobalWindow::EnsureReflowFlushAndPaint (this=0x17348cc0) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:3877
#1  0x131d7ca9 in nsGlobalWindow::ShowModalDialog (this=0x17348cc0, aURI=@0xbfffc2f0, aArgs=0x173601d0, aOptions=@0x173564a0, aRetVal=0xbfffc064) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:5941
#2  0x00477819 in NS_InvokeByIndex_P (that=0x17348cc0, methodIndex=88, paramCount=4, params=0xbfffc034) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
#3  0x111e921f in XPCWrappedNative::CallMethod (ccx=@0xbfffc280, mode=XPCWrappedNative::CALL_METHOD) at /Users/dougt/builds/mozilla/mozilla-central/js/src/xpconnect/src/xpcwrappednative.cpp:2405
#4  0x111f396d in XPC_WN_CallMethod (cx=0xeaba00, obj=0x1df9f1e0, argc=1, argv=0x1e380e34, vp=0xbfffc39c) at /Users/dougt/builds/mozilla/mozilla-central/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1477
#5  0x002479a7 in js_Invoke (cx=0xeaba00, argc=1, vp=0x1e380e2c, flags=2) at jsinterp.cpp:1306
#6  0x00223607 in js_Interpret (cx=0xeaba00) at /Users/dougt/builds/mozilla/mozilla-central/js/src/jsinterp.cpp:5015
#7  0x00247a38 in js_Invoke (cx=0xeaba00, argc=1, vp=0x1e380e20, flags=0) at jsinterp.cpp:1324
#8  0x00247cee in js_InternalInvoke (cx=0xeaba00, obj=0x1df1db00, fval=383877904, flags=0, argc=1, argv=0x173356c0, rval=0xbfffdc3c) at jsinterp.cpp:1381
#9  0x001c3d47 in JS_CallFunctionValue (cx=0xeaba00, obj=0x1df1db00, fval=383877904, argc=1, argv=0x173356c0, rval=0xbfffdc3c) at /Users/dougt/builds/mozilla/mozilla-central/js/src/jsapi.cpp:5235
#10 0x131b4350 in nsJSContext::CallEventHandler (this=0x1462a9b0, aTarget=0x17334220, aScope=0x1df1db00, aHandler=0x16e18310, aargv=0x1734bf44, arv=0xbfffdd84) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsJSEnvironment\
.cpp:1979
#11 0x131df4f0 in nsGlobalWindow::RunTimeout (this=0x17334220, aTimeout=0x1734bf60) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:7658
#12 0x131dfa3e in nsGlobalWindow::TimerCallback (aTimer=0x1734bfa0, aClosure=0x1734bf60) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:7990
#13 0x004630bc in nsTimerImpl::Fire (this=0x1734bfa0) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsTimerImpl.cpp:420
#14 0x004632f4 in nsTimerEvent::Run (this=0x1733b0f0) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsTimerImpl.cpp:512
#15 0x0045c74e in nsThread::ProcessNextEvent (this=0x717d00, mayWait=0, result=0xbfffdfe4) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsThread.cpp:510
#16 0x003e5e3c in NS_ProcessPendingEvents_P (thread=0x717d00, timeout=20) at nsThreadUtils.cpp:180
#17 0x1181606b in nsBaseAppShell::NativeEventCallback (this=0x7642f0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#18 0x117ce728 in nsAppShell::ProcessGeckoEvents (aInfo=0x7642f0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/cocoa/nsAppShell.mm:373
#19 0x955cd615 in CFRunLoopRunSpecific ()
#20 0x955cdcf8 in CFRunLoopRunInMode ()
#21 0x92221480 in RunCurrentEventLoopInMode ()
#22 0x92221299 in ReceiveNextEventCommon ()
#23 0x9222110d in BlockUntilNextEventMatchingListInMode ()
#24 0x927c63ed in _DPSNextEvent ()
#25 0x927c5ca0 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#26 0x927becdb in -[NSApplication run] ()
#27 0x117ccfea in nsAppShell::Run (this=0x7642f0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/cocoa/nsAppShell.mm:692
#28 0x124b6b46 in nsAppStartup::Run (this=0x77e000) at /Users/dougt/builds/mozilla/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:192
#29 0x000e3ce3 in XRE_main (argc=1, argv=0xbffff62c, aAppData=0x70f000) at /Users/dougt/builds/mozilla/mozilla-central/toolkit/xre/nsAppRunner.cpp:3263
#30 0x000027cb in main (argc=1, argv=0xbffff62c) at /Users/dougt/builds/mozilla/mozilla-central/browser/app/nsBrowserApp.cpp:156


mDocShell is null, we assert, then dereference it.


We set the mDocShell to null when we close the frame here:


#0  nsGlobalWindow::SetDocShell (this=0x1d18ab70, aDocShell=0x0) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:2004
#1  0x1285b4c1 in nsDocShell::Destroy (this=0x1cad2fb0) at /Users/dougt/builds/mozilla/mozilla-central/docshell/base/nsDocShell.cpp:3765
#2  0x12f3d8a0 in nsFrameLoader::Finalize (this=0x1d162310) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsFrameLoader.cpp:287
#3  0x12f2455c in nsDocument::InitializeFinalizeFrameLoaders (this=0xfbc800) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsDocument.cpp:5135
#4  0x12f24683 in nsDocument::EndUpdate (this=0xfbc800, aUpdateType=1) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsDocument.cpp:3650
#5  0x130a2eff in nsHTMLDocument::EndUpdate (this=0xfbc800, aUpdateType=1) at /Users/dougt/builds/mozilla/mozilla-central/content/html/document/src/nsHTMLDocument.cpp:3095
#6  0x12cf0ae1 in mozAutoDocUpdate::~mozAutoDocUpdate (this=0xbfffc214) at ../../content/base/src/mozAutoDocUpdate.h:66
#7  0x12f55157 in nsGenericElement::doRemoveChildAt (aIndex=1, aNotify=1, aKid=0x1a7e2930, aParent=0x1cabeaf0, aDocument=0xfbc800, aChildArray=@0x1cabeb08) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsGenericElemen\
t.cpp:3368
#8  0x12f55257 in nsGenericElement::RemoveChildAt (this=0x1cabeaf0, aIndex=1, aNotify=1) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsGenericElement.cpp:3293
#9  0x12f4e245 in nsGenericElement::doRemoveChild (aOldChild=0x1a7e2958, aParent=0x1cabeaf0, aDocument=0xfbc800, aReturn=0xbfffc3fc) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsGenericElement.cpp:3955
#10 0x12f4e2ae in nsGenericElement::RemoveChild (this=0x1cabeaf0, aOldChild=0x1a7e2958, aReturn=0xbfffc3fc) at /Users/dougt/builds/mozilla/mozilla-central/content/base/src/nsGenericElement.cpp:3521
#11 0x13021efd in nsHTMLBodyElement::RemoveChild (this=0x1cabeaf0, oldChild=0x1a7e2958, _retval=0xbfffc3fc) at /Users/dougt/builds/mozilla/mozilla-central/content/html/content/src/nsHTMLBodyElement.cpp:95
#12 0x11225e1e in nsIDOMNode_RemoveChild (cx=0xd7ca00, argc=1, vp=0xc98c2c) at dom_quickstubs.cpp:2757
#13 0x002234f1 in js_Interpret (cx=0xd7ca00) at /Users/dougt/builds/mozilla/mozilla-central/js/src/jsinterp.cpp:4998
#14 0x00247a38 in js_Invoke (cx=0xd7ca00, argc=1, vp=0xc98c20, flags=0) at jsinterp.cpp:1324
#15 0x00247cee in js_InternalInvoke (cx=0xd7ca00, obj=0x1a6d7140, fval=382191808, flags=0, argc=1, argv=0x1d169240, rval=0xbfffdc3c) at jsinterp.cpp:1381
#16 0x001c3d47 in JS_CallFunctionValue (cx=0xd7ca00, obj=0x1a6d7140, fval=382191808, argc=1, argv=0x1d169240, rval=0xbfffdc3c) at /Users/dougt/builds/mozilla/mozilla-central/js/src/jsapi.cpp:5235
#17 0x131b4350 in nsJSContext::CallEventHandler (this=0x14647a20, aTarget=0x1f35fa00, aScope=0x1a6d7140, aHandler=0x16c7c8c0, aargv=0x1d172384, arv=0xbfffdd84) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsJSEnvironment\
.cpp:1979
#18 0x131df4f0 in nsGlobalWindow::RunTimeout (this=0x1f35fa00, aTimeout=0x1d194fe0) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:7658
#19 0x131dfa3e in nsGlobalWindow::TimerCallback (aTimer=0x1f3b4ff0, aClosure=0x1d194fe0) at /Users/dougt/builds/mozilla/mozilla-central/dom/src/base/nsGlobalWindow.cpp:7990
#20 0x004630bc in nsTimerImpl::Fire (this=0x1f3b4ff0) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsTimerImpl.cpp:420
#21 0x004632f4 in nsTimerEvent::Run (this=0x16a2af80) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsTimerImpl.cpp:512
#22 0x0045c74e in nsThread::ProcessNextEvent (this=0x7178f0, mayWait=0, result=0xbfffdfe4) at /Users/dougt/builds/mozilla/mozilla-central/xpcom/threads/nsThread.cpp:510
#23 0x003e5e3c in NS_ProcessPendingEvents_P (thread=0x7178f0, timeout=20) at nsThreadUtils.cpp:180
#24 0x1181606b in nsBaseAppShell::NativeEventCallback (this=0x763ea0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#25 0x117ce728 in nsAppShell::ProcessGeckoEvents (aInfo=0x763ea0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/cocoa/nsAppShell.mm:373
#26 0x955cd615 in CFRunLoopRunSpecific ()
#27 0x955cdcf8 in CFRunLoopRunInMode ()
#28 0x92221480 in RunCurrentEventLoopInMode ()
#29 0x922211d2 in ReceiveNextEventCommon ()
#30 0x9222110d in BlockUntilNextEventMatchingListInMode ()
#31 0x927c63ed in _DPSNextEvent ()
#32 0x927c5ca0 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#33 0x927becdb in -[NSApplication run] ()
#34 0x117ccfea in nsAppShell::Run (this=0x763ea0) at /Users/dougt/builds/mozilla/mozilla-central/widget/src/cocoa/nsAppShell.mm:692
#35 0x124b6b46 in nsAppStartup::Run (this=0x77dbb0) at /Users/dougt/builds/mozilla/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:192
#36 0x000e3ce3 in XRE_main (argc=1, argv=0xbffff62c, aAppData=0x70eb60) at /Users/dougt/builds/mozilla/mozilla-central/toolkit/xre/nsAppRunner.cpp:3263
#37 0x000027cb in main (argc=1, argv=0xbffff62c) at /Users/dougt/builds/mozilla/mozilla-central/browser/app/nsBrowserApp.cpp:156




In nsGlobaWindow, we check for a null mDocShell all over the place, but we don't in EnsureReflowFlushAndPaint.  We do have a NS_ASSERTION, but that doesn't return when we see the null.  

There are a bunch of other places where we use NS_ASSERTION() to test for null, but then do not do anything about it.  I am pretty sure that this issue is tracked elsewhere, but it would probably be a good idea to double check these cases to see if there is a better way to fail.

	Johnny Stenback (:jst)
	Comment 4 • 16 years ago

I think we should also add a check and fail to open the modal dialog if we have no docshell, as we do in Alert() and friends.

	Doug Turner (:dougt) Assignee
	Comment 5 • 16 years ago

we might not need the patch to EnsureReflowFlushAndPaint(), but i left it there -- belts and suspenders, or something.

	Doug Turner (:dougt) Assignee
	Comment 6 • 16 years ago

Comment on attachment 346762 [details] [diff] [review]
patch v.2

do we want to also make a test case for this?

	Johnny Stenback (:jst)
	Comment 7 • 16 years ago

Comment on attachment 346762 [details] [diff] [review]
patch v.2

r+sr=jst. A test would be good, w=window.open(); w.close(); w.showModalDialog(...) or something. Probably need a timeout between close() and showModalDialog(), and the dialog would need to close itself of course if it actually opened (and flag that as an error I guess).

	Johnny Stenback (:jst)
	Comment 8 • 16 years ago

Duping.

	Doug Turner (:dougt) Assignee
	Comment 9 • 16 years ago

removing request flags since this is a dup.