394800 - "ASSERTION: Some objects allocated with AllocateFrame were not freed" with xul:menulist, xul:tooltip

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Jesse Ruderman]

###!!! ASSERTION: Shallow unbind won't clear document and binding parent on kids!: 'aDeep || (!GetCurrentDoc() && !GetBindingParent())', file /Users/jruderman/trunk/mozilla/content/base/src/nsGenericElement.cpp, line 2090

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 673

This bug appears to be exploitable via the usual method for exploiting "ASSERTION: Some objects allocated with AllocateFrame were not freed" bugs.

Comment 1

[Jesse Ruderman]

Attachment: testcase (triggers the assertion but not the crash)

Comment 2

[Jesse Ruderman]

You have to reload to see the second assertion, as usual.

Comment 3

[Mats Palmgren (inactive)]

We're calling nsMenuFrame::InsertFrames with aPrevFrame == the popup frame
on its ::popupList although we insert the new frame into the principal list.
This is same problem as we had with nsFieldSetFrame when aPrevFrame was
the legend frame in bug 345249.  Patch coming up.

Comment 4

[Mats Palmgren (inactive)]

Attachment: Mochitest (.tar.gz)

Comment 5

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Fixes the second assertion (and the crash I hope -- Jesse, confirm?).
The first assertion still occurs but I think it's unrelated and can
be handled in a separate bug.

Comment 6

[Jesse Ruderman]

make[5]: *** No rule to make target `/Users/jruderman/trunk/mozilla/layout/xul/test/Makefile.in', needed by `test/Makefile'.  Stop.

Did you forget to include your new tests in the patch?

Comment 7

[Jesse Ruderman]

The patch does fix the crash :)

Comment 8

[Mats Palmgren (inactive)]

(In reply to comment #6)
> Did you forget to include your new tests in the patch?

It's in the attached .tar.gz file.  I don't think it's possible to make
a cvs diff with files in new directories (layout/xul/test), IIRC "cvs add"
of a directory is immediate and I didn't want to affect the repository
without approval.

(In reply to comment #7)
> The patch does fix the crash :)

Thanks!

Comment 9

[[e-mail]]

(In reply to comment #8)
>I don't think it's possible to make a cvs diff with files in new directories
There are third-party utilities that allow this (e.g. cvsdo).

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 279581 [details] [diff] [review]
Patch rev. 1

sr=bzbarsky

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 279581 [details] [diff] [review]
Patch rev. 1

a1.9=dbaron

Comment 12

[Mats Palmgren (inactive)]

mozilla/layout/xul/test/Makefile.in 	1.2
mozilla/layout/xul/test/test_bug394800.xhtml 	1.1
mozilla/layout/xul/base/src/nsBoxFrame.cpp 	1.341
mozilla/layout/xul/base/src/nsMenuFrame.cpp 	1.362 

-> FIXED

Comment 13

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 debug no AllocateFrame assertion with attachment 279517 [details] on linux/macppc/windows. Also no assertion on linux/windows with the mochikit test (mac wouldn't run mochikit...)

Comment 14

[Jesse Ruderman]

This bug doesn't seem to affect branch.  I ported the patch for bug 334514 to branch just to make sure ;)