Move cursor property into CSSPropertyParser

third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSCalculationValue.h"
+#include "core/css/CSSCursorImageValue.h"
 #include "core/css/CSSCustomIdentValue.h"
 #include "core/css/CSSFontFaceSrcValue.h"
 #include "core/css/CSSFontFeatureValue.h"
 #include "core/css/CSSFunctionValue.h"
+#include "core/css/CSSImageSetValue.h"
 #include "core/css/CSSPathValue.h"
 #include "core/css/CSSPrimitiveValueMappings.h"
 #include "core/css/CSSQuadValue.h"
 #include "core/css/CSSSVGDocumentValue.h"
 #include "core/css/CSSShadowValue.h"
 #include "core/css/CSSStringValue.h"
 #include "core/css/CSSTimingFunctionValue.h"
 #include "core/css/CSSURIValue.h"
 #include "core/css/CSSUnicodeRangeValue.h"
 #include "core/css/CSSValuePair.h"
@@ skipping 1813 matching lines @@
 }
 
 static PassRefPtrWillBeRawPtr<CSSValue> consumeFlexBasis(CSSParserTokenRange& range, CSSParserMode cssParserMode)
 {
     // FIXME: Support intrinsic dimensions too.
     if (range.peek().id() == CSSValueAuto)
         return consumeIdent(range);
     return consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
 }
 
+PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::consumeImageSet(CSSParserTokenRange functionArgs)
+{
+    if (functionArgs.atEnd())
+        return nullptr;
+
+    RefPtrWillBeRawPtr<CSSImageSetValue> imageSet = CSSImageSetValue::create();
+
+    while (!functionArgs.atEnd()) {
+        AtomicString urlValue(consumeUrl(functionArgs));
+        if (urlValue.isNull())
+            return nullptr;
+
+        RefPtrWillBeRawPtr<CSSValue> image = createCSSImageValueWithReferrer(urlValue, completeURL(urlValue));
+        imageSet->append(image);
+
+        const CSSParserToken& token = functionArgs.consumeIncludingWhitespace();
+
+        if (token.type() != DimensionToken)
+            return nullptr;
+        ASSERT(token.unitType() == CSSPrimitiveValue::UnitType::Unknown);

[Timothy Loh, 2015/11/20 04:09:07]

This assertion doesn't look right, since we haven't actually checked the unit is
'x' until the line below.

[rwlbuis, 2015/11/20 20:00:21]

Done.

+        if (String(token.value()) != "x")
+            return nullptr;
+        double imageScaleFactor = token.numericValue();
+        if (imageScaleFactor <= 0)
+            return nullptr;
+        imageSet->append(cssValuePool().createValue(imageScaleFactor, CSSPrimitiveValue::UnitType::Number));
+
+        if (functionArgs.atEnd())
+            break;
+
+        if (!consumeCommaIncludingWhitespace(functionArgs))
+            return nullptr;
+    }
+    return imageSet.release();
+}
+
+PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::consumeCursor(CSSParserTokenRange& range)
+{
+    RefPtrWillBeRawPtr<CSSValueList> list = nullptr;
+    while (!range.atEnd()) {
+        RefPtrWillBeRawPtr<CSSValue> image = nullptr;
+        AtomicString uri(consumeUrl(range));
+        if (!uri.isNull()) {
+            image = createCSSImageValueWithReferrer(uri, completeURL(uri));
+        } else if (range.peek().type() == FunctionToken && range.peek().functionId() == CSSValueWebkitImageSet) {
+            image = consumeImageSet(consumeFunction(range));

[Timothy Loh, 2015/11/20 04:09:07]

Doesn't this mean that any invalid -webkit-image-set(..) is just dropped instead
of making us fail parsing? Maybe we should move the consumeFunction call to
consumeImageSet.

[rwlbuis, 2015/11/20 20:00:21]

I think I see what you mean. I guess we really need to return nullptr if
consumeImageSet fails. But as you said that would be a behavior change.

[Timothy Loh, 2015/12/02 23:59:10]

Err this wouldn't be a behavior change, currently we don't consume anything for
an invalid -webkit-image-set. We could make it just return nullptr if
consumeImageSet fails, since at that point we'll never succeed anyway, but I'm
not sure if that'll be good for composition in consumeImage.

+            if (!image)
+                break;
+        } else {
+            break;
+        }
+
+        double num;
+        IntPoint hotSpot(-1, -1);
+        bool hotSpotSpecified = false;
+        if (consumeNumberRaw(range, num)) {
+            hotSpot.setX(int(num));
+            if (!consumeNumberRaw(range, num))
+                return nullptr;
+            hotSpot.setY(int(num));
+            hotSpotSpecified = true;
+        }
+
+        if (!list)
+            list = CSSValueList::createCommaSeparated();
+
+        if (image)

[Timothy Loh, 2015/11/20 04:09:07]

looks like this is always true

[rwlbuis, 2015/11/20 20:00:21]

Done.

+            list->append(CSSCursorImageValue::create(image, hotSpotSpecified, hotSpot));
+
+        if (!consumeCommaIncludingWhitespace(range))
+            return nullptr;
+    }

[Timothy Loh, 2015/11/20 04:09:06]

blank line after here might be nice.

[rwlbuis, 2015/11/20 20:00:20]

Done.

+    CSSValueID id = range.peek().id();
+    if (!range.atEnd() && m_context.useCounter()) {
+        if (id == CSSValueWebkitZoomIn)
+            m_context.useCounter()->count(UseCounter::PrefixedCursorZoomIn);
+        else if (id == CSSValueWebkitZoomOut)
+            m_context.useCounter()->count(UseCounter::PrefixedCursorZoomOut);
+    }
+    if (list) {

[Timothy Loh, 2015/11/20 04:09:07]

Can we remove the copy-paste and fix the allowing random junk here? e.g.

RefPtrWillBeRawPtr<CSSValue> cursorType;
if (inQuirksMode() && id == CSSValueHand) {
    // Non-standard behavior
    cursorType = cssValuePool().createIdentifierValue(CSSValuePointer);
    range.consumeIncludingWhitespace();
} else if ((id >= CSSValueAuto && id <= CSSValueWebkitZoomOut) || id ==
CSSValueCopy || id == CSSValueNone) {
    cursorType = consumeIdent(range);
} else {
    return nullptr;
}

if (!list)
    return cursorType.release();
list->append(cursorType.release());
return list.release();

[rwlbuis, 2015/11/20 20:00:21]

Done.

+        if (range.atEnd())
+            return nullptr;
+        id = range.consumeIncludingWhitespace().id();
+        if (inQuirksMode() && id == CSSValueHand) // MSIE 5 compatibility :/
+            list->append(cssValuePool().createIdentifierValue(CSSValuePointer));
+        else if ((id >= CSSValueAuto && id <= CSSValueWebkitZoomOut) || id == CSSValueCopy || id == CSSValueNone)
+            list->append(cssValuePool().createIdentifierValue(id));
+        return list.release();

[Timothy Loh, 2015/11/20 04:09:06]

If you didn't notice, we accept "cursor: url(a), bla" with this (and the
current) code, this is the "random junk" I mentioned above.

[Timothy Loh, 2015/11/20 06:56:25]

On the other hand, maybe it's better to keep behaviour changes like this
separate? In any case, I think we should remove the copy-paste.

[rwlbuis, 2015/11/20 20:00:20]

I kept the behavior as-is but removed the copy-paste.

+    }
+    if (!range.atEnd()) {
+        id = range.consumeIncludingWhitespace().id();
+        if (inQuirksMode() && id == CSSValueHand) // MSIE 5 compatibility :/
+            return cssValuePool().createIdentifierValue(CSSValuePointer);
+        if ((id >= CSSValueAuto && id <= CSSValueWebkitZoomOut) || id == CSSValueCopy || id == CSSValueNone)
+            return cssValuePool().createIdentifierValue(id);
+        return nullptr;
+    }
+    ASSERT_NOT_REACHED();
+    return nullptr;
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID unresolvedProperty)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
     m_range.consumeWhitespace();
     switch (property) {
     case CSSPropertyWillChange:
         return consumeWillChange(m_range);
     case CSSPropertyPage:
         return consumePage(m_range);
     case CSSPropertyQuotes:
@@ skipping 145 matching lines @@
         return consumePaintOrder(m_range);
     case CSSPropertyMarkerStart:
     case CSSPropertyMarkerMid:
     case CSSPropertyMarkerEnd:
         return consumeNoneOrURI(m_range);
     case CSSPropertyFlexBasis:
         return consumeFlexBasis(m_range, m_context.mode());
     case CSSPropertyFlexGrow:
     case CSSPropertyFlexShrink:
         return consumeNumber(m_range, ValueRangeNonNegative);
+    case CSSPropertyCursor:
+        return consumeCursor(m_range);
     default:
         return nullptr;
     }
 }
 
 static PassRefPtrWillBeRawPtr<CSSValueList> consumeFontFaceUnicodeRange(CSSParserTokenRange& range)
 {
     RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
 
     do {
@@ skipping 545 matching lines @@
         return consumeFlex(important);
     case CSSPropertyFlexFlow:
         return consumeShorthandGreedily(flexFlowShorthand(), important);
     default:
         m_currentShorthand = oldShorthand;
         return false;
     }
 }
 
 } // namespace blink