Closed Bug 1597177 Opened 3 months ago Closed 3 months ago

Only mark flex containers as having dirty/interrupted measurements if they're actually interrupted before the measurements are complete

Categories

(Core :: Layout: Flexbox, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Emilio suggests in review in https://phabricator.services.mozilla.com/D53313 that we only mark flex containers as having dirty measurements if they actually get interrupted during the cached-measurement phase of their reflow. (As opposed to: right now [or at least as of that phab revision landing], we'll mark them as having dirty measurements if there's an interruption anywhere before the end of the flex container's reflow, which is perhaps overly sensitive.)

I think this should be a valid change, and I'm spinning off this followup to make that change.

Assignee: nobody → dholbert
Status: NEW → ASSIGNED

Notably: as of this patch, flex containers will no longer care about interrupts that happen during their "final/official reflow" of their flex items.

This is fine, because any interruptions during that phase will be appropriately queued up for further reflows, via the interrupted frame's call to mPresShell->FrameNeedsToContinueReflow() inside of nsPresContext::CheckForInterrupt. That will ensure that the next reflow will be aware that this frame needs another reflow. (This doesn't caused cached measurements to be purged, though, which is why we have to check for interrupts during measurement & purge those specially.)

In looking at the FrameNeedsToContinueReflow pipeline, though, I actually think we can address the TODO(emilio) comment and correctly invalidate cached measurements on just the frames that had some descendant reflow interrupted... I think we can & should add some cache-purging to https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/layout/base/PresShell.cpp#9230-9240 , and then we can do away with the interrupt-related cache-purging code in nsFlexContainerFrame.

That almost makes me want to just morph this bug, but I think I'm going to keep it as-is and spin off the further-improvement as another followup bug. I think this patch here is still an incremental improvement, even if it's going to be removed in the next followup, and I wouldn't be surprised if the next step causes an unforseen behavior-change/issue (in which case it'll be nice to have this bug's fix in as a rollback point)

Blocks: 1597348
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b42230b2ad6e
Make flex containers check for interruptions a bit earlier (right after the measurement whose interruption they care about). r=emilio
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.