Modified: trunk/Source/WebCore/ChangeLog (201381 => 201382)
--- trunk/Source/WebCore/ChangeLog 2016-05-25 13:18:26 UTC (rev 201381)
+++ trunk/Source/WebCore/ChangeLog 2016-05-25 14:44:48 UTC (rev 201382)
@@ -1,3 +1,42 @@
+2016-05-25 Manuel Rego Casasnovas <r...@igalia.com>
+
+ [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.
+
2016-05-25 Sergio Villar Senin <svil...@igalia.com>
[css-grid] Refactor populateGridPositions()
Modified: trunk/Source/WebCore/css/CSSParser.cpp (201381 => 201382)
--- trunk/Source/WebCore/css/CSSParser.cpp 2016-05-25 13:18:26 UTC (rev 201381)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2016-05-25 14:44:48 UTC (rev 201382)
@@ -5817,25 +5817,17 @@
return true;
}
-static bool allTracksAreFixedSized(CSSValueList& valueList)
+static bool isGridTrackFixedSized(const CSSValue& value)
{
- 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;
- }
+ 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() || primitiveValue.isPercentage() || primitiveValue.isCalculated());
return true;
}
@@ -5857,24 +5849,30 @@
bool seenTrackSizeOrRepeatFunction = false;
bool seenAutoRepeat = false;
+ bool allTracksAreFixedSized = true;
while (CSSParserValue* currentValue = m_valueList->current()) {
if (isForwardSlashOperator(*currentValue))
break;
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)
return nullptr;
seenAutoRepeat = seenAutoRepeat || isAutoRepeat;
} else {
- RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList, seenAutoRepeat ? FixedSizeOnly : AllowAll);
+ RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
if (!value)
return nullptr;
+
+ 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.
value = m_valueList->current();
if (value && value->unit == CSSParserValue::ValueList)
@@ -5884,15 +5882,10 @@
if (!seenTrackSizeOrRepeatFunction)
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());
@@ -5922,15 +5915,15 @@
parseGridLineNames(*arguments, repeatedValues);
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;
+ allTracksAreFixedSized = allTracksAreFixedSized && isGridTrackFixedSized(*trackSize);
repeatedValues->append(trackSize.releaseNonNull());
++numberOfTracks;
@@ -5961,7 +5954,7 @@
return true;
}
-RefPtr<CSSValue> CSSParser::parseGridTrackSize(CSSParserValueList& inputList, TrackSizeRestriction restriction)
+RefPtr<CSSValue> CSSParser::parseGridTrackSize(CSSParserValueList& inputList)
{
ASSERT(isCSSGridLayoutEnabled());
@@ -5977,9 +5970,8 @@
if (!arguments || arguments->size() != 3 || !isComma(arguments->valueAt(1)))
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;
RefPtr<CSSPrimitiveValue> maxTrackBreadth = parseGridBreadth(*arguments->valueAt(2));
@@ -5992,23 +5984,17 @@
return CSSFunctionValue::create("minmax(", WTFMove(parsedArguments));
}
- return parseGridBreadth(currentValue, restriction);
+ return parseGridBreadth(currentValue);
}
-RefPtr<CSSPrimitiveValue> CSSParser::parseGridBreadth(CSSParserValue& value, TrackSizeRestriction restriction)
+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;
// Fractional unit is a non-negative dimension.
Modified: trunk/Source/WebCore/css/CSSParser.h (201381 => 201382)
--- trunk/Source/WebCore/css/CSSParser.h 2016-05-25 13:18:26 UTC (rev 201381)
+++ trunk/Source/WebCore/css/CSSParser.h 2016-05-25 14:44:48 UTC (rev 201382)
@@ -239,10 +239,9 @@
bool parseGridGapShorthand(bool important);
bool parseSingleGridAreaLonghand(RefPtr<CSSValue>&);
RefPtr<CSSValue> parseGridTrackList();
- bool parseGridTrackRepeatFunction(CSSValueList&, bool& isAutoRepeat);
- enum TrackSizeRestriction { FixedSizeOnly, InflexibleSizeOnly, AllowAll };
- RefPtr<CSSValue> parseGridTrackSize(CSSParserValueList& inputList, TrackSizeRestriction = AllowAll);
- RefPtr<CSSPrimitiveValue> parseGridBreadth(CSSParserValue&, TrackSizeRestriction = AllowAll);
+ bool parseGridTrackRepeatFunction(CSSValueList&, bool& isAutoRepeat, bool& allTracksAreFixedSized);
+ RefPtr<CSSValue> parseGridTrackSize(CSSParserValueList& inputList);
+ RefPtr<CSSPrimitiveValue> parseGridBreadth(CSSParserValue&);
bool parseGridTemplateAreasRow(NamedGridAreaMap&, const unsigned, unsigned&);
RefPtr<CSSValue> parseGridTemplateAreas();
bool parseGridLineNames(CSSParserValueList&, CSSValueList&, CSSGridLineNamesValue* = nullptr);
