Add support for <image> type parsing using CSSParserTokens

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

Index: third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
index 6e2b2211d22e2763058d7ae3233c3358c86d817f..a2ef560f4c6edbdc74aeb6f2cf6295a280a63a7f 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -7,6 +7,7 @@
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSCalculationValue.h"
+#include "core/css/CSSCrossfadeValue.h"
 #include "core/css/CSSCursorImageValue.h"
 #include "core/css/CSSCustomIdentValue.h"
 #include "core/css/CSSFontFaceSrcValue.h"
@@ -2405,6 +2406,227 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCursor(CSSParserTokenRange& range
     return list.release();
 }
 
+static bool consumeGradientColorStops(CSSParserTokenRange& range, CSSParserContext context, CSSGradientValue* gradient, bool expectComma)
+{
+    bool supportsColorHints = gradient->gradientType() == CSSLinearGradient || gradient->gradientType() == CSSRadialGradient;
+
+    // The first color stop cannot be a color hint.
+    bool previousStopWasColorHint = true;
+    while (!range.atEnd()) {

[Timothy Loh, 2015/12/09 06:59:57]

Can we just use a regular do-while loop here and move the initial comma
consumption to the callers? As an interface, it isn't immediately clear what the
expectComma boolean should do, so I think it should be removed.

[rwlbuis, 2015/12/09 21:31:13]

Done.

+        if (expectComma && !consumeCommaIncludingWhitespace(range))
+            return false;
+
+        CSSGradientColorStop stop;
+        stop.m_color = consumeColor(range, context);
+
+        // Two hints in a row are not allowed.
+        if (!stop.m_color && (!supportsColorHints || previousStopWasColorHint))
+            return false;
+        previousStopWasColorHint = !stop.m_color;
+        stop.m_position = consumeLengthOrPercent(range, context.mode(), ValueRangeAll);
+        if (!stop.m_color && !stop.m_position)
+            return false;
+
+        gradient->addStop(stop);
+        expectComma = true;
+    }
+
+    // The last color stop cannot be a color hint.
+    if (previousStopWasColorHint)
+        return false;
+
+    // Must have 2 or more stops to be valid.
+    return gradient->stopCount() >= 2;
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeRadialGradient(CSSParserTokenRange args, CSSParserContext context, CSSGradientRepeat repeating)
+{
+    RefPtrWillBeRawPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating, CSSRadialGradient);
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> shapeValue = nullptr;

[Timothy Loh, 2015/12/09 06:59:57]

imo shapeValue -> shape, sizeValue -> sizeKeyword

[rwlbuis, 2015/12/09 21:31:13]

Done.

+    RefPtrWillBeRawPtr<CSSPrimitiveValue> sizeValue = nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> horizontalSize = nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> verticalSize = nullptr;
+
+    // First part of grammar, the size/shape clause:
+    // [ circle || <length> ] |
+    // [ ellipse || [ <length> | <percentage> ]{2} ] |
+    // [ [ circle | ellipse] || <size-keyword> ]
+    for (int i = 0; i < 3; ++i) {
+        if (args.peek().type() == IdentToken) {
+            CSSValueID id = args.peek().id();
+            if (id == CSSValueCircle || id == CSSValueEllipse) {
+                if (shapeValue)
+                    return nullptr;
+                shapeValue = consumeIdent(args);
+            } else if (id == CSSValueClosestSide || id == CSSValueClosestCorner || id == CSSValueFarthestSide || id == CSSValueFarthestCorner) {
+                if (sizeValue || horizontalSize)

[Timothy Loh, 2015/12/09 06:59:57]

Probably nicer to leave out the horizontalSize check here. This way it's clearer
that this loop is only about filling in the cssvalues, and all the checks that
the correct things were set happens below.

[rwlbuis, 2015/12/09 21:31:14]

Done.

+                    return nullptr;
+                sizeValue = consumeIdent(args);
+            } else {
+                break;
+            }
+        } else {
+            RefPtrWillBeRawPtr<CSSPrimitiveValue> center = consumeLengthOrPercent(args, context.mode(), ValueRangeAll);
+            if (!center)
+                break;
+            if (sizeValue || horizontalSize)

[Timothy Loh, 2015/12/09 06:59:57]

and leave out the sizeValue check here

[rwlbuis, 2015/12/09 21:31:14]

Done.

+                return nullptr;
+            horizontalSize = center;
+
+            if ((center = consumeLengthOrPercent(args, context.mode(), ValueRangeAll))) {
+                verticalSize = center.release();
+                ++i;
+            }
+        }
+    }
+
+    // You can specify size as a keyword or a length/percentage, not both.
+    if (sizeValue && horizontalSize)
+        return nullptr;
+    // Circles must have 0 or 1 lengths.
+    if (shapeValue && shapeValue->getValueID() == CSSValueCircle && verticalSize)
+        return nullptr;
+    // Ellipses must have 0 or 2 length/percentages.
+    if (shapeValue && shapeValue->getValueID() == CSSValueEllipse && horizontalSize && !verticalSize)
+        return nullptr;
+    // If there's only one size, it must be a length.
+    if (!verticalSize && horizontalSize && horizontalSize->isPercentage())

[Timothy Loh, 2015/12/09 06:59:57]

// TODO(timloh): Calcs with both lengths and percentages should be rejected.

[rwlbuis, 2015/12/09 21:31:13]

Done.

+        return nullptr;
+
+    result->setShape(shapeValue);
+    result->setSizingBehavior(sizeValue);
+    result->setEndHorizontalSize(horizontalSize);
+    result->setEndVerticalSize(verticalSize);
+
+    RefPtrWillBeRawPtr<CSSValue> centerX = nullptr;
+    RefPtrWillBeRawPtr<CSSValue> centerY = nullptr;
+    if (args.peek().id() == CSSValueAt) {
+        args.consumeIncludingWhitespace();
+
+        consumePosition(args, context.mode(), UnitlessQuirk::Forbid, centerX, centerY);
+        if (!(centerX && centerY))
+            return nullptr;
+
+        result->setFirstX(centerX);
+        result->setFirstY(centerY);
+        // Right now, CSS radial gradients have the same start and end centers.
+        result->setSecondX(centerX);
+        result->setSecondY(centerY);
+    }
+
+    bool expectComma = shapeValue || sizeValue || horizontalSize || centerX || centerY;
+    if (!consumeGradientColorStops(args, context, result.get(), expectComma) || !args.atEnd())
+        return nullptr;
+
+    return result.release();
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeLinearGradient(CSSParserTokenRange args, CSSParserContext context, CSSGradientRepeat repeating, CSSGradientType gradientType)
+{
+    RefPtrWillBeRawPtr<CSSLinearGradientValue> result = CSSLinearGradientValue::create(repeating, gradientType);
+
+    bool expectComma = true;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> angle = consumeAngle(args, context.mode());
+    if (angle) {
+        result->setAngle(angle.release());
+    } else if (gradientType == CSSPrefixedLinearGradient || consumeIdent<CSSValueTo>(args)) {
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> endX = consumeIdent<CSSValueLeft, CSSValueRight>(args);
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> endY = consumeIdent<CSSValueBottom, CSSValueTop>(args);
+        if (!endX && !endY)
+            return nullptr;
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> location = consumeIdent<CSSValueLeft, CSSValueRight>(args);

[Timothy Loh, 2015/12/09 06:59:57]

Should be enough to just write

if (!endX)
    endX = consume...

I don't see how the branch below assigning to endY is ever going to be hit.

[rwlbuis, 2015/12/09 21:31:13]

I think endX and location have to be separate. Otherwise we can't return null if
endX was already set and location is not null.
Ouch, I now see what you mean, will have a look.

[rwlbuis, 2015/12/09 23:08:46]

Oh, I see why you mean here too now, should be fixed.

+        if (location) {
+            if (endX)
+                return nullptr;
+            endX = location;
+        } else if ((location = consumeIdent<CSSValueBottom, CSSValueTop>(args))) {
+            if (endY)
+                return nullptr;
+            endY = location;
+        }
+
+        if (!endX && !endY) {
+            endY = cssValuePool().createIdentifierValue(CSSValueTop);

[Timothy Loh, 2015/12/09 06:59:57]

Unreachable?

[rwlbuis, 2015/12/09 21:31:13]

Sorry, I merged a patch wrong, this is still needed for the deprecated
linear-gradient cases.

+            expectComma = false;
+        }
+
+        result->setFirstX(endX.release());
+        result->setFirstY(endY.release());
+    }
+
+    if (!consumeGradientColorStops(args, context, result.get(), expectComma) || !args.atEnd())
+        return nullptr;
+
+    return result.release();
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeImage(CSSParserTokenRange&, CSSParserContext);
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeCrossFade(CSSParserTokenRange args, CSSParserContext context)
+{
+    RefPtrWillBeRawPtr<CSSValue> fromImageValue = consumeImage(args, context);
+    if (!fromImageValue || !consumeCommaIncludingWhitespace(args))
+        return nullptr;
+    RefPtrWillBeRawPtr<CSSValue> toImageValue = consumeImage(args, context);
+    if (!toImageValue || !consumeCommaIncludingWhitespace(args))
+        return nullptr;
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> percentage = nullptr;
+    const CSSParserToken& percentageArg = args.consumeIncludingWhitespace();
+    if (percentageArg.type() == PercentageToken)
+        percentage = cssValuePool().createValue(clampTo<double>(percentageArg.numericValue() / 100, 0, 1), CSSPrimitiveValue::UnitType::Number);
+    else if (percentageArg.type() == NumberToken)
+        percentage = cssValuePool().createValue(clampTo<double>(percentageArg.numericValue(), 0, 1), CSSPrimitiveValue::UnitType::Number);
+
+    if (!percentage || !args.atEnd())
+        return nullptr;
+
+    return CSSCrossfadeValue::create(fromImageValue, toImageValue, percentage);
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeGeneratedImage(CSSParserTokenRange& range, CSSParserContext context)

[Timothy Loh, 2015/12/09 06:59:57]

As usual, we need to make sure this doesn't accidentally consume garbage
functions. Also it might be easier to do the end-of-function checks here? Maybe
we should do something like

CSSParserTokenRange rangeCopy = range;
CSSParserTokenRange args = consumeFunction(rangeCopy);
RefPtrWillBeRawPtr<CSSValue> result = nullptr;
if (id == ..)
    result == consume..();
else if (id == ..)
    result == consume..();
else
    return nullptr;

if (!result || !args.atEnd())
    return nullptr;
range = rangeCopy;
return result;

[rwlbuis, 2015/12/09 21:31:14]

Done.

[Timothy Loh, 2015/12/10 04:20:07]

Any reason to not use else if here? (which imo makes it super obvious that we
only take one case)

+{
+    CSSValueID id = range.peek().functionId();
+    if (id == CSSValueRadialGradient)
+        return consumeRadialGradient(consumeFunction(range), context, NonRepeating);
+    if (id == CSSValueWebkitLinearGradient) {
+        // FIXME: This should send a deprecation message.
+        if (context.useCounter())
+            context.useCounter()->count(UseCounter::DeprecatedWebKitLinearGradient);
+        return consumeLinearGradient(consumeFunction(range), context, NonRepeating, CSSPrefixedLinearGradient);
+    }
+    if (id == CSSValueWebkitRepeatingLinearGradient) {
+        // FIXME: This should send a deprecation message.
+        if (context.useCounter())
+            context.useCounter()->count(UseCounter::DeprecatedWebKitRepeatingLinearGradient);
+        return consumeLinearGradient(consumeFunction(range), context, Repeating, CSSPrefixedLinearGradient);
+    }
+    if (id == CSSValueLinearGradient)
+        return consumeLinearGradient(consumeFunction(range), context, NonRepeating, CSSLinearGradient);
+    if (id == CSSValueWebkitCrossFade)
+        return consumeCrossFade(consumeFunction(range), context);
+    return nullptr;
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeImage(CSSParserTokenRange& range, CSSParserContext context)
+{
+    if (range.peek().id() == CSSValueNone)
+        return consumeIdent(range);
+
+    AtomicString uri(consumeUrl(range));
+    if (!uri.isNull())
+        return CSSPropertyParser::createCSSImageValueWithReferrer(uri, context);
+    if (range.peek().type() == FunctionToken) {
+        CSSValueID id = range.peek().functionId();
+        if (id == CSSValueWebkitImageSet)
+            return consumeImageSet(range, context);
+        if (CSSPropertyParser::isGeneratedImage(id))
+            return consumeGeneratedImage(range, context);
+    }
+    return nullptr;
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID unresolvedProperty)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);