1784208 - Remove ColumnBalanceData::mHasExcessBSize

Status: RESOLVED FIXED

(Core :: Layout: Columns, task)

Description

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

This flag is used when the top-level multicol container frame is nsColumnSet. But nowadays (after implementing column-span), the top-lovel multicol container is ColumnSetWrapperFrame.

See the upcoming patches for details.

Comment 1

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

Attachment: Bug 1784208 Part 1 - Remove ColumnBalanceData::mHasExcessBSize flag.

Remove the flag because it doesn't serve its purpose for current multicolumn
frame hierarchy (i.e. after we introduce ColumnSetWrapperFrame). Nowadays, we
limit the column's block-size with the column container's block-size and
max-block-size in [1]. If the column container has content which exceeds its
max-block-size, we are going to give up column balancing and reach [2] after the
first iteration in the column balancing while loop in FindBestBalanceBSize().

However, the flag can still be set in cases with absurd nscoord values, so
this patch still changes the behavior. For example, in crashtests with bogus
sizes such as 673770.html,

contentBEnd > aReflowInput.mCBReflowInput->ComputedMaxBSize() && aConfig.mIsBalancing

can still be true when contentBEnd is greater than nscoord_MAX. In such
cases, we might spend extra iterations in column balancing. Other than that, our
rendering shouldn't have perceived behavior change.

[1] https://searchfox.org/mozilla-central/rev/6a37a2ab9328bec6a29f688d1b2fba6974d34905/layout/generic/nsBlockFrame.cpp#3834-3844
[2] https://searchfox.org/mozilla-central/rev/6a37a2ab9328bec6a29f688d1b2fba6974d34905/layout/generic/nsColumnSetFrame.cpp#1145-1162,1169-1173

Comment 2

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

Attachment: Bug 1784208 Part 2 - Remove nsColumnSetFrame::ReflowChildren.

With part 1, ReflowChildren and ReflowColumns are identical except for the
constness of the ReflowConfig. I choose to remove ReflowChildren because
ReflowColumns is more meaningful and keeping it requires less change to the
existing code.

Depends on D154427

Comment 3

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

Attachment: Bug 1784208 Part 3 - Remove checking mIsBalancing in FindBestBalanceBSize().

According to our code coverage, this if is not reachable via any tests on our
CI.
https://coverage.moz.tools/#view=file&revision=7169b8faa7e112c9c4323753434324b93270fc3d&path=layout%2Fgeneric%2FnsColumnSetFrame.cpp&line=1129

After part 1 & 2, ReflowColumns can no longer change
ReflowConfig::mIsBalancing from true to false since the ReflowConfig
argument is now a const. Thus, mIsBalancing can never be false in the column
balancing loop.

Depends on D154428

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57b5c5421d9b
Part 1 - Remove ColumnBalanceData::mHasExcessBSize flag. r=emilio
https://hg.mozilla.org/integration/autoland/rev/5a8a837c6045
Part 2 - Remove nsColumnSetFrame::ReflowChildren. r=emilio
https://hg.mozilla.org/integration/autoland/rev/39829c1712e1
Part 3 - Remove checking mIsBalancing in FindBestBalanceBSize(). r=emilio

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/57b5c5421d9b
https://hg.mozilla.org/mozilla-central/rev/5a8a837c6045
https://hg.mozilla.org/mozilla-central/rev/39829c1712e1