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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2010. 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
@@ skipping 351 matching lines @@
     if (startOfContinuations && startOfContinuations->styleRef().outlineStyleIsAuto())
         startOfContinuations->invalidateDisplayItemClient(*startOfContinuations);
 }
 
 void LayoutBlock::invalidateDisplayItemClients(const LayoutBoxModelObject& paintInvalidationContainer, PaintInvalidationReason invalidationReason, const LayoutRect& previousPaintInvalidationRect, const LayoutRect& newPaintInvalidationRect) const
 {
     LayoutBox::invalidateDisplayItemClients(paintInvalidationContainer, invalidationReason, previousPaintInvalidationRect, newPaintInvalidationRect);
     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) {
         LayoutObject* beforeChildContainer = beforeChild->parent();
         while (beforeChildContainer->parent() != this)
             beforeChildContainer = beforeChildContainer->parent();
         ASSERT(beforeChildContainer);
 
         if (beforeChildContainer->isAnonymous()) {
             // If the requested beforeChild is not one of our children, then this is because
@@ skipping 61 matching lines @@
             LayoutBlock* newBox = createAnonymousBlock();
             LayoutBox::addChild(newBox, beforeChild);
             // Reparent adjacent floating or out-of-flow siblings to the new box.
             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;
-            }
+            addSubsequentFloatingOrOutOfFlowSiblingsToBlock(newBox, this);
             return;
         }
     }
 
     LayoutBox::addChild(newChild, beforeChild);
 
     if (madeBoxesNonInline && parent() && isAnonymousBlock() && parent()->isLayoutBlock())
         toLayoutBlock(parent())->removeLeftoverAnonymousBlock(this);
     // this object may be dead here
 }
@@ skipping 254 matching lines @@
     } else if (((prev && prev->isAnonymousBlock()) || (next && next->isAnonymousBlock())) && canCollapseAnonymousBlockChild()) {
         // It's possible that the removal has knocked us down to a single anonymous
         // block with pseudo-style element siblings (e.g. first-letter). If these
         // are floating, then we need to pull the content up also.
         LayoutBlock* anonymousBlock = toLayoutBlock((prev && prev->isAnonymousBlock()) ? prev : next);
         if ((anonymousBlock->previousSibling() || anonymousBlock->nextSibling())
             && (!anonymousBlock->previousSibling() || (anonymousBlock->previousSibling()->style()->styleType() != NOPSEUDO && anonymousBlock->previousSibling()->isFloating() && !anonymousBlock->previousSibling()->previousSibling()))
             && (!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()) {
         // If this was our last child be sure to clear out our line boxes.
         if (childrenInline())
             deleteLineBoxTree();
 
         // If we are an empty anonymous block in the continuation chain,
         // we need to remove ourself and fix the continuation chain.
         if (!beingDestroyed() && isAnonymousBlockContinuation() && !oldChild->isListMarker()) {
@@ skipping 2137 matching lines @@
 void LayoutBlock::showLineTreeAndMark(const InlineBox* markedBox1, const char* markedLabel1, const InlineBox* markedBox2, const char* markedLabel2, const LayoutObject* obj) const
 {
     showLayoutObject();
     for (const RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox())
         root->showLineTreeAndMark(markedBox1, markedLabel1, markedBox2, markedLabel2, obj, 1);
 }
 
 #endif
 
 } // namespace blink