501900 - Crash [@ LazyGeneratePopupDone] with openPopup() and DOMAttrModified event handler removing window

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

Attachment: zipped up testcase

See zipped up testcase. To reproduce, open the file named 'Kopie van parentframe.htm'. After opening Mozilla crashes within 400ms.
It also crashes in Firefox 3.

http://crash-stats.mozilla.com/report/index/fb9fbb8f-537a-4a37-b9e6-f557b2090701?p=1
0  	xul.dll  	LazyGeneratePopupDone  	 layout/xul/base/src/nsMenuPopupFrame.cpp:578
1 	xul.dll 	nsCSSFrameConstructor::LazyGenerateChildrenEvent::Run 	layout/base/nsCSSFrameConstructor.cpp:11773
2 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:527
3 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
4 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:193
5 	nspr4.dll 	PR_GetEnv 	
6 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
7 	firefox.exe 	firefox.exe@0x21a7 	
8 	kernel32.dll 	kernel32.dll@0x17076 

Firefox 3 crash report (garbage, it seems):
http://crash-stats.mozilla.com/report/index/04487184-afa9-4932-8327-18c352090701
0  	 	@0x39e318f

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

In this case we really want to use weak frame. re-getting the frame might cause
callback to use an nsIFrame for which a different runnable has been dispatched.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 3

[Samuel Sidler (old account; do not CC)]

For 1.9.1, we'll take this in 1.9.1.2.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/9cee3c24b53e

Comment 5

[Boris Zbarsky [:bzbarsky]]

So what, exactly, kills the frame here?

Comment 6

[Daniel Veditz [:dveditz]]

please request approval if this patch works for 1.9.1 and 1.9.0

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #5)
> So what, exactly, kills the frame here?
It goes something like observer getting onDOMAttrModified attr, and that eexutes the mutation listener, which removes the iframe and kills the layout objects of that document.
Martijn's testcases are 'interesting' :)

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 386475 [details] [diff] [review]
patch

Approved for 1.9.1.3 and 1.9.0.14, a=dveditz

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Checking in layout/base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1486; previous revision: 1.1485
done

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/049629a2fe9f

Comment 10

[Boris Zbarsky [:bzbarsky]]

So you mean in this case mCallback does something to destroy frames?

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.14 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.14pre) Gecko/2009081305 GranParadiso/3.0.14pre (.NET CLR 3.5.30729). It no longer crashes. I verified that crash with 1.9.0.13 as well.

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1.3 also with  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3pre) Gecko/20090817 Shiretoko/3.5.3pre (.NET CLR 3.5.30729).