Changed Pair to be a CSSValue

Source/core/css/parser/CSSPropertyParser.cpp

Index: Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/Source/core/css/parser/CSSPropertyParser.cpp b/Source/core/css/parser/CSSPropertyParser.cpp
index 2de05cca2236969981c01c5f12eab0dd9b034961..82a87ba30a10ebf18b12e38cd5822ff14dc20064 100644
--- a/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/Source/core/css/parser/CSSPropertyParser.cpp
@@ -52,9 +52,9 @@
 #include "core/css/CSSShadowValue.h"
 #include "core/css/CSSTimingFunctionValue.h"
 #include "core/css/CSSUnicodeRangeValue.h"
+#include "core/css/CSSValuePair.h"
 #include "core/css/CSSValuePool.h"
 #include "core/css/HashTools.h"
-#include "core/css/Pair.h"
 #include "core/css/parser/CSSParserFastPaths.h"
 #include "core/css/parser/CSSParserValues.h"
 #include "core/frame/UseCounter.h"
@@ -75,9 +75,9 @@ static bool equalIgnoringCase(const CSSParserString& a, const char (&b)[N])
     return a.is8Bit() ? WTF::equalIgnoringCase(b, a.characters8(), length) : WTF::equalIgnoringCase(b, a.characters16(), length);
 }
 
-static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> createPrimitiveValuePair(PassRefPtrWillBeRawPtr<CSSPrimitiveValue> first, PassRefPtrWillBeRawPtr<CSSPrimitiveValue> second, Pair::IdenticalValuesPolicy identicalValuesPolicy = Pair::DropIdenticalValues)
+static PassRefPtrWillBeRawPtr<CSSValuePair> createCSSValuePair(PassRefPtrWillBeRawPtr<CSSValue> first, PassRefPtrWillBeRawPtr<CSSValue> second, CSSValuePair::IdenticalValuesPolicy identicalValuesPolicy = CSSValuePair::DropIdenticalValues)

[Timothy Loh, 2015/08/28 05:51:46]

I would probably just drop this function, all it does now is provide a default
value which would probably be better if it were explicit.

[sashab, 2015/08/31 00:33:51]

Was thinking the same thing actually. Done.

Should I add 'using CSSValuePair::DropIdenticalValues,
CSSValuePair::KeepIdenticalValues' so we don't have to use the prefix
everywhere?

[Timothy Loh, 2015/08/31 01:08:04]

It's probably fine to add the using statement to this file, but I think it's
slightly clearer without it.

[sashab, 2015/08/31 01:29:47]

Sure, just left it. Makes for some long lines in the file but there are some
ridiculously long lines in it anyway

 {
-    return cssValuePool().createValue(Pair::create(first, second, identicalValuesPolicy));
+    return CSSValuePair::create(first, second, identicalValuesPolicy);
 }
 
 CSSPropertyParser::CSSPropertyParser(CSSParserValueList* valueList,
@@ -987,7 +987,7 @@ bool CSSPropertyParser::parseValue(CSSPropertyID unresolvedProperty, bool import
 
         if (m_valueList->next())
             return false;
-        addProperty(propId, createPrimitiveValuePair(parsedValue1.release(), parsedValue2.release()), important);
+        addProperty(propId, createCSSValuePair(parsedValue1.release(), parsedValue2.release()), important);
         return true;
     }
     case CSSPropertyTabSize:
@@ -2496,8 +2496,8 @@ void CSSPropertyParser::parse4ValuesFillPosition(CSSParserValueList* valueList,
     if (isFillPositionKeyword(value4->getValueID()))
         return;
 
-    value1 = createPrimitiveValuePair(parsedValue1, parsedValue2);
-    value2 = createPrimitiveValuePair(value3, value4);
+    value1 = createCSSValuePair(parsedValue1, parsedValue2);
+    value2 = createCSSValuePair(value3, value4);
 
     if (ident1 == CSSValueTop || ident1 == CSSValueBottom)
         value1.swap(value2);
@@ -2538,8 +2538,8 @@ void CSSPropertyParser::parse3ValuesFillPosition(CSSParserValueList* valueList,
             firstPositionKeyword = CSSValueTop;
             swapNeeded = true;
         }
-        value1 = createPrimitiveValuePair(cssValuePool().createIdentifierValue(firstPositionKeyword), cssValuePool().createValue(50, CSSPrimitiveValue::UnitType::Percentage));
-        value2 = createPrimitiveValuePair(parsedValue2, value3);
+        value1 = createCSSValuePair(cssValuePool().createIdentifierValue(firstPositionKeyword), cssValuePool().createValue(50, CSSPrimitiveValue::UnitType::Percentage));
+        value2 = createCSSValuePair(parsedValue2, value3);
     } else if (ident3 == CSSValueCenter) {
         if (isFillPositionKeyword(ident2))
             return;
@@ -2549,8 +2549,8 @@ void CSSPropertyParser::parse3ValuesFillPosition(CSSParserValueList* valueList,
             secondPositionKeyword = CSSValueLeft;
             swapNeeded = true;
         }
-        value1 = createPrimitiveValuePair(parsedValue1, parsedValue2);
-        value2 = createPrimitiveValuePair(cssValuePool().createIdentifierValue(secondPositionKeyword), cssValuePool().createValue(50, CSSPrimitiveValue::UnitType::Percentage));
+        value1 = createCSSValuePair(parsedValue1, parsedValue2);
+        value2 = createCSSValuePair(cssValuePool().createIdentifierValue(secondPositionKeyword), cssValuePool().createValue(50, CSSPrimitiveValue::UnitType::Percentage));
     } else {
         RefPtrWillBeRawPtr<CSSPrimitiveValue> firstPositionValue = nullptr;
         RefPtrWillBeRawPtr<CSSPrimitiveValue> secondPositionValue = nullptr;
@@ -2578,18 +2578,18 @@ void CSSPropertyParser::parse3ValuesFillPosition(CSSParserValueList* valueList,
         if (isValueConflictingWithCurrentEdge(ident1, secondPositionKeyword))
             return;
 
-        value1 = createPrimitiveValuePair(parsedValue1, firstPositionValue);
-        value2 = createPrimitiveValuePair(cssValuePool().createIdentifierValue(secondPositionKeyword), secondPositionValue);
+        value1 = createCSSValuePair(parsedValue1, firstPositionValue);
+        value2 = createCSSValuePair(cssValuePool().createIdentifierValue(secondPositionKeyword), secondPositionValue);
     }
 
     if (ident1 == CSSValueTop || ident1 == CSSValueBottom || swapNeeded)
         value1.swap(value2);
 
 #if ENABLE(ASSERT)
-    CSSPrimitiveValue* first = toCSSPrimitiveValue(value1.get());
-    CSSPrimitiveValue* second = toCSSPrimitiveValue(value2.get());
-    ident1 = first->getPairValue()->first()->getValueID();
-    ident2 = second->getPairValue()->first()->getValueID();
+    const CSSValuePair* first = toCSSValuePair(value1.get());
+    const CSSValuePair* second = toCSSValuePair(value2.get());
+    ident1 = toCSSPrimitiveValue(first->first())->getValueID();
+    ident2 = toCSSPrimitiveValue(second->first())->getValueID();
     ASSERT(ident1 == CSSValueLeft || ident1 == CSSValueRight);
     ASSERT(ident2 == CSSValueBottom || ident2 == CSSValueTop);
 #endif
@@ -2785,7 +2785,7 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseFillSize(CSSPropertyID
     if (!parsedValue2)
         return parsedValue1;
 
-    return createPrimitiveValuePair(parsedValue1.release(), parsedValue2.release(), Pair::KeepIdenticalValues);
+    return createCSSValuePair(parsedValue1.release(), parsedValue2.release(), CSSValuePair::KeepIdenticalValues);
 }
 
 bool CSSPropertyParser::parseFillProperty(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2,
@@ -4098,10 +4098,10 @@ PassRefPtrWillBeRawPtr<CSSBasicShape> CSSPropertyParser::parseInsetRoundedCorner
     } else {
         completeBorderRadii(radii[1]);
     }
-    shape->setTopLeftRadius(createPrimitiveValuePair(radii[0][0].release(), radii[1][0].release()));
-    shape->setTopRightRadius(createPrimitiveValuePair(radii[0][1].release(), radii[1][1].release()));
-    shape->setBottomRightRadius(createPrimitiveValuePair(radii[0][2].release(), radii[1][2].release()));
-    shape->setBottomLeftRadius(createPrimitiveValuePair(radii[0][3].release(), radii[1][3].release()));
+    shape->setTopLeftRadius(createCSSValuePair(radii[0][0].release(), radii[1][0].release()));
+    shape->setTopRightRadius(createCSSValuePair(radii[0][1].release(), radii[1][1].release()));
+    shape->setBottomRightRadius(createCSSValuePair(radii[0][2].release(), radii[1][2].release()));
+    shape->setBottomLeftRadius(createCSSValuePair(radii[0][3].release(), radii[1][3].release()));
 
     return shape;
 }
@@ -4206,7 +4206,7 @@ bool CSSPropertyParser::parseLegacyPosition(CSSPropertyID propId, bool important
         return false;
     }
 
-    addProperty(propId, createPrimitiveValuePair(cssValuePool().createIdentifierValue(CSSValueLegacy), cssValuePool().createIdentifierValue(value->id)), important);
+    addProperty(propId, createCSSValuePair(cssValuePool().createIdentifierValue(CSSValueLegacy), cssValuePool().createIdentifierValue(value->id)), important);
     return !m_valueList->next();
 }
 
@@ -4304,7 +4304,7 @@ bool CSSPropertyParser::parseItemPositionOverflowPosition(CSSPropertyID propId,
 
     ASSERT(position);
     if (overflowAlignmentKeyword)
-        addProperty(propId, createPrimitiveValuePair(position, overflowAlignmentKeyword), important);
+        addProperty(propId, createCSSValuePair(position, overflowAlignmentKeyword), important);
     else
         addProperty(propId, position.release(), important);
 
@@ -4353,10 +4353,8 @@ PassRefPtrWillBeRawPtr<CSSBasicShape> CSSPropertyParser::parseBasicShapeCircle(C
             RefPtrWillBeRawPtr<CSSValue> centerY = nullptr;
             parseFillPosition(args, centerX, centerY);
             if (centerX && centerY && !args->current()) {
-                ASSERT(centerX->isPrimitiveValue());
-                ASSERT(centerY->isPrimitiveValue());
-                shape->setCenterX(toCSSPrimitiveValue(centerX.get()));
-                shape->setCenterY(toCSSPrimitiveValue(centerY.get()));
+                shape->setCenterX(centerX);
+                shape->setCenterY(centerY);
             } else {
                 return nullptr;
             }
@@ -4407,10 +4405,8 @@ PassRefPtrWillBeRawPtr<CSSBasicShape> CSSPropertyParser::parseBasicShapeEllipse(
         if (!centerX || !centerY || args->current())
             return nullptr;
 
-        ASSERT(centerX->isPrimitiveValue());
-        ASSERT(centerY->isPrimitiveValue());
-        shape->setCenterX(toCSSPrimitiveValue(centerX.get()));
-        shape->setCenterY(toCSSPrimitiveValue(centerY.get()));
+        shape->setCenterX(centerX);
+        shape->setCenterY(centerY);
     }
 
     return shape;
@@ -5410,7 +5406,7 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parsePosition(CSSParserValue
     parseFillPosition(valueList, xValue, yValue);
     if (!xValue || !yValue)
         return nullptr;
-    return createPrimitiveValuePair(toCSSPrimitiveValue(xValue.get()), toCSSPrimitiveValue(yValue.get()), Pair::KeepIdenticalValues);
+    return createCSSValuePair(xValue.release(), yValue.release(), CSSValuePair::KeepIdenticalValues);
 }
 
 // Parses a list of comma separated positions. i.e., <position>#
@@ -5700,7 +5696,7 @@ bool CSSPropertyParser::parseBorderImageRepeat(RefPtrWillBeRawPtr<CSSValue>& res
     } else
         secondValue = firstValue;
 
-    result = createPrimitiveValuePair(firstValue, secondValue);
+    result = createCSSValuePair(firstValue, secondValue);
     return true;
 }
 
@@ -5958,10 +5954,10 @@ bool CSSPropertyParser::parseBorderRadius(CSSPropertyID unresolvedProperty, bool
         completeBorderRadii(radii[1]);
 
     ImplicitScope implicitScope(this);
-    addProperty(CSSPropertyBorderTopLeftRadius, createPrimitiveValuePair(radii[0][0].release(), radii[1][0].release()), important);
-    addProperty(CSSPropertyBorderTopRightRadius, createPrimitiveValuePair(radii[0][1].release(), radii[1][1].release()), important);
-    addProperty(CSSPropertyBorderBottomRightRadius, createPrimitiveValuePair(radii[0][2].release(), radii[1][2].release()), important);
-    addProperty(CSSPropertyBorderBottomLeftRadius, createPrimitiveValuePair(radii[0][3].release(), radii[1][3].release()), important);
+    addProperty(CSSPropertyBorderTopLeftRadius, createCSSValuePair(radii[0][0].release(), radii[1][0].release()), important);
+    addProperty(CSSPropertyBorderTopRightRadius, createCSSValuePair(radii[0][1].release(), radii[1][1].release()), important);
+    addProperty(CSSPropertyBorderBottomRightRadius, createCSSValuePair(radii[0][2].release(), radii[1][2].release()), important);
+    addProperty(CSSPropertyBorderBottomLeftRadius, createCSSValuePair(radii[0][3].release(), radii[1][3].release()), important);
     return true;
 }
 
@@ -5983,7 +5979,7 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseCounter(int defaultValu
             m_valueList->next();
         }
 
-        list->append(createPrimitiveValuePair(counterName.release(),
+        list->append(createCSSValuePair(counterName.release(),
             cssValuePool().createValue(i, CSSPrimitiveValue::UnitType::Number)));
     }
 
@@ -6589,11 +6585,11 @@ bool CSSPropertyParser::parseRadialGradient(CSSParserValueList* valueList, RefPt
         a = args->current();
         if (!a)
             return false;
-        result->setFirstX(toCSSPrimitiveValue(centerX.get()));
-        result->setFirstY(toCSSPrimitiveValue(centerY.get()));
+        result->setFirstX(centerX);
+        result->setFirstY(centerY);
         // Right now, CSS radial gradients have the same start and end centers.
-        result->setSecondX(toCSSPrimitiveValue(centerX.get()));
-        result->setSecondY(toCSSPrimitiveValue(centerY.get()));
+        result->setSecondX(centerX);
+        result->setSecondY(centerY);
     }
 
     if (shapeValue || sizeValue || horizontalSize || centerX || centerY)