[LayoutNG] Fix stability issues with NGBlockLayoutAlgorithm.
...we had all the right concepts, but we're holding them the wrong way.
This patch does a few inter-related things to ensure we don't have
different parts of the block layout algorithm fighting each other.
Previously we could end up in a state where a child believed it should
be placed at a particular BFC-block-offset, and the parent disagreeing
with that. This caused us to perform three layouts, and fail the
DCHECK_EQ(layout_result->Status(), kSuccess) check.
(if we did the layouts in a loop, this would otherwise result in a
timeout).
This introduces/changes:
- NGLayoutResult::IsEmptyBlock This flag is determined at the very
end of the layout algorithm. If we didn't resolve our BFC
block-offset, or were an empty-inline line-box we get this flag.
This flag is more stable than checking if the
NGLayoutResult::BlockBfcOffset had a value as this would sometimes
be present for empty-blocks.
- Changes the NGConstraintSpace::FloatsBfcBlockOffset to
NGConstraintSpace::ForcedBfcBlockOffset. This is used in *almost*
an identical way, except that:
- Empty-blocks will always set their position to this.
- Non-empty-blocks will typically also set their position to this
(unless shifted by clearance).
- AdjoiningFloatsTypes are now "passed" parent->child->sibling much
like exclusion spaces are. Only at the end of an algorithm do we
determine if they should be "passed" up to a parent based on if it
is an empty block or not.
This helps in determining clearance past adjoining floats.
- Determining the BFC-block-offset when a child has aborted now has a
more complex check to determine if the current layout should be
considered as "clearing" floats also.
- Removes the "forced-clearance" flag, in favour of the "forced" BFC
block-offset, and adds the "ancestor has clearance past adjoining
floats" flag.
Bug: 962344, 962300
Change-Id: Ife4030847f2e2b97aa5726072fadf581696ab8e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1611111
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661462}
diff --git a/third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc b/third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc
index 3f5c0cda..78ef986 100644
--- a/third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc
+++ b/third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc
@@ -349,6 +349,7 @@
// Even if we have something in-flow, it may just be empty items that
// shouldn't trigger creation of a line. Exit now if that's the case.
if (line_info->IsEmptyLine()) {
+ container_builder_.SetIsEmptyBlock();
container_builder_.SetIsEmptyLineBox();
container_builder_.SetBaseDirection(line_info->BaseDirection());
container_builder_.AddChildren(line_box_);
@@ -569,9 +570,8 @@
bool is_empty_inline = Node().IsEmptyInline();
LayoutUnit bfc_block_offset = line_info.BfcOffset().block_offset;
- if (is_empty_inline && ConstraintSpace().FloatsBfcBlockOffset()) {
- bfc_block_offset = *ConstraintSpace().FloatsBfcBlockOffset();
- }
+ if (is_empty_inline && ConstraintSpace().ForcedBfcBlockOffset())
+ bfc_block_offset = *ConstraintSpace().ForcedBfcBlockOffset();
LayoutUnit bfc_line_offset = container_builder_.BfcLineOffset();
@@ -917,6 +917,11 @@
// TODO(ikilpatrick): Move this into ng_block_layout_algorithm.
container_builder_.SetBlockSize(
ComputeContentSize(line_info, exclusion_space, line_height));
+
+ // As we aren't an empty inline we should have correctly placed all
+ // our adjoining floats, and shouldn't propagate this information
+ // to siblings.
+ container_builder_.ResetAdjoiningFloatTypes();
}
break;
}
@@ -957,13 +962,13 @@
? kFloatTypeLeft
: kFloatTypeRight);
- // If we are an empty inline, and don't have the special floats BFC
+ // If we are an empty inline, and don't have the special forced BFC
// block-offset yet, there is no way to position any floats.
- if (is_empty_inline && !ConstraintSpace().FloatsBfcBlockOffset())
+ if (is_empty_inline && !ConstraintSpace().ForcedBfcBlockOffset())
continue;
LayoutUnit origin_bfc_block_offset =
- is_empty_inline ? *ConstraintSpace().FloatsBfcBlockOffset()
+ is_empty_inline ? *ConstraintSpace().ForcedBfcBlockOffset()
: ConstraintSpace().BfcOffset().block_offset;
NGPositionedFloat positioned_float = PositionFloat(
diff --git a/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc b/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
index 316199d..5d8417640 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
@@ -71,8 +71,7 @@
bool ApplyClearance(const NGConstraintSpace& constraint_space,
LayoutUnit* bfc_block_offset) {
if (constraint_space.HasClearanceOffset() &&
- (*bfc_block_offset < constraint_space.ClearanceOffset() ||
- constraint_space.ShouldForceClearance())) {
+ *bfc_block_offset < constraint_space.ClearanceOffset()) {
*bfc_block_offset = constraint_space.ClearanceOffset();
return true;
}
@@ -82,32 +81,21 @@
// Returns if the resulting fragment should be considered an "empty block".
// There is special casing for fragments like this, e.g. margins "collapse
// through", etc.
-inline bool IsEmptyBlock(bool is_new_fc, const NGLayoutResult& layout_result) {
- // TODO(ikilpatrick): This should be a DCHECK.
- if (is_new_fc)
- return false;
-
- if (layout_result.BfcBlockOffset())
- return false;
-
+inline bool IsEmptyBlock(const NGLayoutResult& layout_result) {
#if DCHECK_IS_ON()
- const NGPhysicalFragment& physical_fragment =
- layout_result.PhysicalFragment();
- // This just checks that the fragments block size is actually zero. We can
- // assume that its in the same writing mode as its parent, as a different
- // writing mode child will be caught by the is_new_fc check.
- NGFragment fragment(physical_fragment.Style().GetWritingMode(),
- physical_fragment);
- DCHECK_EQ(LayoutUnit(), fragment.BlockSize());
+ if (layout_result.IsEmptyBlock()) {
+ const NGPhysicalFragment& physical_fragment =
+ layout_result.PhysicalFragment();
+ // This just checks that the fragments block size is actually zero. We can
+ // assume that its in the same writing mode as its parent, as a different
+ // writing mode child will be caught by the is_new_fc check.
+ NGFragment fragment(physical_fragment.Style().GetWritingMode(),
+ physical_fragment);
+ DCHECK_EQ(LayoutUnit(), fragment.BlockSize());
+ }
#endif
- return true;
-}
-
-// As above; for convenience if you have a child_space.
-inline bool IsEmptyBlock(const NGConstraintSpace& child_space,
- const NGLayoutResult& layout_result) {
- return IsEmptyBlock(child_space.IsNewFormattingContext(), layout_result);
+ return layout_result.IsEmptyBlock();
}
LayoutUnit LogicalFromBfcLineOffset(LayoutUnit child_bfc_line_offset,
@@ -333,7 +321,7 @@
LayoutUnit inline_size = container_builder_.Size().inline_size;
TextDirection direction = ConstraintSpace().Direction();
- if (child_bfc_block_offset) {
+ if (child_bfc_block_offset && container_builder_.BfcBlockOffset()) {
return LogicalFromBfcOffsets(
{child_bfc_line_offset, *child_bfc_block_offset}, ContainerBfcOffset(),
fragment.InlineSize(), inline_size, direction);
@@ -343,10 +331,9 @@
child_bfc_line_offset, container_builder_.BfcLineOffset(),
fragment.InlineSize(), inline_size, direction);
- // If we've reached here, both the child and the current layout don't have a
- // BFC block offset yet. Children in this situation are always placed at a
- // logical block offset of 0.
- DCHECK(!container_builder_.BfcBlockOffset());
+ // If we've reached here, either the parent, or the child don't have a BFC
+ // block-offset yet. Children in this situation are always placed at a
+ // logical block-offset of zero.
return {inline_offset, LayoutUnit()};
}
@@ -397,7 +384,7 @@
// offset is updated.
abort_when_bfc_block_offset_updated_ = true;
- container_builder_.AddAdjoiningFloatTypes(float_types);
+ container_builder_.SetAdjoiningFloatTypes(float_types);
}
// If we are resuming from a break token our start border and padding is
@@ -407,7 +394,7 @@
NGPreviousInflowPosition previous_inflow_position = {
LayoutUnit(), ConstraintSpace().MarginStrut(),
- /* empty_block_affected_by_clearance */ false};
+ /* empty_block_had_clearance */ false};
// Do not collapse margins between parent and its child if:
//
@@ -459,7 +446,7 @@
if (!container_builder_.BfcBlockOffset()) {
// New formatting contexts, and where we have an empty block affected by
// clearance should already have their BFC block offset resolved.
- DCHECK(!previous_inflow_position.empty_block_affected_by_clearance);
+ DCHECK(!previous_inflow_position.empty_block_had_clearance);
DCHECK(!ConstraintSpace().IsNewFormattingContext());
}
#endif
@@ -569,7 +556,7 @@
// - We are a new formatting context.
// Additionally this fragment produces no end margin strut.
if (border_scrollbar_padding_.block_end ||
- previous_inflow_position->empty_block_affected_by_clearance ||
+ previous_inflow_position->empty_block_had_clearance ||
ConstraintSpace().IsNewFormattingContext()) {
// If we are a quirky container, we ignore any quirky margins and
// just consider normal margins to extend our size. Other UAs
@@ -665,6 +652,25 @@
container_builder_.SetEndMarginStrut(end_margin_strut);
container_builder_.SetIntrinsicBlockSize(intrinsic_block_size_);
+ if (container_builder_.BfcBlockOffset()) {
+ // If we know our BFC block-offset we should have correctly placed all
+ // adjoining floats, and shouldn't propagate this information to siblings.
+ container_builder_.ResetAdjoiningFloatTypes();
+ } else {
+ // If we don't know our BFC block-offset yet, we know that for
+ // margin-collapsing purposes we are an empty-block.
+ container_builder_.SetIsEmptyBlock();
+
+ // If we've been forced at a particular BFC block-offset, (either from
+ // clearance past adjoining floats, or a re-layout), we can safely set our
+ // BFC block-offset now.
+ if (ConstraintSpace().ForcedBfcBlockOffset()) {
+ DCHECK(unpositioned_floats_.IsEmpty());
+ container_builder_.SetBfcBlockOffset(
+ *ConstraintSpace().ForcedBfcBlockOffset());
+ }
+ }
+
// We only finalize for fragmentation if the fragment has a BFC block offset.
// This may occur with a zero block size fragment. We need to know the BFC
// block offset to determine where the fragmentation line is relative to us.
@@ -833,14 +839,14 @@
// No need to postpone the positioning if we know the correct offset.
if (container_builder_.BfcBlockOffset() ||
- ConstraintSpace().FloatsBfcBlockOffset()) {
+ ConstraintSpace().ForcedBfcBlockOffset()) {
// Adjust origin point to the margins of the last child.
// Example: <div style="margin-bottom: 20px"><float></div>
// <div style="margin-bottom: 30px"></div>
LayoutUnit origin_block_offset =
container_builder_.BfcBlockOffset()
? NextBorderEdge(previous_inflow_position)
- : *ConstraintSpace().FloatsBfcBlockOffset();
+ : *ConstraintSpace().ForcedBfcBlockOffset();
PositionPendingFloats(origin_block_offset);
}
}
@@ -857,11 +863,9 @@
const ComputedStyle& child_style = child.Style();
const TextDirection direction = ConstraintSpace().Direction();
- bool has_clearance_past_adjoining_floats = HasClearancePastAdjoiningFloats(
- container_builder_.AdjoiningFloatTypes(), child_style, Style());
- NGInflowChildData child_data = ComputeChildData(
- *previous_inflow_position, child, child_break_token,
- has_clearance_past_adjoining_floats, /* is_new_fc */ true);
+ NGInflowChildData child_data =
+ ComputeChildData(*previous_inflow_position, child, child_break_token,
+ /* is_new_fc */ true);
LayoutUnit child_origin_line_offset =
ConstraintSpace().BfcOffset().line_offset +
@@ -895,11 +899,22 @@
if (!container_builder_.BfcBlockOffset()) {
had_pending_floats = !unpositioned_floats_.IsEmpty();
- if (ConstraintSpace().FloatsBfcBlockOffset()) {
+ // If this node, or an arbitrary ancestor had clearance past adjoining
+ // floats, we consider the margin "separated". We should *never* attempt to
+ // re-resolve the BFC block-offset in this case.
+ bool has_clearance_past_adjoining_floats =
+ ConstraintSpace().AncestorHasClearancePastAdjoiningFloats() ||
+ HasClearancePastAdjoiningFloats(
+ container_builder_.AdjoiningFloatTypes(), child_style, Style());
+
+ if (has_clearance_past_adjoining_floats) {
+ child_bfc_offset_estimate = NextBorderEdge(*previous_inflow_position);
+ child_margin_got_separated = true;
+ } else if (ConstraintSpace().ForcedBfcBlockOffset()) {
// This is not the first time we're here. We already have a suggested BFC
// block offset.
bfc_offset_already_resolved = true;
- child_bfc_offset_estimate = *ConstraintSpace().FloatsBfcBlockOffset();
+ child_bfc_offset_estimate = *ConstraintSpace().ForcedBfcBlockOffset();
// We require that the BFC block offset be the one we'd get with either
// margins adjoining or margins separated. Anything else is a bug.
DCHECK(child_bfc_offset_estimate == adjoining_bfc_offset_estimate ||
@@ -908,9 +923,6 @@
// margin strut or not.
child_margin_got_separated =
child_bfc_offset_estimate != adjoining_bfc_offset_estimate;
- } else if (has_clearance_past_adjoining_floats) {
- child_bfc_offset_estimate = NextBorderEdge(*previous_inflow_position);
- child_margin_got_separated = true;
}
// The BFC block offset of this container gets resolved because of this
@@ -962,8 +974,15 @@
// re-resolve the BFC block offset without the child's margin.
LayoutUnit old_offset = *container_builder_.BfcBlockOffset();
container_builder_.ResetBfcBlockOffset();
+
+ // Re-resolving the BFC block-offset with a different "forced" BFC
+ // block-offset is only safe if an ancestor *never* had clearance past
+ // adjoining floats.
+ DCHECK(!ConstraintSpace().AncestorHasClearancePastAdjoiningFloats());
ResolveBfcBlockOffset(previous_inflow_position,
- non_adjoining_bfc_offset_estimate);
+ non_adjoining_bfc_offset_estimate,
+ /* forced_bfc_block_offset */ base::nullopt);
+
if ((bfc_offset_already_resolved || had_pending_floats) &&
old_offset != *container_builder_.BfcBlockOffset()) {
// The first BFC block offset resolution turned out to be wrong, and we
@@ -1064,7 +1083,7 @@
*previous_inflow_position = ComputeInflowPosition(
*previous_inflow_position, child, child_data,
child_bfc_offset.block_offset, logical_offset, *layout_result, fragment,
- /* empty_block_affected_by_clearance */ false);
+ /* empty_block_had_clearance */ false);
return true;
}
@@ -1138,8 +1157,8 @@
(opportunity.rect.InlineSize() - inline_negative_margin + inline_margin)
.ClampNegativeToZero(),
child_available_size_.block_size};
- NGConstraintSpace child_space =
- CreateConstraintSpaceForChild(child, child_data, child_available_size);
+ NGConstraintSpace child_space = CreateConstraintSpaceForChild(
+ child, child_data, child_available_size, /* is_new_fc */ true);
// All formatting context roots (like this child) should start with an empty
// exclusion space.
@@ -1193,10 +1212,12 @@
bool is_non_empty_inline =
child_inline_node && !child_inline_node->IsEmptyInline();
bool has_clearance_past_adjoining_floats =
- child.IsBlock() &&
+ !container_builder_.BfcBlockOffset() && child.IsBlock() &&
HasClearancePastAdjoiningFloats(container_builder_.AdjoiningFloatTypes(),
child.Style(), Style());
+ base::Optional<LayoutUnit> forced_bfc_block_offset;
+
// If we can separate the previous margin strut from what is to follow, do
// that. Then we're able to resolve *our* BFC block offset and position any
// pending floats. There are two situations where this is necessary:
@@ -1208,14 +1229,25 @@
if (has_clearance_past_adjoining_floats || is_non_empty_inline) {
if (!ResolveBfcBlockOffset(previous_inflow_position))
return false;
+
+ // If we had clearance past any adjoining floats, we already know where the
+ // child is going to be (the child's margins won't have any effect).
+ //
+ // Set the forced BFC block-offset to the appropriate clearance offset to
+ // force this placement of this child.
+ if (has_clearance_past_adjoining_floats) {
+ forced_bfc_block_offset = exclusion_space_.ClearanceOffset(
+ ResolvedClear(child.Style(), Style()));
+ }
}
// Perform layout on the child.
- NGInflowChildData child_data = ComputeChildData(
- *previous_inflow_position, child, child_break_token,
- has_clearance_past_adjoining_floats, /* is_new_fc */ false);
- NGConstraintSpace child_space =
- CreateConstraintSpaceForChild(child, child_data, child_available_size_);
+ NGInflowChildData child_data =
+ ComputeChildData(*previous_inflow_position, child, child_break_token,
+ /* is_new_fc */ false);
+ NGConstraintSpace child_space = CreateConstraintSpaceForChild(
+ child, child_data, child_available_size_, /* is_new_fc */ false,
+ forced_bfc_block_offset, has_clearance_past_adjoining_floats);
scoped_refptr<const NGLayoutResult> layout_result =
child.Layout(child_space, child_break_token, inline_child_layout_context);
@@ -1223,6 +1255,7 @@
// above, the rest of this function is continued within |FinishInflow|.
// However it should be read as one function.
return FinishInflow(child, child_break_token, child_space,
+ has_clearance_past_adjoining_floats,
std::move(layout_result), &child_data,
previous_inflow_position, inline_child_layout_context,
previous_inline_break_token);
@@ -1232,6 +1265,7 @@
NGLayoutInputNode child,
const NGBreakToken* child_break_token,
const NGConstraintSpace& child_space,
+ bool has_clearance_past_adjoining_floats,
scoped_refptr<const NGLayoutResult> layout_result,
NGInflowChildData* child_data,
NGPreviousInflowPosition* previous_inflow_position,
@@ -1239,6 +1273,7 @@
scoped_refptr<const NGInlineBreakToken>* previous_inline_break_token) {
base::Optional<LayoutUnit> child_bfc_block_offset =
layout_result->BfcBlockOffset();
+
// TODO(layout-dev): A more optimal version of this is to set
// relayout_child_when_bfc_resolved only if the child tree itself _added_ any
// floats that it failed to position. Currently, we risk relaying out the
@@ -1246,39 +1281,55 @@
// distinction.
bool relayout_child_when_bfc_resolved =
layout_result->AdjoiningFloatTypes() && !child_bfc_block_offset &&
- !child_space.FloatsBfcBlockOffset();
- bool is_empty_block = IsEmptyBlock(child_space, *layout_result);
+ !child_space.ForcedBfcBlockOffset();
- bool has_clearance = layout_result->IsPushedByFloats();
+ bool is_empty_block = IsEmptyBlock(*layout_result);
- // A child may have aborted its layout if it resolved its BFC block offset.
- // If we don't have a BFC block offset yet, we need to propagate the abortion
- // up to our parent.
+ // Only non-empty children can be pushed by floats in this way.
+ bool non_empty_block_had_clearance = layout_result->IsPushedByFloats();
+ DCHECK(!non_empty_block_had_clearance || !is_empty_block);
+
+ // A child may have aborted its layout if it resolved its BFC block-offset.
+ // If we don't have a BFC block-offset yet, we need to propagate the abort
+ // signal up to our parent.
if (layout_result->Status() == NGLayoutResult::kBfcBlockOffsetResolved &&
!container_builder_.BfcBlockOffset()) {
- // There's no need to do anything apart from resolving the BFC block offset
+ // There's no need to do anything apart from resolving the BFC block-offset
// here, so make sure that it aborts before trying to position floats or
- // anything like that, which would just be waste of time. This is simply
- // propagating an abort up to a node which is able to restart the layout (a
- // node that has resolved its BFC block offset).
+ // anything like that, which would just be waste of time.
+ //
+ // This is simply propagating an abort up to a node which is able to
+ // restart the layout (a node that has resolved its BFC block-offset).
DCHECK(child_bfc_block_offset);
abort_when_bfc_block_offset_updated_ = true;
- ResolveBfcBlockOffset(previous_inflow_position,
- has_clearance
- ? NextBorderEdge(*previous_inflow_position)
- : *child_bfc_block_offset);
+
+ LayoutUnit bfc_block_offset = *child_bfc_block_offset;
+
+ if (non_empty_block_had_clearance) {
+ // If the child has the same clearance-offset as ourselves it means that
+ // we should *also* resolve ourselves at that offset, (and we also have
+ // been pushed by floats).
+ if (ConstraintSpace().ClearanceOffset() == child_space.ClearanceOffset())
+ container_builder_.SetIsPushedByFloats();
+ else
+ bfc_block_offset = NextBorderEdge(*previous_inflow_position);
+ }
+
+ ResolveBfcBlockOffset(previous_inflow_position, bfc_block_offset);
return false;
}
// We have special behaviour for an empty block which gets pushed down due to
- // clearance, see comment inside ComputeInflowPosition.
- bool empty_block_affected_by_clearance = false;
+ // clearance, see comment inside |ComputeInflowPosition|.
+ bool empty_block_had_clearance =
+ is_empty_block && has_clearance_past_adjoining_floats;
// We try and position the child within the block formatting context. This
// may cause our BFC block offset to be resolved, in which case we should
// abort our layout if needed.
if (!child_bfc_block_offset) {
- if (!has_clearance && child_space.HasClearanceOffset() &&
+ DCHECK(is_empty_block);
+ if (child_space.HasClearanceOffset() &&
child.Style().Clear() != EClear::kNone) {
// This is an empty block child that we collapsed through, so we have to
// detect clearance manually. See if the child's hypothetical border edge
@@ -1286,12 +1337,14 @@
// before it.
LayoutUnit child_block_offset_estimate =
BfcBlockOffset() + layout_result->EndMarginStrut().Sum();
- if (child_block_offset_estimate < child_space.ClearanceOffset() ||
- child_space.ShouldForceClearance())
- has_clearance = empty_block_affected_by_clearance = true;
+ if (child_block_offset_estimate < child_space.ClearanceOffset())
+ empty_block_had_clearance = true;
}
}
- if (has_clearance) {
+
+ bool child_had_clearance =
+ empty_block_had_clearance || non_empty_block_had_clearance;
+ if (child_had_clearance) {
// The child has clearance. Clearance inhibits margin collapsing and acts as
// spacing before the block-start margin of the child. Our BFC block offset
// is therefore resolvable, and if it hasn't already been resolved, we'll
@@ -1299,27 +1352,34 @@
if (!ResolveBfcBlockOffset(previous_inflow_position))
return false;
}
+
if (!child_bfc_block_offset) {
DCHECK(is_empty_block);
+
// Layout wasn't able to determine the BFC block offset of the child. This
// has to mean that the child is empty (block-size-wise).
if (container_builder_.BfcBlockOffset()) {
// Since we know our own BFC block offset, though, we can calculate that
// of the child as well.
- child_bfc_block_offset = PositionEmptyChildWithParentBfc(
+ child_bfc_block_offset = PositionEmptyBlockWithParentBfc(
child, child_space, *child_data, *layout_result);
}
- } else if (!has_clearance) {
+ } else if (!child_had_clearance) {
// We shouldn't have any pending floats here, since an in-flow child found
// its BFC block offset.
DCHECK(unpositioned_floats_.IsEmpty());
- // The child's BFC block offset is known, and since there's no clearance,
- // this container will get the same offset, unless it has already been
- // resolved.
- if (!ResolveBfcBlockOffset(previous_inflow_position,
- *child_bfc_block_offset))
- return false;
+ // Only non-empty children are allowed resolve the BFC block-offset. We
+ // check the BFC block-offset at the end of layout determine if this
+ // fragment is empty.
+ if (!is_empty_block) {
+ // The child's BFC block offset is known, and since there's no clearance,
+ // this container will get the same offset, unless it has already been
+ // resolved.
+ if (!ResolveBfcBlockOffset(previous_inflow_position,
+ *child_bfc_block_offset))
+ return false;
+ }
}
// We need to re-layout a child if it was affected by clearance in order to
@@ -1337,9 +1397,9 @@
// relayout with an empty incoming margin strut.
//
// The resulting margin strut in the above example will be {40, -30}. See
- // ComputeInflowPosition for how this end margin strut is used.
- bool empty_block_affected_by_clearance_needs_relayout = false;
- if (empty_block_affected_by_clearance) {
+ // |ComputeInflowPosition| for how this end margin strut is used.
+ bool empty_block_had_clearance_needs_relayout = false;
+ if (empty_block_had_clearance) {
NGMarginStrut margin_strut;
margin_strut.Append(child_data->margins.block_start,
child.Style().HasMarginBeforeQuirk());
@@ -1348,7 +1408,7 @@
// previous one.
if (child_data->margin_strut != margin_strut) {
child_data->margin_strut = margin_strut;
- empty_block_affected_by_clearance_needs_relayout = true;
+ empty_block_had_clearance_needs_relayout = true;
}
}
@@ -1358,10 +1418,11 @@
// - It was affected by clearance.
if ((layout_result->Status() == NGLayoutResult::kBfcBlockOffsetResolved ||
relayout_child_when_bfc_resolved ||
- empty_block_affected_by_clearance_needs_relayout) &&
+ empty_block_had_clearance_needs_relayout) &&
child_bfc_block_offset) {
NGConstraintSpace new_child_space = CreateConstraintSpaceForChild(
- child, *child_data, child_available_size_, child_bfc_block_offset);
+ child, *child_data, child_available_size_, /* is_new_fc */ false,
+ child_bfc_block_offset);
layout_result = child.Layout(new_child_space, child_break_token,
inline_child_layout_context);
@@ -1374,7 +1435,8 @@
child_bfc_block_offset = layout_result->BfcBlockOffset();
DCHECK(child_bfc_block_offset);
new_child_space = CreateConstraintSpaceForChild(
- child, *child_data, child_available_size_, child_bfc_block_offset);
+ child, *child_data, child_available_size_, /* is_new_fc */ false,
+ child_bfc_block_offset);
layout_result = child.Layout(new_child_space, child_break_token,
inline_child_layout_context);
}
@@ -1383,9 +1445,15 @@
relayout_child_when_bfc_resolved = false;
}
- // It is now safe to update our version of the exclusion space.
+ // It is now safe to update our version of the exclusion space, and any
+ // propagated adjoining floats.
exclusion_space_ = layout_result->ExclusionSpace();
+ // Only empty-blocks should have adjoining floats.
+ DCHECK(!layout_result->AdjoiningFloatTypes() || is_empty_block);
+ container_builder_.SetAdjoiningFloatTypes(
+ layout_result->AdjoiningFloatTypes());
+
// If we don't know our BFC block offset yet, and the child stumbled into
// something that needs it (unable to position floats when the BFC block
// offset is unknown), we need abort layout once we manage to resolve it, and
@@ -1425,11 +1493,6 @@
PositionOrPropagateListMarker(*layout_result, &logical_offset);
- if (is_empty_block && !container_builder_.BfcBlockOffset()) {
- container_builder_.AddAdjoiningFloatTypes(
- layout_result->AdjoiningFloatTypes());
- }
-
container_builder_.AddChild(physical_fragment, logical_offset);
if (child.IsBlock())
container_builder_.PropagateBreak(*layout_result);
@@ -1451,8 +1514,7 @@
*previous_inflow_position = ComputeInflowPosition(
*previous_inflow_position, child, *child_data, child_bfc_block_offset,
- logical_offset, *layout_result, fragment,
- empty_block_affected_by_clearance);
+ logical_offset, *layout_result, fragment, empty_block_had_clearance);
*previous_inline_break_token =
child.IsInline() ? To<NGInlineBreakToken>(physical_fragment.BreakToken())
@@ -1465,7 +1527,6 @@
const NGPreviousInflowPosition& previous_inflow_position,
NGLayoutInputNode child,
const NGBreakToken* child_break_token,
- bool force_clearance,
bool is_new_fc) {
DCHECK(child);
DCHECK(!child.IsFloating());
@@ -1500,8 +1561,7 @@
margins.LineLeft(ConstraintSpace().Direction()),
BfcBlockOffset() + logical_block_offset};
- return {child_bfc_offset, margin_strut, margins,
- margins_fully_resolved, force_clearance, is_new_fc};
+ return {child_bfc_offset, margin_strut, margins, margins_fully_resolved};
}
NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition(
@@ -1512,17 +1572,17 @@
const LogicalOffset& logical_offset,
const NGLayoutResult& layout_result,
const NGFragment& fragment,
- bool empty_block_affected_by_clearance) {
+ bool empty_block_had_clearance) {
// Determine the child's end logical offset, for the next child to use.
LayoutUnit logical_block_offset;
- bool is_empty_block = IsEmptyBlock(child_data.is_new_fc, layout_result);
+ bool is_empty_block = IsEmptyBlock(layout_result);
if (is_empty_block) {
// The default behaviour for empty blocks is they just pass through the
// previous inflow position.
logical_block_offset = previous_inflow_position.logical_block_offset;
- if (empty_block_affected_by_clearance) {
+ if (empty_block_had_clearance) {
// If there's clearance, we must have applied that by now and thus
// resolved our BFC block offset.
DCHECK(container_builder_.BfcBlockOffset());
@@ -1603,21 +1663,20 @@
// </div>
// In the above case #container's size will depend on the end margin strut of
// #another-zero, even though usually it wouldn't.
- bool empty_or_sibling_empty_affected_by_clearance =
- empty_block_affected_by_clearance ||
- (previous_inflow_position.empty_block_affected_by_clearance &&
- is_empty_block);
+ bool empty_or_sibling_empty_had_clearance =
+ empty_block_had_clearance ||
+ (previous_inflow_position.empty_block_had_clearance && is_empty_block);
return {logical_block_offset, margin_strut,
- empty_or_sibling_empty_affected_by_clearance};
+ empty_or_sibling_empty_had_clearance};
}
-LayoutUnit NGBlockLayoutAlgorithm::PositionEmptyChildWithParentBfc(
+LayoutUnit NGBlockLayoutAlgorithm::PositionEmptyBlockWithParentBfc(
const NGLayoutInputNode& child,
const NGConstraintSpace& child_space,
const NGInflowChildData& child_data,
const NGLayoutResult& layout_result) const {
- DCHECK(IsEmptyBlock(child_space, layout_result));
+ DCHECK(IsEmptyBlock(layout_result));
// The child must be an in-flow zero-block-size fragment, use its end margin
// strut for positioning.
@@ -1990,14 +2049,16 @@
const NGLayoutInputNode child,
const NGInflowChildData& child_data,
const LogicalSize child_available_size,
- const base::Optional<LayoutUnit> floats_bfc_block_offset) {
+ bool is_new_fc,
+ const base::Optional<LayoutUnit> forced_bfc_block_offset,
+ bool has_clearance_past_adjoining_floats) {
const ComputedStyle& style = Style();
const ComputedStyle& child_style = child.Style();
WritingMode child_writing_mode =
child.IsInline() ? style.GetWritingMode() : child_style.GetWritingMode();
NGConstraintSpaceBuilder builder(ConstraintSpace(), child_writing_mode,
- child_data.is_new_fc);
+ is_new_fc);
SetOrthogonalFallbackInlineSizeIfNeeded(Style(), child, &builder);
if (!IsParallelWritingMode(ConstraintSpace().GetWritingMode(),
@@ -2022,13 +2083,22 @@
builder.SetBfcOffset(child_data.bfc_offset_estimate)
.SetMarginStrut(child_data.margin_strut);
- if (!container_builder_.BfcBlockOffset() &&
- ConstraintSpace().FloatsBfcBlockOffset()) {
- builder.SetFloatsBfcBlockOffset(*ConstraintSpace().FloatsBfcBlockOffset());
- }
+ bool has_bfc_block_offset = container_builder_.BfcBlockOffset().has_value();
- if (floats_bfc_block_offset)
- builder.SetFloatsBfcBlockOffset(floats_bfc_block_offset);
+ // Propagate the |NGConstraintSpace::ForcedBfcBlockOffset| down to our
+ // children.
+ if (!has_bfc_block_offset && ConstraintSpace().ForcedBfcBlockOffset())
+ builder.SetForcedBfcBlockOffset(*ConstraintSpace().ForcedBfcBlockOffset());
+ if (forced_bfc_block_offset)
+ builder.SetForcedBfcBlockOffset(forced_bfc_block_offset);
+
+ // Propagate the |NGConstraintSpace::AncestorHasClearancePastAdjoiningFloats|
+ // flag down to our children.
+ if (!has_bfc_block_offset &&
+ ConstraintSpace().AncestorHasClearancePastAdjoiningFloats())
+ builder.SetAncestorHasClearancePastAdjoiningFloats();
+ if (has_clearance_past_adjoining_floats)
+ builder.SetAncestorHasClearancePastAdjoiningFloats();
LayoutUnit clearance_offset = ConstraintSpace().IsNewFormattingContext()
? LayoutUnit::Min()
@@ -2049,15 +2119,10 @@
}
builder.SetClearanceOffset(clearance_offset);
- // If we were told to force clearance, we need to propagate this down to our
- // children if we haven't resolved our BFC-block-offset yet.
- if (child_data.force_clearance || (!container_builder_.BfcBlockOffset() &&
- ConstraintSpace().ShouldForceClearance()))
- builder.SetShouldForceClearance(true);
-
- if (!child_data.is_new_fc) {
+ if (!is_new_fc) {
builder.SetExclusionSpace(exclusion_space_);
- builder.SetAdjoiningFloatTypes(container_builder_.AdjoiningFloatTypes());
+ if (!has_bfc_block_offset)
+ builder.SetAdjoiningFloatTypes(container_builder_.AdjoiningFloatTypes());
}
LayoutUnit space_available;
@@ -2066,9 +2131,9 @@
// If a block establishes a new formatting context we must know our
// position in the formatting context, and are able to adjust the
// fragmentation line.
- if (child_data.is_new_fc) {
+ if (is_new_fc)
space_available -= child_data.bfc_offset_estimate.block_offset;
- }
+
// The policy regarding collapsing block-start margin with the fragmentainer
// block-start is the same throughout the entire fragmentainer (although it
// really only matters at the beginning of each fragmentainer, we don't need
@@ -2177,17 +2242,19 @@
bool NGBlockLayoutAlgorithm::ResolveBfcBlockOffset(
NGPreviousInflowPosition* previous_inflow_position,
- LayoutUnit bfc_block_offset) {
+ LayoutUnit bfc_block_offset,
+ base::Optional<LayoutUnit> forced_bfc_block_offset) {
if (container_builder_.BfcBlockOffset()) {
DCHECK(unpositioned_floats_.IsEmpty());
return true;
}
+ bfc_block_offset = forced_bfc_block_offset.value_or(bfc_block_offset);
+
if (ApplyClearance(ConstraintSpace(), &bfc_block_offset))
container_builder_.SetIsPushedByFloats();
container_builder_.SetBfcBlockOffset(bfc_block_offset);
- container_builder_.ResetAdjoiningFloatTypes();
if (NeedsAbortOnBfcBlockOffsetChange())
return false;
@@ -2217,11 +2284,11 @@
if (!abort_when_bfc_block_offset_updated_)
return false;
// If no previous BFC block offset was set, we need to abort.
- if (!ConstraintSpace().FloatsBfcBlockOffset())
+ if (!ConstraintSpace().ForcedBfcBlockOffset())
return true;
// If the previous BFC block offset matches the new one, we can continue.
// Otherwise, we need to abort.
- LayoutUnit old_bfc_block_offset = *ConstraintSpace().FloatsBfcBlockOffset();
+ LayoutUnit old_bfc_block_offset = *ConstraintSpace().ForcedBfcBlockOffset();
// In order to determine if the two offsets are equal, we also need to adjust
// floats offset by the clearance offset.
@@ -2232,7 +2299,7 @@
void NGBlockLayoutAlgorithm::PositionPendingFloats(
LayoutUnit origin_block_offset) {
DCHECK(container_builder_.BfcBlockOffset() ||
- ConstraintSpace().FloatsBfcBlockOffset())
+ ConstraintSpace().ForcedBfcBlockOffset())
<< "The parent BFC block offset should be known here.";
NGBfcOffset origin_bfc_offset = {
@@ -2242,7 +2309,7 @@
LayoutUnit bfc_block_offset = container_builder_.BfcBlockOffset()
? *container_builder_.BfcBlockOffset()
- : *ConstraintSpace().FloatsBfcBlockOffset();
+ : *ConstraintSpace().ForcedBfcBlockOffset();
NGBfcOffset bfc_offset = {ConstraintSpace().BfcOffset().line_offset,
bfc_block_offset};
diff --git a/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.h b/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.h
index 7351989..2f54670 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.h
@@ -29,7 +29,7 @@
struct NGPreviousInflowPosition {
LayoutUnit logical_block_offset;
NGMarginStrut margin_strut;
- bool empty_block_affected_by_clearance;
+ bool empty_block_had_clearance;
};
// This strut holds information for the current inflow child. The data is not
@@ -39,8 +39,6 @@
NGMarginStrut margin_strut;
NGBoxStrut margins;
bool margins_fully_resolved;
- bool force_clearance;
- bool is_new_fc;
};
// A class for general block layout (e.g. a <div> with no special style).
@@ -102,13 +100,14 @@
const NGLayoutInputNode child,
const NGInflowChildData& child_data,
const LogicalSize child_available_size,
- const base::Optional<LayoutUnit> floats_bfc_block_offset = base::nullopt);
+ bool is_new_fc,
+ const base::Optional<LayoutUnit> forced_bfc_block_offset = base::nullopt,
+ bool has_clearance_past_adjoining_floats = false);
// @return Estimated BFC block offset for the "to be layout" child.
NGInflowChildData ComputeChildData(const NGPreviousInflowPosition&,
NGLayoutInputNode,
const NGBreakToken* child_break_token,
- bool force_clearance,
bool is_new_fc);
NGPreviousInflowPosition ComputeInflowPosition(
@@ -119,16 +118,16 @@
const LogicalOffset&,
const NGLayoutResult&,
const NGFragment&,
- bool empty_block_affected_by_clearance);
+ bool empty_block_had_clearance);
- // Position an empty child using the parent BFC block offset.
+ // Position an empty-block using the parent BFC block offset.
// The fragment doesn't know its offset, but we can still calculate its BFC
// position because the parent fragment's BFC is known.
// Example:
// BFC Offset is known here because of the padding.
// <div style="padding: 1px">
// <div id="empty-div" style="margin: 1px"></div>
- LayoutUnit PositionEmptyChildWithParentBfc(
+ LayoutUnit PositionEmptyBlockWithParentBfc(
const NGLayoutInputNode& child,
const NGConstraintSpace& child_space,
const NGInflowChildData& child_data,
@@ -192,6 +191,7 @@
NGLayoutInputNode child,
const NGBreakToken* child_break_token,
const NGConstraintSpace&,
+ bool has_clearance_past_adjoining_floats,
scoped_refptr<const NGLayoutResult>,
NGInflowChildData*,
NGPreviousInflowPosition*,
@@ -246,18 +246,35 @@
// If still unresolved, resolve the fragment's BFC block offset.
//
- // This includes applying clearance, so the bfc_block_offset passed won't be
- // the final BFC block offset, if it wasn't large enough to get past all
- // relevant floats. The updated BFC block offset can be read out with
- // ContainerBfcBlockOffset().
+ // This includes applying clearance, so the |bfc_block_offset| passed won't
+ // be the final BFC block-offset, if it wasn't large enough to get past all
+ // relevant floats. The updated BFC block-offset can be read out with
+ // |ContainerBfcBlockOffset()|.
+ //
+ // If the |forced_bfc_block_offset| has a value, it will override the given
+ // |bfc_block_offset|. Typically this comes from the input constraints, when
+ // the current node has clearance past adjoining floats, or has a re-layout
+ // due to a child resolving the BFC block-offset.
//
// In addition to resolving our BFC block offset, this will also position
- // pending floats, and update our in-flow layout state. Returns false if
- // resolving the BFC block offset resulted in needing to abort layout. It
- // will always return true otherwise. If the BFC block offset was already
- // resolved, this method does nothing (and returns true).
- bool ResolveBfcBlockOffset(NGPreviousInflowPosition*,
- LayoutUnit bfc_block_offset);
+ // pending floats, and update our in-flow layout state.
+ //
+ // Returns false if resolving the BFC block-offset resulted in needing to
+ // abort layout. It will always return true otherwise. If the BFC
+ // block-offset was already resolved, this method does nothing (and returns
+ // true).
+ bool ResolveBfcBlockOffset(
+ NGPreviousInflowPosition*,
+ LayoutUnit bfc_block_offset,
+ const base::Optional<LayoutUnit> forced_bfc_block_offset);
+
+ // This passes in the |forced_bfc_block_offset| from the input constraints,
+ // which is almost always desired.
+ bool ResolveBfcBlockOffset(NGPreviousInflowPosition* previous_inflow_position,
+ LayoutUnit bfc_block_offset) {
+ return ResolveBfcBlockOffset(previous_inflow_position, bfc_block_offset,
+ ConstraintSpace().ForcedBfcBlockOffset());
+ }
// A very common way to resolve the BFC block offset is to simply commit the
// pending margin, so here's a convenience overload for that.
diff --git a/third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h b/third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h
index d8663099..875b903 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h
@@ -240,6 +240,7 @@
bool did_break_;
bool has_forced_break_ = false;
bool is_new_fc_ = false;
+ bool is_empty_block_ = false;
LayoutUnit used_block_size_;
LayoutUnit minimal_space_shortage_ = LayoutUnit::Max();
diff --git a/third_party/blink/renderer/core/layout/ng/ng_constraint_space.h b/third_party/blink/renderer/core/layout/ng/ng_constraint_space.h
index 410c459..7937d6e 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_constraint_space.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_constraint_space.h
@@ -66,7 +66,7 @@
kNewFormattingContext = 1 << 4,
kAnonymous = 1 << 5,
kUseFirstLineStyle = 1 << 6,
- kForceClearance = 1 << 7,
+ kAncestorHasClearancePastAdjoiningFloats = 1 << 7,
// Size of bitfield used to store the flags.
kNumberOfConstraintSpaceFlags = 8
@@ -276,6 +276,16 @@
// Also note this is true only when the document has ':first-line' rules.
bool UseFirstLineStyle() const { return HasFlag(kUseFirstLineStyle); }
+ // Returns true if an ancestor had clearance past adjoining floats.
+ //
+ // Typically this can be detected by seeing if a |ForcedBfcBlockOffset| is
+ // set. However new formatting contexts may require additional passes (if
+ // margins are adjoining or not), and without this extra bit of information
+ // can get into a bad state.
+ bool AncestorHasClearancePastAdjoiningFloats() const {
+ return HasFlag(kAncestorHasClearancePastAdjoiningFloats);
+ }
+
// Some layout modes “stretch” their children to a fixed size (e.g. flex,
// grid). These flags represented whether a layout needs to produce a
// fragment that satisfies a fixed constraint in the inline and block
@@ -346,27 +356,28 @@
return HasRareData() ? rare_data_->bfc_offset : bfc_offset_;
}
- // If present, and the current layout hasn't resolved its BFC offset yet (see
- // BfcOffset), the layout should position all of its unpositioned floats at
- // this offset. This value is the BFC offset that we calculated in the
- // previous pass, a pass which aborted once the BFC offset got resolved,
- // because we had walked past content (i.e. floats) that depended on it being
- // resolved.
+ // If present, and the current layout hasn't resolved its BFC block-offset
+ // yet (see BfcOffset), the layout should position all of its unpositioned
+ // floats at this offset.
//
- // This value should be propogated to child layouts if the current layout
+ // This value is present if:
+ // - An ancestor had clearance past adjoining floats. In this case this
+ // value is calculated ahead of time.
+ // - A second layout pass is required as there were unpositioned floats
+ // within the tree, and an arbitrary sibling determined their BFC
+ // block-offset.
+ //
+ // This value should be propagated to child layouts if the current layout
// hasn't resolved its BFC offset yet.
- //
- // This value is calculated *after* an initial pass of the tree, and should
- // only be present during subsequent passes.
- base::Optional<LayoutUnit> FloatsBfcBlockOffset() const {
- return HasRareData() ? rare_data_->floats_bfc_block_offset : base::nullopt;
+ base::Optional<LayoutUnit> ForcedBfcBlockOffset() const {
+ return HasRareData() ? rare_data_->forced_bfc_block_offset : base::nullopt;
}
// Return the types (none, left, right, both) of preceding adjoining
// floats. These are floats that are added while the in-flow BFC offset is
// still unknown. The floats may or may not be unpositioned (pending). That
// depends on which layout pass we're in. They are typically positioned if
- // FloatsBfcOffset() is known. Adjoining floats should be treated differently
+ // ForcedBfcOffset() is known. Adjoining floats should be treated differently
// when calculating clearance on a block with adjoining block-start margin.
// (in such cases we will know up front that the block will need clearance,
// since, if it doesn't, the float will be pulled along with the block, and
@@ -386,28 +397,6 @@
return HasRareData() ? rare_data_->clearance_offset : LayoutUnit::Min();
}
- // Return true if the fragment needs to have clearance applied to it,
- // regardless of its hypothetical position. The fragment will then go exactly
- // below the relevant floats. This happens when a cleared child gets separated
- // from floats that would otherwise be adjoining; example:
- //
- // <div id="container">
- // <div id="float" style="float:left; width:100px; height:100px;"></div>
- // <div id="clearee" style="clear:left; margin-top:12345px;">text</div>
- // </div>
- //
- // Clearance separates #clearee from #container, and #float is positioned at
- // the block-start content edge of #container. Without clearance, margins
- // would have been adjoining and the large margin on #clearee would have
- // pulled both #container and #float along with it. No margin, no matter how
- // large, would ever be able to pull #clearee below the float then. But we
- // have clearance, the margins are separated, and in this case we know that we
- // have clearance even before we have laid out (because of the adjoining
- // float). So it would just be wrong to check for clearance when we position
- // #clearee. Nothing can prevent clearance here. A large margin on the cleared
- // child will be canceled out with negative clearance.
- bool ShouldForceClearance() const { return HasFlag(kForceClearance); }
-
const NGBaselineRequestList BaselineRequests() const {
return NGBaselineRequestList(bitfields_.baseline_requests);
}
@@ -517,7 +506,7 @@
NGBfcOffset bfc_offset;
NGMarginStrut margin_strut;
- base::Optional<LayoutUnit> floats_bfc_block_offset;
+ base::Optional<LayoutUnit> forced_bfc_block_offset;
LayoutUnit clearance_offset = LayoutUnit::Min();
LayoutUnit fragmentainer_block_size = kIndefiniteSize;
@@ -527,7 +516,7 @@
bool MaySkipLayout(const RareData& other) const {
return margin_strut == other.margin_strut &&
- floats_bfc_block_offset == other.floats_bfc_block_offset &&
+ forced_bfc_block_offset == other.forced_bfc_block_offset &&
fragmentainer_block_size == other.fragmentainer_block_size &&
fragmentainer_space_at_bfc_start ==
other.fragmentainer_space_at_bfc_start &&
@@ -538,7 +527,7 @@
// Must be kept in sync with members checked within |MaySkipLayout|.
bool IsInitialForMaySkipLayout() const {
return margin_strut == NGMarginStrut() &&
- floats_bfc_block_offset == base::nullopt &&
+ forced_bfc_block_offset == base::nullopt &&
fragmentainer_block_size == kIndefiniteSize &&
fragmentainer_space_at_bfc_start == kIndefiniteSize &&
block_direction_fragmentation_type == kFragmentNone;
diff --git a/third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h b/third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h
index 8204ec1f..919cc16 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h
@@ -184,6 +184,11 @@
return *this;
}
+ NGConstraintSpaceBuilder& SetAncestorHasClearancePastAdjoiningFloats() {
+ SetFlag(NGConstraintSpace::kAncestorHasClearancePastAdjoiningFloats, true);
+ return *this;
+ }
+
NGConstraintSpaceBuilder& SetAdjoiningFloatTypes(NGFloatTypes floats) {
if (!is_new_fc_)
space_.bitfields_.adjoining_floats = static_cast<unsigned>(floats);
@@ -212,15 +217,15 @@
return *this;
}
- NGConstraintSpaceBuilder& SetFloatsBfcBlockOffset(
- const base::Optional<LayoutUnit>& floats_bfc_block_offset) {
+ NGConstraintSpaceBuilder& SetForcedBfcBlockOffset(
+ const base::Optional<LayoutUnit>& forced_bfc_block_offset) {
#if DCHECK_IS_ON()
- DCHECK(!is_floats_bfc_block_offset_set_);
- is_floats_bfc_block_offset_set_ = true;
+ DCHECK(!is_forced_bfc_block_offset_set_);
+ is_forced_bfc_block_offset_set_ = true;
#endif
- if (LIKELY(!is_new_fc_ && floats_bfc_block_offset != base::nullopt)) {
- space_.EnsureRareData()->floats_bfc_block_offset =
- floats_bfc_block_offset;
+ if (LIKELY(!is_new_fc_ && forced_bfc_block_offset != base::nullopt)) {
+ space_.EnsureRareData()->forced_bfc_block_offset =
+ forced_bfc_block_offset;
}
return *this;
@@ -237,11 +242,6 @@
return *this;
}
- NGConstraintSpaceBuilder& SetShouldForceClearance(bool b) {
- SetFlag(NGConstraintSpace::kForceClearance, b);
- return *this;
- }
-
NGConstraintSpaceBuilder& SetTableCellChildLayoutPhase(
NGTableCellChildLayoutPhase table_cell_child_layout_phase) {
space_.bitfields_.table_cell_child_layout_phase =
@@ -312,7 +312,7 @@
bool is_fragmentainer_space_at_bfc_start_set_ = false;
bool is_block_direction_fragmentation_type_set_ = false;
bool is_margin_strut_set_ = false;
- bool is_floats_bfc_block_offset_set_ = false;
+ bool is_forced_bfc_block_offset_set_ = false;
bool is_clearance_offset_set_ = false;
bool to_constraint_space_called_ = false;
diff --git a/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h b/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h
index 8410fe4..c3c91cf 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h
@@ -148,6 +148,11 @@
nullptr);
}
+ NGContainerFragmentBuilder& SetIsEmptyBlock() {
+ is_empty_block_ = true;
+ return *this;
+ }
+
NGContainerFragmentBuilder& SetIsPushedByFloats() {
is_pushed_by_floats_ = true;
return *this;
@@ -162,6 +167,10 @@
adjoining_floats_ |= floats;
return *this;
}
+ NGContainerFragmentBuilder& SetAdjoiningFloatTypes(NGFloatTypes floats) {
+ adjoining_floats_ = floats;
+ return *this;
+ }
NGFloatTypes AdjoiningFloatTypes() const { return adjoining_floats_; }
NGContainerFragmentBuilder& SetHasBlockFragmentation() {
@@ -244,6 +253,7 @@
NGFloatTypes adjoining_floats_ = kFloatTypeNone;
+ bool is_empty_block_ = false;
bool is_pushed_by_floats_ = false;
bool is_legacy_layout_root_ = false;
diff --git a/third_party/blink/renderer/core/layout/ng/ng_layout_result.cc b/third_party/blink/renderer/core/layout/ng/ng_layout_result.cc
index aed07c4..6b7b91e0 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_layout_result.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_layout_result.cc
@@ -67,6 +67,7 @@
final_break_after_(other.final_break_after_),
has_valid_space_(other.has_valid_space_),
has_forced_break_(other.has_forced_break_),
+ is_empty_block_(other.is_empty_block_),
is_pushed_by_floats_(other.is_pushed_by_floats_),
adjoining_floats_(other.adjoining_floats_),
is_initial_block_size_indefinite_(
@@ -87,6 +88,7 @@
end_margin_strut_(builder->end_margin_strut_),
has_valid_space_(cache_space && builder->space_),
has_forced_break_(false),
+ is_empty_block_(builder->is_empty_block_),
is_pushed_by_floats_(builder->is_pushed_by_floats_),
adjoining_floats_(builder->adjoining_floats_),
is_initial_block_size_indefinite_(false),
@@ -147,6 +149,7 @@
DCHECK_EQ(has_valid_space_, other.has_valid_space_);
DCHECK_EQ(has_forced_break_, other.has_forced_break_);
+ DCHECK_EQ(is_empty_block_, other.is_empty_block_);
DCHECK_EQ(is_pushed_by_floats_, other.is_pushed_by_floats_);
DCHECK_EQ(adjoining_floats_, other.adjoining_floats_);
diff --git a/third_party/blink/renderer/core/layout/ng/ng_layout_result.h b/third_party/blink/renderer/core/layout/ng/ng_layout_result.h
index 3260354..9b91c7b 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_layout_result.h
+++ b/third_party/blink/renderer/core/layout/ng/ng_layout_result.h
@@ -95,6 +95,10 @@
// Return true if the fragment broke because a forced break before a child.
bool HasForcedBreak() const { return has_forced_break_; }
+ // Returns true if the fragment should be considered empty for margin
+ // collapsing purposes (e.g. margins "collapse through").
+ bool IsEmptyBlock() const { return is_empty_block_; }
+
// Return true if this fragment got its block offset increased by the presence
// of floats.
bool IsPushedByFloats() const { return is_pushed_by_floats_; }
@@ -217,6 +221,7 @@
unsigned has_valid_space_ : 1;
unsigned has_forced_break_ : 1;
+ unsigned is_empty_block_ : 1;
unsigned is_pushed_by_floats_ : 1;
unsigned adjoining_floats_ : 2; // NGFloatTypes
diff --git a/third_party/blink/renderer/core/layout/ng/ng_layout_utils.cc b/third_party/blink/renderer/core/layout/ng/ng_layout_utils.cc
index 6aa70c9..1ab4873 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_layout_utils.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_layout_utils.cc
@@ -517,8 +517,7 @@
if (space.IsIntermediateLayout())
return false;
// Check that we're done positioning pending floats.
- return !result.AdjoiningFloatTypes() || result.BfcBlockOffset() ||
- space.FloatsBfcBlockOffset();
+ return !result.AdjoiningFloatTypes() || result.BfcBlockOffset();
}
} // namespace blink
diff --git a/third_party/blink/renderer/core/layout/ng/ng_simplified_layout_algorithm.cc b/third_party/blink/renderer/core/layout/ng/ng_simplified_layout_algorithm.cc
index b817b8d..4cb444c 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_simplified_layout_algorithm.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_simplified_layout_algorithm.cc
@@ -56,9 +56,11 @@
container_builder_.SetIntrinsicBlockSize(result.IntrinsicBlockSize());
container_builder_.SetUnpositionedListMarker(result.UnpositionedListMarker());
- container_builder_.AddAdjoiningFloatTypes(result.AdjoiningFloatTypes());
+ if (result.IsEmptyBlock())
+ container_builder_.SetIsEmptyBlock();
if (result.IsPushedByFloats())
container_builder_.SetIsPushedByFloats();
+ container_builder_.AddAdjoiningFloatTypes(result.AdjoiningFloatTypes());
for (const auto& request : ConstraintSpace().BaselineRequests()) {
base::Optional<LayoutUnit> baseline = physical_fragment.Baseline(request);
diff --git a/third_party/blink/renderer/core/layout/ng/ng_space_utils.cc b/third_party/blink/renderer/core/layout/ng/ng_space_utils.cc
index a51ed9a..ba44dfd 100644
--- a/third_party/blink/renderer/core/layout/ng/ng_space_utils.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_space_utils.cc
@@ -39,7 +39,6 @@
.SetPercentageResolutionSize(indefinite_size)
.SetReplacedPercentageResolutionSize(indefinite_size)
.SetIsIntermediateLayout(true)
- .SetFloatsBfcBlockOffset(LayoutUnit())
.ToConstraintSpace();
}
diff --git a/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-002.html b/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-002.html
new file mode 100644
index 0000000..ed8ffd2
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-002.html
@@ -0,0 +1,75 @@
+<!DOCTYPE html>
+<link rel="help" href="https://www.w3.org/TR/CSS22/visuren.html#flow-control" title="9.5.2 Controlling flow next to floats: the 'clear' property">
+<link rel="match" href="../../reference/ref-filled-green-100px-square-only.html">
+<style>
+#container {
+ width: 100px;
+ background: green;
+}
+#left {
+ float: left;
+ width: 25px;
+ height: 10px;
+}
+#right {
+ float: right;
+ width: 25px;
+ height: 20px;
+}
+#clears-left {
+ clear: left;
+}
+#zero {
+ margin-bottom: 40px;
+ margin-top: -20px;
+}
+#nested-float {
+ float: left;
+ width: 25px;
+ height: 20px;
+}
+#new-formatting-context {
+ overflow: hidden;
+ width: 60px;
+ height: 80px;
+ margin-top: -30px;
+}
+</style>
+<p>Test passes if there is a filled green square.</p>
+<div id=container>
+ <div id=left></div>
+ <div id=right></div>
+ <div>
+ <div id=clears-left>
+ <div>
+ <div id=zero></div>
+ <div id=nested-float></div>
+ <!--
+ The margins up to this new formatting context are chosen to hit an
+ edge condition. At this point there are two possible margins:
+ - (adjoining) {-30px, 40px} => 10px
+ - (non-adjoining) {-20px, 40px} => 20px
+
+ The logic for placing this new formatting context however shouldn't
+ check these margins, as there is an ancestor ("clears-left") which
+ has clearance past adjoining floats ("left", and "right").
+
+ And "nested-float" should get placed at "10px".
+
+ However if we didn't have this logic the following would occur.
+ 1. We'd try and place the formatting context using the "adjoining"
+ margins.
+ 2. The new formatting context doesn't "fit" on the same edge as the
+ floats, so it would trigger a retry using with a new position
+ calculated using the "non-adjoining" margins.
+ 3. During the next pass, it still doesn't think the margins have
+ been separated yet as the parent is still using the position
+ calculated by the forced clearance from above.
+ 4. It will trigger a retry again (and if an implementation does this
+ in a loop, will timeout).
+ -->
+ <div id=new-formatting-context></div>
+ </div>
+ </div>
+ </div>
+</div>
diff --git a/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-003.html b/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-003.html
new file mode 100644
index 0000000..4c080d2b
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/CSS2/floats-clear/adjoining-float-nested-forced-clearance-003.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht" />
+<link rel="help" href="https://www.w3.org/TR/CSS22/visuren.html#flow-control" title="9.5.2 Controlling flow next to floats: the 'clear' property">
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="overflow: hidden; width: 100px; background: red; position: relative;">
+ <div style="float: left; height: 50px; width: 50px; background: green"></div>
+ <div style="height: 40px; background: green;"></div>
+ <div style="margin-top: 15px; clear: both;">
+ <div style="width: 50px; height: 50px; background: green; float: left;"></div>
+ <div style="margin-top: -10px;">
+ <span style="display: inline-block; width: 50px; height: 40px; background: green;"></span>
+ </div>
+ <div style="position: absolute; width: 50px; height: 10px; right: 0; top: 40px; background: green;"></div>
+ <div style="position: absolute; width: 50px; height: 10px; right: 0; bottom: 0; background: green;"></div>
+ </div>
+</div>
