Merge parseBorderRadius and parseInsetRoundedCorners

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

 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  * Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
@@ skipping 514 matching lines @@
             parsedValue2 = createPrimitiveNumericValue(value);
         } else
             parsedValue2 = parsedValue1;
 
         if (m_valueList->next())
             return false;
         addProperty(propId, CSSValuePair::create(parsedValue1.release(), parsedValue2.release(), CSSValuePair::DropIdenticalValues), important);
         return true;
     }
     case CSSPropertyBorderRadius:
-    case CSSPropertyAliasWebkitBorderRadius:
-        return parseBorderRadius(unresolvedProperty, important);
+    case CSSPropertyAliasWebkitBorderRadius: {

[Timothy Loh, 2016/01/11 02:01:46]

btw the switch is on a resolved property so the alias case isn't actually
needed.

[rwlbuis, 2016/01/11 14:46:18]

Done.

+        ShorthandScope scope(this, unresolvedProperty);
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[4];
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii2[4];
+        if (!parseRadii(radii, radii2, m_valueList, unresolvedProperty))
+            return false;
+        ImplicitScope implicitScope(this);
+        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;
+    }
     case CSSPropertyWebkitBoxReflect:
         if (id == CSSValueNone)
             validPrimitive = true;
         else
             parsedValue = parseReflect();
         break;
     case CSSPropertyFontSizeAdjust: // none | <number>
         ASSERT(RuntimeEnabledFeatures::cssFontSizeAdjustEnabled());
         validPrimitive = (id == CSSValueNone) ? true : validUnit(value, FNumber | FNonNeg);
         break;
@@ skipping 2247 matching lines @@
     if (radii[3])
         return;
     if (!radii[2]) {
         if (!radii[1])
             radii[1] = radii[0];
         radii[2] = radii[0];
     }
     radii[3] = radii[1];
 }
 
-// FIXME: This should be refactored with parseBorderRadius.
-// parseBorderRadius contains support for some legacy radius construction.
-PassRefPtrWillBeRawPtr<CSSBasicShapeInsetValue> CSSPropertyParser::parseInsetRoundedCorners(PassRefPtrWillBeRawPtr<CSSBasicShapeInsetValue> shape, CSSParserValueList* args)
-{
-    CSSParserValue* argument = args->next();
-
-    if (!argument)
-        return nullptr;
-
-    Vector<CSSParserValue*> radiusArguments;
-    while (argument) {
-        radiusArguments.append(argument);
-        argument = args->next();
-    }
-
-    unsigned num = radiusArguments.size();
-    if (!num || num > 9)
-        return nullptr;
-
-    // FIXME: Refactor completeBorderRadii and the array
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[2][4];
-#if ENABLE(OILPAN)
-    // Zero initialize the array of raw pointers.
-    memset(&radii, 0, sizeof(radii));
-#endif
-
-    unsigned indexAfterSlash = 0;
-    for (unsigned i = 0; i < num; ++i) {
-        CSSParserValue* value = radiusArguments.at(i);
-        if (value->m_unit == CSSParserValue::Operator) {
-            if (value->iValue != '/')
-                return nullptr;
-
-            if (!i || indexAfterSlash || i + 1 == num)
-                return nullptr;
-
-            indexAfterSlash = i + 1;
-            completeBorderRadii(radii[0]);
-            continue;
-        }
-
-        if (i - indexAfterSlash >= 4)
-            return nullptr;
-
-        if (!validUnit(value, FLength | FPercent | FNonNeg))
-            return nullptr;
-
-        RefPtrWillBeRawPtr<CSSPrimitiveValue> radius = createPrimitiveNumericValue(value);
-
-        if (!indexAfterSlash)
-            radii[0][i] = radius;
-        else
-            radii[1][i - indexAfterSlash] = radius.release();
-    }
-
-    if (!indexAfterSlash) {
-        completeBorderRadii(radii[0]);
-        for (unsigned i = 0; i < 4; ++i)
-            radii[1][i] = radii[0][i];
-    } else {
-        completeBorderRadii(radii[1]);
-    }
-    shape->setTopLeftRadius(CSSValuePair::create(radii[0][0].release(), radii[1][0].release(), CSSValuePair::DropIdenticalValues));
-    shape->setTopRightRadius(CSSValuePair::create(radii[0][1].release(), radii[1][1].release(), CSSValuePair::DropIdenticalValues));
-    shape->setBottomRightRadius(CSSValuePair::create(radii[0][2].release(), radii[1][2].release(), CSSValuePair::DropIdenticalValues));
-    shape->setBottomLeftRadius(CSSValuePair::create(radii[0][3].release(), radii[1][3].release(), CSSValuePair::DropIdenticalValues));
-
-    return shape;
-}
-
 PassRefPtrWillBeRawPtr<CSSBasicShapeInsetValue> CSSPropertyParser::parseBasicShapeInset(CSSParserValueList* args)
 {
     ASSERT(args);
 
     RefPtrWillBeRawPtr<CSSBasicShapeInsetValue> shape = CSSBasicShapeInsetValue::create();
 
     CSSParserValue* argument = args->current();
     WillBeHeapVector<RefPtrWillBeMember<CSSPrimitiveValue>> widthArguments;
     bool hasRoundedInset = false;
 
     while (argument) {
         if (argument->m_unit == CSSParserValue::Identifier && argument->id == CSSValueRound) {
+            if (!args->next())
+                return nullptr;
             hasRoundedInset = true;
             break;
         }
 
         Units unitFlags = FLength | FPercent;
         if (!validUnit(argument, unitFlags) || widthArguments.size() > 4)
             return nullptr;
 
         widthArguments.append(createPrimitiveNumericValue(argument));
         argument = args->next();
@@ skipping 13 matching lines @@
         break;
     }
     case 4: {
         shape->updateShapeSize4Values(widthArguments[0].get(), widthArguments[1].get(), widthArguments[2].get(), widthArguments[3].get());
         break;
     }
     default:
         return nullptr;
     }
 
-    if (hasRoundedInset)
-        return parseInsetRoundedCorners(shape.release(), args);
+    if (hasRoundedInset) {
+        // FIXME: Refactor completeBorderRadii and the array

[Timothy Loh, 2016/01/11 02:01:46]

What did you have in mind? Or is this just a legacy comment?

[rwlbuis, 2016/01/11 14:46:18]

Well I am still not completely sure the array is the best solution, hence the
FIXME. Maybe it can be addressed once it is ported to use CSSParserTokens.

+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[4];
+        RefPtrWillBeRawPtr<CSSPrimitiveValue> radii2[4];
+        if (!parseRadii(radii, radii2, args))
+            return nullptr;
+        shape->setTopLeftRadius(CSSValuePair::create(radii[0].release(), radii2[0].release(), CSSValuePair::DropIdenticalValues));
+        shape->setTopRightRadius(CSSValuePair::create(radii[1].release(), radii2[1].release(), CSSValuePair::DropIdenticalValues));
+        shape->setBottomRightRadius(CSSValuePair::create(radii[2].release(), radii2[2].release(), CSSValuePair::DropIdenticalValues));
+        shape->setBottomLeftRadius(CSSValuePair::create(radii[3].release(), radii2[3].release(), CSSValuePair::DropIdenticalValues));
+    }
+
     return shape.release();
 }
 
 static bool isContentDistributionKeyword(CSSValueID id)
 {
     return id == CSSValueSpaceBetween || id == CSSValueSpaceAround
         || id == CSSValueSpaceEvenly || id == CSSValueStretch;
 }
 
 static bool isContentPositionKeyword(CSSValueID id)
@@ skipping 1094 matching lines @@
 bool CSSPropertyParser::parseBorderImageWidth(RefPtrWillBeRawPtr<CSSQuadValue>& result)
 {
     return parseBorderImageQuad(FLength | FNumber | FNonNeg | FPercent, result);
 }
 
 bool CSSPropertyParser::parseBorderImageOutset(RefPtrWillBeRawPtr<CSSQuadValue>& result)
 {
     return parseBorderImageQuad(FLength | FNumber | FNonNeg, result);
 }
 
-bool CSSPropertyParser::parseBorderRadius(CSSPropertyID unresolvedProperty, bool important)
+bool CSSPropertyParser::parseRadii(RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[4], RefPtrWillBeRawPtr<CSSPrimitiveValue> radii2[4], CSSParserValueList* args, CSSPropertyID unresolvedProperty)
 {
-    unsigned num = m_valueList->size();
-    if (num > 9)
-        return false;
-
-    ShorthandScope scope(this, unresolvedProperty);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> radii[2][4];
-#if ENABLE(OILPAN)
-    // Zero initialize the array of raw pointers.
-    memset(&radii, 0, sizeof(radii));
-#endif
-
-    unsigned indexAfterSlash = 0;
-    for (unsigned i = 0; i < num; ++i) {
-        CSSParserValue* value = m_valueList->valueAt(i);
-        if (value->m_unit == CSSParserValue::Operator) {
-            if (value->iValue != '/')
-                return false;
-
-            if (!i || indexAfterSlash || i + 1 == num || num > i + 5)
-                return false;
-
-            indexAfterSlash = i + 1;
-            completeBorderRadii(radii[0]);
-            continue;
-        }
-
-        if (i - indexAfterSlash >= 4)
-            return false;
-
+    CSSParserValue* value = args->current();
+    int i;
+    for (i = 0; i < 4 && value && value->m_unit != CSSParserValue::Operator;++i, value = args->next()) {
         if (!validUnit(value, FLength | FPercent | FNonNeg))
             return false;
 
-        RefPtrWillBeRawPtr<CSSPrimitiveValue> radius = createPrimitiveNumericValue(value);
-
-        if (!indexAfterSlash) {
-            radii[0][i] = radius;
-
-            // Legacy syntax: -webkit-border-radius: l1 l2; is equivalent to border-radius: l1 / l2;
-            if (num == 2 && unresolvedProperty == CSSPropertyAliasWebkitBorderRadius) {
-                indexAfterSlash = 1;
-                completeBorderRadii(radii[0]);
-            }
-        } else
-            radii[1][i - indexAfterSlash] = radius.release();
+        radii[i] = createPrimitiveNumericValue(value);
     }
 
-    if (!indexAfterSlash) {
-        completeBorderRadii(radii[0]);
+    if (!i || (value && value->iValue != '/'))

[Timothy Loh, 2016/01/11 02:01:46]

probably need to check that m_unit is Operator

[rwlbuis, 2016/01/11 14:46:18]

Done.

+        return false;
+    // Legacy syntax: -webkit-border-radius: l1 l2; is equivalent to border-radius: l1 / l2;
+    if (!value && i == 2 && unresolvedProperty == CSSPropertyAliasWebkitBorderRadius) {
+        radii2[0] = radii[1];
+        radii[1] = nullptr;
+        completeBorderRadii(radii);
+        completeBorderRadii(radii2);
+        return true;
+    }
+    completeBorderRadii(radii);
+    if (value) {
+        value = args->next();
+        for (i = 0; i < 4 && value && validUnit(value, FLength | FPercent | FNonNeg); ++i, value = args->next())

[Timothy Loh, 2016/01/11 02:01:46]

I don't like having so much logic inside a for-loop header, but I guess it's ok
for now -- I'll comment on the coding style more when we start to move the logic
across to use CSSParserTokens.

[rwlbuis, 2016/01/11 14:46:18]

Acknowledged.

+            radii2[i] = createPrimitiveNumericValue(value);
+        if (!i || value)
+            return false;
+        completeBorderRadii(radii2);
+    } else {
         for (unsigned i = 0; i < 4; ++i)
-            radii[1][i] = radii[0][i];
-    } else
-        completeBorderRadii(radii[1]);
+            radii2[i] = radii[i];
+    }
 
-    ImplicitScope implicitScope(this);
-    addProperty(CSSPropertyBorderTopLeftRadius, CSSValuePair::create(radii[0][0].release(), radii[1][0].release(), CSSValuePair::DropIdenticalValues), important);
-    addProperty(CSSPropertyBorderTopRightRadius, CSSValuePair::create(radii[0][1].release(), radii[1][1].release(), CSSValuePair::DropIdenticalValues), important);
-    addProperty(CSSPropertyBorderBottomRightRadius, CSSValuePair::create(radii[0][2].release(), radii[1][2].release(), CSSValuePair::DropIdenticalValues), important);
-    addProperty(CSSPropertyBorderBottomLeftRadius, CSSValuePair::create(radii[0][3].release(), radii[1][3].release(), CSSValuePair::DropIdenticalValues), important);
     return true;
 }
 
 // This should go away once we drop support for -webkit-gradient
 static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> parseDeprecatedGradientPoint(CSSParserValue* a, bool horizontal)
 {
     RefPtrWillBeRawPtr<CSSPrimitiveValue> result = nullptr;
     if (a->m_unit == CSSParserValue::Identifier) {
         if ((a->id == CSSValueLeft && horizontal)
             || (a->id == CSSValueTop && !horizontal))
@@ skipping 821 matching lines @@
     ASSERT(!m_parsedCalculation);
     m_parsedCalculation = CSSCalcValue::create(args, range);
 
     if (!m_parsedCalculation)
         return false;
 
     return true;
 }
 
 } // namespace blink