[GridNG] Mitigate perf issue w/ replaced elements and block-constraints.
This is effectively a revert of:
https://chromium-review.googlesource.com/c/chromium/src/+/3154713
This change made us correct on the newly added test:
grid-intrinsic-size-dynamic-block-size.html
However this has large performance issues on sites with a replaced
element embedded somewhere in a nested-grid structure with a dependence
on block-constraints.
This patch mitigates the issue, to properly solve this we need additional
cache slots for both Layout, and ComputeMinMaxSizes
(see crbug.com/1272533).
(cherry picked from commit 90c957f7f80adb9dd496b816e8d03b87dadfb1f7)
Bug: 1271648, 1272533
Change-Id: I37bede27f2a7988304ec11a37c75f763c79a50e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3295042
Auto-Submit: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Reviewed-by: Daniel Libby <dlibby@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#944159}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3308392
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/branch-heads/4664@{#1190}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
diff --git a/third_party/blink/perf_tests/layout/grid-with-block-constraints-dependence.html b/third_party/blink/perf_tests/layout/grid-with-block-constraints-dependence.html
new file mode 100644
index 0000000..7aaa992
--- /dev/null
+++ b/third_party/blink/perf_tests/layout/grid-with-block-constraints-dependence.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<script src="../resources/runner.js"></script>
+<div id=c1 style="display: grid; grid-template: auto / 1fr;">
+ <div id=c2 style="display: grid; grid-template: auto / 1fr;">
+ <div id=c3 style="display: grid; grid-template: auto auto / 1fr;">
+ <div id="target" style="display: grid; height: 100px;"></div>
+ <div id=i1 style="display: grid;">
+ <div id=i2 style="display: grid;">
+ <div id=i3 style="width: 100%; height: 100%;">
+ <canvas id=i4 width=50 height=50 style="width: 100%; height: 100%;"></canvas>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+</div>
+<script>
+PerfTestRunner.measureRunsPerSecond({
+ description: 'Layout of nested grid with a replaced element dependant on block-constraints.',
+ run: () => {
+ document.getElementById('target').innerText = 'text';
+ PerfTestRunner.forceLayout();
+ document.getElementById('target').innerText = '';
+ PerfTestRunner.forceLayout();
+ },
+});
+</script>
diff --git a/third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc b/third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc
index 6964fc4..8f6d93b 100644
--- a/third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc
+++ b/third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc
@@ -521,7 +521,11 @@
MinMaxSizes sizes{ComputeTotalColumnSize(SizingConstraint::kMinContent),
ComputeTotalColumnSize(SizingConstraint::kMaxContent)};
sizes += BorderScrollbarPadding().InlineSum();
- return MinMaxSizesResult(sizes, depends_on_block_constraints);
+
+ // TODO(crbug.com/1272533): This should be |depends_on_block_constraints|
+ // (rather than false). However we need more cache slots to handle the
+ // performance degredation we currently experience. See bug for more details.
+ return MinMaxSizesResult(sizes, /* depends_on_block_constraints */ false);
}
const TrackSpanProperties& GridItemData::GetTrackSpanProperties(
diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
index 63ba8d5..045a45b 100644
--- a/third_party/blink/web_tests/TestExpectations
+++ b/third_party/blink/web_tests/TestExpectations
@@ -4102,6 +4102,9 @@
crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-rows-interpolation.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-definition/grid-repeat-max-width-001.html [ Failure ]
+# Simple to fix - but blocked on performance regression requiring cache logic change.
+crbug.com/1272533 external/wpt/css/css-grid/layout-algorithm/grid-intrinsic-size-dynamic-block-size.html [ Failure ]
+
# The 'last baseline' keyword is not implemented yet
crbug.com/885175 external/wpt/css/css-grid/alignment/grid-item-self-baseline-001.html [ Skip ]
crbug.com/885175 external/wpt/css/css-grid/alignment/grid-self-alignment-baseline-with-grid-001.html [ Skip ]