Commit 21b73090 authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot

Revert "[css-grid] Clear the override width for computing percent margins"

This reverts commit a445d168.

Reason for revert: It caused a performance regression (issue #1002700)

Original change's description:
> [css-grid] Clear the override width for computing percent margins
>
> When calculating the min-content contribution of a grid item of an auto
> sized grid track we must consider the grid item's margin. When the grid
> item's area is indefinite, a percent margin is resolved to zero.
> However, when performing a relayout, the percent margin may be solved
> against the previously computed grid area, since the grid item has
> already an OverrideContainingBlockLogicalWidth value.
>
> In order to re-compute the percent margin properly, we need to clear
> the previously override value. It's important be careful of not
> clearing the override value set during intrinsic size, since we need
> it for the actual layout phase. Hence, we only reset the 'override'
> value when we are executing a definite strategy.
>
> Bug: 834643
> Change-Id: Ib936b26bee1da76afbdc886eb775746e13d40988
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1782840
> Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#694849}

TBR=cbiesinger@chromium.org,jfernandez@igalia.com,rego@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 834643, 1002700
Change-Id: I66f2b94417be0c74dc408bc55eee3a8d44447480
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796803Reviewed-by: default avatarJavier Fernandez <jfernandez@igalia.com>
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#695531}
parent d322b291
......@@ -135,7 +135,6 @@ class DefiniteSizeStrategy final : public GridTrackSizingAlgorithmStrategy {
return false;
}
LayoutUnit FreeSpaceForStretchAutoTracksStep() const override;
LayoutUnit MinContentForChild(LayoutBox&) const override;
bool IsComputingSizeContainment() const override { return false; }
};
......@@ -150,15 +149,10 @@ bool GridTrackSizingAlgorithmStrategy::
GridLayoutUtils::FlowAwareDirectionForChild(grid, child, kForColumns);
if (direction == child_inline_direction) {
return child.HasRelativeLogicalWidth() ||
child.StyleRef().LogicalWidth().IsIntrinsicOrAuto() ||
child.StyleRef().MarginStart().IsPercentOrCalc() ||
child.StyleRef().MarginEnd().IsPercentOrCalc();
child.StyleRef().LogicalWidth().IsIntrinsicOrAuto();
}
return child.HasRelativeLogicalHeight() ||
child.StyleRef().LogicalHeight().IsIntrinsicOrAuto() ||
child.StyleRef().MarginBefore().IsPercentOrCalc() ||
child.StyleRef().MarginAfter().IsPercentOrCalc();
;
child.StyleRef().LogicalHeight().IsIntrinsicOrAuto();
}
void GridTrackSizingAlgorithmStrategy::
......@@ -583,21 +577,6 @@ LayoutUnit DefiniteSizeStrategy::FreeSpaceForStretchAutoTracksStep() const {
return algorithm_.FreeSpace(Direction()).value();
}
DISABLE_CFI_PERF
LayoutUnit DefiniteSizeStrategy::MinContentForChild(LayoutBox& child) const {
GridTrackSizingDirection child_inline_direction =
GridLayoutUtils::FlowAwareDirectionForChild(*GetLayoutGrid(), child,
kForColumns);
if (Direction() == child_inline_direction &&
ShouldClearOverrideContainingBlockContentSizeForChild(
*GetLayoutGrid(), child, child_inline_direction)) {
SetOverrideContainingBlockContentSizeForChild(child, child_inline_direction,
LayoutUnit());
}
return GridTrackSizingAlgorithmStrategy::MinContentForChild(child);
}
void IndefiniteSizeStrategy::LayoutGridItemForMinSizeComputation(
LayoutBox& child,
bool override_size_has_changed) const {
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid items with percentage margins</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-grid-1/#item-margins">
<meta name="assert" content="Checks grid items percentage margins are resolved correctly in a 'auto' sized grid area after changing the item's width and forcing a new layout.">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="stylesheet" href="support/grid.css">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
.container {
font: 25px/1 Ahem;
width: 100px;
height: 100px;
}
.child {
margin: 50px;
color: red;
}
.ref {
position: absolute;
z-index: -1;
background: green;
}
.grid {
background: none;
}
#item {
margin: 50%;
color: green;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="container ref"><div class="child">X</div></div>
<div class="container grid"><div class="child" id="item">X</div></div>
<script>
item.style.width = "0px";
item.offsetLeft;
item.style.width = "auto";
item.offsetLeft;
</script>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid items with percentage margins</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-grid-1/#item-margins">
<meta name="assert" content="Checks grid items percentage margins are resolved correctly in a 'auto' sized grid area after changing the item's width and forcing a new layout.">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="stylesheet" href="support/grid.css">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
.container {
width: 100px;
height: 100px;
}
.child {
width: 25px;
height: 25px;
margin: 50px;
background: red;
}
.ref {
position: absolute;
z-index: -1;
background: green;
}
.grid {
background: none;
}
#item {
margin: 50%;
background: green;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="container ref"><div class="child"></div></div>
<div class="container grid"><div class="child" id="item"></div></div>
<script>
item.style.width = "0px";
item.offsetLeft;
item.style.width = "25px";
item.offsetLeft;
</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