Changeset 201373 in webkit for trunk/Source/WebCore/css/CSSParser.cpp

Timestamp:

May 25, 2016, 2:22:37 AM (10 years ago)

Author:

Manuel Rego Casasnovas

Message:

[css-grid] Simplify grid track sizes parsing
​https://bugs.webkit.org/show_bug.cgi?id=158021

Reviewed by Sergio Villar Senin.

Previously once we saw an auto-repeat function,
we passed the "FixedSizeOnly" restriction to the rest of methods.
That way we were sure that all the tracks after the auto-repeat
had fixed sizes.
But we needed to call allTracksAreFixedSized() to be sure that
the tracks before the auto-repeat had fixed sizes too.

Now we're introducing a new boolean |allTracksAreFixedSized|,
to check in advance if the declaration contains any track not fixed.
If that's the case and we found an auto-repeat method,
we consider it invalid.
With this approach we avoid the loop to verify
that all the tracks (before and after the auto-repeat) are fixed.
It also allows us to simplify the code and avoid passing
the restriction to all the methods parsing the track size.

No new tests, no change of behavior.

• css/CSSParser.cpp:

(WebCore::isGridTrackFixedSized): New method to check if a grid track
size is fixed or not (based on old allTracksAreFixedSized()).
(WebCore::CSSParser::parseGridTrackList): Add new boolean to detect
if any track has not a fixed size.
(WebCore::CSSParser::parseGridTrackRepeatFunction): Ditto.
(WebCore::CSSParser::parseGridTrackSize): Remove usage of
TrackSizeRestriction enum.
Check here if |minTrackBreadth| is a flexible size.
(WebCore::CSSParser::parseGridBreadth): Remove usage of
TrackSizeRestriction enum.
(WebCore::allTracksAreFixedSized): Deleted.

• css/CSSParser.h: Remove TrackSizeRestriction enum and update headers.

File:

• trunk/Source/WebCore/css/CSSParser.cpp (modified) (9 diffs)

trunk/Source/WebCore/css/CSSParser.cpp

@@ -5818 +5818 @@
 }
 
-static bool allTracksAreFixedSized(CSSValueList& valueList)
-{
-    for (auto& value : valueList) {
-        if (is<CSSGridLineNamesValue>(value))
-            continue;
-        // The auto-repeat value holds a <fixed-size> = <fixed-breadth> | minmax( <fixed-breadth>, <track-breadth> )
-        if (is<CSSGridAutoRepeatValue>(value)) {
-            if (!allTracksAreFixedSized(downcast<CSSValueList>(value.get())))
-                return false;
-            continue;
-        }
-        ASSERT(value->isPrimitiveValue() || (value->isFunctionValue() && downcast<CSSFunctionValue>(value.get()).arguments()));
-        const CSSPrimitiveValue& primitiveValue = value->isPrimitiveValue()
-            ? downcast<CSSPrimitiveValue>(value.get())
-            : downcast<CSSPrimitiveValue>(*downcast<CSSFunctionValue>(value.get()).arguments()->item(0));
-        CSSValueID valueID = primitiveValue.getValueID();
-        if (valueID == CSSValueWebkitMinContent || valueID == CSSValueWebkitMaxContent || valueID == CSSValueAuto || primitiveValue.isFlex())
-            return false;
-    }
+static bool isGridTrackFixedSized(const CSSValue& value)
+{
+    ASSERT(value.isPrimitiveValue() || (value.isFunctionValue() && downcast<CSSFunctionValue>(value).arguments()));
+    const auto& primitiveValue = value.isPrimitiveValue()
+        ? downcast<CSSPrimitiveValue>(value)
+        : downcast<CSSPrimitiveValue>(*downcast<CSSFunctionValue>(value).arguments()->item(0));
+    CSSValueID valueID = primitiveValue.getValueID();
+    if (valueID == CSSValueWebkitMinContent || valueID == CSSValueWebkitMaxContent || valueID == CSSValueAuto || primitiveValue.isFlex())
+        return false;
+
+    ASSERT(primitiveValue.isLength());
     return true;
 }
@@ -5858 +5850 @@
     bool seenTrackSizeOrRepeatFunction = false;
     bool seenAutoRepeat = false;
+    bool allTracksAreFixedSized = true;
     while (CSSParserValue* currentValue = m_valueList->current()) {
         if (isForwardSlashOperator(*currentValue))
@@ -5863 +5856 @@
         if (currentValue->unit == CSSParserValue::Function && equalLettersIgnoringASCIICase(currentValue->function->name, "repeat(")) {
             bool isAutoRepeat;
-            if (!parseGridTrackRepeatFunction(values, isAutoRepeat))
+            if (!parseGridTrackRepeatFunction(values, isAutoRepeat, allTracksAreFixedSized))
                 return nullptr;
             if (isAutoRepeat && seenAutoRepeat)
@@ -5869 +5862 @@
             seenAutoRepeat = seenAutoRepeat || isAutoRepeat;
         } else {
-            RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList, seenAutoRepeat ? FixedSizeOnly : AllowAll);
+            RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
             if (!value)
                 return nullptr;
+            if (allTracksAreFixedSized)
+                allTracksAreFixedSized = isGridTrackFixedSized(*value);
             values->append(value.releaseNonNull());
         }
         seenTrackSizeOrRepeatFunction = true;
+
+        if (seenAutoRepeat && !allTracksAreFixedSized)
+            return nullptr;
 
         // This will handle the trailing <custom-ident>* in the grammar.
@@ -5885 +5883 @@
         return nullptr;
 
-    // <auto-repeat> requires definite minimum track sizes in order to compute the number of repetitions.
-    // The above while loop detects those appearances after the <auto-repeat> but not the ones before.
-    if (seenAutoRepeat && !allTracksAreFixedSized(values))
-        return nullptr;
-
     return WTFMove(values);
 }
 
-bool CSSParser::parseGridTrackRepeatFunction(CSSValueList& list, bool& isAutoRepeat)
+bool CSSParser::parseGridTrackRepeatFunction(CSSValueList& list, bool& isAutoRepeat, bool& allTracksAreFixedSized)
 {
     ASSERT(isCSSGridLayoutEnabled());
@@ -5923 +5916 @@
 
     unsigned numberOfTracks = 0;
-    TrackSizeRestriction restriction = isAutoRepeat ? FixedSizeOnly : AllowAll;
     while (arguments->current()) {
         if (isAutoRepeat && numberOfTracks)
             return false;
 
-        RefPtr<CSSValue> trackSize = parseGridTrackSize(*arguments, restriction);
+        RefPtr<CSSValue> trackSize = parseGridTrackSize(*arguments);
         if (!trackSize)
             return false;
+        if (allTracksAreFixedSized)
+            allTracksAreFixedSized = isGridTrackFixedSized(*trackSize);
 
         repeatedValues->append(trackSize.releaseNonNull());
@@ -5962 +5956 @@
 }
 
-RefPtr<CSSValue> CSSParser::parseGridTrackSize(CSSParserValueList& inputList, TrackSizeRestriction restriction)
+RefPtr<CSSValue> CSSParser::parseGridTrackSize(CSSParserValueList& inputList)
 {
     ASSERT(isCSSGridLayoutEnabled());
@@ -5978 +5972 @@
             return nullptr;
 
-        TrackSizeRestriction minTrackBreadthRestriction = restriction == AllowAll ? InflexibleSizeOnly : restriction;
-        RefPtr<CSSPrimitiveValue> minTrackBreadth = parseGridBreadth(*arguments->valueAt(0), minTrackBreadthRestriction);
-        if (!minTrackBreadth)
+        RefPtr<CSSPrimitiveValue> minTrackBreadth = parseGridBreadth(*arguments->valueAt(0));
+        if (!minTrackBreadth || minTrackBreadth->isFlex())
             return nullptr;
 
@@ -5993 +5986 @@
     }
 
-    return parseGridBreadth(currentValue, restriction);
-}
-
-RefPtr<CSSPrimitiveValue> CSSParser::parseGridBreadth(CSSParserValue& value, TrackSizeRestriction restriction)
+    return parseGridBreadth(currentValue);
+}
+
+RefPtr<CSSPrimitiveValue> CSSParser::parseGridBreadth(CSSParserValue& value)
 {
     ASSERT(isCSSGridLayoutEnabled());
 
-    if (value.id == CSSValueWebkitMinContent || value.id == CSSValueWebkitMaxContent || value.id == CSSValueAuto) {
-        if (restriction == FixedSizeOnly)
-            return nullptr;
+    if (value.id == CSSValueWebkitMinContent || value.id == CSSValueWebkitMaxContent || value.id == CSSValueAuto)
         return CSSValuePool::singleton().createIdentifierValue(value.id);
-    }
 
     if (value.unit == CSSPrimitiveValue::CSS_FR) {
-        if (restriction == FixedSizeOnly || restriction == InflexibleSizeOnly)
-            return nullptr;
-
         double flexValue = value.fValue;