Skip to content
Permalink
Browse files
RenderElement::removeChild() should take child as a reference.
<https://webkit.org/b/122888>

We can't remove a child without a child to remove.

Reviewed by Antti Koivisto.


Canonical link: https://commits.webkit.org/140979@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157515 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Andreas Kling committed Oct 16, 2013
1 parent 9d80c8d commit 52a583cf9f0624b6df77814fd0b062423943dace
Showing with 138 additions and 131 deletions.
  1. +9 −0 Source/WebCore/ChangeLog
  2. +11 −11 Source/WebCore/rendering/RenderBlock.cpp
  3. +1 −1 Source/WebCore/rendering/RenderBlock.h
  4. +2 −2 Source/WebCore/rendering/RenderBoxModelObject.cpp
  5. +4 −4 Source/WebCore/rendering/RenderButton.cpp
  6. +1 −1 Source/WebCore/rendering/RenderButton.h
  7. +5 −5 Source/WebCore/rendering/RenderCounter.cpp
  8. +1 −1 Source/WebCore/rendering/RenderCounter.h
  9. +26 −26 Source/WebCore/rendering/RenderElement.cpp
  10. +2 −2 Source/WebCore/rendering/RenderElement.h
  11. +5 −5 Source/WebCore/rendering/RenderInline.cpp
  12. +2 −2 Source/WebCore/rendering/RenderMenuList.cpp
  13. +1 −1 Source/WebCore/rendering/RenderMenuList.h
  14. +2 −2 Source/WebCore/rendering/RenderObject.cpp
  15. +24 −24 Source/WebCore/rendering/RenderRuby.cpp
  16. +2 −2 Source/WebCore/rendering/RenderRuby.h
  17. +5 −5 Source/WebCore/rendering/RenderRubyRun.cpp
  18. +1 −1 Source/WebCore/rendering/RenderRubyRun.h
  19. +16 −18 Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp
  20. +4 −4 Source/WebCore/rendering/mathml/RenderMathMLScripts.h
  21. +2 −2 Source/WebCore/rendering/svg/RenderSVGContainer.cpp
  22. +1 −1 Source/WebCore/rendering/svg/RenderSVGContainer.h
  23. +3 −3 Source/WebCore/rendering/svg/RenderSVGInline.cpp
  24. +1 −1 Source/WebCore/rendering/svg/RenderSVGInline.h
  25. +2 −2 Source/WebCore/rendering/svg/RenderSVGRoot.cpp
  26. +1 −1 Source/WebCore/rendering/svg/RenderSVGRoot.h
  27. +3 −3 Source/WebCore/rendering/svg/RenderSVGText.cpp
  28. +1 −1 Source/WebCore/rendering/svg/RenderSVGText.h
@@ -1,3 +1,12 @@
2013-10-16 Andreas Kling <akling@apple.com>

RenderElement::removeChild() should take child as a reference.
<https://webkit.org/b/122888>

We can't remove a child without a child to remove.

Reviewed by Antti Koivisto.

2013-10-16 Antti Koivisto <antti@apple.com>

Move test for contained caret offset to RenderTextLineBoxes
@@ -1045,9 +1045,9 @@ static bool canMergeAnonymousBlock(RenderBlock* anonymousBlock)
return true;
}

static bool canMergeContiguousAnonymousBlocks(RenderObject* oldChild, RenderObject* previous, RenderObject* next)
static bool canMergeContiguousAnonymousBlocks(RenderObject& oldChild, RenderObject* previous, RenderObject* next)
{
if (oldChild->documentBeingDestroyed() || oldChild->isInline() || oldChild->virtualContinuation())
if (oldChild.documentBeingDestroyed() || oldChild.isInline() || oldChild.virtualContinuation())
return false;

if (previous) {
@@ -1085,7 +1085,7 @@ void RenderBlock::collapseAnonymousBoxChild(RenderBlock* parent, RenderBlock* ch
RenderFlowThread* childFlowThread = child->flowThreadContainingBlock();
CurrentRenderFlowThreadMaintainer flowThreadMaintainer(childFlowThread);

parent->removeChildInternal(child, child->hasLayer() ? NotifyChildren : DontNotifyChildren);
parent->removeChildInternal(*child, child->hasLayer() ? NotifyChildren : DontNotifyChildren);
child->moveAllChildrenTo(parent, nextSibling, child->hasLayer());
// Delete the now-empty block's lines and nuke it.
child->deleteLineBoxTree();
@@ -1094,7 +1094,7 @@ void RenderBlock::collapseAnonymousBoxChild(RenderBlock* parent, RenderBlock* ch
child->destroy();
}

void RenderBlock::removeChild(RenderObject* oldChild)
void RenderBlock::removeChild(RenderObject& oldChild)
{
// No need to waste time in merging or removing empty anonymous blocks.
// We can just bail out if our document is getting destroyed.
@@ -1109,8 +1109,8 @@ void RenderBlock::removeChild(RenderObject* oldChild)
// If this child is a block, and if our previous and next siblings are
// both anonymous blocks with inline content, then we can go ahead and
// fold the inline content back together.
RenderObject* prev = oldChild->previousSibling();
RenderObject* next = oldChild->nextSibling();
RenderObject* prev = oldChild.previousSibling();
RenderObject* next = oldChild.nextSibling();
bool canMergeAnonymousBlocks = canMergeContiguousAnonymousBlocks(oldChild, prev, next);
if (canMergeAnonymousBlocks && prev && next) {
prev->setNeedsLayoutAndPrefWidthsRecalc();
@@ -1130,7 +1130,7 @@ void RenderBlock::removeChild(RenderObject* oldChild)
// Cache this value as it might get changed in setStyle() call.
bool inlineChildrenBlockHasLayer = inlineChildrenBlock->hasLayer();
inlineChildrenBlock->setStyle(newStyle);
removeChildInternal(inlineChildrenBlock, inlineChildrenBlockHasLayer ? NotifyChildren : DontNotifyChildren);
removeChildInternal(*inlineChildrenBlock, inlineChildrenBlockHasLayer ? NotifyChildren : DontNotifyChildren);

// Now just put the inlineChildrenBlock inside the blockChildrenBlock.
RenderObject* beforeChild = prev == inlineChildrenBlock ? blockChildrenBlock->firstChild() : nullptr;
@@ -1183,7 +1183,7 @@ void RenderBlock::removeChild(RenderObject* oldChild)

// If we are an empty anonymous block in the continuation chain,
// we need to remove ourself and fix the continuation chain.
if (!beingDestroyed() && isAnonymousBlockContinuation() && !oldChild->isListMarker()) {
if (!beingDestroyed() && isAnonymousBlockContinuation() && !oldChild.isListMarker()) {
auto containingBlockIgnoringAnonymous = containingBlock();
while (containingBlockIgnoringAnonymous && containingBlockIgnoringAnonymous->isAnonymousBlock())
containingBlockIgnoringAnonymous = containingBlockIgnoringAnonymous->containingBlock();
@@ -5032,7 +5032,7 @@ void RenderBlock::updateFirstLetterStyle(RenderObject* firstLetterBlock, RenderO
while (RenderObject* child = firstLetter->firstChild()) {
if (child->isText())
toRenderText(child)->removeAndDestroyTextBoxes();
firstLetter->removeChild(child);
firstLetter->removeChild(*child);
newFirstLetter->addChild(child, 0);
}

@@ -5045,7 +5045,7 @@ void RenderBlock::updateFirstLetterStyle(RenderObject* firstLetterBlock, RenderO
}
// To prevent removal of single anonymous block in RenderBlock::removeChild and causing
// |nextSibling| to go stale, we remove the old first letter using removeChildNode first.
firstLetterContainer->removeChildInternal(firstLetter, NotifyChildren);
firstLetterContainer->removeChildInternal(*firstLetter, NotifyChildren);
firstLetter->destroy();
firstLetter = newFirstLetter;
firstLetterContainer->addChild(firstLetter, nextSibling);
@@ -5105,7 +5105,7 @@ void RenderBlock::createFirstLetterRenderer(RenderObject* firstLetterBlock, Rend
remainingText->textNode()->setRenderer(remainingText);

firstLetterContainer->addChild(remainingText, currentTextChild);
firstLetterContainer->removeChild(currentTextChild);
firstLetterContainer->removeChild(*currentTextChild);
remainingText->setFirstLetter(firstLetter);
firstLetter->setFirstLetterRemainingText(remainingText);

@@ -120,7 +120,7 @@ class RenderBlock : public RenderBox {
virtual void deleteLineBoxTree();

virtual void addChild(RenderObject* newChild, RenderObject* beforeChild = 0) OVERRIDE;
virtual void removeChild(RenderObject*) OVERRIDE;
virtual void removeChild(RenderObject&) OVERRIDE;

virtual void layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeight = 0);

@@ -2791,11 +2791,11 @@ void RenderBoxModelObject::moveChildTo(RenderBoxModelObject* toBoxModelObject, R
if (fullRemoveInsert && (toBoxModelObject->isRenderBlock() || toBoxModelObject->isRenderInline())) {
// Takes care of adding the new child correctly if toBlock and fromBlock
// have different kind of children (block vs inline).
removeChildInternal(child, NotifyChildren);
removeChildInternal(*child, NotifyChildren);
toBoxModelObject->addChild(child, beforeChild);
} else {
NotifyChildrenType notifyType = fullRemoveInsert ? NotifyChildren : DontNotifyChildren;
removeChildInternal(child, notifyType);
removeChildInternal(*child, notifyType);
toBoxModelObject->insertChildInternal(child, beforeChild, notifyType);
}
}
@@ -73,15 +73,15 @@ void RenderButton::addChild(RenderObject* newChild, RenderObject* beforeChild)
m_inner->addChild(newChild, beforeChild);
}

void RenderButton::removeChild(RenderObject* oldChild)
void RenderButton::removeChild(RenderObject& oldChild)
{
// m_inner should be the only child, but checking for direct children who
// are not m_inner prevents security problems when that assumption is
// violated.
if (oldChild == m_inner || !m_inner || oldChild->parent() == this) {
ASSERT(oldChild == m_inner || !m_inner);
if (&oldChild == m_inner || !m_inner || oldChild.parent() == this) {
ASSERT(&oldChild == m_inner || !m_inner);
RenderFlexibleBox::removeChild(oldChild);
m_inner = 0;
m_inner = nullptr;
} else
m_inner->removeChild(oldChild);
}
@@ -43,7 +43,7 @@ class RenderButton FINAL : public RenderFlexibleBox {
virtual bool canBeSelectionLeaf() const OVERRIDE;

virtual void addChild(RenderObject* newChild, RenderObject *beforeChild = 0) OVERRIDE;
virtual void removeChild(RenderObject*) OVERRIDE;
virtual void removeChild(RenderObject&) OVERRIDE;
virtual void removeLeftoverAnonymousBlock(RenderBlock*) OVERRIDE { }
virtual bool createsAnonymousWrapper() const OVERRIDE { return true; }

@@ -502,16 +502,16 @@ void RenderCounter::destroyCounterNode(RenderObject* owner, const AtomicString&
// map associated with a renderer, so there is no risk in leaking the map.
}

void RenderCounter::rendererRemovedFromTree(RenderObject* renderer)
void RenderCounter::rendererRemovedFromTree(RenderObject& renderer)
{
if (!renderer->view().hasRenderCounters())
if (!renderer.view().hasRenderCounters())
return;
RenderObject* currentRenderer = renderer->lastLeafChild();
RenderObject* currentRenderer = renderer.lastLeafChild();
if (!currentRenderer)
currentRenderer = renderer;
currentRenderer = &renderer;
while (true) {
destroyCounterNodes(currentRenderer);
if (currentRenderer == renderer)
if (currentRenderer == &renderer)
break;
currentRenderer = currentRenderer->previousInPreOrder();
}
@@ -37,7 +37,7 @@ class RenderCounter FINAL : public RenderText {
static void destroyCounterNodes(RenderObject*);
static void destroyCounterNode(RenderObject*, const AtomicString& identifier);
static void rendererSubtreeAttached(RenderObject*);
static void rendererRemovedFromTree(RenderObject*);
static void rendererRemovedFromTree(RenderObject&);
static void rendererStyleChanged(RenderObject*, const RenderStyle* oldStyle, const RenderStyle* newStyle);

void updateCounter();
@@ -487,7 +487,7 @@ void RenderElement::addChild(RenderObject* newChild, RenderObject* beforeChild)
#endif
}

void RenderElement::removeChild(RenderObject* oldChild)
void RenderElement::removeChild(RenderObject& oldChild)
{
removeChildInternal(oldChild, NotifyChildren);
}
@@ -561,59 +561,59 @@ void RenderElement::insertChildInternal(RenderObject* newChild, RenderObject* be
cache->childrenChanged(this);
}

void RenderElement::removeChildInternal(RenderObject* oldChild, NotifyChildrenType notifyChildren)
void RenderElement::removeChildInternal(RenderObject& oldChild, NotifyChildrenType notifyChildren)
{
ASSERT(canHaveChildren() || canHaveGeneratedChildren());
ASSERT(oldChild->parent() == this);
ASSERT(oldChild.parent() == this);

if (oldChild->isFloatingOrOutOfFlowPositioned())
toRenderBox(oldChild)->removeFloatingOrPositionedChildFromBlockLists();
if (oldChild.isFloatingOrOutOfFlowPositioned())
toRenderBox(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 (!documentBeingDestroyed() && notifyChildren == NotifyChildren && oldChild->everHadLayout()) {
oldChild->setNeedsLayoutAndPrefWidthsRecalc();
if (!documentBeingDestroyed() && notifyChildren == NotifyChildren && 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())
if (oldChild.isBody())
view().repaintRootContents();
else
oldChild->repaint();
oldChild.repaint();
}

// If we have a line box wrapper, delete it.
if (oldChild->isBox())
toRenderBox(oldChild)->deleteLineBoxWrapper();
else if (oldChild->isLineBreak())
toRenderLineBreak(oldChild)->deleteInlineBoxWrapper();
if (oldChild.isBox())
toRenderBox(oldChild).deleteLineBoxWrapper();
else if (oldChild.isLineBreak())
toRenderLineBreak(oldChild).deleteInlineBoxWrapper();

// If oldChild is the start or end of the selection, then clear the selection to
// avoid problems of invalid pointers.
// FIXME: The FrameSelection should be responsible for this when it
// is notified of DOM mutations.
if (!documentBeingDestroyed() && oldChild->isSelectionBorder())
if (!documentBeingDestroyed() && oldChild.isSelectionBorder())
view().clearSelection();

if (!documentBeingDestroyed() && notifyChildren == NotifyChildren)
oldChild->willBeRemovedFromTree();
oldChild.willBeRemovedFromTree();

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

if (oldChild->previousSibling())
oldChild->previousSibling()->setNextSibling(oldChild->nextSibling());
if (oldChild->nextSibling())
oldChild->nextSibling()->setPreviousSibling(oldChild->previousSibling());
if (oldChild.previousSibling())
oldChild.previousSibling()->setNextSibling(oldChild.nextSibling());
if (oldChild.nextSibling())
oldChild.nextSibling()->setPreviousSibling(oldChild.previousSibling());

if (m_firstChild == oldChild)
m_firstChild = oldChild->nextSibling();
if (m_lastChild == oldChild)
m_lastChild = oldChild->previousSibling();
if (m_firstChild == &oldChild)
m_firstChild = oldChild.nextSibling();
if (m_lastChild == &oldChild)
m_lastChild = oldChild.previousSibling();

oldChild->setPreviousSibling(0);
oldChild->setNextSibling(0);
oldChild->setParent(0);
oldChild.setPreviousSibling(nullptr);
oldChild.setNextSibling(nullptr);
oldChild.setParent(nullptr);

// rendererRemovedFromTree walks the whole subtree. We can improve performance
// by skipping this step when destroying the entire tree.
@@ -59,7 +59,7 @@ class RenderElement : public RenderObject {
virtual bool isChildAllowed(const RenderObject&, const RenderStyle&) const { return true; }
virtual void addChild(RenderObject* newChild, RenderObject* beforeChild = 0);
virtual void addChildIgnoringContinuation(RenderObject* newChild, RenderObject* beforeChild = 0) { return addChild(newChild, beforeChild); }
virtual void removeChild(RenderObject*);
virtual void removeChild(RenderObject&);

// The following functions are used when the render tree hierarchy changes to make sure layers get
// properly added and removed. Since containership can be implemented by any subclass, and since a hierarchy
@@ -71,7 +71,7 @@ class RenderElement : public RenderObject {

enum NotifyChildrenType { NotifyChildren, DontNotifyChildren };
void insertChildInternal(RenderObject*, RenderObject* beforeChild, NotifyChildrenType);
void removeChildInternal(RenderObject*, NotifyChildrenType);
void removeChildInternal(RenderObject&, NotifyChildrenType);

virtual RenderElement* hoverAncestor() const;

@@ -366,7 +366,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
while (o) {
RenderObject* tmp = o;
o = tmp->nextSibling();
removeChildInternal(tmp, NotifyChildren);
removeChildInternal(*tmp, NotifyChildren);
cloneInline->addChildIgnoringContinuation(tmp, 0);
tmp->setNeedsLayoutAndPrefWidthsRecalc();
}
@@ -408,7 +408,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
while (o) {
RenderObject* tmp = o;
o = tmp->nextSibling();
inlineCurr->removeChildInternal(tmp, NotifyChildren);
inlineCurr->removeChildInternal(*tmp, NotifyChildren);
cloneInline->addChildIgnoringContinuation(tmp, 0);
tmp->setNeedsLayoutAndPrefWidthsRecalc();
}
@@ -429,7 +429,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
while (o) {
RenderObject* tmp = o;
o = tmp->nextSibling();
fromBlock->removeChildInternal(tmp, NotifyChildren);
fromBlock->removeChildInternal(*tmp, NotifyChildren);
toBlock->insertChildInternal(tmp, nullptr, NotifyChildren);
}
}
@@ -474,7 +474,7 @@ void RenderInline::splitFlow(RenderObject* beforeChild, RenderBlock* newBlockBox
while (o) {
RenderObject* no = o;
o = no->nextSibling();
block->removeChildInternal(no, NotifyChildren);
block->removeChildInternal(*no, NotifyChildren);
pre->insertChildInternal(no, nullptr, NotifyChildren);
no->setNeedsLayoutAndPrefWidthsRecalc();
}
@@ -1275,7 +1275,7 @@ void RenderInline::childBecameNonInline(RenderObject* child)
RenderBoxModelObject* oldContinuation = continuation();
setContinuation(newBox);
RenderObject* beforeChild = child->nextSibling();
removeChildInternal(child, NotifyChildren);
removeChildInternal(*child, NotifyChildren);
splitFlow(beforeChild, newBox, child, oldContinuation);
}

@@ -144,9 +144,9 @@ void RenderMenuList::addChild(RenderObject* newChild, RenderObject* beforeChild)
cache->childrenChanged(this);
}

void RenderMenuList::removeChild(RenderObject* oldChild)
void RenderMenuList::removeChild(RenderObject& oldChild)
{
if (oldChild == m_innerBlock || !m_innerBlock) {
if (&oldChild == m_innerBlock || !m_innerBlock) {
RenderFlexibleBox::removeChild(oldChild);
m_innerBlock = 0;
} else
@@ -64,7 +64,7 @@ class RenderMenuList FINAL : public RenderFlexibleBox, private PopupMenuClient {
virtual bool isMenuList() const OVERRIDE { return true; }

virtual void addChild(RenderObject* newChild, RenderObject* beforeChild = 0) OVERRIDE;
virtual void removeChild(RenderObject*) OVERRIDE;
virtual void removeChild(RenderObject&) OVERRIDE;
virtual bool createsAnonymousWrapper() const OVERRIDE { return true; }

virtual void updateFromElement() OVERRIDE;
@@ -205,7 +205,7 @@ void RenderObject::setParent(RenderElement* parent)
void RenderObject::removeFromParent()
{
if (parent())
parent()->removeChild(this);
parent()->removeChild(*this);
}

RenderObject* RenderObject::nextInPreOrder() const
@@ -1651,7 +1651,7 @@ void RenderObject::handleDynamicFloatPositionChange()
// An anonymous block must be made to wrap this inline.
RenderBlock* block = toRenderBlock(parent())->createAnonymousBlock();
parent()->insertChildInternal(block, this, RenderElement::NotifyChildren);
parent()->removeChildInternal(this, RenderElement::NotifyChildren);
parent()->removeChildInternal(*this, RenderElement::NotifyChildren);
block->insertChildInternal(this, nullptr, RenderElement::NotifyChildren);
}
}

0 comments on commit 52a583c

Please sign in to comment.