Bug 247913 – Percent-width blocks cannot form a re-layout boundary

• Status: REOPENED
• Component: Layout and Rendering

[Ahmad Saleem]

Hi Team,

While going through Blink Commits, I came across another, which we should merge since this fixes another failing test:

Blink Commit - https://chromium.googlesource.com/chromium/src.git/+/da179e152dff0ab3e3dabc72299bd4fb586881c9

Webkit GitHub - https://github.com/WebKit/WebKit/blob/7ed06b348948ce6b702c93bcd2be63960cf56952/Source/WebCore/rendering/RenderObject.cpp#LL534

Failing Test (passes in Chrome Canary 110 & Firefox Nightly 108) - https://jsfiddle.net/8d47mtvL/show

Just wanted to create a bug to track for future purposes.

Note - Both Safari 16.1 and Safari Tech Preview 157 fail this.

Thanks!

[EWS]

Committed 256901@main (b42824bb6c99): <https://commits.webkit.org/256901@main>

Reviewed commits have been landed. Closing PR #6677 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/102577429>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 248967

[Ahmad Saleem]

@Alan - I noticed that I didn't did 'const' part in this merge for object().style(). Could this help in the performance issue in page loading?

I can do PR but is it possible to run it via some A/B testing internally to see whether 'const' part does not regress performance?

const auto* style = object->style();

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #4)
> @Alan - I noticed that I didn't did 'const' part in this merge for
> object().style(). Could this help in the performance issue in page loading?
> 
> I can do PR but is it possible to run it via some A/B testing internally to
> see whether 'const' part does not regress performance?
> 
> const auto* style = object->style();

This compiles:

    const auto& style = object->style();
    if (!style.width().isFixed() || !style.height().isFixed())
        return false;

[alan baradlay]

(In reply to Ahmad Saleem from comment #4)
> @Alan - I noticed that I didn't did 'const' part in this merge for
> object().style(). Could this help in the performance issue in page loading?
> 
> I can do PR but is it possible to run it via some A/B testing internally to
> see whether 'const' part does not regress performance?
> 
> const auto* style = object->style();
I am not sure if that's what the PLT regression is about. I think the expensive part of this change is where we can't (incorrectly) do boundary layout anymore on certain type of content (which means spending more time in layout). We probably need to find a different way of addressing this correctness issue.

[Ahmad Saleem]

Pull request: https://github.com/WebKit/WebKit/pull/41828

[EWS]

Committed 291607@main (8fafd9511671): <https://commits.webkit.org/291607@main>

Reviewed commits have been landed. Closing PR #41828 and removing active labels.

[Ahmad Saleem]

Reverted once again - https://commits.webkit.org/293192@main (Although I have more information now that it only happens on specific machine and specific test case).