Finish 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 6432b846e62d74e23d9fd369d8c4ff6327c26caa..3639ae684110eedee3241067088a6911a8a4f66b 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -832,7 +832,7 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumePosition(CSSParserTokenRange& ran
     return nullptr;
 }
 
-static bool consumeTransformOrigin(CSSParserTokenRange& range, CSSParserMode cssParserMode, UnitlessQuirk unitless, RefPtrWillBeRawPtr<CSSValue>& resultX, RefPtrWillBeRawPtr<CSSValue>& resultY)
+static bool consumePosition2Values(CSSParserTokenRange& range, CSSParserMode cssParserMode, UnitlessQuirk unitless, RefPtrWillBeRawPtr<CSSValue>& resultX, RefPtrWillBeRawPtr<CSSValue>& resultY)

[Timothy Loh, 2015/12/14 06:59:54]

consumeOneOrTwoValuedPosition maybe? :S

[rwlbuis, 2015/12/14 19:38:38]

Done.

 {
     RefPtrWillBeRawPtr<CSSPrimitiveValue> value1 = consumePositionComponent(range, cssParserMode, unitless);
     if (!value1)
@@ -849,7 +849,7 @@ static PassRefPtrWillBeRawPtr<CSSValueList> consumeTransformOrigin(CSSParserToke
 {
     RefPtrWillBeRawPtr<CSSValue> resultX = nullptr;
     RefPtrWillBeRawPtr<CSSValue> resultY = nullptr;
-    if (consumeTransformOrigin(range, cssParserMode, unitless, resultX, resultY)) {
+    if (consumePosition2Values(range, cssParserMode, unitless, resultX, resultY)) {
         RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
         list->append(resultX.release());
         list->append(resultY.release());
@@ -2406,6 +2406,123 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCursor(CSSParserTokenRange& range
     return list.release();
 }
 
+// This should go away once we drop support for -webkit-gradient
+static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> consumeDeprecatedGradientPoint(CSSParserTokenRange& args, bool horizontal)
+{
+    if (args.peek().type() == IdentToken) {
+        if ((horizontal && consumeIdent<CSSValueLeft>(args)) || (!horizontal && consumeIdent<CSSValueTop>(args)))
+            return cssValuePool().createValue(0., CSSPrimitiveValue::UnitType::Percentage);
+        if ((horizontal && consumeIdent<CSSValueRight>(args)) || (!horizontal && consumeIdent<CSSValueBottom>(args)))
+            return cssValuePool().createValue(100., CSSPrimitiveValue::UnitType::Percentage);
+        if (consumeIdent<CSSValueCenter>(args))
+            return cssValuePool().createValue(50., CSSPrimitiveValue::UnitType::Percentage);
+        return nullptr;
+    }
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> result = consumePercent(args, ValueRangeAll);
+    if (!result)
+        result = consumeNumber(args, ValueRangeAll);
+    return result;
+}
+
+// Used to parse colors for -webkit-gradient(...).
+static PassRefPtrWillBeRawPtr<CSSValue> consumeDeprecatedGradientStopColor(CSSParserTokenRange& args, CSSParserMode cssParserMode)
+{
+    // Disallow currentcolor.

[Timothy Loh, 2015/12/14 06:59:55]

This comment doesn't add value

[rwlbuis, 2015/12/14 19:38:38]

Yeah, I thought for a first review I'd keep them but most of them are pretty
obvious indeed. Removed.

+    if (args.peek().id() == CSSValueCurrentcolor)
+        return nullptr;
+    return consumeColor(args, cssParserMode);
+}
+
+static bool consumeDeprecatedGradientColorStop(CSSParserTokenRange& range, CSSGradientColorStop& stop, CSSParserMode cssParserMode)
+{
+    CSSValueID id = range.peek().functionId();
+    if (id != CSSValueFrom && id != CSSValueTo && id != CSSValueColorStop)
+        return false;
+
+    CSSParserTokenRange args = consumeFunction(range);
+    if (id == CSSValueFrom || id == CSSValueTo) {
+        stop.m_position = cssValuePool().createValue((id == CSSValueFrom) ? 0 : 1, CSSPrimitiveValue::UnitType::Number);
+        stop.m_color = consumeDeprecatedGradientStopColor(args, cssParserMode);
+    } else if (id == CSSValueColorStop) {

[Timothy Loh, 2015/12/14 06:59:55]

IMO clearer as

} else {
    ASSERT(id == CSSValueColorStop);

since otherwise it suggests there's a case which isn't caught here.

[rwlbuis, 2015/12/14 19:38:39]

Done. I basically forgot about the if at the top of the method :) Does show that
this was not high quality code/refactored a lot...

+        const CSSParserToken& stopArg = args.consumeIncludingWhitespace();
+        if (stopArg.type() == PercentageToken)
+            stop.m_position = cssValuePool().createValue(stopArg.numericValue() / 100, CSSPrimitiveValue::UnitType::Number);
+        else if (stopArg.type() == NumberToken)
+            stop.m_position = cssValuePool().createValue(stopArg.numericValue(), CSSPrimitiveValue::UnitType::Number);
+        else
+            return false;
+
+        if (!consumeCommaIncludingWhitespace(args))
+            return false;
+
+        stop.m_color = consumeDeprecatedGradientStopColor(args, cssParserMode);

[Timothy Loh, 2015/12/14 06:59:55]

Could put this outside the if/else. Also might be better to have "float
position" before the if/else and assign stop.m_position after the if/else

[rwlbuis, 2015/12/14 19:38:38]

Done.

+    }
+
+    return stop.m_color && args.atEnd();
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeDeprecatedGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode)
+{
+    RefPtrWillBeRawPtr<CSSGradientValue> result = nullptr;
+    CSSValueID id = args.consumeIncludingWhitespace().id();
+    bool isDeprecatedRadialGradient = (id == CSSValueRadial);
+    if (isDeprecatedRadialGradient)
+        result = CSSRadialGradientValue::create(NonRepeating, CSSDeprecatedRadialGradient);
+    else if (id == CSSValueLinear)
+        result = CSSLinearGradientValue::create(NonRepeating, CSSDeprecatedLinearGradient);
+    if (!result || !consumeCommaIncludingWhitespace(args))
+        return nullptr;
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> point = consumeDeprecatedGradientPoint(args, true);
+    if (!point)
+        return nullptr;
+    result->setFirstX(point.release());
+    point = consumeDeprecatedGradientPoint(args, false);
+    if (!point)
+        return nullptr;
+    result->setFirstY(point.release());
+
+    if (!consumeCommaIncludingWhitespace(args))
+        return nullptr;
+
+    // For radial gradients only, we now expect a numeric radius.
+    if (isDeprecatedRadialGradient) {
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radius = consumeNumber(args, ValueRangeAll);
+        if (!radius || !consumeCommaIncludingWhitespace(args))
+            return nullptr;
+        toCSSRadialGradientValue(result.get())->setFirstRadius(radius.release());
+    }
+
+    point = consumeDeprecatedGradientPoint(args, true);
+    if (!point)
+        return nullptr;
+    result->setSecondX(point.release());
+    point = consumeDeprecatedGradientPoint(args, false);
+    if (!point)
+        return nullptr;
+    result->setSecondY(point.release());
+
+    // For radial gradients only, we now expect the second radius.
+    if (isDeprecatedRadialGradient) {
+        if (!consumeCommaIncludingWhitespace(args))
+            return nullptr;
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radius = consumeNumber(args, ValueRangeAll);
+        if (!radius)
+            return nullptr;
+        toCSSRadialGradientValue(result.get())->setSecondRadius(radius.release());
+    }
+
+    CSSGradientColorStop stop;
+    while (!args.atEnd()) {

[Timothy Loh, 2015/12/14 06:59:55]

maybe clearer as while (consumeComma..(args)) ?

[rwlbuis, 2015/12/14 19:38:39]

Oops, forgot, will have a look...

+        // The function name needs to be one of "from", "to", or "color-stop."

[Timothy Loh, 2015/12/14 06:59:55]

not sure this comment helps

[rwlbuis, 2015/12/14 19:38:39]

Done.

+        if (!consumeCommaIncludingWhitespace(args) || !consumeDeprecatedGradientColorStop(args, stop, cssParserMode))
+            return nullptr;
+        result->addStop(stop);
+    }
+
+    return result.release();
+}
+
 static bool consumeGradientColorStops(CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSGradientValue* gradient)
 {
     bool supportsColorHints = gradient->gradientType() == CSSLinearGradient || gradient->gradientType() == CSSRadialGradient;
@@ -2433,6 +2550,50 @@ static bool consumeGradientColorStops(CSSParserTokenRange& range, CSSParserMode
     return gradient->stopCount() >= 2;
 }
 
+static PassRefPtrWillBeRawPtr<CSSValue> consumeDeprecatedRadialGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode, CSSGradientRepeat repeating)
+{
+    RefPtrWillBeRawPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating, CSSPrefixedRadialGradient);
+    // Optional background-position

[Timothy Loh, 2015/12/14 06:59:55]

comment is weird, this isn't background-position, and <position> has a broader
syntax.

[rwlbuis, 2015/12/14 19:38:38]

Done.

+    RefPtrWillBeRawPtr<CSSValue> centerX = nullptr;
+    RefPtrWillBeRawPtr<CSSValue> centerY = nullptr;
+    consumePosition2Values(args, cssParserMode, UnitlessQuirk::Forbid, centerX, centerY);

[Timothy Loh, 2015/12/14 06:59:54]

OK

This function (and the equivalent in the legacy parser) can fail but still
consume, e.g. "background-image: -webkit-radial-gradient(left white, black)"
will be parsed as valid but ignore the "left". I guess it's fine :/

[rwlbuis, 2015/12/14 19:38:39]

Main thing is that everything remains green and we do not accept anything we
should not accept IMHO.

+    if ((centerX || centerY) && !consumeCommaIncludingWhitespace(args))
+        return nullptr;
+
+    result->setFirstX(toCSSPrimitiveValue(centerX.get()));
+    result->setSecondX(toCSSPrimitiveValue(centerX.get()));
+    // CSS3 radial gradients always share the same start and end point.

[Timothy Loh, 2015/12/14 06:59:55]

drop the comment, -webkit-* probably doesn't count as css3 ;)

[rwlbuis, 2015/12/14 19:38:39]

Done.

+    result->setFirstY(toCSSPrimitiveValue(centerY.get()));
+    result->setSecondY(toCSSPrimitiveValue(centerY.get()));
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> shape = consumeIdent<CSSValueCircle, CSSValueEllipse>(args);
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> sizeKeyword = consumeIdent<CSSValueClosestSide, CSSValueClosestCorner, CSSValueFarthestSide, CSSValueFarthestCorner, CSSValueContain, CSSValueCover>(args);
+    if (!shape)
+        shape = consumeIdent<CSSValueCircle, CSSValueEllipse>(args);
+    result->setShape(shape);
+    result->setSizingBehavior(sizeKeyword);
+
+    // Or, two lengths or percentages
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> horizontalSize = nullptr;

[Timothy Loh, 2015/12/14 06:59:54]

I'd move these into the first if branch and the the setEnd*Size calls inside the
nested if branch.

[rwlbuis, 2015/12/14 19:38:38]

Done.

+    RefPtrWillBeRawPtr<CSSPrimitiveValue> verticalSize = nullptr;
+    if (!shape && !sizeKeyword) {
+        if ((horizontalSize = consumeLengthOrPercent(args, cssParserMode, ValueRangeAll))) {
+            verticalSize = consumeLengthOrPercent(args, cssParserMode, ValueRangeAll);
+            if (!verticalSize)
+                return nullptr;
+            consumeCommaIncludingWhitespace(args);
+        }
+    } else {
+        consumeCommaIncludingWhitespace(args);
+    }
+    if (!consumeGradientColorStops(args, cssParserMode, result.get()))
+        return nullptr;
+
+    result->setEndHorizontalSize(horizontalSize);
+    result->setEndVerticalSize(verticalSize);
+    return result.release();
+}
+
 static PassRefPtrWillBeRawPtr<CSSValue> consumeRadialGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode, CSSGradientRepeat repeating)
 {
     RefPtrWillBeRawPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating, CSSRadialGradient);
@@ -2590,6 +2751,20 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeGeneratedImage(CSSParserTokenRang
         result = consumeLinearGradient(args, context.mode(), Repeating, CSSPrefixedLinearGradient);
     } else if (id == CSSValueLinearGradient) {
         result = consumeLinearGradient(args, context.mode(), NonRepeating, CSSLinearGradient);
+    } else if (id == CSSValueWebkitGradient) {
+        // FIXME: This should send a deprecation message.
+        if (context.useCounter())
+            context.useCounter()->count(UseCounter::DeprecatedWebKitGradient);
+        result = consumeDeprecatedGradient(args, context.mode());
+    } else if (id == CSSValueWebkitRadialGradient) {
+        // FIXME: This should send a deprecation message.
+        if (context.useCounter())
+            context.useCounter()->count(UseCounter::DeprecatedWebKitRadialGradient);
+        result = consumeDeprecatedRadialGradient(args, context.mode(), NonRepeating);
+    } else if (id == CSSValueWebkitRepeatingRadialGradient) {
+        if (context.useCounter())
+            context.useCounter()->count(UseCounter::DeprecatedWebKitRepeatingRadialGradient);
+        result = consumeDeprecatedRadialGradient(args, context.mode(), Repeating);
     } else if (id == CSSValueWebkitCrossFade) {
         result = consumeCrossFade(args, context);
     }