Fix crash when establishing an inline continuation inside a block continuation.

Source/core/layout/LayoutTable.h

Index: Source/core/layout/LayoutTable.h
diff --git a/Source/core/layout/LayoutTable.h b/Source/core/layout/LayoutTable.h
index 511ef67a5d5549b7521883e27b11941aa5ad573f..02679b0be15df41ef6d1ade3b6d5215a430667a0 100644
--- a/Source/core/layout/LayoutTable.h
+++ b/Source/core/layout/LayoutTable.h
@@ -122,7 +122,8 @@ public:
     int calcBorderEnd() const;
     void recalcBordersInRowDirection();
 
-    virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0) override;
+    virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0) override final;

[Julien - ping for review, 2015/01/28 17:29:53]

This is redundant, LayoutTable is marked as final already.

[mstensho (USE GERRIT), 2015/01/28 17:51:26]

Done.

+    virtual void addChildIgnoringContinuation(RenderObject* newChild, RenderObject* beforeChild = 0) override;

[mstensho (USE GERRIT), 2015/01/28 10:43:16]

LayoutTable has its own very special implementation of addChild(), but used to
use RenderBlock's implementation of addChildIgnoringContinuation(), which deals
with funny things like insertion of anonymous block boxes around things that
need it (this is only for block containers, and probably flexboxes and grids).
More importantly, though, calling LayoutTable::addChildIgnoringContinuation()
without a local override here would skip LayoutTable::addChild(), which is where
we take care of redirecting to (or even inserting) the right table object
parent.

This change alone (adding a correct addChildIgnoringContinuation() override)
would actually fix the crash, although we might still end up placing the child
inside a table when we shouldn't (which should be harmless apart from
potentially broken layout).

Even with the fix I have in RenderBlock, this change makes sense, though.

[Julien - ping for review, 2015/01/28 17:29:53]

We didn't need the override AFAICT as we never expected a continuation in a
table (thus the ASSERT you're removing). It may be an oversight.

[mstensho (USE GERRIT), 2015/01/28 17:51:26]

The parent of |beforeChild| in RenderBlock::addChildToContinuation() may be an
anonymous table. Well, not after my fix, but I added this override here anyway,
in case there are others who call addChildIgnoringContinuation() on tables and
expect them to behave. I could have called
RenderObject::addChildIgnoringContinuation(newChild, beforeChild) here instead,
BTW, because that method does the exact same thing (except that then you'd
definitely end up with another vtable lookup).

Should I assert !isLayoutTable() in RenderBlock::addChildIgnoringContinuation()
instead?

[Julien - ping for review, 2015/01/29 10:28:31]

I think it's safer to implement the function (think defense-in-depth pattern)
but we should ASSERT that we don't arrive in the override (with a comment as to
why).

[mstensho (USE GERRIT), 2015/01/29 12:08:42]

Done.

[mstensho (USE GERRIT), 2015/01/29 17:49:48]

FYI, removing the assertion again. It got hit by
fast/table/crash-bad-child-table-continuation.html

[mstensho (USE GERRIT), 2015/01/29 18:05:41]

Reported crbug.com/453488 for the
fast/table/crash-bad-child-table-continuation.html misbehavior.

 
     struct ColumnStruct {
         explicit ColumnStruct(unsigned initialSpan = 1)