[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..0dc2d69e5f815e2bbe2c0ed263c0b68dade28d77 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,44 @@ private:
     const Member<CSSTokenStreamValue> m_tokenStreamValue;
 };
 
+StringView findVariableName(CSSParserTokenRange& range)
+{
+    range.consumeWhitespace();
+    return range.consume().value();
+}
+
+CSSStyleVariableReferenceValue* variableReferenceValue(StringView variableName, CSSTokenStreamValue* tokenStreamValue)
+{
+    return CSSStyleVariableReferenceValue::create(variableName.toString(), tokenStreamValue);

[meade_UTC10, 2016/08/26 01:29:22]

It's not really worth creating a new function that just contains a single line.
May as well just put this back inside the other function in this case. 

FWIW, I thought it was tidier when the recursing function only dealt with
tokens, and didn't instantiate any objects, so the functions were

StringOrCSSVariableReferenceValue variableReferenceValue(const StringView& name,
HeapVector<StringOrCSSVariableReferenceValue> tokens)
{
  // Instantiates CSSTokenStreamValue value if tokens not empty
  // Instantiates CSSVariableReferenceValue
  // Wraps CSSVariableReferenceValue in StringOr
}

HeapVector<StringOrCSSVariableReferenceValue>
parserTokenRangeToFragments(CSSParserTokenRange range)
{
  // while !atEnd
    // if var
      // find variable name
      // fragments.append(variableReferenceValue(variableName,
parserTokenRangeToFragments(block));
// etc
}

...

For the fromCSSValue case, I think it's fine to return an empty
CSSTokenStreamValue.

[anthonyhkf, 2016/08/26 03:10:28]

Done.

+}
+
+CSSTokenStreamValue* parserTokenRangeToTokenStreamValue(CSSParserTokenRange range)
+{
+    HeapVector<StringOrCSSVariableReferenceValue> fragments;
+    StringBuilder builder;
+    while (!range.atEnd()) {
+        if (range.peek().functionId() == CSSValueVar) {
+            if (!builder.isEmpty()) {
+                fragments.append(StringOrCSSVariableReferenceValue::fromString(builder.toString()));
+                builder.clear();
+            }
+            CSSParserTokenRange current = range.consumeBlock();

[meade_UTC10, 2016/08/26 01:29:22]

Current is not a good variable name, as it is too general. Variable names should
describe what they are, in this case "block" could be a good name (because it
came from consumeBlock, and "range" is already taken).

[anthonyhkf, 2016/08/26 03:10:28]

Done.

+            StringView variableName = findVariableName(current);
+            while (!current.atEnd() && current.consume().type() != CSSParserTokenType::CommaToken) { }

[meade_UTC10, 2016/08/26 01:29:22]

This will do the wrong thing if there isn't a comma. Why not just do

consumeWhitespace()
if (peek().type() == Comma)
  consume()

?

[anthonyhkf, 2016/08/26 03:10:28]

Done.

+            CSSTokenStreamValue* tokenStreamValue = parserTokenRangeToTokenStreamValue(current);
+            fragments.append(StringOrCSSVariableReferenceValue::fromCSSVariableReferenceValue(variableReferenceValue(variableName, tokenStreamValue)));
+        } else {
+            range.consume().serialize(builder);
+        }
+    }
+    if (!builder.isEmpty())
+        fragments.append(StringOrCSSVariableReferenceValue::fromString(builder.toString()));
+    // returns undefined/null if there is no value following the variable's name

[meade_UTC10, 2016/08/26 01:29:22]

We aren't in a var(--foo, fallback) block, so this comment doesn't make sense.
It's also not really useful to just state what the code is doing (which is
returning null if no fragments were found)

[anthonyhkf, 2016/08/26 03:10:29]

Done.

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