[css-grid] Refactor intrinsic size computation code

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 387 matching lines @@
     LayoutUnit logicalHeight;
 
     for (const auto& row : sizingData.rowTracks)
         logicalHeight += row.baseSize();
 
     logicalHeight += guttersSize(ForRows, 0, sizingData.rowTracks.size(), sizingData.sizingOperation);
 
     return logicalHeight;
 }
 
-void LayoutGrid::computeTrackSizesForDirection(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit availableSpace)
+void LayoutGrid::computeTrackSizesForDefiniteSize(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit availableSpace)

[jfernandez, 2016/09/15 21:47:28]

Any reason why this method is not declared 'const' ?

[svillar, 2016/09/16 12:23:31]

Good point, I think we can do it.

 {
     DCHECK(sizingData.isValidTransition(direction));
     sizingData.setAvailableSpace(availableSpace);
     sizingData.freeSpace(direction) = availableSpace - guttersSize(direction, 0, direction == ForRows ? gridRowCount() : gridColumnCount(), sizingData.sizingOperation);
     sizingData.sizingOperation = TrackSizing;
 
     LayoutUnit baseSizes, growthLimits;
     computeUsedBreadthOfGridTracks(direction, sizingData, baseSizes, growthLimits);
     ASSERT(tracksAreWiderThanMinTrackBreadth(direction, sizingData));
     sizingData.nextState();
 }
 
 void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData, LayoutUnit availableSpaceForColumns, LayoutUnit availableSpaceForRows)
 {
     DCHECK(sizingData.sizingState > GridSizingData::RowSizingFirstIteration);
 
     // In orthogonal flow cases column track's size is determined by using the computed
     // row track's size, which it was estimated during the first cycle of the sizing
     // algorithm. Hence we need to repeat computeUsedBreadthOfGridTracks for both,
     // columns and rows, to determine the final values.
     // TODO (lajava): orthogonal flows is just one of the cases which may require
     // a new cycle of the sizing algorithm; there may be more. In addition, not all the
     // cases with orthogonal flows require this extra cycle; we need a more specific
     // condition to detect whether child's min-content contribution has changed or not.
     if (m_hasAnyOrthogonalChild) {
-        computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns);
-        computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows);
+        computeTrackSizesForDefiniteSize(ForColumns, sizingData, availableSpaceForColumns);
+        computeTrackSizesForDefiniteSize(ForRows, sizingData, availableSpaceForRows);
     }
 }
 
 void LayoutGrid::layoutBlock(bool relayoutChildren)
 {
     ASSERT(needsLayout());
 
     if (!relayoutChildren && simplifiedLayout())
         return;
 
@@ skipping 18 matching lines @@
         placeItemsOnGrid(TrackSizing);
 
         GridSizingData sizingData(gridColumnCount(), gridRowCount());
 
         // 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 updateLogicalHeight() require a previous call to setLogicalHeight() to resolve
         // heights properly (like for positioned items for example).
         LayoutUnit availableSpaceForColumns = availableLogicalWidth();
-        computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns);
+        computeTrackSizesForDefiniteSize(ForColumns, sizingData, availableSpaceForColumns);
 
         // 2- Next, the track sizing algorithm resolves the sizes of the grid rows, using the
         // grid column sizes calculated in the previous step.
-        if (cachedHasDefiniteLogicalHeight())
-            computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
-        else
-            computeIntrinsicLogicalHeight(sizingData);
+        if (cachedHasDefiniteLogicalHeight()) {
+            computeTrackSizesForDefiniteSize(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
+        } else {
+            computeTrackSizesForIndefiniteSize(ForRows, sizingData, m_minContentHeight, m_maxContentHeight);
+            sizingData.nextState();
+            sizingData.sizingOperation = TrackSizing;
+        }
         setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight());
 
         LayoutUnit oldClientAfterEdge = clientLogicalBottom();
         updateLogicalHeight();
 
         // 3- If the min-content contribution of any grid items have changed based on the row
         // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content
         // contribution (once only).
         repeatTracksSizingIfNeeded(sizingData, availableSpaceForColumns, contentLogicalHeight());
 
@@ skipping 102 matching lines @@
     }
 
     return gapAccumulator;
 }
 
 void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
 {
     const_cast<LayoutGrid*>(this)->placeItemsOnGrid(IntrinsicSizeComputation);
 
     GridSizingData sizingData(gridColumnCount(), gridRowCount());
-    sizingData.setAvailableSpace(LayoutUnit());
-    sizingData.freeSpace(ForColumns) = LayoutUnit();
-    sizingData.sizingOperation = IntrinsicSizeComputation;
-    computeUsedBreadthOfGridTracks(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
-
-    LayoutUnit totalGuttersSize = guttersSize(ForColumns, 0, sizingData.columnTracks.size(), sizingData.sizingOperation);
-    minLogicalWidth += totalGuttersSize;
-    maxLogicalWidth += totalGuttersSize;
+    computeTrackSizesForIndefiniteSize(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
 
     LayoutUnit scrollbarWidth = LayoutUnit(scrollbarLogicalWidth());
     minLogicalWidth += scrollbarWidth;
     maxLogicalWidth += scrollbarWidth;
 }
 
-void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
+void LayoutGrid::computeTrackSizesForIndefiniteSize(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit& minIntrinsicSize, LayoutUnit& maxIntrinsicSize) const
 {
-    DCHECK(sizingData.isValidTransition(ForRows));
-    ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData));
+    DCHECK(sizingData.isValidTransition(direction));
     sizingData.setAvailableSpace(LayoutUnit());
     sizingData.freeSpace(ForRows) = LayoutUnit();

[jfernandez, 2016/09/15 21:47:28]

Shouldn't we use 'direction' here, instead of 'ForRows' ? 

[svillar, 2016/09/16 12:23:31]

Uhu, right

     sizingData.sizingOperation = IntrinsicSizeComputation;
-    computeUsedBreadthOfGridTracks(ForRows, sizingData, m_minContentHeight, m_maxContentHeight);
 
-    LayoutUnit totalGuttersSize = guttersSize(ForRows, 0, gridRowCount(), sizingData.sizingOperation);
-    m_minContentHeight += totalGuttersSize;
-    m_maxContentHeight += totalGuttersSize;
+    computeUsedBreadthOfGridTracks(direction, sizingData, minIntrinsicSize, maxIntrinsicSize);
 
-    ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData));
-    sizingData.nextState();
-    sizingData.sizingOperation = TrackSizing;
+    size_t numberOfTracks = direction == ForColumns ? sizingData.columnTracks.size() : sizingData.rowTracks.size();
+    LayoutUnit totalGuttersSize = guttersSize(direction, 0, numberOfTracks, sizingData.sizingOperation);
+    minIntrinsicSize += totalGuttersSize;
+    maxIntrinsicSize += totalGuttersSize;
+
+    DCHECK(tracksAreWiderThanMinTrackBreadth(direction, sizingData));
 }
 
 LayoutUnit LayoutGrid::computeIntrinsicLogicalContentHeightUsing(const Length& logicalHeightLength, LayoutUnit intrinsicContentHeight, LayoutUnit borderAndPadding) const
 {
     if (logicalHeightLength.isMinContent())
         return m_minContentHeight;
 
     if (logicalHeightLength.isMaxContent())
         return m_maxContentHeight;
 
@@ skipping 753 matching lines @@
             track->growSizeDuringDistribution(growthShare);
             availableLogicalSpace -= growthShare;
         }
     }
 
     for (auto* track : tracks)
         track->setPlannedSize(track->plannedSize() == infinity ? track->sizeDuringDistribution() : std::max(track->plannedSize(), track->sizeDuringDistribution()));
 }
 
 #if ENABLE(ASSERT)
-bool LayoutGrid::tracksAreWiderThanMinTrackBreadth(GridTrackSizingDirection direction, GridSizingData& sizingData)
+bool LayoutGrid::tracksAreWiderThanMinTrackBreadth(GridTrackSizingDirection direction, GridSizingData& sizingData) const

[jfernandez, 2016/09/15 21:47:28]

sizingData should be const here.

[svillar, 2016/09/16 12:23:31]

It should but we cannot do it right now because freeSpace() is not const
(returns a reference). Perhaps in some other change.

 {
     const Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
     LayoutUnit& maxSize = sizingData.freeSpace(direction);
     for (size_t i = 0; i < tracks.size(); ++i) {
         GridTrackSize trackSize = gridTrackSize(direction, i, sizingData.sizingOperation);
         if (computeUsedBreadthOfMinLength(trackSize, maxSize) > tracks[i].baseSize())
             return false;
     }
     return true;
 }
@@ skipping 1252 matching lines @@
         GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 bool LayoutGrid::cachedHasDefiniteLogicalHeight() const
 {
     SECURITY_DCHECK(m_hasDefiniteLogicalHeight);
     return m_hasDefiniteLogicalHeight.value();
 }
 
 } // namespace blink