[Typed-OM] Get CSSTokenStreamValue from StyleMap

third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp

Index: third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp b/third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp
index fdb62cc2379dda0c5debdeedf8735653af398397..6d511b5162c32c9310f44fa621b548570777b76d 100644
--- a/third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp
+++ b/third_party/WebKit/Source/core/css/cssom/CSSTokenStreamValue.cpp
@@ -4,7 +4,6 @@
 
 #include "core/css/cssom/CSSTokenStreamValue.h"
 
-#include "core/css/CSSVariableReferenceValue.h"
 #include "core/css/cssom/CSSStyleVariableReferenceValue.h"
 #include "core/css/parser/CSSTokenizer.h"
 #include "wtf/text/StringBuilder.h"
@@ -38,6 +37,52 @@ private:
     const Member<CSSTokenStreamValue> m_tokenStreamValue;
 };
 
+HeapVector<StringOrCSSVariableReferenceValue> parserTokenRangeToFragments(CSSParserTokenRange);

[meade_UTC10, 2016/08/25 01:00:01]

You shouldn't need to have so complicated a function calling chain that this is
necessary.

+
+CSSTokenStreamValue* fromParserTokenRangeToCSSTokenStreamValue(const CSSParserTokenRange& range)

[meade_UTC10, 2016/08/25 01:00:02]

Still don't need to have "from" in the method name for conversion functions of
the type classXToClassY.

In any case, this function is unnecessary (and your code simplified) if you keep
CSSParserToken handling strictly inside parserTokenRangeToFragments.

+{
+    const HeapVector<StringOrCSSVariableReferenceValue>& fragments = parserTokenRangeToFragments(range);
+    // return undefined if there is no value following the variable's name

[meade_UTC10, 2016/08/25 01:00:01]

s/undefined/null

[anthonyhkf, 2016/08/25 03:48:31]

Done.

+    if (fragments.size() == 0)
+        return nullptr;
+    return CSSTokenStreamValue::create(fragments);
+}
+
+StringView findVariableName(CSSParserTokenRange& range)
+{
+    while (range.peek().type() != CSSParserTokenType::IdentToken)

[meade_UTC10, 2016/08/25 01:00:01]

You shouldn't need to do this. At most you'd need to consumeWhitespace(). 
Remember that the token stream is already validated by the time it gets here,
and anyway if it wasn't, this would potentially return the wrong thing.

[anthonyhkf, 2016/08/25 03:48:31]

Done.

+        range.consume();
+    return range.consume().value();
+}
+
+CSSStyleVariableReferenceValue* variableReferenceValue(CSSParserTokenRange range)

[meade_UTC10, 2016/08/25 01:00:01]

It would be clearer if this function took (StringView name, HeapVector
fragments) instead of doing more parsing. That way you can simplify your
recursion in parserTokenRangeToFragments.

+{
+    StringView variableName = findVariableName(range);
+    // skip the first comma token
+    while (!range.atEnd() && range.consume().type() != CSSParserTokenType::CommaToken) { }

[meade_UTC10, 2016/08/25 01:00:01]

If there's no comma, this will just skip a bunch of stuff... (e.g. var(--foo))

[anthonyhkf, 2016/08/25 03:48:31]

Hmm.. I think if there's no comma, it will go to range.atEnd(), and then it will
stop (?)

+    CSSTokenStreamValue* tokenStreamValue = fromParserTokenRangeToCSSTokenStreamValue(range);
+    return CSSStyleVariableReferenceValue::create(variableName.toString(), tokenStreamValue);
+}
+
+HeapVector<StringOrCSSVariableReferenceValue> parserTokenRangeToFragments(CSSParserTokenRange range)
+{
+    HeapVector<StringOrCSSVariableReferenceValue> fragments;
+    do {
+        const CSSParserToken* start = &range.peek();
+        while (!range.atEnd() && (range.peek().functionId() != CSSValueVar))
+            range.consume();
+        if (start != &range.peek()) {
+            fragments.append(StringOrCSSVariableReferenceValue::fromString(range.makeSubRange(start, &range.peek()).serialize()));
+        } else if (!range.atEnd()) {
+            if (range.peek().functionId() == CSSValueVar)
+                fragments.append(StringOrCSSVariableReferenceValue::fromCSSVariableReferenceValue(variableReferenceValue(range.consumeBlock())));
+            else
+                NOTREACHED();
+        }
+    } while (!range.atEnd());

[meade_UTC10, 2016/08/25 01:00:01]

This doesn't need to be a do/while loop, nor does it need to have nested loops.
It's easier to see that 

while(!range.atEnd()) {
  //dostuff
}

will just get skipped and return an empty Vector, as opposed to

do {
  while (!range.atEnd()) { ...}
  if (start != &range.peek()) {
    //do stuff
  } else if (!range.atEnd()) {
    // do other stuff
  }
} while (!range.atEnd())

Which also does nothing if the range is empty.



You can simplify to

while(!range.atEnd()) {
  if (functionId == CSSValueVar) {
    // Serialize and clear your StringBuilder
    // Consume block
    // Extract variable name
    // Remove comma if necessary
    // Create CSSVariableReferenceValue, recursing tokenToFragments on consumed
block
  } else {
    // Handle non-variable case
  }
}

[anthonyhkf, 2016/08/25 03:48:31]

Done.

+    return fragments;
+}
+
 } // namespace
 
 ValueIterable<StringOrCSSVariableReferenceValue>::IterationSource* CSSTokenStreamValue::startIteration(ScriptState*, ExceptionState&)
@@ -45,6 +90,11 @@ ValueIterable<StringOrCSSVariableReferenceValue>::IterationSource* CSSTokenStrea
     return new TokenStreamValueIterationSource(this);
 }
 
+CSSTokenStreamValue* CSSTokenStreamValue::fromCSSValue(const CSSVariableReferenceValue& cssVariableReferenceValue)
+{
+    return fromParserTokenRangeToCSSTokenStreamValue(cssVariableReferenceValue.variableDataValue()->tokenRange());
+}
+
 CSSValue* CSSTokenStreamValue::toCSSValue() const
 {
     StringBuilder tokens;