Parse border-radius shorthand in CSSPropertyParser with 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 816d91fc0e161728344bbba6368399dea1f37fc3..93b4cd80ca75d9e3e0da9f6d2ab3317c6304ee59 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -2923,6 +2923,17 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeScrollSnapPoints(CSSParserTokenRa
     return nullptr;
 }
 
+static PassRefPtrWillBeRawPtr<CSSValue> consumeBorderRadiusCorner(CSSParserTokenRange& range, CSSParserMode cssParserMode)
+{
+    RefPtrWillBeRawPtr<CSSValue> parsedValue1 = consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
+    if (!parsedValue1)
+        return nullptr;
+    RefPtrWillBeRawPtr<CSSValue> parsedValue2 = consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
+    if (!parsedValue2)
+        parsedValue2 = parsedValue1;
+    return CSSValuePair::create(parsedValue1.release(), parsedValue2.release(), CSSValuePair::DropIdenticalValues);
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID unresolvedProperty)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
@@ -3175,6 +3186,11 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSProperty
     case CSSPropertyScrollSnapPointsX:
     case CSSPropertyScrollSnapPointsY:
         return consumeScrollSnapPoints(m_range, m_context.mode());
+    case CSSPropertyBorderTopRightRadius:
+    case CSSPropertyBorderTopLeftRadius:
+    case CSSPropertyBorderBottomLeftRadius:
+    case CSSPropertyBorderBottomRightRadius:
+        return consumeBorderRadiusCorner(m_range, m_context.mode());
     default:
         return nullptr;
     }
@@ -3656,6 +3672,48 @@ bool CSSPropertyParser::consume4Values(const StylePropertyShorthand& shorthand,
     return m_range.atEnd();
 }
 
+static bool consumeRadii(RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[4], RefPtrWillBeRawPtr<CSSPrimitiveValue> radii2[4], CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSPropertyID unresolvedProperty)

[Timothy Loh, 2016/01/14 06:38:45]

I'd rather if the last argument was useLegacyParsing. Also maybe radii ->
horizontalRadii and radii2 -> verticalRadii?

[rwlbuis, 2016/01/14 22:45:50]

Done.

+{
+#if ENABLE(OILPAN)
+    // Unconditionally zero initialize the arrays of raw pointers.
+    memset(radii, 0, 4 * sizeof(radii[0]));
+    memset(radii2, 0, 4 * sizeof(radii2[0]));
+#endif
+    for (unsigned i = 0; i < 4 && !range.atEnd() && range.peek().type() != DelimiterToken; ++i) {
+        radii[i] = consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
+        if (!radii[i])
+            return false;
+        // Legacy syntax: -webkit-border-radius: l1 l2; is equivalent to border-radius: l1 / l2;
+        if (range.atEnd() && i == 1 && unresolvedProperty == CSSPropertyAliasWebkitBorderRadius) {

[Timothy Loh, 2016/01/14 06:38:45]

I think this code should be adjacent to the code handling the rest of the
no-slash cases, instead of having the slash case in between.

[rwlbuis, 2016/01/14 22:45:50]

Actually it is hard to find a nice structure... Hope my new version is ok.

[Timothy Loh, 2016/01/15 01:11:56]

I know, otherwise I would've suggested something explicitly :S

Anyway, I think it's an improvement.

+            radii2[0] = radii[1];
+            radii[1] = nullptr;
+            completeBorderRadii(radii);
+            completeBorderRadii(radii2);
+            return true;
+        }
+    }
+    if (!radii[0])
+        return false;
+    completeBorderRadii(radii);
+    if (!range.atEnd()) {
+        if (range.peek().type() != DelimiterToken || range.peek().delimiter() != '/')
+            return false;
+        range.consumeIncludingWhitespace();
+        for (unsigned i = 0; i < 4 && !range.atEnd(); ++i) {
+            radii2[i] = consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
+            if (!radii2[i])
+                break;

[Timothy Loh, 2016/01/14 06:38:45]

The first loop returns false in this case, can we make these consistent with
each other?

[rwlbuis, 2016/01/14 22:45:50]

Done.

+        }
+        if (!radii2[0] || !range.atEnd())
+            return false;
+        completeBorderRadii(radii2);
+    } else {
+        for (unsigned i = 0; i < 4; ++i)
+            radii2[i] = radii[i];
+    }
+    return true;
+}
+
 bool CSSPropertyParser::parseShorthand(CSSPropertyID unresolvedProperty, bool important)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
@@ -3770,6 +3828,18 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID unresolvedProperty, bool im
         return consumeShorthandGreedily(webkitColumnRuleShorthand(), important);
     case CSSPropertyListStyle:
         return consumeShorthandGreedily(listStyleShorthand(), important);
+    case CSSPropertyBorderRadius: {
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[4];
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii2[4];
+        if (!consumeRadii(radii, radii2, m_range, m_context.mode(), unresolvedProperty))
+            return false;
+        ImplicitScope implicitScope(this);

[Timothy Loh, 2016/01/14 06:38:45]

I think this isn't needed.

[rwlbuis, 2016/01/14 22:45:50]

Done.

+        addProperty(CSSPropertyBorderTopLeftRadius, CSSValuePair::create(radii[0].release(), radii2[0].release(), CSSValuePair::DropIdenticalValues), important);
+        addProperty(CSSPropertyBorderTopRightRadius, CSSValuePair::create(radii[1].release(), radii2[1].release(), CSSValuePair::DropIdenticalValues), important);
+        addProperty(CSSPropertyBorderBottomRightRadius, CSSValuePair::create(radii[2].release(), radii2[2].release(), CSSValuePair::DropIdenticalValues), important);
+        addProperty(CSSPropertyBorderBottomLeftRadius, CSSValuePair::create(radii[3].release(), radii2[3].release(), CSSValuePair::DropIdenticalValues), important);
+        return true;
+    }
     default:
         m_currentShorthand = oldShorthand;
         return false;