[CSS Parser] Make CSSFunctionValue derive from CSSValueList
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 03:24:42 +0000 (03:24 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 03:24:42 +0000 (03:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165832

Reviewed by Dean Jackson.

With the old parser gone, we can now shrink CSSFunctionValue a bit by
having it derive from CSSValueList instead of having an extra member
that holds a value list of arguments. This is similar to the trick
already employed by WebkitCSSTransformValue.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::specifiedValueForGridTrackSize):
* css/CSSFunctionValue.cpp:
(WebCore::CSSFunctionValue::customCSSText):
(WebCore::CSSFunctionValue::CSSFunctionValue): Deleted.
(WebCore::CSSFunctionValue::equals): Deleted.
(WebCore::CSSFunctionValue::append): Deleted.
* css/CSSFunctionValue.h:
* css/StyleBuilderConverter.h:
(WebCore::StyleBuilderConverter::convertScrollSnapPoints):
(WebCore::StyleBuilderConverter::createGridTrackSize):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::createFilterOperations):
* css/parser/CSSPropertyParser.cpp:
(WebCore::isGridTrackFixedSized):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209788 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSFunctionValue.cpp
Source/WebCore/css/CSSFunctionValue.h
Source/WebCore/css/StyleBuilderConverter.h
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/parser/CSSPropertyParser.cpp

index 85661db..1e6965f 100644 (file)
@@ -1,3 +1,31 @@
+2016-12-13  Dave Hyatt  <hyatt@apple.com>
+
+        [CSS Parser] Make CSSFunctionValue derive from CSSValueList
+        https://bugs.webkit.org/show_bug.cgi?id=165832
+
+        Reviewed by Dean Jackson.
+
+        With the old parser gone, we can now shrink CSSFunctionValue a bit by
+        having it derive from CSSValueList instead of having an extra member
+        that holds a value list of arguments. This is similar to the trick
+        already employed by WebkitCSSTransformValue.
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::specifiedValueForGridTrackSize):
+        * css/CSSFunctionValue.cpp:
+        (WebCore::CSSFunctionValue::customCSSText):
+        (WebCore::CSSFunctionValue::CSSFunctionValue): Deleted.
+        (WebCore::CSSFunctionValue::equals): Deleted.
+        (WebCore::CSSFunctionValue::append): Deleted.
+        * css/CSSFunctionValue.h:
+        * css/StyleBuilderConverter.h:
+        (WebCore::StyleBuilderConverter::convertScrollSnapPoints):
+        (WebCore::StyleBuilderConverter::createGridTrackSize):
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::createFilterOperations):
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::isGridTrackFixedSized):
+
 2016-12-13  Chris Dumez  <cdumez@apple.com>
 
         Make Document a FrameDestructionObserver
index 3bc58d0..c8a6ca3 100644 (file)
@@ -1006,19 +1006,19 @@ static Ref<CSSValue> specifiedValueForGridTrackSize(const GridTrackSize& trackSi
     case LengthTrackSizing:
         return specifiedValueForGridTrackBreadth(trackSize.minTrackBreadth(), style);
     case FitContentTrackSizing: {
-        auto fitContentTrackSize = CSSValueList::createCommaSeparated();
+        auto fitContentTrackSize = CSSFunctionValue::create(CSSValueFitContent);
         fitContentTrackSize->append(zoomAdjustedPixelValueForLength(trackSize.fitContentTrackBreadth().length(), style));
-        return CSSFunctionValue::create(CSSValueFitContent, WTFMove(fitContentTrackSize));
+        return WTFMove(fitContentTrackSize);
     }
     default:
         ASSERT(trackSize.type() == MinMaxTrackSizing);
         if (trackSize.minTrackBreadth().isAuto() && trackSize.maxTrackBreadth().isFlex())
             return CSSValuePool::singleton().createValue(trackSize.maxTrackBreadth().flex(), CSSPrimitiveValue::CSS_FR);
 
-        auto minMaxTrackBreadths = CSSValueList::createCommaSeparated();
+        auto minMaxTrackBreadths = CSSFunctionValue::create(CSSValueMinmax);
         minMaxTrackBreadths->append(specifiedValueForGridTrackBreadth(trackSize.minTrackBreadth(), style));
         minMaxTrackBreadths->append(specifiedValueForGridTrackBreadth(trackSize.maxTrackBreadth(), style));
-        return CSSFunctionValue::create(CSSValueMinmax, WTFMove(minMaxTrackBreadths));
+        return WTFMove(minMaxTrackBreadths);
     }
 }
 
index cc5591d..75ef0c1 100644 (file)
 #include "config.h"
 #include "CSSFunctionValue.h"
 
-#include "CSSValueList.h"
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
-
-CSSFunctionValue::CSSFunctionValue(CSSValueID name, Ref<CSSValueList>&& args)
-    : CSSValue(FunctionClass)
-    , m_name(name)
-    , m_args(WTFMove(args))
-{
-}
-
-CSSFunctionValue::CSSFunctionValue(CSSValueID name)
-    : CSSValue(FunctionClass)
-    , m_name(name)
-{
-}
     
 String CSSFunctionValue::customCSSText() const
 {
     StringBuilder result;
-    if (m_name != CSSValueInvalid) {
-        result.append(getValueName(m_name));
-        result.append('(');
-    }
-    if (m_args)
-        result.append(m_args->cssText());
+    result.append(getValueName(m_name));
+    result.append('(');
+    result.append(CSSValueList::customCSSText());
     result.append(')');
     return result.toString();
 }
 
-bool CSSFunctionValue::equals(const CSSFunctionValue& other) const
-{
-    return m_name == other.m_name && compareCSSValuePtr(m_args, other.m_args);
-}
-
-void CSSFunctionValue::append(Ref<CSSValue>&& value)
-{
-    if (!m_args)
-        m_args = CSSValueList::createCommaSeparated();
-    m_args->append(WTFMove(value));
-}
-
 }
index 5803eaf..ebb3d5b 100644 (file)
 
 #pragma once
 
-#include "CSSValue.h"
 #include "CSSValueKeywords.h"
+#include "CSSValueList.h"
 
 namespace WebCore {
 
-class CSSValueList;
-
-// FIXME-NEWPARSER: This can just *be* a CSSValueList subclass.
-class CSSFunctionValue final : public CSSValue {
+class CSSFunctionValue final : public CSSValueList {
 public:
-    static Ref<CSSFunctionValue> create(CSSValueID name, Ref<CSSValueList>&& args)
-    {
-        return adoptRef(*new CSSFunctionValue(name, WTFMove(args)));
-    }
-
     static Ref<CSSFunctionValue> create(CSSValueID name)
     {
         return adoptRef(*new CSSFunctionValue(name));
@@ -47,19 +39,18 @@ public:
     
     String customCSSText() const;
 
-    bool equals(const CSSFunctionValue&) const;
-    
     CSSValueID name() const { return m_name; }
-    CSSValueList* arguments() const { return m_args.get(); }
 
-    void append(Ref<CSSValue>&&);
+    bool equals(const CSSFunctionValue& other) const { return m_name == other.m_name && CSSValueList::equals(other); }
 
 private:
-    CSSFunctionValue(CSSValueID, Ref<CSSValueList>&&);
-    CSSFunctionValue(CSSValueID);
+    CSSFunctionValue(CSSValueID name)
+        : CSSValueList(FunctionClass, CommaSeparator)
+        , m_name(name)
+    {
+    }
 
     CSSValueID m_name { CSSValueInvalid };
-    RefPtr<CSSValueList> m_args;
 };
 
 } // namespace WebCore
index 603b0cb..36764c2 100644 (file)
@@ -818,7 +818,7 @@ inline std::unique_ptr<ScrollSnapPoints> StyleBuilderConverter::convertScrollSna
     for (auto& currentValue : downcast<CSSValueList>(value)) {
         if (is<CSSFunctionValue>(currentValue.get())) {
             auto& functionValue = downcast<CSSFunctionValue>(currentValue.get());
-            points->repeatOffset = parseSnapCoordinate(styleResolver, *functionValue.arguments()->item(0));
+            points->repeatOffset = parseSnapCoordinate(styleResolver, *functionValue.item(0));
             points->hasRepeat = true;
             break;
         }
@@ -914,14 +914,14 @@ inline GridTrackSize StyleBuilderConverter::createGridTrackSize(const CSSValue&
         return GridTrackSize(createGridTrackBreadth(downcast<CSSPrimitiveValue>(value), styleResolver));
 
     ASSERT(is<CSSFunctionValue>(value));
-    CSSValueList& arguments = *downcast<CSSFunctionValue>(value).arguments();
+    const auto& function = downcast<CSSFunctionValue>(value);
 
-    if (arguments.length() == 1)
-        return GridTrackSize(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*arguments.itemWithoutBoundsCheck(0)), styleResolver), FitContentTrackSizing);
+    if (function.length() == 1)
+        return GridTrackSize(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*function.itemWithoutBoundsCheck(0)), styleResolver), FitContentTrackSizing);
 
-    ASSERT_WITH_SECURITY_IMPLICATION(arguments.length() == 2);
-    GridLength minTrackBreadth(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*arguments.itemWithoutBoundsCheck(0)), styleResolver));
-    GridLength maxTrackBreadth(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*arguments.itemWithoutBoundsCheck(1)), styleResolver));
+    ASSERT_WITH_SECURITY_IMPLICATION(function.length() == 2);
+    GridLength minTrackBreadth(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*function.itemWithoutBoundsCheck(0)), styleResolver));
+    GridLength maxTrackBreadth(createGridTrackBreadth(downcast<CSSPrimitiveValue>(*function.itemWithoutBoundsCheck(1)), styleResolver));
     return GridTrackSize(minTrackBreadth, maxTrackBreadth);
 }
 
index ea21e9b..a6fe953 100644 (file)
@@ -1924,23 +1924,22 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
 
         auto& filterValue = downcast<CSSFunctionValue>(currentValue.get());
         FilterOperation::OperationType operationType = filterOperationForType(filterValue.name());
-        auto args = filterValue.arguments();
 
         // Check that all parameters are primitive values, with the
         // exception of drop shadow which has a CSSShadowValue parameter.
         const CSSPrimitiveValue* firstValue = nullptr;
-        if (args && operationType != FilterOperation::DROP_SHADOW) {
+        if (operationType != FilterOperation::DROP_SHADOW) {
             bool haveNonPrimitiveValue = false;
-            for (unsigned j = 0; j < args->length(); ++j) {
-                if (!is<CSSPrimitiveValue>(*args->itemWithoutBoundsCheck(j))) {
+            for (unsigned j = 0; j < filterValue.length(); ++j) {
+                if (!is<CSSPrimitiveValue>(*filterValue.itemWithoutBoundsCheck(j))) {
                     haveNonPrimitiveValue = true;
                     break;
                 }
             }
             if (haveNonPrimitiveValue)
                 continue;
-            if (args->length())
-                firstValue = downcast<CSSPrimitiveValue>(args->itemWithoutBoundsCheck(0));
+            if (filterValue.length())
+                firstValue = downcast<CSSPrimitiveValue>(filterValue.itemWithoutBoundsCheck(0));
         }
 
         switch (operationType) {
@@ -1948,7 +1947,7 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
         case FilterOperation::SEPIA:
         case FilterOperation::SATURATE: {
             double amount = 1;
-            if (args && args->length() == 1) {
+            if (filterValue.length() == 1) {
                 amount = firstValue->doubleValue();
                 if (firstValue->isPercentage())
                     amount /= 100;
@@ -1959,7 +1958,7 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
         }
         case FilterOperation::HUE_ROTATE: {
             double angle = 0;
-            if (args && args->length() == 1)
+            if (filterValue.length() == 1)
                 angle = firstValue->computeDegrees();
 
             operations.operations().append(BasicColorMatrixFilterOperation::create(angle, operationType));
@@ -1970,7 +1969,7 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
         case FilterOperation::CONTRAST:
         case FilterOperation::OPACITY: {
             double amount = (operationType == FilterOperation::BRIGHTNESS) ? 0 : 1;
-            if (args && args->length() == 1) {
+            if (filterValue.length() == 1) {
                 amount = firstValue->doubleValue();
                 if (firstValue->isPercentage())
                     amount /= 100;
@@ -1981,7 +1980,7 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
         }
         case FilterOperation::BLUR: {
             Length stdDeviation = Length(0, Fixed);
-            if (args && args->length() >= 1)
+            if (filterValue.length() >= 1)
                 stdDeviation = convertToFloatLength(firstValue, state.cssToLengthConversionData());
             if (stdDeviation.isUndefined())
                 return false;
@@ -1990,14 +1989,14 @@ bool StyleResolver::createFilterOperations(const CSSValue& inValue, FilterOperat
             break;
         }
         case FilterOperation::DROP_SHADOW: {
-            if (args && args->length() != 1)
+            if (filterValue.length() != 1)
                 return false;
 
-            auto& cssValue = *args->itemWithoutBoundsCheck(0);
+            const auto* cssValue = filterValue.itemWithoutBoundsCheck(0);
             if (!is<CSSShadowValue>(cssValue))
                 continue;
 
-            auto& item = downcast<CSSShadowValue>(cssValue);
+            const auto& item = downcast<CSSShadowValue>(*cssValue);
             int x = item.x->computeLength<int>(state.cssToLengthConversionData());
             int y = item.y->computeLength<int>(state.cssToLengthConversionData());
             IntPoint location(x, y);
index 5f52faf..28f74e6 100644 (file)
@@ -3096,11 +3096,11 @@ static bool isGridTrackFixedSized(const CSSValue& value)
 
     ASSERT(value.isFunctionValue());
     auto& function = downcast<CSSFunctionValue>(value);
-    if (function.name() == CSSValueFitContent || function.arguments() == nullptr || function.arguments()->length() < 2)
+    if (function.name() == CSSValueFitContent || function.length() < 2)
         return false;
 
-    CSSValue* minPrimitiveValue = downcast<CSSPrimitiveValue>(function.arguments()->item(0));
-    CSSValue* maxPrimitiveValue = downcast<CSSPrimitiveValue>(function.arguments()->item(1));
+    const CSSValue* minPrimitiveValue = downcast<CSSPrimitiveValue>(function.item(0));
+    const CSSValue* maxPrimitiveValue = downcast<CSSPrimitiveValue>(function.item(1));
     return isGridTrackFixedSized(*minPrimitiveValue) || isGridTrackFixedSized(*maxPrimitiveValue);
 }