Bug 247913 - Percent-width blocks cannot form a re-layout boundary
Summary: Percent-width blocks cannot form a re-layout boundary
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: https://jsfiddle.net/y07soapn/show
Keywords: InRadar
Depends on: 248967
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-14 13:23 PST by Ahmad Saleem
Modified: 2023-04-03 07:59 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2022-11-14 13:23:31 PST
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!
Comment 1 EWS 2022-11-21 02:22:12 PST
Committed 256901@main (b42824bb6c99): <https://commits.webkit.org/256901@main>

Reviewed commits have been landed. Closing PR #6677 and removing active labels.
Comment 2 Radar WebKit Bug Importer 2022-11-21 02:23:15 PST
<rdar://problem/102577429>
Comment 3 WebKit Commit Bot 2022-12-08 11:53:53 PST
Re-opened since this is blocked by bug 248967
Comment 4 Ahmad Saleem 2023-04-03 05:47:01 PDT
@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();
Comment 5 Ahmad Saleem 2023-04-03 07:36:25 PDT
(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;
Comment 6 zalan 2023-04-03 07:59:07 PDT
(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.