Simplify CSS variables resolution logic [2 of 2]

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

 // Copyright 2015 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 "core/css/resolver/CSSVariableResolver.h"
 
 #include "core/CSSPropertyNames.h"
 #include "core/CSSValueKeywords.h"
 #include "core/StyleBuilderFunctions.h"
 #include "core/StylePropertyShorthand.h"
@@ skipping 10 matching lines @@
 #include "wtf/Vector.h"
 
 namespace blink {
 
 bool CSSVariableResolver::resolveFallback(CSSParserTokenRange range, Vector<CSSParserToken>& result)
 {
     if (range.atEnd())
         return false;
     ASSERT(range.peek().type() == CommaToken);
     range.consume();
-    return resolveVariableReferencesFromTokens(range, result);
+    return resolveTokenRange(range, result);
 }
 
-bool CSSVariableResolver::resolveVariableTokensRecursive(CSSParserTokenRange range,
-    Vector<CSSParserToken>& result)
+CSSVariableData* CSSVariableResolver::valueForCustomProperty(AtomicString name)
 {
-    Vector<CSSParserToken> trash;
+    if (m_variablesSeen.contains(name)) {
+        m_cycleStartPoints.add(name);
+        return nullptr;
+    }
+
+    if (!m_styleVariableData)
+        return nullptr;
+    CSSVariableData* variableData = m_styleVariableData->getVariable(name);
+    if (!variableData)
+        return nullptr;
+    if (!variableData->needsVariableResolution())
+        return variableData;
+    RefPtr<CSSVariableData> newVariableData = resolveCustomProperty(name, *variableData);
+    m_styleVariableData->setVariable(name, newVariableData);
+    return newVariableData.get();
+}
+
+PassRefPtr<CSSVariableData> CSSVariableResolver::resolveCustomProperty(AtomicString name, const CSSVariableData& variableData)
+{
+    Vector<CSSParserToken> tokens;
+    m_variablesSeen.add(name);
+    bool success = resolveTokenRange(variableData.tokens(), tokens);
+    m_variablesSeen.remove(name);
+
+    // The old variable data holds onto the backing string the new resolved CSSVariableData
+    // relies on. Ensure it will live beyond us overwriting the RefPtr in StyleVariableData.
+    ASSERT(variableData.refCount() > 1);
+
+    if (!success || !m_cycleStartPoints.isEmpty()) {
+        m_cycleStartPoints.remove(name);
+        return nullptr;
+    }
+    return CSSVariableData::createResolved(tokens, variableData);
+}
+
+bool CSSVariableResolver::resolveVariableReference(CSSParserTokenRange range, Vector<CSSParserToken>& result)
+{
     range.consumeWhitespace();
     ASSERT(range.peek().type() == IdentToken);
     AtomicString variableName = range.consumeIncludingWhitespace().value();
     ASSERT(range.atEnd() || (range.peek().type() == CommaToken));
 
-    // Cycle detection.
-    if (m_variablesSeen.contains(variableName)) {
-        m_cycleStartPoints.add(variableName);
-        resolveFallback(range, trash);
-        return false;
-    }
+    CSSVariableData* variableData = valueForCustomProperty(variableName);
+    if (!variableData)
+        return resolveFallback(range, result);
 
-    CSSVariableData* variableData = m_styleVariableData ? m_styleVariableData->getVariable(variableName) : nullptr;
-    if (variableData) {
-        Vector<CSSParserToken> tokens;
-        if (variableData->needsVariableResolution()) {
-            m_variablesSeen.add(variableName);
-            bool referenceValid = resolveVariableReferencesFromTokens(variableData->tokens(), tokens);
-            m_variablesSeen.remove(variableName);
-
-            // The old variable data holds onto the backing string the new resolved CSSVariableData
-            // relies on. Ensure it will live beyond us overwriting the RefPtr in StyleVariableData.
-            ASSERT(variableData->refCount() > 1);
-
-            if (!referenceValid || !m_cycleStartPoints.isEmpty()) {
-                m_styleVariableData->setVariable(variableName, nullptr);
-                m_cycleStartPoints.remove(variableName);
-                if (!m_cycleStartPoints.isEmpty()) {
-                    resolveFallback(range, trash);
-                    return false;
-                }
-                return resolveFallback(range, result);
-            }
-
-            m_styleVariableData->setVariable(variableName, CSSVariableData::createResolved(tokens, variableData));
-        } else {
-            tokens = variableData->tokens();
-        }
-
-        ASSERT(!tokens.isEmpty());
-        // Check that loops are not induced by the fallback.
-        resolveFallback(range, trash);
-        if (m_cycleStartPoints.isEmpty()) {
-            // It's OK if the fallback fails to resolve - we're not actually taking it.
-            result.appendVector(tokens);
-            return true;
-        }
-        m_cycleStartPoints.remove(variableName);
-        return false;
-    }
-
-    return resolveFallback(range, result);
+    result.appendVector(variableData->tokens());
+    Vector<CSSParserToken> trash;
+    resolveFallback(range, trash);
+    return true;
 }
 
-bool CSSVariableResolver::resolveVariableReferencesFromTokens(CSSParserTokenRange range,
+bool CSSVariableResolver::resolveTokenRange(CSSParserTokenRange range,
     Vector<CSSParserToken>& result)
 {
     bool success = true;
     while (!range.atEnd()) {
         if (range.peek().functionId() == CSSValueVar) {
-            success &= resolveVariableTokensRecursive(range.consumeBlock(), result);
+            success &= resolveVariableReference(range.consumeBlock(), result);
         } else {
             result.append(range.consume());
         }
     }
     return success;
 }
 
 PassRefPtrWillBeRawPtr<CSSValue> CSSVariableResolver::resolveVariableReferences(StyleVariableData* styleVariableData, CSSPropertyID id, const CSSVariableReferenceValue& value)
 {
     ASSERT(!isShorthandProperty(id));
 
     CSSVariableResolver resolver(styleVariableData);
     Vector<CSSParserToken> tokens;
-    if (!resolver.resolveVariableReferencesFromTokens(value.variableDataValue()->tokens(), tokens))
+    if (!resolver.resolveTokenRange(value.variableDataValue()->tokens(), tokens))
         return cssValuePool().createUnsetValue();
 
     CSSParserContext context(HTMLStandardMode, nullptr);
     WillBeHeapVector<CSSProperty, 256> parsedProperties;
     // TODO(timloh): This should be CSSParser::parseSingleValue and not need a vector.
     if (!CSSPropertyParser::parseValue(id, false, CSSParserTokenRange(tokens), context, parsedProperties, StyleRule::Type::Style))
         return cssValuePool().createUnsetValue();
     ASSERT(parsedProperties.size() == 1);
     return parsedProperties[0].value();
 }
 
 void CSSVariableResolver::resolveAndApplyVariableReferences(StyleResolverState& state, CSSPropertyID id, const CSSVariableReferenceValue& value)
 {
-
-    // TODO(leviw): This should be a stack

[Timothy Loh, 2016/02/16 03:34:56]

I assume this means to unroll the recursion into iteration and use an explicit
stack. I don't think it'd be worthwhile to consider this unless lots of websites
started having custom property dependency chains which were thousands of custom
properties long...

     CSSVariableResolver resolver(state.style()->variables());
 
     Vector<CSSParserToken> tokens;
-    if (resolver.resolveVariableReferencesFromTokens(value.variableDataValue()->tokens(), tokens)) {
+    if (resolver.resolveTokenRange(value.variableDataValue()->tokens(), tokens)) {
         CSSParserContext context(HTMLStandardMode, 0);
 
         WillBeHeapVector<CSSProperty, 256> parsedProperties;
 
         if (CSSPropertyParser::parseValue(id, false, CSSParserTokenRange(tokens), context, parsedProperties, StyleRule::Type::Style)) {
             unsigned parsedPropertiesCount = parsedProperties.size();
             for (unsigned i = 0; i < parsedPropertiesCount; ++i)
                 StyleBuilder::applyProperty(parsedProperties[i].id(), state, parsedProperties[i].value());
             return;
         }
     }
 
     RefPtrWillBeRawPtr<CSSUnsetValue> unset = cssValuePool().createUnsetValue();
     if (isShorthandProperty(id)) {
         StylePropertyShorthand shorthand = shorthandForProperty(id);
         for (unsigned i = 0; i < shorthand.length(); i++)
             StyleBuilder::applyProperty(shorthand.properties()[i], state, unset.get());
         return;
     }
 
     StyleBuilder::applyProperty(id, state, unset.get());
 }
 
 void CSSVariableResolver::resolveVariableDefinitions(StyleVariableData* variables)
 {
     if (!variables)
         return;
 
+    CSSVariableResolver resolver(variables);
     for (auto& variable : variables->m_data) {
-        if (!variable.value || !variable.value->needsVariableResolution())
-            continue;
-        Vector<CSSParserToken> resolvedTokens;
-
-        CSSVariableResolver resolver(variables, variable.key);
-        if (resolver.resolveVariableReferencesFromTokens(variable.value->tokens(), resolvedTokens))
-            variable.value = CSSVariableData::createResolved(resolvedTokens, variable.value);
-        else
-            variable.value = nullptr;
+        if (variable.value && variable.value->needsVariableResolution())
+            variable.value = resolver.resolveCustomProperty(variable.key, *variable.value);
     }
 }
 
 CSSVariableResolver::CSSVariableResolver(StyleVariableData* styleVariableData)
     : m_styleVariableData(styleVariableData)
 {
 }
 
-CSSVariableResolver::CSSVariableResolver(StyleVariableData* styleVariableData, AtomicString& variable)
-    : m_styleVariableData(styleVariableData)
-{
-    m_variablesSeen.add(variable);
-}
-
 } // namespace blink