Commit d35cc136 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

Provide adjusted block-end border/padding to FinishFragmentation().

Don't provide the entire strut, since it was used incorrectly anyway.

FinishFragmentation() sometimes calls IsNodeFullyGrown(), and used to
pass the border_padding strut that was passed to it. This was wrong for
fieldsets, because, internally, fieldset padding isn't part of the
fieldset, but rather the anonymous wrapper child block. To fix this, get
the actual border/padding from the fragment builder, which is what
IsNodeFullyGrown() needs to use when calculating a block-size.

This isn't fixing any known issue, but I was playing with some code, and
stumbled into the DCHECK_GE() failure in IsNodeFullyGrown() when
fieldset block-size was non-auto. Example:

<fieldset style="height:100px; border:10px solid; padding:30px;">

The border-box size should be 180px. This is what would be passed as
current_total_block_size (correct). The border_padding passed would be
10px on each side (just the border, no padding - WRONG!), and the call
to ComputeBlockSizeForFragment() would only return 120px, triggering the
DCHECK.

No behavior changes intended.

Change-Id: I9782f785e2b915b8a4784c443389ba9d434ec5c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438061Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarAlison Maher <almaher@microsoft.com>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811976}
parent bcbbe28c
...@@ -2180,7 +2180,8 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() { ...@@ -2180,7 +2180,8 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
} }
return FinishFragmentation(Node(), ConstraintSpace(), BreakToken(), return FinishFragmentation(Node(), ConstraintSpace(), BreakToken(),
BorderPadding(), space_left, &container_builder_); BorderPadding().block_end, space_left,
&container_builder_);
} }
NGBreakStatus NGBlockLayoutAlgorithm::BreakBeforeChildIfNeeded( NGBreakStatus NGBlockLayoutAlgorithm::BreakBeforeChildIfNeeded(
......
...@@ -303,7 +303,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::Layout() { ...@@ -303,7 +303,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::Layout() {
// In addition to establishing one, we're nested inside another // In addition to establishing one, we're nested inside another
// fragmentation context. // fragmentation context.
FinishFragmentation( FinishFragmentation(
Node(), ConstraintSpace(), BreakToken(), BorderPadding(), Node(), ConstraintSpace(), BreakToken(), BorderPadding().block_end,
FragmentainerSpaceAtBfcStart(ConstraintSpace()), &container_builder_); FragmentainerSpaceAtBfcStart(ConstraintSpace()), &container_builder_);
} }
......
...@@ -126,9 +126,9 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() { ...@@ -126,9 +126,9 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() {
container_builder_.SetIsFieldsetContainer(); container_builder_.SetIsFieldsetContainer();
if (ConstraintSpace().HasBlockFragmentation()) { if (ConstraintSpace().HasBlockFragmentation()) {
FinishFragmentation(Node(), ConstraintSpace(), BreakToken(), borders_, FinishFragmentation(
FragmentainerSpaceAtBfcStart(ConstraintSpace()), Node(), ConstraintSpace(), BreakToken(), borders_.block_end,
&container_builder_); FragmentainerSpaceAtBfcStart(ConstraintSpace()), &container_builder_);
} }
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run(); NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
......
...@@ -232,7 +232,7 @@ bool IsNodeFullyGrown(NGBlockNode node, ...@@ -232,7 +232,7 @@ bool IsNodeFullyGrown(NGBlockNode node,
bool FinishFragmentation(NGBlockNode node, bool FinishFragmentation(NGBlockNode node,
const NGConstraintSpace& space, const NGConstraintSpace& space,
const NGBlockBreakToken* previous_break_token, const NGBlockBreakToken* previous_break_token,
const NGBoxStrut& border_padding, LayoutUnit trailing_border_padding,
LayoutUnit space_left, LayoutUnit space_left,
NGBoxFragmentBuilder* builder) { NGBoxFragmentBuilder* builder) {
LayoutUnit previously_consumed_block_size; LayoutUnit previously_consumed_block_size;
...@@ -280,7 +280,7 @@ bool FinishFragmentation(NGBlockNode node, ...@@ -280,7 +280,7 @@ bool FinishFragmentation(NGBlockNode node,
// haven't already consumed (100px - 20px = 80px) over two columns. We get // haven't already consumed (100px - 20px = 80px) over two columns. We get
// two fragments for #container after the spanner, each 40px tall. // two fragments for #container after the spanner, each 40px tall.
final_block_size = std::min(final_block_size, intrinsic_block_size) - final_block_size = std::min(final_block_size, intrinsic_block_size) -
border_padding.block_end; trailing_border_padding;
} else if (space_left != kIndefiniteSize && desired_block_size > space_left) { } else if (space_left != kIndefiniteSize && desired_block_size > space_left) {
// We're taller than what we have room for. We don't want to use more than // We're taller than what we have room for. We don't want to use more than
// |space_left|, but if the intrinsic block-size is larger than that, it // |space_left|, but if the intrinsic block-size is larger than that, it
...@@ -294,12 +294,12 @@ bool FinishFragmentation(NGBlockNode node, ...@@ -294,12 +294,12 @@ bool FinishFragmentation(NGBlockNode node,
// //
// There is a last-resort breakpoint before trailing border and padding, so // There is a last-resort breakpoint before trailing border and padding, so
// first check if we can break there and still make progress. // first check if we can break there and still make progress.
DCHECK_GE(intrinsic_block_size, border_padding.block_end); DCHECK_GE(intrinsic_block_size, trailing_border_padding);
DCHECK_GE(desired_block_size, border_padding.block_end); DCHECK_GE(desired_block_size, trailing_border_padding);
LayoutUnit subtractable_border_padding; LayoutUnit subtractable_border_padding;
if (desired_block_size > border_padding.block_end) if (desired_block_size > trailing_border_padding)
subtractable_border_padding = border_padding.block_end; subtractable_border_padding = trailing_border_padding;
final_block_size = final_block_size =
std::min(desired_block_size - subtractable_border_padding, std::min(desired_block_size - subtractable_border_padding,
...@@ -381,7 +381,7 @@ bool FinishFragmentation(NGBlockNode node, ...@@ -381,7 +381,7 @@ bool FinishFragmentation(NGBlockNode node,
// more), we're only at the end if no in-flow content inside broke. // more), we're only at the end if no in-flow content inside broke.
if (!builder->HasInflowChildBreakInside() || if (!builder->HasInflowChildBreakInside() ||
IsNodeFullyGrown(node, space, fragments_total_block_size, IsNodeFullyGrown(node, space, fragments_total_block_size,
border_padding, builder->BorderPadding(),
builder->InitialBorderBoxSize().inline_size)) builder->InitialBorderBoxSize().inline_size))
builder->SetIsAtBlockEnd(); builder->SetIsAtBlockEnd();
......
...@@ -149,7 +149,7 @@ bool IsNodeFullyGrown(NGBlockNode, ...@@ -149,7 +149,7 @@ bool IsNodeFullyGrown(NGBlockNode,
bool FinishFragmentation(NGBlockNode node, bool FinishFragmentation(NGBlockNode node,
const NGConstraintSpace&, const NGConstraintSpace&,
const NGBlockBreakToken* previous_break_token, const NGBlockBreakToken* previous_break_token,
const NGBoxStrut& border_padding, LayoutUnit trailing_border_padding,
LayoutUnit space_left, LayoutUnit space_left,
NGBoxFragmentBuilder*); NGBoxFragmentBuilder*);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment