Skip to content

Commit

Permalink
Keep on updating early-breaks after having inserted actual breaks.
Browse files Browse the repository at this point in the history
We may be inside a parallel flow, and there may be parent flows that
have break avoidance requests. The best breakpoint may be right before
the child that we broke inside (if the break already inserted is in a
parallel flow, in which case it will typically be discarded if we're
going to break before its container).

Bug: 1381118
Change-Id: I07cf04caaf787f746e88233deda44b9c7f583a40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4002109
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1067715}
  • Loading branch information
mstensho authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent 8f4d677 commit 74a5cf1
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 21 deletions.
50 changes: 29 additions & 21 deletions third_party/blink/renderer/core/layout/ng/ng_fragmentation_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,7 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
return false;
}

bool move_past = false;
NGBreakAppeal appeal_inside =
CalculateBreakAppealInside(space, layout_result);
if (IsResumingLayout(break_token) || appeal_inside < kBreakAppealPerfect) {
Expand All @@ -936,16 +937,16 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
if (flex_column_break_info) {
if (!flex_column_break_info->early_break ||
appeal_inside >= flex_column_break_info->early_break->BreakAppeal())
return true;
move_past = true;
} else if (!builder || !builder->HasEarlyBreak() ||
appeal_inside >= builder->EarlyBreak().BreakAppeal()) {
return true;
move_past = true;
}
}
} else {
LayoutUnit block_size =
BlockSizeForFragmentation(layout_result, space.GetWritingDirection());
bool move_past = refuse_break_before;
move_past = refuse_break_before;
if (!move_past) {
if (block_size <= space_left) {
// The fragment fits! We can move past.
Expand All @@ -960,29 +961,31 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
if (move_past) {
// The child either fits, or we are not allowed to break. So we can move
// past this breakpoint.
if (child.IsBlock() && builder && !is_row_item) {
// We're tentatively not going to break before or inside this child, but
// we'll check the appeal of breaking there anyway. It may be the best
// breakpoint we'll ever find. (Note that we only do this for block
// children, since, when it comes to inline layout, we first need to lay
// out all the line boxes, so that we know what do to in order to honor
// orphans and widows, if at all possible. We also only do this for
// non-row items since items in a row will be parallel to one another.)
UpdateEarlyBreakAtBlockChild(space, To<NGBlockNode>(child),
layout_result, appeal_before, builder,
flex_column_break_info);
}

if (block_size > space_left && builder) {
// We're moving past the breakpoint even if the child doesn't fit. This
// may happen with monolithic content at the beginning of the
// fragmentainer. Report space shortage.
PropagateSpaceShortage(space, &layout_result,
fragmentainer_block_offset, builder);
}
}
}

return true;
if (move_past) {
if (child.IsBlock() && builder && !is_row_item) {
// We're tentatively not going to break before this child, but we'll check
// the appeal of breaking there anyway. It may be the best breakpoint
// we'll ever find. (Note that we only do this for block children, since,
// when it comes to inline layout, we first need to lay out all the line
// boxes, so that we know what do to in order to honor orphans and widows,
// if at all possible. We also only do this for non-row items since items
// in a row will be parallel to one another.)
UpdateEarlyBreakAtBlockChild(space, To<NGBlockNode>(child), layout_result,
appeal_before, builder,
flex_column_break_info);
}

return true;
}

// We don't want to break inside, so we should attempt to break before.
Expand All @@ -996,13 +999,18 @@ void UpdateEarlyBreakAtBlockChild(
NGBreakAppeal appeal_before,
NGBoxFragmentBuilder* builder,
NGFlexColumnBreakInfo* flex_column_break_info) {
// If the child already broke, it's a little too late to look for breakpoints.
DCHECK(!IsResumingLayout(
To<NGBlockBreakToken>(layout_result.PhysicalFragment().BreakToken())));

// We may need to create early-breaks even if we have broken inside the child,
// in case it establishes a parallel flow, in which case a break inside won't
// help honor any break avoidance requests that come after this child. But
// breaking *before* the child might help.
const auto* break_token =
To<NGBlockBreakToken>(layout_result.PhysicalFragment().BreakToken());
// See if there's a good breakpoint inside the child.
NGBreakAppeal appeal_inside = kBreakAppealLastResort;
if (const NGEarlyBreak* breakpoint = layout_result.GetEarlyBreak()) {
// If the child broke inside, it shouldn't have any early-break.
DCHECK(!IsResumingLayout(break_token));

appeal_inside = CalculateBreakAppealInside(space, layout_result,
breakpoint->BreakAppeal());
if (flex_column_break_info) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ crbug.com/829028 external/wpt/css/css-break/break-between-avoid-009.html [ Failu
crbug.com/829028 external/wpt/css/css-break/break-between-avoid-010.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/break-between-avoid-011.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/break-between-avoid-012.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/break-between-avoid-013.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/break-between-avoid-014.html [ Failure ]
crbug.com/726125 external/wpt/css/css-break/chrome-bug-1289999-crash.https.html [ Skip ]
crbug.com/829028 external/wpt/css/css-break/clearance-parallel-flow-001.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/clearance-parallel-flow-002.html [ Failure ]
Expand Down Expand Up @@ -218,6 +220,8 @@ crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragm
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-048.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-049.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-051.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-052.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-053.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-007.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-008.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-010.html [ Failure ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#break-propagation">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1381118">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width:100px; height:100px; background:red;">
<div style="columns:2; gap:0; column-fill:auto; height:150px;">
<div style="height:50px; background:green;"></div>
<div style="height:50px; background:green;"></div>
<div style="height:10px;">
<div style="height:20px; contain:size; background:green;"></div>
<div style="height:40px; contain:size; background:green;"></div>
</div>
<div style="break-before:avoid; break-inside:avoid; height:90px;">
<div style="height:50px;"></div>
<div style="height:40px; background:green;"></div>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#break-propagation">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1381118">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width:100px; height:100px; background:red;">
<div style="columns:2; gap:0; column-fill:auto; height:150px;">
<div style="height:100px; background:green;"></div>
<div style="height:20px; background:green;"></div>
<div style="break-before:avoid; height:10px;">
<div style="height:20px; contain:size; background:green;"></div>
<div style="height:20px; contain:size; background:green;"></div>
</div>
<div style="break-before:avoid; break-inside:avoid; height:90px;">
<div style="height:30px;"></div>
<div style="height:40px; background:green;"></div>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1381118">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width:100px; height:100px; background:red;">
<div style="columns:2; column-fill:auto; gap:0; height:150px;">
<div style="display:flex; flex-direction:column; flex-wrap:wrap; height:250px;">
<div style="width:50px; height:75px; background:green;"></div>
<div style="width:50px; height:25px; background:green;"></div>
<div style="width:50px; height:25px; background:green;">
<div style="height:100px;"></div>
</div>
<div style="break-before:avoid; break-inside:avoid; height:55px; background:green;"></div>
<div style="width:50px; height:20px; background:green;"></div>
<div style="height:200px;"></div>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1381118">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width:100px; height:100px; background:red;">
<div style="columns:2; column-fill:auto; gap:0; height:150px;">
<div style="display:flex; flex-direction:column; flex-wrap:wrap; height:250px;">
<div style="width:50px; height:100px; background:green;"></div>
<div style="width:50px; height:25px; background:green;"></div>
<div style="break-before:avoid; height:25px;">
<div style="height:75px; background:green;"></div>
</div>
<div style="break-before:avoid; break-inside:avoid; height:55px;"></div>
<div style="height:200px;"></div>
</div>
</div>
</div>

0 comments on commit 74a5cf1

Please sign in to comment.