[webkit-reviews] review granted: [Bug 182762] [RenderTreeBuilder] Move RenderElement::takeChild() to RenderTreeBuilder : [Attachment 333760] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 13 23:09:39 PST 2018

Daniel Bates <dbates at webkit.org> has granted zalan <zalan at apple.com>'s request
for review:
Bug 182762: [RenderTreeBuilder] Move RenderElement::takeChild() to

Attachment 333760: Patch


--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 333760
  --> https://bugs.webkit.org/attachment.cgi?id=333760

View in context: https://bugs.webkit.org/attachment.cgi?id=333760&action=review

> Source/WebCore/ChangeLog:9
> +	   This patch removes the remaining takeChild() related mutation logic
from the renderers.


> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:571
> +RenderPtr<RenderObject>
RenderTreeBuilder::takeChildFromRenderElement(RenderElement& parent,
RenderObject& child)

The order of the parameters does not match the order implied by the name of
this function. We should swap the parameters such that this function takes the
child as the first argument and the parent as the second.

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:586
> +	   // We only repaint |oldChild| if we have a RenderLayer as its visual
overflow may not be tracked by its parent.

oldChild => child

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:602
> +    // If oldChild is the start or end of the selection, then clear the
selection to


> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:612
> +    // WARNING: There should be no code running between
willBeRemovedFromTree and the actual removal below.

willBeRemovedFromTree => willBeRemovedFromTree()

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:613
> +    // This is needed to avoid race conditions where willBeRemovedFromTree
would dirty the tree's structure


> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:614
> +    // and the code running here would force an untimely rebuilding, leaving
|oldChild| dangling.

oldChild => child

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:617
> +    // rendererRemovedFromTree walks the whole subtree. We can improve

rendererRemovedFromTree => rendererRemovedFromTree()

More information about the webkit-reviews mailing list