Merge r228224 - [RenderTreeBuilder] Move RenderBlock::removeLeftoverA…

…nonymousBlock to RenderTreeBuilder https://bugs.webkit.org/show_bug.cgi?id=182510 <rdar://problem/37250037> Reviewed by Antti Koivisto. Do not reinvent subtree reparenting. Covered by existing tests. * rendering/RenderBlock.cpp: (WebCore::RenderBlock::removeLeftoverAnonymousBlock): Deleted. * rendering/RenderBlock.h: * rendering/RenderBoxModelObject.cpp: (WebCore::RenderBoxModelObject::moveAllChildrenToInternal): * rendering/RenderBoxModelObject.h: * rendering/RenderButton.h: * rendering/RenderElement.cpp: (WebCore::RenderElement::detachRendererInternal): (WebCore::RenderElement::attachRendererInternal): (WebCore::RenderElement::insertChildInternal): (WebCore::RenderElement::takeChildInternal): * rendering/RenderElement.h: * rendering/RenderRuby.h: * rendering/RenderRubyRun.h: * rendering/RenderTextControl.h: * rendering/updating/RenderTreeBuilderBlock.cpp: (WebCore::RenderTreeBuilder::Block::insertChildIgnoringContinuation): (WebCore::RenderTreeBuilder::Block::childBecameNonInline): (WebCore::RenderTreeBuilder::Block::removeLeftoverAnonymousBlock): * rendering/updating/RenderTreeBuilderBlock.h:
WebKit · Feb 19, 2018 · f474661 · f474661
1 parent 6d4e3ed
commit f474661
Show file tree

Hide file tree

Showing 13 changed files with 119 additions and 98 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2018-02-07  Zalan Bujtas  <zalan@apple.com>
+
+        [RenderTreeBuilder] Move RenderBlock::removeLeftoverAnonymousBlock to RenderTreeBuilder
+        https://bugs.webkit.org/show_bug.cgi?id=182510
+        <rdar://problem/37250037>
+
+        Reviewed by Antti Koivisto.
+
+        Do not reinvent subtree reparenting.
+
+        Covered by existing tests.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::removeLeftoverAnonymousBlock): Deleted.
+        * rendering/RenderBlock.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::moveAllChildrenToInternal):
+        * rendering/RenderBoxModelObject.h:
+        * rendering/RenderButton.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::detachRendererInternal):
+        (WebCore::RenderElement::attachRendererInternal):
+        (WebCore::RenderElement::insertChildInternal):
+        (WebCore::RenderElement::takeChildInternal):
+        * rendering/RenderElement.h:
+        * rendering/RenderRuby.h:
+        * rendering/RenderRubyRun.h:
+        * rendering/RenderTextControl.h:
+        * rendering/updating/RenderTreeBuilderBlock.cpp:
+        (WebCore::RenderTreeBuilder::Block::insertChildIgnoringContinuation):
+        (WebCore::RenderTreeBuilder::Block::childBecameNonInline):
+        (WebCore::RenderTreeBuilder::Block::removeLeftoverAnonymousBlock):
+        * rendering/updating/RenderTreeBuilderBlock.h:
+
 2018-02-06  Don Olmstead  <don.olmstead@sony.com>
 
         Remove WebCore/ForwardingHeaders directory

diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
@@ -482,58 +482,6 @@ void RenderBlock::deleteLines()
         cache->deferRecomputeIsIgnored(element());
 }
 
-void RenderBlock::removeLeftoverAnonymousBlock(RenderBlock* child)
-{
-    ASSERT(child->isAnonymousBlock());
-    ASSERT(!child->childrenInline());
-
-    if (child->continuation())
-        return;
-
-    RenderObject* firstAnChild = child->firstChild();
-    RenderObject* lastAnChild = child->lastChild();
-    if (firstAnChild) {
-        RenderObject* o = firstAnChild;
-        while (o) {
-            o->setParent(this);
-            o = o->nextSibling();
-        }
-        firstAnChild->setPreviousSibling(child->previousSibling());
-        lastAnChild->setNextSibling(child->nextSibling());
-        if (child->previousSibling())
-            child->previousSibling()->setNextSibling(firstAnChild);
-        if (child->nextSibling())
-            child->nextSibling()->setPreviousSibling(lastAnChild);
-
-        if (child == firstChild())
-            setFirstChild(firstAnChild);
-        if (child == lastChild())
-            setLastChild(lastAnChild);
-    } else {
-        if (child == firstChild())
-            setFirstChild(child->nextSibling());
-        if (child == lastChild())
-            setLastChild(child->previousSibling());
-
-        if (child->previousSibling())
-            child->previousSibling()->setNextSibling(child->nextSibling());
-        if (child->nextSibling())
-            child->nextSibling()->setPreviousSibling(child->previousSibling());
-    }
-
-    child->setFirstChild(nullptr);
-    child->m_next = nullptr;
-
-    // Remove all the information in the flow thread associated with the leftover anonymous block.
-    child->resetFragmentedFlowStateOnRemoval();
-
-    child->setParent(nullptr);
-    child->setPreviousSibling(nullptr);
-    child->setNextSibling(nullptr);
-
-    child->destroy();
-}
-
 static bool canDropAnonymousBlock(const RenderBlock& anonymousBlock)
 {
     if (anonymousBlock.beingDestroyed() || anonymousBlock.continuation())

diff --git a/Source/WebCore/rendering/RenderBlock.h b/Source/WebCore/rendering/RenderBlock.h
@@ -396,7 +396,6 @@ class RenderBlock : public RenderBox {
     void adjustBorderBoxRectForPainting(LayoutRect&) override;
     LayoutRect paintRectToClipOutFromBorder(const LayoutRect&) override;
     void addChildIgnoringContinuation(RenderTreeBuilder&, RenderPtr<RenderObject> newChild, RenderObject* beforeChild) override;
-    virtual void removeLeftoverAnonymousBlock(RenderBlock* child);
     bool isInlineBlockOrInlineTable() const final { return isInline() && isReplaced(); }
 
 protected:

diff --git a/Source/WebCore/rendering/RenderBoxModelObject.cpp b/Source/WebCore/rendering/RenderBoxModelObject.cpp
@@ -2742,4 +2742,10 @@ void RenderBoxModelObject::moveChildrenTo(RenderBoxModelObject* toBoxModelObject
     }
 }
 
+void RenderBoxModelObject::moveAllChildrenToInternal(RenderElement& newParent)
+{
+    while (firstChild())
+        newParent.attachRendererInternal(detachRendererInternal(*firstChild()), this);
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/RenderBoxModelObject.h b/Source/WebCore/rendering/RenderBoxModelObject.h
@@ -290,6 +290,7 @@ class RenderBoxModelObject : public RenderLayerModelObject {
     {
         moveChildrenTo(toBoxModelObject, firstChild(), nullptr, beforeChild, normalizeAfterInsertion);
     }
+    void moveAllChildrenToInternal(RenderElement& newParent);
     // Move all of the kids from |startChild| up to but excluding |endChild|. 0 can be passed as the |endChild| to denote
     // that all the kids from |startChild| onwards should be moved.
     void moveChildrenTo(RenderBoxModelObject* toBoxModelObject, RenderObject* startChild, RenderObject* endChild, NormalizeAfterInsertion normalizeAfterInsertion)

diff --git a/Source/WebCore/rendering/RenderButton.h b/Source/WebCore/rendering/RenderButton.h
@@ -42,7 +42,6 @@ class RenderButton final : public RenderFlexibleBox {
     bool canBeSelectionLeaf() const override;
 
     RenderPtr<RenderObject> takeChild(RenderTreeBuilder&, RenderObject&) override;
-    void removeLeftoverAnonymousBlock(RenderBlock*) override { }
     bool createsAnonymousWrapper() const override { return true; }
 
     void updateFromElement() override;

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
@@ -499,6 +499,51 @@ void RenderElement::destroyLeftoverChildren()
     }
 }
 
+RenderObject* RenderElement::attachRendererInternal(RenderPtr<RenderObject> child, RenderObject* beforeChild)
+{
+    child->setParent(this);
+
+    if (m_firstChild == beforeChild)
+        m_firstChild = child.get();
+
+    if (beforeChild) {
+        auto* previousSibling = beforeChild->previousSibling();
+        if (previousSibling)
+            previousSibling->setNextSibling(child.get());
+        child->setPreviousSibling(previousSibling);
+        child->setNextSibling(beforeChild);
+        beforeChild->setPreviousSibling(child.get());
+        return child.release();
+    }
+    if (m_lastChild)
+        m_lastChild->setNextSibling(child.get());
+    child->setPreviousSibling(m_lastChild);
+    m_lastChild = child.get();
+    return child.release();
+}
+
+RenderPtr<RenderObject> RenderElement::detachRendererInternal(RenderObject& renderer)
+{
+    auto* parent = renderer.parent();
+    ASSERT(parent);
+    auto* nextSibling = renderer.nextSibling();
+
+    if (renderer.previousSibling())
+        renderer.previousSibling()->setNextSibling(nextSibling);
+    if (nextSibling)
+        nextSibling->setPreviousSibling(renderer.previousSibling());
+
+    if (parent->firstChild() == &renderer)
+        parent->m_firstChild = nextSibling;
+    if (parent->lastChild() == &renderer)
+        parent->m_lastChild = renderer.previousSibling();
+
+    renderer.setPreviousSibling(nullptr);
+    renderer.setNextSibling(nullptr);
+    renderer.setParent(nullptr);
+    return RenderPtr<RenderObject>(&renderer);
+}
+
 void RenderElement::insertChildInternal(RenderPtr<RenderObject> newChildPtr, RenderObject* beforeChild)
 {
     RELEASE_ASSERT_WITH_MESSAGE(!view().frameView().layoutContext().layoutState(), "Layout must not mutate render tree");
@@ -514,26 +559,7 @@ void RenderElement::insertChildInternal(RenderPtr<RenderObject> newChildPtr, Ren
     ASSERT(!is<RenderText>(beforeChild) || !downcast<RenderText>(*beforeChild).inlineWrapperForDisplayContents());
 
     // Take the ownership.
-    auto* newChild = newChildPtr.release();
-
-    newChild->setParent(this);
-
-    if (m_firstChild == beforeChild)
-        m_firstChild = newChild;
-
-    if (beforeChild) {
-        RenderObject* previousSibling = beforeChild->previousSibling();
-        if (previousSibling)
-            previousSibling->setNextSibling(newChild);
-        newChild->setPreviousSibling(previousSibling);
-        newChild->setNextSibling(beforeChild);
-        beforeChild->setPreviousSibling(newChild);
-    } else {
-        if (lastChild())
-            lastChild()->setNextSibling(newChild);
-        newChild->setPreviousSibling(lastChild());
-        m_lastChild = newChild;
-    }
+    auto* newChild = attachRendererInternal(WTFMove(newChildPtr), beforeChild);
 
     newChild->initializeFragmentedFlowStateOnInsertion();
     if (!renderTreeBeingDestroyed()) {
@@ -599,34 +625,19 @@ RenderPtr<RenderObject> RenderElement::takeChildInternal(RenderObject& oldChild)
     // 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.
-
-    RenderObject* nextSibling = oldChild.nextSibling();
-
-    if (oldChild.previousSibling())
-        oldChild.previousSibling()->setNextSibling(nextSibling);
-    if (nextSibling)
-        nextSibling->setPreviousSibling(oldChild.previousSibling());
-
-    if (m_firstChild == &oldChild)
-        m_firstChild = nextSibling;
-    if (m_lastChild == &oldChild)
-        m_lastChild = oldChild.previousSibling();
-
-    oldChild.setPreviousSibling(nullptr);
-    oldChild.setNextSibling(nullptr);
-    oldChild.setParent(nullptr);
+    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>(oldChild))
-        RenderCounter::rendererRemovedFromTree(downcast<RenderElement>(oldChild));
+    if (!renderTreeBeingDestroyed() && is<RenderElement>(*childToTake))
+        RenderCounter::rendererRemovedFromTree(downcast<RenderElement>(*childToTake));
 
     if (!renderTreeBeingDestroyed()) {
         if (AXObjectCache* cache = document().existingAXObjectCache())
             cache->childrenChanged(this);
     }
 
-    return RenderPtr<RenderObject>(&oldChild);
+    return childToTake;
 }
 
 RenderBlock* RenderElement::containingBlockForFixedPosition() const

diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
@@ -229,6 +229,8 @@ class RenderElement : public RenderObject {
     void setIsFirstLetter() { m_isFirstLetter = true; }
 
     void destroyLeftoverChildren();
+    RenderObject* attachRendererInternal(RenderPtr<RenderObject> child, RenderObject* beforeChild);
+    RenderPtr<RenderObject> detachRendererInternal(RenderObject&);
 
 protected:
     enum BaseTypeFlag {

diff --git a/Source/WebCore/rendering/RenderRuby.h b/Source/WebCore/rendering/RenderRuby.h
@@ -85,7 +85,6 @@ class RenderRubyAsBlock final : public RenderBlockFlow {
     bool isRubyBlock() const final { return true; }
     const char* renderName() const override { return "RenderRuby (block)"; }
     bool createsAnonymousWrapper() const override { return true; }
-    void removeLeftoverAnonymousBlock(RenderBlock*) override { ASSERT_NOT_REACHED(); }
 };
 
 

diff --git a/Source/WebCore/rendering/RenderRubyRun.h b/Source/WebCore/rendering/RenderRubyRun.h
@@ -78,7 +78,6 @@ class RenderRubyRun final : public RenderBlockFlow {
     bool isRubyRun() const override { return true; }
     const char* renderName() const override { return "RenderRubyRun (anonymous)"; }
     bool createsAnonymousWrapper() const override { return true; }
-    void removeLeftoverAnonymousBlock(RenderBlock*) override { }
 
 private:
     UChar m_lastCharacter;

diff --git a/Source/WebCore/rendering/RenderTextControl.h b/Source/WebCore/rendering/RenderTextControl.h
@@ -74,7 +74,6 @@ class RenderTextControl : public RenderBlockFlow {
     bool isTextControl() const final { return true; }
     void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
     void computePreferredLogicalWidths() override;
-    void removeLeftoverAnonymousBlock(RenderBlock*) override { }
     bool avoidsFloats() const override { return true; }
     bool canHaveGeneratedChildren() const override { return false; }
 

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp
@@ -26,8 +26,12 @@
 #include "config.h"
 #include "RenderTreeBuilderBlock.h"
 
+#include "RenderButton.h"
 #include "RenderChildIterator.h"
 #include "RenderFullScreen.h"
+#include "RenderRuby.h"
+#include "RenderRubyRun.h"
+#include "RenderTextControl.h"
 
 namespace WebCore {
 
@@ -193,16 +197,35 @@ void RenderTreeBuilder::Block::insertChildIgnoringContinuation(RenderBlock& pare
     parent.RenderBox::addChild(m_builder, WTFMove(child), beforeChild);
 
     if (madeBoxesNonInline && is<RenderBlock>(parent.parent()) && parent.isAnonymousBlock())
-        downcast<RenderBlock>(*parent.parent()).removeLeftoverAnonymousBlock(&parent);
+        removeLeftoverAnonymousBlock(parent);
     // parent object may be dead here
 }
 
 void RenderTreeBuilder::Block::childBecameNonInline(RenderBlock& parent, RenderElement&)
 {
     m_builder.makeChildrenNonInline(parent);
     if (parent.isAnonymousBlock() && is<RenderBlock>(parent.parent()))
-        downcast<RenderBlock>(*parent.parent()).removeLeftoverAnonymousBlock(&parent);
+        removeLeftoverAnonymousBlock(parent);
     // parent may be dead here
 }
 
+void RenderTreeBuilder::Block::removeLeftoverAnonymousBlock(RenderBlock& anonymousBlock)
+{
+    ASSERT(anonymousBlock.isAnonymousBlock());
+    ASSERT(!anonymousBlock.childrenInline());
+    ASSERT(anonymousBlock.parent());
+
+    if (anonymousBlock.continuation())
+        return;
+
+    auto* parent = anonymousBlock.parent();
+    if (is<RenderButton>(*parent) || is<RenderTextControl>(*parent) || is<RenderRubyAsBlock>(*parent) || is<RenderRubyRun>(*parent))
+        return;
+
+    // FIXME: This should really just be a moveAllChilrenTo (see webkit.org/b/182495)
+    anonymousBlock.moveAllChildrenToInternal(*parent);
+    auto toBeDestroyed = parent->takeChildInternal(anonymousBlock);
+    // anonymousBlock is dead here.
+}
+
 }
diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h b/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h
@@ -40,6 +40,7 @@ class RenderTreeBuilder::Block {
 
 private:
     void insertChildToContinuation(RenderBlock& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild);
+    void removeLeftoverAnonymousBlock(RenderBlock& anonymousBlock);
 
     RenderTreeBuilder& m_builder;
 };