Avoid use of CSSValue in MediaQueryExp and MediaQueryEvaluator

Source/core/css/MediaQueryExp.cpp

 /*
  * CSS Media Query
  *
  * Copyright (C) 2006 Kimmo Kinnunen <kimmo.t.kinnunen@nokia.com>.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
  * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
@@ skipping 12 matching lines @@
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/css/MediaQueryExp.h"
 
-#include "CSSValueKeywords.h"
 #include "core/css/CSSAspectRatioValue.h"
 #include "core/css/CSSParserValues.h"
 #include "core/css/CSSPrimitiveValue.h"
 #include "core/html/parser/HTMLParserIdioms.h"
+#include "wtf/DecimalNumber.h"
+#include "wtf/text/StringBuffer.h"
 #include "wtf/text/StringBuilder.h"
 
 namespace WebCore {
 
 using namespace MediaFeatureNames;
 
 static inline bool featureWithCSSValueID(const String& mediaFeature, const CSSParserValue* value)
 {
     if (!value->id)
         return false;
@@ skipping 146 matching lines @@
         || m_mediaFeature == orientationMediaFeature
         || m_mediaFeature == aspectRatioMediaFeature
         || m_mediaFeature == minAspectRatioMediaFeature
         || m_mediaFeature == devicePixelRatioMediaFeature
         || m_mediaFeature == resolutionMediaFeature
         || m_mediaFeature == maxAspectRatioMediaFeature;
 }
 
 MediaQueryExp::MediaQueryExp(const MediaQueryExp& other)
     : m_mediaFeature(other.mediaFeature())
-    , m_value(other.value())
+    , m_expValue(other.expValue())
 {
 }
 
-MediaQueryExp::MediaQueryExp(const String& mediaFeature, PassRefPtrWillBeRawPtr<CSSValue> value)
+MediaQueryExp::MediaQueryExp(const String& mediaFeature, const MediaQueryExpValue& expValue)
     : m_mediaFeature(mediaFeature)
-    , m_value(value)
+    , m_expValue(expValue)
 {
 }
 
 PassOwnPtrWillBeRawPtr<MediaQueryExp> MediaQueryExp::createIfValid(const String& mediaFeature, CSSParserValueList* valueList)
 {
     ASSERT(!mediaFeature.isNull());
 
-    // FIXME - Creation of CSSValue here may not be thread safe.
-    // It should be replaced by a different way to pass the values to MediaQueryEvaluator.
-    RefPtrWillBeRawPtr<CSSValue> cssValue = nullptr;
+    MediaQueryExpValue expValue;
     bool isValid = false;
     String lowerMediaFeature = attemptStaticStringCreation(mediaFeature.lower());
 
     // Create value for media query expression that must have 1 or more values.
     if (valueList && valueList->size() > 0) {
         if (valueList->size() == 1) {
             CSSParserValue* value = valueList->current();
             ASSERT(value);
 
-            if (featureWithCSSValueID(lowerMediaFeature, value)) {
+            if (featureWithCSSValueID(lowerMediaFeature, value) && featureWithValidIdent(lowerMediaFeature, value->id)) {
                 // Media features that use CSSValueIDs.
-                cssValue = CSSPrimitiveValue::createIdentifier(value->id);
-                if (!featureWithValidIdent(lowerMediaFeature, toCSSPrimitiveValue(cssValue.get())->getValueID()))
-                    cssValue.clear();
-            } else if (featureWithValidDensity(lowerMediaFeature, value)) {
-                // Media features that must have non-negative <density>, ie. dppx, dpi or dpcm.
-                cssValue = CSSPrimitiveValue::create(value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit);
-            } else if (featureWithValidPositiveLength(lowerMediaFeature, value)) {
-                // Media features that must have non-negative <lenght> or number value.
-                cssValue = CSSPrimitiveValue::create(value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit);
-            } else if (featureWithPositiveInteger(lowerMediaFeature, value)) {
-                // Media features that must have non-negative integer value.
-                cssValue = CSSPrimitiveValue::create(value->fValue, CSSPrimitiveValue::CSS_NUMBER);
-            } else if (featureWithPositiveNumber(lowerMediaFeature, value)) {
-                // Media features that must have non-negative number value.
-                cssValue = CSSPrimitiveValue::create(value->fValue, CSSPrimitiveValue::CSS_NUMBER);
-            } else if (featureWithZeroOrOne(lowerMediaFeature, value)) {
-                // Media features that must have (0|1) value.
-                cssValue = CSSPrimitiveValue::create(value->fValue, CSSPrimitiveValue::CSS_NUMBER);
+                expValue.id = value->id;
+                expValue.unit = CSSPrimitiveValue::CSS_VALUE_ID;
+                expValue.isID = true;
+            } else if (featureWithValidDensity(lowerMediaFeature, value)
+                || featureWithValidPositiveLength(lowerMediaFeature, value)) {
+                // Media features that must have non-negative <density>, ie. dppx, dpi or dpcm,
+                // or Media features that must have non-negative <length> or number value.
+                expValue.value = value->fValue;
+                expValue.unit = value->unit;
+                expValue.isValue = true;
+                expValue.isInteger = value->isInt;
+            } else if (featureWithPositiveInteger(lowerMediaFeature, value)
+                || featureWithPositiveNumber(lowerMediaFeature, value)
+                || featureWithZeroOrOne(lowerMediaFeature, value)) {
+                // Media features that must have non-negative integer value,
+                // or media features that must have non-negative number value,
+                // or media features that must have (0|1) value.
+                expValue.value = value->fValue;
+                expValue.unit = CSSPrimitiveValue::CSS_NUMBER;
+                expValue.isValue = true;
+                expValue.isInteger = value->isInt;
             }
 
-            isValid = cssValue;
+            isValid = (expValue.isID || expValue.isValue);
 
         } else if (valueList->size() == 3 && featureWithAspectRatio(lowerMediaFeature)) {
             // Create list of values.
             // Currently accepts only <integer>/<integer>.
             // Applicable to device-aspect-ratio and aspec-ratio.
             isValid = true;
             float numeratorValue = 0;
             float denominatorValue = 0;
             // The aspect-ratio must be <integer> (whitespace)? / (whitespace)? <integer>.
             for (unsigned i = 0; i < 3; ++i, valueList->next()) {
                 const CSSParserValue* value = valueList->current();
                 if (i != 1 && value->unit == CSSPrimitiveValue::CSS_NUMBER && value->fValue > 0 && value->isInt) {
                     if (!i)
                         numeratorValue = value->fValue;
                     else
                         denominatorValue = value->fValue;
                 } else if (i == 1 && value->unit == CSSParserValue::Operator && value->iValue == '/') {
                     continue;
                 } else {
                     isValid = false;
                     break;
                 }
             }
 
-            if (isValid)
-                cssValue = CSSAspectRatioValue::create(numeratorValue, denominatorValue);
+            if (isValid) {
+                expValue.numerator = (unsigned)numeratorValue;
+                expValue.denominator = (unsigned)denominatorValue;
+                expValue.isRatio = true;
+            }
         }
     } else if (featureWithoutValue(lowerMediaFeature)) {
         isValid = true;
     }
 
     if (!isValid)
         return nullptr;
 
-    return adoptPtrWillBeNoop(new MediaQueryExp(lowerMediaFeature, cssValue));
+    return adoptPtrWillBeNoop(new MediaQueryExp(lowerMediaFeature, expValue));
 }
 
 MediaQueryExp::~MediaQueryExp()
 {
 }
 
+bool MediaQueryExp::operator==(const MediaQueryExp& other) const
+{
+    return (other.m_mediaFeature == m_mediaFeature)
+        && ((!other.m_expValue.set() && !m_expValue.set())
+            || (other.m_expValue.set() && m_expValue.set() && other.m_expValue.equals(m_expValue)));
+}
+
 String MediaQueryExp::serialize() const
 {
     StringBuilder result;
     result.append("(");
     result.append(m_mediaFeature.lower());
-    if (m_value) {
+    if (m_expValue.set()) {
         result.append(": ");
-        result.append(m_value->cssText());
+        result.append(m_expValue.cssText());
     }
     result.append(")");
 
     return result.toString();
 }
 
+static String unitToString(unsigned short unit)

[eseidel, 2014/04/22 16:22:23]

I wonder if this doesn't belong on CSSPrimitiveValue itself?

+{
+    switch (unit) {
+    case CSSPrimitiveValue::CSS_PERCENTAGE:
+        return String("%");
+    case CSSPrimitiveValue::CSS_EMS:
+        return String("em");
+    case CSSPrimitiveValue::CSS_EXS:
+        return String("ex");
+    case CSSPrimitiveValue::CSS_REMS:
+        return String("rem");
+    case CSSPrimitiveValue::CSS_CHS:
+        return String("ch");
+    case CSSPrimitiveValue::CSS_PX:
+        return String("px");
+    case CSSPrimitiveValue::CSS_CM:
+        return String("cm");
+    case CSSPrimitiveValue::CSS_DPPX:
+        return String("dppx");
+    case CSSPrimitiveValue::CSS_DPI:
+        return String("dpi");
+    case CSSPrimitiveValue::CSS_DPCM:
+        return String("dpcm");
+    }
+    return String();
+}
+
+static String printNumber(double number)
+{
+    DecimalNumber decimal(number);
+    StringBuffer<LChar> buffer(decimal.bufferLengthForStringDecimal());
+    decimal.toStringDecimal(buffer.characters(), buffer.length());
+    return String::adopt(buffer);
+}
+
+String MediaQueryExpValue::cssText() const
+{
+    StringBuilder output;
+    if (isValue) {
+        output.append(printNumber(value));
+        output.append(unitToString(unit));
+    } else if (isRatio) {
+        output.append(printNumber(numerator));
+        output.append("/");
+        output.append(printNumber(denominator));
+    } else if (isID) {
+        output.append(getValueName(id));
+    }
+
+    return output.toString();
+}
+
 } // namespace