35785 - Crash if <menulist> doesn't have any child nodes (such as <menupopup>) in XUL file

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Charles Manske]

I'm not sure who owns this code, but it looks like box code, so I'll try Eric.
I would like to be able to use <menulist id="foo"/> in XUL and have the
<menupopup> and <menuitems> added by JS at runtime. If I try that, we crash
in nsMenuFrame::GetPrefSize()in this code:
        nsIFrame* frame = mPopupFrames.FirstChild();
        if (!frame) {
          MarkAsGenerated();
          frame = mPopupFrames.FirstChild();
        }
frame is null, but MarkAsGenerated() doesn't correct for the completely missing
<menupopup>, I guess.
Checking "frame" for nsnull and returning NS_ERROR_FAILURE seems to be ok,
so I'll attach a diff for that proposed fix.

Stack:
<?xml version="1.0"?>
MenuFrame::GetPrefSize(nsMenuFrame * const 0x028abd20, nsBoxLayoutState & {...}, 
nsSize & {...}) line 1482 + 19 bytes
nsBoxSizeListNodeImpl::GetBoxSize(nsBoxLayoutState & {...}) line 258
nsBoxSizeListImpl::GetBoxSize(nsBoxLayoutState & {...}) line 220 + 18 bytes
nsObeliskLayout::GetPrefSize(nsObeliskLayout * const 0x040aa9e0, nsIBox * 
0x028ab4a0, nsBoxLayoutState & {...}, nsSize & {...}) line 102 + 18 bytes
nsContainerBox::GetPrefSize(nsContainerBox * const 0x028ab4a0, nsBoxLayoutState 
& {...}, nsSize & {...}) line 402 + 38 bytes
nsBoxFrame::GetPrefSize(nsBoxFrame * const 0x028ab4a0, nsBoxLayoutState & {...}, 
nsSize & {...}) line 721 + 23 bytes
nsSprocketLayout::GetPrefSize(nsSprocketLayout * const 0x040aafe0, nsIBox * 
0x028ab2e4, nsBoxLayoutState & {...}, nsSize & {...}) line 1058
nsContainerBox::GetPrefSize(nsContainerBox * const 0x028ab2e4, nsBoxLayoutState 
& {...}, nsSize & {...}) line 402 + 38 bytes
nsBoxFrame::GetPrefSize(nsBoxFrame * const 0x028ab2e4, nsBoxLayoutState & {...}, 
nsSize & {...}) line 721 + 23 bytes
nsStackLayout::GetPrefSize(nsStackLayout * const 0x040a9050, nsIBox * 
0x028ab250, nsBoxLayoutState & {...}, nsSize & {...}) line 86
nsContainerBox::GetPrefSize(nsContainerBox * const 0x028ab250, nsBoxLayoutState 
& {...}, nsSize & {...}) line 402 + 38 bytes
nsBoxFrame::GetPrefSize(nsBoxFrame * const 0x028ab250, nsBoxLayoutState & {...}, 
nsSize & {...}) line 721 + 23 bytes
nsSprocketLayout::PopulateBoxSizes(nsIBox * 0x028aafb0, nsBoxLayoutState & 
{...}, nsBoxSize * & 0x028d4990, nsComputedBoxSize * & 0x00000000, int & 0, int 
& 1073741824, int & 0) line 627
nsSprocketLayout::Layout(nsSprocketLayout * const 0x019849a0, nsIBox * 
0x028aafb0, nsBoxLayoutState & {...}) line 127
nsContainerBox::Layout(nsContainerBox * const 0x028aafb0, nsBoxLayoutState & 
{...}) line 489 + 34 bytes
nsBoxFrame::Layout(nsBoxFrame * const 0x028aafb0, nsBoxLayoutState & {...}) line 
822
nsStackLayout::Layout(nsStackLayout * const 0x019838b0, nsIBox * 0x028aaf1c, 
nsBoxLayoutState & {...}) line 235
nsContainerBox::Layout(nsContainerBox * const 0x028aaf1c, nsBoxLayoutState & 
{...}) line 489 + 34 bytes
nsBoxFrame::Layout(nsBoxFrame * const 0x028aaf1c, nsBoxLayoutState & {...}) line 
822
nsBoxFrame::Reflow(nsBoxFrame * const 0x028aaee4, nsIPresContext * 0x03f7b6d0, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 678
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x028aaee4, nsIPresContext * 
0x03f7b6d0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 206

Comment 1

[Charles Manske]

Attachment: Patch to check for null pointer

Comment 2

[Charles Manske]

Eric approved my fix
I'll check it in.

Comment 3

[Charles Manske]

checked in

Comment 4

[leger]

Adding crash keyword