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

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

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 529 matching lines @@
   size_t repetitions =
       1 + (freeSpace / (autoRepeatTracksSize + gapSize)).toInt();
 
   // Provided the grid container does not have a definite size or max-size in
   // the relevant axis, if the min size is definite then the number of
   // repetitions is the largest possible positive integer that fulfills that
   // minimum requirement.
   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;
 }
 
 std::unique_ptr<OrderedTrackIndexSet>
 LayoutGrid::computeEmptyTracksForAutoRepeat(
     Grid& grid,
     GridTrackSizingDirection direction) const {
   bool isRowAxis = direction == ForColumns;
   if ((isRowAxis && styleRef().gridAutoRepeatColumnsType() != AutoFit) ||
       (!isRowAxis && styleRef().gridAutoRepeatRowsType() != AutoFit))
@@ skipping 20 matching lines @@
       if (!iterator.nextGridItem()) {
         if (!emptyTrackIndexes)
           emptyTrackIndexes = WTF::wrapUnique(new OrderedTrackIndexSet);
         emptyTrackIndexes->add(trackIndex);
       }
     }
   }
   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);
     grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);
   }
 
   if (!grid.needsItemsPlacement())
     return;
 
   DCHECK(!grid.hasGridItems());
@@ skipping 1462 matching lines @@
   if (direction == ForRows)
     return grid.numTracks(ForRows);
 
   return grid.numTracks(ForRows)
              ? grid.numTracks(ForColumns)
              : GridPositionsResolver::explicitGridColumnCount(
                    styleRef(), grid.autoRepeatTracks(ForColumns));
 }
 
 }  // namespace blink