Skip to content

Commit

Permalink
[GridNG] Proper relayout of grid and out of flow items
Browse files Browse the repository at this point in the history 
Previously, grid items and out of flow items were going through relayout
independently of their containing block. This caused crashes and/or
incorrect positioning and sizing of out of flow items when computing
their containing block rect, as the container builder did not have
information about the grid (|NGGridData|).

In this change a condition was added to prevent relayout of grid's
children without their containing block, a DCHECK was added to
|NGGridData|'s getter, and three web tests were added to validate the
behavior in different circumstances: direct children of the grid,
descendant and dynamic change.

Bug: 1045599, 1232781, 1232654
Change-Id: Ie8a082c10bf518ea9537c16ebd4fb63695572f5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3057859
Commit-Queue: Ana Sollano Kim <ansollan@microsoft.com>
Reviewed-by: Daniel Libby <dlibby@microsoft.com>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#906818}
  • Loading branch information
Ana SollanoKim authored and Chromium LUCI CQ committed Jul 29, 2021
1 parent f1dcb70 commit aa46421
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 1 deletion.
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/layout/layout_object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,13 @@ static inline bool ObjectIsRelayoutBoundary(const LayoutObject* object) {
if (layout_box->IsFlexItemIncludingNG())
return false;

// Similarly to flex items, we can't relayout a grid item independently of
// its container. This also applies to out of flow items of the grid, as we
// need the cached information of the grid to recompute the out of flow
// item's containing block rect.
if (layout_box->ContainingBlock()->IsLayoutGridIncludingNG())
return false;

// In LayoutNG, if box has any OOF descendants, they are propagated to
// parent. Therefore, we must mark parent chain for layout.
if (const NGLayoutResult* layout_result =
Expand Down
5 changes: 4 additions & 1 deletion third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,10 @@ class CORE_EXPORT NGBoxFragmentBuilder final
grid_data_ = std::move(grid_data);
}

const NGGridData& GetNGGridData() const { return *grid_data_.get(); }
const NGGridData& GetNGGridData() const {
DCHECK(grid_data_);
return *grid_data_.get();
}

// The |NGFragmentItemsBuilder| for the inline formatting context of this box.
NGFragmentItemsBuilder* ItemsBuilder() { return items_builder_; }
Expand Down
38 changes: 38 additions & 0 deletions ...b_tests/external/wpt/css/css-grid/abspos/grid-positioned-item-dynamic-change-005-ref.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!DOCTYPE html>
<html lang=en>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid positioned item dynamic change.</title>
<link rel="help" href="https://drafts.csswg.org/css-grid-2/#abspos">
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
}
#child {
grid-area: 2 / 2 / 2 / 2;
width: 50px;
height: 50px;
background-color: green;
}
#grandchild {
width: 25px;
height: 25px;
background-color: red;
}
</style>

<body>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child">
<div id="grandchild"></div>
</div>
</div>

</body>

</html>
54 changes: 54 additions & 0 deletions ...k/web_tests/external/wpt/css/css-grid/abspos/grid-positioned-item-dynamic-change-005.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!DOCTYPE html>
<html lang=en class="reftest-wait">
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid positioned item dynamic change.</title>
<link rel="help" href="https://drafts.csswg.org/css-grid-2/#abspos">
<link rel="match" href="grid-positioned-item-dynamic-change-005-ref.html">
<meta name="assert" content="This test checks that positioned items can be dynamically changed.">
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
position: relative;
}
#child {
grid-area: 2 / 2 / 2 / 2;
width: 100%;
height: 100%;
background-color: green;
position: absolute;
}
#grandchild {
width: 50%;
height: 50%;
background-color: red;
}
</style>

<body onload=updateConstraints()>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child">
<marquee id="grandchild"></marquee>
</div>
</div>

</body>

<script>
function updateConstraints() {
document.body.offsetTop;

document.getElementById('child').style.bottom = '0';
document.getElementById('child').style.left = '0';
document.getElementById('child').style.contain = 'strict';

document.documentElement.classList.remove('reftest-wait');
}
</script>

</html>
37 changes: 37 additions & 0 deletions ...blink/web_tests/external/wpt/css/css-grid/abspos/positioned-grid-descendants-017-ref.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Positioned grid descendants eference file</title>
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
}
#child-1 {
grid-area: 1 / 1 / 1 / 1;
width: 50px;
height: 50px;
background-color: hotpink;
}
#child-2 {
grid-area: 2 / 2 / 2 / 2;
width: 50px;
height: 50px;
background-color: green;
}
#grandchild {
width: 25px;
height: 25px;
background-color: red;
}
</style>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child-1"></div>
<div id="child-2">
<div id="grandchild"></div>
</div>
</div>
44 changes: 44 additions & 0 deletions ...rty/blink/web_tests/external/wpt/css/css-grid/abspos/positioned-grid-descendants-017.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Positioned grid descendants</title>
<link rel="help" href="https://drafts.csswg.org/css-grid/#abspos" title="9. Absolute Positioning">
<link rel="match" href="positioned-grid-descendants-017-ref.html">
<meta name="assert" content="Checks that absolutely positioned elements inside grid items are properly placed and sized.">
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
position: relative;
}
#child {
grid-area: 1 / 1 / 1 / 1;
background-color: hotpink;
}
#abspos {
grid-area: 2 / 2 / 2 / 2;
bottom: 0;
left: 0;
width: 100%;
height: 100%;
background-color: green;
contain: strict;
position: absolute;
}
#grandchild {
width: 50%;
height: 50%;
background-color: red;
}
</style>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child">
<div id="abspos">
<marquee id="grandchild"></marquee>
</div>
</div>
</div>
30 changes: 30 additions & 0 deletions ...party/blink/web_tests/external/wpt/css/css-grid/abspos/positioned-grid-items-023-ref.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Positioned grid items reference file</title>
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
}
#child {
grid-area: 2 / 2 / 2 / 2;
width: 50px;
height: 50px;
background-color: green;
}
#grandchild {
width: 25px;
height: 25px;
background-color: red;
}
</style>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child">
<div id="grandchild"></div>
</div>
</div>
38 changes: 38 additions & 0 deletions third_party/blink/web_tests/external/wpt/css/css-grid/abspos/positioned-grid-items-023.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Positioned grid items</title>
<link rel="help" href="https://drafts.csswg.org/css-grid-2/#abspos" title="9. Absolute Positioning">
<link rel="match" href="positioned-grid-items-023-ref.html">
<meta name="assert" content="Checks that absolutely positioned grid items with a strict contain attribute are properly placed in a grid.">
<style>
#grid {
display: inline-grid;
grid-template-columns: 50px 50px;
grid-template-rows: 50px 50px;
background-color: blue;
position: relative;
}
#child {
grid-area: 2 / 2 / 2 / 2;
bottom: 0;
left: 0;
width: 100%;
height: 100%;
background-color: green;
contain: strict;
position: absolute;
}
#grandchild {
width: 50%;
height: 50%;
background-color: red;
}
</style>

<p>Test passes if it matches the reference.</p>

<div id="grid">
<div id="child">
<marquee id="grandchild"></marquee>
</div>
</div>

0 comments on commit aa46421

Please sign in to comment.