Ribbon: Apply values via CustomProperty.

Instead of applying custom properties via the static Variable instance,
use the new CustomProperty class (which knows about its name and
registration) to apply values. This brings the custom property code path
closer to the standard property code path.

To assist CSSPropertyRef in producing a CSSProperty&, this CL adds the
class CSSPropertyName. The idea is that CSSPropertyName can be used instead
of CSSPropertyID/AtomicString where both standard and custom properties are
expected. My intent is to store a CSSPropertyName in
CSSPropertyValueMetadata (instead of the CSSProperty*), but this is a big
change, as everything that creates property/value pairs must carry the
CSSPropertyName. So, for now PropertyReference.Name() is faking it by
digging the name out of the associated CSSValue.

Note that CSSPropertyName::ToAtomicString intentionally does not return
a const AtomicString&. This is because 1) I want to try and reduce the
size of CSSPropertyName to one pointer, which means it will contain a
StringImpl* rather than an AtomicString, and 2) I want CSSPropertyName to
be something that can be passed to other threads if needed, which again
means StringImpl* (or String, at least) rather than AtomicString.

In general, this CL is a step on the path to achieve these things:

 1. Removing the useless and dangerous static Variable instance.
 2. Not having to piggy-back the custom property name in a CSSValue.
 3. Not having to smuggle initial/inherit/unset via the
    CSSCustomPropertyDeclaration (vs. applying CSSIdentifierValues
    holding these values, like civilized individuals).

R=flackr@chromium.org, futhark@chromium.org

Change-Id: I1a062138fc86c5a398542ac147bf9d71feed7b3c
Reviewed-on: https://chromium-review.googlesource.com/c/1323113
Commit-Queue: Anders Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606829}

third_party/blink/renderer/core/BUILD.gn

@@ -1731,6 +1731,7 @@ jumbo_source_set("unit_tests") {
     "css/css_page_rule_test.cc",
     "css/css_paint_value_test.cc",
     "css/css_primitive_value_test.cc",
+    "css/css_property_name_test.cc",
     "css/css_selector_test.cc",
     "css/css_selector_watch_test.cc",
     "css/css_style_declaration_test.cc",

third_party/blink/renderer/core/animation/css_default_interpolation_type.cc

@@ -39,7 +39,7 @@ void CSSDefaultInterpolationType::Apply(
     InterpolationEnvironment& environment) const {
   DCHECK(ToCSSDefaultNonInterpolableValue(non_interpolable_value)->CssValue());
   StyleBuilder::ApplyProperty(
-      GetProperty().GetCSSProperty(),
+      GetProperty().GetCSSPropertyName(),
       ToCSSInterpolationEnvironment(environment).GetState(),
       *ToCSSDefaultNonInterpolableValue(non_interpolable_value)->CssValue());
 }

third_party/blink/renderer/core/animation/css_var_cycle_interpolation_type.cc

@@ -125,7 +125,7 @@ void CSSVarCycleInterpolationType::Apply(
     const NonInterpolableValue*,
     InterpolationEnvironment& environment) const {
   StyleBuilder::ApplyProperty(
-      GetProperty().GetCSSProperty(),
+      GetProperty().GetCSSPropertyName(),
       ToCSSInterpolationEnvironment(environment).GetState(),
       *CSSCustomPropertyDeclaration::Create(GetProperty().CustomPropertyName(),
                                             CSSValueUnset));

third_party/blink/renderer/core/animation/property_handle.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_ANIMATION_PROPERTY_HANDLE_H_
 
 #include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/css/css_property_name.h"
 #include "third_party/blink/renderer/core/css/properties/css_property.h"
 #include "third_party/blink/renderer/core/css_property_names.h"
 #include "third_party/blink/renderer/core/dom/qualified_name.h"
@@ -71,6 +72,13 @@ class CORE_EXPORT PropertyHandle {
     return *svg_attribute_;
   }
 
+  CSSPropertyName GetCSSPropertyName() const {
+    if (handle_type_ == kHandleCSSCustomProperty)
+      return CSSPropertyName(property_name_);
+    DCHECK(IsCSSProperty() || IsPresentationAttribute());
+    return CSSPropertyName(css_property_->PropertyID());
+  }
+
  private:
   enum HandleType {
     kHandleEmptyValueForHashTraits,

third_party/blink/renderer/core/animation/property_handle_test.cc

@@ -111,12 +111,19 @@ TEST_F(PropertyHandleTest, Accessors) {
   EXPECT_FALSE(PropertyHandle(kAmplitudeAttr).IsCSSCustomProperty());
 
   EXPECT_EQ(
-      PropertyHandle(GetCSSPropertyOpacity()).GetCSSProperty().PropertyID(),
-      CSSPropertyOpacity);
-  EXPECT_EQ(PropertyHandle(name).GetCSSProperty().PropertyID(),
-            CSSPropertyVariable);
-  EXPECT_EQ(PropertyHandle(name).CustomPropertyName(), name);
-  EXPECT_EQ(PropertyHandle(kAmplitudeAttr).SvgAttribute(), kAmplitudeAttr);
+      CSSPropertyOpacity,
+      PropertyHandle(GetCSSPropertyOpacity()).GetCSSProperty().PropertyID());
+  EXPECT_EQ(CSSPropertyVariable,
+            PropertyHandle(name).GetCSSProperty().PropertyID());
+  EXPECT_EQ(name, PropertyHandle(name).CustomPropertyName());
+  EXPECT_EQ(kAmplitudeAttr, PropertyHandle(kAmplitudeAttr).SvgAttribute());
+
+  EXPECT_EQ(name, PropertyHandle(name).GetCSSPropertyName().ToAtomicString());
+  EXPECT_EQ(CSSPropertyOpacity,
+            PropertyHandle(GetCSSPropertyOpacity()).GetCSSPropertyName().Id());
+  EXPECT_EQ(
+      CSSPropertyColor,
+      PropertyHandle(GetCSSPropertyColor(), true).GetCSSPropertyName().Id());
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/BUILD.gn

@@ -120,6 +120,8 @@ blink_core_sources("css") {
     "css_property_equality.cc",
     "css_property_equality.h",
     "css_property_id_templates.h",
+    "css_property_name.cc",
+    "css_property_name.h",
     "css_property_source_data.cc",
     "css_property_source_data.h",
     "css_property_value.cc",

third_party/blink/renderer/core/css/css_property_name.cc

+// 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_property_name.h"
+
+#include "third_party/blink/renderer/core/css/properties/css_property.h"
+
+namespace blink {
+
+namespace {
+
+// TODO(andruud): Reduce this to sizeof(void*).
+struct SameSizeAsCSSPropertyName {
+  CSSPropertyID property_id_;
+  AtomicString custom_property_name_;
+};
+
+static_assert(sizeof(CSSPropertyName) == sizeof(SameSizeAsCSSPropertyName),
+              "CSSPropertyName should stay small");
+
+}  // namespace
+
+AtomicString CSSPropertyName::ToAtomicString() const {
+  if (IsCustomProperty())
+    return custom_property_name_;
+  return CSSProperty::Get(property_id_).GetPropertyNameAtomicString();
+}
+
+}  // namespace blink

third_party/blink/renderer/core/css/css_property_name.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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_NAME_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_NAME_H_
+
+#include "third_party/blink/renderer/core/css_property_names.h"
+#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
+
+namespace blink {
+
+// This class may be used to represent the name of any valid CSS property,
+// including custom properties.
+class CORE_EXPORT CSSPropertyName {
+  DISALLOW_NEW();
+
+ public:
+  explicit CSSPropertyName(CSSPropertyID property_id)
+      : property_id_(property_id) {
+    DCHECK_NE(property_id, CSSPropertyInvalid);
+    DCHECK_NE(property_id, CSSPropertyVariable);
+  }
+
+  explicit CSSPropertyName(const AtomicString& custom_property_name)
+      : property_id_(CSSPropertyVariable),
+        custom_property_name_(custom_property_name) {
+    DCHECK(!custom_property_name.IsNull());
+  }
+
+  CSSPropertyID Id() const { return property_id_; }
+
+  bool IsCustomProperty() const { return property_id_ == CSSPropertyVariable; }
+
+  AtomicString ToAtomicString() const;
+
+ private:
+  CSSPropertyID property_id_;
+  AtomicString custom_property_name_;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_NAME_H_

third_party/blink/renderer/core/css/css_property_name_test.cc

+// 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_property_name.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+TEST(CSSPropertyNameTest, IdStandardProperty) {
+  CSSPropertyName name(CSSPropertyFontSize);
+  EXPECT_EQ(CSSPropertyFontSize, name.Id());
+}
+
+TEST(CSSPropertyNameTest, IdCustomProperty) {
+  CSSPropertyName name(AtomicString("--x"));
+  EXPECT_EQ(CSSPropertyVariable, name.Id());
+  EXPECT_TRUE(name.IsCustomProperty());
+}
+
+TEST(CSSPropertyNameTest, GetNameStandardProperty) {
+  CSSPropertyName name(CSSPropertyFontSize);
+  EXPECT_EQ(AtomicString("font-size"), name.ToAtomicString());
+}
+
+TEST(CSSPropertyNameTest, GetNameCustomProperty) {
+  CSSPropertyName name(AtomicString("--x"));
+  EXPECT_EQ(AtomicString("--x"), name.ToAtomicString());
+}
+
+}  // namespace blink

third_party/blink/renderer/core/css/css_property_value_set.cc

@@ -59,6 +59,12 @@ ImmutableCSSPropertyValueSet* ImmutableCSSPropertyValueSet::Create(
       ImmutableCSSPropertyValueSet(properties, count, css_parser_mode);
 }
 
+CSSPropertyName CSSPropertyValueSet::PropertyReference::Name() const {
+  if (Id() != CSSPropertyVariable)
+    return CSSPropertyName(Id());
+  return CSSPropertyName(ToCSSCustomPropertyDeclaration(Value()).GetName());
+}
+
 ImmutableCSSPropertyValueSet* CSSPropertyValueSet::ImmutableCopyIfNeeded()
     const {
   if (!IsMutable()) {

third_party/blink/renderer/core/css/css_property_value_set.h

@@ -24,6 +24,7 @@
 #include "base/macros.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/css/css_primitive_value.h"
+#include "third_party/blink/renderer/core/css/css_property_name.h"
 #include "third_party/blink/renderer/core/css/css_property_value.h"
 #include "third_party/blink/renderer/core/css/parser/css_parser_mode.h"
 #include "third_party/blink/renderer/core/css/property_set_css_style_declaration.h"
@@ -67,6 +68,8 @@ class CORE_EXPORT CSSPropertyValueSet
       return PropertyMetadata().ShorthandID();
     }
 
+    CSSPropertyName Name() const;
+
     bool IsImportant() const { return PropertyMetadata().important_; }
     bool IsInherited() const { return PropertyMetadata().inherited_; }
     bool IsImplicit() const { return PropertyMetadata().implicit_; }

third_party/blink/renderer/core/css/properties/css_property_ref.cc

@@ -14,6 +14,14 @@ CSSPropertyRef::CSSPropertyRef(const String& name, const Document& document)
     custom_property_ = CustomProperty(AtomicString(name), document);
 }
 
+CSSPropertyRef::CSSPropertyRef(const CSSPropertyName& name,
+                               const Document& document)
+    : property_id_(name.Id()) {
+  DCHECK_NE(name.Id(), CSSPropertyInvalid);
+  if (property_id_ == CSSPropertyVariable)
+    custom_property_ = CustomProperty(name.ToAtomicString(), document);
+}
+
 CSSPropertyRef::CSSPropertyRef(const CSSProperty& property)
     : property_id_(property.PropertyID()) {
   if (property.PropertyID() == CSSPropertyVariable) {

third_party/blink/renderer/core/css/properties/css_property_ref.h

@@ -9,6 +9,7 @@
 
 namespace blink {
 
+class CSSPropertyName;
 class Document;
 
 // Use this class to acquire a reference to a CSSProperty instance. The
@@ -33,9 +34,13 @@ class CORE_EXPORT CSSPropertyRef {
  public:
   // Look up (or create) a CSSProperty.
   //
-  // If the incoming 'name' is not a CSS property, the CSSProperty is invalid.
+  // If the incoming 'name' is not a CSS property, the CSSPropertyRef is
+  // invalid.
   CSSPropertyRef(const String& name, const Document&);
 
+  // Like above, but will never produce an invalid CSSPropertyRef.
+  CSSPropertyRef(const CSSPropertyName&, const Document&);
+
   // If you already have a CSSProperty& object, you may use it to get
   // a CSSPropertyRef again.
   //

third_party/blink/renderer/core/css/properties/css_property_ref_test.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/css/css_property_name.h"
 #include "third_party/blink/renderer/core/css/property_descriptor.h"
 #include "third_party/blink/renderer/core/css/property_registration.h"
 #include "third_party/blink/renderer/core/css/property_registry.h"
@@ -36,20 +37,20 @@ class CSSPropertyRefTest : public PageTestBase {
 TEST_F(CSSPropertyRefTest, LookupUnregistred) {
   CSSPropertyRef ref("--x", GetDocument());
   EXPECT_TRUE(ref.IsValid());
-  EXPECT_EQ(ref.GetProperty().PropertyID(), CSSPropertyVariable);
+  EXPECT_EQ(CSSPropertyVariable, ref.GetProperty().PropertyID());
 }
 
 TEST_F(CSSPropertyRefTest, LookupRegistered) {
   RegisterProperty("--x", "<length>", "42px", false);
   CSSPropertyRef ref("--x", GetDocument());
   EXPECT_TRUE(ref.IsValid());
-  EXPECT_EQ(ref.GetProperty().PropertyID(), CSSPropertyVariable);
+  EXPECT_EQ(CSSPropertyVariable, ref.GetProperty().PropertyID());
 }
 
 TEST_F(CSSPropertyRefTest, LookupStandard) {
   CSSPropertyRef ref("font-size", GetDocument());
   EXPECT_TRUE(ref.IsValid());
-  EXPECT_EQ(ref.GetProperty().PropertyID(), CSSPropertyFontSize);
+  EXPECT_EQ(CSSPropertyFontSize, ref.GetProperty().PropertyID());
 }
 
 TEST_F(CSSPropertyRefTest, IsValid) {
@@ -61,13 +62,13 @@ TEST_F(CSSPropertyRefTest, FromCustomProperty) {
   CustomProperty custom(AtomicString("--x"), GetDocument());
   CSSPropertyRef ref(custom);
   EXPECT_TRUE(ref.IsValid());
-  EXPECT_EQ(ref.GetProperty().PropertyID(), CSSPropertyVariable);
+  EXPECT_EQ(CSSPropertyVariable, ref.GetProperty().PropertyID());
 }
 
 TEST_F(CSSPropertyRefTest, FromStandardProperty) {
   CSSPropertyRef ref(GetCSSPropertyFontSize());
   EXPECT_TRUE(ref.IsValid());
-  EXPECT_EQ(ref.GetProperty().PropertyID(), CSSPropertyFontSize);
+  EXPECT_EQ(CSSPropertyFontSize, ref.GetProperty().PropertyID());
 }
 
 TEST_F(CSSPropertyRefTest, FromStaticVariableInstance) {
@@ -100,4 +101,15 @@ TEST_F(CSSPropertyRefTest, GetResolvedPropertyAlias) {
   EXPECT_EQ("transform", ref.GetProperty().GetPropertyNameString());
 }
 
+TEST_F(CSSPropertyRefTest, FromCSSPropertyNameCustom) {
+  RegisterProperty("--x", "<length>", "42px", false);
+  CSSPropertyRef ref(CSSPropertyName("--x"), GetDocument());
+  EXPECT_EQ(CSSPropertyVariable, ref.GetProperty().PropertyID());
+}
+
+TEST_F(CSSPropertyRefTest, FromCSSPropertyNameStandard) {
+  CSSPropertyRef ref(CSSPropertyName(CSSPropertyFontSize), GetDocument());
+  EXPECT_EQ(CSSPropertyFontSize, ref.GetProperty().PropertyID());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/css/properties/longhands/custom_property.cc

@@ -4,8 +4,10 @@
 
 #include "third_party/blink/renderer/core/css/properties/longhands/custom_property.h"
 
+#include "third_party/blink/renderer/core/css/css_custom_property_declaration.h"
 #include "third_party/blink/renderer/core/css/property_registration.h"
 #include "third_party/blink/renderer/core/css/property_registry.h"
+#include "third_party/blink/renderer/core/style/computed_style.h"
 
 namespace blink {
 
@@ -21,4 +23,56 @@ const AtomicString& CustomProperty::GetPropertyNameAtomicString() const {
   return name_;
 }
 
+void CustomProperty::ApplyInitial(StyleResolverState& state) const {
+  state.Style()->RemoveVariable(name_, IsInherited());
+}
+
+void CustomProperty::ApplyInherit(StyleResolverState& state) const {
+  bool is_inherited_property = IsInherited();
+  state.Style()->RemoveVariable(name_, is_inherited_property);
+
+  CSSVariableData* parent_value =
+      state.ParentStyle()->GetVariable(name_, is_inherited_property);
+
+  if (!parent_value)
+    return;
+
+  state.Style()->SetVariable(name_, parent_value, is_inherited_property);
+
+  if (registration_) {
+    const CSSValue* parent_css_value =
+        parent_value ? state.ParentStyle()->GetNonInitialRegisteredVariable(
+                           name_, is_inherited_property)
+                     : nullptr;
+    state.Style()->SetRegisteredVariable(name_, parent_css_value,
+                                         is_inherited_property);
+  }
+}
+
+void CustomProperty::ApplyValue(StyleResolverState& state,
+                                const CSSValue& value) const {
+  const CSSCustomPropertyDeclaration& declaration =
+      ToCSSCustomPropertyDeclaration(value);
+
+  bool is_inherited_property = IsInherited();
+  bool initial = declaration.IsInitial(is_inherited_property);
+  bool inherit = declaration.IsInherit(is_inherited_property);
+  DCHECK(!(initial && inherit));
+
+  // TODO(andruud): Use regular initial/inherit dispatch in StyleBuilder
+  //                once custom properties are Ribbonized.
+  if (initial) {
+    ApplyInitial(state);
+  } else if (inherit) {
+    ApplyInherit(state);
+  } else {
+    state.Style()->SetVariable(name_, declaration.Value(),
+                               is_inherited_property);
+    if (registration_) {
+      state.Style()->SetRegisteredVariable(name_, nullptr,
+                                           is_inherited_property);
+    }
+  }
+}
+
 }  // namespace blink

third_party/blink/renderer/core/css/properties/longhands/custom_property.h

@@ -28,6 +28,10 @@ class CORE_EXPORT CustomProperty : public Variable {
   bool IsInherited() const override;
   const AtomicString& GetPropertyNameAtomicString() const override;
 
+  void ApplyInitial(StyleResolverState&) const override;
+  void ApplyInherit(StyleResolverState&) const override;
+  void ApplyValue(StyleResolverState&, const CSSValue&) const override;
+
   void Trace(blink::Visitor* visitor) { visitor->Trace(registration_); }
 
  private:

third_party/blink/renderer/core/css/properties/longhands/variable.cc

@@ -4,79 +4,10 @@
 
 #include "third_party/blink/renderer/core/css/properties/longhands/variable.h"
 
-#include "third_party/blink/renderer/core/css/css_custom_property_declaration.h"
-#include "third_party/blink/renderer/core/css/property_registration.h"
-#include "third_party/blink/renderer/core/css/property_registry.h"
-#include "third_party/blink/renderer/core/style/computed_style.h"
+#include "third_party/blink/renderer/core/css/properties/css_property.h"
 
 namespace blink {
 
-namespace {
-
-void ApplyInitialValue(StyleResolverState& state,
-                       const AtomicString& name,
-                       const PropertyRegistration* registration) {
-  bool is_inherited_property = !registration || registration->Inherits();
-  state.Style()->RemoveVariable(name, is_inherited_property);
-}
-
-void ApplyInheritValue(StyleResolverState& state,
-                       const AtomicString& name,
-                       const PropertyRegistration* registration) {
-  bool is_inherited_property = !registration || registration->Inherits();
-  state.Style()->RemoveVariable(name, is_inherited_property);
-
-  CSSVariableData* parent_value =
-      state.ParentStyle()->GetVariable(name, is_inherited_property);
-
-  if (!parent_value)
-    return;
-
-  state.Style()->SetVariable(name, parent_value, is_inherited_property);
-
-  if (registration) {
-    const CSSValue* parent_css_value =
-        parent_value ? state.ParentStyle()->GetNonInitialRegisteredVariable(
-                           name, is_inherited_property)
-                     : nullptr;
-    state.Style()->SetRegisteredVariable(name, parent_css_value,
-                                         is_inherited_property);
-  }
-}
-
-}  // namespace
-
-void Variable::ApplyValue(StyleResolverState& state,
-                          const CSSValue& value) const {
-  const CSSCustomPropertyDeclaration& declaration =
-      ToCSSCustomPropertyDeclaration(value);
-  const AtomicString& name = declaration.GetName();
-  const PropertyRegistration* registration = nullptr;
-  const PropertyRegistry* registry = state.GetDocument().GetPropertyRegistry();
-  if (registry)
-    registration = registry->Registration(name);
-
-  bool is_inherited_property = !registration || registration->Inherits();
-  bool initial = declaration.IsInitial(is_inherited_property);
-  bool inherit = declaration.IsInherit(is_inherited_property);
-  DCHECK(!(initial && inherit));
-
-  // TODO(andruud): Use regular initial/inherit dispatch in StyleBuilder
-  //                once custom properties are Ribbonized.
-  if (initial) {
-    ApplyInitialValue(state, name, registration);
-  } else if (inherit) {
-    ApplyInheritValue(state, name, registration);
-  } else {
-    state.Style()->SetVariable(name, declaration.Value(),
-                               is_inherited_property);
-    if (registration) {
-      state.Style()->SetRegisteredVariable(name, nullptr,
-                                           is_inherited_property);
-    }
-  }
-}
-
 bool Variable::IsStaticInstance(const CSSProperty& property) {
   return &property == &GetCSSPropertyVariable();
 }

third_party/blink/renderer/core/css/properties/longhands/variable.h

@@ -37,9 +37,6 @@ class CORE_EXPORT Variable : public Longhand {
     return nullptr;
   }
 
-  void ApplyValue(StyleResolverState& state,
-                  const CSSValue& value) const override;
-
   static bool IsStaticInstance(const CSSProperty&);
 };
 

third_party/blink/renderer/core/css/resolver/css_variable_resolver_test.cc

@@ -11,6 +11,7 @@
 #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/properties/longhand.h"
+#include "third_party/blink/renderer/core/css/properties/longhands/custom_property.h"
 #include "third_party/blink/renderer/core/css/property_registration.h"
 #include "third_party/blink/renderer/core/css/property_registry.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
@@ -267,9 +268,9 @@ TEST_F(CSSVariableResolverTest, NeedsResolutionClearedByResolver) {
   GetDocument().GetPropertyRegistry()->RegisterProperty("--prop3",
                                                         *registration);
 
-  ToLonghand(GetCSSPropertyVariable()).ApplyValue(state, *prop1);
-  ToLonghand(GetCSSPropertyVariable()).ApplyValue(state, *prop2);
-  ToLonghand(GetCSSPropertyVariable()).ApplyValue(state, *prop3);
+  CustomProperty("--prop1", GetDocument()).ApplyValue(state, *prop1);
+  CustomProperty("--prop2", GetDocument()).ApplyValue(state, *prop2);
+  CustomProperty("--prop3", GetDocument()).ApplyValue(state, *prop3);
 
   EXPECT_TRUE(state.Style()->InheritedVariables()->NeedsResolution());
   EXPECT_TRUE(state.Style()->NonInheritedVariables()->NeedsResolution());

third_party/blink/renderer/core/css/resolver/style_builder.cc

@@ -42,7 +42,10 @@
 #include <utility>
 
 #include "third_party/blink/renderer/core/animation/css/css_animations.h"
+#include "third_party/blink/renderer/core/css/css_property_name.h"
+#include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
 #include "third_party/blink/renderer/core/css/properties/longhand.h"
+#include "third_party/blink/renderer/core/css/properties/longhands/variable.h"
 #include "third_party/blink/renderer/core/css/resolver/css_variable_resolver.h"
 #include "third_party/blink/renderer/core/css/resolver/style_builder.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
@@ -50,9 +53,21 @@
 
 namespace blink {
 
+void StyleBuilder::ApplyProperty(const CSSPropertyName& name,
+                                 StyleResolverState& state,
+                                 const CSSValue& value) {
+  CSSPropertyRef ref(name, state.GetDocument());
+  DCHECK(ref.IsValid());
+
+  ApplyProperty(ref.GetProperty(), state, value);
+}
+
 void StyleBuilder::ApplyProperty(const CSSProperty& property,
                                  StyleResolverState& state,
                                  const CSSValue& value) {
+  DCHECK(!Variable::IsStaticInstance(property))
+      << "Please use a CustomProperty instance to apply custom properties";
+
   CSSPropertyID id = property.PropertyID();
   bool is_inherited = property.IsInherited();
   if (id != CSSPropertyVariable && (value.IsVariableReferenceValue() ||

third_party/blink/renderer/core/css/resolver/style_builder.h

@@ -38,6 +38,7 @@
 
 namespace blink {
 
+class CSSPropertyName;
 class CSSValue;
 class StyleResolverState;
 
@@ -45,6 +46,19 @@ class CORE_EXPORT StyleBuilder {
   STATIC_ONLY(StyleBuilder);
 
  public:
+  // Apply a property/value pair to the ComputedStyle.
+  //
+  // If the incoming CSSPropertyName is a custom property, a temporary
+  // CustomProperty instance is created to carry out the application.
+  static void ApplyProperty(const CSSPropertyName&,
+                            StyleResolverState&,
+                            const CSSValue&);
+
+  // Apply a property/value pair to the ComputedStyle.
+  //
+  // If you are applying a custom property, please ensure that the incoming
+  // CSSProperty is an instance of CustomProperty, and not the static Variable
+  // instance. See Variable::IsStaticInstance.
   static void ApplyProperty(const CSSProperty&,
                             StyleResolverState&,
                             const CSSValue&);

third_party/blink/renderer/core/css/resolver/style_resolver.cc

@@ -60,6 +60,7 @@
 #include "third_party/blink/renderer/core/css/page_rule_collector.h"
 #include "third_party/blink/renderer/core/css/part_names.h"
 #include "third_party/blink/renderer/core/css/properties/css_property.h"
+#include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
 #include "third_party/blink/renderer/core/css/resolver/animated_style_builder.h"
 #include "third_party/blink/renderer/core/css/resolver/css_variable_animator.h"
 #include "third_party/blink/renderer/core/css/resolver/css_variable_resolver.h"
@@ -860,7 +861,7 @@ AnimatableValue* StyleResolver::CreateAnimatableValueSnapshot(
                            parent_style);
   state.SetStyle(ComputedStyle::Clone(base_style));
   if (value) {
-    StyleBuilder::ApplyProperty(property.GetCSSProperty(), state, *value);
+    StyleBuilder::ApplyProperty(property.GetCSSPropertyName(), state, *value);
     state.GetFontBuilder().CreateFont(
         state.GetDocument().GetStyleEngine().GetFontSelector(),
         state.StyleRef());
@@ -1491,6 +1492,25 @@ void StyleResolver::ApplyAllProperty(
   }
 }
 
+template <CSSPropertyPriority priority>
+static inline void ApplyProperty(
+    const CSSPropertyValueSet::PropertyReference& reference,
+    StyleResolverState& state) {
+  static_assert(
+      priority != kResolveVariables,
+      "Application of custom properties must use specialized template");
+  DCHECK_NE(reference.Id(), CSSPropertyVariable);
+  StyleBuilder::ApplyProperty(reference.Property(), state, reference.Value());
+}
+
+template <>
+inline void ApplyProperty<kResolveVariables>(
+    const CSSPropertyValueSet::PropertyReference& reference,
+    StyleResolverState& state) {
+  CSSPropertyRef ref(reference.Name(), state.GetDocument());
+  StyleBuilder::ApplyProperty(ref.GetProperty(), state, reference.Value());
+}
+
 template <CSSPropertyPriority priority,
           StyleResolver::ShouldUpdateNeedsApplyPass shouldUpdateNeedsApplyPass>
 void StyleResolver::ApplyProperties(
@@ -1543,7 +1563,7 @@ void StyleResolver::ApplyProperties(
     if (!CSSPropertyPriorityData<priority>::PropertyHasPriority(property_id))
       continue;
 
-    StyleBuilder::ApplyProperty(current.Property(), state, current.Value());
+    ApplyProperty<priority>(current, state);
   }
 }