CSS Properties and Values API: Support more syntax strings

third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp

Index: third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp
diff --git a/third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp b/third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp
index 3ee259ffbd873bef9329faaacd0d18db14a0decb..106314567314d986a3c18a3bd43b6c2bb82f78f5 100644
--- a/third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp
+++ b/third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp
@@ -8,19 +8,110 @@
 #include "core/css/CSSURIValue.h"
 #include "core/css/CSSValueList.h"
 #include "core/css/CSSVariableReferenceValue.h"
+#include "core/css/parser/CSSParserIdioms.h"
 #include "core/css/parser/CSSPropertyParserHelpers.h"
 #include "core/css/parser/CSSTokenizer.h"
 #include "core/css/parser/CSSVariableParser.h"
+#include "core/html/parser/HTMLParserIdioms.h"
 
 namespace blink {
 
+void consumeWhitespace(const String& string, size_t& offset)
+{
+    while (isHTMLSpace(string[offset]))
+        offset++;
+}
+
+bool consumeCharacterAndWhitespace(const String& string, char character, size_t& offset)
+{
+    if (string[offset] != character)
+        return false;
+    offset++;
+    consumeWhitespace(string, offset);
+    return true;
+}
+
+CSSSyntaxType parseSyntaxType(String type)
+{
+    // TODO(timloh): Are these supposed to be case sensitive?
+    if (type == "length")

[meade_UTC10, 2016/09/14 12:54:13]

Would it be better to use a switch statement here? Then if you do need to make
type lowercase for comparison, you only need to do it once.

[Timothy Loh, 2016/09/19 07:14:14]

C++ doesn't support switch statements over strings. This function is only called
when registering a string so I don't think we need to add any complexity to make
it more efficient.

[Timothy Loh, 2016/09/19 07:14:42]

registering a property*

+        return CSSSyntaxType::Length;
+    if (type == "number")
+        return CSSSyntaxType::Number;
+    if (type == "percentage")
+        return CSSSyntaxType::Percentage;
+    if (type == "length-percentage")
+        return CSSSyntaxType::LengthPercentage;
+    if (type == "color")
+        return CSSSyntaxType::Color;
+    if (type == "image")
+        return CSSSyntaxType::Image;
+    if (type == "url")
+        return CSSSyntaxType::Url;
+    if (type == "integer")
+        return CSSSyntaxType::Integer;
+    if (type == "angle")
+        return CSSSyntaxType::Angle;
+    if (type == "time")
+        return CSSSyntaxType::Time;
+    if (type == "resolution")
+        return CSSSyntaxType::Resolution;
+    if (type == "transform-function")
+        return CSSSyntaxType::TransformFunction;
+    if (type == "custom-ident")
+        return CSSSyntaxType::CustomIdent;
+    // Not an Ident, just used to indicate failure
+    return CSSSyntaxType::Ident;
+}
+
 CSSSyntaxDescriptor::CSSSyntaxDescriptor(String input)
 {
-    // TODO(timloh): Implement proper parsing
-    if (input.contains('*'))
-        m_syntaxComponents.append(CSSSyntaxComponent(CSSSyntaxType::TokenStream));
-    else
-        m_syntaxComponents.append(CSSSyntaxComponent(CSSSyntaxType::Length));
+    size_t offset = 0;
+    consumeWhitespace(input, offset);
+
+    if (consumeCharacterAndWhitespace(input, '*', offset)) {
+        if (offset != input.length())
+            return;
+        m_syntaxComponents.append(CSSSyntaxComponent(CSSSyntaxType::TokenStream, emptyString(), false));
+        return;
+    }
+
+    do {
+        CSSSyntaxType type;
+        String ident;
+
+        if (input[offset] == '<') {
+            offset++;
+            size_t typeStart = offset;
+            while (offset < input.length() && input[offset] != '>')
+                offset++;

[meade_UTC10, 2016/09/14 12:54:13]

This ends up with a lot of nesting, would it be clearer to move these inner
loops into helper functions? e.g. consumeUntilChar, consumeIdent?
Even better, is there a way to use the other parsing helpers here, like
CSSParserToken etc?

[Timothy Loh, 2016/09/19 07:14:14]

I moved some of this logic into helper functions.
No, because the meta-syntax is not CSS (which allows comments everywhere and has
escape processing).

+            type = parseSyntaxType(input.substring(typeStart, offset - typeStart));
+            if (offset == input.length() || type == CSSSyntaxType::Ident) {
+                m_syntaxComponents.clear();
+                return;
+            }
+            offset++;
+        } else {
+            // TODO(timloh): Are CSS-wide keywords allowed here?
+            size_t identStart = offset;
+            while (isNameCodePoint(input[offset]))
+                offset++;
+            if (offset == identStart) {
+                m_syntaxComponents.clear();
+                return;
+            }
+            type = CSSSyntaxType::Ident;
+            ident = input.substring(identStart, offset - identStart);
+        }
+
+        bool repeatable = consumeCharacterAndWhitespace(input, '+', offset);
+        consumeWhitespace(input, offset);
+        m_syntaxComponents.append(CSSSyntaxComponent(type, ident, repeatable));
+
+    } while (consumeCharacterAndWhitespace(input, '|', offset));
+
+    if (offset != input.length())
+        m_syntaxComponents.clear();
 }
 
 const CSSValue* consumeSingleType(const CSSSyntaxComponent& syntax, CSSParserTokenRange& range)
@@ -29,8 +120,40 @@ const CSSValue* consumeSingleType(const CSSSyntaxComponent& syntax, CSSParserTok
 
     // TODO(timloh): Calc values need to be normalized
     switch (syntax.m_type) {
+    case CSSSyntaxType::Ident:
+        if (range.peek().type() == IdentToken
+            && range.peek().value() == syntax.m_string) {
+            range.consumeIncludingWhitespace();
+            return CSSCustomIdentValue::create(AtomicString(syntax.m_string));
+        }
+        return nullptr;
     case CSSSyntaxType::Length:
         return consumeLength(range, HTMLStandardMode, ValueRange::ValueRangeAll);
+    case CSSSyntaxType::Number:
+        return consumeNumber(range, ValueRange::ValueRangeAll);
+    case CSSSyntaxType::Percentage:
+        return consumePercent(range, ValueRange::ValueRangeAll);
+    case CSSSyntaxType::LengthPercentage:
+        return consumeLengthOrPercent(range, HTMLStandardMode, ValueRange::ValueRangeAll);
+    case CSSSyntaxType::Color:
+        return consumeColor(range, HTMLStandardMode);
+    case CSSSyntaxType::Image:
+        // TODO(timloh): This probably needs a proper parser context for relative URL resolution.
+        return consumeImage(range, strictCSSParserContext());
+    case CSSSyntaxType::Url:
+        return consumeUrl(range);
+    case CSSSyntaxType::Integer:
+        return consumeInteger(range);
+    case CSSSyntaxType::Angle:
+        return consumeAngle(range);
+    case CSSSyntaxType::Time:
+        return consumeTime(range, ValueRange::ValueRangeAll);
+    case CSSSyntaxType::Resolution:
+        return nullptr; // TODO(timloh): Implement this.
+    case CSSSyntaxType::TransformFunction:
+        return nullptr; // TODO(timloh): Implement this.
+    case CSSSyntaxType::CustomIdent:
+        return consumeCustomIdent(range);
     default:
         NOTREACHED();
         return nullptr;
@@ -40,6 +163,16 @@ const CSSValue* consumeSingleType(const CSSSyntaxComponent& syntax, CSSParserTok
 const CSSValue* consumeSyntaxComponent(const CSSSyntaxComponent& syntax, CSSParserTokenRange range)
 {
     // CSS-wide keywords are already handled by the CSSPropertyParser
+    if (syntax.m_repeatable) {
+        CSSValueList* list = CSSValueList::createSpaceSeparated();
+        while (!range.atEnd()) {
+            const CSSValue* value = consumeSingleType(syntax, range);
+            if (!value)
+                return nullptr;
+            list->append(*value);
+        }
+        return list;
+    }
     const CSSValue* result = consumeSingleType(syntax, range);
     if (!range.atEnd())
         return nullptr;