Skip to content

Commit

Permalink
[GridFragmentation] Store placement data upon HasBlockFragmentation
Browse files Browse the repository at this point in the history 
In https://chromium-review.googlesource.com/c/chromium/src/+/4109928,
we added support to store grid placement data on `NGGridLayoutData`.

This was originally stored when `HasBlockFragmentation`. During review,
this was moved to `InvolvedInBlockFragmentation`, but when
`IsBreakInside` is false. This was supposed to be `IsBreakInside`
is true. This didn't impact the test case or my original repro, so it
seemed safe. However, now that the change has landed, I retested the
live site and saw that it's still crashing.

Moving the data storage under the true case for `IsBreakInside`
fixes the live site.

(cherry picked from commit 67934e0)

Bug: 1399336
Change-Id: Ic702d2ae229ef23d582d75d98c0860be55e69ccb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4112318
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#1084467}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4131062
Commit-Queue: Ethan Jimenez <ethavar@microsoft.com>
Cr-Commit-Position: refs/branch-heads/5481@{#109}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@KurtCattiSchmidt
KurtCattiSchmidt authored and Chromium LUCI CQ committed Jan 3, 2023
1 parent 9fcc645 commit d4ac995
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 deletions.
17 changes: 8 additions & 9 deletions third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,6 @@ const NGLayoutResult* NGGridLayoutAlgorithm::LayoutInternal() {
LayoutUnit consumed_grid_block_size;
Vector<GridItemPlacementData> grid_items_placement_data;
Vector<LayoutUnit> row_offset_adjustments;
Vector<GridItemIndices> column_range_indices;
Vector<GridItemIndices> row_range_indices;
Vector<GridArea> resolved_positions;
if (UNLIKELY(InvolvedInBlockFragmentation(container_builder_))) {
// Either retrieve all items offsets, or generate them using the
// non-fragmented |PlaceGridItems| pass.
Expand All @@ -291,12 +288,6 @@ const NGLayoutResult* NGGridLayoutAlgorithm::LayoutInternal() {
row_break_between = grid_data->row_break_between;
oof_children = grid_data->oof_children;
} else {
for (auto& grid_item : grid_items) {
column_range_indices.push_back(grid_item.column_range_indices);
row_range_indices.push_back(grid_item.row_range_indices);
resolved_positions.push_back(grid_item.resolved_position);
}

row_offset_adjustments =
Vector<LayoutUnit>(layout_data.Rows()->GetSetCount() + 1);
PlaceGridItems(grid_items, layout_data, &row_break_between,
Expand Down Expand Up @@ -373,6 +364,14 @@ const NGLayoutResult* NGGridLayoutAlgorithm::LayoutInternal() {
PlaceOutOfFlowItems(layout_data, block_size, oof_children);

if (ConstraintSpace().HasBlockFragmentation()) {
Vector<GridItemIndices> column_range_indices;
Vector<GridItemIndices> row_range_indices;
Vector<GridArea> resolved_positions;
for (auto& grid_item : grid_items) {
column_range_indices.push_back(grid_item.column_range_indices);
row_range_indices.push_back(grid_item.row_range_indices);
resolved_positions.push_back(grid_item.resolved_position);
}
container_builder_.SetBreakTokenData(
MakeGarbageCollected<NGGridBreakTokenData>(
container_builder_.GetBreakTokenData(), layout_data,
Expand Down
35 changes: 35 additions & 0 deletions ...y/blink/web_tests/external/wpt/css/css-break/grid/grid-item-placement-crash-multiple.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!DOCTYPE html>
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1399336">
<body>
<div style="width: 6000px; height: 6000px;">
<div style="columns: 4; border: 1px solid black;">
<div style="height: 50px; width: 550px; float: right;">Text</div>
<div style="display: grid; grid-template-columns: repeat(auto-fill,250px);">
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
<div style="width: 250px;">
<div style="height: 550px;"></div>
</div>
</div>
</div>
</div>
</body>

0 comments on commit d4ac995

Please sign in to comment.