Move the grid shorthand into CSSPropertyParser

third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSBasicShapeValues.h"
 #include "core/css/CSSBorderImage.h"
 #include "core/css/CSSContentDistributionValue.h"
 #include "core/css/CSSCounterValue.h"
 #include "core/css/CSSCrossfadeValue.h"
 #include "core/css/CSSCursorImageValue.h"
 #include "core/css/CSSCustomIdentValue.h"
 #include "core/css/CSSFontFaceSrcValue.h"
 #include "core/css/CSSFontFeatureValue.h"
 #include "core/css/CSSFunctionValue.h"
 #include "core/css/CSSGradientValue.h"
 #include "core/css/CSSGridAutoRepeatValue.h"
 #include "core/css/CSSGridLineNamesValue.h"
+#include "core/css/CSSGridTemplateAreasValue.h"
 #include "core/css/CSSImageSetValue.h"
 #include "core/css/CSSPaintValue.h"
 #include "core/css/CSSPathValue.h"
 #include "core/css/CSSPrimitiveValueMappings.h"
 #include "core/css/CSSQuadValue.h"
 #include "core/css/CSSReflectValue.h"
 #include "core/css/CSSSVGDocumentValue.h"
 #include "core/css/CSSShadowValue.h"
 #include "core/css/CSSStringValue.h"
 #include "core/css/CSSTimingFunctionValue.h"
@@ skipping 21 matching lines @@
     const CSSParserContext& context, HeapVector<CSSProperty, 256>* parsedProperties)
     : m_range(range)
     , m_context(context)
     , m_parsedProperties(parsedProperties)
     , m_inParseShorthand(0)
     , m_currentShorthand(CSSPropertyInvalid)
 {
     m_range.consumeWhitespace();
 }
 
+void CSSPropertyParser::addProperty(CSSPropertyID propId, CSSValue* value, bool important, bool implicit)

[Timothy Loh, 2016/04/13 06:09:41]

since you're moving this, can you rename propId to property

[rwlbuis, 2016/04/13 22:16:29]

Done.

+{
+    ASSERT(!isPropertyAlias(propId));
+
+    int shorthandIndex = 0;
+    bool setFromShorthand = false;
+
+    if (m_currentShorthand) {
+        Vector<StylePropertyShorthand, 4> shorthands;
+        getMatchingShorthandsForLonghand(propId, &shorthands);
+        setFromShorthand = true;
+        if (shorthands.size() > 1)
+            shorthandIndex = indexOfShorthandForLonghand(m_currentShorthand, shorthands);
+    }
+
+    m_parsedProperties->append(CSSProperty(propId, value, important, setFromShorthand, shorthandIndex, implicit));
+}
+
+void CSSPropertyParser::addExpandedPropertyForValue(CSSPropertyID propId, CSSValue* value, bool important)

[Timothy Loh, 2016/04/13 06:09:42]

propId -> property

[rwlbuis, 2016/04/13 22:16:29]

Done.

+{
+    const StylePropertyShorthand& shorthand = shorthandForProperty(propId);
+    unsigned shorthandLength = shorthand.length();
+    if (!shorthandLength) {
+        addProperty(propId, value, important);
+        return;
+    }
+
+    ShorthandScope scope(this, propId);
+    const CSSPropertyID* longhands = shorthand.properties();
+    for (unsigned i = 0; i < shorthandLength; ++i)
+        addProperty(longhands[i], value, important);
+}
+
 static bool hasInvalidNumericValues(const CSSParserTokenRange& range)
 {
     for (const CSSParserToken& token : range) {
         CSSParserTokenType type = token.type();
         if ((type == NumberToken || type == DimensionToken || type == PercentageToken)
             && !CSSPropertyParser::isValidNumericValue(token.numericValue()))
             return true;
     }
     return false;
 }
@@ skipping 2844 matching lines @@
             vertical = consumeLengthOrPercent(range, mode, ValueRangeAll, UnitlessQuirk::Forbid);
     } else if (unresolvedProperty == CSSPropertyAliasWebkitBackgroundSize) {
         // Legacy syntax: "-webkit-background-size: 10px" is equivalent to "background-size: 10px 10px".
         vertical = horizontal;
     }
     if (!vertical)
         return horizontal;
     return CSSValuePair::create(horizontal, vertical, CSSValuePair::KeepIdenticalValues);
 }
 
-CSSValueList* consumeGridAutoFlow(CSSParserTokenRange& range)
+static CSSValueList* consumeGridAutoFlow(CSSParserTokenRange& range)
 {
     CSSPrimitiveValue* rowOrColumnValue = consumeIdent<CSSValueRow, CSSValueColumn>(range);
     CSSPrimitiveValue* denseAlgorithm = consumeIdent<CSSValueDense>(range);
     if (!rowOrColumnValue) {
         rowOrColumnValue = consumeIdent<CSSValueRow, CSSValueColumn>(range);
         if (!rowOrColumnValue && !denseAlgorithm)
             return nullptr;
     }
     CSSValueList* parsedValues = CSSValueList::createSpaceSeparated();
     if (rowOrColumnValue)
@@ skipping 158 matching lines @@
     if (spanValue)
         values->append(spanValue);
     if (numericValue)
         values->append(numericValue);
     if (gridLineName)
         values->append(gridLineName);
     ASSERT(values->length());
     return values;
 }
 
+static bool allTracksAreFixedSized(CSSValueList& valueList)
+{
+    for (auto value : valueList) {

[Timothy Loh, 2016/04/13 06:09:42]

does it work to write

for (CSSValue* value : valueList)

?

[rwlbuis, 2016/04/13 22:16:29]

Done.

+        if (value->isGridLineNamesValue())
+            continue;
+        // The auto-repeat value holds a <fixed-size> = <fixed-breadth> | minmax( <fixed-breadth>, <track-breadth> )
+        if (value->isGridAutoRepeatValue()) {
+            if (!allTracksAreFixedSized(toCSSValueList(*value)))
+                return false;
+            continue;
+        }
+        ASSERT(value->isPrimitiveValue() || (value->isFunctionValue() && toCSSFunctionValue(*value).item(0)));

[Timothy Loh, 2016/04/13 06:09:42]

I guess this assertion is unnecessary since the conversions below will check the
same

[rwlbuis, 2016/04/13 22:16:29]

It is not 100% covered just by that code. In theory the value could be something
else than a primitive of function value. In reality, the way
consumeGridTrackList works it can't be something else. I don't feel too strongly
about this, can remove it if you want, but it is not completely useless just
because of the ternary below.

[Timothy Loh, 2016/04/14 02:04:12]

Not sure I follow. Ternary below says:

non-null primitive value (case 1)
OR
function value, first value is non-null primitive value (case 2)

If it's neither a primitive nor function value, then toCSSFunctionValue will hit
an assertion. So unless I'm misunderstanding, I think it'd be better to not have
the assertion because it isn't useful.

+        const CSSPrimitiveValue& primitiveValue = value->isPrimitiveValue()
+            ? toCSSPrimitiveValue(*value)
+            : toCSSPrimitiveValue(*toCSSFunctionValue(*value).item(0));
+        CSSValueID valueID = primitiveValue.getValueID();
+        if (valueID == CSSValueMinContent || valueID == CSSValueMaxContent || valueID == CSSValueAuto || primitiveValue.isFlex())
+            return false;
+    }
+    return true;
+}
+
+static Vector<String> parseGridTemplateAreasColumnNames(const String& gridRowNames)
+{
+    ASSERT(!gridRowNames.isEmpty());
+    Vector<String> columnNames;
+    // Using StringImpl to avoid checks and indirection in every call to String::operator[].
+    StringImpl& text = *gridRowNames.impl();
+
+    StringBuilder areaName;
+    for (unsigned i = 0; i < text.length(); ++i) {
+        if (text[i] == ' ') {

[Timothy Loh, 2016/04/13 06:09:42]

Needs a TODO that this whitespace check is missing \n and \t

https://drafts.csswg.org/css-grid/#valdef-grid-template-areas-string
https://drafts.csswg.org/css-syntax-3/#whitespace

[rwlbuis, 2016/04/13 22:16:29]

Done.

+            if (!areaName.isEmpty()) {
+                columnNames.append(areaName.toString());
+                areaName.clear();
+            }
+            continue;
+        }
+        if (text[i] == '.') {
+            if (areaName == ".")
+                continue;
+            if (!areaName.isEmpty()) {
+                columnNames.append(areaName.toString());
+                areaName.clear();
+            }
+        } else {

[Timothy Loh, 2016/04/13 06:09:42]

Needs a TODO to handle trash tokens (should only allow name code points here)

[rwlbuis, 2016/04/13 22:16:28]

Done.

+            if (areaName == ".") {
+                columnNames.append(areaName.toString());
+                areaName.clear();
+            }
+        }
+
+        areaName.append(text[i]);
+    }
+
+    if (!areaName.isEmpty())
+        columnNames.append(areaName.toString());
+
+    return columnNames;
+}
+
+static bool parseGridTemplateAreasRow(const String& gridRowNames, NamedGridAreaMap& gridAreaMap, const size_t rowCount, size_t& columnCount)
+{
+    if (gridRowNames.isEmpty() || gridRowNames.containsOnlyWhitespace())

[Timothy Loh, 2016/04/13 06:09:42]

I feel like I had this discussion before... but isEmpty is redundant here
because it vacuously contains only whitespace.

Anyway I think this is a strange way to handle this error condition. I think
it'd be easiest if parseGridTemplateAreasColumnNames just returned an empty
vector as its failure case (when we support rejecting trash characters in the
string), and if we do that then there's no point in explicitly checking for
whitespace-only up-front (and it doesn't really make sense as an optimisation
because authors shouldn't be writing these invalid declarations in the first
place). But let's make that a separate change.

[rwlbuis, 2016/04/13 22:16:29]

Acknowledged.

+        return false;
+
+    Vector<String> columnNames = parseGridTemplateAreasColumnNames(gridRowNames);
+    if (!columnCount) {

[Timothy Loh, 2016/04/13 06:09:42]

IMO more obvious if you write if (rowCount == 0)

[rwlbuis, 2016/04/13 22:16:29]

You meant columnCount == 0, right? Done.

[Timothy Loh, 2016/04/14 02:04:12]

Nope, I meant rowCount. I think it makes more sense to check (rowCount == 0),
which is "this is the first row", rather than (columnCount == 0), which is "we
don't know how many columns yet, which we signal by setting it to 0".

+        columnCount = columnNames.size();
+        ASSERT(columnCount);
+    } else if (columnCount != columnNames.size()) {
+        // The declaration is invalid is all the rows don't have the number of columns.

[Timothy Loh, 2016/04/13 06:09:41]

invalid is -> invalid if

[rwlbuis, 2016/04/13 22:16:29]

Done.

+        return false;
+    }
+
+    for (size_t currentCol = 0; currentCol < columnCount; ++currentCol) {

[Timothy Loh, 2016/04/13 06:09:41]

currentColumn

[rwlbuis, 2016/04/13 22:16:29]

Done.

+        const String& gridAreaName = columnNames[currentCol];
+
+        // Unamed areas are always valid (we consider them to be 1x1).
+        if (gridAreaName == ".")
+            continue;
+
+        // We handle several grid areas with the same name at once to simplify the validation code.

[Timothy Loh, 2016/04/13 06:09:42]

I'm not really sure what this comment is trying to say. Maybe better to remove
it. Grid cells with the same name form a single grid area.

[rwlbuis, 2016/04/13 22:16:29]

Done.

+        size_t lookAheadCol;

[Timothy Loh, 2016/04/13 06:09:42]

imo more readable as

size_t lookAheadColumn = currentColumn + 1;
while (lookAheadColumn < columnCount && columnNames[lookAheadColumn] !=
gridAreaName)
    lookAheadColumn++;

[rwlbuis, 2016/04/13 22:16:29]

Done.

+        for (lookAheadCol = currentCol + 1; lookAheadCol < columnCount; ++lookAheadCol) {
+            if (columnNames[lookAheadCol] != gridAreaName)
+                break;
+        }
+
+        NamedGridAreaMap::iterator gridAreaIt = gridAreaMap.find(gridAreaName);
+        if (gridAreaIt == gridAreaMap.end()) {
+            gridAreaMap.add(gridAreaName, GridArea(GridSpan::translatedDefiniteGridSpan(rowCount, rowCount + 1), GridSpan::translatedDefiniteGridSpan(currentCol, lookAheadCol)));
+        } else {
+            GridArea& gridArea = gridAreaIt->value;
+
+            // The following checks test that the grid area is a single filled-in rectangle.
+            // 1. The new row is adjacent to the previously parsed row.
+            if (rowCount != gridArea.rows.endLine())
+                return false;
+
+            // 2. The new area starts at the same position as the previously parsed area.
+            if (currentCol != gridArea.columns.startLine())
+                return false;
+
+            // 3. The new area ends at the same position as the previously parsed area.
+            if (lookAheadCol != gridArea.columns.endLine())
+                return false;
+
+            gridArea.rows = GridSpan::translatedDefiniteGridSpan(gridArea.rows.startLine(), gridArea.rows.endLine() + 1);
+        }
+        currentCol = lookAheadCol - 1;
+    }
+
+    return true;
+}
+
+enum TrackSizeRestriction { FixedSizeOnly, AllowAll };
+
 static CSSPrimitiveValue* consumeGridBreadth(CSSParserTokenRange& range, CSSParserMode cssParserMode, TrackSizeRestriction restriction = AllowAll)
 {
     if (restriction == AllowAll) {
         const CSSParserToken& token = range.peek();
         if (identMatches<CSSValueMinContent, CSSValueMaxContent, CSSValueAuto>(token.id()))
             return consumeIdent(range);
         if (token.type() == DimensionToken && token.unitType() == CSSPrimitiveValue::UnitType::Fraction) {
             if (range.peek().numericValue() < 0)
                 return nullptr;
             return cssValuePool().createValue(range.consumeIncludingWhitespace().numericValue(), CSSPrimitiveValue::UnitType::Fraction);
         }
     }
     return consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative, UnitlessQuirk::Allow);
 }
 
 // TODO(rob.buis): This needs a bool parameter so we can disallow <auto-track-list> for the grid shorthand.
-CSSValue* consumeGridTrackSize(CSSParserTokenRange& range, CSSParserMode cssParserMode, TrackSizeRestriction restriction)
+static CSSValue* consumeGridTrackSize(CSSParserTokenRange& range, CSSParserMode cssParserMode, TrackSizeRestriction restriction = AllowAll)
 {
     const CSSParserToken& token = range.peek();
     if (restriction == AllowAll && identMatches<CSSValueAuto>(token.id()))
         return consumeIdent(range);
 
     if (token.functionId() == CSSValueMinmax) {
         CSSParserTokenRange rangeCopy = range;
         CSSParserTokenRange args = consumeFunction(rangeCopy);
         CSSPrimitiveValue* minTrackBreadth = consumeGridBreadth(args, cssParserMode, restriction);
         if (!minTrackBreadth || !consumeCommaIncludingWhitespace(args))
@@ skipping 1424 matching lines @@
         addProperty(CSSPropertyGridTemplateColumns, columnsValue, important);
         addProperty(CSSPropertyGridTemplateAreas, cssValuePool().createIdentifierValue(CSSValueNone), important);
         return true;
     }
 
     // 3- [ <line-names>? <string> <track-size>? <line-names>? ]+ [ / <track-list> ]?
     m_range = rangeCopy;
     return consumeGridTemplateRowsAndAreasAndColumns(important);
 }
 
+bool CSSPropertyParser::consumeGridShorthand(bool important)
+{
+    ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+    ASSERT(shorthandForProperty(CSSPropertyGrid).length() == 8);
+
+    CSSParserTokenRange rangeCopy = m_range;
+
+    // 1- <grid-template>
+    if (consumeGridTemplateShorthand(important)) {
+        // It can only be specified the explicit or the implicit grid properties in a single grid declaration.
+        // The sub-properties not specified are set to their initial value, as normal for shorthands.
+        addProperty(CSSPropertyGridAutoFlow, cssValuePool().createImplicitInitialValue(), important);
+        addProperty(CSSPropertyGridAutoColumns, cssValuePool().createImplicitInitialValue(), important);
+        addProperty(CSSPropertyGridAutoRows, cssValuePool().createImplicitInitialValue(), important);
+        addProperty(CSSPropertyGridColumnGap, cssValuePool().createImplicitInitialValue(), important);
+        addProperty(CSSPropertyGridRowGap, cssValuePool().createImplicitInitialValue(), important);
+        return true;
+    }
+
+    m_range = rangeCopy;
+
+    // 2- <grid-auto-flow> [ <grid-auto-rows> [ / <grid-auto-columns> ]? ]
+    CSSValueList* gridAutoFlow = consumeGridAutoFlow(m_range);
+    if (!gridAutoFlow)
+        return false;
+
+    CSSValue* autoColumnsValue = nullptr;
+    CSSValue* autoRowsValue = nullptr;
+
+    if (!m_range.atEnd()) {
+        autoRowsValue = consumeGridTrackSize(m_range, m_context.mode());
+        if (!autoRowsValue)
+            return false;
+        if (consumeSlashIncludingWhitespace(m_range)) {
+            autoColumnsValue = consumeGridTrackSize(m_range, m_context.mode());
+            if (!autoColumnsValue)
+                return false;
+        }
+        if (!m_range.atEnd())
+            return false;
+    } else {
+        // Other omitted values are set to their initial values.
+        autoColumnsValue = cssValuePool().createImplicitInitialValue();
+        autoRowsValue = cssValuePool().createImplicitInitialValue();
+    }
+
+    // if <grid-auto-columns> value is omitted, it is set to the value specified for grid-auto-rows.
+    if (!autoColumnsValue)
+        autoColumnsValue = autoRowsValue;
+
+    addProperty(CSSPropertyGridAutoFlow, gridAutoFlow, important);
+    addProperty(CSSPropertyGridAutoColumns, autoColumnsValue, important);
+    addProperty(CSSPropertyGridAutoRows, autoRowsValue, important);
+
+    // It can only be specified the explicit or the implicit grid properties in a single grid declaration.
+    // The sub-properties not specified are set to their initial value, as normal for shorthands.
+    addProperty(CSSPropertyGridTemplateColumns, cssValuePool().createImplicitInitialValue(), important);

[Timothy Loh, 2016/04/13 06:09:42]

I think it makes sense for the longhands to always get added in the same order,
so lets move the grid-template-* longhands before the grid-auto-* longhands to
match the above order and the CSSProperties.in order.

[rwlbuis, 2016/04/13 22:16:29]

Done.

+    addProperty(CSSPropertyGridTemplateRows, cssValuePool().createImplicitInitialValue(), important);
+    addProperty(CSSPropertyGridTemplateAreas, cssValuePool().createImplicitInitialValue(), important);
+    addProperty(CSSPropertyGridColumnGap, cssValuePool().createImplicitInitialValue(), important);
+    addProperty(CSSPropertyGridRowGap, cssValuePool().createImplicitInitialValue(), important);
+
+    return true;
+}
+
 bool CSSPropertyParser::parseShorthand(CSSPropertyID unresolvedProperty, bool important)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
 
     CSSPropertyID oldShorthand = m_currentShorthand;
     // TODO(rob.buis): Remove this when the legacy property parser is gone
     m_currentShorthand = property;
     switch (property) {
     case CSSPropertyWebkitMarginCollapse: {
         CSSValueID id = m_range.consumeIncludingWhitespace().id();
@@ skipping 164 matching lines @@
         addProperty(CSSPropertyGridColumnGap, columnGap, important);
         return true;
     }
     case CSSPropertyGridColumn:
     case CSSPropertyGridRow:
         return consumeGridItemPositionShorthand(property, important);
     case CSSPropertyGridArea:
         return consumeGridAreaShorthand(important);
     case CSSPropertyGridTemplate:
         return consumeGridTemplateShorthand(important);
+    case CSSPropertyGrid:
+        return consumeGridShorthand(important);
     default:
         m_currentShorthand = oldShorthand;
-        CSSParserValueList valueList(m_range);
-        if (!valueList.size())
-            return false;
-        m_valueList = &valueList;
-        return legacyParseShorthand(unresolvedProperty, important);
+        return false;
     }
 }
 
 } // namespace blink