[GridNG] Refactor track direction to be contained in track collection

1. Refactoring `NGGridLayoutAlgorithmTrackCollection::SetIterator` to
   allow callers access to the iterator from a const reference/pointer
   to the collection; this change improves the `NGGridLayoutAlgorithm`
   implementation since previously we were not able to pass const
   references to an algorithm track collection as an input parameter
   (because of the non-const restriction imposed by `SetIterator`).

2. Refactoring use of `GridTrackSizingDirection` into track collections;
   several methods in `NGGridLayoutAlgorithm` received both, track
   collection and its respective direction, as parameters. Since the
   direction is already tied to the specified collection, this change
   moves the `GridTrackSizingDirection` into the collection class.

Bug: 1045599
Change-Id: Ibbc7d2d93c5947106b0fe627d67dce2fb24916f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2573889Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Jacques Newman <janewman@microsoft.com>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ethan Jimenez <ethavar@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#833870}

third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.h

@@ -122,7 +122,6 @@ class CORE_EXPORT NGGridLayoutAlgorithm
   // in the relevant track collection.
   static NGGridLayoutAlgorithmTrackCollection::SetIterator
   GetSetIteratorForItem(const GridItemData& item,
-                        GridTrackSizingDirection track_direction,
                         NGGridLayoutAlgorithmTrackCollection& track_collection);
 
   // Returns the size that a grid item will distribute across the tracks with an
@@ -149,8 +148,7 @@ class CORE_EXPORT NGGridLayoutAlgorithm
       NGGridLayoutAlgorithmTrackCollection* row_track_collection) const;
 
   // Sets specified track lists on |track_collection|.
-  void SetSpecifiedTracks(GridTrackSizingDirection track_direction,
-                          wtf_size_t automatic_repetitions,
+  void SetSpecifiedTracks(wtf_size_t automatic_repetitions,
                           NGGridBlockTrackCollection* track_collection) const;
   // Determines the explicit column and row track starts.
   void DetermineExplicitTrackStarts(wtf_size_t automatic_column_repetitions,
@@ -164,32 +162,27 @@ class CORE_EXPORT NGGridLayoutAlgorithm
   // "begin" and "end" such that the item spans every set from the respective
   // collection's |sets_| with an index in the range [begin, end).
   void CacheItemSetIndices(
-      GridTrackSizingDirection track_direction,
-      const NGGridLayoutAlgorithmTrackCollection* track_collection,
+      const NGGridLayoutAlgorithmTrackCollection& track_collection,
       Vector<GridItemData>* grid_items) const;
   // For every grid item, determines if it spans a track with an intrinsic or
   // flexible sizing function and caches the answer in its |GridItemData|.
   void DetermineGridItemsSpanningIntrinsicOrFlexTracks(
-      GridTrackSizingDirection track_direction,
+      const NGGridLayoutAlgorithmTrackCollection& track_collection,
       Vector<GridItemData>* grid_items,
-      Vector<wtf_size_t>* reordered_item_indices,
-      NGGridLayoutAlgorithmTrackCollection* track_collection) const;
+      Vector<wtf_size_t>* reordered_item_indices) const;
 
   // Calculates from the min and max track sizing functions the used track size.
   void ComputeUsedTrackSizes(
-      GridTrackSizingDirection track_direction,
-      Vector<GridItemData>* grid_items,
-      NGGridLayoutAlgorithmTrackCollection* track_collection) const;
+      NGGridLayoutAlgorithmTrackCollection* track_collection,
+      Vector<GridItemData>* grid_items) const;
 
   // These methods implement the steps of the algorithm for intrinsic track size
   // resolution defined in https://drafts.csswg.org/css-grid-1/#algo-content.
   void ResolveIntrinsicTrackSizes(
-      GridTrackSizingDirection track_direction,
+      NGGridLayoutAlgorithmTrackCollection* track_collection,
       Vector<GridItemData>* grid_items,
-      Vector<wtf_size_t>* reordered_item_indices,
-      NGGridLayoutAlgorithmTrackCollection* track_collection) const;
+      Vector<wtf_size_t>* reordered_item_indices) const;
   void IncreaseTrackSizesToAccommodateGridItems(
-      GridTrackSizingDirection track_direction,
       ReorderedGridItems::Iterator group_begin,
       ReorderedGridItems::Iterator group_end,
       GridItemContributionType contribution_type,
@@ -220,9 +213,8 @@ class CORE_EXPORT NGGridLayoutAlgorithm
 
   // Calculates inline and block offsets for all tracks.
   Vector<LayoutUnit> ComputeSetOffsets(
-      GridTrackSizingDirection track_direction,
-      LayoutUnit grid_gap,
-      NGGridLayoutAlgorithmTrackCollection& track_collection) const;
+      const NGGridLayoutAlgorithmTrackCollection& track_collection,
+      LayoutUnit grid_gap) const;
 
   // Tests whether the row gap is unresolvable based on its type and the
   // available size.

third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm_test.cc

@@ -50,22 +50,18 @@ class NGGridLayoutAlgorithmTest
         &algorithm_row_track_collection_);
 
     // Cache set indices.
-    algorithm.CacheItemSetIndices(GridTrackSizingDirection::kForColumns,
-                                  &algorithm_column_track_collection_,
+    algorithm.CacheItemSetIndices(algorithm_column_track_collection_,
                                   &grid_items_);
-    algorithm.CacheItemSetIndices(GridTrackSizingDirection::kForRows,
-                                  &algorithm_row_track_collection_,
+    algorithm.CacheItemSetIndices(algorithm_row_track_collection_,
                                   &grid_items_);
 
     // Resolve inline size.
-    algorithm.ComputeUsedTrackSizes(GridTrackSizingDirection::kForColumns,
-                                    &grid_items_,
-                                    &algorithm_column_track_collection_);
+    algorithm.ComputeUsedTrackSizes(&algorithm_column_track_collection_,
+                                    &grid_items_);
 
     // Resolve block size.
-    algorithm.ComputeUsedTrackSizes(GridTrackSizingDirection::kForRows,
-                                    &grid_items_,
-                                    &algorithm_row_track_collection_);
+    algorithm.ComputeUsedTrackSizes(&algorithm_row_track_collection_,
+                                    &grid_items_);
   }
 
   NGGridLayoutAlgorithmTrackCollection& TrackCollection(
@@ -116,15 +112,14 @@ class NGGridLayoutAlgorithmTest
   }
 
   void DetermineGridItemsSpanningIntrinsicOrFlexTracks(
-      NGGridLayoutAlgorithm& algorithm,
-      GridTrackSizingDirection track_direction) {
+      const NGGridLayoutAlgorithm& algorithm,
+      const NGGridLayoutAlgorithmTrackCollection& track_collection) {
     Vector<wtf_size_t> reordered_item_indices;
     reordered_item_indices.ReserveInitialCapacity(grid_items_.size());
     for (wtf_size_t i = 0; i < grid_items_.size(); ++i)
       reordered_item_indices.push_back(i);
     algorithm.DetermineGridItemsSpanningIntrinsicOrFlexTracks(
-        track_direction, &grid_items_, &reordered_item_indices,
-        &TrackCollection(track_direction));
+        track_collection, &grid_items_, &reordered_item_indices);
   }
 
   Vector<wtf_size_t> GridItemsSpanningIntrinsicTrack(
@@ -1196,7 +1191,8 @@ TEST_F(NGGridLayoutAlgorithmTest,
   NGGridLayoutAlgorithm algorithm({node, fragment_geometry, space});
   BuildGridItemsAndTrackCollections(algorithm);
 
-  DetermineGridItemsSpanningIntrinsicOrFlexTracks(algorithm, kForColumns);
+  DetermineGridItemsSpanningIntrinsicOrFlexTracks(algorithm,
+                                                  TrackCollection(kForColumns));
   Vector<wtf_size_t> expected_grid_items_spanning_intrinsic_track = {0, 1, 3};
   Vector<wtf_size_t> expected_grid_items_spanning_flex_track = {1};
 
@@ -1212,7 +1208,8 @@ TEST_F(NGGridLayoutAlgorithmTest,
   for (wtf_size_t i = 0; i < actual_items.size(); ++i)
     EXPECT_EQ(expected_grid_items_spanning_flex_track[i], actual_items[i]);
 
-  DetermineGridItemsSpanningIntrinsicOrFlexTracks(algorithm, kForRows);
+  DetermineGridItemsSpanningIntrinsicOrFlexTracks(algorithm,
+                                                  TrackCollection(kForRows));
   expected_grid_items_spanning_intrinsic_track = {1, 2, 3};
   expected_grid_items_spanning_flex_track = {2};
 

third_party/blink/renderer/core/layout/ng/grid/ng_grid_track_collection.cc

@@ -136,6 +136,10 @@ bool NGGridTrackCollectionBase::RangeRepeatIterator::SetRangeIndex(
   return true;
 }
 
+NGGridBlockTrackCollection::NGGridBlockTrackCollection(
+    GridTrackSizingDirection direction)
+    : direction_(direction) {}
+
 void NGGridBlockTrackCollection::SetSpecifiedTracks(
     const NGGridTrackList* explicit_tracks,
     const NGGridTrackList* implicit_tracks,
@@ -481,36 +485,10 @@ NGGridLayoutAlgorithmTrackCollection::Range::Range(
       starting_set_index(starting_set_index),
       is_collapsed(block_track_range.is_collapsed) {}
 
-NGGridLayoutAlgorithmTrackCollection::SetIterator::SetIterator(
-    NGGridLayoutAlgorithmTrackCollection* collection,
-    wtf_size_t begin_set_index,
-    wtf_size_t end_set_index)
-    : collection_(collection),
-      current_set_index_(begin_set_index),
-      end_set_index_(end_set_index) {
-  DCHECK(collection_);
-  DCHECK_LE(current_set_index_, end_set_index_);
-}
-
-bool NGGridLayoutAlgorithmTrackCollection::SetIterator::IsAtEnd() const {
-  DCHECK_LE(current_set_index_, end_set_index_);
-  return current_set_index_ == end_set_index_;
-}
-
-bool NGGridLayoutAlgorithmTrackCollection::SetIterator::MoveToNextSet() {
-  current_set_index_ = std::min(current_set_index_ + 1, end_set_index_);
-  return current_set_index_ < end_set_index_;
-}
-
-NGGridSet& NGGridLayoutAlgorithmTrackCollection::SetIterator::CurrentSet()
-    const {
-  DCHECK_LT(current_set_index_, end_set_index_);
-  return collection_->SetAt(current_set_index_);
-}
-
 NGGridLayoutAlgorithmTrackCollection::NGGridLayoutAlgorithmTrackCollection(
     const NGGridBlockTrackCollection& block_track_collection,
-    bool is_content_box_size_indefinite) {
+    bool is_content_box_size_indefinite)
+    : direction_(block_track_collection.Direction()) {
   for (auto range_iterator = block_track_collection.RangeIterator();
        !range_iterator.IsAtEnd(); range_iterator.MoveToNextRange()) {
     const NGGridBlockTrackCollection::Range& block_track_range =
@@ -602,19 +580,35 @@ NGGridSet& NGGridLayoutAlgorithmTrackCollection::SetAt(wtf_size_t set_index) {
   return sets_[set_index];
 }
 
+const NGGridSet& NGGridLayoutAlgorithmTrackCollection::SetAt(
+    wtf_size_t set_index) const {
+  DCHECK_LT(set_index, SetCount());
+  return sets_[set_index];
+}
+
 NGGridLayoutAlgorithmTrackCollection::SetIterator
 NGGridLayoutAlgorithmTrackCollection::GetSetIterator() {
   return SetIterator(this, 0u, SetCount());
 }
 
+NGGridLayoutAlgorithmTrackCollection::ConstSetIterator
+NGGridLayoutAlgorithmTrackCollection::GetSetIterator() const {
+  return ConstSetIterator(this, 0u, SetCount());
+}
+
 NGGridLayoutAlgorithmTrackCollection::SetIterator
 NGGridLayoutAlgorithmTrackCollection::GetSetIterator(wtf_size_t begin_set_index,
                                                      wtf_size_t end_set_index) {
-  DCHECK_LE(end_set_index, SetCount());
-  DCHECK_LE(begin_set_index, end_set_index);
   return SetIterator(this, begin_set_index, end_set_index);
 }
 
+NGGridLayoutAlgorithmTrackCollection::ConstSetIterator
+NGGridLayoutAlgorithmTrackCollection::GetSetIterator(
+    wtf_size_t begin_set_index,
+    wtf_size_t end_set_index) const {
+  return ConstSetIterator(this, begin_set_index, end_set_index);
+}
+
 wtf_size_t NGGridLayoutAlgorithmTrackCollection::RangeSetCount(
     wtf_size_t range_index) const {
   DCHECK_LT(range_index, RangeCount());

third_party/blink/renderer/core/layout/ng/grid/ng_grid_track_collection.h

@@ -58,13 +58,10 @@ class CORE_EXPORT NGGridTrackCollectionBase {
  protected:
   // Returns the first track number of a range.
   virtual wtf_size_t RangeTrackNumber(wtf_size_t range_index) const = 0;
-
   // Returns the number of tracks in a range.
   virtual wtf_size_t RangeTrackCount(wtf_size_t range_index) const = 0;
-
   // Returns true if the range at the given index is collapsed.
   virtual bool IsRangeCollapsed(wtf_size_t range_index) const = 0;
-
   // Returns the number of track ranges in the collection.
   virtual wtf_size_t RangeCount() const = 0;
 };
@@ -82,6 +79,9 @@ class CORE_EXPORT NGGridBlockTrackCollection
     bool is_collapsed : 1;
   };
 
+  explicit NGGridBlockTrackCollection(
+      GridTrackSizingDirection track_direction = kForColumns);
+
   // Sets the specified, implicit tracks, along with a given auto repeat value.
   void SetSpecifiedTracks(const NGGridTrackList* explicit_tracks,
                           const NGGridTrackList* implicit_tracks,
@@ -98,9 +98,11 @@ class CORE_EXPORT NGGridBlockTrackCollection
   // Returns the range at the given track.
   const Range& RangeAtTrackNumber(wtf_size_t track_number) const;
 
+  GridTrackSizingDirection Direction() const { return direction_; }
+  bool IsForColumns() const { return direction_ == kForColumns; }
+
   const NGGridTrackList& ExplicitTracks() const;
   const NGGridTrackList& ImplicitTracks() const;
-
   String ToString() const;
 
  protected:
@@ -117,6 +119,7 @@ class CORE_EXPORT NGGridBlockTrackCollection
   wtf_size_t ImplicitRepeatSize() const;
 
   bool track_indices_need_sort_ = false;
+  GridTrackSizingDirection direction_;
   wtf_size_t auto_repeat_count_ = 0;
 
   // Stores the specified and implicit tracks specified by SetSpecifiedTracks.
@@ -233,23 +236,51 @@ class CORE_EXPORT NGGridLayoutAlgorithmTrackCollection
     bool is_collapsed : 1;
   };
 
-  // Note that this iterator can alter any set's data.
-  class CORE_EXPORT SetIterator {
+  template <bool is_const>
+  class CORE_EXPORT SetIteratorBase {
    public:
-    SetIterator(NGGridLayoutAlgorithmTrackCollection* collection,
+    using TrackCollectionPtr =
+        typename std::conditional<is_const,
+                                  const NGGridLayoutAlgorithmTrackCollection*,
+                                  NGGridLayoutAlgorithmTrackCollection*>::type;
+    using NGGridSetRef =
+        typename std::conditional<is_const, const NGGridSet&, NGGridSet&>::type;
+
+    SetIteratorBase(TrackCollectionPtr track_collection,
                     wtf_size_t begin_set_index,
-                wtf_size_t end_set_index);
+                    wtf_size_t end_set_index)
+        : track_collection_(track_collection),
+          current_set_index_(begin_set_index),
+          end_set_index_(end_set_index) {
+      DCHECK(track_collection_);
+      DCHECK_LE(current_set_index_, end_set_index_);
+      DCHECK_LE(end_set_index_, track_collection_->SetCount());
+    }
 
-    bool IsAtEnd() const;
-    bool MoveToNextSet();
-    NGGridSet& CurrentSet() const;
+    bool IsAtEnd() const {
+      DCHECK_LE(current_set_index_, end_set_index_);
+      return current_set_index_ == end_set_index_;
+    }
+
+    bool MoveToNextSet() {
+      current_set_index_ = std::min(current_set_index_ + 1, end_set_index_);
+      return current_set_index_ < end_set_index_;
+    }
+
+    NGGridSetRef CurrentSet() const {
+      DCHECK_LT(current_set_index_, end_set_index_);
+      return track_collection_->SetAt(current_set_index_);
+    }
 
    private:
-    NGGridLayoutAlgorithmTrackCollection* collection_;
+    TrackCollectionPtr track_collection_;
     wtf_size_t current_set_index_;
     wtf_size_t end_set_index_;
   };
 
+  using SetIterator = SetIteratorBase<false>;
+  using ConstSetIterator = SetIteratorBase<true>;
+
   NGGridLayoutAlgorithmTrackCollection() = default;
   // |is_content_box_size_indefinite| is used to normalize percentage track
   // sizing functions (see the constructor for |NGGridSet|).
@@ -261,12 +292,16 @@ class CORE_EXPORT NGGridLayoutAlgorithmTrackCollection
   wtf_size_t SetCount() const;
   // Returns a reference to the set located at position |set_index|.
   NGGridSet& SetAt(wtf_size_t set_index);
+  const NGGridSet& SetAt(wtf_size_t set_index) const;
   // Returns an iterator for all the sets contained in this collection.
   SetIterator GetSetIterator();
+  ConstSetIterator GetSetIterator() const;
   // Returns an iterator for every set in this collection's |sets_| located at
   // an index in the interval [begin_set_index, end_set_index).
   SetIterator GetSetIterator(wtf_size_t begin_set_index,
                              wtf_size_t end_set_index);
+  ConstSetIterator GetSetIterator(wtf_size_t begin_set_index,
+                                  wtf_size_t end_set_index) const;
 
   wtf_size_t RangeSetCount(wtf_size_t range_index) const;
   wtf_size_t RangeStartingSetIndex(wtf_size_t range_index) const;
@@ -276,6 +311,9 @@ class CORE_EXPORT NGGridLayoutAlgorithmTrackCollection
   // Returns true if the range contains a set with a flexible sizing function.
   bool IsRangeSpanningFlexTrack(wtf_size_t range_index) const;
 
+  GridTrackSizingDirection Direction() const { return direction_; }
+  bool IsForColumns() const { return direction_ == kForColumns; }
+
  protected:
   // NGGridTrackCollectionBase overrides.
   wtf_size_t RangeTrackNumber(wtf_size_t range_index) const override;
@@ -289,6 +327,8 @@ class CORE_EXPORT NGGridLayoutAlgorithmTrackCollection
       const NGGridTrackList& specified_track_list,
       bool is_content_box_size_indefinite);
 
+  GridTrackSizingDirection direction_;
+
   Vector<Range> ranges_;
   // A vector of every set element that compose the entire collection's ranges;
   // track definitions from the same set are stored in consecutive positions,