Closed Bug 1784208 Opened 4 months ago Closed 4 months ago

Remove ColumnBalanceData::mHasExcessBSize

Categories

(Core :: Layout: Columns, task)

task

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

Details

Attachments

(3 files)

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.

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

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

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

Attachment #9289511 - Attachment description: Bug 1784208 Part 3 - Remove an unreachable `if` branch in FindBestBalanceBSize(). → Bug 1784208 Part 3 - Remove checking mIsBalancing in FindBestBalanceBSize().
Pushed by aethanyc@gmail.com:
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
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.