Move text-shadow/box-shadow properties 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/CSSCustomIdentValue.h"
 #include "core/css/CSSFontFaceSrcValue.h"
 #include "core/css/CSSFontFeatureValue.h"
 #include "core/css/CSSPrimitiveValueMappings.h"
 #include "core/css/CSSQuadValue.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"
 #include "core/css/CSSValuePool.h"
 #include "core/css/FontFace.h"
 #include "core/css/parser/CSSParserFastPaths.h"
 #include "core/css/parser/CSSParserValues.h"
 #include "core/frame/UseCounter.h"
@@ skipping 1300 matching lines @@
 }
 
 static PassRefPtrWillBeRawPtr<CSSValue> consumeWidowsOrOrphans(CSSParserTokenRange& range)
 {
     // Support for auto is non-standard and for backwards compatibility.
     if (range.peek().id() == CSSValueAuto)
         return consumeIdent(range);
     return consumePositiveInteger(range);
 }
 
+static PassRefPtrWillBeRawPtr<CSSShadowValue> parseSingleShadow(CSSParserTokenRange& range, const CSSParserContext& context, bool allowInset, bool allowSpread)
+{
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> style = nullptr;
+    RefPtrWillBeRawPtr<CSSValue> color = nullptr;
+    WillBeHeapVector<RefPtrWillBeMember<CSSPrimitiveValue>, 4> lengths;

[Timothy Loh, 2015/10/29 02:53:27]

Probably nicer to just have four separate variables horizontalOffset,
verticalOffset, blurRadius, spreadDistance.

[rwlbuis, 2015/10/29 20:48:14]

That actually makes the code much more compact, done.

+
+    if (range.atEnd())
+        return nullptr;
+    if (range.peek().id() == CSSValueInset) {
+        if (!allowInset)
+            return nullptr;
+        style = consumeIdent(range);
+    }
+    color = consumeColor(range, context);
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> length = consumeLength(range, context.mode(), ValueRangeAll);
+    if (!length)
+        return nullptr;
+    lengths.append(length.release());
+
+    length = consumeLength(range, context.mode(), ValueRangeAll);
+    if (!length)
+        return nullptr;
+    lengths.append(length.release());
+
+    length = consumeLength(range, context.mode(), ValueRangeAll);
+    if (length) {
+        // Blur radius must be non-negative.
+        if (length->getDoubleValue() < 0)
+            return nullptr;
+        lengths.append(length.release());
+        length = consumeLength(range, context.mode(), ValueRangeAll);
+        if (length) {
+            if (!allowSpread)

[Timothy Loh, 2015/10/29 02:53:26]

I think it makes more sense to not try and consumeLength at all if allowSpread
is false

[rwlbuis, 2015/10/29 20:48:14]

Done.

+                return nullptr;
+            lengths.append(length.release());
+        }
+    }
+
+    if (!range.atEnd()) {
+        if (RefPtrWillBeRawPtr<CSSValue> colorValue = consumeColor(range, context)) {

[Timothy Loh, 2015/10/29 02:53:27]

Can't we write:
if (!color)
    color = consumeColor(..);

[rwlbuis, 2015/10/29 20:48:14]

Done.

+            if (color)
+                return nullptr;
+            color = colorValue;
+        }
+        if (range.peek().id() == CSSValueInset) {
+            if (!allowInset || style)
+                return nullptr;
+            style = consumeIdent(range);
+        }
+    }
+    unsigned lengthsSeen = lengths.size();
+    return CSSShadowValue::create(lengths.at(0), lengths.at(1), lengthsSeen > 2 ? lengths.at(2) : nullptr,
+        lengthsSeen > 3 ? lengths.at(3) : nullptr, style.release(), color.release());
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeShadow(CSSParserTokenRange& range, const CSSParserContext& context, bool isBoxShadowProperty)
+{
+    if (range.peek().id() == CSSValueNone)
+        return consumeIdent(range);
+
+    RefPtrWillBeRawPtr<CSSValueList> shadowValueList = CSSValueList::createCommaSeparated();
+    do {
+        if (RefPtrWillBeRawPtr<CSSShadowValue> shadowValue = parseSingleShadow(range, context, isBoxShadowProperty, isBoxShadowProperty))

[Timothy Loh, 2015/10/29 02:53:26]

Doesn't this allow extraneous commas everywhere?
text-shadow: <shadow>,,,,,
text-shadow: ,,,<shadow>,,

I think we can just put an else clause which returns nullptr here (this removes
the need for an empty list check later).

[rwlbuis, 2015/10/29 20:48:14]

Ouch! Fixed now.
I noticed box-shadow-interpolation.html using one comma at the end in two cases,
I assume this was an error (Firefox sees this as invalid) so I fixed the test,
adding alancutter to check if the original ending comma's were intended.
Done.

+            shadowValueList->append(shadowValue.release());
+    } while (consumeCommaIncludingWhitespace(range));
+    if (shadowValueList->length() == 0)
+        return nullptr;
+    return shadowValueList;
+}
+
 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 82 matching lines @@
     case CSSPropertyTransitionTimingFunction:
         return consumeAnimationPropertyList(property, m_range, m_context, unresolvedProperty == CSSPropertyAliasWebkitAnimationName);
     case CSSPropertyOrphans:
     case CSSPropertyWidows:
         return consumeWidowsOrOrphans(m_range);
     case CSSPropertyWebkitTextFillColor:
     case CSSPropertyWebkitTapHighlightColor:
         return consumeColor(m_range, m_context);
     case CSSPropertyColor:
         return consumeColor(m_range, m_context, inQuirksMode());
+    case CSSPropertyTextShadow: // CSS2 property, dropped in CSS2.1, back in CSS3, so treat as CSS3
+    case CSSPropertyBoxShadow:
+        return consumeShadow(m_range, m_context, property == CSSPropertyBoxShadow);
     default:
         return nullptr;
     }
 }
 
 static PassRefPtrWillBeRawPtr<CSSValueList> consumeFontFaceUnicodeRange(CSSParserTokenRange& range)
 {
     RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
 
     do {
@@ skipping 410 matching lines @@
         m_currentShorthand = oldShorthand;
         return consumeColumns(important);
     }
     default:
         m_currentShorthand = oldShorthand;
         return false;
     }
 }
 
 } // namespace blink