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

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 aef12064b28b22e8fc7cb60aa7930439b557840b..ad2d435e6ed18da3b5827c4c95ab8faaf56402e2 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -418,7 +418,7 @@ void LayoutGrid::computeTrackSizesForDirection(GridTrackSizingDirection directio
     sizingData.nextState();
 }
 
-void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData, LayoutUnit availableSpaceForColumns, LayoutUnit availableSpaceForRows)
+void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData)
 {
     DCHECK(sizingData.sizingState > GridSizingData::RowSizingFirstIteration);
 
@@ -430,12 +430,43 @@ void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData, LayoutUn
     // 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);
+    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());
@@ -453,7 +484,6 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
         LayoutSize previousSize = size();
 
         updateLogicalWidth();
-        bool logicalHeightWasIndefinite = !hasDefiniteLogicalHeight();
 
         TextAutosizer::LayoutScope textAutosizerLayoutScope(this, &layoutScope);
 
@@ -470,30 +500,17 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
         // 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())
@@ -1565,12 +1582,13 @@ void LayoutGrid::placeItemsOnGrid(SizingOperation sizingOperation)
 
     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())