[css-grid] Isolate intrinsic size computation from layout

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 622 matching lines @@
         child->clearContainingBlockOverrideSize();
         child->forceLayout();
       }
     }
 
     updateLogicalWidth();
     m_hasDefiniteLogicalHeight = hasDefiniteLogicalHeight();
 
     TextAutosizer::LayoutScope textAutosizerLayoutScope(this, &layoutScope);
 
-    // TODO(svillar): we won't need to do this once the intrinsic width
-    // computation is isolated from the LayoutGrid object state (it should not
-    // touch any attribute) (see crbug.com/627812)
-    size_t autoRepeatColumns = m_grid.autoRepeatTracks(ForColumns);
-    if (autoRepeatColumns &&
-        autoRepeatColumns !=
-            computeAutoRepeatTracksCount(ForColumns, TrackSizing))
-      dirtyGrid();
     placeItemsOnGrid(m_grid, TrackSizing);
 
     GridSizingData sizingData(numTracks(ForColumns, m_grid),
                               numTracks(ForRows, m_grid), m_grid);
 
     // 1- First, the track sizing algorithm is used to resolve the sizes of the
     // grid columns.
     // At this point the logical width is always definite as the above call to
     // updateLogicalWidth() properly resolves intrinsic sizes. We cannot do the
     // same for heights though because many code paths inside
@@ skipping 140 matching lines @@
     if (nonEmptyTracksAfterEndLine)
       gapAccumulator += gap;
   }
 
   return gapAccumulator;
 }
 
 void LayoutGrid::computeIntrinsicLogicalWidths(
     LayoutUnit& minLogicalWidth,
     LayoutUnit& maxLogicalWidth) const {
-  const_cast<LayoutGrid*>(this)->placeItemsOnGrid(const_cast<Grid&>(m_grid),
-                                                  IntrinsicSizeComputation);
+  Grid grid(this);
+  placeItemsOnGrid(grid, IntrinsicSizeComputation);

[jfernandez, 2016/12/20 15:57:55]

What do you think about creating the new Grid instantiate inside
placeItemsOnGrid ? Is there any other way where the Grid instance is created ?

[svillar, 2016/12/20 16:21:43]

We cannot move it inside the method because for the case of layout we don't need
to create anything, we just use m_grid.

 
-  GridSizingData sizingData(numTracks(ForColumns, m_grid),
-                            numTracks(ForRows, m_grid),
-                            const_cast<Grid&>(m_grid));
+  GridSizingData sizingData(numTracks(ForColumns, grid),
+                            numTracks(ForRows, grid), grid);
+
   computeTrackSizesForIndefiniteSize(ForColumns, sizingData, minLogicalWidth,
                                      maxLogicalWidth);
 
   LayoutUnit scrollbarWidth = LayoutUnit(scrollbarLogicalWidth());
   minLogicalWidth += scrollbarWidth;
   maxLogicalWidth += scrollbarWidth;
 }
 
 void LayoutGrid::computeTrackSizesForIndefiniteSize(
     GridTrackSizingDirection direction,
@@ skipping 1235 matching lines @@
 
   std::unique_ptr<OrderedTrackIndexSet> emptyTrackIndexes;
   size_t insertionPoint = isRowAxis
                               ? styleRef().gridAutoRepeatColumnsInsertionPoint()
                               : styleRef().gridAutoRepeatRowsInsertionPoint();
   size_t firstAutoRepeatTrack =
       insertionPoint + std::abs(grid.smallestTrackStart(direction));
   size_t lastAutoRepeatTrack =
       firstAutoRepeatTrack + grid.autoRepeatTracks(direction);
 
-  if (!m_grid.hasGridItems()) {
+  if (!grid.hasGridItems()) {
     emptyTrackIndexes = WTF::wrapUnique(new OrderedTrackIndexSet);
     for (size_t trackIndex = firstAutoRepeatTrack;
          trackIndex < lastAutoRepeatTrack; ++trackIndex)
       emptyTrackIndexes->add(trackIndex);
   } else {
     for (size_t trackIndex = firstAutoRepeatTrack;
          trackIndex < lastAutoRepeatTrack; ++trackIndex) {
       GridIterator iterator(grid, direction, trackIndex);
       if (!iterator.nextGridItem()) {
         if (!emptyTrackIndexes)
           emptyTrackIndexes = WTF::wrapUnique(new OrderedTrackIndexSet);
         emptyTrackIndexes->add(trackIndex);
       }
     }
   }
   return emptyTrackIndexes;
 }
 
 void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid& grid,
-                                  SizingOperation sizingOperation) {
+                                  SizingOperation sizingOperation) const {
+  size_t autoRepeatRows =
+      computeAutoRepeatTracksCount(ForRows, sizingOperation);
+  size_t autoRepeatColumns =
+      computeAutoRepeatTracksCount(ForColumns, sizingOperation);
+  if (autoRepeatRows != grid.autoRepeatTracks(ForRows) ||
+      autoRepeatColumns != grid.autoRepeatTracks(ForColumns)) {
+    grid.setNeedsItemsPlacement(true);
+    grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);

[jfernandez, 2016/12/20 15:57:55]

Since we are going to get the autoRepeatRows/Columns anyway, why not just call
the the setter method and let it decide whether to sets also the flag for
needsItemsPlacement ?

[svillar, 2016/12/20 16:21:43]

Because I don't want to add any placement behavior to the Grid class. The Grid
class is just a grid representation and the methods to manipulate it. The items'
placement algorithm should be part of LayoutGrid as it is general enough to work
against any kind of grid representation.

+  }
+
   if (!grid.needsItemsPlacement())
     return;
 
   DCHECK(!grid.hasGridItems());
-
-  size_t autoRepeatColumns;
-  size_t autoRepeatRows =
-      computeAutoRepeatTracksCount(ForRows, sizingOperation);
-  if (sizingOperation == IntrinsicSizeComputation) {
-    autoRepeatColumns = styleRef().gridAutoRepeatColumns().size();
-  } else {
-    autoRepeatColumns =
-        computeAutoRepeatTracksCount(ForColumns, sizingOperation);
-  }
-  m_grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);
-
   populateExplicitGridAndOrderIterator(grid);
 
   Vector<LayoutBox*> autoMajorAxisAutoGridItems;
   Vector<LayoutBox*> specifiedMajorAxisAutoGridItems;
 #if ENABLE(ASSERT)
   DCHECK(!grid.hasAnyGridItemPaintOrder());
 #endif
   DCHECK(!grid.hasAnyOrthogonalGridItem());
   bool hasAnyOrthogonalGridItem = false;
   size_t childIndex = 0;
@@ skipping 1508 matching lines @@
   if (direction == ForRows)
     return grid.numTracks(ForRows);
 
   return grid.numTracks(ForRows)
              ? grid.numTracks(ForColumns)
              : GridPositionsResolver::explicitGridColumnCount(
                    styleRef(), grid.autoRepeatTracks(ForColumns));
 }
 
 }  // namespace blink