Bug 180964 – [RenderTreeBuilder] Move finding-the-parent/creating-wrapper logic from RenderTableRow::addChild to RenderTreeBuilder

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

Move mutation under RenderTreeBuilder.

[Radar WebKit Bug Importer]

<rdar://problem/36123315>

[zalan]

Created attachment 329731 [details]
Patch

[Antti Koivisto]

Comment on attachment 329731 [details]
Patch

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

> Source/WebCore/rendering/RenderTableRow.cpp:-117
>  void RenderTableRow::addChild(RenderTreeBuilder& builder, RenderPtr<RenderObject> child, RenderObject* beforeChild)
>  {
> -    if (!is<RenderTableCell>(*child)) {
> -        RenderObject* last = beforeChild;

Why not move the entire addChild?

> Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:34
> +#include "RenderTableRowBuilder.h"

I have been using file name pattern like RenderTreeBuilderTableRow.h/.cpp for inner classes like RenderTreeBuilder::TableRow. It makes easy to see what is really part of the RenderTreeBuilder.

[Antti Koivisto]

Also I think we could just have RenderTreeBuilder::Table for all table related code.

[zalan]

Created attachment 329763 [details]
Patch

[zalan]

(In reply to Antti Koivisto from comment #3)
> > -    if (!is<RenderTableCell>(*child)) {
> > -        RenderObject* last = beforeChild;
> 
> Why not move the entire addChild?
splitAnonymousBoxesAroundChild() mutation is not table row specific and I am planning to cover that in a dedicated patch for all the different renderers.
The rest of the addChild() is not about tree mutation and we need to figure out where to move them (styleChanged, childInserted, etc)

[Antti Koivisto]

Comment on attachment 329763 [details]
Patch

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

> Source/WebCore/rendering/updating/RenderTreeBuilderTable.cpp:35
> +RenderElement& RenderTreeBuilder::Table::findOrCreateParentForChild(RenderTreeBuilder& builder, RenderTableRow& parent,

These could be member instances of RenderTreeBuilder similar to RenderTreeUpdater::GeneratedContent. That avoid having to pass the builder everywhere, just call m_builder.insertChild()

[zalan]

Created attachment 329771 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 329771 [details]
Patch

Clearing flags on attachment: 329771

Committed r226127: <https://trac.webkit.org/changeset/226127>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.