1856640 - Investigate WPT container-queries/font-relative-units.html intermittents

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Shin[:dshin]]

Currently annotated as known intermittent failures.

Discovered during Bug 1310170. On the opt build, there's a pretty good chance that all of them will fail, but sometimes they pass.

Comment 1

[David Shin[:dshin]]

(Running into this while implementing bug 1856640)
For the case where every tests fail, during the first styling, Gecko_GetQueryContainerSize returns early with no primary frame, so the container ends up with a size container with unknown width & height.
During getComputedStyle(..).color we don't particularly flush style, so we fail the test.
Second styling happens after the test fails, so subsequent getComputedStyle(..).color shows the intended green color.

Comment 2

[David Shin[:dshin]]

(Just taking the em relative inline-size test of the overall suite)

1. Stylesheet for Ahem font starts loading @import url("/fonts/ahem.css");. Layout will not start til all pending sheets are loaded.
2. Styling starts
3. In styling #em_test, we test the query container condition for width.
4. It finds that #em_container is a inlne-size container, so the search stops and its information is queried.
5. We hit the early return in Gecko_GetQueryContainerSize because the layout hasn't started and the frames aren't constructed
6. Container condition does not match.
7. Test runs, it has no reason to flush layout (Or style), so it fails
8. Stylesheet load from 1) completes, layout starts

Comment 3

[David Shin[:dshin]]

(Corrected sequence of events)

1. Stylesheet for Ahem font starts loading @import url("/fonts/ahem.css");.
2. Stylesheet completes loading, PresShell starts initializing
3. Layout starts, calling down to nsCSSFrameConstructor::ConstructDocElementFrame
4. Styling happens
5. Since no primary frame is constructed, all queries fail
6. Frames are constructed
7. Reflow is triggered, through InsertAnonymousContentInContainer - note that it's a frame flush, not a layout flush.
8. Container query happens - it has a frame, but the width of the container is 0, so the container query condition fails, again.
9. Test happens and fails
10. Restyle happens at refresh driver tick, with an interruptible layout flush.
11. Container condition query succeeds

Comment 4

[David Shin[:dshin]]

Attachment: Bug 1856640: Ensure layout flush for as-of-yet reflowed frames when querying computed style of elements potentially matching container query. r=#style

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dcd15392aae
Ensure layout flush for as-of-yet reflowed frames when querying computed style of elements potentially matching container query. r=firefox-style-system-reviewers,emilio

Comment 6

[Cristian Tuns]

Backed out for causing build bustages with HasUnreflowedContainerQueryAncestor()

• Backout link
• Push with failures
• Failure Log
• Failure line: ld64.lld: error: undefined symbol: nsIFrame::HasUnreflowedContainerQueryAncestor() const

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e78502740130
Ensure layout flush for as-of-yet reflowed frames when querying computed style of elements potentially matching container query. r=firefox-style-system-reviewers,emilio

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e78502740130

Comment 9

[Andra Esanu (needinfo me)]

(In reply to Cristian Tuns from comment #6)

Backed out for causing build bustages with HasUnreflowedContainerQueryAncestor()

• Backout link
• Push with failures
• Failure Log
• Failure line: ld64.lld: error: undefined symbol: nsIFrame::HasUnreflowedContainerQueryAncestor() const

== Change summary for alert #41670 (as of Tue, 05 Mar 2024 11:01:32 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
15%	imgur loadtime	windows10-64-shippable-qr	cold fission webrender	1,969.94 -> 2,273.90	Before/After
3%	imgur LastVisualChange	windows10-64-shippable-qr	cold fission webrender	7,470.24 -> 7,663.06	Before/After

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
7%	microsoft FirstVisualChange	windows10-64-shippable-qr	fission warm webrender	177.98 -> 166.13
6%	microsoft ContentfulSpeedIndex	windows10-64-shippable-qr	fission warm webrender	178.61 -> 167.42
6%	microsoft PerceptualSpeedIndex	windows10-64-shippable-qr	fission warm webrender	186.38 -> 174.73
3%	imgur loadtime	windows10-64-shippable-qr	fission warm webrender	485.05 -> 472.37
2%	imgur fcp	windows10-64-shippable-qr	fission warm webrender	425.82 -> 416.03

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=41670

Comment 10

[David Shin[:dshin]]

Perf impact is evaluated on the backout build, so not sure how valid that impact is?
Either way, I've taken a look at the graph for the two regressing tests, and I don't think I see any step-wise increase around this change landed & stuck.