Bug 182762 – [RenderTreeBuilder] Move RenderElement::takeChild() to RenderTreeBuilder

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

It means no more takeChild() mutation logic in the renderers.

[Radar WebKit Bug Importer]

<rdar://problem/37523756>

[zalan]

Created attachment 333760 [details]
Patch

[Daniel Bates]

Comment on attachment 333760 [details]
Patch

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.

Yay!

> 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

Ditto.

> 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

Ditto.

> 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 performance

rendererRemovedFromTree => rendererRemovedFromTree()

[zalan]

Created attachment 333803 [details]
Patch

[zalan]

> > 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.
Indeed. Since, at this point all the takeChild*() functions have this order, I'd rather address this issue in a separate patch.

[WebKit Commit Bot]

Comment on attachment 333803 [details]
Patch

Clearing flags on attachment: 333803

Committed r228464: <https://trac.webkit.org/changeset/228464>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.