375299 - [FIX]Crash [@ nsFileControlFrame::CreateAnonymousContent] when removing stylesheet with binding and removing file input

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk builds within 200ms after load.
Talkback ID: TB30579018E
nsFileControlFrame::CreateAnonymousContent  [mozilla/layout/forms/nsfilecontrolframe.cpp, line 171]

This regressed between 2005-11-05 and 2005-11-08:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-11-05+05&maxdate=2005-11-08+09&cvsroot=%2Fcvsroot
I suspect a regression from bug 315306, somehow.
This doesn't happen on the branch.

This crash also happens when using an isindex, btw.

Comment 1

[Martijn Wargers (dead)]

(In reply to comment #0)
> I suspect a regression from bug 315306, somehow.

Er, I meant bug 314776 here

Comment 2

[Boris Zbarsky [:bzbarsky]]

This seems to be very similar to bug 375839, with a simpler testcase... Looking.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Binding for testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that crashes in older builds too

All that happened with bug 314776 is the ordering changed.  If I manually flip the ordering, then this crashes with builds from before bug 314776 as well.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch.

This fixes this bug and bug 375839.  In this case we do not have anything in the mContentListTable but we do have our single insertion point in mAnonymousNodesTable.

I did some CVS digging, and the reason this is needed is because of http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xbl/src/nsXBLBinding.cpp&rev=1.226&mark=353-359#352 -- that code means we could have insertion points in either hashtable.

It looks like this has NEVER worked.  The nsIDocumentObserver impl for nsBindingManager landed 5 days after the nsXBLBinding code linked to above, and was already buggy.  I suspect the two patches were just worked on independently... :(

Sicking, you think this is worth taking on branches?  So far I've only seen null-pointer derefs resulting from this, but at the same time this is a really simple patch.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed.  We need some tests here... :(

Comment 7

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070427 Minefield/3.0a4pre

Comment 8

[Reed Loden [:reed]]

Check-in of the branch patch in bug 382376 included this fix.

Comment 9

[Daniel Veditz [:dveditz]]

Verified in FF 2.0.0.8 on WinXP using "Testcase that crashes older builds too" (which did indeed crash me in 2.0.0.7).

Comment 10

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/235c56c14822

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/235c56c14822