Closed Bug 1562122 Opened 5 months ago Closed 5 months ago

Refactor and cleanup nsBlockFrame::ComputeFinalSize() when the block size is constrained


(Core :: Layout: Block and Inline, task, P3)




Tracking Status
firefox69 --- fixed


(Reporter: TYLin, Assigned: TYLin)




(4 files)

I notice when the block size is constrained, the computation of final block size of a block frame is hard to understand. The logic involve nsBlockFrame::ComputeFinalBSize() and the if-hunk after it, and both place contains some redundant logic.

I want to move the if-hunk into nsBlockFrame::ComputeFinalBSize, and see if I can further simplify the computation. My goal is to make no behavior changes.

This patch only moves the logic, and rename some variables. More
clean-up follows.

Note in the middle of ComputeFinalBSize(), ShouldAvoidBreakInside() can
do early return under the condition that aStatus is complete. The logic
moved in this patch is executed only when aStatus is incomplete, so no
behavior is changed after applying this change.

No need to call GetEffectiveComputedBSize() twice.

Also, calling aState.ConsumedBSize() instead of using
aState.mConsumedBSize() directly because the accessor function cache
mConsumedBSize properly if it is called the first time.

Depends on D36288

After the deleted logic

aFinalSize.BSize(wm) =
    std::max(aReflowInput.AvailableBSize(), aContentBSize);

aStatus changes to incomplete, so it computes the same thing again.

Depends on D36289

One thing I'm not so sure how to refactor is the ComputeFinalBSize()'s third argument aContentBSize.

The value aState.mBCoord + nonCarriedOutBDirMargin passing to ComputeFinalBSize() is somewhat like a raw version of blockEndEdgeOfChildren a few lines above, which includes the block start of border and padding, because aState.mBCoord is initialized as mBorderPadding.BStart(wm). Calling it aContentBSize is definitely wrong.

We should maybe do either 1) rename aContentBSize to aBlockEndEdgeOfChild, and figure out whether we want to use the current aState.mBCoord + nonCarriedOutBDirMargin value, or use the blockEndEdgeOfChildren computed a few lines above; or 2) subtract mBorderPadding.BStart(wm) from it, and add back mBorderPadding.BStart(wm) in ComputeFinalBSize(). Again, these all depend on which is one is the correct behavior, and I'm still struggling to figure out.

Pushed by
Part 1 - Move the logic after ComputeFinalBSize() into ComputeFinalBSize(). r=dholbert
Part 2 - Replace effectiveComputedBSize with computedBSizeLeftOver. r=dholbert
Part 3 - Delete the duplicate logic computing the final block size. r=dholbert
Part 4 - Return early in the overflow incomplete case, and add an assertion for the bound of final block size. r=dholbert
You need to log in before you can comment on or make changes to this bug.