[CSS Grid Layout] Fix auto-placement algorithm

Source/core/rendering/RenderGrid.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 914 matching lines @@
             // This will happen when we have "span <custom-indent>", which has a different resolution based on the position.
             GridSpan majorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMajorAxisDirection(), majorAxisIndex);
             emptyGridArea = findEmptyGridAreaAtSpecifiedPositionsInsideGrid(gridItem, autoPlacementMajorAxisDirection(), majorAxisPositions);
 
             if (emptyGridArea) {
                 // Check that it fits in the minor axis direction, as we shouldn't grow in that direction here (it was already managed in populateExplicitGridAndOrderIterator()).
                 GridResolvedPosition minorAxisFinalPositionIndex = autoPlacementMinorAxisDirection() == ForColumns ? emptyGridArea->columns.resolvedFinalPosition : emptyGridArea->rows.resolvedFinalPosition;
                 const size_t endOfMinorAxis = autoPlacementMinorAxisDirection() == ForColumns ? gridColumnCount() : gridRowCount();
                 if (minorAxisFinalPositionIndex.toInt() < endOfMinorAxis)
                     break;
+
+                // Discard empty grid area as it does not fit in the minor axis direction.
+                emptyGridArea = nullptr;

[svillar, 2014/06/04 09:48:22]

Wouldn't it be possible to call here
createEmptyGridAreaAtSpecifiedPositionsOutsideGrid() directly instead of
nullifying the variable?

[Manuel Rego, 2014/06/04 10:02:51]

We're looping over the major axis, as you can read in the previous comment:
            // We need to resolve the position for every different index as the
number of cells might vary depending on it.
            // This will happen when we have "span <custom-indent>", which has a
different resolution based on the position.

So we don't have to call createEmptyGridAreaAtSpecifiedPositionsOutsideGrid() if
the empty grid area doesn't fit. As maybe, we found an empty grid area that fits
in the next row/column or whatever.

We just need to call createEmptyGridAreaAtSpecifiedPositionsOutsideGrid() later
if we haven't been able to find any empty grid area.

[Julien - ping for review, 2014/06/10 00:08:05]

It would be good to add such a test case, that would ensure that we don't
regress it by mistake later.

I think this explanation would complement the comment above too.

[Manuel Rego, 2014/06/10 09:49:02]

That was what I was doing in the modification of the original test and now with
the new test case.
I've modified the comment to add some more context.

             }
         }
 
         if (!emptyGridArea) {
             GridSpan minorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMinorAxisDirection(), GridResolvedPosition(0));
             emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(gridItem, autoPlacementMinorAxisDirection(), minorAxisPositions);
         }
     }
 
     insertItemIntoGrid(gridItem, *emptyGridArea);
@@ skipping 314 matching lines @@
     if (isOutOfFlowPositioned())
         return "RenderGrid (positioned)";
     if (isAnonymous())
         return "RenderGrid (generated)";
     if (isRelPositioned())
         return "RenderGrid (relative positioned)";
     return "RenderGrid";
 }
 
 } // namespace WebCore