Bug 237732

Summary: [css-grid] Grid contents sometimes layout with the wrong width
Product: WebKit Reporter: Matt Woodrow <mattwoodrow>
Component: Layout and RenderingAssignee: Matt Woodrow <mattwoodrow>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, obrufau, pdr, rego, sebastian.sams, simon.fraser, svillar, webkit-bug-importer, zalan, zsun
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://www.economist.com/the-economist-explains/2022/02/03/how-apples-privacy-push-cost-meta-10bn
See Also: https://bugs.webkit.org/show_bug.cgi?id=209282
Attachments:
Description Flags
Patch
none
Patch none

Description Matt Woodrow 2022-03-10 13:46:38 PST 
<rdar://88512506>

Resizing the browser on the provided URL sometimes lays out the main article text to the wrong width, where it doesn't line wrap at all and just extends off the right side of the page.

It looks like we call RenderGrid::computeIntrinsicLogicalWidths and that results in us calling layoutIfNeeded on the child RenderBlock with an estimated width (which turns out to be wrong).

Now that we've mutated the child and laid it out incorrectly, nothing marks the RenderGrid itself as needing layout, so we never do a proper layout with the final grid column sizes to fix this again.

The bug happens intermittently, since anytime we do a full layout of the grid, we also layout the children correctly.
Comment 1 Matt Woodrow 2022-03-13 14:16:16 PDT 
*** Bug 237710 has been marked as a duplicate of this bug. ***
Comment 2 Matt Woodrow 2022-03-13 14:17:25 PDT 
Bug 237710 has a reduced test case for this: https://bugs.webkit.org/attachment.cgi?id=454343

This is a regression from https://bugs.webkit.org/show_bug.cgi?id=232140
Comment 3 Matt Woodrow 2022-03-13 14:32:28 PDT 
It seems like we shouldn't be laying out children of a RenderGrid from (const!) computeIntrinsicLogicalWidths using estimated grid breadths.

Could we cache the intrinsic sizes when we do a full layout, and reuse those if we haven't marked the RenderGrid as needing layout?

A quick prototype of that seems to work.
Comment 4 Matt Woodrow 2022-03-13 18:53:49 PDT 
Looks like this showed up previously, but went away due to a change in the flex code. This is new variant of the same existing issue.

https://bugs.webkit.org/show_bug.cgi?id=209282
Comment 5 Matt Woodrow 2022-03-16 20:08:42 PDT 
Created attachment 454926 [details]
Patch
Comment 6 Javier Fernandez 2022-03-17 15:39:40 PDT 
Comment on attachment 454926 [details]
Patch

The change looks fine, but there are a few tests that regressed.
Comment 7 Matt Woodrow 2022-03-28 17:02:38 PDT 
Created attachment 455971 [details]
Patch
Comment 8 EWS 2022-03-29 16:05:31 PDT 
Committed r292079 (249006@main): <https://commits.webkit.org/249006@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455971 [details].