[css-typed-om] Normalize CSS variables in property values.

When we specify CSS variables in a property value (e.g. color: var(--A)) we need to return a CSSUnparsedValue. This patch implements this by first trying to parse the property value according to the property's grammar. If it fails, we then check if it has var references and parse it as a variable reference. The code for converting a variable reference to a CSSUnparsedValue was already in the codebase so we just hook into that. Bug: 788570 Change-Id: I65b32d29e80918c61e1731e7a0424b72e7d9b1d7 Reviewed-on: https://chromium-review.googlesource.com/804980 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#521955}

[css-typed-om] Normalize CSS variables in property values.
When we specify CSS variables in a property value (e.g. color: var(--A)) we need to return a CSSUnparsedValue. This patch implements this by first trying to parse the property value according to the property's grammar. If it fails, we then check if it has var references and parse it as a variable reference. The code for converting a variable reference to a CSSUnparsedValue was already in the codebase so we just hook into that. Bug: 788570 Change-Id: I65b32d29e80918c61e1731e7a0424b72e7d9b1d7 Reviewed-on: https://chromium-review.googlesource.com/804980 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#521955}
0365acdc · Darren Shen · Commit Bot · 04349b4f · 0365acdc · 0365acdc
Commit 0365acdc authored Dec 06, 2017 by Darren Shen Committed by Commit Bot Dec 06, 2017
2 changed files
--- a/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-tokens.html
+++ b/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-tokens.html
+<!doctype html>
+<meta charset="utf-8">
+<title>Normalization of raw CSS tokens tests</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#normalize-tokens">
+<script src="../..//resources/testharness.js"></script>
+<script src="../..//resources/testharnessreport.js"></script>
+<script src="../resources/testhelper.js"></script>
+<script>
+'use strict';
+
+const gTestCases = [
+  {
+    value: 'var(--A)',
+    expectedResult: [
+      createReferenceValue('--A', null),
+    ]
+  },
+  {
+    value: 'var(--A, 1em)',
+    expectedResult: [
+      createReferenceValue('--A', new CSSUnparsedValue(' 1em')),
+    ]
+  },
+  {
+    value: 'var(--A, var(--B))',
+    expectedResult: [
+      createReferenceValue('--A', new CSSUnparsedValue(' ', createReferenceValue('--B', null))),
+    ]
+  },
+  {
+    value: 'calc(42px + var(--foo, 15em) + var(--bar, var(--far) + 15px))',
+    expectedResult: [
+      'calc(42px +',
+      createReferenceValue('--foo', new CSSUnparsedValue(' 15em')),
+      ' + ',
+      createReferenceValue('--bar', new CSSUnparsedValue(' ', createReferenceValue('--far', null), ' + 15px')),
+      ')',
+    ]
+  },
+];
+
+for (const {value, expectedResult} of gTestCases) {
+  // TODO(788570): Add similar tests using a custom property instead of 'color'
+  test(() => {
+    const result = CSSStyleValue.parse('color', value);
+    assert_style_value_equals(result, new CSSUnparsedValue(...expectedResult));
+  }, 'Normalizing "' + value + '" from String returns correct CSSUnparsedValue');
+
+  test(() => {
+    const result = newDivWithStyle('color:' + value).attributeStyleMap.get('color');
+    assert_style_value_equals(result, new CSSUnparsedValue(...expectedResult));
+  }, 'Normalizing "' + value + '" from CSSOM returns correct CSSUnparsedValue');
+}
+
+</script>
--- a/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp
+++ b/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp
@@ -15,7 +15,9 @@
 #include "core/css/cssom/CSSURLImageValue.h"
 #include "core/css/cssom/CSSUnparsedValue.h"
 #include "core/css/cssom/CSSUnsupportedStyleValue.h"
-#include "core/css/parser/CSSParser.h"
+#include "core/css/parser/CSSPropertyParser.h"
+#include "core/css/parser/CSSTokenizer.h"
+#include "core/css/parser/CSSVariableParser.h"
 #include "core/css/properties/CSSProperty.h"

 namespace blink {
@@ -64,11 +66,33 @@ CSSStyleValueVector UnsupportedCSSValue(const CSSValue& value) {
  return style_value_vector;
 }

+const CSSValue* ParseProperty(CSSPropertyID property_id,
+                              const String& css_text,
+                              const CSSParserContext* context) {
+  CSSTokenizer tokenizer(css_text);
+  const auto tokens = tokenizer.TokenizeToEOF();
+  const CSSParserTokenRange range(tokens);
+
+  if (const CSSValue* value =
+          CSSPropertyParser::ParseSingleValue(property_id, range, context)) {
+    return value;
+  }
+
+  if (CSSVariableParser::ContainsValidVariableReferences(range)) {
+    return CSSVariableReferenceValue::Create(
+        CSSVariableData::Create(range, false /* is_animation_tainted */,
+                                false /* needs variable resolution */),
+        *context);
+  }
+
+  return nullptr;
+}
+
 }  // namespace

 CSSStyleValueVector StyleValueFactory::FromString(
    CSSPropertyID property_id,
-    const String& value,
+    const String& css_text,
    const CSSParserContext* parser_context) {
  DCHECK_NE(property_id, CSSPropertyInvalid);
  DCHECK(!CSSProperty::Get(property_id).IsShorthand());
@@ -78,13 +102,12 @@ CSSStyleValueVector StyleValueFactory::FromString(
    return CSSStyleValueVector();
  }

-  const CSSValue* css_value =
-      CSSParser::ParseSingleValue(property_id, value, parser_context);
-  if (!css_value)
+  const CSSValue* value = ParseProperty(property_id, css_text, parser_context);
+  if (!value)
    return CSSStyleValueVector();

  CSSStyleValueVector style_value_vector =
-      StyleValueFactory::CssValueToStyleValueVector(property_id, *css_value);
+      StyleValueFactory::CssValueToStyleValueVector(property_id, *value);
  DCHECK(!style_value_vector.IsEmpty());
  return style_value_vector;
 }