[css-grid] Isolate intrinsic size computation from layout

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 ac08e774401aa3279283f0381f9c7d4476ba68f9..eef54fb123031d0c59c34df4a23d138abee928e3 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -640,14 +640,6 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) {
 
     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),
@@ -808,12 +800,12 @@ LayoutUnit LayoutGrid::guttersSize(const Grid& grid,
 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, grid),
+                            numTracks(ForRows, grid), grid);
 
-  GridSizingData sizingData(numTracks(ForColumns, m_grid),
-                            numTracks(ForRows, m_grid),
-                            const_cast<Grid&>(m_grid));
   computeTrackSizesForIndefiniteSize(ForColumns, sizingData, minLogicalWidth,
                                      maxLogicalWidth);
 
@@ -2069,7 +2061,7 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
   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)
@@ -2089,23 +2081,21 @@ LayoutGrid::computeEmptyTracksForAutoRepeat(
 }
 
 void LayoutGrid::placeItemsOnGrid(LayoutGrid::Grid& grid,
-                                  SizingOperation sizingOperation) {
-  if (!grid.needsItemsPlacement())
-    return;
-
-  DCHECK(!grid.hasGridItems());
-
-  size_t autoRepeatColumns;
+                                  SizingOperation sizingOperation) const {
   size_t autoRepeatRows =
       computeAutoRepeatTracksCount(ForRows, sizingOperation);
-  if (sizingOperation == IntrinsicSizeComputation) {
-    autoRepeatColumns = styleRef().gridAutoRepeatColumns().size();
-  } else {
-    autoRepeatColumns =
-        computeAutoRepeatTracksCount(ForColumns, 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.

   }
-  m_grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);
 
+  if (!grid.needsItemsPlacement())
+    return;
+
+  DCHECK(!grid.hasGridItems());
   populateExplicitGridAndOrderIterator(grid);
 
   Vector<LayoutBox*> autoMajorAxisAutoGridItems;