CSS Properties and Values API: Support non-inherited custom properties

third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp

Index: third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp
diff --git a/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp b/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp
index a9faee2eeb16c8d9afe3a28c8dd038dd3ce8ab86..0446291c7a21118797d1b6325e8ccd1e0029ebc8 100644
--- a/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp
+++ b/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp
@@ -70,7 +70,8 @@
 #include "core/style/QuotesData.h"
 #include "core/style/SVGComputedStyle.h"
 #include "core/style/StyleGeneratedImage.h"
-#include "core/style/StyleVariableData.h"
+#include "core/style/StyleInheritedVariables.h"
+#include "core/style/StyleNonInheritedVariables.h"
 #include "platform/fonts/FontDescription.h"
 #include "wtf/MathExtras.h"
 #include "wtf/PtrUtil.h"
@@ -804,47 +805,74 @@ void StyleBuilderFunctions::applyValueCSSPropertyVariable(StyleResolverState& st
     if (registry)
         registration = registry->registration(name);
 
-    switch (declaration.id()) {
-    case CSSValueInitial:
-        state.style()->removeVariable(name);
-        break;
+    bool initial = declaration.id() == CSSValueInitial
+        || (declaration.id() == CSSValueUnset && registration && !registration->inherits());
+    bool inherit = declaration.id() == CSSValueInherit
+        || (declaration.id() == CSSValueUnset && (!registration || registration->inherits()));

[meade_UTC10, 2016/09/28 04:23:29]

Can these be simplified by there being a method on CSSCustomPropertyDeclaration
and PropertyRegistry::Registration that's like "isInitial()" and "inherits()"?

[Timothy Loh, 2016/09/29 06:18:08]

Good idea -- I'll update CSSCustomPropertyDeclaration in a follow-up patch (this
one is big enough...) and I'll think about whether it makes sense to have a
default registration for unregistered properties.

+    DCHECK(!initial || !inherit);

[meade_UTC10, 2016/09/28 04:23:29]

equivalent to !(initial && inherit), but either way is fine

[Timothy Loh, 2016/09/29 06:18:08]

Changed (maybe it's marginally better with &&)

 
-    case CSSValueUnset:
-    case CSSValueInherit: {
-        state.style()->removeVariable(name);
-        StyleVariableData* parentVariables = state.parentStyle()->variables();
-        if (!parentVariables)
+    if (declaration.id() == CSSValueInternalVariableValue) {
+        if (declaration.value()->needsVariableResolution()) {
+            if (!registration || registration->inherits())
+                state.style()->setUnresolvedInheritedVariable(name, declaration.value());
+            else
+                state.style()->setUnresolvedNonInheritedVariable(name, declaration.value());
             return;
-        CSSVariableData* value = parentVariables->getVariable(name);
-        if (!value)
+        }
+
+        if (!registration) {
+            state.style()->setResolvedUnregisteredVariable(name, declaration.value());
             return;
-        state.style()->setVariable(name, value);
-        if (registration)
-            state.style()->setRegisteredInheritedProperty(name, parentVariables->registeredInheritedProperty(name));
-        break;
-    }
-    case CSSValueInternalVariableValue:
-        if (registration) {
-            if (declaration.value()->needsVariableResolution()) {
-                state.style()->setVariable(name, declaration.value());
-                return;
-            }
-            const CSSValue* parsedValue = declaration.value()->parseForSyntax(registration->syntax());
-            if (!parsedValue) {
-                state.style()->setVariable(name, nullptr);
-                state.style()->setRegisteredInheritedProperty(name, nullptr);
-                return;
-            }
+        }
+
+        const CSSValue* parsedValue = declaration.value()->parseForSyntax(registration->syntax());
+        if (parsedValue) {
             parsedValue = &StyleBuilderConverter::convertRegisteredPropertyValue(state, *parsedValue);
-            state.style()->setVariable(name, declaration.value());
-            state.style()->setRegisteredInheritedProperty(name, parsedValue);
+            DCHECK(parsedValue);
+            if (registration->inherits())
+                state.style()->setResolvedInheritedVariable(name, declaration.value(), parsedValue);
+            else
+                state.style()->setResolvedNonInheritedVariable(name, declaration.value(), parsedValue);
             return;
         }
-        state.style()->setVariable(name, declaration.value());
-        break;
-    default:
-        NOTREACHED();
+        if (registration->inherits())
+            inherit = true;
+        else
+            initial = true;
     }
+
+    DCHECK(initial ^ inherit);

[meade_UTC10, 2016/09/28 04:23:29]

Shouldn't this be ||? I realise it has the same effect, but it can be confusing
to use bitwise operations on regular bools.

[Timothy Loh, 2016/09/29 06:18:08]

We want exactly one of the flags set here but || allows both set. I could write
initial != inherit but I think ^ is clearer.

+
+    if (initial) {
+        if (!registration || registration->inherits())
+            state.style()->removeInheritedVariable(name);
+        else
+            state.style()->removeNonInheritedVariable(name);
+        return;
+    }
+
+    if (!registration || registration->inherits()) {
+        state.style()->removeInheritedVariable(name);
+        StyleInheritedVariables* parentVariables = state.parentStyle()->inheritedVariables();
+        if (!parentVariables)
+            return;
+        CSSVariableData* parentValue = parentVariables->getVariable(name);
+        if (parentValue) {
+            if (!registration)
+                state.style()->setResolvedUnregisteredVariable(name, parentValue);
+            else
+                state.style()->setResolvedInheritedVariable(name, parentValue, parentVariables->registeredVariable(name));
+        }
+        return;
+    }
+
+    state.style()->removeNonInheritedVariable(name);
+    StyleNonInheritedVariables* parentVariables = state.parentStyle()->nonInheritedVariables();
+    if (!parentVariables)
+        return;
+    CSSVariableData* parentValue = parentVariables->getVariable(name);
+    if (parentValue)
+        state.style()->setResolvedNonInheritedVariable(name, parentValue, parentVariables->registeredVariable(name));
 }
 
 void StyleBuilderFunctions::applyInheritCSSPropertyBaselineShift(StyleResolverState& state)