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

Source/core/rendering/RenderInline.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 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
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 485 matching lines @@
     if (madeNewBeforeBlock) {
         RenderObject* o = boxFirst;
         while (o) {
             RenderObject* no = o;
             o = no->nextSibling();
             pre->children()->appendChildNode(pre, block->children()->removeChildNode(block, no));
             no->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
         }
     }
 
+    // Always just do a full layout in order to ensure that line boxes (especially wrappers for images)
+    // get deleted properly.  Because objects moves from the pre block into the post block, we want to
+    // make new line boxes instead of leaving the old line boxes around.
+    pre->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
+    block->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
+    post->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();

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

If we only keep this change but drop the ones in RenderBlock and LayoutTable,
the crash will go away, but we'll still get assertion failures in
RenderInline::layout() in the test case. Which means that it'd be hard to write
a test that doesn't crash in debug.

What splitInlines() might end up doing on a rainy day is beyond anyone's
anticipation. The crash in this bug happens because |post| is dead after having
called splitInlines(), but that was because the render tree was borked. With
either the fix in RenderBlock or LayoutTable, this won't happen anymore in our
case, but there may be other cases that we're not aware of where it may also
happen. So I was thinking that we might as well order layout with side dishes up
front, before the earthquake.

[dsinclair, 2015/01/28 16:35:01]

So, this is moved up just to guard against the case where splitInlines could
delete the pre, block or post renderers? This sounds like it's still going to be
a pretty nasty pit to step in later. Would it be possible to leave this part of
the change out and file a seperate bug to figure out how to make this sane?

Or, immediately after splitInlines, can we null out pre, block and post so we
make sure they aren't touched again in case they go wonky in splitInlines?

[mstensho (USE GERRIT), 2015/01/28 16:51:19]

Correct. But I don't know if would still be possible for that to happen if the
rest of the patch is taken in. But splitInlines() calls moveChildrenTo(), which
calls addChild(). RenderBlock::addChildIgnoringAnonymousColumnBlocks() ends with
"// this object may be dead here". But it is admittedly weird to worry about
that here in splitFlow(). moveChildrenTo() would also be in trouble if its
|toBoxModelObject| gets killed. In my case it did happen at the last child,
though, so it survived.
I can just remove it. It's a lousy fix anyway, since it depends on the
underlying code to attempt to shoot itself in the foot and miss.

[dsinclair, 2015/01/28 17:08:06]

Yea, lets put this back in that case, and file a bug to look into make this
safer (if possible).

[mstensho (USE GERRIT), 2015/01/28 17:19:54]

Done.

Sounds like something to do while working on crbug.com/417556 - added a comment
there.

+
     splitInlines(pre, post, newBlockBox, beforeChild, oldCont);
 
     // We already know the newBlockBox isn't going to contain inline kids, so avoid wasting
     // time in makeChildrenNonInline by just setting this explicitly up front.
     newBlockBox->setChildrenInline(false);
 
     newBlockBox->addChild(newChild);
-
-    // Always just do a full layout in order to ensure that line boxes (especially wrappers for images)
-    // get deleted properly.  Because objects moves from the pre block into the post block, we want to
-    // make new line boxes instead of leaving the old line boxes around.
-    pre->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
-    block->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
-    post->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
 }
 
 void RenderInline::addChildToContinuation(RenderObject* newChild, RenderObject* beforeChild)
 {
     RenderBoxModelObject* flow = continuationBefore(beforeChild);
     ASSERT(!beforeChild || beforeChild->parent()->isRenderBlock() || beforeChild->parent()->isRenderInline());
     RenderBoxModelObject* beforeChildParent = 0;
     if (beforeChild)
         beforeChildParent = toRenderBoxModelObject(beforeChild->parent());
     else {
@@ skipping 920 matching lines @@
         container = this;
 
     FloatPoint absPos = container->localToAbsolute();
     region.bounds.setX(absPos.x() + region.bounds.x());
     region.bounds.setY(absPos.y() + region.bounds.y());
 
     regions.append(region);
 }
 
 } // namespace blink