309732 - Crash moving <treechildren> out of tree, then moving <treeitem> out of it [@ nsTreeContentView::InsertRowFor]

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050922
Firefox/1.6a1

Same signature as bug 307298, new testcase, crashes in a build that contains the
fix for bug 307298.

Comment 1

[Jesse Ruderman]

Attachment: fairly simple testcase (click the "Crash" button at the bottom)

Comment 2

[Jesse Ruderman]

0    nsTreeContentView::InsertRowFor(nsIContent*, nsIContent*) + 72
1    nsTreeContentView::ContentInserted(nsIDocument*, nsIContent*, nsIContent*,
int) + 888
2    nsDocument::ContentAppended(nsIContent*, int) + 116
3    nsGenericElement::InsertChildAt(nsIContent*, unsigned, int) + 504
4    nsGenericElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) + 1640
5    _XPTC_InvokeByIndex + 216
6    XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) +
2508
7    XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) + 220
8    js_Invoke + 1768
9    js_Interpret + 28824
...

Comment 3

[[e-mail]]

Attachment: Alternate test case

This crashes for the same reason, in this case mRoot is null which means that
the root element check fails completely.

Comment 4

[[e-mail]]

Attachment: Proposed patch

Don't listen to document notifications after our tree body's been removed.
When a tree body is created it calls SetTree to set the view up (again).

Comment 5

[Jan Varga [:janv]]

Hmm, it would be better to put that observer removal somewhere else.
Take a look at DocumentWillBeDestroyed(), it removes the observer and calls
ClearRows()
A new inline method would be nice, e.g. RemoveDocumentObserver()

Comment 6

[[e-mail]]

(In reply to comment #5)
>Hmm, it would be better to put that observer removal somewhere else. Take a
>look at DocumentWillBeDestroyed(), it removes the observer and calls ClearRows()
I'm just moving the code, not duplicating it... or have I misunderstood you?

Comment 7

[Jan Varga [:janv]]

Ah, sorry, you're right. r=varga

Comment 8

[[e-mail]]

Fix checked in.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using SeaMonkey 1.1a: Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.9a1) Gecko/20050926 Mozilla/1.0 with the two testcases here.

Comment 10

[Jesse Ruderman]

Crashtests checked in.