Clear intruding floats when moving child to inline parent.

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. 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
@@ skipping 992 matching lines @@
 }
 
 void LayoutBoxModelObject::moveChildTo(LayoutBoxModelObject* toBoxModelObject, LayoutObject* child, LayoutObject* beforeChild, bool fullRemoveInsert)
 {
     // We assume that callers have cleared their positioned objects list for child moves (!fullRemoveInsert) so the
     // positioned layoutObject maps don't become stale. It would be too slow to do the map lookup on each call.
     ASSERT(!fullRemoveInsert || !isLayoutBlock() || !toLayoutBlock(this)->hasPositionedObjects());
 
     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).
         toBoxModelObject->addChild(virtualChildren()->removeChildNode(this, child), beforeChild);
     } else {
         toBoxModelObject->virtualChildren()->insertChildNode(toBoxModelObject, virtualChildren()->removeChildNode(this, child, fullRemoveInsert), beforeChild, fullRemoveInsert);
     }
 }
 
 void LayoutBoxModelObject::moveChildrenTo(LayoutBoxModelObject* toBoxModelObject, LayoutObject* startChild, LayoutObject* endChild, LayoutObject* beforeChild, bool fullRemoveInsert)
@@ skipping 34 matching lines @@
     if (rootElementStyle->hasBackground())
         return false;
 
     if (node() != document().firstBodyElement())
         return false;
 
     return true;
 }
 
 } // namespace blink