Strip anonymous wrappers when a block flow no longer requires them

Source/core/layout/LayoutBlock.cpp

Index: Source/core/layout/LayoutBlock.cpp
diff --git a/Source/core/layout/LayoutBlock.cpp b/Source/core/layout/LayoutBlock.cpp
index 5da005c2fcc76890474949487d16347e60c14874..d6cfe1b9258687fe951aeef8e25378dcb9599af7 100644
--- a/Source/core/layout/LayoutBlock.cpp
+++ b/Source/core/layout/LayoutBlock.cpp
@@ -300,8 +300,14 @@ void LayoutBlock::styleDidChange(StyleDifference diff, const ComputedStyle* oldS
 {
     LayoutBox::styleDidChange(diff, oldStyle);
 
-    if (isFloatingOrOutOfFlowPositioned() && oldStyle && !oldStyle->isFloating() && !oldStyle->hasOutOfFlowPosition() && parent() && parent()->isLayoutBlockFlow())
+    if (isFloatingOrOutOfFlowPositioned() && oldStyle && !oldStyle->isFloating() && !oldStyle->hasOutOfFlowPosition() && parent() && parent()->isLayoutBlockFlow()) {
         toLayoutBlock(parent())->removeAnonymousWrappersIfRequired();
+        // Reparent to an adjacent anonymous block if one is available.
+        if (previousSibling() && previousSibling()->isAnonymousBlock())
+            toLayoutBlock(parent())->moveChildTo(toLayoutBlock(previousSibling()), this, nullptr, false);
+        if (nextSibling() && nextSibling()->isAnonymousBlock())

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

else if. Not that there should be two adjacent anonymous blocks ever, but no
need to perform this check if we found a suitable previousSibling().

+            toLayoutBlock(parent())->moveChildTo(toLayoutBlock(nextSibling()), this, nextSibling()->slowFirstChild(), false);
+    }
 
     const ComputedStyle& newStyle = styleRef();
 
@@ -478,7 +484,20 @@ void LayoutBlock::addChildIgnoringContinuation(LayoutObject* newChild, LayoutObj
             // No suitable existing anonymous box - create a new one.
             LayoutBlock* newBox = createAnonymousBlock();
             LayoutBox::addChild(newBox, beforeChild);
+            // Reparent adjacent inline siblings to the new box.

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

Not inline. Float or out-of-flow positioned.

+            LayoutObject* child = newBox->previousSibling();
+            while (child && child->isFloatingOrOutOfFlowPositioned()) {
+                LayoutObject* sibling = child->previousSibling();
+                moveChildTo(newBox, child, newBox->firstChild(), false);
+                child = sibling;
+            }
             newBox->addChild(newChild);
+            child = newBox->nextSibling();
+            while (child && child->isFloatingOrOutOfFlowPositioned()) {
+                LayoutObject* sibling = child->nextSibling();
+                moveChildTo(newBox, child, nullptr, false);
+                child = sibling;
+            }
             return;
         }
     }
@@ -672,6 +691,19 @@ void LayoutBlock::collapseAnonymousBlockChild(LayoutBlock* parent, LayoutBlock*
     child->destroy();
 }
 
+static bool shouldMakeChildrenInline(LayoutBlock* block)

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

I hope the parameter can be const.

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

Could be static inline?

+{
+    if (!block->isLayoutBlockFlow())
+        return false;
+    LayoutObject* child = block->firstChild();
+    while (child) {
+        if (!child->isFloatingOrOutOfFlowPositioned())

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

I still believe that if a LayoutBlockFlow only contains floats, out-of-flow
objects and/or anonymous blocks, we should get rid of all anonymous blocks and
promote their children. I think it's wrong to keep anonymous blocks for no
reason. But, by all means, not in this CL. Could you add a TODO, though?

+            return false;
+        child = child->nextSibling();
+    }
+    return true;
+}
+
 void LayoutBlock::removeChild(LayoutObject* oldChild)
 {
     // No need to waste time in merging or removing empty anonymous blocks.
@@ -774,6 +806,10 @@ void LayoutBlock::removeChild(LayoutObject* oldChild)
             setContinuation(nullptr);
             destroy();
         }
+    } else if (!beingDestroyed() && !child->isFloatingOrOutOfFlowPositioned() && shouldMakeChildrenInline(this)) {
+        // If we are a block that allows inline children and the child we're removing means that

[mstensho (USE GERRIT), 2015/09/15 11:47:30]

The "that allows inline children" confuses me. I understand that it means "not
flexbox, not grid, not table", but this is currently a block that has BLOCK
children. Ok, so there's nothing incorrect with this sentence, but could you
just cut the beginning so that it becomes "If the child we're removing means
that ..."?

+        // we can now treat all children as inline without the need for anonymous blocks, then do that.
+        setChildrenInline(true);
     }
 }