885424 - Refactor flexbox layout logic to make pagination more straightforward

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Filing this bug for a few minor logic-shuffling-patches for flexbox reflow, to make bug 811024 more straightforward.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 0: move 'frameCrossSize' decl down to where it actually gets assigned a value

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: Initialize aStatus outparam at beginning of reflow, and call NS_FRAME_SET_TRUNCATION at the very end

Right now we set 'aStatus' to NS_FRAME_COMPLETE towards the end of reflow. This patch moves it to the beginning, so that (in bug 811024) we can update it to INCOMPLETE if & when we detect the need for a split.

This also moves the NS_FRAME_SET_TRUNCATION line to the very end, for consistency with other frame classes. (In nsBlockFrame, nsFieldSetFrame, nsTableFrame, and every other frame class I checked, we call NS_FRAME_SET_TRUNCATION as the absolute final thing before returning.)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: split main-axis positioning into its own loop

Currently, we do cross-axis sizing and main-axis alignment of flex items as part of the same loop, because they're completely independent, which means we can do them at the same time rather than looping twice.

However, the flexbox layout algorithm in the spec lists these as separate steps -- and importantly, the Sample Flex Fragmentation Algorithm adds some things to do in between them.

So, this patch splits out main-axis alignment into its own loop, and adds some labels that correspond to sections of the spec, for clarity.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: rename a variable for clarity, and mark 2 variables as const

This patch:
 * Renames the local variable 'flexContainerMainSize' to 'contentBoxMainSize' (since it's specifically the size of the flex container's *content-box*)

 * Marks that variable and its friend 'frameMainSize' as 'const'. (These variables will be used at various points throughout reflow (& in some of the code that'll be added in bug 811024), and it's nice to make it clear that we won't be modifying them.)

(Note: 'frameMainSize' is the main-size of the flex container's border-box; maybe that should be renamed to borderBoxMainSize, but I haven't done that yet because that variable name is less ambiguous, since frame sizes are always border-box sizes.)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: part 4: Refactor ComputeFlexContainerMainSize to have a clearer horizontal case vs. vertical case

This patch refactors ComputeFlexContainerMainSize to have an easy early-return case for horizontal flex containers (since they're guaranteed to have a resolved width), and it dedicates the more complicated logic to vertical flex containers. (which might not have a constrained height)

This is effectively how it already was, per the "Computed width should always be constrained" assertion that this patch removes -- but we just had a bit of abstraction-theater, with us repeatedly querying the axis tracker to tell us whether the height or width was the main component, etc, when really we know it's a vertical flex container.

This patch makes things simpler in bug 811024, because it'll let us subtract the heights of flex items' prev-in-flows specifically in the vertical-flex-container case, while avoiding that for horizontal flex containers, without any need for abstraction-theater. (This asymmetry is essentially due to the fact that we only paginate vertically.)

Comment 6

[Daniel Holbert [:dholbert]]

Landed parts 0 through 3:
 part 0: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba51eb5cccf2
 part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/d34ed2e650aa
 part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/9da12a105835
 part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/9dac032e1ffc

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ba51eb5cccf2
https://hg.mozilla.org/mozilla-central/rev/d34ed2e650aa
https://hg.mozilla.org/mozilla-central/rev/9da12a105835
https://hg.mozilla.org/mozilla-central/rev/9dac032e1ffc

Comment 8

[Daniel Holbert [:dholbert]]

 part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/285c7cb20fc2

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/285c7cb20fc2