[css-grid] Avoid HashSet when iterating grid items

Both GridTrackSizingAlgorithm::ResolveIntrinsicTrackSizes and
IndefiniteSizeStrategy::FindUsedFlexFraction have a vector of track
indices and want to iterate the grid items in these tracks. But they
only want to do the work once for each item, even if it spans multiple
cells in one of these tracks, or it spans multiple of these tracks.

What they used to do was to insert the items provided by NextGridItem
into a HashSet, and check whether it was a new entry or it was already
there.

Instead, this patch modifies ListGridIterator::NextGridItem so that it
skips items already processed in an earlier cell of that track, and adds
a IterateGridItemsInTrackIndices helper function which takes care of
creating the iterators, calls NextGridItem, and skips items already
processed in an earlier track.

The already processed items are detected looking at their indices, so
the HashSet is no longer used in release builds. It's only kept in debug
builds to ensure that the logic is solid.

This should have no effect in practice, other than better performance:
the nested-grid-lots-of-tracks perf test improves by ~26%, and the
nested-grid, auto-grid-lots-of-data and auto-grid-lots-of-spanning-data
improve by 5-7%. On the other hand, fixed-grid-lots-of-data and
fixed-grid-lots-of-stretched-data worsen a bit by 2-3%.

Change-Id: Ibedbfa48c8825cff129797ba759dc22cf6972017
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508472Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#822623}

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

@@ -397,16 +397,26 @@ LayoutBox* ListGridIterator::NextGridItem() {
     return cell_node_->Items()[child_index_++];
   }
 
-  if (child_index_ < cell_node_->Items().size())
-    return cell_node_->Items()[child_index_++];
-
-  child_index_ = 0;
-  cell_node_ = cell_node_->NextInDirection(direction_);
-  if (!cell_node_)
-    return nullptr;
-
-  DCHECK(!cell_node_->Items().IsEmpty());
-  return cell_node_->Items()[child_index_++];
+  GridTrackSizingDirection other_direction =
+      is_row_axis ? kForRows : kForColumns;
+  while (true) {
+    LayoutBox* candidate;
+    if (child_index_ < cell_node_->Items().size()) {
+      candidate = cell_node_->Items()[child_index_++];
+    } else {
+      child_index_ = 0;
+      cell_node_ = cell_node_->NextInDirection(direction_);
+      if (!cell_node_)
+        return nullptr;
+
+      DCHECK(!cell_node_->Items().IsEmpty());
+      candidate = cell_node_->Items()[child_index_++];
+    }
+    // Skip items already processed in an earlier cell of the track.
+    const GridSpan& span = grid_.GridItemSpan(*candidate, other_direction);
+    if (span.StartLine() == cell_node_->Index(other_direction))
+      return candidate;
+  }
 }
 
 std::unique_ptr<GridArea> ListGridIterator::NextEmptyGridArea(

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

@@ -90,6 +90,32 @@ static GridTrackSizingDirection GridDirectionForAxis(GridAxis axis) {
   return axis == kGridRowAxis ? kForColumns : kForRows;
 }
 
+template <typename F>
+static void IterateGridItemsInTrackIndices(const Grid& grid,
+                                           GridTrackSizingDirection direction,
+                                           Vector<size_t>& track_indices,
+                                           F callback) {
+#if DCHECK_IS_ON()
+  HashSet<LayoutBox*> items_set;
+#endif
+  for (size_t i = 0; i < track_indices.size(); ++i) {
+    auto iterator = grid.CreateIterator(direction, track_indices[i]);
+    while (LayoutBox* grid_item = iterator->NextGridItem()) {
+      const GridSpan& span = grid.GridItemSpan(*grid_item, direction);
+      if (i > 0) {
+        // Skip items already processed in an earlier track.
+        DCHECK_LT(track_indices[i - 1], track_indices[i]);
+        if (span.StartLine() <= track_indices[i - 1])
+          continue;
+      }
+#if DCHECK_IS_ON()
+      DCHECK(items_set.insert(grid_item).is_new_entry);
+#endif
+      callback(grid_item, span);
+    }
+  }
+}
+
 class IndefiniteSizeStrategy final : public GridTrackSizingAlgorithmStrategy {
  public:
   IndefiniteSizeStrategy(GridTrackSizingAlgorithm& algorithm)
@@ -694,27 +720,19 @@ double IndefiniteSizeStrategy::FindUsedFlexFraction(
   if (!grid.HasGridItems())
     return flex_fraction;
 
-  HashSet<LayoutBox*> items_set;
-  for (const auto& track_index : flexible_sized_tracks_index) {
-    auto iterator = grid.CreateIterator(direction, track_index);
-    while (LayoutBox* grid_item = iterator->NextGridItem()) {
-      // Do not include already processed items.
-      if (!items_set.insert(grid_item).is_new_entry)
-        continue;
-
-      const GridSpan& span = grid.GridItemSpan(*grid_item, direction);
-
-      // Removing gutters from the max-content contribution of the item,
-      // so they are not taken into account in FindFrUnitSize().
-      LayoutUnit left_over_space =
-          MaxContentForChild(*grid_item) -
-          GetLayoutGrid()->GuttersSize(algorithm_.GetGrid(), direction,
-                                       span.StartLine(), span.IntegerSpan(),
-                                       AvailableSpace());
-      flex_fraction =
-          std::max(flex_fraction, FindFrUnitSize(span, left_over_space));
-    }
-  }
+  IterateGridItemsInTrackIndices(
+      grid, direction, flexible_sized_tracks_index,
+      [&](LayoutBox* grid_item, const GridSpan& span) {
+        // Removing gutters from the max-content contribution of the item,
+        // so they are not taken into account in FindFrUnitSize().
+        LayoutUnit left_over_space =
+            MaxContentForChild(*grid_item) -
+            GetLayoutGrid()->GuttersSize(algorithm_.GetGrid(), direction,
+                                         span.StartLine(), span.IntegerSpan(),
+                                         AvailableSpace());
+        flex_fraction =
+            std::max(flex_fraction, FindFrUnitSize(span, left_over_space));
+      });
 
   return flex_fraction;
 }
@@ -1449,22 +1467,17 @@ void GridTrackSizingAlgorithm::ResolveIntrinsicTrackSizes() {
   Vector<GridTrack>& all_tracks = Tracks(direction_);
   Vector<GridItemWithSpan> items_sorted_by_increasing_span;
   if (grid_.HasGridItems()) {
-    HashSet<LayoutBox*> items_set;
-    for (const auto& track_index : content_sized_tracks_index_) {
-      auto iterator = grid_.CreateIterator(direction_, track_index);
-      GridTrack& track = all_tracks[track_index];
-      while (auto* grid_item = iterator->NextGridItem()) {
-        if (items_set.insert(grid_item).is_new_entry) {
-          const GridSpan& span = grid_.GridItemSpan(*grid_item, direction_);
+    IterateGridItemsInTrackIndices(
+        grid_, direction_, content_sized_tracks_index_,
+        [&](LayoutBox* grid_item, const GridSpan& span) {
           if (span.IntegerSpan() == 1) {
-            SizeTrackToFitNonSpanningItem(span, *grid_item, track);
+            SizeTrackToFitNonSpanningItem(span, *grid_item,
+                                          all_tracks[span.StartLine()]);
           } else if (!SpanningItemCrossesFlexibleSizedTracks(span)) {
             items_sorted_by_increasing_span.push_back(
                 GridItemWithSpan(*grid_item, span));
           }
-        }
-      }
-    }
+        });
     std::sort(items_sorted_by_increasing_span.begin(),
               items_sorted_by_increasing_span.end());
   }