Parse border-radius shorthand in CSSPropertyParser with CSSParserTokens

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/CSSCalculationValue.h"
 #include "core/css/CSSCounterValue.h"
 #include "core/css/CSSCrossfadeValue.h"
@@ skipping 2905 matching lines @@
         RefPtrWillBeRawPtr<CSSPrimitiveValue> parsedValue = consumeLengthOrPercent(args, cssParserMode, ValueRangeNonNegative);
         if (args.atEnd() && parsedValue && (parsedValue->isCalculated() || parsedValue->getDoubleValue() > 0)) {
             RefPtrWillBeRawPtr<CSSFunctionValue> result = CSSFunctionValue::create(CSSValueRepeat);
             result->append(parsedValue.release());
             return result.release();
         }
     }
     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);
     switch (property) {
     case CSSPropertyWillChange:
         return consumeWillChange(m_range);
     case CSSPropertyPage:
         return consumePage(m_range);
     case CSSPropertyQuotes:
         return consumeQuotes(m_range);
@@ skipping 232 matching lines @@
         return consumeContent(m_range, m_context);
     case CSSPropertyListStyleImage:
         return consumeImage(m_range, m_context);
     case CSSPropertyPerspective:
         return consumePerspective(m_range, m_context.mode(), unresolvedProperty);
     case CSSPropertyScrollSnapCoordinate:
         return consumeScrollSnapCoordinate(m_range, m_context.mode());
     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;
     }
 }
 
 static PassRefPtrWillBeRawPtr<CSSValueList> consumeFontFaceUnicodeRange(CSSParserTokenRange& range)
 {
     RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
 
     do {
@@ skipping 461 matching lines @@
         left = right;
 
     addProperty(longhands[0], top.release(), important);
     addProperty(longhands[1], right.release(), important);
     addProperty(longhands[2], bottom.release(), important);
     addProperty(longhands[3], left.release(), important);
 
     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);
 
     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 94 matching lines @@
         return true;
     }
     case CSSPropertyFlex:
         return consumeFlex(important);
     case CSSPropertyFlexFlow:
         return consumeShorthandGreedily(flexFlowShorthand(), important);
     case CSSPropertyWebkitColumnRule:
         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;
     }
 }
 
 } // namespace blink