[css-grid] Do not recursively call layout during auto repeat computation

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 423 matching lines @@
 
         LayoutSize previousSize = size();
 
         updateLogicalWidth();
         bool logicalHeightWasIndefinite = computeContentLogicalHeight(MainOrPreferredSize, style()->logicalHeight(), LayoutUnit(-1)) == LayoutUnit(-1);
 
         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 autoRepeatColumnsCount = computeAutoRepeatTracksCount(ForColumns);
-        if (m_autoRepeatColumns && m_autoRepeatColumns != autoRepeatColumnsCount)
+        if (m_autoRepeatColumns && m_autoRepeatColumns != computeAutoRepeatTracksCount(ForColumns, TrackSizing))
             dirtyGrid();
-        placeItemsOnGrid(autoRepeatColumnsCount);
+        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);
@@ skipping 110 matching lines @@
         }
         if (nonEmptyTracksAfterEndLine)
             gapAccumulator += gap;
     }
 
     return gapAccumulator;
 }
 
 void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
 {
-    const_cast<LayoutGrid*>(this)->placeItemsOnGrid(styleRef().gridAutoRepeatColumns().size());
+    const_cast<LayoutGrid*>(this)->placeItemsOnGrid(IntrinsicSizeComputation);
 
     GridSizingData sizingData(gridColumnCount(), gridRowCount());
     sizingData.freeSpaceForDirection(ForColumns) = LayoutUnit();
     sizingData.sizingOperation = IntrinsicSizeComputation;
     computeUsedBreadthOfGridTracks(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
 
     LayoutUnit totalGuttersSize = guttersSize(ForColumns, 0, sizingData.columnTracks.size());
     minLogicalWidth += totalGuttersSize;
     maxLogicalWidth += totalGuttersSize;
 
@@ skipping 788 matching lines @@
 {
     RELEASE_ASSERT(area.rows.isTranslatedDefinite() && area.columns.isTranslatedDefinite());
     ensureGridSize(area.rows.endLine(), area.columns.endLine());
 
     for (const auto& row : area.rows) {
         for (const auto& column: area.columns)
             m_grid[row][column].append(&child);
     }
 }
 
-size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direction) const
+size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direction, SizingOperation sizingOperation) const
 {
     bool isRowAxis = direction == ForColumns;
     const auto& autoRepeatTracks = isRowAxis ? styleRef().gridAutoRepeatColumns() : styleRef().gridAutoRepeatRows();
     size_t autoRepeatTrackListLength = autoRepeatTracks.size();
 
     if (!autoRepeatTrackListLength)
         return 0;
 
-    LayoutUnit availableSize = isRowAxis ? availableLogicalWidth() : computeContentLogicalHeight(MainOrPreferredSize, styleRef().logicalHeight(), LayoutUnit(-1));
-    if (availableSize == -1) {
-        const Length& maxLength = isRowAxis ? styleRef().logicalMaxWidth() : styleRef().logicalMaxHeight();
-        if (!maxLength.isMaxSizeNone()) {
-            availableSize = isRowAxis
-                ? computeLogicalWidthUsing(MaxSize, maxLength, containingBlockLogicalWidthForContent(), containingBlock())

[jfernandez, 2016/08/22 14:44:48]

Where is this case defined in the new logic ? Is it something we don't need to
do now ? why ¿

[svillar, 2016/08/22 16:01:52]

You can see it defined for rows in the else block of "if(isRowAxis)". For
columns we don't need it as I mentioned in the new comment I'm adding, widths
are never indefinite during layout, they can only be indefinite when computing
intrinsic sizes.

[jfernandez, 2016/08/22 16:20:20]

got it now, thanks.

-                : computeContentLogicalHeight(MaxSize, maxLength, LayoutUnit(-1));
+    LayoutUnit availableSize(-1);

[Manuel Rego, 2016/08/23 07:43:03]

Mmm, this means that for columns during intrinsic size computation
we always consider an indefinite width. Even when the width
might have a fixed size.

I guess this is fine, as later during the track sizing layout
we're using the right value.

I'm wondering if we really need to call computeAutoRepeatTracksCount()
during intrinsic size phase or we can simply consider that we've 1 repetition
and that the track sizing phase will calculate the right value.
I guess this might apply to both columns and rows, not only columns.
What do you think?

[svillar, 2016/08/25 09:33:10]

There is no intrinsic size computation with fixed widths. The preferred widths
computation is only performed for intrinsic widths.
 
One repetition is one possibility but not the only one, as you can see bellow in
the method sometimes you're forced to fulfill a minimum size.
placeItemsOnGrid() is only called from layout() or width intrinsic size
computation. Grid is indeed symmetrical but the rest of the engine is not.

[Manuel Rego, 2016/08/25 09:59:35]

Thanks for the explanations I knew I was missing some points. :-)

+    // Widths are always definite except during intrinsic size computation. For heights we need to
+    // check whether the computed logical height is -1 or not to determine it.
+    if (sizingOperation != IntrinsicSizeComputation || !isRowAxis) {
+        if (isRowAxis) {

[Manuel Rego, 2016/08/23 07:43:03]

Just in case I was wrong in my previous comment,
I think this could be simpler if we use something like:
  if (isRowAxis) {
    if sizingOperation != IntrinsicSizeComputation)
      availableSize = availableLogicalWidth();
  } else {
    ...
  }

[svillar, 2016/08/25 09:33:10]

Acknowledged.

+            DCHECK_NE(sizingOperation, IntrinsicSizeComputation);

[jfernandez, 2016/08/22 14:44:48]

Wouldn't be clearer if we define the DCHECK like isRowAxis) && sizingOperation
!= IntrinsicSizeComputation outside the if ?

[svillar, 2016/08/22 16:01:52]

I really don't get what you mean. Could you rephrase?

That DCHECK basically checks that if we have entered the outer if and we're
handling columns it could only be the case that we're in the layout phase and
never in intrinsic size computation.

[jfernandez, 2016/08/22 16:20:20]

I was mistaken, forget about this suggestion. 

+            availableSize = availableLogicalWidth();
+        } else {
+            availableSize = computeContentLogicalHeight(MainOrPreferredSize, styleRef().logicalHeight(), LayoutUnit(-1));
+            if (availableSize == -1) {
+                const Length& maxLength = styleRef().logicalMaxHeight();
+                if (!maxLength.isMaxSizeNone())
+                    availableSize = computeContentLogicalHeight(MaxSize, maxLength, LayoutUnit(-1));
+            } else {
+                availableSize = constrainLogicalHeightByMinMax(availableSize, LayoutUnit(-1));
+            }
         }
-    } else {
-        availableSize = isRowAxis
-            ? constrainLogicalWidthByMinMax(availableSize, availableLogicalWidth(), containingBlock())
-            : constrainLogicalHeightByMinMax(availableSize, LayoutUnit(-1));
     }
 
     bool needsToFulfillMinimumSize = false;
     bool indefiniteMainAndMaxSizes = availableSize == LayoutUnit(-1);
     if (indefiniteMainAndMaxSizes) {
         const Length& minSize = isRowAxis ? styleRef().logicalMinWidth() : styleRef().logicalMinHeight();
         if (!minSize.isSpecified())
             return autoRepeatTrackListLength;
 
         LayoutUnit containingBlockAvailableSize = isRowAxis ? containingBlockLogicalWidthForContent() : containingBlockLogicalHeightForContent(ExcludeMarginBorderPadding);
@@ skipping 66 matching lines @@
             if (!iterator.nextGridItem()) {
                 if (!emptyTrackIndexes)
                     emptyTrackIndexes = wrapUnique(new OrderedTrackIndexSet);
                 emptyTrackIndexes->add(trackIndex);
             }
         }
     }
     return emptyTrackIndexes;
 }
 
-void LayoutGrid::placeItemsOnGrid(size_t autoRepeatColumnsCount)
+void LayoutGrid::placeItemsOnGrid(SizingOperation sizingOperation)
 {
     if (!m_gridIsDirty)
         return;
 
-    ASSERT(m_gridItemArea.isEmpty());
+    DCHECK(m_gridItemArea.isEmpty());
+    DCHECK(m_gridItemsIndexesMap.isEmpty());
 
-    m_autoRepeatColumns = autoRepeatColumnsCount;
-    m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows);
+    if (sizingOperation == IntrinsicSizeComputation)
+        m_autoRepeatColumns = styleRef().gridAutoRepeatColumns().size();
+    else
+        m_autoRepeatColumns = computeAutoRepeatTracksCount(ForColumns, sizingOperation);
+    m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows, sizingOperation);
 
     populateExplicitGridAndOrderIterator();
 
     // We clear the dirty bit here as the grid sizes have been updated.
     m_gridIsDirty = false;
 
     Vector<LayoutBox*> autoMajorAxisAutoGridItems;
     Vector<LayoutBox*> specifiedMajorAxisAutoGridItems;
     m_hasAnyOrthogonalChild = false;
     for (LayoutBox* child = m_orderIterator.first(); child; child = m_orderIterator.next()) {
@@ skipping 1094 matching lines @@
     return isOrthogonalChild(child) ? childLocation.transposedPoint() : childLocation;
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const
 {
     if (!m_gridItemArea.isEmpty())
         GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink