[css-properties-values-api] Refactor setting of custom properties.

We currently have lots of different functions on ComputedStyle for setting
registered properties in different states (resolved/unresolved, inherited/
non-inherited). This is not terribly useful, and mostly just makes the
calling code much more complicated.

Also, the current implementation of Variable::ApplyValue is hard to
understand. This CL refactors that using the new setters on ComputedStyle.

R=futhark@chromium.org

Bug: 641877
Change-Id: I5fc6bed24ec5aadd469a81353dc3b4e221e0f9d1
Reviewed-on: https://chromium-review.googlesource.com/1236004
Commit-Queue: Anders Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593141}

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

@@ -314,13 +314,9 @@ void CSSInterpolationType::ApplyCustomPropertyValue(
   ComputedStyle& style = *state.Style();
   const PropertyHandle property = GetProperty();
   const AtomicString& property_name = property.CustomPropertyName();
-  if (Registration().Inherits()) {
-    style.SetResolvedInheritedVariable(property_name, std::move(variable_data),
-                                       css_value);
-  } else {
-    style.SetResolvedNonInheritedVariable(property_name,
-                                          std::move(variable_data), css_value);
-  }
+  bool inherits = Registration().Inherits();
+  style.SetVariable(property_name, std::move(variable_data), inherits);
+  style.SetRegisteredVariable(property_name, css_value, inherits);
 }
 
 }  // namespace blink

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

@@ -11,6 +11,41 @@
 
 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()->GetRegisteredVariable(
+                           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 =
@@ -26,71 +61,27 @@ void Variable::ApplyValue(StyleResolverState& state,
   bool inherit = declaration.IsInherit(is_inherited_property);
   DCHECK(!(initial && inherit));
 
-  if (!initial && !inherit) {
-    if (declaration.Value()->NeedsVariableResolution()) {
-      if (is_inherited_property) {
-        state.Style()->SetUnresolvedInheritedVariable(name,
-                                                      declaration.Value());
-      } else {
-        state.Style()->SetUnresolvedNonInheritedVariable(name,
-                                                         declaration.Value());
-      }
-      return;
-    }
-
-    if (!registration) {
-      state.Style()->SetResolvedUnregisteredVariable(name, declaration.Value());
-      return;
-    }
+  // 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);
 
-    const CSSValue* parsed_value = declaration.Value()->ParseForSyntax(
-        registration->Syntax(), state.GetDocument().GetSecureContextMode());
-    if (parsed_value) {
-      DCHECK(parsed_value);
-      if (is_inherited_property) {
-        state.Style()->SetResolvedInheritedVariable(name, declaration.Value(),
-                                                    parsed_value);
+    if (registration && !declaration.Value()->NeedsVariableResolution()) {
+      const CSSValue* parsed_value = declaration.Value()->ParseForSyntax(
+          registration->Syntax(), state.GetDocument().GetSecureContextMode());
+      if (parsed_value) {
+        state.Style()->SetRegisteredVariable(name, parsed_value,
+                                             is_inherited_property);
+      } else if (is_inherited_property) {
+        ApplyInheritValue(state, name, registration);
       } else {
-        state.Style()->SetResolvedNonInheritedVariable(
-            name, declaration.Value(), parsed_value);
+        ApplyInitialValue(state, name, registration);
       }
-      return;
-    }
-    if (is_inherited_property)
-      inherit = true;
-    else
-      initial = true;
-  }
-  DCHECK(initial ^ inherit);
-
-  state.Style()->RemoveVariable(name, is_inherited_property);
-  if (initial) {
-    return;
-  }
-
-  DCHECK(inherit);
-  CSSVariableData* parent_value =
-      state.ParentStyle()->GetVariable(name, is_inherited_property);
-  const CSSValue* parent_css_value =
-      registration && parent_value ? state.ParentStyle()->GetRegisteredVariable(
-                                         name, is_inherited_property)
-                                   : nullptr;
-
-  if (!is_inherited_property) {
-    DCHECK(registration);
-    if (parent_value) {
-      state.Style()->SetResolvedNonInheritedVariable(name, parent_value,
-                                                     parent_css_value);
-    }
-    return;
-  }
-
-  if (parent_value) {
-    if (!registration) {
-      state.Style()->SetResolvedUnregisteredVariable(name, parent_value);
-    } else {
-      state.Style()->SetResolvedInheritedVariable(name, parent_value,
-                                                  parent_css_value);
     }
   }
 }

third_party/blink/renderer/core/style/computed_style.cc

@@ -1664,49 +1664,22 @@ StyleNonInheritedVariables& ComputedStyle::MutableNonInheritedVariables() {
   return *variables;
 }
 
-void ComputedStyle::SetUnresolvedInheritedVariable(
-    const AtomicString& name,
-    scoped_refptr<CSSVariableData> value) {
-  DCHECK(value && value->NeedsVariableResolution());
-  MutableInheritedVariables().SetVariable(name, std::move(value));
-}
-
-void ComputedStyle::SetUnresolvedNonInheritedVariable(
-    const AtomicString& name,
-    scoped_refptr<CSSVariableData> value) {
-  DCHECK(value && value->NeedsVariableResolution());
-  MutableNonInheritedVariables().SetVariable(name, std::move(value));
-}
-
-void ComputedStyle::SetResolvedUnregisteredVariable(
-    const AtomicString& name,
-    scoped_refptr<CSSVariableData> value) {
-  DCHECK(value && !value->NeedsVariableResolution());
-  MutableInheritedVariables().SetVariable(name, std::move(value));
-}
-
-void ComputedStyle::SetResolvedInheritedVariable(
-    const AtomicString& name,
-    scoped_refptr<CSSVariableData> value,
-    const CSSValue* parsed_value) {
-  DCHECK(!!value == !!parsed_value);
-  DCHECK(!(value && value->NeedsVariableResolution()));
-
-  StyleInheritedVariables& variables = MutableInheritedVariables();
-  variables.SetVariable(name, std::move(value));
-  variables.SetRegisteredVariable(name, parsed_value);
+void ComputedStyle::SetVariable(const AtomicString& name,
+                                scoped_refptr<CSSVariableData> value,
+                                bool is_inherited_property) {
+  if (is_inherited_property)
+    MutableInheritedVariables().SetVariable(name, std::move(value));
+  else
+    MutableNonInheritedVariables().SetVariable(name, std::move(value));
 }
 
-void ComputedStyle::SetResolvedNonInheritedVariable(
-    const AtomicString& name,
-    scoped_refptr<CSSVariableData> value,
-    const CSSValue* parsed_value) {
-  DCHECK(!!value == !!parsed_value);
-  DCHECK(!(value && value->NeedsVariableResolution()));
-
-  StyleNonInheritedVariables& variables = MutableNonInheritedVariables();
-  variables.SetVariable(name, std::move(value));
-  variables.SetRegisteredVariable(name, parsed_value);
+void ComputedStyle::SetRegisteredVariable(const AtomicString& name,
+                                          const CSSValue* value,
+                                          bool is_inherited_property) {
+  if (is_inherited_property)
+    MutableInheritedVariables().SetRegisteredVariable(name, value);
+  else
+    MutableNonInheritedVariables().SetRegisteredVariable(name, value);
 }
 
 void ComputedStyle::RemoveVariable(const AtomicString& name,

third_party/blink/renderer/core/style/computed_style.h

@@ -1082,19 +1082,13 @@ class ComputedStyle : public ComputedStyleBase,
   CORE_EXPORT StyleInheritedVariables* InheritedVariables() const;
   CORE_EXPORT StyleNonInheritedVariables* NonInheritedVariables() const;
 
-  void SetUnresolvedInheritedVariable(const AtomicString&,
-                                      scoped_refptr<CSSVariableData>);
-  void SetUnresolvedNonInheritedVariable(const AtomicString&,
-                                         scoped_refptr<CSSVariableData>);
-
-  void SetResolvedUnregisteredVariable(const AtomicString&,
-                                       scoped_refptr<CSSVariableData>);
-  void SetResolvedInheritedVariable(const AtomicString&,
-                                    scoped_refptr<CSSVariableData>,
-                                    const CSSValue*);
-  void SetResolvedNonInheritedVariable(const AtomicString&,
-                                       scoped_refptr<CSSVariableData>,
-                                       const CSSValue*);
+  void SetVariable(const AtomicString&,
+                   scoped_refptr<CSSVariableData>,
+                   bool is_inherited_property);
+
+  void SetRegisteredVariable(const AtomicString&,
+                             const CSSValue*,
+                             bool is_inherited_property);
 
   void RemoveVariable(const AtomicString&, bool is_inherited_property);