1702401 - {inc} WPT tests dynamic-baseline-change.html and dynamic-baseline-change-nested.html fail in Firefox

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Daniel Holbert [:dholbert]]

Live versions of tests:
https://wpt.live/css/css-flexbox/dynamic-baseline-change.html
https://wpt.live/css/css-flexbox/dynamic-baseline-change-nested.html

Reference case (for both):
https://wpt.live/css/css-flexbox/dynamic-baseline-change-ref.html

Pass/Fail dashboard:
https://wpt.fyi/results/css/css-flexbox/dynamic-baseline-change.html
https://wpt.fyi/results/css/css-flexbox/dynamic-baseline-change-nested.html

These test failures are pointing to a real bug.

These tests have a dynamic change, which we fail to fully account for in our incremental layout, so the testcase's rendering ends up a bit too short. If I change the width of my browser viewport, that causes enough layout-invalidation damage to force us to re-render correctly, though.

(Note: this test is currently not part of the compat2021 test list, so I'm not flagging it as blocking that metabug.)

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1702401 - Reject existing CachedBAxisMeasurement if the item's subtree is dirty. r?dholbert

When a flex item's subtree is dirty, we should reject the cached
measurement and perform a measuring reflow for the item to get the
correct ascent. Without this patch, we are going to call
FlexItem::ResolvedAscent() in the flex algorithm based on a dirty flex
item subtree, and get a wrong ascent. (Although we'll still perform the
final reflow for the item, it's too late to get the correct ascent to
compute flex container's ascent.)

With the above modification, it exposes an existing issue that
layout/generic/crashtests/1666592.html can now trigger
MOZ_ASSERT(!mFinalReflowSize) in UpdateFinalReflowSize() in
fragmentation scenario. The problem is: when we clone a FlexItem for a
child frame in a flex container's continuation via FlexItem::CloneFor(),
we didn't reset the mHadMeasuringReflow flag, so we wrongly assume it
had a measuring reflow and may update its final reflow size based on the
cached metrics. However, we never run measuring reflow for flex items in
flex container's continuation, so we should reset the flag to prevent it
from falling into wrong path in FlexItem::NeedsFinalReflow().

Depends on D121404

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebbc084701a4
Reject existing CachedBAxisMeasurement if the item's subtree is dirty. r=dholbert

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing reftest failures in flexbox-align-self-baseline-horiz

Backout link: https://hg.mozilla.org/integration/autoland/rev/eba83205abe13ff5abac9b31661b089464aeebcd

Push with failures

Failure log

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

The reftest failures is caused by the patch in bug 1686961. I'll land the patch in this bug independently.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a11c2bd2de51
Reject existing CachedBAxisMeasurement if the item's subtree is dirty. r=dholbert

Comment 7

[Cosmin Sabou [:CosminS]]

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