1145323 - nsHTMLReflowMetrics instances in nsFlexContainerFrame.cpp and should be given container's reflow state (not child's)

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Daniel Holbert [:dholbert]]

In nsFlexContainerFrame.cpp right now, whenever we reflow a child, we create its nsHTMLReflowMetrics like this:
> 3875   nsHTMLReflowMetrics childDesiredSize(childReflowState);

Specifically: we pass in the *child's* reflow-state to the reflow-metrics constructor.

But, it looks like most other code passes in the *container's* reflow state. e.g. here in nsBlockFrame::Reflow():
> 1224     // Reflow the bullet
> 1225     nsHTMLReflowMetrics metrics(aReflowState);
[...[
> 1235     ReflowBullet(bullet, state, metrics, lineBStart);
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?rev=8949ff85977e#1225

Note that we're passing in the *block's* reflow state to the 'metrics' constructor, whereas we're passing in the child's reflow state ('state') to the Reflow call.


One more example:
> 145   nsHTMLReflowState reflowState(aPresContext, aReflowState,
> 146                                 aBarFrame, availSize);
[...]
> 196   nsHTMLReflowMetrics barDesiredSize(aReflowState);
> 197   ReflowChild(aBarFrame, aPresContext, barDesiredSize, reflowState, xoffset,
> 198               yoffset, 0, aStatus);
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsProgressFrame.cpp?rev=c04e1a0e1920#196
Here, "aReflowState" (passed into the metrics constructor) is the parent's reflow state, and is different from the reflow state that we pass into ReflowChild.

Anyway -- we should bring nsFlexContainerFrame into compliance with this.  I noticed one other similarly-broken spot in nsFieldSetFrame, too:
> 520     nsHTMLReflowMetrics kidDesiredSize(kidReflowState,
> 521                                        aDesiredSize.mFlags);
[...]
> 527     ReflowChild(inner, aPresContext, kidDesiredSize, kidReflowState,
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsFieldSetFrame.cpp?rev=70dda75a914f#520

Comment 1

[Daniel Holbert [:dholbert]]

(It looks like the reflow-state nsHTMLReflowMetrics constructor-param is only used for its writing-mode. So I think this only actually affects behavior when a child frame's writing-mode differs from its parent's writing-mode.)

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1145323 - Audit callers of ReflowOutput constructor in nsFlexContainerFrame. r?dholbert

ReflowOutput can accept any writing mode or ReflowInput passing to its
constructor, so a caller can pass a writing mode that is more convenient
to retrieve the size.

After reflowing flex items, we want to cache its metrics in flex item's
writing mode, so we pass item's ReflowInput into ReflowOutput. However,
for out-of-flow children, we can change ReflowOutput to use flex
container's writing mode by convention.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

nsHTMLReflowMetrics is now ReflowOutput. According to ReflowOutput's comment, it can accept any writing mode. A caller can pass a writing mode which is convenient to access the size.

Comment 4

[Daniel Holbert [:dholbert]]

Thanks for following up on this!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0be9d8da537d
Audit callers of ReflowOutput constructor in nsFlexContainerFrame. r=dholbert

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/0be9d8da537d