[css-grid] Clamp the number of auto repeat tracks in all cases

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
index 34b55a70a2063202930120719a8669c2e797dda3..f4e0f5b16cc5692a5dd07600daa664392e721c44 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -547,13 +547,6 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(
   if (needsToFulfillMinimumSize)
     ++repetitions;
 
-  // Clamp the number of repetitions so we don't end up with too many tracks.
-  if (repetitions > kGridMaxTracks) {
-    DCHECK_GT(autoRepeatTrackListLength, 0u);
-    repetitions =
-        (kGridMaxTracks - trackSizes.size()) / autoRepeatTrackListLength;
-  }
-
   return repetitions * autoRepeatTrackListLength;
 }
 
@@ -594,12 +587,42 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
   return emptyTrackIndexes;
 }
 
+size_t LayoutGrid::clampAutoRepeatTracks(GridTrackSizingDirection direction,
+                                         size_t autoRepeatTracks) const {
+  if (!autoRepeatTracks)
+    return 0;
+
+  bool isRowAxis = direction == ForColumns;
+  size_t otherTracks = isRowAxis ? styleRef().gridTemplateColumns().size()
+                                 : styleRef().gridTemplateRows().size();
+  size_t totalTracks = autoRepeatTracks + otherTracks;
+
+  if (totalTracks <= kGridMaxTracks)
+    return autoRepeatTracks;
+
+  if (otherTracks >= kGridMaxTracks)

[Manuel Rego, 2017/02/06 09:51:12]

We can move this if above.

[svillar, 2017/02/06 10:06:03]

Acknowledged.

+    return 0;
+
+  size_t autoRepeatTrackListLength =
+      isRowAxis ? styleRef().gridAutoRepeatColumns().size()
+                : styleRef().gridAutoRepeatRows().size();
+  DCHECK_GT(autoRepeatTrackListLength, 0u);
+
+  size_t repetitions =
+      (kGridMaxTracks - otherTracks) / autoRepeatTrackListLength;
+  return repetitions * autoRepeatTrackListLength;

[Manuel Rego, 2017/02/06 09:51:12]

Might be worth to explain here that we've decided to reduce the auto-repeat
tracks.

Also do we have tests for this?

For example if we've:
  width: 2000px;
  grid-template-columns: 10px 5px repeat(auto-fill, 1px);

If I understand it properly the final result would be:
  grid-template-columns: 10px 5px repeat(998, 1px);

[svillar, 2017/02/06 10:06:03]

Yeah, you're right. As this is not in the specs we should document it.
Pretty similar ones were already present in the test I'm modifying. For example:

.lotsOfAutoRepeatWithAutoFitCols { grid-template-columns: repeat(auto-fit, 10px
2px 8px) repeat(10, 1px); }

[Manuel Rego, 2017/02/06 10:09:52]

Oops, sorry but I believe this is against the spec:
 "Since memory is limited, UAs may clamp the possible size
  of the grid to be within a UA-defined limit,
  dropping all lines outside that limit."

So in the following example:
  width: 2000px;
  grid-template-columns: repeat(auto-fill, 1px) 10px 5px;
The result right now would be (considering a limit of 1000 tracks):
  grid-template-columns: repeat(998, 1px) 10px 5px;
But I think per spec it should be:
  grid-template-columns: repeat(1000, 1px);

So no-lgtm until we change this.
Nice, I missed them as they were not added in this CL.

+}
+
 void LayoutGrid::placeItemsOnGrid(Grid& grid,
                                   SizingOperation sizingOperation) const {
   size_t autoRepeatRows =
       computeAutoRepeatTracksCount(ForRows, sizingOperation);
   size_t autoRepeatColumns =
       computeAutoRepeatTracksCount(ForColumns, sizingOperation);
+
+  autoRepeatRows = clampAutoRepeatTracks(ForRows, autoRepeatRows);
+  autoRepeatColumns = clampAutoRepeatTracks(ForColumns, autoRepeatColumns);
+
   if (autoRepeatRows != grid.autoRepeatTracks(ForRows) ||
       autoRepeatColumns != grid.autoRepeatTracks(ForColumns)) {
     grid.setNeedsItemsPlacement(true);