Remove unnecessary anonymous blocks when splitting anonymous boxes

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

Index: third_party/WebKit/Source/core/layout/LayoutBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index fb82a7c0afc2369dcb56c705915c16ed387d0d39..2eabd41a472a538b2f76cd0ac81669ba83d564d0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -4670,6 +4670,23 @@ static void markBoxForRelayoutAfterSplit(LayoutBox* box)
     box->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation(LayoutInvalidationReason::AnonymousBlockChange);
 }
 
+static bool collapseIfWillBecomeLoneAnonymousBlock(LayoutBox* child)

[mstensho (USE GERRIT), 2016/01/28 20:45:14]

I think this would fit better in LayoutBlock. Doing so much LayoutBlock-specific
stuff here isn't so great.

[rhogan, 2016/01/30 12:38:58]

Waaaah! I moved it here because you thought it should be static. :)

+{
+    if (!child->isAnonymousBlock())
+        return false;
+    LayoutBlock* childBlock = toLayoutBlock(child);
+    if (childBlock->beingDestroyed())
+        return false;
+    // We're splitting from our previousSibling().

[mstensho (USE GERRIT), 2016/01/28 20:45:14]

I'm still uneasy about this.

I think we should rather nuke superfluous anonymous blocks after the split,
rather than guessing what the tree is going to look like before making the
split.

So, basically, for each postBox (once each is created) in
LayoutBox::splitAnonymousBoxesAroundChild():
If postBox only has one child, and that child is an anonyomus block that's
collapsable, collapseAnonymousBlockChild() it!

And don't we really have to do the same thing for the other side of the split as
well? Might end up with a lone anonymous block there too:

<div style="display:table-row-group;">
    <div style="display:inline-block; width:75px; height:50%;
background:blue;"></div>
    <div id="splitter" style="display:table-row;"></div>
    <div></div>
</div>

[rhogan, 2016/01/30 12:38:57]

We're not guessing though - if we create postBox here and it has no
nextSibling() then it's a lone anonymous block and can be collapsed right away.
All good points - applied.

+    if (child->nextSibling() || childBlock->continuation())
+        return false;
+    LayoutBlock* parent = child->containingBlock();

[mstensho (USE GERRIT), 2016/01/28 20:45:14]

This should always be the same as child->parent(), or we'd be in trouble, right?

Then I think it would be safer (and slightly faster, woho!) to call
child->parent() and bail if it's not a block.

[rhogan, 2016/01/30 12:38:57]

Done.

+    if (parent->isRubyBase() || parent->isRubyRun())
+        return false;
+    LayoutBlock::collapseAnonymousBlockChild(parent, childBlock);
+    return true;
+}
+
 LayoutObject* LayoutBox::splitAnonymousBoxesAroundChild(LayoutObject* beforeChild)
 {
     bool didSplitParentAnonymousBoxes = false;
@@ -4696,7 +4713,10 @@ LayoutObject* LayoutBox::splitAnonymousBoxesAroundChild(LayoutObject* beforeChil
 
             beforeChild = postBox;
         } else {
-            beforeChild = boxToSplit;
+            // If a lone anonymous block is now unnecessary collapse it away - this will make beforeChild's current
+            // grandparent its parent so no need to set it before looping.
+            if (!collapseIfWillBecomeLoneAnonymousBlock(boxToSplit))
+                beforeChild = boxToSplit;
         }
     }