151826 - crash when using splitter within scrollbox [@ nsViewManager::UpdateView]

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Richard Fine]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.0; Mac_PowerPC)
BuildID:    

In an XUL page (running outside of chrome), creating a <scrollbox> or one 
of its derivatives (e.g. arrowbox), and then putting a <splitter> inside 
causes a crash. It only happens when there is other content both before 
and after the splitter.

Reproducible: Always
Steps to Reproduce:
1. Create an XUL page using the chrome://global/skin stylesheet.
2. Add a window element, containing <vbox flex="1"><scrollbox flex="1">
3. Add something like <hbox><description>test</description></hbox><
splitter/><hbox><description>test2</description></hbox> to the scrollbox

Actual Results:  Invalid page fault in GKVIEW.DLL. Total browser crash. 
Talkback launches OK.

Expected Results:  There should have been a simple scrollable box, 
containing 'test', then a splitter, then 'test2.'

Using the Modern theme. Talkback results and testcase to be posted 
soon.

Comment 1

[Richard Fine]

Attachment: testcase

Comment 2

[Richard Fine]

Attachment: Talkback crash info

Comment 3

[Richard Fine]

Beware that clicking the 'testcase' link when in Mozilla *will* crash the browser.

Build ID is: 2002053012 (omitted from original report)

Comment 4

[Andrew Schultz]

were you testing Mozilla 1.0 (release) or a trunk build?

we need the talkback ID for the crash, rather than the data it gave you.  Run
the Quality Feedback Agent to get the ID.

Comment 5

[John Morrison]

|view| is (according to an optimized build) null in nsViewManager::UpdateView() 
which is a little surprising since there is a check for null just before that
in the code.

nsViewManager::UpdateView(nsViewManager * const 0x02100158, nsIView * 
0x00000000, const nsRect & {...}, unsigned int 0x00000004) line 1572 + 8 bytes
nsViewManager::MoveViewTo(nsViewManager * const 0x02100158, nsIView * 
0x00000000, int 0x00000000, int 0x00000001) line 2354
nsContainerFrame::PositionFrameView(nsIPresContext * 0x020ff0f8, nsIFrame * 
0x00000000) line 473
nsBox::SetBounds(nsBox * const 0x02afe19c, nsBoxLayoutState & {...}, const 
nsRect & {...}) line 603
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01a0eef8, nsIBox * 
0x02afd4ec, nsBoxLayoutState & {...}) line 493
nsContainerBox::DoLayout(nsContainerBox * const 0x02afd4ec, nsBoxLayoutState & 
{...}) line 605 + 8 bytes
nsBox::Layout(nsBox * const 0x02afd4ec, nsBoxLayoutState & {...}) line 1062
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01a0eef8, nsIBox * 
0x02aa5328, nsBoxLayoutState & {...}) line 529
nsContainerBox::DoLayout(nsContainerBox * const 0x02aa5328, nsBoxLayoutState & 
{...}) line 605 + 8 bytes
nsBox::Layout(nsBox * const 0x02aa5328, nsBoxLayoutState & {...}) line 1062
nsScrollBoxFrame::DoLayout(nsScrollBoxFrame * const 0x00000001, 
nsBoxLayoutState & {...}) line 394
nsBox::Layout(nsBox * const 0x02aa5194, nsBoxLayoutState & {...}) line 1062
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01a0eef8, nsIBox * 
0x02aa4fec, nsBoxLayoutState & {...}) line 529
nsContainerBox::DoLayout(nsContainerBox * const 0x02aa4fec, nsBoxLayoutState & 
{...}) line 605 + 8 bytes
nsBox::Layout(nsBox * const 0x02aa4fec, nsBoxLayoutState & {...}) line 1062
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01a0eef8, nsIBox * 
0x02aa47b4, nsBoxLayoutState & {...}) line 529
nsContainerBox::DoLayout(nsContainerBox * const 0x02aa47b4, nsBoxLayoutState & 
{...}) line 605 + 8 bytes
nsBox::Layout(nsBox * const 0x02aa47b4, nsBoxLayoutState & {...}) line 1062
nsStackLayout::Layout(nsStackLayout * const 0x0195e1e8, nsIBox * 0x02aa4514, 
nsBoxLayoutState & {...}) line 331
nsContainerBox::DoLayout(nsContainerBox * const 0x02aa4514, nsBoxLayoutState & 
{...}) line 605 + 8 bytes
nsBox::Layout(nsBox * const 0x02aa4514, nsBoxLayoutState & {...}) line 1062
nsBoxFrame::Reflow(nsBoxFrame * const 0x02aa44e0, nsIPresContext * 0x020ff0f8, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 
0x00000000) line 1000
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x02aa44e0, nsIPresContext * 
0x020ff0f8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0x00000000) line 242
nsContainerFrame::ReflowChild(nsContainerFrame * const 0x0012efe0, nsIFrame * 
0x02aa44e0, nsIPresContext * 0x020ff0f8, nsHTMLReflowMetrics & {...}, const 
nsHTMLReflowState & {...}, int 0x00000000, int 0x00000000, unsigned int 
0x00000000, unsigned int & 0x00000000) line 801
ViewportFrame::Reflow(ViewportFrame * const 0x02aa44a8, nsIPresContext * 
0x020ff0f8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0x00000000) line 577
PresShell::InitialReflow(PresShell * const 0x02aa44a8, int 0x021c0310, int 
0x00002454) line 2850
nsXULDocument::StartLayout(nsXULDocument * const 0x0012efe0) line 4568
nsXULDocument::ResumeWalk(nsXULDocument * const 0x0012efe0) line 5696
nsXULDocument::EndLoad(nsXULDocument * const 0x02074470) line 1792 + 7 bytes
XULContentSinkImpl::DidBuildModel(XULContentSinkImpl * const 0x02074470, int 
0x00000000) line 532
nsExpatDriver::DidBuildModel(nsExpatDriver * const 0x029eb520, unsigned int 
0x00000000, int 0x00000001, nsIParser * 0x0201f5a8, nsIContentSink * 
0x02a2f870) line 938 + 22 bytes
nsParser::DidBuildModel(nsParser * const 0x0012efe0, unsigned int 0x00000000) 
line 1251 + 13 bytes
nsParser::ResumeParse(nsParser * const 0x0012efe0, int 0x00000001, int 
0x00000001, int 0x00000001) line 1796
nsParser::OnStopRequest(nsParser * const 0x0201f5ac, nsIRequest * 0x02a54c28, 
nsISupports * 0x00000000, unsigned int 0x00000000) line 2425
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x0201f5ac, 
nsIRequest * 0x02a54c28, nsISupports * 0x00000000, unsigned int 0x00000000) 
line 256
nsFileChannel::OnStopRequest(nsFileChannel * const 0x02a54c30, nsIRequest * 
0x02a32a54, nsISupports * 0x00000000, unsigned int 0x00000000) line 519 + 14 
bytes
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x0012efe0) line 
213
PL_HandleEvent(PLEvent * 0x020e4924) line 597
PL_ProcessPendingEvents(PLEventQueue * 0x10031635) line 526 + 6 bytes
_md_EventReceiverProc(HWND__ * 0x01928c50, unsigned int 0x00402023, unsigned 
int 0x00ee58f8, long 0x7803ce38) line 1078
nsAppShellService::Run(nsAppShellService * const 0x00ee58f8) line 458
main1(int 0x00000002, char * * 0x00252ba8, nsISupports * 0x00252bd8) line 1456 
+ 9 bytes
main(int 0x00000002, char * * 0x00252ba8) line 1805 + 26 bytes
WinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00400000, char * 0x00133339, 
HINSTANCE__ * 0x00400000) line 1825 + 23 bytes
MOZILLA! WinMainCRTStartup + 308 bytes
KERNEL32! 77e87903()

Comment 6

[Andrew Schultz]

crashing with Linux build 20020615
I don't see the check for null... where is it?

This is what I see (nsViewManager::MoveViewTo)

nsView* parentView = view->GetParent();
(gdb refuses to acknowledge the existence of "parentView", but says that
view->GetParent() is null)

UpdateView(parentView, oldArea, NS_VMREFRESH_NO_SYNC);

UpdateView ASSERTs that aView is nsnull

view->ConvertFromParentCoords(&clippedRect.x, &clippedRect.y);
crash

Comment 7

[mk]

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/view/src/nsViewManager.cpp&rev=3.246&mark=1564%2c1572#1562    This link shows the code in question (check for aView being null) is an  NS_PRECONDITION macro.  This does not get defined in non-debug builds.  That's  why the crash is occuring. NS_PRECONDITION is defined here    http://lxr.mozilla.org/seamonkey/source/xpcom/glue/nsDebug.h#290  

Comment 8

[timeless]

.

Comment 9

[Richard Fine]

OK, glad to see it's been found.

I'm not entirely sure what the correct behavior of this should be, anyway. I 
was trying to seperate 'entries' in a window (I'm using an <HTML:hr> till 
this is fixed) but I don't know if this would be the correct way to do it.

Comment 10

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

This is a XUL frame construction issue.

I observe the following order of events. I will attach stack traces to show...
-- First the nsScrollBoxFrame for the scrollbox is created and Init()ed. At this
point a view is created for the nsScrollBoxFrame.
-- The nsSplitterFrame for the splitter is created and Init()ed. This creates a
view for the splitter. This view is attached to the nearest parent view ---
which happens to be the nsScrollBoxFrame's view.
-- The nsScrollBoxFrame's initial child list is set. This calls SetupScrollFrame
which creates a view for the scrolled frame. This view is set by SetScrolledView
to be the sole child of the nsScrollBoxFrame's view --- displacing the view we
created for the splitter! This causes havoc because now the splitter frame has a
view which is not in the view tree.

What should be happening is that the splitter's view should be made a child of
the view for the scrolled content. This will happen automatically as long as the
splitter frame's view is created after the view for the scrolled content. Either
we create the view for the scrolled content when the nsScrollBoxFrame is
initialized, or we create the view for the splitter frame when it is added as a
child of the scrollbox.

Comment 11

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

Attachment: Stack traces and notes

Comment 12

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

It looks like other scrolling frames create a view for their scrolled frame
child in FinishBuildingScrolledFrame, which happens before any more children are
created. That's what nsScrollBoxFrame should be doing too.

Comment 13

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

I can't see where nsScrollBoxFrame ensures that it only has one frame child.
Somehow it has to be hacked to work like a normal scrolling box. Over to you,
Brian :-)

Comment 14

[Alex Vincent [:WeirdAl]]

Bug 168724 apparently has a crash in the same spot, according to comment 5. 
(Thanks Andrew for the info.)

Comment 15

[greer]

*** Bug 174347 has been marked as a duplicate of this bug. ***

Comment 16

[Matthias Versen [:Matti]]

*** Bug 184382 has been marked as a duplicate of this bug. ***

Comment 17

[Carine Held]

Crashed using testcase on 1/03/03 Trunk build.

Comment 18

[Matthias Versen [:Matti]]

*** Bug 206885 has been marked as a duplicate of this bug. ***

Comment 19

[Boris Zbarsky [:bzbarsky]]

*** Bug 225092 has been marked as a duplicate of this bug. ***

Comment 20

[Stefan [:stefanh]]

*** Bug 230992 has been marked as a duplicate of this bug. ***

Comment 21

[Neil Paris]

Crash with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040316

Talback ID is TB10329G
Captured at 04/02/04 12:03 AM

Comment 22

[Jay Patel [:jay]]

Bryner:  You working on this?  Should this be a dup of bug 107518?  If you have
a chance, take a look at bug 237787 too...that looks related and could be a dup too.

Comment 23

[Alex Vincent [:WeirdAl]]

I might have this crash again.  I don't have the current stack trace or 
testcase yet (long story), but it involved a scrollbox whose anonymous content 
has overflow:scroll.

Comment 24

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

Alex, the important thing is whether your build is before or after the checkin
for bug 107518, which should have fixed this.

Comment 25

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

This was fixed by the fix for 107518. Try using a 1.8 trunk build. The next
Firefox release with this fix will be 1.1.

Comment 26

[Jesse Ruderman]

Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/54417ebbaea2