Bug 1524375 Comment 6 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Mats Palmgren (:mats) from comment #5)
> I don't really understand what the last paragraph is about though:
> https://searchfox.org/mozilla-central/rev/6e3cc153566f5f288ae768a2172385b8436d61dd/layout/generic/nsFlexContainerFrame.cpp#1718-1722

It's possible that paragraph is off-base... It's trying to justify why we need to include the item's ComputedBSize as part of the cache key.  Superficially, it might seem like it's unneeded, because we already clear the cache whenever 'height' and 'width' change.

From looking at [the reftest that landed with the patch that added this comment](https://searchfox.org/mozilla-central/source/layout/reftests/w3c-css/submitted/flexbox/flexbox-align-items-center-nested-001.html) (and started including ComputedBSize in the cache key), I suspect the real reason is that:
 - the flex item might be measured twice in the same overall reflow, first in an "indefinite percent-basis" scenario and then later in a "definite percent-basis" scenario.
 - And if the flex item happens to involve percent heights, then the definiteness of the percent basis will influence the resolution of those heights.
 - So, the same element with the same styles may produce a mComputedBSize of NS_INTRINSICSIZE in the first of those reflows, vs. some resolved percentage value in the second of those reflows. And it's invalid for us to skip over the second reflow and reuse the measurement from the first one.  So, checking for ReflowInput::ComputedBSize differences (between cached value's cache key & the current ReflowInput) will help us avoid making that mistake.
(In reply to Mats Palmgren (:mats) from comment #5)
> I don't really understand what the last paragraph is about though:
> https://searchfox.org/mozilla-central/rev/6e3cc153566f5f288ae768a2172385b8436d61dd/layout/generic/nsFlexContainerFrame.cpp#1718-1722

It's possible that paragraph is off-base... It's trying to justify why we need to include the item's ComputedBSize as part of the cache key.  Superficially, it might seem like it's unneeded, because we already clear the cached measurements whenever 'height' and 'width' change (or whenever anything invalidates our cached intrinsic measurements).

From looking at [the reftest that landed with the patch that added this comment](https://searchfox.org/mozilla-central/source/layout/reftests/w3c-css/submitted/flexbox/flexbox-align-items-center-nested-001.html) (and started including ComputedBSize in the cache key), I suspect the real reason is that:
 - the flex item might be measured twice in the same overall reflow, first in an "indefinite percent-basis" scenario and then later in a "definite percent-basis" scenario.
 - And if the flex item happens to involve percent heights, then the definiteness of the percent basis will influence the resolution of those heights.
 - So, the same element with the same styles may produce a mComputedBSize of NS_INTRINSICSIZE in the first of those reflows, vs. some resolved percentage value in the second of those reflows. And it's invalid for us to skip over the second reflow and reuse the measurement from the first one.  So, checking for ReflowInput::ComputedBSize differences (between cached value's cache key & the current ReflowInput) will help us avoid making that mistake.
(In reply to Mats Palmgren (:mats) from comment #5)
> I don't really understand what the last paragraph is about though:
> https://searchfox.org/mozilla-central/rev/6e3cc153566f5f288ae768a2172385b8436d61dd/layout/generic/nsFlexContainerFrame.cpp#1718-1722

It's possible that paragraph is off-base... It's trying to justify why we need to include the item's ComputedBSize as part of the cache key.  Superficially, it might seem like we wouldn't need to bother checking it, because we already clear the cached measurements whenever 'height' and 'width' change (or whenever anything invalidates our cached intrinsic measurements).

From looking at [the reftest that landed with the patch that added this comment](https://searchfox.org/mozilla-central/source/layout/reftests/w3c-css/submitted/flexbox/flexbox-align-items-center-nested-001.html) (and started including ComputedBSize in the cache key), I suspect the real reason is that:
 - the flex item might be measured twice in the same overall reflow, first in an "indefinite percent-basis" scenario and then later in a "definite percent-basis" scenario.
 - And if the flex item happens to involve percent heights, then the definiteness of the percent basis will influence the resolution of those heights.
 - So, the same element with the same styles may produce a mComputedBSize of NS_INTRINSICSIZE in the first of those reflows, vs. some resolved percentage value in the second of those reflows. And it's invalid for us to skip over the second reflow and reuse the measurement from the first one.  So, checking for ReflowInput::ComputedBSize differences (between cached value's cache key & the current ReflowInput) will help us avoid making that mistake.

Back to Bug 1524375 Comment 6