[css-grid] Computing free space on indefinite sized grids

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 674 matching lines @@
     GridSizingData& sizingData,
     LayoutUnit& minIntrinsicSize,
     LayoutUnit& maxIntrinsicSize) const {
   DCHECK(sizingData.isValidTransition(direction));
   sizingData.setAvailableSpace(LayoutUnit());
   sizingData.freeSpace(direction) = LayoutUnit();
   sizingData.sizingOperation = IntrinsicSizeComputation;
 
   computeUsedBreadthOfGridTracks(direction, sizingData, minIntrinsicSize,
                                  maxIntrinsicSize);
+  // Free Space is always 0 for indefinite sizing computation.
+  sizingData.freeSpace(direction) = LayoutUnit();

[svillar, 2016/10/17 08:20:33]

I disagree with this change for 2 reasons:

1) the statement is false, free space is not 0 for indefinite sizing
computations, it is indefinite. Also you're resetting it _after_ the indefinite
size computation.

2) I don't see the relationship with the other change. This code is executed
only during intrinsic size computation while the other change is on a method
executed during the actual layout so doing this should have 0 influence on the
other change.

[jfernandez, 2016/10/17 08:43:54]

Well, since we are initializing it to 0, we are getting a negative value after 
the indefinite size computation. This is confusing, as it's also the case when
grid containers available space is not enough for definite sized tracks; in this
case we assume the negative value as a potential overflow of the grid, which
will be handled by the <overflow-alignment> keyword.

About your  second point, it's not true that  computeTrackSizesForIndefiniteSize

is only called during intrinsic size computation; it's called as part of the
regular 
layoutBlock logic in case of  cachedHasDefiniteLogicalHeight() being false. It's
indeed confusing that we are setting the internal sizingOperation attribute 
to IntrinsicSizeComputation.

[svillar, 2016/10/17 09:13:19]

I understand but what you're doing here is exactly the same. By setting 0 you
make indistinguishable the cases of using all the available space and having an
indefinite size.

For me the actual fix would be to make both availableSize and freeSpace Optional
in SizingData and only update them if they are not null. That way we could
easily distinguish the cases you mention.
Correct for heights, I was thinking only about widths

 
   size_t numberOfTracks = direction == ForColumns
                               ? sizingData.columnTracks.size()
                               : sizingData.rowTracks.size();
   LayoutUnit totalGuttersSize =
       guttersSize(direction, 0, numberOfTracks, sizingData.sizingOperation);
   minIntrinsicSize += totalGuttersSize;
   maxIntrinsicSize += totalGuttersSize;
 
 #if ENABLE(ASSERT)
@@ skipping 2500 matching lines @@
   // If <content-distribution> value can't be applied, 'position' will become
   // the associated <content-position> fallback value.
   ContentAlignmentData contentAlignment = contentDistributionOffset(
       availableFreeSpace, position, distribution, numberOfGridTracks);
   if (contentAlignment.isValid())
     return contentAlignment;
 
   OverflowAlignment overflow =
       isRowAxis ? styleRef().justifyContentOverflowAlignment()
                 : styleRef().alignContentOverflowAlignment();
-  if (availableFreeSpace <= 0 && overflow == OverflowAlignmentSafe)
+  // TODO (lajava): Default value for overflow isn't exaclty as 'unsafe'.
+  // https://drafts.csswg.org/css-align/#overflow-values
+  if (availableFreeSpace == 0 ||
+      (availableFreeSpace < 0 && overflow == OverflowAlignmentSafe))
     return {LayoutUnit(), LayoutUnit()};
 
   switch (position) {
     case ContentPositionLeft:
       // The align-content's axis is always orthogonal to the inline-axis.
       return {LayoutUnit(), LayoutUnit()};
     case ContentPositionRight:
       if (isRowAxis)
         return {availableFreeSpace, LayoutUnit()};
       // The align-content's axis is always orthogonal to the inline-axis.
@@ skipping 82 matching lines @@
   if (!m_gridItemArea.isEmpty())
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 bool LayoutGrid::cachedHasDefiniteLogicalHeight() const {
   SECURITY_DCHECK(m_hasDefiniteLogicalHeight);
   return m_hasDefiniteLogicalHeight.value();
 }
 
 }  // namespace blink