Fold out-of-flow objects into anonymous blocks when removing children

third_party/WebKit/Source/core/layout/LayoutBlock.cpp

Index: third_party/WebKit/Source/core/layout/LayoutBlock.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
index 75fabaddfae7c3372eb547349e52000fce1ab42a..66a070e6b986aa160974c9913820ce73418c874c 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
@@ -369,6 +369,16 @@ void LayoutBlock::invalidateDisplayItemClients(const LayoutBoxModelObject& paint
     invalidateDisplayItemClientForStartOfContinuationsIfNeeded(*this);
 }
 
+static void addSubsequentFloatingOrOutOfFlowSiblingsToBlock(LayoutBlock* block, LayoutBlock* container)
+{
+    LayoutObject* child = block->nextSibling();
+    while (child && child->isFloatingOrOutOfFlowPositioned()) {
+        LayoutObject* sibling = child->nextSibling();
+        container->moveChildTo(block, child, nullptr, false);
+        child = sibling;
+    }
+}
+
 void LayoutBlock::addChildIgnoringContinuation(LayoutObject* newChild, LayoutObject* beforeChild)
 {
     if (beforeChild && beforeChild->parent() != this) {
@@ -450,12 +460,7 @@ void LayoutBlock::addChildIgnoringContinuation(LayoutObject* newChild, LayoutObj
                 child = sibling;
             }
             newBox->addChild(newChild);
-            child = newBox->nextSibling();
-            while (child && child->isFloatingOrOutOfFlowPositioned()) {
-                LayoutObject* sibling = child->nextSibling();
-                moveChildTo(newBox, child, nullptr, false);
-                child = sibling;
-            }
+            addSubsequentFloatingOrOutOfFlowSiblingsToBlock(newBox, this);
             return;
         }
     }
@@ -730,6 +735,8 @@ void LayoutBlock::removeChild(LayoutObject* oldChild)
             && (!anonymousBlock->nextSibling() || (anonymousBlock->nextSibling()->style()->styleType() != NOPSEUDO && anonymousBlock->nextSibling()->isFloating() && !anonymousBlock->nextSibling()->nextSibling()))) {
             collapseAnonymousBlockChild(this, anonymousBlock);
         }
+        if (prev && prev->isAnonymousBlock())

[mstensho (USE GERRIT), 2015/10/30 08:15:14]

Don't we also need something similar for |next|?

<div id="outer">
    <div id="float" style="float:left;"></div>
    <div id="killMe"></div>
    Ost.
</div>

Now remove #killMe. I imagine we'd end up with

LayoutBlockFlow DIV #outer
    LayoutBlockFlow DIV #float
    LayoutBlockFlow anonymous
        LayoutText "Ost."

There should no longer be any need for the anonymous block at this point.

Admittedly, the rendering will be the same either way with this test, but isn't
the idea (what you've been working on lately) that we should be strict about
removing anonymous blocks when they are no longer needed?

[rhogan, 2015/10/30 19:28:16]

I would expect https://codereview.chromium.org/1423573002 to take care of that -
there is only floats/out-of-flows and anonymous blocks on the line after the
child is removed, so makeChildrenInlineIfPossible() collapses the anonymous
block. (Checked and it does.)

[mstensho (USE GERRIT), 2015/11/02 12:31:57]

Actually, the test in this CL also passes with only that CL (but
http://jsfiddle.net/U76jj/3/ doesn't), so we need a new test.

[rhogan, 2015/11/02 19:38:05]

Oops, fixed the test - it was missing a <div> before the <br>.

+            addSubsequentFloatingOrOutOfFlowSiblingsToBlock(toLayoutBlock(prev), this);
     }
 
     if (!firstChild()) {