[css-grid] GetGridTrackSize must be used after setting up the algorithm

This is just a refactoring, so no there is no behavior change in this
patch.

We want to avoid issues like the one described in bug 847839, so that
functions that depend on the AvailableSize() result must be used only
after having set up the algorithm.

In the rare cases where we need to figure out the track size, we
must use the RawGridTrackSize function, which retrives the info
directly from the style.

Change-Id: I22061c6063cf557cc23e8492f625f0b2299f1ccd
Reviewed-on: https://chromium-review.googlesource.com/1087465
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#564950}

third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc

@@ -164,13 +164,56 @@ void GridTrackSizingAlgorithmStrategy::
     child.SetOverrideContainingBlockContentLogicalHeight(size);
 }
 
+LayoutSize GridTrackSizingAlgorithm::EstimatedGridAreaBreadthForChild(
+    const LayoutBox& child) const {
+  return {EstimatedGridAreaBreadthForChild(child, kForColumns),
+          EstimatedGridAreaBreadthForChild(child, kForRows)};
+}
+
+LayoutUnit GridTrackSizingAlgorithm::EstimatedGridAreaBreadthForChild(
+    const LayoutBox& child,
+    GridTrackSizingDirection direction) const {
+  const GridSpan& span = grid_.GridItemSpan(child, direction);
+  LayoutUnit grid_area_size;
+  bool grid_area_is_indefinite = false;
+  base::Optional<LayoutUnit> available_size = AvailableSpace(direction);
+  for (auto track_position : span) {
+    // We may need to estimate the grid area size before running the track
+    // sizing algorithm in order to perform the pre-layout of orthogonal
+    // items.
+    GridTrackSize track_size =
+        WasSetup() ? GetGridTrackSize(direction, track_position)
+                   : RawGridTrackSize(direction, track_position);
+    GridLength max_track_size = track_size.MaxTrackBreadth();
+    if (max_track_size.IsContentSized() || max_track_size.IsFlex() ||
+        IsRelativeGridLengthAsAuto(max_track_size, direction)) {
+      grid_area_is_indefinite = true;
+    } else {
+      grid_area_size += ValueForLength(max_track_size.length(),
+                                       available_size.value_or(LayoutUnit()));
+    }
+  }
+
+  grid_area_size += layout_grid_->GuttersSize(
+      grid_, direction, span.StartLine(), span.IntegerSpan(), available_size);
+
+  GridTrackSizingDirection child_inline_direction =
+      GridLayoutUtils::FlowAwareDirectionForChild(*layout_grid_, child,
+                                                  kForColumns);
+  if (grid_area_is_indefinite) {
+    return direction == child_inline_direction
+               ? std::max(child.MaxPreferredLogicalWidth(), grid_area_size)
+               : LayoutUnit(-1);
+  }
+  return grid_area_size;
+}
+
 LayoutUnit GridTrackSizingAlgorithm::GridAreaBreadthForChild(
     const LayoutBox& child,
     GridTrackSizingDirection direction) const {
   if (direction == kForRows && sizing_state_ == kColumnSizingFirstIteration) {
     DCHECK(GridLayoutUtils::IsOrthogonalChild(*layout_grid_, child));
-    return layout_grid_->EstimatedGridAreaBreadthForChild(grid_, child,
-                                                          kForRows);
+    return EstimatedGridAreaBreadthForChild(child, kForRows);
   }
 
   const Vector<GridTrack>& all_tracks = Tracks(direction);
@@ -308,8 +351,7 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::MinSizeForChild(
         algorithm_.GetGrid().GridItemSpan(child, Direction());
     LayoutUnit max_breadth;
     for (const auto& track_position : span) {
-      GridTrackSize track_size =
-          algorithm_.GetGridTrackSize(Direction(), track_position);
+      GridTrackSize track_size = GetGridTrackSize(Direction(), track_position);
       if (!track_size.HasFixedMaxTrackBreadth())
         return min_size;
       max_breadth += ValueForLength(track_size.MaxTrackBreadth().length(),
@@ -512,11 +554,10 @@ double IndefiniteSizeStrategy::FindUsedFlexFraction(
     // really matter as we know the track is a flex sized track. It'd be nice
     // not to have to do that.
     flex_fraction = std::max(
-        flex_fraction, NormalizedFlexFraction(
-                           all_tracks[track_index],
-                           algorithm_.GetGridTrackSize(direction, track_index)
-                               .MaxTrackBreadth()
-                               .Flex()));
+        flex_fraction,
+        NormalizedFlexFraction(
+            all_tracks[track_index],
+            GetGridTrackSize(direction, track_index).MaxTrackBreadth().Flex()));
   }
 
   const Grid& grid = algorithm_.GetGrid();
@@ -642,6 +683,7 @@ base::Optional<LayoutUnit> GridTrackSizingAlgorithm::AvailableSpace(
 }
 
 base::Optional<LayoutUnit> GridTrackSizingAlgorithm::AvailableSpace() const {
+  DCHECK(WasSetup());
   return AvailableSpace(direction_);
 }
 
@@ -729,9 +771,34 @@ GridTrackSize GridTrackSizingAlgorithm::RawGridTrackSize(
   return track_styles[untranslated_index - auto_repeat_tracks_count];
 }
 
+bool GridTrackSizingAlgorithm::IsRelativeGridLengthAsAuto(
+    const GridLength& length,
+    GridTrackSizingDirection direction) const {
+  if (!length.HasPercentage())
+    return false;
+  // TODO(svillar): we should remove the second check later. We need it
+  // because during the second iteration of the algorithm we set definite
+  // sizes in the grid container so percents would not resolve properly (it
+  // would think that the height is definite when it is not).
+  return !AvailableSpace(direction) ||
+         (direction == kForRows &&
+          !layout_grid_->CachedHasDefiniteLogicalHeight());
+}
+
+bool GridTrackSizingAlgorithm::IsRelativeSizedTrackAsAuto(
+    const GridTrackSize& track_size,
+    GridTrackSizingDirection direction) const {
+  if (track_size.MinTrackBreadth().HasPercentage())
+    return IsRelativeGridLengthAsAuto(track_size.MinTrackBreadth(), direction);
+  if (track_size.MaxTrackBreadth().HasPercentage())
+    return IsRelativeGridLengthAsAuto(track_size.MaxTrackBreadth(), direction);
+  return false;
+}
+
 GridTrackSize GridTrackSizingAlgorithm::GetGridTrackSize(
     GridTrackSizingDirection direction,
     size_t translated_index) const {
+  DCHECK(WasSetup());
   // Collapse empty auto repeat tracks if auto-fit.
   if (grid_.HasAutoRepeatEmptyTracks(direction) &&
       grid_.IsEmptyAutoRepeatTrack(direction, translated_index))
@@ -746,21 +813,13 @@ GridTrackSize GridTrackSizingAlgorithm::GetGridTrackSize(
   GridLength max_track_breadth = track_size.MaxTrackBreadth();
   // If the logical width/height of the grid container is indefinite, percentage
   // values are treated as <auto>.
-  if (min_track_breadth.HasPercentage() || max_track_breadth.HasPercentage()) {
-    // TODO(svillar): we should remove the second check later. We need it
-    // because during the second iteration of the algorithm we set definite
-    // sizes in the grid container so percents would not resolve properly (it
-    // would think that the height is definite when it is not).
-    if (!AvailableSpace(direction) ||
-        (direction == kForRows &&
-         !layout_grid_->CachedHasDefiniteLogicalHeight())) {
-      UseCounter::Count(layout_grid_->GetDocument(),
-                        WebFeature::kGridRowTrackPercentIndefiniteHeight);
-      if (min_track_breadth.HasPercentage())
-        min_track_breadth = Length(kAuto);
-      if (max_track_breadth.HasPercentage())
-        max_track_breadth = Length(kAuto);
-    }
+  if (IsRelativeSizedTrackAsAuto(track_size, direction)) {
+    UseCounter::Count(layout_grid_->GetDocument(),
+                      WebFeature::kGridRowTrackPercentIndefiniteHeight);
+    if (min_track_breadth.HasPercentage())
+      min_track_breadth = Length(kAuto);
+    if (max_track_breadth.HasPercentage())
+      max_track_breadth = Length(kAuto);
   }
 
   // Flex sizes are invalid as a min sizing function. However we still can have
@@ -1517,7 +1576,8 @@ void GridTrackSizingAlgorithm::UpdateBaselineAlignmentContext(
        child.HasRelativeLogicalWidth()) ||
       (can_participate_in_column_axis_baseline &&
        child.HasRelativeLogicalHeight())) {
-    layout_grid_->SetEstimatedGridAreaLogicalSize(grid_, child);
+    layout_grid_->UpdateGridAreaLogicalSize(
+        child, EstimatedGridAreaBreadthForChild(child));
   }
   child.LayoutIfNeeded();
 
@@ -1536,6 +1596,7 @@ void GridTrackSizingAlgorithm::ComputeBaselineAlignmentContext() {
 
 // Described in https://drafts.csswg.org/css-grid/#algo-track-sizing
 void GridTrackSizingAlgorithm::Run() {
+  DCHECK(WasSetup());
   StateMachine state_machine(*this);
 
   // Step 1.
@@ -1573,6 +1634,7 @@ void GridTrackSizingAlgorithm::Run() {
 }
 
 void GridTrackSizingAlgorithm::Reset() {
+  DCHECK(WasSetup());
   sizing_state_ = kColumnSizingFirstIteration;
   columns_.Shrink(0);
   rows_.Shrink(0);

third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.h

@@ -94,10 +94,9 @@ class GridTrackSizingAlgorithm final {
 
   // Required by LayoutGrid. Try to minimize the exposed surface.
   const Grid& GetGrid() const { return grid_; }
-  // TODO (jfernandez): We should declare this function as private and add
-  // CHECKS so that it's only run after setting up the algorithm.
-  GridTrackSize GetGridTrackSize(GridTrackSizingDirection,
-                                 size_t translated_index) const;
+  // TODO (jfernandez): We should remove any public getter for this attribute
+  // and encapsulate any access in the algorithm class.
+  Grid& GetMutableGrid() const { return grid_; }
   LayoutUnit MinContentSize() const { return min_content_size_; };
   LayoutUnit MaxContentSize() const { return max_content_size_; };
 
@@ -105,6 +104,8 @@ class GridTrackSizingAlgorithm final {
   LayoutUnit BaselineOffsetForChild(const LayoutBox&, GridAxis) const;
   void ClearBaselineAlignment();
 
+  LayoutSize EstimatedGridAreaBreadthForChild(const LayoutBox& child) const;
+
   Vector<GridTrack>& Tracks(GridTrackSizingDirection);
   const Vector<GridTrack>& Tracks(GridTrackSizingDirection) const;
 
@@ -120,6 +121,12 @@ class GridTrackSizingAlgorithm final {
 
  private:
   base::Optional<LayoutUnit> AvailableSpace() const;
+  bool IsRelativeGridLengthAsAuto(const GridLength&,
+                                  GridTrackSizingDirection) const;
+  bool IsRelativeSizedTrackAsAuto(const GridTrackSize&,
+                                  GridTrackSizingDirection) const;
+  GridTrackSize GetGridTrackSize(GridTrackSizingDirection,
+                                 size_t translated_index) const;
   GridTrackSize RawGridTrackSize(GridTrackSizingDirection,
                                  size_t translated_index) const;
   LayoutUnit ComputeTrackBasedSize() const;
@@ -145,6 +152,8 @@ class GridTrackSizingAlgorithm final {
       Vector<GridTrack*>& tracks,
       Vector<GridTrack*>* grow_beyond_growth_limits_tracks,
       LayoutUnit& available_logical_space) const;
+  LayoutUnit EstimatedGridAreaBreadthForChild(const LayoutBox&,
+                                              GridTrackSizingDirection) const;
   LayoutUnit GridAreaBreadthForChild(const LayoutBox&,
                                      GridTrackSizingDirection) const;
 
@@ -186,6 +195,7 @@ class GridTrackSizingAlgorithm final {
   bool IsValidTransition() const;
 
   // Data.
+  bool WasSetup() const { return !!strategy_; }
   bool needs_setup_{true};
   base::Optional<LayoutUnit> available_space_columns_;
   base::Optional<LayoutUnit> available_space_rows_;
@@ -292,6 +302,11 @@ class GridTrackSizingAlgorithmStrategy {
     return algorithm_.AvailableSpace();
   }
 
+  GridTrackSize GetGridTrackSize(GridTrackSizingDirection direction,
+                                 size_t translated_index) const {
+    return algorithm_.GetGridTrackSize(direction, translated_index);
+  }
+
   // Helper functions
   static bool ShouldClearOverrideContainingBlockContentSizeForChild(
       const LayoutGrid&,

third_party/blink/renderer/core/layout/layout_grid.cc

@@ -317,7 +317,7 @@ void LayoutGrid::UpdateBlockLayout(bool relayout_children) {
     TextAutosizer::LayoutScope text_autosizer_layout_scope(this, &layout_scope);
 
     LayoutUnit available_space_for_columns = AvailableLogicalWidth();
-    PlaceItemsOnGrid(*grid_, available_space_for_columns);
+    PlaceItemsOnGrid(track_sizing_algorithm_, available_space_for_columns);
 
     track_sizing_algorithm_.ComputeBaselineAlignmentContext();
 
@@ -337,7 +337,7 @@ void LayoutGrid::UpdateBlockLayout(bool relayout_children) {
           kForRows, AvailableLogicalHeight(kExcludeMarginBorderPadding));
     } else {
       ComputeTrackSizesForIndefiniteSize(track_sizing_algorithm_, kForRows,
-                                         *grid_, min_content_height_,
+                                         min_content_height_,
                                          max_content_height_);
     }
     LayoutUnit track_based_logical_height = ComputeTrackBasedLogicalHeight() +
@@ -498,12 +498,12 @@ void LayoutGrid::ComputeIntrinsicLogicalWidths(
     LayoutUnit& min_logical_width,
     LayoutUnit& max_logical_width) const {
   std::unique_ptr<Grid> grid = Grid::Create(this);
-  PlaceItemsOnGrid(*grid, base::nullopt);
-
   GridTrackSizingAlgorithm algorithm(this, *grid);
+  PlaceItemsOnGrid(algorithm, base::nullopt);
+
   algorithm.ComputeBaselineAlignmentContext();
-  ComputeTrackSizesForIndefiniteSize(algorithm, kForColumns, *grid,
-                                     min_logical_width, max_logical_width);
+  ComputeTrackSizesForIndefiniteSize(algorithm, kForColumns, min_logical_width,
+                                     max_logical_width);
 
   LayoutUnit scrollbar_width = LayoutUnit(ScrollbarLogicalWidth());
   min_logical_width += scrollbar_width;
@@ -513,9 +513,9 @@ void LayoutGrid::ComputeIntrinsicLogicalWidths(
 void LayoutGrid::ComputeTrackSizesForIndefiniteSize(
     GridTrackSizingAlgorithm& algo,
     GridTrackSizingDirection direction,
-    Grid& grid,
     LayoutUnit& min_intrinsic_size,
     LayoutUnit& max_intrinsic_size) const {
+  const Grid& grid = algo.GetGrid();
   algo.Setup(direction, NumTracks(direction, grid), base::nullopt);
   algo.Run();
 
@@ -746,8 +746,9 @@ size_t LayoutGrid::ClampAutoRepeatTracks(GridTrackSizingDirection direction,
 // (preferred widths). That's why we pass the responsibility to the caller who
 // does know whether the available logical width is indefinite or not.
 void LayoutGrid::PlaceItemsOnGrid(
-    Grid& grid,
+    GridTrackSizingAlgorithm& algorithm,
     base::Optional<LayoutUnit> available_logical_width) const {
+  Grid& grid = algorithm.GetMutableGrid();
   size_t auto_repeat_rows = ComputeAutoRepeatTracksCount(
       kForRows, ConvertLayoutUnitToOptional(
                     AvailableLogicalHeightForPercentageComputation()));
@@ -843,7 +844,7 @@ void LayoutGrid::PlaceItemsOnGrid(
   // don't participate in this process (see the function
   // PrepareOrthogonalWritingModeRootForLayout) because it's useless
   // and even wrong if they don't have their corresponding Grid Area.
-  LayoutOrthogonalWritingModeRoots(grid, orthogonal_grid_items);
+  LayoutOrthogonalWritingModeRoots(algorithm, orthogonal_grid_items);
 
 #if DCHECK_IS_ON()
   for (LayoutBox* child = grid.GetOrderIterator().First(); child;
@@ -872,7 +873,7 @@ static bool PrepareOrthogonalWritingModeRootForLayout(LayoutObject& root) {
 // TODO(lajava): Consider rafactoring this code with
 // LocalFrameView::LayoutOrthogonalWritingModeRoots
 void LayoutGrid::LayoutOrthogonalWritingModeRoots(
-    const Grid& grid,
+    const GridTrackSizingAlgorithm& algorithm,
     const Vector<LayoutBox*>& orthogonal_grid_items) const {
   if (!GetDocument().View()->IsInPerformLayout())
     return;
@@ -881,9 +882,7 @@ void LayoutGrid::LayoutOrthogonalWritingModeRoots(
     DCHECK(GridLayoutUtils::IsOrthogonalChild(*this, *root));
     if (PrepareOrthogonalWritingModeRootForLayout(*root)) {
       UpdateGridAreaLogicalSize(
-          *root,
-          LayoutSize(EstimatedGridAreaBreadthForChild(grid, *root, kForColumns),
-                     EstimatedGridAreaBreadthForChild(grid, *root, kForRows)));
+          *root, algorithm.EstimatedGridAreaBreadthForChild(*root));
       root->LayoutIfNeeded();
     }
   }
@@ -1349,40 +1348,6 @@ void LayoutGrid::LayoutPositionedObjects(bool relayout_children,
   }
 }
 
-LayoutUnit LayoutGrid::EstimatedGridAreaBreadthForChild(
-    const Grid& grid,
-    const LayoutBox& child,
-    GridTrackSizingDirection direction) const {
-  const GridSpan& span = grid.GridItemSpan(child, direction);
-  LayoutUnit grid_area_size;
-  bool grid_area_is_indefinite = false;
-  base::Optional<LayoutUnit> available_size =
-      track_sizing_algorithm_.AvailableSpace(direction);
-  for (auto track_position : span) {
-    GridLength max_track_size =
-        track_sizing_algorithm_.GetGridTrackSize(direction, track_position)
-            .MaxTrackBreadth();
-    if (max_track_size.IsContentSized() || max_track_size.IsFlex()) {
-      grid_area_is_indefinite = true;
-    } else {
-      grid_area_size += ValueForLength(max_track_size.length(),
-                                       available_size.value_or(LayoutUnit()));
-    }
-  }
-
-  grid_area_size += GuttersSize(*grid_, direction, span.StartLine(),
-                                span.IntegerSpan(), available_size);
-
-  GridTrackSizingDirection child_inline_direction =
-      GridLayoutUtils::FlowAwareDirectionForChild(*this, child, kForColumns);
-  if (grid_area_is_indefinite) {
-    return direction == child_inline_direction
-               ? std::max(child.MaxPreferredLogicalWidth(), grid_area_size)
-               : LayoutUnit(-1);
-  }
-  return grid_area_size;
-}
-
 LayoutUnit LayoutGrid::GridAreaBreadthForChildIncludingAlignmentOffsets(
     const LayoutBox& child,
     GridTrackSizingDirection direction) const {
@@ -2365,14 +2330,6 @@ size_t LayoutGrid::NumTracks(GridTrackSizingDirection direction,
                    StyleRef(), grid.AutoRepeatTracks(kForColumns));
 }
 
-void LayoutGrid::SetEstimatedGridAreaLogicalSize(const Grid& grid,
-                                                 LayoutBox& child) const {
-  UpdateGridAreaLogicalSize(
-      child,
-      LayoutSize(EstimatedGridAreaBreadthForChild(grid, child, kForColumns),
-                 EstimatedGridAreaBreadthForChild(grid, child, kForRows)));
-}
-
 LayoutUnit LayoutGrid::GridItemOffset(
     GridTrackSizingDirection direction) const {
   return direction == kForRows ? offset_between_rows_ : offset_between_columns_;

third_party/blink/renderer/core/layout/layout_grid.h

@@ -107,11 +107,8 @@ class LayoutGrid final : public LayoutBlock {
 
   LayoutUnit GridGap(GridTrackSizingDirection) const;
   LayoutUnit GridItemOffset(GridTrackSizingDirection) const;
-  void SetEstimatedGridAreaLogicalSize(const Grid&, LayoutBox&) const;
 
-  LayoutUnit EstimatedGridAreaBreadthForChild(const Grid&,
-                                              const LayoutBox&,
-                                              GridTrackSizingDirection) const;
+  void UpdateGridAreaLogicalSize(LayoutBox&, LayoutSize) const;
 
   StyleContentAlignmentData ContentAlignment(GridTrackSizingDirection) const;
 
@@ -165,11 +162,11 @@ class LayoutGrid final : public LayoutBlock {
       Grid&,
       GridTrackSizingDirection) const;
 
-  void LayoutOrthogonalWritingModeRoots(const Grid&,
+  void LayoutOrthogonalWritingModeRoots(const GridTrackSizingAlgorithm&,
                                         const Vector<LayoutBox*>&) const;
 
   void PlaceItemsOnGrid(
-      Grid&,
+      GridTrackSizingAlgorithm&,
       base::Optional<LayoutUnit> available_logical_width) const;
   void PopulateExplicitGridAndOrderIterator(Grid&) const;
   std::unique_ptr<GridArea> CreateEmptyGridAreaAtSpecifiedPositionsOutsideGrid(
@@ -189,7 +186,6 @@ class LayoutGrid final : public LayoutBlock {
 
   void ComputeTrackSizesForIndefiniteSize(GridTrackSizingAlgorithm&,
                                           GridTrackSizingDirection,
-                                          Grid&,
                                           LayoutUnit& min_intrinsic_size,
                                           LayoutUnit& max_intrinsic_size) const;
   LayoutUnit ComputeTrackBasedLogicalHeight() const;
@@ -199,7 +195,6 @@ class LayoutGrid final : public LayoutBlock {
   void RepeatTracksSizingIfNeeded(LayoutUnit available_space_for_columns,
                                   LayoutUnit available_space_for_rows);
 
-  void UpdateGridAreaLogicalSize(LayoutBox&, LayoutSize) const;
   void LayoutGridItems();
   void PrepareChildForPositionedLayout(LayoutBox&);
   bool HasStaticPositionForChild(const LayoutBox&,