Skip to site navigation (Press enter)

[webkit-changes] [196851] trunk/Source/WebCore

simon . fraser Fri, 19 Feb 2016 18:47:27 -0800

Title: [196851] trunk/Source/WebCore

Revision: 196851
Author: [email protected]
Date: 2016-02-19 18:47:08 -0800 (Fri, 19 Feb 2016)

Log Message

Use more concrete types for parsing positions
https://bugs.webkit.org/show_bug.cgi?id=154481


Reviewed by Dean Jackson.

Use CSSPrimitiveValues for position-parsing functions where possible, to avoid
the need to downcast<> the values returned by the parsing functions.

* css/CSSParser.cpp:
(WebCore::CSSParser::parseValue):
(WebCore::CSSParser::parsePositionX):
(WebCore::CSSParser::parsePositionY):
(WebCore::CSSParser::parse4ValuesFillPosition):
(WebCore::CSSParser::parse3ValuesFillPosition):
(WebCore::CSSParser::parseFillPosition):
(WebCore::CSSParser::parse2ValuesFillPosition):
(WebCore::CSSParser::parseFillProperty):
(WebCore::CSSParser::parseTransformOriginShorthand):
(WebCore::CSSParser::parseBasicShapeCircle):
(WebCore::CSSParser::parseBasicShapeEllipse):
(WebCore::CSSParser::parseDeprecatedRadialGradient):
(WebCore::CSSParser::parseRadialGradient):
(WebCore::CSSParser::parseTransformOrigin):
(WebCore::CSSParser::parsePerspectiveOrigin):
* css/CSSParser.h:

Modified Paths

trunk/Source/WebCore/ChangeLog
trunk/Source/WebCore/css/CSSParser.cpp
trunk/Source/WebCore/css/CSSParser.h

Diff

Modified: trunk/Source/WebCore/ChangeLog (196850 => 196851)


--- trunk/Source/WebCore/ChangeLog	2016-02-20 01:58:06 UTC (rev 196850)
+++ trunk/Source/WebCore/ChangeLog	2016-02-20 02:47:08 UTC (rev 196851)
@@ -1,3 +1,31 @@
+2016-02-19  Simon Fraser  <[email protected]>
+
+        Use more concrete types for parsing positions
+        https://bugs.webkit.org/show_bug.cgi?id=154481
+
+        Reviewed by Dean Jackson.
+
+        Use CSSPrimitiveValues for position-parsing functions where possible, to avoid
+        the need to downcast<> the values returned by the parsing functions.
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseValue):
+        (WebCore::CSSParser::parsePositionX):
+        (WebCore::CSSParser::parsePositionY):
+        (WebCore::CSSParser::parse4ValuesFillPosition):
+        (WebCore::CSSParser::parse3ValuesFillPosition):
+        (WebCore::CSSParser::parseFillPosition):
+        (WebCore::CSSParser::parse2ValuesFillPosition):
+        (WebCore::CSSParser::parseFillProperty):
+        (WebCore::CSSParser::parseTransformOriginShorthand):
+        (WebCore::CSSParser::parseBasicShapeCircle):
+        (WebCore::CSSParser::parseBasicShapeEllipse):
+        (WebCore::CSSParser::parseDeprecatedRadialGradient):
+        (WebCore::CSSParser::parseRadialGradient):
+        (WebCore::CSSParser::parseTransformOrigin):
+        (WebCore::CSSParser::parsePerspectiveOrigin):
+        * css/CSSParser.h:
+
 2016-02-18  Gavin Barraclough  <[email protected]>
 
         JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow

Modified: trunk/Source/WebCore/css/CSSParser.cpp (196850 => 196851)


--- trunk/Source/WebCore/css/CSSParser.cpp	2016-02-20 01:58:06 UTC (rev 196850)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2016-02-20 02:47:08 UTC (rev 196851)
@@ -2697,8 +2697,8 @@
     case CSSPropertyTransformOriginX:
     case CSSPropertyTransformOriginY:
     case CSSPropertyTransformOriginZ: {
-        RefPtr<CSSValue> val1;
-        RefPtr<CSSValue> val2;
+        RefPtr<CSSPrimitiveValue> val1;
+        RefPtr<CSSPrimitiveValue> val2;
         RefPtr<CSSValue> val3;
         CSSPropertyID propId1, propId2, propId3;
         if (parseTransformOrigin(propId, propId1, propId2, propId3, val1, val2, val3)) {
@@ -2729,8 +2729,8 @@
     case CSSPropertyPerspectiveOrigin:
     case CSSPropertyPerspectiveOriginX:
     case CSSPropertyPerspectiveOriginY: {
-        RefPtr<CSSValue> val1;
-        RefPtr<CSSValue> val2;
+        RefPtr<CSSPrimitiveValue> val1;
+        RefPtr<CSSPrimitiveValue> val2;
         CSSPropertyID propId1, propId2;
         if (parsePerspectiveOrigin(propId, propId1, propId2, val1, val2)) {
             addProperty(propId1, val1.release(), important);
@@ -4384,7 +4384,7 @@
     return false;
 }
 
-RefPtr<CSSValue> CSSParser::parsePositionX(CSSParserValueList& valueList)
+RefPtr<CSSPrimitiveValue> CSSParser::parsePositionX(CSSParserValueList& valueList)
 {
     int id = valueList.current()->id;
     if (id == CSSValueLeft || id == CSSValueRight || id == CSSValueCenter) {
@@ -4401,7 +4401,7 @@
     return nullptr;
 }
 
-RefPtr<CSSValue> CSSParser::parsePositionY(CSSParserValueList& valueList)
+RefPtr<CSSPrimitiveValue> CSSParser::parsePositionY(CSSParserValueList& valueList)
 {
     int id = valueList.current()->id;
     if (id == CSSValueTop || id == CSSValueBottom || id == CSSValueCenter) {
@@ -4481,7 +4481,7 @@
     return value == CSSValueLeft || value == CSSValueTop || value == CSSValueBottom || value == CSSValueRight || value == CSSValueCenter;
 }
 
-void CSSParser::parse4ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2, RefPtr<CSSPrimitiveValue>&& parsedValue1, RefPtr<CSSPrimitiveValue>&& parsedValue2)
+void CSSParser::parse4ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSPrimitiveValue>& value1, RefPtr<CSSPrimitiveValue>& value2, RefPtr<CSSPrimitiveValue>&& parsedValue1, RefPtr<CSSPrimitiveValue>&& parsedValue2)
 {
     // [ left | right ] [ <percentage] | <length> ] && [ top | bottom ] [ <percentage> | <length> ]
     // In the case of 4 values <position> requires the second value to be a length or a percentage.
@@ -4529,7 +4529,7 @@
 
     valueList.next();
 }
-void CSSParser::parse3ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2, RefPtr<CSSPrimitiveValue>&& parsedValue1, RefPtr<CSSPrimitiveValue>&& parsedValue2)
+void CSSParser::parse3ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSPrimitiveValue>& value1, RefPtr<CSSPrimitiveValue>& value2, RefPtr<CSSPrimitiveValue>&& parsedValue1, RefPtr<CSSPrimitiveValue>&& parsedValue2)
 {
     unsigned cumulativeFlags = 0;
     FillPositionFlag value3Flag = InvalidFillPosition;
@@ -4612,8 +4612,8 @@
         value1.swap(value2);
 
 #ifndef NDEBUG
-    CSSPrimitiveValue& first = downcast<CSSPrimitiveValue>(*value1);
-    CSSPrimitiveValue& second = downcast<CSSPrimitiveValue>(*value2);
+    CSSPrimitiveValue& first = *value1;
+    CSSPrimitiveValue& second = *value2;
     ident1 = first.getPairValue()->first()->getValueID();
     ident2 = second.getPairValue()->first()->getValueID();
     ASSERT(ident1 == CSSValueLeft || ident1 == CSSValueRight);
@@ -4629,7 +4629,7 @@
     return validateUnit(valueWithCalculation, FPercent | FLength);
 }
 
-void CSSParser::parseFillPosition(CSSParserValueList& valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2)
+void CSSParser::parseFillPosition(CSSParserValueList& valueList, RefPtr<CSSPrimitiveValue>& value1, RefPtr<CSSPrimitiveValue>& value2)
 {
     unsigned numberOfValues = 0;
     for (unsigned i = valueList.currentIndex(); i < valueList.size(); ++i, ++numberOfValues) {
@@ -4676,8 +4676,8 @@
         return;
     }
 
-    RefPtr<CSSPrimitiveValue> parsedValue1 = downcast<CSSPrimitiveValue>(value1.get());
-    RefPtr<CSSPrimitiveValue> parsedValue2 = downcast<CSSPrimitiveValue>(value2.get());
+    RefPtr<CSSPrimitiveValue> parsedValue1 = value1;
+    RefPtr<CSSPrimitiveValue> parsedValue2 = value2;
 
     value1 = nullptr;
     value2 = nullptr;
@@ -4692,7 +4692,7 @@
         parse4ValuesFillPosition(valueList, value1, value2, WTFMove(parsedValue1), WTFMove(parsedValue2));
 }
 
-void CSSParser::parse2ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2)
+void CSSParser::parse2ValuesFillPosition(CSSParserValueList& valueList, RefPtr<CSSPrimitiveValue>& value1, RefPtr<CSSPrimitiveValue>& value2)
 {
     CSSParserValue* value = valueList.current();
 
@@ -4903,10 +4903,15 @@
                     }
                     break;
                 case CSSPropertyBackgroundPosition:
-                case CSSPropertyWebkitMaskPosition:
-                    parseFillPosition(*m_valueList, currValue, currValue2);
+                case CSSPropertyWebkitMaskPosition: {
+                    RefPtr<CSSPrimitiveValue> value1;
+                    RefPtr<CSSPrimitiveValue> value2;
+                    parseFillPosition(*m_valueList, value1, value2);
+                    currValue = value1;
+                    currValue2 = value2;
                     // parseFillPosition advances the m_valueList pointer.
                     break;
+                }
                 case CSSPropertyBackgroundPositionX:
                 case CSSPropertyWebkitMaskPositionX: {
                     currValue = parsePositionX(*m_valueList);
@@ -5165,7 +5170,7 @@
     return keys;
 }
 
-bool CSSParser::parseTransformOriginShorthand(RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2, RefPtr<CSSValue>& value3)
+bool CSSParser::parseTransformOriginShorthand(RefPtr<CSSPrimitiveValue>& value1, RefPtr<CSSPrimitiveValue>& value2, RefPtr<CSSValue>& value3)
 {
     parse2ValuesFillPosition(*m_valueList, value1, value2);
 
@@ -6530,12 +6535,12 @@
         }
 
         if (argument->id == CSSValueAt && args.next()) {
-            RefPtr<CSSValue> centerX;
-            RefPtr<CSSValue> centerY;
+            RefPtr<CSSPrimitiveValue> centerX;
+            RefPtr<CSSPrimitiveValue> centerY;
             parseFillPosition(args, centerX, centerY);
             if (centerX && centerY && !args.current()) {
-                shape->setCenterX(downcast<CSSPrimitiveValue>(centerX.get()));
-                shape->setCenterY(downcast<CSSPrimitiveValue>(centerY.get()));
+                shape->setCenterX(centerX);
+                shape->setCenterY(centerY);
             } else
                 return nullptr;
         } else
@@ -6577,14 +6582,14 @@
         if (argument->id != CSSValueAt || !args.next()) // expecting ellipse(.. at <position>)
             return nullptr;
 
-        RefPtr<CSSValue> centerX;
-        RefPtr<CSSValue> centerY;
+        RefPtr<CSSPrimitiveValue> centerX;
+        RefPtr<CSSPrimitiveValue> centerY;
         parseFillPosition(args, centerX, centerY);
         if (!centerX || !centerY || args.current())
             return nullptr;
 
-        shape->setCenterX(downcast<CSSPrimitiveValue>(centerX.get()));
-        shape->setCenterY(downcast<CSSPrimitiveValue>(centerY.get()));
+        shape->setCenterX(centerX);
+        shape->setCenterY(centerY);
     }
 
     return shape;
@@ -8965,8 +8970,8 @@
     bool expectComma = false;
 
     // Optional background-position
-    RefPtr<CSSValue> centerX;
-    RefPtr<CSSValue> centerY;
+    RefPtr<CSSPrimitiveValue> centerX;
+    RefPtr<CSSPrimitiveValue> centerY;
     // parse2ValuesFillPosition advances the args next pointer.
     parse2ValuesFillPosition(*args, centerX, centerY);
     argument = args->current();
@@ -8983,11 +8988,11 @@
             return false;
     }
 
-    result->setFirstX(downcast<CSSPrimitiveValue>(centerX.get()));
-    result->setSecondX(downcast<CSSPrimitiveValue>(centerX.get()));
+    result->setFirstX(centerX);
+    result->setSecondX(centerX);
     // CSS3 radial gradients always share the same start and end point.
-    result->setFirstY(downcast<CSSPrimitiveValue>(centerY.get()));
-    result->setSecondY(downcast<CSSPrimitiveValue>(centerY.get()));
+    result->setFirstY(centerY);
+    result->setSecondY(centerY);
 
     RefPtr<CSSPrimitiveValue> shapeValue;
     RefPtr<CSSPrimitiveValue> sizeValue;
@@ -9236,8 +9241,8 @@
 
     // Second part of grammar, the center-position clause:
     // at <position>
-    RefPtr<CSSValue> centerX;
-    RefPtr<CSSValue> centerY;
+    RefPtr<CSSPrimitiveValue> centerX;
+    RefPtr<CSSPrimitiveValue> centerY;
     if (argument->unit == CSSPrimitiveValue::CSS_IDENT && equalLettersIgnoringASCIICase(*argument, "at")) {
         argument = args->next();
         if (!argument)
@@ -9251,11 +9256,11 @@
         if (!argument)
             return false;
 
-        result->setFirstX(downcast<CSSPrimitiveValue>(centerX.get()));
-        result->setFirstY(downcast<CSSPrimitiveValue>(centerY.get()));
+        result->setFirstX(centerX);
+        result->setFirstY(centerY);
         // Right now, CSS radial gradients have the same start and end centers.
-        result->setSecondX(downcast<CSSPrimitiveValue>(centerX.get()));
-        result->setSecondY(downcast<CSSPrimitiveValue>(centerY.get()));
+        result->setSecondX(centerX);
+        result->setSecondY(centerY);
     }
 
     if (shapeValue || sizeValue || horizontalSize || centerX || centerY)
@@ -10199,7 +10204,7 @@
 }
 #endif
 
-bool CSSParser::parseTransformOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, CSSPropertyID& propId3, RefPtr<CSSValue>& value, RefPtr<CSSValue>& value2, RefPtr<CSSValue>& value3)
+bool CSSParser::parseTransformOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, CSSPropertyID& propId3, RefPtr<CSSPrimitiveValue>& value, RefPtr<CSSPrimitiveValue>& value2, RefPtr<CSSValue>& value3)
 {
     propId1 = propId;
     propId2 = propId;
@@ -10244,7 +10249,7 @@
     return value;
 }
 
-bool CSSParser::parsePerspectiveOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, RefPtr<CSSValue>& value, RefPtr<CSSValue>& value2)
+bool CSSParser::parsePerspectiveOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, RefPtr<CSSPrimitiveValue>& value, RefPtr<CSSPrimitiveValue>& value2)
 {
     propId1 = propId;
     propId2 = propId;

Modified: trunk/Source/WebCore/css/CSSParser.h (196850 => 196851)


--- trunk/Source/WebCore/css/CSSParser.h	2016-02-20 01:58:06 UTC (rev 196850)
+++ trunk/Source/WebCore/css/CSSParser.h	2016-02-20 02:47:08 UTC (rev 196851)
@@ -166,13 +166,13 @@
     enum FillPositionFlag { InvalidFillPosition = 0, AmbiguousFillPosition = 1, XFillPosition = 2, YFillPosition = 4 };
     enum FillPositionParsingMode { ResolveValuesAsPercent = 0, ResolveValuesAsKeyword = 1 };
     RefPtr<CSSPrimitiveValue> parseFillPositionComponent(CSSParserValueList&, unsigned& cumulativeFlags, FillPositionFlag& individualFlag, FillPositionParsingMode = ResolveValuesAsPercent);
-    RefPtr<CSSValue> parsePositionX(CSSParserValueList&);
-    RefPtr<CSSValue> parsePositionY(CSSParserValueList&);
-    void parse2ValuesFillPosition(CSSParserValueList&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
+    RefPtr<CSSPrimitiveValue> parsePositionX(CSSParserValueList&);
+    RefPtr<CSSPrimitiveValue> parsePositionY(CSSParserValueList&);
+    void parse2ValuesFillPosition(CSSParserValueList&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&);
     bool isPotentialPositionValue(CSSParserValue&);
-    void parseFillPosition(CSSParserValueList&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
-    void parse3ValuesFillPosition(CSSParserValueList&, RefPtr<CSSValue>&, RefPtr<CSSValue>&, RefPtr<CSSPrimitiveValue>&&, RefPtr<CSSPrimitiveValue>&&);
-    void parse4ValuesFillPosition(CSSParserValueList&, RefPtr<CSSValue>&, RefPtr<CSSValue>&, RefPtr<CSSPrimitiveValue>&&, RefPtr<CSSPrimitiveValue>&&);
+    void parseFillPosition(CSSParserValueList&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&);
+    void parse3ValuesFillPosition(CSSParserValueList&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&&, RefPtr<CSSPrimitiveValue>&&);
+    void parse4ValuesFillPosition(CSSParserValueList&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&&, RefPtr<CSSPrimitiveValue>&&);
 
     void parseFillRepeat(RefPtr<CSSValue>&, RefPtr<CSSValue>&);
     RefPtr<CSSValue> parseFillSize(CSSPropertyID, bool &allowComma);
@@ -197,7 +197,7 @@
 #endif
     static Vector<double> parseKeyframeSelector(const String&);
 
-    bool parseTransformOriginShorthand(RefPtr<CSSValue>&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
+    bool parseTransformOriginShorthand(RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSValue>&);
     bool parseCubicBezierTimingFunctionValue(CSSParserValueList& args, double& result);
     bool parseAnimationProperty(CSSPropertyID, RefPtr<CSSValue>&, AnimationParseContext&);
     bool parseTransitionShorthand(CSSPropertyID, bool important);
@@ -324,8 +324,8 @@
 
     RefPtr<CSSValueList> parseTransform();
     RefPtr<CSSValue> parseTransformValue(CSSParserValue&);
-    bool parseTransformOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, CSSPropertyID& propId3, RefPtr<CSSValue>&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
-    bool parsePerspectiveOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2,  RefPtr<CSSValue>&, RefPtr<CSSValue>&);
+    bool parseTransformOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, CSSPropertyID& propId3, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&, RefPtr<CSSValue>&);
+    bool parsePerspectiveOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2,  RefPtr<CSSPrimitiveValue>&, RefPtr<CSSPrimitiveValue>&);
 
     bool parseTextEmphasisStyle(bool important);
     bool parseTextEmphasisPosition(bool important);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes