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

Source/core/layout/LayoutTable.h

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2009, 2010 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 104 matching lines @@
     {
         if (style()->isHorizontalWritingMode())
             return style()->isFlippedBlocksWritingMode() ? outerBorderBefore() : outerBorderAfter();
         return style()->isLeftToRightDirection() ? outerBorderEnd() : outerBorderStart();
     }
 
     int calcBorderStart() const;
     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)
             : span(initialSpan)
         {
         }
 
         unsigned span;
     };
 
@@ skipping 223 matching lines @@
     if (m_firstBody)
         return m_firstBody;
     return m_foot;
 }
 
 DEFINE_RENDER_OBJECT_TYPE_CASTS(LayoutTable, isTable());
 
 } // namespace blink
 
 #endif // LayoutTable_h