1508420 - Add tests for dynamic reflow root handling

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Daniel Holbert [:dholbert]]

Filing this bug to add some tests to exercise the various special cases in bug 1159042.

In particular, ideally it'd be great to have a testcase that would fail without:

 - ...overflow getting correctly calculated/repainted on frames that are dynamic reflow roots (to exercise https://phabricator.services.mozilla.com/D9488 )

 - ...the needPaddingProp special case in https://phabricator.services.mozilla.com/D9492 (perhaps as a variant of testcases associated with https://bugzilla.mozilla.org/show_bug.cgi?id=157846#c108 )

 - ...as many as possible of the "canBeDynamicReflowRoot = false" special-cases in https://phabricator.services.mozilla.com/D9491

Feel free to chime in on other cases that need testing.

Comment 1

[Daniel Holbert [:dholbert]]

I thought of one other thing that we can't allow to become a dynamic reflow root:
 - flex/grid items with "align-self:baseline"

(because their internal contents determine their baseline, which then influences their positioning and the positioning of their baseline-aligned siblings)

Technically we should include this in bug 1159042, but I think it's also OK to address as a followup (and with a test), to avoid moving goalposts over there.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1508420: Add mochitest for conditions that disqualify a frame from becoming a dynamic reflow root.

Comment 3

[Daniel Holbert [:dholbert]]

WIP mochitest posted on phabricator. So far, it provides coverage for the width/height and {min,max}-{width,height} cases in https://searchfox.org/mozilla-central/source/layout/generic/ReflowInput.cpp#844

(i.e. if I comment out any of those checks, the mochitest fails and dumps a reftest-analyzer-friendly screenshot of a scenario where we produce the wrong result for an incremental reflow)

I'm planning on extending it to cover the rest of the "if" checks, and then I'll request review.

Comment 4

[Daniel Holbert [:dholbert]]

Try run looks good BTW:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=464a5b00771a0c53d4c0d6a057f4bd7852c2fc8a

(This is with the patch from phabricator, as of a few hours ago:
https://phabricator.services.mozilla.com/differential/diff/48609/ )

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b99e71654921
Add mochitest for conditions that disqualify a frame from becoming a dynamic reflow root. r=dbaron

Comment 6

[Cristina Coroiu [:ccoroiu]]

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