Commit 0c0063c3 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[layout] Abort simplified layout if baselines shift.

If we ran simplified layout, or triggered layout from a subtree-root
the baseline of the fragment may have changed. While most of the time
this doesn't have any visible side-effect, there may be an arbitrary
ancestor which relies on this baseline, causing elements to be baseline
aligned incorrectly.

This patch does two things:
1) When simplified layout is triggered from simplified layout, check if
   the Baseline shifted, and return nullptr.
2) When we are being laid out as a subtree root (as an inflow element),
   check if the Baseline shifted, and mark our containing block chain
   for layout.

Bug: 1137905
Change-Id: I8c8fa2c981e9001cf6ec7f685d4451e1fe13c848
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2509644Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823197}
parent 3f27e07c
......@@ -327,7 +327,8 @@ void LayoutNGMixin<Base>::UpdateOutOfFlowBlockLayout() {
template <typename Base>
scoped_refptr<const NGLayoutResult>
LayoutNGMixin<Base>::UpdateInFlowBlockLayout() {
const auto* previous_result = Base::GetCachedLayoutResult();
scoped_refptr<const NGLayoutResult> previous_result =
Base::GetCachedLayoutResult();
bool is_layout_root = !Base::View()->GetLayoutState()->Next();
// If we are a layout root, use the previous space if available. This will
......@@ -340,10 +341,25 @@ LayoutNGMixin<Base>::UpdateInFlowBlockLayout() {
scoped_refptr<const NGLayoutResult> result =
NGBlockNode(this).Layout(constraint_space);
const auto& physical_fragment =
To<NGPhysicalBoxFragment>(result->PhysicalFragment());
for (const auto& descendant :
result->PhysicalFragment().OutOfFlowPositionedDescendants())
physical_fragment.OutOfFlowPositionedDescendants())
descendant.node.UseLegacyOutOfFlowPositioning();
// Even if we are a layout root, our baseline may have shifted. In this
// (rare) case, mark our containing-block for layout.
if (is_layout_root && previous_result) {
if (To<NGPhysicalBoxFragment>(previous_result->PhysicalFragment())
.Baseline() != physical_fragment.Baseline()) {
if (auto* containing_block = Base::ContainingBlock()) {
containing_block->SetNeedsLayout(
layout_invalidation_reason::kChildChanged, kMarkContainerChain);
}
}
}
return result;
}
......
......@@ -576,11 +576,21 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::SimplifiedLayout(
scoped_refptr<const NGLayoutResult> result =
Layout(space, /* break_token */ nullptr);
// If we changed size from performing "simplified" layout, we have
// added/removed scrollbars. Return null indicating to our parent that it
// needs to perform a full layout.
if (previous_result->PhysicalFragment().Size() !=
result->PhysicalFragment().Size())
const auto& old_fragment =
To<NGPhysicalBoxFragment>(previous_result->PhysicalFragment());
const auto& new_fragment =
To<NGPhysicalBoxFragment>(result->PhysicalFragment());
// Simplified layout has the ability to add/remove scrollbars, this can cause
// a couple (rare) edge-cases which will make the fragment different enough
// that the parent should perform a full layout.
// - The size has changed.
// - The alignment baseline has shifted.
// We return a nullptr in these cases indicating to our parent that it needs
// to perform a full layout.
if (old_fragment.Size() != new_fragment.Size())
return nullptr;
if (old_fragment.Baseline() != new_fragment.Baseline())
return nullptr;
#if DCHECK_IS_ON()
......
......@@ -54,6 +54,8 @@ crbug.com/591099 external/wpt/css/CSS2/normal-flow/min-height-separates-margin.h
crbug.com/591099 external/wpt/css/CSS2/text/white-space-mixed-003.xht [ Failure ]
### external/wpt/css/css-flexbox/
crbug.com/591099 external/wpt/css/css-flexbox/dynamic-baseline-change.html [ Failure ]
crbug.com/591099 external/wpt/css/css-flexbox/dynamic-baseline-change-nested.html [ Failure ]
crbug.com/591099 external/wpt/css/css-flexbox/image-as-flexitem-size-003.html [ Failure ]
crbug.com/591099 external/wpt/css/css-flexbox/image-as-flexitem-size-003v.html [ Failure ]
crbug.com/591099 external/wpt/css/css-flexbox/image-as-flexitem-size-004.html [ Failure ]
......
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#baseline-participation">
<link rel="match" href="dynamic-baseline-change-ref.html">
<meta name="assert" content="This test ensures proper baseline alignment for a sub-tree which adds a scrollbar.">
<div style="display: flex; width: 100px; border: solid; align-items: baseline;">
<div style="width: 50px; height: 50px;">
<div style="width: 50px; height: 50px; position: relative; overflow: auto;">
<canvas width=10 height=10 style="width: 80%; background: green;"></canvas> <!-- Baseline of the canvas will move when a scrollbar is added. -->
<div id="target" style="position: absolute; width: 10px; height: 10px; background: red; top: 0;"></div>
</div>
</div>
<div style="width: 50px; height: 50px; background: green;"></div> <!-- Baseline is synthesized at the block-end edge. -->
</div>
<script>
document.body.offsetTop;
document.getElementById('target').style.top = '100px';
</script>
<!DOCTYPE html>
<div style="display: flex; width: 100px; border: solid; align-items: baseline;">
<div style="width: 50px; height: 50px;">
<div style="width: 50px; height: 50px; position: relative; overflow: auto;">
<canvas width=10 height=10 style="width: 80%; background: green;"></canvas>
<div id="target" style="position: absolute; width: 10px; height: 10px; background: red; top: 100px;"></div>
</div>
</div>
<div style="width: 50px; height: 50px; background: green;"></div>
</div>
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#baseline-participation">
<link rel="match" href="dynamic-baseline-change-ref.html">
<meta name="assert" content="This test ensures proper baseline alignment for a sub-tree which adds a scrollbar.">
<div style="display: flex; width: 100px; border: solid; align-items: baseline;">
<div style="width: 50px; height: 50px; position: relative; overflow: auto;">
<canvas width=10 height=10 style="width: 80%; background: green;"></canvas> <!-- Baseline of the canvas will move when a scrollbar is added. -->
<div id="target" style="position: absolute; width: 10px; height: 10px; background: red; top: 0;"></div>
</div>
<div style="width: 50px; height: 50px; background: green;"></div> <!-- Baseline is synthesized at the block-end edge. -->
</div>
<script>
document.body.offsetTop;
document.getElementById('target').style.top = '100px';
</script>
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