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

[LayoutNG] Allow empty fragmentainers.

These fragmentainers are created naturally by the block layout
algorithm. CL:1766070 added code to ignore them (just drop them on the
floor), but it seems that it's less cumbersome to just keep them. We
kept them anyway, if there were unhandled OOFs inside (and manually
propagating such OOFs sounds messy). In addition, we end up with empty
fragmentainers if the first child of a multicol container is a spanner.
Even if we discarded the actual fragmentainer, we'd still keep the break
token generated (to resume at column content after the spanner(s)), and
this in turn would cause trouble when calucalting the fragmentainer
index in NGBlockNode::AddColumnResult() (we'd essentially never add a
fragment at index 0, but jump straight to 1).

We may need to revisit this again in the future, though. Overflow
calculation comes to mind. We'll now create a 1px tall fragmentainer
even inside 0px tall multicol containers. But we can either just prevent
fragmentainers from contributing to overflow (this is not what other
browsers do, though), or actually make such fragmentainers 0px tall
(they become 1px because the spec says that fragmentainers may not be
shorter than that, but we might get away with it anyway).

Add a DCHECK in LayoutBox::AddLayoutResult() to assert that we don't
mess up the index (like we used to, which in turn made us keep fragments
associated with deleted layout objects).

Quite a few unit tests had to be updated because of this. I changed some
of them slightly, to give the new empty fragmentainers a nicer width
that they'd otherwise get.

Bug: 1105758
Change-Id: I44e1b93dea4cd5e0c5f7944798e2336530a6d1a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346389
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796859}
parent 2eab7dfd
...@@ -2615,6 +2615,7 @@ void LayoutBox::AddLayoutResult(scoped_refptr<const NGLayoutResult> result, ...@@ -2615,6 +2615,7 @@ void LayoutBox::AddLayoutResult(scoped_refptr<const NGLayoutResult> result,
ReplaceLayoutResult(std::move(result), index); ReplaceLayoutResult(std::move(result), index);
return; return;
} }
DCHECK_EQ(index, layout_results_.size());
const auto& fragment = To<NGPhysicalBoxFragment>(result->PhysicalFragment()); const auto& fragment = To<NGPhysicalBoxFragment>(result->PhysicalFragment());
layout_results_.push_back(std::move(result)); layout_results_.push_back(std::move(result));
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
......
...@@ -703,27 +703,16 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow( ...@@ -703,27 +703,16 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
column_size.block_size = new_column_block_size; column_size.block_size = new_column_block_size;
} while (true); } while (true);
bool is_empty = false;
// If there was no content inside to process, we don't want the resulting
// empty column fragment.
if (new_columns.size() == 1u) {
const NGPhysicalBoxFragment& column = new_columns[0].Fragment();
if (column.Children().size() == 0) {
// No content. Keep the trailing margin from any previous column spanner.
is_empty = true;
// TODO(mstensho): It's wrong to keep the empty fragment, just so that
// out-of-flow descendants get propagated correctly. Find some other way
// of propagating them.
if (!column.HasOutOfFlowPositionedDescendants())
return result;
}
}
intrinsic_block_size_ = column_block_offset + column_size.block_size; intrinsic_block_size_ = column_block_offset + column_size.block_size;
// If we just have one empty fragmentainer, we need to keep the trailing
// margin from any previous column spanner, and also make sure that we don't
// incorrectly consider this to be a class A breakpoint. A fragmentainer may
// end up empty if there's no in-flow content at all inside the multicol
// container, or if the multicol container starts with a spanner.
bool is_empty =
new_columns.size() == 1 && new_columns[0].Fragment().Children().empty();
if (!is_empty) { if (!is_empty) {
has_processed_first_child_ = true; has_processed_first_child_ = true;
container_builder_.SetPreviousBreakAfter(EBreakBetween::kAuto); container_builder_.SetPreviousBreakAfter(EBreakBetween::kAuto);
......
...@@ -1096,7 +1096,6 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/composited-opacity-2 ...@@ -1096,7 +1096,6 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/composited-opacity-2
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/composited-with-overflow-in-next-column.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/composited-with-overflow-in-next-column.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/doubly-nested-with-top-padding-crossing-row-boundaries.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/doubly-nested-with-top-padding-crossing-row-boundaries.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/change-second-row-height.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/change-second-row-height.html [ Failure ]
crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-before-content.html [ Crash ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-constrained-width.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-constrained-width.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-unconstrained-width.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-unconstrained-width.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/relpos-becomes-static-has-abspos.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/relpos-becomes-static-has-abspos.html [ Failure ]
......
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