Skip to content

Commit

Permalink
Prevent OOFs in clipped containers from adding more fragmentainers.
Browse files Browse the repository at this point in the history 
If the containing block of an out-of-flow positioned box is inside a
overflow-clipped container inside a fragmentation context, prevent the
out-of-flow positioned box from generating additional fragmentainers.

An OOF will have its fragments placed as direct children of
fragmentainers, so any clipping caused by actual ancestor in the real
containing block chain will not be detected during layout. Therefore,
when reaching the last fragmentainer seen (so far), force the OOF node
to finish there, placing all its remaining content in the last
fragmentainer. Anything overflowing the clipped container will be
clipped during paint / pre-paint - normally. This isn't a perfect
solution, since the clipped container doesn't necessarily end in the
last fragmentainer created so far. It may end earlier. This is to say
that this fix doesn't catch every problem in this area. But hopefully
good enough for now.

(cherry picked from commit 1f5612e)

Bug: 1399313
Change-Id: I27be47bd09d7212abd6aea0715e1cc8fe0b3440d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4110949
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1086314}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4131396
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Morten Stenshorne <mstensho@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#98}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@mstensho
mstensho authored and Chromium LUCI CQ committed Jan 2, 2023
1 parent e6ff131 commit 04d4a74
Show file tree
Hide file tree
Showing 16 changed files with 231 additions and 39 deletions.
10 changes: 7 additions & 3 deletions third_party/blink/renderer/core/layout/ng/ng_constraint_space.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,7 @@ class CORE_EXPORT NGConstraintSpace final {
NGConstraintSpace CloneWithoutFragmentation() const {
DCHECK(HasBlockFragmentation());
NGConstraintSpace copy = *this;
DCHECK(copy.rare_data_);
copy.rare_data_->block_direction_fragmentation_type = kFragmentNone;
copy.rare_data_->is_block_fragmentation_forced_off = true;
copy.DisableFurtherFragmentation();
return copy;
}

Expand Down Expand Up @@ -1677,6 +1675,12 @@ class CORE_EXPORT NGConstraintSpace final {
return rare_data_;
}

void DisableFurtherFragmentation() {
DCHECK(rare_data_);
rare_data_->block_direction_fragmentation_type = kFragmentNone;
rare_data_->is_block_fragmentation_forced_off = true;
}

LogicalSize available_size_;

// To save a little space, we union these two fields. rare_data_ is valid if
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ class CORE_EXPORT NGConstraintSpaceBuilder final {
space_.EnsureRareData()->is_inside_repeatable_content = b;
}

void DisableFurtherFragmentation() { space_.DisableFurtherFragmentation(); }

void SetIsFixedInlineSize(bool b) {
if (LIKELY(is_in_parallel_flow_))
space_.bitfields_.is_fixed_inline_size = b;
Expand Down
14 changes: 11 additions & 3 deletions third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,9 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment, is_inside_column_spanner,
multicol_info->fixedpos_containing_block
.RequiresContentBeforeBreaking()),
.RequiresContentBeforeBreaking(),
multicol_info->fixedpos_containing_block
.IsFragmentedInsideClippedContainer()),
new_fixedpos_inline_container));
}
}
Expand Down Expand Up @@ -677,19 +679,25 @@ void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(
fixedpos_containing_block_rel_offset =
fixedpos_containing_block->RelativeOffset();
}
bool is_clipped_container = fragment.HasNonVisibleBlockOverflow();
NGLogicalOOFNodeForFragmentation oof_node(
descendant.Node(), static_position, new_inline_container,
NGContainingBlock<LogicalOffset>(
containing_block_offset, containing_block_rel_offset,
containing_block_fragment, container_inside_column_spanner,
descendant.containing_block.RequiresContentBeforeBreaking()),
descendant.containing_block.RequiresContentBeforeBreaking(),
descendant.containing_block.IsFragmentedInsideClippedContainer() ||
is_clipped_container),
NGContainingBlock<LogicalOffset>(
fixedpos_containing_block_offset,
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment,
fixedpos_container_inside_column_spanner,
descendant.fixedpos_containing_block
.RequiresContentBeforeBreaking()),
.RequiresContentBeforeBreaking(),
descendant.fixedpos_containing_block
.IsFragmentedInsideClippedContainer() ||
is_clipped_container),
new_fixedpos_inline_container);

if (out_list) {
Expand Down
85 changes: 62 additions & 23 deletions third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1048,14 +1048,17 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInMulticol(
containing_block_offset, containing_block_rel_offset,
containing_block_fragment,
descendant.containing_block.IsInsideColumnSpanner(),
descendant.containing_block.RequiresContentBeforeBreaking()),
descendant.containing_block.RequiresContentBeforeBreaking(),
descendant.containing_block.IsFragmentedInsideClippedContainer()),
NGContainingBlock<LogicalOffset>(
fixedpos_containing_block_offset,
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment,
descendant.fixedpos_containing_block.IsInsideColumnSpanner(),
descendant.fixedpos_containing_block
.RequiresContentBeforeBreaking()),
.RequiresContentBeforeBreaking(),
descendant.fixedpos_containing_block
.IsFragmentedInsideClippedContainer()),
fixedpos_inline_container};
oof_nodes_to_layout.push_back(node);
}
Expand Down Expand Up @@ -1434,12 +1437,15 @@ NGOutOfFlowLayoutPart::NodeInfo NGOutOfFlowLayoutPart::SetupNodeInfo(
// case, we also need to adjust the offset to account for this.
NGLogicalStaticPosition static_position = oof_node.static_position;
static_position.offset -= container_info.rect.offset;
bool is_fragmented_inside_clipped_container = false;
if (containing_block_fragment) {
const auto& containing_block_for_fragmentation =
To<NGLogicalOOFNodeForFragmentation>(oof_node).containing_block;
static_position.offset += containing_block_for_fragmentation.Offset();
requires_content_before_breaking =
containing_block_for_fragmentation.RequiresContentBeforeBreaking();
is_fragmented_inside_clipped_container =
containing_block_for_fragmentation.IsFragmentedInsideClippedContainer();
}

NGLogicalStaticPosition oof_static_position =
Expand Down Expand Up @@ -1480,14 +1486,16 @@ NGOutOfFlowLayoutPart::NodeInfo NGOutOfFlowLayoutPart::SetupNodeInfo(
/* is_fragmentainer_descendant */ containing_block_fragment,
fixedpos_containing_block, fixedpos_inline_container,
oof_node.inline_container.container,
requires_content_before_breaking);
requires_content_before_breaking,
is_fragmented_inside_clipped_container);
}

const NGLayoutResult* NGOutOfFlowLayoutPart::LayoutOOFNode(
NodeToLayout& oof_node_to_layout,
const LayoutBox* only_layout,
const NGConstraintSpace* fragmentainer_constraint_space,
bool is_known_to_be_last_fragmentainer) {
bool is_last_fragmentainer_so_far,
bool is_known_to_be_very_last_fragmentainer) {
const NodeInfo& node_info = oof_node_to_layout.node_info;
OffsetInfo& offset_info = oof_node_to_layout.offset_info;
if (offset_info.has_cached_layout_result) {
Expand All @@ -1497,9 +1505,9 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::LayoutOOFNode(

NGBoxStrut scrollbars_before =
ComputeScrollbarsForNonAnonymous(node_info.node);
const NGLayoutResult* layout_result =
Layout(oof_node_to_layout, fragmentainer_constraint_space,
is_known_to_be_last_fragmentainer);
const NGLayoutResult* layout_result = Layout(
oof_node_to_layout, fragmentainer_constraint_space,
is_last_fragmentainer_so_far, is_known_to_be_very_last_fragmentainer);

// Since out-of-flow positioning sets up a constraint space with fixed
// inline-size, the regular layout code (|NGBlockNode::Layout()|) cannot
Expand Down Expand Up @@ -1564,7 +1572,8 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::LayoutOOFNode(
}

layout_result = Layout(oof_node_to_layout, fragmentainer_constraint_space,
is_known_to_be_last_fragmentainer);
is_last_fragmentainer_so_far,
is_known_to_be_very_last_fragmentainer);

scrollbars_after = ComputeScrollbarsForNonAnonymous(node_info.node);
DCHECK(!freeze_horizontal || !freeze_vertical ||
Expand Down Expand Up @@ -1796,7 +1805,8 @@ NGOutOfFlowLayoutPart::TryCalculateOffset(
const NGLayoutResult* NGOutOfFlowLayoutPart::Layout(
const NodeToLayout& oof_node_to_layout,
const NGConstraintSpace* fragmentainer_constraint_space,
bool is_known_to_be_last_fragmentainer) {
bool is_last_fragmentainer_so_far,
bool is_known_to_be_very_last_fragmentainer) {
const NodeInfo& node_info = oof_node_to_layout.node_info;
const WritingDirectionMode candidate_writing_direction =
node_info.node.Style().GetWritingDirection();
Expand Down Expand Up @@ -1832,18 +1842,20 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::Layout(
// Fixed-positioned elements are repeated when paginated, if contained by
// the initial containing block (i.e. when not contained by a transformed
// element or similar).
if (is_known_to_be_last_fragmentainer)
if (is_known_to_be_very_last_fragmentainer) {
repeat_mode = kRepeatedLast;
else
} else {
repeat_mode = kMayRepeatAgain;
}
}

layout_result = GenerateFragment(
node_info.node, container_content_size_in_candidate_writing_mode,
oof_node_to_layout, container_content_size_in_candidate_writing_mode,
offset_info.block_estimate, offset_info.node_dimensions,
offset.block_offset, oof_node_to_layout.break_token,
fragmentainer_constraint_space, should_use_fixed_block_size,
node_info.requires_content_before_breaking, repeat_mode);
node_info.requires_content_before_breaking,
is_last_fragmentainer_so_far, repeat_mode);
}

if (layout_result->Status() != NGLayoutResult::kSuccess) {
Expand Down Expand Up @@ -1899,7 +1911,9 @@ bool NGOutOfFlowLayoutPart::IsContainingBlockForCandidate(
// 2. To compute final fragment, when block size is known from the absolute
// position calculation.
const NGLayoutResult* NGOutOfFlowLayoutPart::GenerateFragment(
NGBlockNode node,
// TODO(mstensho): Reduce the number of arguments. Now that we pass
// NodeToLayout, many of the others are redundant.
const NodeToLayout& oof_node_to_layout,
const LogicalSize& container_content_size_in_candidate_writing_mode,
const absl::optional<LayoutUnit>& block_estimate,
const NGLogicalOutOfFlowDimensions& node_dimensions,
Expand All @@ -1908,7 +1922,10 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::GenerateFragment(
const NGConstraintSpace* fragmentainer_constraint_space,
bool should_use_fixed_block_size,
bool requires_content_before_breaking,
bool is_last_fragmentainer_so_far,
RepeatMode repeat_mode) {
const NodeInfo& node_info = oof_node_to_layout.node_info;
const NGBlockNode& node = node_info.node;
const auto& style = node.Style();

LayoutUnit inline_size = node_dimensions.size.inline_size;
Expand Down Expand Up @@ -1946,6 +1963,24 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::GenerateFragment(
SetupSpaceBuilderForFragmentation(
*fragmentainer_constraint_space, node, block_offset, &builder,
/* is_new_fc */ true, requires_content_before_breaking);

// Out-of-flow positioned elements whose containing block is inside
// clipped overflow shouldn't generate any additional fragmentainers. Just
// place everything in the last fragmentainer. This is similar to what
// NGLayoutAlgorithm::RelayoutWithoutFragmentation() does for in-flow
// content overflowing a clipped ancestor, except that in this case we
// know up front that we should disable fragmentation.
//
// Note that this approach isn't perfect. We don't know where (in which
// fragmentainer) the clipped container ends. It may have ended in some
// fragmentainer earlier than the last one, in which case we should have
// finished this OOF there. But we have no (easy) way of telling where
// that might be. But as long as the OOF doesn't contribute to any
// additional fragmentainers, we should be (pretty) good.
if (is_last_fragmentainer_so_far &&
node_info.is_fragmented_inside_clipped_container) {
builder.DisableFurtherFragmentation();
}
}
} else if (container_builder_->IsInitialColumnBalancingPass()) {
SetupSpaceBuilderForFragmentation(
Expand All @@ -1969,6 +2004,7 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInFragmentainer(
auto& children = FragmentationContextChildren();
wtf_size_t num_children = children.size();
bool is_new_fragment = index >= num_children;
bool is_last_fragmentainer_so_far = index + 1 == num_children;
bool is_known_to_have_more_fragmentainers =
index + 1 < num_children || !is_last_fragmentainer_with_oof_descendants;

Expand Down Expand Up @@ -2019,7 +2055,7 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInFragmentainer(
previous_break_token,
/* early_break */ nullptr);

bool is_known_to_be_last_fragmentainer = false;
bool is_known_to_be_very_last_fragmentainer = false;

do {
// |algorithm| corresponds to the "mutable copy" of our original
Expand All @@ -2030,15 +2066,17 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInFragmentainer(
// Layout any OOF elements that are a continuation of layout first.
for (auto& descendant : descendants_continued) {
AddOOFToFragmentainer(descendant, &space, fragmentainer_offset, index,
is_known_to_be_last_fragmentainer, &algorithm,
is_last_fragmentainer_so_far,
is_known_to_be_very_last_fragmentainer, &algorithm,
fragmented_descendants);
}
// Once we've laid out the OOF elements that are a continuation of layout,
// we can layout the OOF elements that start layout in the current
// fragmentainer.
for (auto& descendant : pending_descendants) {
AddOOFToFragmentainer(descendant, &space, fragmentainer_offset, index,
is_known_to_be_last_fragmentainer, &algorithm,
is_last_fragmentainer_so_far,
is_known_to_be_very_last_fragmentainer, &algorithm,
fragmented_descendants);
}

Expand All @@ -2061,7 +2099,7 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInFragmentainer(
// repeat break token. But they are not going to repeat any further, so
// we now need a re-layout with that in mind (so that they don't get
// outgoing break tokens).
is_known_to_be_last_fragmentainer = true;
is_known_to_be_very_last_fragmentainer = true;
fragmented_descendants->clear();
continue;
}
Expand All @@ -2079,12 +2117,13 @@ void NGOutOfFlowLayoutPart::AddOOFToFragmentainer(
const NGConstraintSpace* fragmentainer_space,
LogicalOffset fragmentainer_offset,
wtf_size_t index,
bool is_known_to_be_last_fragmentainer,
bool is_last_fragmentainer_so_far,
bool is_known_to_be_very_last_fragmentainer,
NGSimplifiedOOFLayoutAlgorithm* algorithm,
HeapVector<NodeToLayout>* fragmented_descendants) {
const NGLayoutResult* result =
LayoutOOFNode(descendant, /* only_layout */ nullptr, fragmentainer_space,
is_known_to_be_last_fragmentainer);
const NGLayoutResult* result = LayoutOOFNode(
descendant, /* only_layout */ nullptr, fragmentainer_space,
is_last_fragmentainer_so_far, is_known_to_be_very_last_fragmentainer);

if (result->Status() != NGLayoutResult::kSuccess) {
DCHECK_EQ(result->Status(), NGLayoutResult::kOutOfFragmentainerSpace);
Expand Down Expand Up @@ -2148,7 +2187,7 @@ void NGOutOfFlowLayoutPart::AddOOFToFragmentainer(
To<NGPhysicalBoxFragment>(result->PhysicalFragment());
const NGBlockBreakToken* break_token = physical_fragment.BreakToken();
if (break_token) {
DCHECK(!is_known_to_be_last_fragmentainer);
DCHECK(!is_known_to_be_very_last_fragmentainer);
// We must continue layout in the next fragmentainer. Update any information
// in NodeToLayout, and add the node to |fragmented_descendants|.
NodeToLayout fragmented_descendant = descendant;
Expand Down
22 changes: 15 additions & 7 deletions third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const NGInlineContainer<LogicalOffset> fixedpos_inline_container;
bool inline_container = false;
bool requires_content_before_breaking = false;
bool is_fragmented_inside_clipped_container = false;

NodeInfo(NGBlockNode node,
const NGConstraintSpace constraint_space,
Expand All @@ -191,7 +192,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const NGContainingBlock<LogicalOffset>& fixedpos_containing_block,
const NGInlineContainer<LogicalOffset>& fixedpos_inline_container,
bool inline_container,
bool requires_content_before_breaking)
bool requires_content_before_breaking,
bool is_fragmented_inside_clipped_container)
: node(node),
constraint_space(constraint_space),
static_position(static_position),
Expand All @@ -201,7 +203,9 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
fixedpos_containing_block(fixedpos_containing_block),
fixedpos_inline_container(fixedpos_inline_container),
inline_container(inline_container),
requires_content_before_breaking(requires_content_before_breaking) {}
requires_content_before_breaking(requires_content_before_breaking),
is_fragmented_inside_clipped_container(
is_fragmented_inside_clipped_container) {}

void Trace(Visitor* visitor) const;
};
Expand Down Expand Up @@ -319,7 +323,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
NodeToLayout& oof_node_to_layout,
const LayoutBox* only_layout,
const NGConstraintSpace* fragmentainer_constraint_space = nullptr,
bool is_known_to_be_last_fragmentainer = false);
bool is_last_fragmentainer_so_far = false,
bool is_known_to_be_very_last_fragmentainer = false);

// TODO(almaher): We are calculating more than just the offset. Consider
// changing this to a more accurate name.
Expand All @@ -343,20 +348,22 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const NGLayoutResult* Layout(
const NodeToLayout& oof_node_to_layout,
const NGConstraintSpace* fragmentainer_constraint_space,
bool is_known_to_be_last_fragmentainer);
bool is_last_fragmentainer_so_far,
bool is_known_to_be_very_last_fragmentainer);

bool IsContainingBlockForCandidate(const NGLogicalOutOfFlowPositionedNode&);

const NGLayoutResult* GenerateFragment(
NGBlockNode node,
const LogicalSize& container_content_size_in_child_writing_mode,
const NodeToLayout& oof_node_to_layout,
const LogicalSize& container_content_size_in_candidate_writing_mode,
const absl::optional<LayoutUnit>& block_estimate,
const NGLogicalOutOfFlowDimensions& node_dimensions,
const LayoutUnit block_offset,
const NGBlockBreakToken* break_token,
const NGConstraintSpace* fragmentainer_constraint_space,
bool should_use_fixed_block_size,
bool requires_content_before_breaking,
bool is_last_fragmentainer_so_far,
RepeatMode repeat_mode);

// Performs layout on the OOFs stored in |pending_descendants| and
Expand All @@ -378,7 +385,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const NGConstraintSpace* fragmentainer_space,
LogicalOffset fragmentainer_offset,
wtf_size_t index,
bool is_known_to_be_last_fragmentainer,
bool is_last_fragmentainer_so_far,
bool is_known_to_be_very_last_fragmentainer,
NGSimplifiedOOFLayoutAlgorithm* algorithm,
HeapVector<NodeToLayout>* fragmented_descendants);
void ReplaceFragmentainer(wtf_size_t index,
Expand Down

0 comments on commit 04d4a74

Please sign in to comment.