1599159 - returning a BreakBefore reflow status shouldn't require DesiredSize to be assigned too

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Mats Palmgren (inactive)]

Attachment: WIP

As noted in this comment:
https://phabricator.services.mozilla.com/D53709?id=194022#inline-324289
We have some DEBUG code that requires DesiredSize to be assigned to a value:
https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/layout/generic/nsBlockReflowContext.cpp#286-287,291,304-305

IIRC, there's also some other block reflow code that actually used the size even when BreakBefore was returned, which is bogus. (The attached WIP avoided that code, but I've forgot the exact details.)

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1599159 - Break the child block frame reflow loop if there is a break-before.

This patch is based on Mats Palmgren's idea bug 1599159 comment 0.

When a child block frame reports inline-break-before status after being
reflowed, its size in the returned reflow output shouldn't matter because the
parent is expected to push it to the next page/column [1].

Here is the detail of the debug assertion if the workaround in fieldset frame is
removed. In nsBlockReflowContext::ReflowBlock(), it sets mReflowOutput::BSize()
to 0xdeadbeef, which is a negative number in 32-bits integer. Without the early
break of the loop in nsBlockFrame::ReflowBlockFrame(), then later in the loop we
are going to pass the negative block-size to
BlockReflowState::GetFloatAvailableSpaceForBSize(), which triggers a aBSize>=0
assertion further down in the callstack in nsFloatManager::GetFlowArea().

layout/reftests/pagination/fieldset-00G.html can trigger the above assertion
after removing the workaround in fieldset frame.

I don't expect this patch will change the behavior even if there are some float
elements interacting with this block. Without the early breaking out of the
loop, we still end up in [1].

[1] https://searchfox.org/mozilla-central/rev/ace2c59e6c56b2dcba25af1aa8903a5e7f9a5857/layout/generic/nsBlockFrame.cpp#4026-4035

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55dc239c0828
Break the child block frame reflow loop if there is a break-before. r=emilio

Comment 3

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/55dc239c0828