Skip to content
Permalink
Browse files
[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.


Canonical link: https://commits.webkit.org/198548@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228464 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
alanbujtas committed Feb 14, 2018
1 parent 00e1364 commit b3a8b1f36d70b21ad4d2e5774caeb6fbae846cab
@@ -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 Antti Koivisto <antti@apple.com>

REGRESSION(r228313): Membuster | macOS | All Devices | 1.5 MB
@@ -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);
}
}
@@ -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();
@@ -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;

@@ -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:
@@ -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;
}


}
@@ -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);
@@ -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();
@@ -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);
}

@@ -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.

0 comments on commit b3a8b1f

Please sign in to comment.