586713 - crash: setTimeout for appending processing instruction [@ nsMenuGroupOwnerX::ContentInserted]

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Danil Ivanov]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre
Build Identifier: 

JavaScript in the extension overlay (overlay browser.xul):

function appendPI() {
  let instruction = document.createProcessingInstruction("xml-stylesheet",
                    'href="chrome://picrashtest/content/css.css" type="text/css"');
  document.insertBefore(instruction, document.documentElement);
}

appendPI();//working
setTimeout(function() appendPI(), 5000);//crash

Reproducible: Always




Talkback crash ID: e6057076-27ae-40a3-989d-a116a2100812

Comment 1

[Danil Ivanov]

Attachment: Sample extension

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, nsMenuGroupOwnerX::ContentInserted is just buggy.  It assumes a non-null aContainer, which is a bogus assumption.  ccing the person who seems to have the blame for this code...

Comment 3

[Tom Dyas]

Bug 576593 was a similar crash, but in the nsMenuGroupOwnerX::ContentRemoved method.

Comment 4

[timeless]

Signature	nsMenuGroupOwnerX::ContentInserted
UUID	e6057076-27ae-40a3-989d-a116a2100812
Time	2010-08-12 10:39:27.657506
Uptime	6
Last Crash	22 seconds before submission
Install Age	311 seconds (5.2 minutes) since version was first installed.
Product	Firefox
Version	4.0b4pre
Build ID	20100812030710
Branch	1.9.3
OS	Mac OS X
OS Version	10.6.4 10F569
CPU	x86
CPU Info	GenuineIntel family 6 model 23 stepping 6
Crash Reason	EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash Address	0xc
User Comments	setTimeout for append processing instruction in browser overlay -> crash
Processor Notes	
EMCheckCompatibility	False
Crashing Thread

Frame	Module	Signature [Expand]	Source
0	XUL	nsMenuGroupOwnerX::ContentInserted	 nsINode.h:712
1	XUL	nsNodeUtils::ContentInserted	content/base/src/nsNodeUtils.cpp:163
2	XUL	nsINode::doInsertChildAt	content/base/src/nsGenericElement.cpp:3608
3	XUL	nsDocument::InsertChildAt	content/base/src/nsDocument.cpp:3289
4	XUL	nsINode::ReplaceOrInsertBefore	content/base/src/nsGenericElement.cpp:4294
5	XUL	nsIDOMNode_InsertBefore	 nsINode.h:1217
6	libmozjs.dylib	js::Interpret	js/src/jsinterp.cpp:4713
7	libmozjs.dylib	js::InvokeCommon<JSBool >	js/src/jsinterp.cpp:588
8	libmozjs.dylib	js::Invoke	js/src/jsinterp.cpp:714
9	libmozjs.dylib	js::InternalInvoke	js/src/jsinterp.cpp:754
10	libmozjs.dylib	JS_CallFunctionValue	js/src/jsinterp.h:341
11	XUL	nsJSContext::CallEventHandler	dom/base/nsJSEnvironment.cpp:2248
12	XUL	nsGlobalWindow::RunTimeout	dom/base/nsGlobalWindow.cpp:8527
13	XUL	nsGlobalWindow::TimerCallback	dom/base/nsGlobalWindow.cpp:8872
14	XUL	nsTimerImpl::Fire	xpcom/threads/nsTimerImpl.cpp:425
15	XUL	nsTimerEvent::Run	xpcom/threads/nsTimerImpl.cpp:517
16	XUL	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:547
17	XUL	NS_ProcessPendingEvents_P	 nsThreadUtils.cpp:200
18	XUL	nsBaseAppShell::NativeEventCallback	widget/src/xpwidgets/nsBaseAppShell.cpp:126
19	XUL	nsAppShell::ProcessGeckoEvents	widget/src/cocoa/nsAppShell.mm:394
20	CoreFoundation	__CFRunLoopDoSources0	
21	CoreFoundation	__CFRunLoopRun	
22	CoreFoundation	CFRunLoopRunSpecific	
23	CoreFoundation	CFRunLoopRunInMode	
24	HIToolbox	RunCurrentEventLoopInMode	
25	HIToolbox	ReceiveNextEventCommon	
26	HIToolbox	BlockUntilNextEventMatchingListInMode	
27	AppKit	_DPSNextEvent	
28	AppKit	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]	
29	AppKit	-[NSApplication run]	
30	XUL	nsAppShell::Run	widget/src/cocoa/nsAppShell.mm:747
31	XUL	nsAppStartup::Run	toolkit/components/startup/src/nsAppStartup.cpp:191
32	XUL	XRE_main	toolkit/xre/nsAppRunner.cpp:3659
33	firefox-bin	main	browser/app/nsBrowserApp.cpp:158
34	firefox-bin	firefox-bin@0xbf5	
35		@0x3

Comment 5

[Josh Aas]

Attachment: fix v1.0

I don't think we need to do anything in those methods if aContainer is null. We're not going to find an observer in the observer table for null either.

Comment 6

[Josh Aas]

Attachment: fix v1.0 w/test

Includes a crash test mochitest, because we need a top-level XUL window for the test its much easier to do this as a mochitest.

Comment 7

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/1887d4c88687