Merge r228464 - [RenderTreeBuilder] Move RenderElement::takeChild() t…

…o RenderTreeBuilder

https://bugs.webkit.org/show_bug.cgi?id=182762
<rdar://problem/37523756>

Reviewed by Daniel Bates.

This patch removes the remaining takeChild() related mutation logic from the renderers.

No change in functionality.

* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::moveChildTo):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::takeChild): Deleted.
(WebCore::RenderElement::takeChildInternal): Deleted.
* rendering/RenderElement.h:
* rendering/RenderObject.h:
* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::takeChild):
(WebCore::RenderTreeBuilder::childFlowStateChangesAndAffectsParentBlock):
(WebCore::RenderTreeBuilder::takeChildFromRenderElement):
* rendering/updating/RenderTreeBuilder.h:
* rendering/updating/RenderTreeBuilderBlock.cpp:
(WebCore::RenderTreeBuilder::Block::removeLeftoverAnonymousBlock):
(WebCore::RenderTreeBuilder::Block::takeChild):
(WebCore::RenderTreeBuilder::Block::dropAnonymousBoxChild):
* rendering/updating/RenderTreeBuilderInline.cpp:
(WebCore::RenderTreeBuilder::Inline::splitFlow):
(WebCore::RenderTreeBuilder::Inline::splitInlines):
(WebCore::RenderTreeBuilder::Inline::childBecameNonInline):
* rendering/updating/RenderTreeBuilderRuby.cpp:
(WebCore::RenderTreeBuilder::Ruby::takeChild):
* rendering/updating/RenderTreeBuilderSVG.cpp:
(WebCore::RenderTreeBuilder::SVG::takeChild): Leftover from the previous patch.

Source/WebCore/ChangeLog

@@ -1,3 +1,40 @@
+2018-02-14  Zalan Bujtas  <zalan@apple.com>
+
+        [RenderTreeBuilder] Move RenderElement::takeChild() to RenderTreeBuilder
+        https://bugs.webkit.org/show_bug.cgi?id=182762
+        <rdar://problem/37523756>
+
+        Reviewed by Daniel Bates.
+
+        This patch removes the remaining takeChild() related mutation logic from the renderers.
+
+        No change in functionality.
+
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::moveChildTo):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::takeChild): Deleted.
+        (WebCore::RenderElement::takeChildInternal): Deleted.
+        * rendering/RenderElement.h:
+        * rendering/RenderObject.h:
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::takeChild):
+        (WebCore::RenderTreeBuilder::childFlowStateChangesAndAffectsParentBlock):
+        (WebCore::RenderTreeBuilder::takeChildFromRenderElement):
+        * rendering/updating/RenderTreeBuilder.h:
+        * rendering/updating/RenderTreeBuilderBlock.cpp:
+        (WebCore::RenderTreeBuilder::Block::removeLeftoverAnonymousBlock):
+        (WebCore::RenderTreeBuilder::Block::takeChild):
+        (WebCore::RenderTreeBuilder::Block::dropAnonymousBoxChild):
+        * rendering/updating/RenderTreeBuilderInline.cpp:
+        (WebCore::RenderTreeBuilder::Inline::splitFlow):
+        (WebCore::RenderTreeBuilder::Inline::splitInlines):
+        (WebCore::RenderTreeBuilder::Inline::childBecameNonInline):
+        * rendering/updating/RenderTreeBuilderRuby.cpp:
+        (WebCore::RenderTreeBuilder::Ruby::takeChild):
+        * rendering/updating/RenderTreeBuilderSVG.cpp:
+        (WebCore::RenderTreeBuilder::SVG::takeChild): Leftover from the previous patch.
+
 2018-02-13  Zalan Bujtas  <zalan@apple.com>
 
         [RenderTreeBuilder] Move RenderBlock::takeChild() to RenderTreeBuilder

Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -2696,10 +2696,10 @@ void RenderBoxModelObject::moveChildTo(RenderTreeBuilder& builder, RenderBoxMode
     if (normalizeAfterInsertion == NormalizeAfterInsertion::Yes && (toBoxModelObject->isRenderBlock() || toBoxModelObject->isRenderInline())) {
         // Takes care of adding the new child correctly if toBlock and fromBlock
         // have different kind of children (block vs inline).
-        auto childToMove = takeChildInternal(*child);
+        auto childToMove = builder.takeChildFromRenderElement(*this, *child);
         builder.insertChild(*toBoxModelObject, WTFMove(childToMove), beforeChild);
     } else {
-        auto childToMove = takeChildInternal(*child);
+        auto childToMove = builder.takeChildFromRenderElement(*this, *child);
         toBoxModelObject->insertChildInternal(WTFMove(childToMove), beforeChild);
     }
 }

Source/WebCore/rendering/RenderElement.cpp

@@ -481,11 +481,6 @@ void RenderElement::addChildIgnoringContinuation(RenderTreeBuilder& builder, Ren
     builder.insertChild(*this, WTFMove(newChild), beforeChild);
 }
 
-RenderPtr<RenderObject> RenderElement::takeChild(RenderTreeBuilder&, RenderObject& oldChild)
-{
-    return takeChildInternal(oldChild);
-}
-
 void RenderElement::removeAndDestroyChild(RenderTreeBuilder& builder, RenderObject& oldChild)
 {
     if (is<RenderElement>(oldChild)) {
@@ -582,65 +577,6 @@ void RenderElement::insertChildInternal(RenderPtr<RenderObject> newChildPtr, Ren
         newChild->setHasOutlineAutoAncestor();
 }
 
-RenderPtr<RenderObject> RenderElement::takeChildInternal(RenderObject& oldChild)
-{
-    RELEASE_ASSERT_WITH_MESSAGE(!view().frameView().layoutContext().layoutState(), "Layout must not mutate render tree");
-
-    ASSERT(canHaveChildren() || canHaveGeneratedChildren());
-    ASSERT(oldChild.parent() == this);
-
-    if (oldChild.isFloatingOrOutOfFlowPositioned())
-        downcast<RenderBox>(oldChild).removeFloatingOrPositionedChildFromBlockLists();
-
-    // So that we'll get the appropriate dirty bit set (either that a normal flow child got yanked or
-    // that a positioned child got yanked). We also repaint, so that the area exposed when the child
-    // disappears gets repainted properly.
-    if (!renderTreeBeingDestroyed() && oldChild.everHadLayout()) {
-        oldChild.setNeedsLayoutAndPrefWidthsRecalc();
-        // We only repaint |oldChild| if we have a RenderLayer as its visual overflow may not be tracked by its parent.
-        if (oldChild.isBody())
-            view().repaintRootContents();
-        else
-            oldChild.repaint();
-    }
-
-    // If we have a line box wrapper, delete it.
-    if (is<RenderBox>(oldChild))
-        downcast<RenderBox>(oldChild).deleteLineBoxWrapper();
-    else if (is<RenderLineBreak>(oldChild))
-        downcast<RenderLineBreak>(oldChild).deleteInlineBoxWrapper();
-
-    if (!renderTreeBeingDestroyed() && is<RenderFlexibleBox>(this) && !oldChild.isFloatingOrOutOfFlowPositioned() && oldChild.isBox())
-        downcast<RenderFlexibleBox>(this)->clearCachedChildIntrinsicContentLogicalHeight(downcast<RenderBox>(oldChild));
-
-    // If oldChild is the start or end of the selection, then clear the selection to
-    // avoid problems of invalid pointers.
-    if (!renderTreeBeingDestroyed() && oldChild.isSelectionBorder())
-        frame().selection().setNeedsSelectionUpdate();
-
-    if (!renderTreeBeingDestroyed())
-        oldChild.willBeRemovedFromTree();
-
-    oldChild.resetFragmentedFlowStateOnRemoval();
-
-    // WARNING: There should be no code running between willBeRemovedFromTree and the actual removal below.
-    // This is needed to avoid race conditions where willBeRemovedFromTree would dirty the tree's structure
-    // and the code running here would force an untimely rebuilding, leaving |oldChild| dangling.
-    auto childToTake = detachRendererInternal(oldChild);
-
-    // rendererRemovedFromTree walks the whole subtree. We can improve performance
-    // by skipping this step when destroying the entire tree.
-    if (!renderTreeBeingDestroyed() && is<RenderElement>(*childToTake))
-        RenderCounter::rendererRemovedFromTree(downcast<RenderElement>(*childToTake));
-
-    if (!renderTreeBeingDestroyed()) {
-        if (AXObjectCache* cache = document().existingAXObjectCache())
-            cache->childrenChanged(this);
-    }
-
-    return childToTake;
-}
-
 RenderBlock* RenderElement::containingBlockForFixedPosition() const
 {
     auto* renderer = parent();

Source/WebCore/rendering/RenderElement.h

@@ -89,7 +89,6 @@ class RenderElement : public RenderObject {
     virtual bool isChildAllowed(const RenderObject&, const RenderStyle&) const { return true; }
     virtual void addChild(RenderTreeBuilder&, RenderPtr<RenderObject>, RenderObject* beforeChild);
     virtual void addChildIgnoringContinuation(RenderTreeBuilder&, RenderPtr<RenderObject> newChild, RenderObject* beforeChild = nullptr);
-    virtual RenderPtr<RenderObject> takeChild(RenderTreeBuilder&, RenderObject&) WARN_UNUSED_RETURN;
     void removeAndDestroyChild(RenderTreeBuilder&, RenderObject&);
 
     // The following functions are used when the render tree hierarchy changes to make sure layers get
@@ -101,7 +100,6 @@ class RenderElement : public RenderObject {
     RenderLayer* findNextLayer(RenderLayer* parentLayer, RenderObject* startPoint, bool checkParent = true);
 
     void insertChildInternal(RenderPtr<RenderObject>, RenderObject* beforeChild);
-    RenderPtr<RenderObject> takeChildInternal(RenderObject&) WARN_UNUSED_RETURN;
 
     virtual RenderElement* hoverAncestor() const;
 

Source/WebCore/rendering/RenderObject.h

@@ -776,6 +776,9 @@ class RenderObject : public CachedImageClient {
         return outlineBoundsForRepaint(nullptr);
     }
 
+    virtual void willBeRemovedFromTree();
+    void resetFragmentedFlowStateOnRemoval();
+
 protected:
     //////////////////////////////////////////
     // Helper functions. Dangerous to use!
@@ -791,7 +794,6 @@ class RenderObject : public CachedImageClient {
     virtual void willBeDestroyed(RenderTreeBuilder&);
 
     virtual void insertedIntoTree();
-    virtual void willBeRemovedFromTree();
 
     void setNeedsPositionedMovementLayoutBit(bool b) { m_bitfields.setNeedsPositionedMovementLayout(b); }
     void setNormalChildNeedsLayoutBit(bool b) { m_bitfields.setNormalChildNeedsLayout(b); }
@@ -802,7 +804,6 @@ class RenderObject : public CachedImageClient {
     static void calculateBorderStyleColor(const EBorderStyle&, const BoxSide&, Color&);
 
     void initializeFragmentedFlowStateOnInsertion();
-    void resetFragmentedFlowStateOnRemoval();
     static FragmentedFlowState computedFragmentedFlowState(const RenderObject&);
 
 private:

Source/WebCore/rendering/updating/RenderTreeBuilder.cpp

@@ -26,9 +26,14 @@
 #include "config.h"
 #include "RenderTreeBuilder.h"
 
+#include "AXObjectCache.h"
+#include "Frame.h"
+#include "FrameSelection.h"
 #include "RenderButton.h"
+#include "RenderCounter.h"
 #include "RenderElement.h"
 #include "RenderGrid.h"
+#include "RenderLineBreak.h"
 #include "RenderMenuList.h"
 #include "RenderRuby.h"
 #include "RenderRubyBase.h"
@@ -230,7 +235,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::takeChild(RenderElement& parent, Rend
     if (is<RenderBlock>(parent))
         return blockBuilder().takeChild(downcast<RenderBlock>(parent), child);
 
-    return parent.takeChild(*this, child);
+    return takeChildFromRenderElement(parent, child);
 }
 
 void RenderTreeBuilder::insertChild(RenderTreePosition& position, RenderPtr<RenderObject> child)
@@ -359,7 +364,7 @@ void RenderTreeBuilder::childFlowStateChangesAndAffectsParentBlock(RenderElement
         auto newBlock = downcast<RenderBlock>(*parent).createAnonymousBlock();
         auto& block = *newBlock;
         parent->insertChildInternal(WTFMove(newBlock), &child);
-        auto thisToMove = parent->takeChildInternal(child);
+        auto thisToMove = takeChildFromRenderElement(*parent, child);
         block.insertChildInternal(WTFMove(thisToMove), nullptr);
     }
 }
@@ -563,4 +568,64 @@ RenderPtr<RenderObject> RenderTreeBuilder::takeChildFromRenderGrid(RenderGrid& p
     return takenChild;
 }
 
+RenderPtr<RenderObject> RenderTreeBuilder::takeChildFromRenderElement(RenderElement& parent, RenderObject& child)
+{
+    RELEASE_ASSERT_WITH_MESSAGE(!parent.view().frameView().layoutContext().layoutState(), "Layout must not mutate render tree");
+
+    ASSERT(parent.canHaveChildren() || parent.canHaveGeneratedChildren());
+    ASSERT(child.parent() == &parent);
+
+    if (child.isFloatingOrOutOfFlowPositioned())
+        downcast<RenderBox>(child).removeFloatingOrPositionedChildFromBlockLists();
+
+    // So that we'll get the appropriate dirty bit set (either that a normal flow child got yanked or
+    // that a positioned child got yanked). We also repaint, so that the area exposed when the child
+    // disappears gets repainted properly.
+    if (!parent.renderTreeBeingDestroyed() && child.everHadLayout()) {
+        child.setNeedsLayoutAndPrefWidthsRecalc();
+        // We only repaint |child| if we have a RenderLayer as its visual overflow may not be tracked by its parent.
+        if (child.isBody())
+            parent.view().repaintRootContents();
+        else
+            child.repaint();
+    }
+
+    // If we have a line box wrapper, delete it.
+    if (is<RenderBox>(child))
+        downcast<RenderBox>(child).deleteLineBoxWrapper();
+    else if (is<RenderLineBreak>(child))
+        downcast<RenderLineBreak>(child).deleteInlineBoxWrapper();
+
+    if (!parent.renderTreeBeingDestroyed() && is<RenderFlexibleBox>(parent) && !child.isFloatingOrOutOfFlowPositioned() && child.isBox())
+        downcast<RenderFlexibleBox>(parent).clearCachedChildIntrinsicContentLogicalHeight(downcast<RenderBox>(child));
+
+    // If child is the start or end of the selection, then clear the selection to
+    // avoid problems of invalid pointers.
+    if (!parent.renderTreeBeingDestroyed() && child.isSelectionBorder())
+        parent.frame().selection().setNeedsSelectionUpdate();
+
+    if (!parent.renderTreeBeingDestroyed())
+        child.willBeRemovedFromTree();
+
+    child.resetFragmentedFlowStateOnRemoval();
+
+    // WARNING: There should be no code running between willBeRemovedFromTree() and the actual removal below.
+    // This is needed to avoid race conditions where willBeRemovedFromTree() would dirty the tree's structure
+    // and the code running here would force an untimely rebuilding, leaving |child| dangling.
+    auto childToTake = parent.detachRendererInternal(child);
+
+    // rendererRemovedFromTree() walks the whole subtree. We can improve performance
+    // by skipping this step when destroying the entire tree.
+    if (!parent.renderTreeBeingDestroyed() && is<RenderElement>(*childToTake))
+        RenderCounter::rendererRemovedFromTree(downcast<RenderElement>(*childToTake));
+
+    if (!parent.renderTreeBeingDestroyed()) {
+        if (AXObjectCache* cache = parent.document().existingAXObjectCache())
+            cache->childrenChanged(&parent);
+    }
+
+    return childToTake;
+}
+
+
 }

Source/WebCore/rendering/updating/RenderTreeBuilder.h

@@ -78,6 +78,8 @@ class RenderTreeBuilder {
     void insertChildToRenderTableRow(RenderTableRow& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
     void insertChildToRenderMathMLFenced(RenderMathMLFenced& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
 
+    RenderPtr<RenderObject> takeChildFromRenderElement(RenderElement& parent, RenderObject& child) WARN_UNUSED_RETURN;
+
     bool childRequiresTable(const RenderElement& parent, const RenderObject& child);
     void makeChildrenNonInline(RenderBlock& parent, RenderObject* insertionPoint = nullptr);
     RenderObject* splitAnonymousBoxesAroundChild(RenderBox& parent, RenderObject* beforeChild);

Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp

@@ -261,7 +261,7 @@ void RenderTreeBuilder::Block::removeLeftoverAnonymousBlock(RenderBlock& anonymo
 
     // FIXME: This should really just be a moveAllChilrenTo (see webkit.org/b/182495)
     anonymousBlock.moveAllChildrenToInternal(*parent);
-    auto toBeDestroyed = parent->takeChildInternal(anonymousBlock);
+    auto toBeDestroyed = m_builder.takeChildFromRenderElement(*parent, anonymousBlock);
     // anonymousBlock is dead here.
 }
 
@@ -270,7 +270,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::Block::takeChild(RenderBlock& parent,
     // No need to waste time in merging or removing empty anonymous blocks.
     // We can just bail out if our document is getting destroyed.
     if (parent.renderTreeBeingDestroyed())
-        return parent.RenderBox::takeChild(m_builder, oldChild);
+        return m_builder.takeChildFromRenderElement(parent, oldChild);
 
     // If this child is a block, and if our previous and next siblings are both anonymous blocks
     // with inline content, then we can fold the inline content back together.
@@ -293,7 +293,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::Block::takeChild(RenderBlock& parent,
             ASSERT(!inlineChildrenBlock.continuation());
             // Cache this value as it might get changed in setStyle() call.
             inlineChildrenBlock.setStyle(RenderStyle::createAnonymousStyleWithDisplay(parent.style(), BLOCK));
-            auto blockToMove = parent.takeChildInternal(inlineChildrenBlock);
+            auto blockToMove = m_builder.takeChildFromRenderElement(parent, inlineChildrenBlock);
 
             // Now just put the inlineChildrenBlock inside the blockChildrenBlock.
             RenderObject* beforeChild = prev == &inlineChildrenBlock ? blockChildrenBlock.firstChild() : nullptr;
@@ -320,7 +320,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::Block::takeChild(RenderBlock& parent,
 
     parent.invalidateLineLayoutPath();
 
-    auto takenChild = parent.RenderBox::takeChild(m_builder, oldChild);
+    auto takenChild = m_builder.takeChildFromRenderElement(parent, oldChild);
 
     RenderObject* child = prev ? prev : next;
     if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling() && parent.canDropAnonymousBlockChild()) {
@@ -360,7 +360,7 @@ void RenderTreeBuilder::Block::dropAnonymousBoxChild(RenderBlock& parent, Render
     parent.setChildrenInline(child.childrenInline());
     auto* nextSibling = child.nextSibling();
 
-    auto toBeDeleted = parent.takeChildInternal(child);
+    auto toBeDeleted = m_builder.takeChildFromRenderElement(parent, child);
     child.moveAllChildrenTo(m_builder, &parent, nextSibling, RenderBoxModelObject::NormalizeAfterInsertion::No);
     // Delete the now-empty block's lines and nuke it.
     child.deleteLines();

Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp

@@ -235,7 +235,7 @@ void RenderTreeBuilder::Inline::splitFlow(RenderInline& parent, RenderObject* be
         while (o) {
             RenderObject* no = o;
             o = no->nextSibling();
-            auto childToMove = block->takeChildInternal(*no);
+            auto childToMove = m_builder.takeChildFromRenderElement(*block, *no);
             pre->insertChildInternal(WTFMove(childToMove), nullptr);
             no->setNeedsLayoutAndPrefWidthsRecalc();
         }
@@ -305,7 +305,7 @@ void RenderTreeBuilder::Inline::splitInlines(RenderInline& parent, RenderBlock*
             // FIXME: When the anonymous wrapper has multiple children, we end up traversing up to the topmost wrapper
             // every time, which is a bit wasteful.
         }
-        auto childToMove = rendererToMove->parent()->takeChildInternal(*rendererToMove);
+        auto childToMove = m_builder.takeChildFromRenderElement(*rendererToMove->parent(), *rendererToMove);
         cloneInline->addChildIgnoringContinuation(m_builder, WTFMove(childToMove));
         rendererToMove->setNeedsLayoutAndPrefWidthsRecalc();
         rendererToMove = nextSibling;
@@ -343,7 +343,7 @@ void RenderTreeBuilder::Inline::splitInlines(RenderInline& parent, RenderBlock*
             // *after* currentChild and append them all to the clone.
             for (auto* sibling = currentChild->nextSibling(); sibling;) {
                 auto* next = sibling->nextSibling();
-                auto childToMove = current->takeChildInternal(*sibling);
+                auto childToMove = m_builder.takeChildFromRenderElement(*current, *sibling);
                 cloneInline->addChildIgnoringContinuation(m_builder, WTFMove(childToMove));
                 sibling->setNeedsLayoutAndPrefWidthsRecalc();
                 sibling = next;
@@ -367,7 +367,7 @@ void RenderTreeBuilder::Inline::splitInlines(RenderInline& parent, RenderBlock*
     // and put them in the toBlock.
     for (auto* current = currentChild->nextSibling(); current;) {
         auto* next = current->nextSibling();
-        auto childToMove = fromBlock->takeChildInternal(*current);
+        auto childToMove = m_builder.takeChildFromRenderElement(*fromBlock, *current);
         toBlock->insertChildInternal(WTFMove(childToMove), nullptr);
         current = next;
     }
@@ -389,7 +389,7 @@ void RenderTreeBuilder::Inline::childBecameNonInline(RenderInline& parent, Rende
         oldContinuation->removeFromContinuationChain();
     newBox->insertIntoContinuationChainAfter(parent);
     auto* beforeChild = child.nextSibling();
-    auto removedChild = parent.takeChildInternal(child);
+    auto removedChild = m_builder.takeChildFromRenderElement(parent, child);
     splitFlow(parent, beforeChild, WTFMove(newBox), WTFMove(removedChild), oldContinuation);
 }
 

Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp

@@ -382,7 +382,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::Ruby::takeChild(RenderRubyAsInline& p
 #ifndef ASSERT_DISABLED
         ASSERT(isRubyChildForNormalRemoval(child));
 #endif
-        return parent.RenderInline::takeChild(m_builder, child);
+        return m_builder.takeChildFromRenderElement(parent, child);
     }
     // If the child's parent is an anoymous block (must be generated :before/:after content)
     // just use the block's remove method.