[css-grid] Update intrinsic size for the extra sizing alg iteration.

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 400 matching lines @@
     sizingData.setAvailableSpace(availableSpace);
     sizingData.freeSpace(direction) = availableSpace - guttersSize(direction, 0, direction == ForRows ? gridRowCount() : gridColumnCount());
     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)
+void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData)
 {
     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);
+    bool minContentContributionChanged = false;
+    for (auto* child : m_orthogonalChilds) {
+        if (updateOverrideContainingBlockContentSizeForChild(*child, ForRows, sizingData))
+            child->setNeedsLayout(LayoutInvalidationReason::GridChanged);
+        child->layoutIfNeeded();
+        if (!minContentContributionChanged) {

[svillar, 2016/09/13 13:44:18]

I don't get this. For example for the first orthogonal child you will never
check if the content contribution has changed or not. More tests needed?

[jfernandez, 2016/09/13 14:56:59]

Umm, why "first orthogonal child" ? We definitively do it for the first one, and
for the rest of orthogonal children until we find one. After that, we don't need
to continue checking that because it's enough that just one item has changed its
contribution to repeat the whole algorithm. 

We need to complete the loop, though, because we must laid out all the
orthogonal children. I admit that there is room for improvement here, since we
probably only need
to laid out certain items, but let's be defensive for now. 

+            LayoutUnit minContentInlineSize = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
+            minContentContributionChanged = child->logicalHeight() > minContentInlineSize;
+        }
+    }
+
+    if (minContentContributionChanged) {
+        setPreferredLogicalWidthsDirty();
+        updateLogicalWidth();
+        computeTrackSizesForDirection(ForColumns, sizingData, availableLogicalWidth());
+        resolveSizeOfGridRows(sizingData);
+    }

[svillar, 2016/09/13 13:44:18]

I have many doubts about the above block. It's like a very reduced version of
our layoutBlock() method. I am not sure this is correct. As mentioned in private
chats I think we should let the generic code call our layout() method again.

[jfernandez, 2016/09/13 14:56:59]

I understand your concerns. However, notice that we run all the algorithm's
pieces which are relevant for this logic. We just do a simplified layout of the 
items, ignoring the alignment and margin logic.

Perhaps the main issue is to trigger the intrinsic size computation in the
middle
of a layout. The alternative, as we discussed, is to mark the grid container as 
still needing to be laid out and also its intrinsic sizes as dirty. However,
this 
seems to contradict what SubtreeLayoutSCope is trying to ensure, since we
are running this layout logic inside a closed scope. 

We may need to change that, or perhaps there is a layout API I'm not 
aware of to schedule a new layout of the grid container as soon the
one ongoing one is completed. 

+}
+
+void LayoutGrid::resolveSizeOfGridRows(GridSizingData& sizingData)
+{
+    bool logicalHeightWasIndefinite = !hasDefiniteLogicalHeight();
+    if (logicalHeightWasIndefinite)
+        computeIntrinsicLogicalHeight(sizingData);
+    else
+        computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
+    setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight());
+
+    updateLogicalHeight();
+
+    // The above call might have changed the grid's logical height depending on min|max height restrictions.
+    // Update the sizes of the rows whose size depends on the logical height (also on definite|indefinite sizes).
+    LayoutUnit availableSpaceForRows = contentLogicalHeight();
+    if (logicalHeightWasIndefinite)
         computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows);
-    }
 }
 
 void LayoutGrid::layoutBlock(bool relayoutChildren)
 {
     ASSERT(needsLayout());
 
     if (!relayoutChildren && simplifiedLayout())
         return;
 
     SubtreeLayoutScope layoutScope(*this);
 
     {
         // LayoutState needs this deliberate scope to pop before updating scroll information (which
         // may trigger relayout).
         LayoutState state(*this, locationOffset());
 
         LayoutSize previousSize = size();
 
         updateLogicalWidth();
-        bool logicalHeightWasIndefinite = !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)
         if (m_autoRepeatColumns && m_autoRepeatColumns != computeAutoRepeatTracksCount(ForColumns, TrackSizing))
             dirtyGrid();
         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);
+        computeTrackSizesForDirection(ForColumns, sizingData, availableLogicalWidth());
 
         // 2- Next, the track sizing algorithm resolves the sizes of the grid rows, using the
         // grid column sizes calculated in the previous step.
-        if (logicalHeightWasIndefinite)
-            computeIntrinsicLogicalHeight(sizingData);
-        else
-            computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
-        setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight());
-
         LayoutUnit oldClientAfterEdge = clientLogicalBottom();
-        updateLogicalHeight();
-
-        // The above call might have changed the grid's logical height depending on min|max height restrictions.
-        // Update the sizes of the rows whose size depends on the logical height (also on definite|indefinite sizes).
-        LayoutUnit availableSpaceForRows = contentLogicalHeight();
-        if (logicalHeightWasIndefinite)
-            computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows);
+        resolveSizeOfGridRows(sizingData);
 
         // 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, availableSpaceForRows);
+        repeatTracksSizingIfNeeded(sizingData);
 
         // Grid container should have the minimum height of a line if it's editable. That doesn't affect track sizing though.
         if (hasLineIfEmpty())
             setLogicalHeight(std::max(logicalHeight(), minimumLogicalHeightForEmptyLine()));
 
         applyStretchAlignmentToTracksIfNeeded(ForColumns, sizingData);
         applyStretchAlignmentToTracksIfNeeded(ForRows, sizingData);
 
         layoutGridItems(sizingData);
 
@@ skipping 1051 matching lines @@
         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;
+    m_orthogonalChilds.shrink(0);
     for (LayoutBox* child = m_orderIterator.first(); child; child = m_orderIterator.next()) {
         if (child->isOutOfFlowPositioned())
             continue;
 
-        m_hasAnyOrthogonalChild = m_hasAnyOrthogonalChild || isOrthogonalChild(*child);
+        if (isOrthogonalChild(*child))
+            m_orthogonalChilds.append(child);
 
         GridArea area = cachedGridArea(*child);
         if (!area.rows.isIndefinite())
             area.rows.translate(abs(m_smallestRowStart));
         if (!area.columns.isIndefinite())
             area.columns.translate(abs(m_smallestColumnStart));
         m_gridItemArea.set(child, area);
 
         if (area.rows.isIndefinite() || area.columns.isIndefinite()) {
             GridSpan majorAxisPositions = (autoPlacementMajorAxisDirection() == ForColumns) ? area.columns : area.rows;
@@ skipping 1072 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