11393 - crash appending cell via DOM

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect, P3)

Description

[buster]

open the test case
click on the "Append Cell" button
hit the assert, and next line crashes.
this is a regression. something has broken in either incremental reflow or the
table layout strategy, I suspect incremental reflow (despite the incorrect code
in the layout strategy that dereferences a null pointer without checking it
first, it should return an error.)

stack:
nsDebug::Assertion(const char * 0x09ac1160, const char * 0x09ac114c, const char
* 0x09ac110c, int 403) line 176 + 13 bytes
BasicTableLayoutStrategy::AssignPreliminaryColumnWidths(int 2250) line 403 + 32
bytes
BasicTableLayoutStrategy::Initialize(nsSize * 0x00000000, int 2, int 2250) line
81
nsTableFrame::Reflow(nsTableFrame * const 0x09cde0d4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 2287
nsContainerFrame::ReflowChild(nsIFrame * 0x09cde0d0, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 439 + 28 bytes
nsTableOuterFrame::IR_InnerTableReflow(nsTableOuterFrame * const 0x09cde1d0,
nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState &
{...}, unsigned int & 0) line 600 + 34 bytes
nsTableOuterFrame::IR_TargetIsInnerTableFrame(nsTableOuterFrame * const
0x09cde1d0, nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
OuterTableReflowState & {...}, unsigned int & 0) line 370 + 31 bytes
nsTableOuterFrame::IR_TargetIsChild(nsTableOuterFrame * const 0x09cde1d0,
nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState &
{...}, unsigned int & 0, nsIFrame * 0x09cde0d0) line 342 + 31 bytes
nsTableOuterFrame::IncrementalReflow(nsTableOuterFrame * const 0x09cde1d0,
nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState &
{...}, unsigned int & 0) line 324 + 35 bytes
nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x09cde1d4, nsIPresContext &
{...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned
int & 0) line 997 + 35 bytes
nsBlockReflowContext::ReflowBlock(nsIFrame * 0x09cde1d0, const nsRect & {...},
int 1, int 0, int 0, nsMargin & {...}, unsigned int & 0) line 223 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox *
0x09ce15c0, int * 0x00129920) line 2744 + 56 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x09ce15c0, int
* 0x00129920, int 1) line 2133 + 20 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 1928 + 30 bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x09cc52f4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 1238 + 18 bytes
nsBlockReflowContext::ReflowBlock(nsIFrame * 0x09cc52f0, const nsRect & {...},
int 1, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 223 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox *
0x09cc5030, int * 0x0012bd38) line 2744 + 56 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x09cc5030, int
* 0x0012bd38, int 1) line 2133 + 20 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 1928 + 30 bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x09cc5894, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 1238 + 18 bytes
nsAreaFrame::Reflow(nsAreaFrame * const 0x09cc5894, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 344 + 25 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x09cc5890, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 439 + 28 bytes
RootFrame::Reflow(RootFrame * const 0x09cc5e14, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 327
nsContainerFrame::ReflowChild(nsIFrame * 0x09cc5e10, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 439 + 28 bytes
nsScrollFrame::Reflow(nsScrollFrame * const 0x09cc4bb4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 543
nsContainerFrame::ReflowChild(nsIFrame * 0x09cc4bb0, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 439 + 28 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x09cbbf34, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 513
nsHTMLReflowCommand::Dispatch(nsHTMLReflowCommand * const 0x09cf84c0,
nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsSize & {...},
nsIRenderingContext & {...}) line 169
PresShell::ProcessReflowCommands(PresShell * const 0x09c98450) line 1184
PresShell::ExitReflowLock(PresShell * const 0x09c98450) line 564
PresShell::ContentAppended(PresShell * const 0x09c98458, nsIDocument *
0x09b784d0, nsIContent * 0x09cc67ac, int 1) line 1564
nsDocument::ContentAppended(nsDocument * const 0x09b784d0, nsIContent *
0x09cc67ac, int 1) line 1569
nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x09b784d0, nsIContent *
0x09cc67ac, int 1) line 845
nsGenericHTMLContainerElement::AppendChildTo(nsIContent * 0x09cf6160, int 1)
line 2779
nsGenericHTMLContainerElement::InsertBefore(nsIDOMNode * 0x09cf6154, nsIDOMNode
* 0x00000000, nsIDOMNode * * 0x0012e5ac) line 2463 + 14 bytes
nsGenericHTMLContainerElement::AppendChild(nsIDOMNode * 0x09cf6154, nsIDOMNode *
* 0x0012e5ac) line 2619
nsHTMLTableRowElement::AppendChild(nsHTMLTableRowElement * const 0x09cc67a0,
nsIDOMNode * 0x09cf6154, nsIDOMNode * * 0x0012e5ac) line 60 + 22 bytes
NodeAppendChild(JSContext * 0x09b95940, JSObject * 0x01220908, unsigned int 1,
long * 0x012278c8, long * 0x0012e668) line 600 + 25 bytes
js_Invoke(JSContext * 0x09b95940, unsigned int 1, unsigned int 0) line 654 + 26
bytes

Comment 1

[troy]

What are you smoking crack? It's not dereferencing a null pointer without
checking it. It's hitting an assert, which seems like a perfectly reasonable
thing for Chris to have done

Comment 2

[troy]

Re-assigning the bug to me since it's probably related to my reflow command
changes.

Comment 3

[buster]

troy, at least the crack I smoke is good quality.  You must be smoking Comet
cleanser.
Read what I wrote:
"hit the assert, and next line crashes."
yep, it's hit's the assert.  And then it crashes on the next line, by failing to
check for null.  I don't care much about the null check, I want the root cause
of the assert to be fixed, amigo.

Comment 4

[troy]

Added a call to InvalidateColumnCache() and this fixes it. It's so unclear in
the table code how to incrementally update the various table data structures.

I've already added the appended table cell to the cell map, using
AddCellToTable(). You would think that would incrementally update the column
cache as well

Steve, you should be deeply ashamed of yourself for creating this horribly
un-incremental mess. I wouldn't mind so much, except that I have to work on
it...

Comment 5

[Christine Hoffman]

Steve: Unable to verify this fixed - could you please? Thanks

Comment 6

[Christine Hoffman]

Steve: Could you please mark this bug verified fixed, if you agree?

Comment 7

[buster]

the crash is gone, but the table incremental reflow code is not working
correctly.  open the testcase (note: I updated the url to it's new location) and
click "append cell" notice that the new cell does not appear.  you can validate
that it is there by dumping the content model.  Now click "insert cell"  all 3
cells magically become visible.

Troy, Chris K., and Chris D.: I recommend you play with this test case, it
exhibits several other incremental reflow bugs.

I would either re-open this bug with a slightly different summary ("incremental
reflow broken when cell added via DOM"), or close this one since the crash is
gone and open a new one.  your choice.

Comment 8

[leger]

Adding crash keyword

Comment 9

[ckritzer (gone)]

testcase link no longer exists...buster, do you know where the link disappeared 
to?  And/or do you have an idea of where I can find a copy of the 
tableIncrementalReflow.html?  Thanks! -ckritzer

Comment 10

[Christine Hoffman]

Verifying this bug fixed for original reason. If there are additional issues, 
please write up new bug.