232017 - [FIX]<html:input type="file"/> in XUL document causes crash (nsBlockBandData.cpp#71) after failing NS_PRECONDITION

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Alex Vincent [:WeirdAl]]

#0  0x405a46f6 in nanosleep () from /lib/libc.so.6
#1  0x0000001c in ?? ()
#2  0x080721be in ah_crap_handler(int) (signum=11) at nsSigHandlers.cpp:135
#3  0x419368a9 in nsProfileLock::FatalSignalHandler(int) (signo=11)
    at nsProfileLock.cpp:209
#4  0x4012bc2d in __pthread_sighandler () from /lib/libpthread.so.0
#5  0x4051fd58 in __libc_sigaction () from /lib/libc.so.6
#6  0x410131da in nsBlockBandData::Init(nsSpaceManager*, nsSize const&) (
    this=0xbfffd450, aSpaceManager=0x0, aSpace=@0xbfffd400)
    at nsBlockBandData.cpp:71
#7  0x41027b8f in nsBlockReflowState (this=0xbfffd3e0,
    aReflowState=@0xbfffd830, aPresContext=0x87831d8, aFrame=0x87fdb50,
    aMetrics=@0xbfffdad0, aBlockMarginRoot=0) at nsBlockReflowState.cpp:148
#8  0x41015117 in nsBlockFrame::Reflow(nsIPresContext*, nsHTMLReflowMetrics&,
nsHTMLReflowState const&, unsigned&) (this=0x87fdb50, aPresContext=0x87831d8,
    aMetrics=@0xbfffdad0, aReflowState=@0xbfffd830, aStatus=@0xbfffdb20)
    at nsBlockFrame.cpp:654
#9  0x410dea85 in nsFileControlFrame::Reflow(nsIPresContext*,
nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) (this=0x87fdb50,
    aPresContext=0x87831d8, aDesiredSize=@0xbfffdad0,
    aReflowState=@0xbfffd830, aStatus=@0xbfffdb20)
    at nsFileControlFrame.cpp:371
#10 0x4117e58f in nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState&,
nsIPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&, int,
int, int, int,---Type <return> to continue, or q <return> to quit---
 int) (this=0x87faf9c, aState=@0xbfffe0d0, aPresContext=0x87831d8,
    aDesiredSize=@0xbfffdad0, aReflowState=@0xbfffd9d0, aStatus=@0xbfffdb20,
    aX=0, aY=0, aWidth=1073741824, aHeight=1073741824, aMoveFrame=1)
    at nsBoxToBlockAdaptor.cpp:878
#11 0x4117d757 in nsBoxToBlockAdaptor::RefreshSizeCache(nsBoxLayoutState&) (
    this=0x87faf9c, aState=@0xbfffe0d0) at nsBoxToBlockAdaptor.cpp:375
#12 0x4117de02 in nsBoxToBlockAdaptor::GetAscent(nsBoxLayoutState&, int&) (
    this=0x87faf9c, aState=@0xbfffe0d0, aAscent=@0xbfffdba4)
    at nsBoxToBlockAdaptor.cpp:589
#13 0x4118360c in nsSprocketLayout::GetAscent(nsIBox*, nsBoxLayoutState&, int&)
    (this=0x8263318, aBox=0x87bc388, aState=@0xbfffe0d0, aAscent=@0x87bc3c8)
    at nsSprocketLayout.cpp:1509
#14 0x41180644 in nsContainerBox::GetAscent(nsBoxLayoutState&, int&) (
    this=0x87bc388, aState=@0xbfffe0d0, aAscent=@0x87bc3c8)
    at nsContainerBox.cpp:594
#15 0x411787ca in nsBoxFrame::GetAscent(nsBoxLayoutState&, int&) (
    this=0x87bc350, aBoxLayoutState=@0xbfffe0d0, aAscent=@0xbfffdc44)
    at nsBoxFrame.cpp:952
#16 0x4118360c in nsSprocketLayout::GetAscent(nsIBox*, nsBoxLayoutState&, int&)
    (this=0x8263318, aBox=0x87bc188, aState=@0xbfffe0d0, aAscent=@0x87bc1c8)
    at nsSprocketLayout.cpp:1509
#17 0x41180644 in nsContainerBox::GetAscent(nsBoxLayoutState&, int&) (
    this=0x87bc188, aState=@0xbfffe0d0, aAscent=@0x87bc1c8)
    at nsContainerBox.cpp:594
#18 0x411787ca in nsBoxFrame::GetAscent(nsBoxLayoutState&, int&) (
    this=0x87bc150, aBoxLayoutState=@0xbfffe0d0, aAscent=@0xbfffdce4)
    at nsBoxFrame.cpp:952
#19 0x4118360c in nsSprocketLayout::GetAscent(nsIBox*, nsBoxLayoutState&, int&)
    (this=0x8263318, aBox=0x8781e1c, aState=@0xbfffe0d0, aAscent=@0x8781e5c)
    at nsSprocketLayout.cpp:1509
#20 0x41180644 in nsContainerBox::GetAscent(nsBoxLayoutState&, int&) (
    this=0x8781e1c, aState=@0xbfffe0d0, aAscent=@0x8781e5c)
    at nsContainerBox.cpp:594
#21 0x411787ca in nsBoxFrame::GetAscent(nsBoxLayoutState&, int&) (
    this=0x8781de4, aBoxLayoutState=@0xbfffe0d0, aAscent=@0xbfffde64)
    at nsBoxFrame.cpp:952
#22 0x4118113a in nsSprocketLayout::Layout(nsIBox*, nsBoxLayoutState&) (
    this=0x8263318, aBox=0x8781e1c, aState=@0xbfffe0d0)
    at nsSprocketLayout.cpp:228
#23 0x41180707 in nsContainerBox::DoLayout(nsBoxLayoutState&) (this=0x8781e1c,
    aState=@0xbfffe0d0) at nsContainerBox.cpp:610
#24 0x41178b49 in nsBoxFrame::DoLayout(nsBoxLayoutState&) (this=0x8781de4,
    aState=@0xbfffe0d0) at nsBoxFrame.cpp:1052
#25 0x41173b3d in nsBox::Layout(nsBoxLayoutState&) (this=0x8781e1c,
    aState=@0xbfffe0d0) at nsBox.cpp:994
#26 0x41184c7b in nsStackLayout::Layout(nsIBox*, nsBoxLayoutState&) (
    this=0x84013d8, aBox=0x8781c00, aState=@0xbfffe0d0)
    at nsStackLayout.cpp:319
#27 0x41180707 in nsContainerBox::DoLayout(nsBoxLayoutState&) (this=0x8781c00,
    aState=@0xbfffe0d0) at nsContainerBox.cpp:610
#28 0x41178b49 in nsBoxFrame::DoLayout(nsBoxLayoutState&) (this=0x8781bc8,
    aState=@0xbfffe0d0) at nsBoxFrame.cpp:1052
#29 0x41173b3d in nsBox::Layout(nsBoxLayoutState&) (this=0x8781c00,
    aState=@0xbfffe0d0) at nsBox.cpp:994
#30 0x411784c0 in nsBoxFrame::Reflow(nsIPresContext*, nsHTMLReflowMetrics&,
nsHTMLReflowState const&, unsigned&) (this=0x8781bc8, aPresContext=0x87831d8,
    aDesiredSize=@0xbfffe2d0, aReflowState=@0xbfffe200, aStatus=@0xbfffe4d8)
    at nsBoxFrame.cpp:865
#31 0x411717a3 in nsRootBoxFrame::Reflow(nsIPresContext*, nsHTMLReflowMetrics&,
nsHTMLReflowState const&, unsigned&) (this=0x8781bc8, aPresContext=0x87831d8,
    aDesiredSize=@0xbfffe2d0, aReflowState=@0xbfffe200, aStatus=@0xbfffe4d8)
    at nsRootBoxFrame.cpp:239
#32 0x41030e4a in nsContainerFrame::ReflowChild(nsIFrame*, nsIPresContext*,
nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned, unsigned&) (
    this=0x8781acc, aKidFrame=0x8781bc8, aPresContext=0x87831d8,
    aDesiredSize=@0xbfffe2d0, aReflowState=@0xbfffe200, aX=0, aY=0, aFlags=0,
    aStatus=@0xbfffe4d8) at nsContainerFrame.cpp:934
#33 0x410c8d48 in ViewportFrame::Reflow(nsIPresContext*, nsHTMLReflowMetrics&,
nsHTMLReflowState const&, unsigned&) (this=0x8781acc, aPresContext=0x87831d8,
    aDesiredSize=@0xbfffe480, aReflowState=@0xbfffe3b0, aStatus=@0xbfffe4d8)
    at nsViewportFrame.cpp:247
#34 0x41097842 in PresShell::InitialReflow(int, int) (this=0x87812f0,
    aWidth=15210, aHeight=8955) at nsPresShell.cpp:2800
#35 0x4149e169 in nsXULDocument::StartLayout() (this=0x877e608)
    at nsXULDocument.cpp:2197
#36 0x414a1e20 in nsXULDocument::ResumeWalk() (this=0x877e608)
    at nsXULDocument.cpp:3053
#37 0x41499526 in nsXULDocument::EndLoad() (this=0x877e608)
    at nsXULDocument.cpp:720
#38 0x41491646 in XULContentSinkImpl::DidBuildModel() (this=0x877f358)
    at nsXULContentSink.cpp:457
#39 0x419d5124 in nsExpatDriver::DidBuildModel(unsigned, int, nsIParser*,
nsIContentSink*) (this=0x8785760, anErrorCode=0, aNotifySink=1, aParser=0x877f4a8,
    aSink=0x877f358) at nsExpatDriver.cpp:1042
#40 0x419f3212 in nsParser::DidBuildModel(unsigned) (this=0x877f4a8,
    anErrorCode=0) at nsParser.cpp:1245
#41 0x419f42e3 in nsParser::ResumeParse(int, int, int) (this=0x877f4a8,
    allowIteration=1, aIsFinalChunk=1, aCanInterrupt=1) at nsParser.cpp:1806
#42 0x419f343b in nsParser::ContinueParsing() (this=0x877f4a8)
    at nsParser.cpp:1359
#43 0x413e4e58 in CSSLoaderImpl::SheetComplete(SheetLoadData*, int) (
    this=0x877f3e8, aLoadData=0x8792be0, aSucceeded=1) at nsCSSLoader.cpp:1530
#44 0x413e4ef1 in CSSLoaderImpl::SheetComplete(SheetLoadData*, int) (
    this=0x877f3e8, aLoadData=0x878ba58, aSucceeded=1) at nsCSSLoader.cpp:1547
#45 0x413e4a6e in CSSLoaderImpl::ParseSheet(nsIUnicharInputStream*,
SheetLoadData*, int&) (this=0x877f3e8, aStream=0x87930b0, aLoadData=0x878ba58,
    aCompleted=@0xbfffec2c) at nsCSSLoader.cpp:1462
#46 0x413e2517 in SheetLoadData::OnStreamComplete(nsIUnicharStreamLoader*,
nsISupports*, unsigned, nsIUnicharInputStream*) (this=0x878ba58, aLoader=0x878be30,
    aContext=0x0, aStatus=0, aDataStream=0x87930b0) at nsCSSLoader.cpp:805
#47 0x40bae74c in nsUnicharStreamLoader::OnStopRequest(nsIRequest*,
nsISupports*, unsigned) (this=0x878be30, request=0x8797068, ctxt=0x0, aStatus=0)
    at nsUnicharStreamLoader.cpp:194
#48 0x40c16ae1 in nsFileChannel::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x8797068, req=0x8797228, ctx=0x0, status=0) at
nsFileChannel.cpp:577
#49 0x40b81318 in nsInputStreamPump::OnStateStop() (this=0x8797228)
    at nsInputStreamPump.cpp:498
#50 0x40b80cdb in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (
    this=0x8797228, stream=0x87970f4) at nsInputStreamPump.cpp:339
#51 0x407f3da1 in nsInputStreamReadyEvent::EventHandler(PLEvent*) (
    plevent=0x87973e4) at nsStreamUtils.cpp:118
#52 0x408196c4 in PL_HandleEvent (self=0x87973e4) at plevent.c:671
#53 0x40819565 in PL_ProcessPendingEvents (self=0x813e098) at plevent.c:606
#54 0x4081bb0e in nsEventQueueImpl::ProcessPendingEvents() (this=0x813de98)
    at nsEventQueue.cpp:391
#55 0x4188bd74 in event_processor_callback (data=0x813de98, source=4,
    condition=GDK_INPUT_READ) at nsAppShell.cpp:187
#56 0x4188b6dd in our_gdk_io_invoke (source=0x8209aa8, condition=G_IO_IN,
    data=0x8209a98) at nsAppShell.cpp:72
#57 0x4030d7d6 in g_io_channel_unix_get_fd () from /usr/lib/libglib-1.2.so.0
#58 0x403103ee in g_idle_remove_by_data () from /usr/lib/libglib-1.2.so.0
#59 0x40310199 in g_idle_remove_by_data () from /usr/lib/libglib-1.2.so.0
#60 0x4030f174 in g_main_run () from /usr/lib/libglib-1.2.so.0

Testcase coming up in a moment.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: crash testcase

There's a couple assertions (actually NS_PRECONDITIONS) which are violated just
before the crash.  From my terminal:

###!!! ASSERTION: SpaceManager should be set in nsBlockReflowState:
'mSpaceManager', file nsBlockReflowState.cpp, line 91
Break: at file nsBlockReflowState.cpp, line 91
###!!! ASSERTION: null pointer: 'aSpaceManager', file nsBlockBandData.cpp, line
68
Break: at file nsBlockBandData.cpp, line 68

I still have gdb running; I'm trying to track down where the space manager is
actually set... it's further back in the stack, I can tell that much...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Sounds like the file input should set the space manager bits?

Comment 3

[Alex Vincent [:WeirdAl]]

Okay, the ill-begotten aReflowState argument in frame 6 is created somewhere in
nsBoxToBlockAdaptor.cpp (frame 10).  Looks like the reflowState value (which is
its name in nsBoxToBlockAdaptor) is created on line 802.

Unfortunately, I cannot tell which nsHTMLReflowState constructor is called. 
Stinking overloading of methods...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: This fixes the crash

Comment 5

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

Why shouldn't the file control frame always have the space manager bit set?

Why isn't the boxtoblockadaptor code working?

Comment 6

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

Comment on attachment 139783 [details] [diff] [review]
This fixes the crash

minusing to trigger response to my questions

Comment 7

[Boris Zbarsky [:bzbarsky]]

The response will be forthcoming once I've had time to figure out the answer to
the second one....

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK, so the answers are:

1) No reason.  In fact, makes sense to always set it.

2) The BoxToBlockAdaptor code is semi-evil:

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsBoxToBlockAdaptor.cpp#146

Naturally the file control frame does not qi to "kBlockFrameCID"

The problem is that the box-to-block adaptor gets wrapped around any non-nsIBox
childframe of a nsContainerBox.  So in fact, its mFrame may not be a block and
setting the space manager flag would not be appropriate then (it's not an
nsIFrame bit, so it may mean different things to different frame types).

The right thing here may be to make nsBoxToBlockAdaptor smarter and to remove
the code in ConstructBlock that's just like the code I proposed here....

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, and the reason the file control frame does not QI to kBlockFrameCID is that
its QI method forwards to nsHTMLContainerFrame, not its direct parent
(nsAreaFrame).  I'm not happy with changing that, since it would possibly
confuse the BlockReflowContext/BlockReflowState code.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed.