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

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 a9d80d7ebbd8775def2668b9adf41c2c2b336d45..24a85f06f9a6ba3421ee12933d25d8d92f15a99c 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -692,6 +692,8 @@ void LayoutGrid::computeTrackSizesForIndefiniteSize(
 
   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()
@@ -3212,7 +3214,10 @@ ContentAlignmentData LayoutGrid::computeContentPositionAndDistributionOffset(
   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) {