Clear intruding floats when moving child to inline parent.

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

Index: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
index 34c0b0792f4ba35358b9df99ac1b8106c46df1f8..74d53b3160cc4b290e148a77b828241d52056b2b 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -1010,6 +1010,13 @@ void LayoutBoxModelObject::moveChildTo(LayoutBoxModelObject* toBoxModelObject, L
 
     ASSERT(this == child->parent());
     ASSERT(!beforeChild || toBoxModelObject == beforeChild->parent());
+
+    // If a child is moving from a block-flow to an inline-flow parent then any floats currently intruding into

[mstensho (USE GERRIT), 2015/12/07 14:07:35]

I think we need an example here. When can things like this happen?

+    // the child can no longer do so. Remove them from their float-lists immediately as markAllDescendantsWithFloatsForLayout
+    // won't attempt to remove floats from parents that have inline-flow if we try later.

[mstensho (USE GERRIT), 2015/12/07 14:07:35]

Yeah... regarding that... Why don't we just attempt to remove floats from
inline-children blocks too? Then we wouldn't have bugs like this, right?

[rhogan, 2015/12/07 17:29:52]

Performance. I removed it once but then added it back in again:
https://codereview.chromium.org/1397383002. 

[mstensho (USE GERRIT), 2015/12/07 21:14:44]

Why did that cause a performance regression? I don't understand why that has to
be particularly expensive.

Is it because you have a DIV with 30000 floats inside (but no in-flow blocks, so
children will be inline)? Looking at
https://code.google.com/p/chromium/issues/attachmentText?id=537111&aid=537111...

What if we change the test case to contain one in-flow block followed by 30000
float siblings? Then the floats would be in a block-children container, and
you'd have serious performance problems as well. But that's OK, since it's
always been like that, or what?

[rhogan, 2015/12/08 18:15:44]

I let markAllDescendantsWithFloatForLayout() descend through elements with
inline children in all cases just because I introduced an edge-case where
elements with inline-children might have intruding floats in their float-lists
temporarily (i.e. before getting cleared at the next layout). So it was an
unnecessary performance regression incurred whenever we removed a float. You're
right - that if a float does indeed intrude many levels down the layout tree it
would take some time to remove it from all of them. But no point in letting it
do that it when it doesn't need to.

+    if (child->isLayoutBlockFlow() && toBoxModelObject->childrenInline() && !child->childrenInline())

[mstensho (USE GERRIT), 2015/12/07 21:14:44]

I think something is missing here.

The condition is: if the child to be moved has block children and it is being
moved into a block with inline children... That could for instance mean an
inline-block with block children being moved from one block to another (both
having inline children). Killing the floats of an inline-block is completely
unnecessary, since inline-blocks establish block formatting contexts.

Add "&& !childrenInline()", just to make clear that the old parent had block
children?

[rhogan, 2015/12/08 18:15:44]

Yes, that's true - it couldn't have any intruding floats from ancestors if all
its siblings were inline.

+        toLayoutBlockFlow(child)->removeFloatingObjectsFromDescendants();
+
     if (fullRemoveInsert && (toBoxModelObject->isLayoutBlock() || toBoxModelObject->isLayoutInline())) {
         // Takes care of adding the new child correctly if toBlock and fromBlock
         // have different kind of children (block vs inline).