Bug 94843 – Nuke markBoxForRelayoutAfterSplit

• Status: NEW
• Component: Layout and Rendering

[Julien Chaffraix]

This function makes me sad, mostly because of the FIXME:

static void markBoxForRelayoutAfterSplit(RenderBox* box)
{
    // FIXME: The table code should handle that automatically. If not,
    // we should fix it and remove the table part checks.
    if (box->isTable()) {
        // Because we may have added some sections with already computed column structures, we need to
        // sync the table structure with them now. This avoids crashes when adding new cells to the table.
        toRenderTable(box)->forceSectionsRecalc();
    } else if (box->isTableSection())
        toRenderTableSection(box)->setNeedsCellRecalc();

    box->setNeedsLayoutAndPrefWidthsRecalc();
}

Now that we have introduced willBeRemovedFromTree / insertedIntoTree we should be able to remove it from the tree and rely on the previous notifications instead.

[Julien Chaffraix]

Created attachment 170051 [details]
Proposed removal.

[Build Bot]

Comment on attachment 170051 [details]
Proposed removal.

Attachment 170051 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14486766

New failing tests:
fast/invalid/018.html
fast/invalid/017.html
fast/invalid/residual-style.html

[WebKit Review Bot]

Comment on attachment 170051 [details]
Proposed removal.

Attachment 170051 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14482792

New failing tests:
fast/invalid/018.html
fast/invalid/017.html
fast/invalid/residual-style.html

[Eric Seidel (no email)]

Master Mitz to the rescue!

[Julien Chaffraix]

Created attachment 170191 [details]
Update change: same fix, added some missing rebaselines.

[Julien Chaffraix]

Created attachment 170290 [details]
Updated TestExpectations.

[Abhishek Arya]

Comment on attachment 170290 [details]
Updated TestExpectations.

1. We probably need RenderTableCell::insertedIntoTree ?
2. Are those layouttest changes mostly because we calculate correct border sizes now ?
3. You are our table perf expert, did you run any benchmark to make sure we are not regressing. markBoxForRelayoutAfterSplit code was rarely used, but this one is generic that i am scared if it affects overall table performance.

[Ahmad Saleem]

We still have 'markBoxForRelayoutAfterSplit':

https://github.com/WebKit/WebKit/blob/60ef17eec70203dc4f2d1aa60517321cd57e0f44/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp#L1038

https://github.com/WebKit/WebKit/blob/60ef17eec70203dc4f2d1aa60517321cd57e0f44/Source/WebCore/rendering/updating/RenderTreeBuilder.h#L63

https://github.com/WebKit/WebKit/blob/60ef17eec70203dc4f2d1aa60517321cd57e0f44/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp#L307

It seems that other usage has already been nuked. Though this FIXME still exist:

// FIXME: The table code should handle that automatically. If not,
    // we should fix it and remove the table part checks.