1499961 - RestyleManager's RecomputePosition may not move reflow roots

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

If a frame gets repositioned, RecomputePosition will bail out if that frame has a view or child with view, in which case a reflow is forced:
https://searchfox.org/mozilla-central/rev/eef79962ba73f7759fd74da658f6e5ceae0fc730/layout/base/RestyleManager.cpp#729-736

But if this frame is a reflow root, then the next reflow will only start at this root and therefore the frame will not actually be moved (as it is the job of its parent).

Passing nsChangeHint_ReflowChangesSizeOrPosition should be enough to reflow at least the parent of that potential root, and therefore handle the position change.

There are a couple more of these later in the function.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1499961 - Indicate position-change reflow hint when RecomputePosition() bails out - r?dbaron,heycam

If a frame cannot be repositioned, the reflow request should hint at the
position change, so that if that frame is a reflow root, the next reflow won't
just start from there and ignore the new position.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=206293785&revision=3e6a389c7d356ec0951fe17ac0f1aefa535997ac

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1499961 - Regression test for future movable reflow roots - r?dbaron

This test would fail with the upcoming bug 1159042 (dynamic reflow roots),
without the RecomputePosition fix in the next patch.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Notes:

The test still has a lot of boilerplate from test_intersectionObservers.html. I decided to keep it, to help with expanding the test later on, to cover more of the new reflow roots.

I've tested locally that:
- The test alone passes -- there are no fancy reflow roots yet.
- It passes with the RecomputePosition patch, as there's nothing to fix yet, but at least it's not worse. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7f7ea7b6c300928c37308f20b007a8b018d271e
- It fails on top of WIP bug 1159042, proving that it catches the issue.
- It passes again with both bug 1159042 and the RecomputePosition patch, showing that the patch does its job.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e0347c2ced9
Regression test for future movable reflow roots - r=dbaron

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26666002fbe3
Indicate position-change reflow hint when RecomputePosition() bails out - r=dbaron,heycam

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4e0347c2ced9
https://hg.mozilla.org/mozilla-central/rev/26666002fbe3