576593 - Crash in [@ nsMenuGroupOwnerX::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, int) ] [@ nsMenuGroupOwnerX::ContentRemoved ] when using Tree Style Tabs Extension

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Marcia Knous [:marcia]]

Filing this for Crash Stats tracking purposes since this is showing up as the top crash on the Mac for beta 1.

Using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:2.0b1) Gecko/20100630 Firefox/4.0b1.

STR:
1. Install the Addon Comp Extension
2. Install Tree Style Tabs extension
3. Close the browser using the "red" button the browser window
4. Crash 100% in this stack

http://crash-stats.mozilla.com/report/index/871468d4-a439-478f-99fa-a5c132100702

Comment 1

[Marcia Knous [:marcia]]

Frame  	Module  Signature [Expand]  	Source
0 	XUL 	nsMenuGroupOwnerX::ContentRemoved 	nsINode.h:710
1 	XUL 	nsNodeUtils::ContentRemoved 	content/base/src/nsNodeUtils.cpp:188
2 	XUL 	nsINode::doRemoveChildAt 	content/base/src/nsGenericElement.cpp:3676
3 	XUL 	nsDocument::RemoveChildAt 	content/base/src/nsDocument.cpp:3218
4 	XUL 	nsIDOMNode_RemoveChild 	nsINode.h:482
5 	libmozjs.dylib 	js_Interpret 	js/src/jsops.cpp:2148
6 	libmozjs.dylib 	js_Invoke 	js/src/jsinterp.cpp:664
7 	XUL 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1689
8 	XUL 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:570
9 	XUL 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_unixish_x86.cpp:93
10 	XUL 	nsXPTCStubBase::Stub3 	
11 	XUL 	nsEventListenerManager::HandleEventSubType 	content/events/src/nsEventListenerManager.cpp:1094
12 	XUL 	nsEventListenerManager::HandleEventInternal 	content/events/src/nsEventListenerManager.cpp:1190
13 	XUL 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventListenerManager.h:146
14 	XUL 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:628
15 	XUL 	DocumentViewerImpl::PageHide 	layout/base/nsDocumentViewer.cpp:1306
16 	XUL 	nsDocShell::FirePageHideNotification 	docshell/base/nsDocShell.cpp:1473
17 	XUL 	nsDocShell::Destroy 	docshell/base/nsDocShell.cpp:4304
18 	XUL 	nsXULWindow::Destroy 	xpfe/appshell/src/nsXULWindow.cpp:515
19 	XUL 	nsWebShellWindow::Destroy 	xpfe/appshell/src/nsWebShellWindow.cpp:788
20 	XUL 	nsWebShellWindow::HandleEvent 	xpfe/appshell/src/nsWebShellWindow.cpp:399
21 	XUL 	nsCocoaWindow::DispatchEvent 	widget/src/cocoa/nsCocoaWindow.mm:1320
22 	XUL 	-[WindowDelegate windowShouldClose:] 	widget/src/cocoa/nsCocoaWindow.mm:1790
23 	AppKit 	-[NSWindow _document:shouldClose:contextInfo:] 	
24 	AppKit 	-[NSWindow __close] 	
25 	AppKit 	-[NSWindow _close:] 	
26 	AppKit 	-[NSApplication sendAction:to:from:] 	
27 	AppKit 	-[NSControl sendAction:to:] 	
28 	AppKit 	-[NSCell _sendActionFrom:] 	
29 	AppKit 	-[NSCell trackMouse:inRect:ofView:untilMouseUp:] 	
30 	AppKit 	-[NSButtonCell trackMouse:inRect:ofView:untilMouseUp:] 	
31 	AppKit 	-[NSControl mouseDown:] 	
32 	AppKit 	-[_NSThemeWidget mouseDown:] 	
33 	AppKit 	-[NSWindow sendEvent:] 	
34 	XUL 	-[ToolbarWindow sendEvent:] 	widget/src/cocoa/nsCocoaWindow.mm:2233
35 	AppKit 	-[NSApplication sendEvent:] 	
36 	AppKit 	-[NSApplication run] 	
37 	XUL 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:747
38 	XUL 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:192
39 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3624
40 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
41 	firefox-bin 	firefox-bin@0xbf5 	
42 		@0x3

Comment 2

[Steven Michaud [:smichaud] (Retired)]

I haven't yet tried to reproduce this, but it's quite apparent that it
has other causes than a bad interaction with the Addon Comp extension:
What correlation data I've been able to find doesn't even list this
extension, and shows no correlation stronger than about 90% for any
extension (and no significant correlation with any plugin):

http://people.mozilla.com/crash_analysis/20100711/20100711_Firefox_4.0b1-interesting-addons.txt
http://people.mozilla.com/crash_analysis/20100710/20100710_Firefox_4.0b1-interesting-addons.txt

http://crash-stats.mozilla.com/ seems to show these crashes starting
with the 20100628201214 trunk build, implying that the "guilty" patch
is in approximately this range:

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-06-27+03%3A00%3A00&enddate=2010-06-28+03%3A00%3A00

I can't guess which patch in this range might have triggered the
problem.  So someone will (presumably) need to tackle the problem with
hg bisect.

Comment 3

[Marcia Knous [:marcia]]

Adding [@ nsMenuGroupOwnerX::ContentRemoved ] to the title since I get a crash with that stack with Tree Style tabs installed. I crashed when I was playing around with app tabs.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I didn't get the regression range right in comment #2.  Here's the
true regression range (from testing nightlies):

firefox-2009-08-13-03-mozilla-central
firefox-2009-08-14-03-mozilla-central

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-08-13+03%3A00%3A00&enddate=2009-08-14+03%3A00%3A00

There are lots of Cocoa-widgets-specific patches in this range, so
I'll need to use hg bisect to find the "guilty" patch.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

I don't need the "Addon Comp" extension to reproduce this bug's crash
-- the Tree Style Tabs extension is enough by itself.

So here's revised/simplfied STR:

1) Make sure the Tree Style Tabs extension is installed.

2) Open a browser window (if there isn't one already open).

3) Close the window using the red "close" button -- and crash.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

hg bisect tells me:

The first bad revision is:
changeset:   31435:6fa9e4164629
user:        Ben Hearsum <[e-mail]>
date:        Thu Aug 13 10:06:28 2009 -0400
summary:     bug 508282: bump mozilla-central version
               to 1.9.3a1pre/3.7a1pre. r=catlee. CLOSED TREE

Which on the face of it seems absurd.  But I've confirmed it "by hand".

I think the Tree Style Tab extension must change its behavior according to the browser version number.  If so, the true regression date is older.

I'll try to find a way to work around this (and find the true regression date).

Comment 7

[Steven Michaud [:smichaud] (Retired)]

I'm going to be away for the rest of this week and part of next ... so I won't be able to continue working on this bug until I get back.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Attachment: Gdb stack trace of crash (on OS X 10.5.8)

I've given up trying to find this bug's true regression range.

But here's something interesting -- this stack trace shows that
aContainer is NULL when nsMenuGroupOwnerX::ContentRemoved() is called.
So it's not surprising that we crash there (though it's still puzzling
that the crash address is 0xc and not 0).  Furthermore
nsNodeUtils::ContentRemoved() (which calls
nsMenuGroupOwnerX::ContentRemoved()) routinely sets aContainer to
NULL, without any indication that this is an error condition.

So the fix for this bug might be very simple, and something along
these lines:

Change this (in nsMenuGroupOwnerX::ContentRemoved()):

  else if (aContainer != mContent) {

to this:

  else if (aContainer && (aContainer != mContent)) {

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Attachment: Possible fix

This patch stops the crashes, and doesn't appear to have any ill effects.

Comment 10

[Josh Aas]

This is a safe null pointer fix that I'd like to see on 1.9.2 as well.

Comment 11

[Daniel Veditz [:dveditz]]

Not "blocking" 1.9.2 but we'll look at patch approval

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 457238 [details] [diff] [review]
Possible fix

Approved for 1.9.2.8, a=dveditz for release-drivers

Please land it on the default, not the relbranch (which tip sometimes points to).

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Josh, you checking this in tonight?

Comment 14

[Josh Aas]

Comment on attachment 457238 [details] [diff] [review]
Possible fix

Clearing 1.9.2.8 approval, my bad - I thought this code existed on the 1.9.2 branch but it does not.

Comment 15

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/7f6916d28430