Commit 4aa028f8 authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

[css-properties-values-api] Reify specified values as CSSUnparsedValue

According to a recent spec change, the syntax of a registered custom
property must be ignored until computed-value time. This means that
we can unfortunately not reify specified values according to the
syntax; instead all custom properties reify as CSSUnparsedValue,
regardless of their registration status.

This means that we can remove large parts of the Typed OM code which
deals with PropertyRegistation for specified values and matching of
incoming CSSStyleValues against registered syntax.

The typedom.html test was also mostly re-written, as most of the things
it tested are now not relevant anymore, or at least no longer as
interesting as before. (The new reworked test basically verifies that
registered custom properties behave as unregistered for
attributeStyleMap and styleMap. For computedStyleMap, we should of
course still reify according to the syntax).

Bug: 641877

Change-Id: I4da7c25b8f066dd01388b6229bebe181513e9fd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713557
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680595}
parent 28f009ce
......@@ -14,7 +14,6 @@
#include "third_party/blink/renderer/core/css/cssom/css_unsupported_style_value.h"
#include "third_party/blink/renderer/core/css/cssom/cssom_keywords.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"
#include "third_party/blink/renderer/core/css/property_registration.h"
namespace blink {
......@@ -93,27 +92,18 @@ bool CSSOMTypes::IsPropertySupported(CSSPropertyID id) {
bool CSSOMTypes::PropertyCanTake(CSSPropertyID id,
const AtomicString& custom_property_name,
const PropertyRegistration* registration,
const CSSStyleValue& value,
const CSSSyntaxComponent*& match) {
const CSSStyleValue& value) {
DCHECK_EQ(id == CSSPropertyID::kVariable, !custom_property_name.IsNull());
if (id == CSSPropertyID::kVariable && registration) {
if (auto* unsupported_style_value =
DynamicTo<CSSUnsupportedStyleValue>(value)) {
return unsupported_style_value->IsValidFor(
CSSPropertyName(custom_property_name));
}
match = registration->Syntax().Match(value);
return match != nullptr;
}
if (auto* css_keyword_value = DynamicTo<CSSKeywordValue>(value)) {
return CSSOMKeywords::ValidKeywordForProperty(id, *css_keyword_value);
}
if (auto* unsupported_style_value =
DynamicTo<CSSUnsupportedStyleValue>(value)) {
return unsupported_style_value->IsValidFor(CSSPropertyName(id));
auto name = (id == CSSPropertyID::kVariable)
? CSSPropertyName(custom_property_name)
: CSSPropertyName(id);
return unsupported_style_value->IsValidFor(name);
}
if (value.GetType() == CSSStyleValue::kUnparsedType) {
return true;
......
......@@ -181,7 +181,6 @@ blink_core_sources("css") {
"css_style_sheet.h",
"css_supports_rule.cc",
"css_supports_rule.h",
"css_syntax_component.cc",
"css_syntax_component.h",
"css_syntax_descriptor.cc",
"css_syntax_descriptor.h",
......
// Copyright 2018 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 "third_party/blink/renderer/core/css/css_syntax_component.h"
#include "third_party/blink/renderer/core/css/cssom/css_keyword_value.h"
#include "third_party/blink/renderer/core/css/cssom/css_style_value.h"
#include "third_party/blink/renderer/core/css/cssom/cssom_types.h"
namespace blink {
bool CSSSyntaxComponent::CanTake(const CSSStyleValue& value) const {
switch (type_) {
case CSSSyntaxType::kTokenStream:
return value.GetType() == CSSStyleValue::kUnparsedType;
case CSSSyntaxType::kIdent:
return value.GetType() == CSSStyleValue::kKeywordType &&
static_cast<const CSSKeywordValue&>(value).value() == string_;
case CSSSyntaxType::kLength:
return CSSOMTypes::IsCSSStyleValueLength(value);
case CSSSyntaxType::kInteger:
// TODO(andruud): Support rounding.
// https://drafts.css-houdini.org/css-typed-om-1/#numeric-objects
FALLTHROUGH;
case CSSSyntaxType::kNumber:
return CSSOMTypes::IsCSSStyleValueNumber(value);
case CSSSyntaxType::kPercentage:
return CSSOMTypes::IsCSSStyleValuePercentage(value);
case CSSSyntaxType::kLengthPercentage:
// TODO(andruud): Support calc(X% + Ypx).
return CSSOMTypes::IsCSSStyleValueLength(value) ||
CSSOMTypes::IsCSSStyleValuePercentage(value);
case CSSSyntaxType::kColor:
// TODO(andruud): Support custom properties in CSSUnsupportedStyleValue.
return false;
case CSSSyntaxType::kImage:
case CSSSyntaxType::kUrl:
return value.GetType() == CSSStyleValue::kURLImageType;
case CSSSyntaxType::kAngle:
return CSSOMTypes::IsCSSStyleValueAngle(value);
case CSSSyntaxType::kTime:
return CSSOMTypes::IsCSSStyleValueTime(value);
case CSSSyntaxType::kResolution:
return CSSOMTypes::IsCSSStyleValueResolution(value);
case CSSSyntaxType::kTransformFunction:
// TODO(andruud): Currently not supported by Typed OM.
// https://github.com/w3c/css-houdini-drafts/issues/290
// For now, this should accept a CSSUnsupportedStyleValue, such that
// <transform-function> values can be moved from one registered property
// to another.
// TODO(andruud): Support custom properties in CSSUnsupportedStyleValue.
return false;
case CSSSyntaxType::kTransformList:
return value.GetType() == CSSStyleValue::kTransformType;
case CSSSyntaxType::kCustomIdent:
return value.GetType() == CSSStyleValue::kKeywordType;
default:
return false;
}
}
} // namespace blink
......@@ -12,8 +12,6 @@
namespace blink {
class CSSStyleValue;
enum class CSSSyntaxType {
kTokenStream,
kIdent,
......@@ -58,8 +56,6 @@ class CSSSyntaxComponent {
return repeat_ == CSSSyntaxRepeat::kSpaceSeparated ? ' ' : ',';
}
bool CanTake(const CSSStyleValue&) const;
private:
CSSSyntaxType type_;
String string_; // Only used when type_ is CSSSyntaxType::kIdent
......
......@@ -127,19 +127,6 @@ const CSSValue* ConsumeSyntaxComponent(const CSSSyntaxComponent& syntax,
} // namespace
const CSSSyntaxComponent* CSSSyntaxDescriptor::Match(
const CSSStyleValue& value) const {
for (const CSSSyntaxComponent& component : syntax_components_) {
if (component.CanTake(value))
return &component;
}
return nullptr;
}
bool CSSSyntaxDescriptor::CanTake(const CSSStyleValue& value) const {
return Match(value);
}
const CSSValue* CSSSyntaxDescriptor::Parse(CSSParserTokenRange range,
const CSSParserContext* context,
bool is_animation_tainted) const {
......
......@@ -19,8 +19,6 @@ class CORE_EXPORT CSSSyntaxDescriptor {
const CSSValue* Parse(CSSParserTokenRange,
const CSSParserContext*,
bool is_animation_tainted) const;
const CSSSyntaxComponent* Match(const CSSStyleValue&) const;
bool CanTake(const CSSStyleValue&) const;
bool IsTokenStream() const {
return syntax_components_.size() == 1 &&
syntax_components_[0].GetType() == CSSSyntaxType::kTokenStream;
......
......@@ -9,7 +9,6 @@
#include "third_party/blink/renderer/core/css/cssom/style_value_factory.h"
#include "third_party/blink/renderer/core/css/parser/css_parser_context.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"
#include "third_party/blink/renderer/core/css/property_registration.h"
#include "third_party/blink/renderer/core/style_property_shorthand.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
......@@ -35,14 +34,8 @@ CSSStyleValueVector ParseCSSStyleValue(
AtomicString custom_property_name = property_id == CSSPropertyID::kVariable
? AtomicString(property_name)
: g_null_atom;
const PropertyRegistration* registration =
(property_id == CSSPropertyID::kVariable)
? PropertyRegistration::From(execution_context, custom_property_name)
: nullptr;
const auto style_values = StyleValueFactory::FromString(
property_id, custom_property_name, registration, value,
property_id, custom_property_name, value,
MakeGarbageCollected<CSSParserContext>(*execution_context));
if (style_values.IsEmpty()) {
exception_state.ThrowTypeError("The value provided ('" + value +
......
......@@ -15,7 +15,6 @@
namespace blink {
class CSSSyntaxComponent;
class ExceptionState;
class ExecutionContext;
enum class SecureContextMode;
......@@ -66,9 +65,7 @@ class CORE_EXPORT CSSStyleValue : public ScriptWrappable {
virtual const CSSValue* ToCSSValue() const = 0;
// FIXME: We should make this a method on CSSProperty instead.
virtual const CSSValue* ToCSSValueWithProperty(
CSSPropertyID,
const CSSSyntaxComponent*) const {
virtual const CSSValue* ToCSSValueWithProperty(CSSPropertyID) const {
return ToCSSValue();
}
virtual String toString() const;
......
......@@ -40,8 +40,7 @@ CSSPrimitiveValue::UnitType ToCanonicalUnitIfPossible(
bool IsValueOutOfRangeForProperty(CSSPropertyID property_id,
double value,
CSSPrimitiveValue::UnitType unit,
const CSSSyntaxComponent* match) {
CSSPrimitiveValue::UnitType unit) {
// FIXME: Avoid this CSSProperty::Get call as it can be costly.
// The caller often has a CSSProperty already, so we can just pass it here.
if (LengthPropertyFunctions::GetValueRange(CSSProperty::Get(property_id)) ==
......@@ -51,10 +50,6 @@ bool IsValueOutOfRangeForProperty(CSSPropertyID property_id,
// For non-length properties and special cases.
switch (property_id) {
case CSSPropertyID::kVariable:
if (match && match->IsInteger())
return round(value) != value;
return false;
case CSSPropertyID::kOrder:
case CSSPropertyID::kZIndex:
return round(value) != value;
......@@ -175,9 +170,8 @@ const CSSNumericLiteralValue* CSSUnitValue::ToCSSValue() const {
}
const CSSPrimitiveValue* CSSUnitValue::ToCSSValueWithProperty(
CSSPropertyID property_id,
const CSSSyntaxComponent* match) const {
if (IsValueOutOfRangeForProperty(property_id, value_, unit_, match)) {
CSSPropertyID property_id) const {
if (IsValueOutOfRangeForProperty(property_id, value_, unit_)) {
// Wrap out of range values with a calc.
CSSMathExpressionNode* node = ToCalcExpressionNode();
node->SetIsNestedCalc();
......
......@@ -52,9 +52,7 @@ class CORE_EXPORT CSSUnitValue final : public CSSNumericValue {
// From CSSStyleValue.
StyleValueType GetType() const final;
const CSSNumericLiteralValue* ToCSSValue() const final;
const CSSPrimitiveValue* ToCSSValueWithProperty(
CSSPropertyID,
const CSSSyntaxComponent*) const final;
const CSSPrimitiveValue* ToCSSValueWithProperty(CSSPropertyID) const final;
CSSMathExpressionNode* ToCalcExpressionNode() const final;
private:
......
......@@ -11,8 +11,6 @@
namespace blink {
class PropertyRegistration;
// This class provides utility functions for determining whether a property
// can accept a CSSStyleValue type or instance. Its implementation is generated
// using input from css_properties.json5 and the script
......@@ -33,15 +31,9 @@ class CSSOMTypes {
static bool IsCSSStyleValuePosition(const CSSStyleValue&);
static bool IsPropertySupported(CSSPropertyID);
// For registered custom properties, if the CSSStyleValue is accepted
// because it matches the registered grammar (and not because it is
// a CSSUnsupportedStyleValue with matching name), 'match' will be set
// to the component that was matched.
static bool PropertyCanTake(CSSPropertyID,
const AtomicString& custom_property_name,
const PropertyRegistration*,
const CSSStyleValue&,
const CSSSyntaxComponent*& match);
const CSSStyleValue&);
};
} // namespace blink
......
......@@ -73,10 +73,9 @@ CSSStyleValue* StylePropertyMapReadOnlyMainThread::get(
if (property.IsShorthand())
return GetShorthandProperty(property);
const CSSValue* value =
(name->IsCustomProperty())
? GetCustomProperty(*execution_context, name->ToAtomicString())
: GetProperty(name->Id());
const CSSValue* value = (name->IsCustomProperty())
? GetCustomProperty(name->ToAtomicString())
: GetProperty(name->Id());
if (!value)
return nullptr;
......@@ -110,10 +109,9 @@ CSSStyleValueVector StylePropertyMapReadOnlyMainThread::getAll(
return values;
}
const CSSValue* value =
(name->IsCustomProperty())
? GetCustomProperty(*execution_context, name->ToAtomicString())
: GetProperty(name->Id());
const CSSValue* value = (name->IsCustomProperty())
? GetCustomProperty(name->ToAtomicString())
: GetProperty(name->Id());
if (!value)
return CSSStyleValueVector();
......@@ -127,41 +125,14 @@ bool StylePropertyMapReadOnlyMainThread::has(
return !getAll(execution_context, property_name, exception_state).IsEmpty();
}
const CSSValue* StylePropertyMapReadOnlyMainThread::GetCustomProperty(
const ExecutionContext& execution_context,
const AtomicString& property_name) const {
const CSSValue* value = GetCustomProperty(property_name);
const auto* document = DynamicTo<Document>(execution_context);
if (!document)
return value;
return PropertyRegistry::ParseIfRegistered(*document, property_name, value);
}
StylePropertyMapReadOnlyMainThread::IterationSource*
StylePropertyMapReadOnlyMainThread::StartIteration(ScriptState* script_state,
ExceptionState&) {
HeapVector<StylePropertyMapReadOnlyMainThread::StylePropertyMapEntry> result;
const ExecutionContext& execution_context =
*ExecutionContext::From(script_state);
ForEachProperty([&result, &execution_context](const CSSPropertyName& name,
const CSSValue& css_value) {
const CSSValue* value = &css_value;
// TODO(andruud): Refactor this. ForEachProperty should yield the correct,
// already-parsed value in the first place.
if (name.IsCustomProperty()) {
const auto* document = DynamicTo<Document>(execution_context);
if (document) {
value = PropertyRegistry::ParseIfRegistered(
*document, name.ToAtomicString(), value);
}
}
auto values = StyleValueFactory::CssValueToStyleValueVector(name, *value);
ForEachProperty([&result](const CSSPropertyName& name,
const CSSValue& value) {
auto values = StyleValueFactory::CssValueToStyleValueVector(name, value);
result.emplace_back(name.ToAtomicString(), std::move(values));
});
......
......@@ -43,9 +43,6 @@ class CORE_EXPORT StylePropertyMapReadOnlyMainThread
virtual String SerializationForShorthand(const CSSProperty&) const = 0;
const CSSValue* GetCustomProperty(const ExecutionContext&,
const AtomicString&) const;
private:
IterationSource* StartIteration(ScriptState*, ExceptionState&) override;
......
......@@ -26,7 +26,6 @@
#include "third_party/blink/renderer/core/css/parser/css_tokenizer.h"
#include "third_party/blink/renderer/core/css/parser/css_variable_parser.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"
#include "third_party/blink/renderer/core/css/property_registration.h"
#include "third_party/blink/renderer/core/style_property_shorthand.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
......@@ -244,7 +243,6 @@ CSSStyleValueVector UnsupportedCSSValue(const CSSPropertyName& name,
CSSStyleValueVector StyleValueFactory::FromString(
CSSPropertyID property_id,
const AtomicString& custom_property_name,
const PropertyRegistration* registration,
const String& css_text,
const CSSParserContext* parser_context) {
DCHECK_NE(property_id, CSSPropertyID::kInvalid);
......@@ -277,17 +275,6 @@ CSSStyleValueVector StyleValueFactory::FromString(
return result;
}
if (property_id == CSSPropertyID::kVariable && registration) {
const bool is_animation_tainted = false;
const CSSValue* value = registration->Syntax().Parse(tokens, parser_context,
is_animation_tainted);
if (!value)
return CSSStyleValueVector();
return StyleValueFactory::CssValueToStyleValueVector(
CSSPropertyName(custom_property_name), *value);
}
if ((property_id == CSSPropertyID::kVariable && !tokens.IsEmpty()) ||
CSSVariableParser::ContainsValidVariableReferences(range)) {
const auto variable_data = CSSVariableData::Create(
......@@ -316,7 +303,6 @@ CSSStyleValue* StyleValueFactory::CssValueToStyleValue(
CSSStyleValueVector StyleValueFactory::CoerceStyleValuesOrStrings(
const CSSProperty& property,
const AtomicString& custom_property_name,
const PropertyRegistration* registration,
const HeapVector<CSSStyleValueOrString>& values,
const ExecutionContext& execution_context) {
const CSSParserContext* parser_context = nullptr;
......@@ -335,8 +321,8 @@ CSSStyleValueVector StyleValueFactory::CoerceStyleValuesOrStrings(
}
const auto subvalues = StyleValueFactory::FromString(
property.PropertyID(), custom_property_name, registration,
value.GetAsString(), parser_context);
property.PropertyID(), custom_property_name, value.GetAsString(),
parser_context);
if (subvalues.IsEmpty())
return CSSStyleValueVector();
......
......@@ -17,7 +17,6 @@ class CSSProperty;
class CSSPropertyName;
class CSSValue;
class ExecutionContext;
class PropertyRegistration;
class CORE_EXPORT StyleValueFactory {
STATIC_ONLY(StyleValueFactory);
......@@ -26,7 +25,6 @@ class CORE_EXPORT StyleValueFactory {
static CSSStyleValueVector FromString(
CSSPropertyID,
const AtomicString& custom_property_name,
const PropertyRegistration*,
const String&,
const CSSParserContext*);
static CSSStyleValue* CssValueToStyleValue(const CSSPropertyName&,
......@@ -37,7 +35,6 @@ class CORE_EXPORT StyleValueFactory {
static CSSStyleValueVector CoerceStyleValuesOrStrings(
const CSSProperty& property,
const AtomicString& custom_property_name,
const PropertyRegistration*,
const HeapVector<CSSStyleValueOrString>& values,
const ExecutionContext&);
// Reify a CSSStyleValue without the context of a CSS property. For most
......
......@@ -3,7 +3,6 @@
// found in the LICENSE file.
#include "third_party/blink/renderer/core/css/property_registry.h"
#include "third_party/blink/renderer/core/css/css_custom_property_declaration.h"
namespace blink {
......@@ -28,37 +27,6 @@ PropertyRegistry::RegistrationMap::const_iterator PropertyRegistry::end()
return registrations_.end();
}
const CSSValue* PropertyRegistry::ParseIfRegistered(
const Document& document,
const AtomicString& property_name,
const CSSValue* value) {
auto* custom_property_declaration =
DynamicTo<CSSCustomPropertyDeclaration>(value);
if (!custom_property_declaration)
return value;
const PropertyRegistry* registry = document.GetPropertyRegistry();
if (!registry)
return value;
const PropertyRegistration* registration =
registry->Registration(property_name);
if (!registration)
return value;
CSSVariableData* tokens = custom_property_declaration->Value();
if (!tokens || tokens->NeedsVariableResolution())
return value;
const CSSValue* parsed_value = tokens->ParseForSyntax(
registration->Syntax(), document.GetSecureContextMode());
return parsed_value ? parsed_value : value;
}
void PropertyRegistry::MarkReferenced(const AtomicString& property_name) const {
const PropertyRegistration* registration = Registration(property_name);
if (registration) {
......
......@@ -25,15 +25,6 @@ class CORE_EXPORT PropertyRegistry : public GarbageCollected<PropertyRegistry> {
void Trace(blink::Visitor* visitor) { visitor->Trace(registrations_); }
// Parse the incoming value and return the parsed result, if:
// 1. A registration with the specified name exists, and
// 2. The incoming value is a CSSCustomPropertyDeclaration, has no
// unresolved var-references and matches the registered syntax.
// Otherwise the incoming value is returned.
static const CSSValue* ParseIfRegistered(const Document& document,
const AtomicString& property_name,
const CSSValue*);
void MarkReferenced(const AtomicString&) const;
bool WasReferenced(const AtomicString&) const;
......
......@@ -22,23 +22,23 @@ CSS.registerProperty({
test(() => {
let value = CSSStyleValue.parse('--tf', 'translateX(0px)');
assert_equals(value.constructor, CSSStyleValue);
}, 'Result of CSSStyleValue.parse for <transform-function> is a direct CSSStyleValue');
assert_true(value instanceof CSSUnparsedValue);
}, 'Result of CSSStyleValue.parse for <transform-function> is a CSSUnparsedValue');
test(() => {
target.style = '--tf: translateX(0px)';
let value = target.attributeStyleMap.get('--tf');
assert_equals(value.constructor, CSSStyleValue);
assert_true(value instanceof CSSUnparsedValue);
target.style = '';
}, 'Result of attributeStyleMap.get for <transform-function> is a direct CSSStyleValue');
}, 'Result of attributeStyleMap.get for <transform-function> is a CSSUnparsedValue');
test(() => {
style.textContent = '#target { --tf: translateX(0px); }';
let styleMap = style.sheet.rules[0].styleMap;
let value = styleMap.get('--tf');
assert_equals(value.constructor, CSSStyleValue);
assert_true(value instanceof CSSUnparsedValue);
style.textContent = '';
}, 'Result of styleMap.get for <transform-function> is a direct CSSStyleValue');
}, 'Result of styleMap.get for <transform-function> is a CSSUnparsedValue');
test(() => {
let value = target.computedStyleMap().get('--tf');
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment