[css-grid] Implement repeat(auto-fit)

third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp

Index: third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
index c0377df923235f63f5ab729e2474e687f4d5e76d..331107af7ce1790412e6d6400675a9543ad0f771 100644
--- a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
+++ b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
@@ -743,6 +743,9 @@ public:
         , m_insertionPoint(isRowAxis ? style.gridAutoRepeatColumnsInsertionPoint() : style.gridAutoRepeatRowsInsertionPoint())
         , m_repetitions(repetitions)
     {
+        auto autoRepeatTrackSizes = isRowAxis ? style.gridAutoRepeatColumns() : style.gridAutoRepeatRows();
+        auto autoRepeatType = isRowAxis ? style.gridAutoRepeatColumnsType() : style.gridAutoRepeatRowsType();
+        m_hasRepeatAutoFit = autoRepeatTrackSizes.size() && autoRepeatType == AutoFit;
     }
 
     bool isEmpty() const { return m_orderedNamedGridLines.isEmpty() && m_orderedNamedAutoRepeatGridLines.isEmpty(); }
@@ -757,6 +760,7 @@ private:
     const OrderedNamedGridLines& m_orderedNamedAutoRepeatGridLines;
     size_t m_insertionPoint;
     size_t m_repetitions;
+    bool m_hasRepeatAutoFit;
 };
 
 void OrderedNamedLinesCollector::appendLines(CSSGridLineNamesValue& lineNamesValue, size_t index, NamedLinesType type) const
@@ -773,8 +777,20 @@ void OrderedNamedLinesCollector::appendLines(CSSGridLineNamesValue& lineNamesVal
 void OrderedNamedLinesCollector::collectLineNamesForIndex(CSSGridLineNamesValue& lineNamesValue, size_t i) const
 {
     DCHECK(!isEmpty());
-    if (m_orderedNamedAutoRepeatGridLines.isEmpty() || i < m_insertionPoint) {
-        appendLines(lineNamesValue, i, NamedLines);
+    if (m_orderedNamedAutoRepeatGridLines.isEmpty() || i < m_insertionPoint || !m_repetitions) {
+        if (!m_hasRepeatAutoFit || m_repetitions || i < m_insertionPoint) {

[Manuel Rego, 2016/06/21 21:57:37]

Do we really need to check if m_hasRepeatAutoFit?
I think the only important thing to know here is if m_repetitions
is 0 or not, maybe I'm missing something.

[svillar, 2016/06/23 08:09:42]

Yes we do.

If m_repetitions is 0 then there are 2 possibilities:
1) there is no auto repeat with auto-fit
2) there is an auto-fit which dropped all the tracks.

You need a way to differentiate between these two cases.

+            appendLines(lineNamesValue, i, NamedLines);
+            return;
+        }
+
+        // If all the repetitions were dropped we should merge the line names surrounding the repeat()
+        // function. We should also take the removed tracks into account for the following lines.

[Manuel Rego, 2016/06/21 21:57:37]

I guess we could add an assert here to check that m_repetitions is 0.

[svillar, 2016/06/23 08:09:42]

Do we? There is an if (m_repetitions) with an early return just above so I guess
is pretty obvious from the code.

+        if (i == m_insertionPoint) {
+            appendLines(lineNamesValue, i, NamedLines);
+            appendLines(lineNamesValue, i + 1, NamedLines);
+        } else {
+            appendLines(lineNamesValue, i + 1, NamedLines);

[Manuel Rego, 2016/06/21 21:57:37]

I don't understand this else part.

I guess that here i > m_insertionPoint
(otherwise it should have gone through the previous ifs).
I don't get why we want the "i + 1" line.

[svillar, 2016/06/23 08:09:42]

So we're in the case of having dropped all the repetitions. For something like
this

[a] repeat(auto-fit, 10px) [b] 20px [c]

the way we store line names is more or less a hash map like this:
(0, a)
(1, b)
(2, c)

if we drop all the auto repetitions we should end up with something like this

[a b] 20px [c]

as you can see when we ask for lines at index 1 (remember that they start on 0)
we should not return  'b' (line 1) but 'c' (line 1+1). 

They key is that when we store the line names we assume that there is always 1
repetition for autorepeat (which is true for auto-fill but might be false for
auto-fit), that's why we need to readjust the code here.

+        }
         return;
     }
 
@@ -818,8 +834,9 @@ static CSSValue* valueForGridTrackList(GridTrackSizingDirection direction, const
     const Vector<GridTrackSize>& autoRepeatTrackSizes = isRowAxis ? style.gridAutoRepeatColumns() : style.gridAutoRepeatRows();
     bool isLayoutGrid = layoutObject && layoutObject->isLayoutGrid();
 
-    // Handle the 'none' case.
-    bool trackListIsEmpty = trackSizes.isEmpty() && autoRepeatTrackSizes.isEmpty();
+    // auto-fit might eventually remove all the tracks, we should consider the grid empty in that case too.
+    size_t repetitions = isLayoutGrid ? toLayoutGrid(layoutObject)->autoRepeatCountForDirection(direction) : 0;
+    bool trackListIsEmpty = trackSizes.isEmpty() && (autoRepeatTrackSizes.isEmpty() || !repetitions);

[Manuel Rego, 2016/06/21 21:57:37]

I guess checking !repetitions is enough:
   bool trackListIsEmpty = trackSizes.isEmpty() && !repetitions;

If I'm not wrong if autoRepeatTrackSizes.isEmpty()
autoRepeatCountForDirection() will return 0.

[svillar, 2016/06/23 08:09:42]

Yes we can do that in this case.

     if (isLayoutGrid && trackListIsEmpty) {
         // For grids we should consider every listed track, whether implicitly or explicitly
         // created. Empty grids have a sole grid line per axis.
@@ -830,7 +847,6 @@ static CSSValue* valueForGridTrackList(GridTrackSizingDirection direction, const
     if (trackListIsEmpty)
         return CSSPrimitiveValue::createIdentifier(CSSValueNone);
 
-    size_t repetitions = isLayoutGrid ? toLayoutGrid(layoutObject)->autoRepeatCountForDirection(direction) : 0;
     OrderedNamedLinesCollector collector(style, isRowAxis, repetitions);
     CSSValueList* list = CSSValueList::createSpaceSeparated();
     size_t insertionIndex;