Skip to content

Commit

Permalink
Never force a break before when we're already inside the node.
Browse files Browse the repository at this point in the history
When resuming a node after a fragmentainer break, ignore break-before
values, since we're obviously not *before* it anymore (since we're
*inside*). We used to end up in an infinite loop, until out of memory,
not making any fragmentation progress. This was caused by monolithic
content, which, when printing, may take up space on subsequent pages.
This tricked us into thinking that it was a good place to insert a
forced break before the node we were resuming, since we were not at the
beginning of the page.

(cherry picked from commit 103e40f)

Bug: 1451760
Change-Id: I3c3168478f0f23035abd10597927cee47c00730c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4594563
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1156463}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4615086
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#779}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
mstensho authored and Chromium LUCI CQ committed Jun 15, 2023
1 parent 5ac8ab6 commit 3075182
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,37 @@ EBreakBetween CalculateBreakBetweenValue(NGLayoutInputNode child,
const NGBoxFragmentBuilder& builder) {
if (child.IsInline())
return EBreakBetween::kAuto;

// Since it's not an inline node, if we have a fragment at all, it has to be a
// box fragment.
const NGPhysicalBoxFragment* box_fragment = nullptr;
if (layout_result.Status() == NGLayoutResult::kSuccess) {
box_fragment = &To<NGPhysicalBoxFragment>(layout_result.PhysicalFragment());
if (!box_fragment->IsFirstForNode()) {
// If the node is resumed after a break, we are not *before* it anymore,
// so ignore values. We normally don't even consider breaking before a
// resumed node, since there normally is no container separation. The
// normal place to resume is at the very start of the fragmentainer -
// cannot break there! However, there are cases where a node is resumed
// at a location past the start of the fragmentainer, e.g. when printing
// monolithic overflowing content.
return EBreakBetween::kAuto;
}
}

EBreakBetween break_before = JoinFragmentainerBreakValues(
child.Style().BreakBefore(), layout_result.InitialBreakBefore());
break_before = builder.JoinedBreakBetweenValue(break_before);
const NGConstraintSpace& space = builder.ConstraintSpace();
if (space.IsPaginated() &&
layout_result.Status() == NGLayoutResult::kSuccess &&
if (space.IsPaginated() && box_fragment &&
!IsForcedBreakValue(builder.ConstraintSpace(), break_before)) {
AtomicString current_name = builder.PageName();
if (current_name == g_null_atom) {
current_name = space.PageName();
}
// If the page name propagated from the child differs from what we already
// have, we need to break before the child.
const auto& fragment =
To<NGPhysicalBoxFragment>(layout_result.PhysicalFragment());
if (fragment.PageName() != current_name) {
if (box_fragment->PageName() != current_name) {
return EBreakBetween::kPage;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<style>
body {
margin: 0;
}
</style>
<div style="height:200vh; background:green;"></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!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=1451760">
<link rel="match" href="monolithic-overflow-021-print-ref.html">
<style>
body {
margin: 0;
}
</style>
<div style="break-before:page; background:red;">
<div style="border-bottom:50vh solid green; background:red;">
<div style="contain:size; height:150vh; background:green;"></div>
</div>
</div>

0 comments on commit 3075182

Please sign in to comment.