Skip to content

GitLab

T
tangled
Commit 5c6452a3 authored by robhogan's avatar robhogan Committed by Commit bot
Browse files
Options

Re-land "Detect floats to avoid or clear due to negative margin top (and followup)" 

Re-land https://codereview.chromium.org/2531953002 and
https://codereview.chromium.org/2547933003.

When a negative margin top pushes a block back up into its previous siblings
we need to check for any floats in those siblings it now needs to avoid or clear.

Previously we were just looking at its neighbour, we need to keep looking until
we reach a sibling that we don't overlap.

Fix the performance regression on mossiella.com in page_cycler_v2.tough_layout_cases
by only looking for floats if margin collapsing has actually moved the child up.

BUG=671645, 670325, 666487

Review-Url: https://codereview.chromium.org/2568643002
Cr-Commit-Position: refs/heads/master@{#437951}
parent bf5e7305
Hide whitespace changes
Inline Side-by-side
Showing with 170 additions and 13 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 5c6452a3
...@@ -625,6 +625,12 @@ crbug.com/635619 virtual/layout_ng/fast/block/float/trailing-float-with-columns. ...@@ -625,6 +625,12 @@ crbug.com/635619 virtual/layout_ng/fast/block/float/trailing-float-with-columns.
crbug.com/635619 virtual/layout_ng/fast/block/float/trailing-float-with-content.html [ Skip ] crbug.com/635619 virtual/layout_ng/fast/block/float/trailing-float-with-content.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/vertical-move-relayout.html [ Skip ] crbug.com/635619 virtual/layout_ng/fast/block/float/vertical-move-relayout.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/width-update-after-clear.html [ Skip ] crbug.com/635619 virtual/layout_ng/fast/block/float/width-update-after-clear.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-2.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-3.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-4.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-5.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-6.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top.html [ Skip ]
# ====== LayoutNG-only failures until here ====== # ====== LayoutNG-only failures until here ======
# Requires ServiceWorkerNavigationPreload feature enabled. Run under # Requires ServiceWorkerNavigationPreload feature enabled. Run under
......
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-2.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div class="float"></div>
<br clear=all>
<div style="width: 20px; height; 1px"></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-3.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div>
<div class="float"></div>
<br clear=all>
</div>
<div style="width: 20px; height; 1px"></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-4.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div>
<div class="float"></div>
<br clear=all>
</div>
<div style="float:left;"></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-5.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div>
<div class="float"></div>
<br clear=all>
</div>
<div style="position:absolute;"></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-6.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div>
<div class="float"></div>
<br clear=all>
</div>
<div style="overflow:hidden;"></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top.html 0 → 100644
View file @ 5c6452a3
<!DOCTYPE html>
<style>
body { margin: 0px; }
.float { float: left; width: 50px; height: 100px; background: green;}
.content { overflow: hidden; margin-top: -100px; width: 50px; height: 100px; background: green;}
.container { width: 100px; background: red;}
</style>
<p>crbug.com/666487: There should be a green square below.</p>
<div class="container">
<div class="float"></div>
<br clear=all>
<div></div>
<div class="content" data-offset-x=50></div>
</div>
<script src="../../../resources/check-layout.js"></script>
<script>
checkLayout('.content');
</script>
third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
View file @ 5c6452a3
...@@ -1709,10 +1709,10 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child, ...@@ -1709,10 +1709,10 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child,
LayoutObject* prev = child.previousSibling(); LayoutObject* prev = child.previousSibling();
LayoutBlockFlow* previousBlockFlow = LayoutBlockFlow* previousBlockFlow =
prev && prev->isLayoutBlockFlow() && prev && prev->isLayoutBlockFlow() ? toLayoutBlockFlow(prev) : nullptr;
!prev->isFloatingOrOutOfFlowPositioned() bool previousBlockFlowCanSelfCollapse =
? toLayoutBlockFlow(prev) previousBlockFlow &&
: 0; !previousBlockFlow->isFloatingOrOutOfFlowPositioned();
// If the child's previous sibling is a self-collapsing block that cleared a // If the child's previous sibling is a self-collapsing block that cleared a
// float then its top border edge has been set at the bottom border edge of // float then its top border edge has been set at the bottom border edge of
// the float. Since we want to collapse the child's top margin with the self- // the float. Since we want to collapse the child's top margin with the self-
...@@ -1720,7 +1720,8 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child, ...@@ -1720,7 +1720,8 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child,
// height to match the margin top of the self-collapsing block. If the // height to match the margin top of the self-collapsing block. If the
// resulting collapsed margin leaves the child still intruding into the float // resulting collapsed margin leaves the child still intruding into the float
// then we will want to clear it. // then we will want to clear it.
if (!marginInfo.canCollapseWithMarginBefore() && previousBlockFlow && if (!marginInfo.canCollapseWithMarginBefore() &&
previousBlockFlowCanSelfCollapse &&
marginInfo.lastChildIsSelfCollapsingBlockWithClearance()) marginInfo.lastChildIsSelfCollapsingBlockWithClearance())
setLogicalHeight( setLogicalHeight(
logicalHeight() - logicalHeight() -
...@@ -1801,18 +1802,28 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child, ...@@ -1801,18 +1802,28 @@ LayoutUnit LayoutBlockFlow::collapseMargins(LayoutBox& child,
setLogicalHeight(logicalHeight() + (logicalTop - oldLogicalTop)); setLogicalHeight(logicalHeight() + (logicalTop - oldLogicalTop));
} }
if (previousBlockFlow) { // If |child| has moved up into previous siblings it needs to avoid or clear
// If |child| is a self-collapsing block it may have collapsed into a // any floats they contain.
// previous sibling and although it hasn't reduced the height of the parent if (logicalTop < beforeCollapseLogicalTop) {
// yet any floats from the parent will now overhang.
LayoutUnit oldLogicalHeight = logicalHeight(); LayoutUnit oldLogicalHeight = logicalHeight();
setLogicalHeight(logicalTop); setLogicalHeight(logicalTop);
if (!previousBlockFlow->avoidsFloats() && while (previousBlockFlow) {
(previousBlockFlow->logicalTop() + auto lowestFloat = previousBlockFlow->logicalTop() +
previousBlockFlow->lowestFloatLogicalBottom()) > logicalTop) previousBlockFlow->lowestFloatLogicalBottom();
addOverhangingFloats(previousBlockFlow, false); if (lowestFloat > logicalTop)
addOverhangingFloats(previousBlockFlow, false);
else
break;
LayoutObject* prev = previousBlockFlow->previousSibling();
if (prev && prev->isLayoutBlockFlow())
previousBlockFlow = toLayoutBlockFlow(prev);
else
previousBlockFlow = nullptr;
}
setLogicalHeight(oldLogicalHeight); setLogicalHeight(oldLogicalHeight);
}
if (previousBlockFlowCanSelfCollapse) {
// If |child|'s previous sibling is or contains a self-collapsing block that // If |child|'s previous sibling is or contains a self-collapsing block that
// cleared a float and margin collapsing resulted in |child| moving up // cleared a float and margin collapsing resulted in |child| moving up
// into the margin area of the self-collapsing block then the float it // into the margin area of the self-collapsing block then the float it
......
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