Commit 54862d1e authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[LayoutNG] Fix crash related to clearance and margin collapsing.

The root of this bug was that the parent bfc, and child with clearance
were fighting as to what the BFC block offset should be.

The child was always returning true within:
NeedsAbortOnBfcBlockOffsetChange
... as the floats bfc offset was never equal to the child bfc offset,
as the child got affected by clearance.

This fix is to adjust the FloatsBfcBlockOffset by clearance to check
if we need to relayout or not.

Additionally we were performing more layouts than we should be,
when propagating the BFC block offset up the tree upon an abort. We
should be checking if the child got affected by clearance, and if so
adjust the resolved BFC offset accordingly.

Bug: 923271
Change-Id: Id39905a5445a0fe502c16b956b92d6db13885d36
Reviewed-on: https://chromium-review.googlesource.com/c/1423929Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625411}
parent 4588bfce
......@@ -1245,6 +1245,8 @@ bool NGBlockLayoutAlgorithm::HandleInflow(
!child_space.FloatsBfcBlockOffset();
bool is_empty_block = IsEmptyBlock(child_space, *layout_result);
bool has_clearance = layout_result->IsPushedByFloats();
// 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.
......@@ -1258,7 +1260,9 @@ bool NGBlockLayoutAlgorithm::HandleInflow(
DCHECK(child_bfc_block_offset);
abort_when_bfc_block_offset_updated_ = true;
ResolveBfcBlockOffset(previous_inflow_position,
child_bfc_block_offset.value());
has_clearance
? NextBorderEdge(*previous_inflow_position)
: child_bfc_block_offset.value());
return false;
}
......@@ -1269,7 +1273,6 @@ bool NGBlockLayoutAlgorithm::HandleInflow(
// 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.
bool has_clearance = layout_result->IsPushedByFloats();
if (!child_bfc_block_offset) {
if (!has_clearance && child_space.HasClearanceOffset() &&
child.Style().Clear() != EClear::kNone) {
......@@ -2217,6 +2220,10 @@ bool NGBlockLayoutAlgorithm::NeedsAbortOnBfcBlockOffsetChange() const {
// Otherwise, we need to abort.
LayoutUnit old_bfc_block_offset =
ConstraintSpace().FloatsBfcBlockOffset().value();
// In order to determine if the two offsets are equal, we also need to adjust
// floats offset by the clearance offset.
ApplyClearance(ConstraintSpace(), &old_bfc_block_offset);
return container_builder_.BfcBlockOffset().value() != old_bfc_block_offset;
}
......
......@@ -45,7 +45,7 @@ NGLayoutResult::NGLayoutResult(NGLayoutResultStatus status,
initial_break_before_(EBreakBetween::kAuto),
final_break_after_(EBreakBetween::kAuto),
has_forced_break_(false),
is_pushed_by_floats_(false),
is_pushed_by_floats_(builder->is_pushed_by_floats_),
adjoining_floats_(kFloatTypeNone),
has_orthogonal_flow_roots_(builder->has_orthogonal_flow_roots_),
depends_on_percentage_block_size_(false),
......
<!DOCTYPE html>
<title>Child of block with clear</title>
<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">
<p>Test passes if there is a filled green square.</p>
<div style="width: 100px;background: red;overflow: hidden;">
<div style="float: right; height: 20px; width: 50%; background: green;"></div>
<div>
<div style="float: left; height:100px; width: 50%; background: green;"></div>
<div>
<div style="clear: right; height: 80px; margin-top: 16px; background: green;"></div>
</div>
</div>
</div>
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