372013 - Crash [@ nsHTMLFramesetBorderFrame::SetVisibility] with dynamic changes

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Mike Kaply [:mkaply]]

We are experiencing a crash in nsHTMLFramesetBorderFrame::SetVisibility. We're working on a testcase. Here's what we know so far.

There has been another report of it (same product we are working with)

http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=27967312

The problem is actually in 

nsHTMLFramesetFrame::Reflow here:

http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrameSetFrame.cpp#1231

mVerBorders[verX] is 0xfdfdfdfd so it has somehow been corrupted? Here's a full stack.



>	gklayout.dll!nsHTMLFramesetBorderFrame::SetVisibility(int aVisibility=0)  Line 1619 + 0x6 bytes	C++
 	gklayout.dll!nsHTMLFramesetFrame::Reflow(nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 1233	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsMargin & aComputedOffsets={...}, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0)  Line 371 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012da40)  Line 2877 + 0x3f bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012da40)  Line 2123 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1787 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 911 + 0xf bytes	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x062914b4, nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=0)  Line 754 + 0x21 bytes	C++
 	gklayout.dll!CanvasFrame::Reflow(nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 586	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x0628bdd0, nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=3, unsigned int & aStatus=0)  Line 754 + 0x21 bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::ReflowScrolledFrame(const ScrollReflowState & aState={...}, int aAssumeHScroll=0, int aAssumeVScroll=0, nsHTMLReflowMetrics * aMetrics=0x0012e2e0, int aFirstPass=1)  Line 463 + 0x2e bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::ReflowContents(ScrollReflowState * aState=0x0012e380, const nsHTMLReflowMetrics & aDesiredSize={...})  Line 533 + 0x1b bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::Reflow(nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 748 + 0x13 bytes	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x0628bf14, nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=0)  Line 754 + 0x21 bytes	C++
 	gklayout.dll!ViewportFrame::Reflow(nsPresContext * aPresContext=0x0624d0f8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 286 + 0x2b bytes	C++
 	gklayout.dll!PresShell::ProcessReflowCommands(int aInterruptible=1)  Line 5955	C++
 	gklayout.dll!PresShell::WillPaint()  Line 5640	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012eaec, nsEventStatus * aStatus=0x0012e998)  Line 1380	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012eaec)  Line 174	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012eaec, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1103 + 0xc bytes	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012eaec, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1129	C++
 	gkwidget.dll!nsWindow::OnPaint(HDC__ * aDC=0x00000000)  Line 5946 + 0x1e bytes	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=15, unsigned int wParam=0, long lParam=0, long * aRetValue=0x0012ef90)  Line 4435 + 0x15 bytes	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000b0ed2, unsigned int msg=15, unsigned int wParam=0, long lParam=0)  Line 1316 + 0x1d bytes	C++
 	user32.dll!77d48734() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]	
 	user32.dll!77d48816() 	
 	ntdll.dll!7c91056d() 	
 	user32.dll!77d4b4c0() 	
 	user32.dll!77d4b50c() 	
 	ntdll.dll!7c90eae3() 	
 	user32.dll!77d4d83f() 	
 	user32.dll!77d4d82a() 	
 	gkwidget.dll!nsWindow::DispatchStarvedPaints(HWND__ * aWnd=0x00110e24, long aMsg=0)  Line 4239 + 0xa bytes	C++
 	user32.dll!77d4ccd1() 	
 	user32.dll!77d4da57() 	
 	gkwidget.dll!nsWindow::DispatchPendingEvents()  Line 4276	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=512, unsigned int wParam=0, long lParam=22741460, long * aRetValue=0x0012f520)  Line 4668	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000b0ed2, unsigned int msg=512, unsigned int wParam=0, long lParam=22741460)  Line 1316 + 0x1d bytes	C++
 	user32.dll!77d48734() 	
 	user32.dll!77d48816() 	
 	user32.dll!77d489cd() 	
 	ntdll.dll!7c91056d() 	
 	user32.dll!77d49402() 	
 	user32.dll!77d48a10() 	
 	gkwidget.dll!nsAppShell::ProcessNextNativeEvent(int mayWait=0)  Line 149	C++
 	gkwidget.dll!nsBaseAppShell::DoProcessNextNativeEvent(int mayWait=0)  Line 136 + 0x11 bytes	C++
 	gkwidget.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00b5f218, int mayWait=1, unsigned int recursionDepth=0)  Line 209 + 0xd bytes	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012f70c)  Line 472	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00b5f218, int mayWait=1)  Line 225 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 153 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 171 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=1, char * * argv=0x00b5ba80, const nsXREAppData * aAppData=0x004036b4)  Line 2846 + 0x25 bytes	C++
 	firefox.exe!main(int argc=1, char * * argv=0x00b5ba80)  Line 61 + 0x13 bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	firefox.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!7c816fd7() 	
 	ntdll.dll!7c91056d()

Comment 1

[Mike Kaply [:mkaply]]

Narrowed regression window down to:

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-09-05+7%3A00&maxdate=2005-09-06+8%3A00&cvsroot=%2Fcvsroot

interesting checkin:

Make addition or removal of frames in a frameset actually change the
rendering. Bug 306660, r=jst, sr=roc

Comment 2

[Jesse Ruderman]

Does bug 369150 have the same regression window?

Comment 3

[Martijn Wargers (dead)]

(In reply to comment #2)
> Does bug 369150 have the same regression window?

I answered this question in bug 369150 (the answer was... yes)

Comment 4

[Mike Kaply [:mkaply]]

We actually found out how to work around this problem which might help with diagnosing the crash:

From the developer:

well essentially, if you have a frameset that doesn't have the cols/rows set (perhaps other attributes as well) and you append a child to it, when you actually try to set the cols/rows, it will crash. I found that as long as I set the cols/rows first, THEN append a child frame, it works. Small fix, big debugging effort 

Comment 5

[Daniel Veditz [:dveditz]]

In a windows debug build 0xfdfdfdfd is used to mark a "no man's land" buffer around allocated blocks --> something has run past its boundary, or maybe grabbed some other object's memory. (0xdddddddd is deleted memory, 0xcdcdcdcd is uninitialized allocated memory, and 0xcccccccc is uninitialized stack)

Testcase would be good. Is it a web page testcase, or reachable from an extension (chrome) only? The latter would lower the severity, but seems unlikely to be extension-only from the symptoms.

I assume this is happening on the 1.8 branch? What about FF1.5?

Comment 6

[Mike Kaply [:mkaply]]

This happens on FF 1.5 and FF 2.0 and FF 3.0.

The only way to debug is to connect to a server - we couldn't reduce the testcase.

I've sent bz info on connecting to that server, and I could give that info to anyone else on request.

Comment 7

[Window Snyder]

Critical security bugs need to have an owner.  If you are not the correct person for this bug, please help us find someone else.  Thanks.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Martijn, could you work with mkaply and try to get a usable testcase here?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Bug 369150 has some discussion on who should own this...

Comment 10

[chris hofmann]

over to bz per comment 9

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Did the fix for bug 369150 help with this?

Comment 12

[Mike Kaply [:mkaply]]

Still trying to get a testcase...

Comment 13

[Daniel Veditz [:dveditz]]

Please renominate when there's a testcase and we can answer whether the fix in bug 369150 solves the problem.

Comment 14

[Boris Zbarsky [:bzbarsky]]

To default owner.  Not likely to get time to work on it in the current state.

Comment 15

[Mike Kaply [:mkaply]]

I finally was able to test this on the latest trunk and it is fixed by the patches in the other bug.

Thanks!

Comment 16

[chris hofmann]

sounds like this is ready to mark fixed (on trunk testing in comment 15) and bug 369150 has fixed1.8.0.12, fixed1.8.1.4, keywords applied so doesn't sound like there is anything left to do on this bug, right?

Comment 17

[Jesse Ruderman]

Chris, this bug is already marked as fixed...

Comment 18

[Daniel Veditz [:dveditz]]

Probably fixed on the branch by bug 369150, minusing to get off our queries (since there's no way to test or to reproduce it would otherwise uselessly sit there forever).