Clean parsing for -webkit-transform-origin*

Source/core/css/parser/CSSPropertyParser.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 1133 matching lines @@
         if (!list)
             return false;
         // These values are added to match gecko serialization.
         if (list->length() == 1)
             list->append(cssValuePool().createValue(50, CSSPrimitiveValue::CSS_PERCENTAGE));
         if (list->length() == 2)
             list->append(cssValuePool().createValue(0, CSSPrimitiveValue::CSS_PX));
         addProperty(propId, list.release(), important);
         return true;
     }
+    case CSSPropertyWebkitTransformOriginX:
+        parsedValue = parseFillPositionX(m_valueList.get());
+        if (parsedValue)
+            m_valueList->next();
+        break;
+    case CSSPropertyWebkitTransformOriginY:
+        parsedValue = parseFillPositionY(m_valueList.get());
+        if (parsedValue)
+            m_valueList->next();
+        break;
+    case CSSPropertyWebkitTransformOriginZ:
+        validPrimitive = validUnit(value, FLength);
+        break;
     case CSSPropertyWebkitTransformOrigin:
-    case CSSPropertyWebkitTransformOriginX:
-    case CSSPropertyWebkitTransformOriginY:
-    case CSSPropertyWebkitTransformOriginZ: {
-        RefPtrWillBeRawPtr<CSSValue> val1 = nullptr;
-        RefPtrWillBeRawPtr<CSSValue> val2 = nullptr;
-        RefPtrWillBeRawPtr<CSSValue> val3 = nullptr;
-        CSSPropertyID propId1, propId2, propId3;
-        if (parseWebkitTransformOrigin(propId, propId1, propId2, propId3, val1, val2, val3)) {
-            addProperty(propId1, val1.release(), important);
-            if (val2)
-                addProperty(propId2, val2.release(), important);
-            if (val3)
-                addProperty(propId3, val3.release(), important);
-            return true;
-        }
-        return false;
-    }
+        return parseWebkitTransformOriginShorthand(important);
     case CSSPropertyPerspective:
         if (id == CSSValueNone) {
             validPrimitive = true;
         } else if (validUnit(value, FLength | FNonNeg)) {
             addProperty(propId, createPrimitiveNumericValue(value), important);
             return true;
         }
         break;
     case CSSPropertyWebkitPerspective:
         if (id == CSSValueNone) {
@@ skipping 1945 matching lines @@
     CSSPropertyID result = cssPropertyID(value->string);
     if (result && RuntimeCSSEnabled::isCSSPropertyEnabled(result))
         return cssValuePool().createIdentifierValue(result);
     if (equalIgnoringCase(value, "none"))
         return cssValuePool().createIdentifierValue(CSSValueNone);
     if (equalIgnoringCase(value, "inherit") || equalIgnoringCase(value, "initial"))
         return nullptr;
     return createPrimitiveStringValue(value);
 }
 
-bool CSSPropertyParser::parseWebkitTransformOriginShorthand(RefPtrWillBeRawPtr<CSSValue>& value1, RefPtrWillBeRawPtr<CSSValue>& value2, RefPtrWillBeRawPtr<CSSValue>& value3)
+bool CSSPropertyParser::parseWebkitTransformOriginShorthand(bool important)
 {
-    parse2ValuesFillPosition(m_valueList.get(), value1, value2);
+    RefPtrWillBeRawPtr<CSSValue> originX;
+    RefPtrWillBeRawPtr<CSSValue> originY;
+    parse2ValuesFillPosition(m_valueList.get(), originX, originY);
 
-    // now get z
+    if (m_valueList->current() && !validUnit(m_valueList->current(), FLength))
+        return false;

[alancutter (OOO until 2018), 2014/08/15 01:00:03]

This can be merged into the if block below.

[Timothy Loh, 2014/08/15 01:03:18]

Not sure what you mean here, the "if (m_valueList->current())" branch below
happens after we've already added properties. Not adding properties at all seems
like a better pattern than adding then rolling back.

[Timothy Loh, 2014/08/15 01:11:41]

err yeah I can move adding properties lower, will do that :|

[Timothy Loh, 2014/08/15 01:26:41]

Done.

+
+    addProperty(CSSPropertyWebkitTransformOriginX, originX.release(), important);
+    addProperty(CSSPropertyWebkitTransformOriginY, originY.release(), important);
+
+    RefPtrWillBeRawPtr<CSSValue> originZ;
     if (m_valueList->current()) {
-        if (validUnit(m_valueList->current(), FLength)) {
-            value3 = createPrimitiveNumericValue(m_valueList->current());
-            m_valueList->next();
-            return true;
-        }
-        return false;
+        originZ = createPrimitiveNumericValue(m_valueList->current());
+        m_valueList->next();
+    } else {
+        originZ = cssValuePool().createImplicitInitialValue();
     }
-    value3 = cssValuePool().createImplicitInitialValue();
+    addProperty(CSSPropertyWebkitTransformOriginZ, originZ.release(), important);
+
     return true;
 }
 
 bool CSSPropertyParser::parseCubicBezierTimingFunctionValue(CSSParserValueList*& args, double& result)
 {
     CSSParserValue* v = args->current();
     if (!validUnit(v, FNumber))
         return false;
     result = v->fValue;
     v = args->next();
@@ skipping 4394 matching lines @@
 
     RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
     list->append(xValue.release());
     if (yValue)
         list->append(yValue.release());
     if (zValue)
         list->append(zValue.release());
     return list.release();
 }
 
-bool CSSPropertyParser::parseWebkitTransformOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, CSSPropertyID& propId3, RefPtrWillBeRawPtr<CSSValue>& value, RefPtrWillBeRawPtr<CSSValue>& value2, RefPtrWillBeRawPtr<CSSValue>& value3)
-{
-    propId1 = propId;
-    propId2 = propId;
-    propId3 = propId;
-    if (propId == CSSPropertyWebkitTransformOrigin) {
-        propId1 = CSSPropertyWebkitTransformOriginX;
-        propId2 = CSSPropertyWebkitTransformOriginY;
-        propId3 = CSSPropertyWebkitTransformOriginZ;
-    }
-
-    switch (propId) {
-        case CSSPropertyWebkitTransformOrigin:
-            if (!parseWebkitTransformOriginShorthand(value, value2, value3))
-                return false;
-            // parseWebkitTransformOriginShorthand advances the m_valueList pointer
-            break;
-        case CSSPropertyWebkitTransformOriginX: {
-            value = parseFillPositionX(m_valueList.get());
-            if (value)
-                m_valueList->next();
-            break;
-        }
-        case CSSPropertyWebkitTransformOriginY: {
-            value = parseFillPositionY(m_valueList.get());
-            if (value)
-                m_valueList->next();
-            break;
-        }
-        case CSSPropertyWebkitTransformOriginZ: {
-            if (validUnit(m_valueList->current(), FLength))
-                value = createPrimitiveNumericValue(m_valueList->current());
-            if (value)
-                m_valueList->next();
-            break;
-        }
-        default:
-            ASSERT_NOT_REACHED();
-            return false;
-    }
-
-    return value;
-}
-
 bool CSSPropertyParser::parseWebkitPerspectiveOrigin(CSSPropertyID propId, CSSPropertyID& propId1, CSSPropertyID& propId2, RefPtrWillBeRawPtr<CSSValue>& value, RefPtrWillBeRawPtr<CSSValue>& value2)
 {
     propId1 = propId;
     propId2 = propId;
     if (propId == CSSPropertyWebkitPerspectiveOrigin) {
         propId1 = CSSPropertyWebkitPerspectiveOriginX;
         propId2 = CSSPropertyWebkitPerspectiveOriginY;
     }
 
     switch (propId) {
@@ skipping 914 matching lines @@
         parsedValues->append(CSSPrimitiveValue::createIdentifier(CSSValueFill));
     if (!seenStroke)
         parsedValues->append(CSSPrimitiveValue::createIdentifier(CSSValueStroke));
     if (!seenMarkers)
         parsedValues->append(CSSPrimitiveValue::createIdentifier(CSSValueMarkers));
 
     return parsedValues.release();
 }
 
 } // namespace blink