154952 - removing children from and xbl element causes sigfault

Status: RESOLVED WONTFIX

(Core :: XUL, defect)

Description

[Justin Kirby]

JS code that causes this:

this.mRoster.removeChild(item);

where mRoster is an xbl element and item is a child of mRoster:
xbl:

<binding id="roster">
    <content>
      <xul:box orient="vertical" flex="1">
        <children includes="tree"/>
	<xul:box id="message-container">
	  <children includes="message"/>
	</xul:box>
	<xul:box id="rosteritem-container">
	  <children includes="rosteritem"/>
	</xul:box>
	<xul:box id="presence-container">
	  <children includes="presence"/>
	</xul:box>
	<xul:box id="session-definition-container">
	  <children includes="sessiondef"/>
	</xul:box>
      </xul:box>
    </content>
    <implementation>

#0  0x4274784c in nsTreeContentView::ContentInserted(nsIDocument*, nsIContent*,
nsIContent*, int) (this=0x8354350, aDocument=0x827d0f0, aContainer=0x82a17f8, 
    aChild=0x0, aIndexInContainer=5) at nsTreeContentView.cpp:945
#1  0x427477f9 in nsTreeContentView::ContentAppended(nsIDocument*, nsIContent*,
int) (this=0x8354350, aDocument=0x827d0f0, aContainer=0x82a17f8, 
    aNewIndexInContainer=5) at nsTreeContentView.cpp:931
#2  0x417d5bb0 in nsXULDocument::ContentAppended(nsIContent*, int) (
    this=0x827d0f0, aContainer=0x82a17f8, aNewIndexInContainer=5)
    at nsXULDocument.cpp:2256
#3  0x417b2d64 in nsXULElement::AppendChildTo(nsIContent*, int, int) (
    this=0x82a17f8, aKid=0x81f2548, aNotify=1, aDeepSetDocument=1)
    at nsXULElement.cpp:2333
#4  0x417ae5c7 in nsXULElement::InsertBefore(nsIDOMNode*, nsIDOMNode*,
nsIDOMNode**) (this=0x82a17f8, aNewChild=0x81f254c, aRefChild=0x0,
aReturn=0xbfffd070)
    at nsXULElement.cpp:1045
#5  0x417aeaee in nsXULElement::AppendChild(nsIDOMNode*, nsIDOMNode**) (
    this=0x82a17f8, aNewChild=0x81f254c, aReturn=0xbfffd070)
    at nsXULElement.cpp:1129
#6  0x40263f26 in XPTC_InvokeByIndex (that=0x82a17fc, methodIndex=18, 
    paramCount=2, params=0xbfffd060) at xptcinvoke_unixish_x86.cpp:88
#7  0x4097dd76 in XPCWrappedNative::CallMethod(XPCCallContext&,
XPCWrappedNative::CallMode) (ccx=@0xbfffd130, mode=CALL_METHOD) at
xpcwrappednative.cpp:1993
#8  0x4098613e in XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*,
long*) (cx=0x820c7c0, obj=0x42a19ca0, argc=1, argv=0x835b300, vp=0xbfffd2d0)
    at xpcwrappednativejsops.cpp:1266
#9  0x400cf3ea in js_Invoke (cx=0x820c7c0, argc=1, flags=0) at jsinterp.c:788
#10 0x400ddb50 in js_Interpret (cx=0x820c7c0, result=0xbfffd83c)
    at jsinterp.c:2743
#11 0x400cf472 in js_Invoke (cx=0x820c7c0, argc=1, flags=2) at jsinterp.c:805
#12 0x400cf806 in js_InternalInvoke (cx=0x820c7c0, obj=0x42a199c0, 
    fval=137249288, flags=0, argc=1, argv=0xbfffdc38, rval=0xbfffd9ec)
    at jsinterp.c:880
#13 0x4009e20e in JS_CallFunctionValue (cx=0x820c7c0, obj=0x42a199c0, 
    fval=137249288, argc=1, argv=0xbfffdc38, rval=0xbfffd9ec) at jsapi.c:3428
#14 0x412dd037 in nsJSContext::CallEventHandler(void*, void*, unsigned, void*,
int*, int) (this=0x821f6e8, aTarget=0x42a199c0, aHandler=0x82e4208, argc=1, 
    argv=0xbfffdc38, aBoolResult=0xbfffda98, aReverseReturnResult=0)
    at nsJSEnvironment.cpp:1041
#15 0x41321df8 in nsJSEventListener::HandleEvent(nsIDOMEvent*) (
    this=0x82a1958, aEvent=0x41aef188) at nsJSEventListener.cpp:181
#16 0x4164c691 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*,
nsIDOMEvent*, nsIDOMEventTarget*, unsigned, unsigned) (this=0x82a1920, 
    aListenerStruct=0x82a1888, aDOMEvent=0x41aef188, aCurrentTarget=0x82a18e8, 
    aSubType=8, aPhaseFlags=7) at nsEventListenerManager.cpp:1221
#17 0x4164fbeb in nsEventListenerManager::HandleEvent(nsIPresContext*, nsEvent*,
nsIDOMEvent**, nsIDOMEventTarget*, unsigned, nsEventStatus*) (this=0x82a1920, 
    aPresContext=0x81dda60, aEvent=0xbfffe640, aDOMEvent=0xbfffe46c, 
    aCurrentTarget=0x82a18e8, aFlags=7, aEventStatus=0xbfffe698)
    at nsEventListenerManager.cpp:2218
#18 0x417b78ca in nsXULElement::HandleDOMEvent(nsIPresContext*, nsEvent*,
nsIDOMEvent**, unsigned, nsEventStatus*) (this=0x82a18e0, aPresContext=0x81dda60, 
    aEvent=0xbfffe640, aDOMEvent=0xbfffe46c, aFlags=1, aEventStatus=0xbfffe698)
    at nsXULElement.cpp:3446
#19 0x425d7e60 in PresShell::HandleDOMEventWithTarget(nsIContent*, nsEvent*,
nsEventStatus*) (this=0x827c450, aTargetContent=0x82a18e0, aEvent=0xbfffe640, 
    aStatus=0xbfffe698) at nsPresShell.cpp:6237
#20 0x426b428c in nsButtonBoxFrame::MouseClicked(nsIPresContext*, nsGUIEvent*)
    (this=0x82c4118, aPresContext=0x81dda60, aEvent=0xbfffe8f0)
    at nsButtonBoxFrame.cpp:194
#21 0x426b3dba in nsButtonBoxFrame::HandleEvent(nsIPresContext*, nsGUIEvent*,
nsEventStatus*) (this=0x82c4118, aPresContext=0x81dda60, aEvent=0xbfffe8f0, 
    aEventStatus=0xbfffec94) at nsButtonBoxFrame.cpp:138
#22 0x425d7d00 in PresShell::HandleEventInternal(nsEvent*, nsIView*, unsigned,
nsEventStatus*) (this=0x827c450, aEvent=0xbfffe8f0, aView=0x0, aFlags=1, 
    aStatus=0xbfffec94) at nsPresShell.cpp:6205
#23 0x425d7a8c in PresShell::HandleEventWithTarget(nsEvent*, nsIFrame*,
nsIContent*, unsigned, nsEventStatus*) (this=0x827c450, aEvent=0xbfffe8f0, 
    aFrame=0x82c4118, aContent=0x82a18e0, aFlags=1, aStatus=0xbfffec94)
    at nsPresShell.cpp:6159
#24 0x4165b6b4 in nsEventStateManager::CheckForAndDispatchClick(nsIPresContext*,
nsMouseEvent*, nsEventStatus*) (this=0x82bde50, aPresContext=0x81dda60, 
    aEvent=0xbfffeec0, aStatus=0xbfffec94) at nsEventStateManager.cpp:2750
#25 0x416587b8 in nsEventStateManager::PostHandleEvent(nsIPresContext*,
nsEvent*, nsIFrame*, nsEventStatus*, nsIView*) (this=0x82bde50, 
    aPresContext=0x81dda60, aEvent=0xbfffeec0, aTargetFrame=0x82c4118, 
    aStatus=0xbfffec94, aView=0x828b5c8) at nsEventStateManager.cpp:1755
#26 0x425d7d52 in PresShell::HandleEventInternal(nsEvent*, nsIView*, unsigned,
nsEventStatus*) (this=0x827c450, aEvent=0xbfffeec0, aView=0x828b5c8, aFlags=1, 
    aStatus=0xbfffec94) at nsPresShell.cpp:6210
#27 0x425d7802 in PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*,
int, int&) (this=0x827c450, aView=0x828b5c8, aEvent=0xbfffeec0, 
    aEventStatus=0xbfffec94, aForceHandle=1, aHandled=@0xbfffec28)
    at nsPresShell.cpp:6113
#28 0x420a7acd in nsViewManager::HandleEvent(nsView*, nsGUIEvent*, int) (
    this=0x827ce60, aView=0x828b5c8, aEvent=0xbfffeec0, aCaptured=1)
    at nsViewManager.cpp:2083
#29 0x42099863 in nsView::HandleEvent(nsViewManager*, nsGUIEvent*, int) (
    this=0x828b5c8, aVM=0x827ce60, aEvent=0xbfffeec0, aCaptured=1)
    at nsView.cpp:305
#30 0x420a7281 in nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*) (
    this=0x827ce60, aEvent=0xbfffeec0, aStatus=0xbfffed90)
    at nsViewManager.cpp:1890
#31 0x42098b0d in HandleEvent(nsGUIEvent*) (aEvent=0xbfffeec0) at nsView.cpp:80
#32 0x40c0881a in nsWidget::DispatchEvent(nsGUIEvent*, nsEventStatus&) (
    this=0x8204560, aEvent=0xbfffeec0, aStatus=@0xbfffee4c)
    at nsWidget.cpp:1432
#33 0x40c0843d in nsWidget::DispatchWindowEvent(nsGUIEvent*) (this=0x8204560, 
    event=0xbfffeec0) at nsWidget.cpp:1320
#34 0x40c088da in nsWidget::DispatchMouseEvent(nsMouseEvent&) (this=0x8204560, 
    aEvent=@0xbfffeec0) at nsWidget.cpp:1459
#35 0x40c09697 in nsWidget::OnButtonReleaseSignal(_GdkEventButton*) (
    this=0x8204560, aGdkButtonEvent=0x81d1d20) at nsWidget.cpp:1932
#36 0x40c0eea5 in nsWindow::OnButtonReleaseSignal(_GdkEventButton*) (
    this=0x8204560, aGdkButtonEvent=0x81d1d20) at nsWindow.cpp:1621
#37 0x40c0f217 in nsWindow::HandleGDKEvent(_GdkEvent*) (this=0x8204560, 
    event=0x81d1d20) at nsWindow.cpp:1717
#38 0x40bff333 in dispatch_superwin_event(_GdkEvent*, nsWindow*) (
    event=0x81d1d20, window=0x8204560) at nsGtkEventHandler.cpp:958
#39 0x40bff023 in handle_gdk_event(_GdkEvent*, void*) (event=0x81d1d20, 
    data=0x0) at nsGtkEventHandler.cpp:833
#40 0x404a7d7f in gdk_event_dispatch () from /usr/lib/libgdk-1.2.so.0
#41 0x404db773 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#42 0x404dbd39 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#43 0x404dbeec in g_main_run () from /usr/lib/libglib-1.2.so.0
#44 0x403f6333 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#45 0x40bf4cb5 in nsAppShell::Run() (this=0x81507c8) at nsAppShell.cpp:332
#46 0x40b97e3e in nsAppShellService::Run() (this=0x81554d0)
    at nsAppShellService.cpp:457
#47 0x0805d62d in main1(int, char**, nsISupports*) (argc=3, argv=0xbffff3c4, 
    nativeApp=0x0) at nsAppRunner.cpp:1456
#48 0x0805e2aa in main (argc=3, argv=0xbffff3c4) at nsAppRunner.cpp:1805
#49 0x40622507 in __libc_start_main (main=0x805e0b0 <main>, argc=3, 
    ubp_av=0xbffff3c4, init=0x8056e38 <_init>, fini=0x8069e90 <_fini>, 
    rtld_fini=0x4000dc14 <_dl_fini>, stack_end=0xbffff3bc)
    at ../sysdeps/generic/libc-start.c:129

Comment 1

[Justin Kirby]

Attachment: does a null check on the aChild param. returns NS_OK if null.

All this patch does is check for null condition on aChild param. This is not
intended as a solution, just a short term hack so I can move on with things.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> returnthis: so this is interesting
> returnthis: nsXULDocument::ContentAppended
<returnthis> bz: just added a patch
> returnthis: does ChildAt() with that index
> returnthis: and calls AddSubtreeToDocument on the resulting child
> returnthis: this works, since AddSubtreeToDocument dereferences the arg without
+checking it...
> returnthis: so what gives later?

Comment 3

[Jan Varga [:janv]]

Yeah, I think we should fix caller.

Comment 4

[Boris Zbarsky [:bzbarsky]]

If this method expects a non-null arg, add an assert to that effect too....

Comment 5

[Jan Varga [:janv]]

Attachment: add an assertion to content view

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 115486 [details] [diff] [review]
add an assertion to content view

My kingdom for 3 more lines of context so I don't have to lxr!	;)

Could you make that NS_PRECONDITION just to humor me?

Comment 7

[Boris Zbarsky [:bzbarsky]]

.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So is there a testcase actually showing that bug that would let me debug this
for real?

Comment 9

[Justin]

The code and project that caused this bug is long gone, into the great bit
bucket. So, I do not have a testcase immediately available...

Comment 10

[Jan Varga [:janv]]

I don't think anyone is going to work on this given the plan to remove the XUL "tree" widget (bug 1446335).