[css-grid] Fix track sizing algo w/ size restrictions and intrinsic sizes

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 237 matching lines @@
 
 struct GridItemsSpanGroupRange {
     Vector<GridItemWithSpan>::iterator rangeStart;
     Vector<GridItemWithSpan>::iterator rangeEnd;
 };
 
 LayoutGrid::LayoutGrid(Element* element)
     : LayoutBlock(element)
     , m_gridIsDirty(true)
     , m_orderIterator(this)
+    , m_minContentHeight(-1)
+    , m_maxContentHeight(-1)
 {
     ASSERT(!childrenInline());
 }
 
 LayoutGrid::~LayoutGrid()
 {
 }
 
 void LayoutGrid::addChild(LayoutObject* newChild, LayoutObject* beforeChild)
 {
@@ skipping 33 matching lines @@
     return oldStyle.gridTemplateColumns().size() != styleRef().gridTemplateColumns().size()
         || oldStyle.gridTemplateRows().size() != styleRef().gridTemplateRows().size();
 }
 
 bool LayoutGrid::namedGridLinesDefinitionDidChange(const ComputedStyle& oldStyle) const
 {
     return oldStyle.namedGridRowLines() != styleRef().namedGridRowLines()
         || oldStyle.namedGridColumnLines() != styleRef().namedGridColumnLines();
 }
 
+void LayoutGrid::computeGridTrackSizes(GridSizingData& sizingData, LayoutUnit& freeSpaceForColumns, LayoutUnit& freeSpaceForRows)
+{
+    freeSpaceForColumns = contentLogicalWidth();
+    computeUsedBreadthOfGridTracks(ForColumns, sizingData, freeSpaceForColumns);
+    ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData.columnTracks));
+
+    freeSpaceForRows = computeContentAndScrollbarLogicalHeightUsing(MainOrPreferredSize, style()->logicalHeight(), -1);
+    computeUsedBreadthOfGridTracks(ForRows, sizingData, freeSpaceForRows);
+    ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData.rowTracks));
+}
+
+LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizingData) const
+{
+    LayoutUnit logicalHeight = borderAndPaddingLogicalHeight() + scrollbarLogicalHeight();
+
+    for (const auto& row : sizingData.rowTracks)
+        logicalHeight += row.baseSize();
+
+    return logicalHeight;
+}
+
+void LayoutGrid::updateLogicalHeightDependentGridTracks(const LayoutUnit& trackBasedLogicalHeight, GridSizingData& sizingData, LayoutUnit& availableSpaceForRows)

[jfernandez, 2015/10/19 10:46:22]

Which are those dependent tracks ? This function will recompute logical height
of all row tracks, even those with a fixed size. Is that what we want ? 

If we really want to update all row tracks, shouldn't we change the function
name accordingly ? 

[svillar, 2015/10/19 12:14:20]

We need to rerun the algorithm, and it'll indeed update all row tracks, you
cannot just update a subset of them (updating fixed size tracks is ultra-cheap
BTW). Also it does not happen always as you can see, actually only in cases
where you have indefinite sizes with min|max-content restrictions as far as I
have observed.

+{
+    LayoutUnit contentHeight = contentLogicalHeight();
+    if (trackBasedLogicalHeight == contentHeight)
+        return;
+
+    availableSpaceForRows = contentHeight;
+    computeUsedBreadthOfGridTracks(ForRows, sizingData, availableSpaceForRows);
+    ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData.rowTracks));
+}
+
 void LayoutGrid::layoutBlock(bool relayoutChildren)
 {
     ASSERT(needsLayout());
 
     if (!relayoutChildren && simplifiedLayout())
         return;
 
-    // FIXME: Much of this method is boiler plate that matches LayoutBox::layoutBlock and Layout*FlexibleBox::layoutBlock.
-    // It would be nice to refactor some of the duplicate code.
     {
         // LayoutState needs this deliberate scope to pop before updating scroll information (which
         // may trigger relayout).
         LayoutState state(*this, locationOffset());
 
         LayoutSize previousSize = size();
 
-        setLogicalHeight(0);
         updateLogicalWidth();
 
         TextAutosizer::LayoutScope textAutosizerLayoutScope(this);
 
-        layoutGridItems();
+        placeItemsOnGrid();
+
+        // TODO(svillar): move freeSpaceRows|Columns to GridSizingData.
+        LayoutUnit freeSpaceForRows;
+        LayoutUnit freeSpaceForColumns;
+        GridSizingData sizingData(gridColumnCount(), gridRowCount());
+        computeGridTrackSizes(sizingData, freeSpaceForColumns, freeSpaceForRows);
 
         LayoutUnit oldClientAfterEdge = clientLogicalBottom();
+        LayoutUnit trackBasedLogicalHeight = computeTrackBasedLogicalHeight(sizingData);
+        setLogicalHeight(trackBasedLogicalHeight);
+
         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).
+        updateLogicalHeightDependentGridTracks(trackBasedLogicalHeight, sizingData, freeSpaceForRows);
+
+        applyStretchAlignmentToTracksIfNeeded(ForColumns, sizingData, freeSpaceForColumns);
+        applyStretchAlignmentToTracksIfNeeded(ForRows, sizingData, freeSpaceForRows);
+
+        layoutGridItems(sizingData, freeSpaceForColumns, freeSpaceForRows);
+
         if (size() != previousSize)
             relayoutChildren = true;
 
         layoutPositionedObjects(relayoutChildren || isDocumentElement());
 
         computeOverflow(oldClientAfterEdge);
     }
 
     updateLayerTransformAfterLayout();
     updateScrollInfoAfterLayout();
@@ skipping 15 matching lines @@
 
         minLogicalWidth += minTrackBreadth;
         maxLogicalWidth += maxTrackBreadth;
     }
 
     LayoutUnit scrollbarWidth = intrinsicScrollbarLogicalWidth();
     minLogicalWidth += scrollbarWidth;
     maxLogicalWidth += scrollbarWidth;
 }
 
+LayoutUnit LayoutGrid::computeIntrinsicLogicalContentHeightUsing(const Length& logicalHeightLength, LayoutUnit intrinsicContentHeight, LayoutUnit borderAndPadding) const
+{
+    if (logicalHeightLength.isMinContent())
+        return m_minContentHeight;
+
+    if (logicalHeightLength.isMaxContent())

[jfernandez, 2015/10/19 10:46:22]

Current LayoutBox: logic doesn't implement the new min/max-content logic and it
has a FIXME stating the following:

"// FIXME(cbiesinger): The css-sizing spec is considering changing what
min-content/max-content should resolve to.
// If that happens, this code will have to change."

Are we sure we want to do this for Grid ? 

If we want to add this logic to grid, shouldn't we consider move/refactor some
code to LayoutBox ?

[svillar, 2015/10/19 12:14:20]

Yes it does implement the min/max-content, it's just they behave the same for
normal blocks.
No because the behavior, as I said is totally different for grid. For "normal"
blocks min-content=max-content=fit-content for heights.

That's the reason why I need to add a custom behavior for grids.

+        return m_maxContentHeight;
+
+    if (logicalHeightLength.isFitContent()) {
+        LayoutUnit fillAvailableExtent = containingBlock()->availableLogicalHeight(ExcludeMarginBorderPadding);
+        return std::min<LayoutUnit>(m_maxContentHeight, std::max<LayoutUnit>(m_minContentHeight, fillAvailableExtent));
+    }
+
+    if (logicalHeightLength.isFillAvailable())
+        return containingBlock()->availableLogicalHeight(ExcludeMarginBorderPadding) - borderAndPadding;
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+LayoutUnit LayoutGrid::computeContentAndScrollbarLogicalHeightUsing(SizeType heightType, const Length& height, LayoutUnit intrinsicContentHeight) const
+{
+    // The grid container’s auto block size is its max-content size.
+    if (height.isAuto())
+        return heightType == MinSize ? LayoutUnit(0) : m_maxContentHeight + scrollbarLogicalHeight();
+
+    if (height.isIntrinsic())
+        return computeIntrinsicLogicalContentHeightUsing(height, intrinsicContentHeight, borderAndPaddingLogicalHeight()) + scrollbarLogicalHeight();
+
+    return LayoutBox::computeContentAndScrollbarLogicalHeightUsing(heightType, height, intrinsicContentHeight);
+}
+
 bool LayoutGrid::gridElementIsShrinkToFit()
 {
     return isFloatingOrOutOfFlowPositioned();
 }
 
 static inline double normalizedFlexFraction(const GridTrack& track, double flexFactor)
 {
     return track.baseSize() / std::max<double>(1, flexFactor);
 }
 
@@ skipping 18 matching lines @@
         if (trackSize.isContentSized())
             sizingData.contentSizedTracksIndex.append(i);
         if (trackSize.maxTrackBreadth().isFlex())
             flexibleSizedTracksIndex.append(i);
     }
 
     // 2. Resolve content-based TrackSizingFunctions.
     if (!sizingData.contentSizedTracksIndex.isEmpty())
         resolveContentBasedTrackSizingFunctions(direction, sizingData);
 
+    LayoutUnit totalBaseSizes = 0;
+    LayoutUnit totalGrowthLimits = 0;
+
     for (const auto& track: tracks) {
         ASSERT(!track.infiniteGrowthPotential());
-        freeSpace -= track.baseSize();
+        totalBaseSizes += track.baseSize();
+        totalGrowthLimits += track.growthLimit();
+    }
+    freeSpace = initialFreeSpace - totalBaseSizes;
+
+    // Cache our intrinisc heights so that we could resolve them properly in computeIntrinsicLogicalContentHeightUsing().
+    if (direction == ForRows) {
+        m_minContentHeight = totalBaseSizes;
+        m_maxContentHeight = totalGrowthLimits;
     }
 
-    const bool hasUndefinedRemainingSpace = (direction == ForRows) ? style()->logicalHeight().isAuto() : gridElementIsShrinkToFit();
+    // Grid container should have the minimum height of a line if it's editable. That does not affect track sizing though.
+    if (hasLineIfEmpty()) {

[Manuel Rego, 2015/10/09 14:38:35]

Do we need this if "direction == ForColumns"?

Otherwise, we could move it inside the previous if.

[svillar, 2015/10/19 12:14:20]

Will do that.

+        m_minContentHeight = std::max(m_minContentHeight, minimumLogicalHeightForEmptyLine());
+        m_maxContentHeight = std::max(m_maxContentHeight, m_minContentHeight);
+    }
 
+    const bool hasUndefinedRemainingSpace = (direction == ForRows) ? initialFreeSpace == -1 : false;
     if (!hasUndefinedRemainingSpace && freeSpace <= 0)
         return;
 
     // 3. Grow all Grid tracks in GridTracks from their baseSize up to their growthLimit value until freeSpace is exhausted.
     const size_t tracksSize = tracks.size();
     if (!hasUndefinedRemainingSpace) {
         Vector<GridTrack*> tracksForDistribution(tracksSize);
         for (size_t i = 0; i < tracksSize; ++i) {
             tracksForDistribution[i] = tracks.data() + i;
             tracksForDistribution[i]->setPlannedSize(tracksForDistribution[i]->baseSize());
@@ skipping 30 matching lines @@
                     continue;
 
                 flexFraction = std::max(flexFraction, findFlexFactorUnitSize(tracks, span, direction, maxContentForChild(*gridItem, direction, sizingData.columnTracks)));
             }
         }
     }
 
     for (const auto& trackIndex : flexibleSizedTracksIndex) {
         GridTrackSize trackSize = gridTrackSize(direction, trackIndex);
 
-        LayoutUnit baseSize = std::max<LayoutUnit>(tracks[trackIndex].baseSize(), flexFraction * trackSize.maxTrackBreadth().flex());
-        tracks[trackIndex].setBaseSize(baseSize);
-        freeSpace -= baseSize;
+        LayoutUnit oldBaseSize = tracks[trackIndex].baseSize();
+        LayoutUnit baseSize = std::max<LayoutUnit>(oldBaseSize, flexFraction * trackSize.maxTrackBreadth().flex());
+        if (LayoutUnit increment = baseSize - oldBaseSize) {
+            tracks[trackIndex].setBaseSize(baseSize);
+            freeSpace -= increment;

[Manuel Rego, 2015/10/09 14:38:35]

We're changing this regarding the previous code
that was subtracting "baseSize".

"baseSize" was already subtracted from the "freeSpace"
so probably this version is right.
However it seems not directly related with the rest of the patch.

[jfernandez, 2015/10/19 10:46:22]

Wow, that changes seems like a bug fix, but why we haven't detected it so far ?

[svillar, 2015/10/19 12:14:20]

Because we need very specific tests for that, as the remaining freeSpace will
only be used by the alignment logic, it won't be used by the track sizing
algorithm anymore.

+
+            if (direction == ForRows)
+                m_maxContentHeight += increment;
+        }
     }
 
     // FIXME: Should ASSERT flexible tracks exhaust the freeSpace ? (see issue 739613002).
 }
 
 LayoutUnit LayoutGrid::computeUsedBreadthOfMinLength(GridTrackSizingDirection direction, const GridLength& gridLength) const
 {
     if (gridLength.isFlex())
         return 0;
 
@@ skipping 799 matching lines @@
         return;
 
     LayoutUnit sizeToIncrease = availableSpace / numberOfAutoSizedTracks;
     for (const auto& trackIndex : autoSizedTracksIndex) {
         GridTrack* track = tracks.data() + trackIndex;
         LayoutUnit baseSize = track->baseSize() + sizeToIncrease;
         track->setBaseSize(baseSize);
     }
 }
 
-void LayoutGrid::layoutGridItems()
+void LayoutGrid::layoutGridItems(GridSizingData& sizingData, LayoutUnit& freeSpaceForColumns, LayoutUnit& freeSpaceForRows)
 {
-    placeItemsOnGrid();
-
-    LayoutUnit availableSpaceForColumns = availableLogicalWidth();
-    LayoutUnit availableSpaceForRows = availableLogicalHeight(IncludeMarginBorderPadding);
-    GridSizingData sizingData(gridColumnCount(), gridRowCount());
-    computeUsedBreadthOfGridTracks(ForColumns, sizingData, availableSpaceForColumns);
-    ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData.columnTracks));
-    computeUsedBreadthOfGridTracks(ForRows, sizingData, availableSpaceForRows);
-    ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData.rowTracks));
-
-    applyStretchAlignmentToTracksIfNeeded(ForColumns, sizingData, availableSpaceForColumns);
-    applyStretchAlignmentToTracksIfNeeded(ForRows, sizingData, availableSpaceForRows);
-
-    populateGridPositions(sizingData, availableSpaceForColumns, availableSpaceForRows);
+    populateGridPositions(sizingData, freeSpaceForColumns, freeSpaceForRows);
     m_gridItemsOverflowingGridArea.resize(0);
 
     for (LayoutBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
         if (child->isOutOfFlowPositioned()) {
             prepareChildForPositionedLayout(*child);
             continue;
         }
 
         // Because the grid area cannot be styled, we don't need to adjust
         // the grid breadth to account for 'box-sizing'.
@@ skipping 23 matching lines @@
         ASSERT(coordinate.rows.resolvedInitialPosition.toInt() < sizingData.rowTracks.size());
 #endif
         child->setLogicalLocation(findChildLogicalPosition(*child, sizingData));
 
         // Keep track of children overflowing their grid area as we might need to paint them even if the grid-area is
         // not visible
         if (child->logicalHeight() > overrideContainingBlockContentLogicalHeight
             || child->logicalWidth() > overrideContainingBlockContentLogicalWidth)
             m_gridItemsOverflowingGridArea.append(child);
     }
-
-    for (const auto& row : sizingData.rowTracks)
-        setLogicalHeight(logicalHeight() + row.baseSize());
-
-    LayoutUnit height = logicalHeight() + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight();
-    if (hasLineIfEmpty())
-        height = std::max(height, minimumLogicalHeightForEmptyLine());
-
-    // Min / max logical height is handled by the call to updateLogicalHeight in layoutBlock.
-    setLogicalHeight(height);
 }
 
 void LayoutGrid::prepareChildForPositionedLayout(LayoutBox& child)
 {
     ASSERT(child.isOutOfFlowPositioned());
     child.containingBlock()->insertPositionedObject(&child);
 
     DeprecatedPaintLayer* childLayer = child.layer();
     childLayer->setStaticInlinePosition(borderAndPaddingStart());
     childLayer->setStaticBlockPosition(borderAndPaddingBefore());
@@ skipping 535 matching lines @@
 
     return LayoutPoint(rowAxisOffset, columnAxisOffsetForChild(child));
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink