[css-grid] Compute again the baseline offsets during step 3

The new Baseline Alignment algorithm states that items with sizing cyclic dependencies must be excluded from any baseline context they participate in. One of these cyclic dependencies can happen with intrinsic sized grid areas and relative items. The grid spec states [1] that flex-sized tracks should be considered as content-sized when the grid container has an indefinite size. We were using the AvailableSize(direction) function to determine whether the grid container is indefinite or not. However, this function may provide different results during the different phases of the grid layout logic. This issue causes assert violations like the one described in the bugs listed below. The new Baseline Alignment logic is now integrated in the Grid Track sizing algorithm. Hence, we need to ensure that an item that participates in any baseline alignment context during the track sizing also does during the alignment phase, at the end of the grid layout logic. In order to achieve that, this CL forces a new computation of the Baseline offsets during the step 3 of the Grid sizing algorith, since during this step the available space is not indefinite anymore. It's worth mentioning that this change assumes the issue grid items being excluded and included of Baseline Context during the different phases of the Grid sizing algorithm, which I hope we can clarify in the issue [2] I filed for the CSS WG. [1] https://drafts.csswg.org/css-grid/#fr-unit [2] https://github.com/w3c/csswg-drafts/issues/3046 Bug: 867833, 874861, 876593 Change-Id: I668d399b920c9280a8e20b3e8362f562eded4770 Reviewed-on: https://chromium-review.googlesource.com/1177757Reviewed-by: Sergio Villar <svillar@igalia.com> Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Cr-Commit-Position: refs/heads/master@{#587799}

[css-grid] Compute again the baseline offsets during step 3
The new Baseline Alignment algorithm states that items with sizing cyclic dependencies must be excluded from any baseline context they participate in. One of these cyclic dependencies can happen with intrinsic sized grid areas and relative items. The grid spec states [1] that flex-sized tracks should be considered as content-sized when the grid container has an indefinite size. We were using the AvailableSize(direction) function to determine whether the grid container is indefinite or not. However, this function may provide different results during the different phases of the grid layout logic. This issue causes assert violations like the one described in the bugs listed below. The new Baseline Alignment logic is now integrated in the Grid Track sizing algorithm. Hence, we need to ensure that an item that participates in any baseline alignment context during the track sizing also does during the alignment phase, at the end of the grid layout logic. In order to achieve that, this CL forces a new computation of the Baseline offsets during the step 3 of the Grid sizing algorith, since during this step the available space is not indefinite anymore. It's worth mentioning that this change assumes the issue grid items being excluded and included of Baseline Context during the different phases of the Grid sizing algorithm, which I hope we can clarify in the issue [2] I filed for the CSS WG. [1] https://drafts.csswg.org/css-grid/#fr-unit [2] https://github.com/w3c/csswg-drafts/issues/3046 Bug: 867833, 874861, 876593 Change-Id: I668d399b920c9280a8e20b3e8362f562eded4770 Reviewed-on: https://chromium-review.googlesource.com/1177757Reviewed-by: Sergio Villar <svillar@igalia.com> Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Cr-Commit-Position: refs/heads/master@{#587799}
48e9d031 · Javier Fernandez · Commit Bot · bd2ac01e · 48e9d031 · 48e9d031
Commit 48e9d031 authored Aug 30, 2018 by Javier Fernandez Committed by Commit Bot Aug 30, 2018
4 changed files
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001.html
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-002.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-002.html
--- a/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-self-baseline-and-flex-tracks-with-indefinite-container-crash.html
+++ b/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-self-baseline-and-flex-tracks-with-indefinite-container-crash.html
+<!doctype html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<style>
+.grid {
+  display: grid;
+  grid-template-columns: 10px;
+  grid-template-rows: minmax(4px, 1fr);
+  align-items: baseline;
+  width: 100px;
+}
+</style>
+<div class="grid">
+    <div style="writing-mode: vertical-rl;"></div>
+</div>
+<script>
+    test(() => {
+    }, "This test should not crash in debug.");
+</script>
--- a/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
+++ b/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
@@ -251,6 +251,12 @@ bool GridTrackSizingAlgorithm::IsIntrinsicSizedGridArea(const LayoutBox& child,
    GridTrackSize track_size = RawGridTrackSize(direction, track_position);
    // We consider fr units as 'auto' for the min sizing function.
    // TODO(jfernandez): https://github.com/w3c/csswg-drafts/issues/2611
+    //
+    // The use of AvailableSize function may imply different results
+    // for the same item when assuming indefinite or definite size
+    // constraints depending on the phase we evaluate the item's
+    // baseline participation.
+    // TODO(jfernandez): https://github.com/w3c/csswg-drafts/issues/3046
    if (track_size.IsContentSized() || track_size.IsFitContent() ||
        track_size.MinTrackBreadth().IsFlex() ||
        (track_size.MaxTrackBreadth().IsFlex() && !AvailableSpace(direction)))
@@ -1587,8 +1593,6 @@ void GridTrackSizingAlgorithm::Setup(
 }
 void GridTrackSizingAlgorithm::ComputeBaselineAlignmentContext() {
-  if (sizing_state_ > kRowSizingFirstIteration)
-    return;
  GridAxis axis = GridAxisForDirection(direction_);
  baseline_alignment_.Clear(axis);
  baseline_alignment_.SetBlockFlow(layout_grid_->StyleRef().GetWritingMode());