1503987 - Skip abspos descendants when measuring content for flex layout

Status: ASSIGNED

(Core :: Layout, enhancement, P3)

Description

[Daniel Holbert [:dholbert]]

Part of the reason we're slow in bug 1376536 is as follows:
 - at an outer level, we've got a flex container that needs to measure a stretched flex item's cross size, so it does a reflow with different characteristics from the "final" freflow of that item (not imposing a large "align-self:stretch"-imposed cross size on the item for this reflow).

 - In that stretched flex item, one of the descendants *only* contains an abspos element, which happens to be expensive to reflow under the characteristics of the measuring reflow in this case.

Good news: for the purposes of the "measuring" reflow, we don't actually care about reflowing that abspos element. All we're interested in is getting the content BSize of the flex item, and the abspos descendants don't influence that.

So I think we can skip abspos descendants (which in this case are expensive) when doing this measurement.

Comment 1

[Daniel Holbert [:dholbert]]

We have to be a bit careful here not to break/invalidate another optimization that we already have.

Specifically: we sometimes skip the "actual" reflow of a flex item, if it received a measuring reflow and if its final size ends up being the same as what we ended up with during that measuring reflow.  (See "bool itemNeedsReflow" guarding our call into ReflowFlexItem in nsFlexContainerFrame.cpp.)

If make the optimization that I'm suggesting here (skipping abspos descendants during the measuring reflow), then it means our measuring reflow won't be as thorough anymore.  So this already-existing optimization  will need to get a bit more subtle -- we'll need to *notice when our measuring reflow wasn't thorough (notice that there's an abspos descendant, basically)**, and we need to proceed with a final "real" reflow of the flex item in question, even if the sizes are all the same between the measuring vs. final reflow.

Comment 2

[Daniel Holbert [:dholbert]]

Currently I'm imagining we'd implement this by adding a new flag to the ReflowInput, to indicate "this subtree is undergoing a measuring reflow to determine a BSize", and making nsAbsoluteContainingBlock::Reflow a no-op if that bit is set.

There are a number of ways that we could signal to our ancestor flex container that we skipped some frames during this measuring reflow:
Option #1: Add a bit to nsReflowStatus or ReflowOutput which gets propagated upwards as reflows complete.
 - pros: this is the normal way we communicate information about what happened during a reflow upwards to ancestors.
 - cons: this may require adding some boilerplate to all container-frame reflow methods, so that they propagate this bit upwards to their own nsReflowStatus or ReflowOutput

Option #2: At the moment we decide to skip some frames, walk up the ancestor chain to find the flex item being measured, and set a property table entry on it.
 - pros: the logic would all be nicely localized in one spot.
 - cons: the property table entry would require a heap allocation.

Option #3: Like #2, but instead of a property table entry, use a new dedicated ReflowInput flag, which is declared as 'mutable' so that we can modify it.  We'd get to this flag by walking upwards through the mParentReflowInput chain, looking for ancestors that are flex items and that have the "undergoing a measuring reflow" ReflowInput bit set.
  - pros: simple & cheap
  - cons: feels a bit messy to have a ReflowInput flag that is technically an outparam and is declared as mutable.

I'm currently leaning towards Option #3 since it seems simplest & cheapest, though ultimately we might want a generalized version of Option #1, if this turns out to be something that's generally useful in layout for getting BSize measurements.

Comment 3

[Daniel Holbert [:dholbert]]

One problem I'm not entirely sure about: if we skip reflowing some frames, we need to be sure we don't clear NS_FRAME_HAS_DIRTY_CHILDREN on those frames' ancestors.

Right now that NS_FRAME_HAS_DIRTY_CHILDREN bit gets cleared automatically in nsFrame::DidReflow, and I'm not sure what the best way would be to prevent that clearing.

One idea: if we take "option 3" from comment 2, we could set the new "we-did-skip-over-some-frames" mutable-ReflowInput-flag on *all* frames between the nsAbsoluteContainingBlock and its flex-item ancestor, and we could check that flag in nsFrame::DidReflow and avoid clearing the NS_FRAME_HAS_DIRTY_CHILDREN state-bit if the flag is set.