Web Animations: Use legacy cascade behaviour and the MatchedPropertiesCache

Source/core/css/resolver/StyleResolver.cpp

Index: Source/core/css/resolver/StyleResolver.cpp
diff --git a/Source/core/css/resolver/StyleResolver.cpp b/Source/core/css/resolver/StyleResolver.cpp
index 8e44e6b8da68b5274468ce28ab8cd59dc928ded5..c0a8b6cf1d3f70610a58c87a54412e0bb5da996d 100644
--- a/Source/core/css/resolver/StyleResolver.cpp
+++ b/Source/core/css/resolver/StyleResolver.cpp
@@ -594,9 +594,6 @@ void StyleResolver::matchAllRules(StyleResolverState& state, ElementRuleCollecto
         // Now check SMIL animation override style.
         if (includeSMILProperties && state.element()->isSVGElement())
             collector.addElementStyleProperties(toSVGElement(state.element())->animatedSMILStyleProperties(), false /* isCacheable */);
-
-        if (state.element()->hasActiveAnimations())
-            collector.matchedResult().isCacheable = false;
     }
 }
 
@@ -1053,6 +1050,23 @@ void StyleResolver::collectPseudoRulesForElement(Element* element, ElementRuleCo
 // -------------------------------------------------------------------------------------
 // this is mostly boring stuff on how to apply a certain rule to the renderstyle...
 
+void StyleResolver::applyAnimatedProperties(StyleResolverState& state, Element* animatingElement)
+{
+    // animatingElement may be null, for example if we're calculating the
+    // style for a potential pseudo element that has yet to be created.
+    if (!RuntimeEnabledFeatures::webAnimationsCSSEnabled() || !animatingElement)
+        return;
+    state.setAnimationUpdate(CSSAnimations::calculateUpdate(animatingElement, *state.style(), this));
+    if (state.animationUpdate()) {

[Steve Block, 2013/11/14 03:31:18]

Early-out?

[Timothy Loh, 2013/11/14 03:54:01]

Done.

+        const AnimationEffect::CompositableValueMap& compositableValuesForAnimations = state.animationUpdate()->compositableValuesForAnimations();
+        const AnimationEffect::CompositableValueMap& compositableValuesForTransitions = state.animationUpdate()->compositableValuesForTransitions();
+        applyAnimatedProperties<HighPriorityProperties>(state, compositableValuesForAnimations);
+        applyAnimatedProperties<HighPriorityProperties>(state, compositableValuesForTransitions);
+        applyAnimatedProperties<LowPriorityProperties>(state, compositableValuesForAnimations);
+        applyAnimatedProperties<LowPriorityProperties>(state, compositableValuesForTransitions);
+    }
+}
+
 template <StyleResolver::StyleApplicationPass pass>
 bool StyleResolver::applyAnimatedProperties(StyleResolverState& state, const AnimationEffect::CompositableValueMap& compositableValues)
 {
@@ -1259,6 +1273,12 @@ void StyleResolver::applyMatchedProperties(StyleResolverState& state, const Matc
 
             // Unfortunately the link status is treated like an inherited property. We need to explicitly restore it.
             state.style()->setInsideLink(linkStatus);
+
+            if (RuntimeEnabledFeatures::webAnimationsCSSEnabled()
+                && (animatingElement->hasActiveAnimations()
+                    || (state.style()->transitions() && !state.style()->transitions()->isEmpty())
+                    || (state.style()->animations() && !state.style()->animations()->isEmpty())))
+                applyAnimatedProperties(state, animatingElement);
             return;
         }
         applyInheritedOnly = true;
@@ -1320,43 +1340,17 @@ void StyleResolver::applyMatchedProperties(StyleResolverState& state, const Matc
     applyMatchedProperties<LowPriorityProperties>(state, matchResult, true, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
     applyMatchedProperties<LowPriorityProperties>(state, matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
 
-    // animatingElement may be null, for example if we're calculating the
-    // style for a potential pseudo element that has yet to be created.
-    if (RuntimeEnabledFeatures::webAnimationsEnabled() && animatingElement) {
-        state.setAnimationUpdate(CSSAnimations::calculateUpdate(animatingElement, *state.style(), this));
-        if (state.animationUpdate()) {
-            ASSERT(!applyInheritedOnly);
-            const AnimationEffect::CompositableValueMap& compositableValuesForAnimations = state.animationUpdate()->compositableValuesForAnimations();
-            const AnimationEffect::CompositableValueMap& compositableValuesForTransitions = state.animationUpdate()->compositableValuesForTransitions();
-            // Apply animated properties, then reapply any rules marked important.
-            if (applyAnimatedProperties<HighPriorityProperties>(state, compositableValuesForAnimations)) {
-                bool important = true;
-                applyMatchedProperties<HighPriorityProperties>(state, matchResult, important, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly);
-                applyMatchedProperties<HighPriorityProperties>(state, matchResult, important, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
-                applyMatchedProperties<HighPriorityProperties>(state, matchResult, important, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
-            }
-            applyAnimatedProperties<HighPriorityProperties>(state, compositableValuesForTransitions);
-            if (applyAnimatedProperties<LowPriorityProperties>(state, compositableValuesForAnimations)) {
-                bool important = true;
-                applyMatchedProperties<LowPriorityProperties>(state, matchResult, important, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly);
-                applyMatchedProperties<LowPriorityProperties>(state, matchResult, important, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
-                applyMatchedProperties<LowPriorityProperties>(state, matchResult, important, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
-            }
-            applyAnimatedProperties<LowPriorityProperties>(state, compositableValuesForTransitions);
-        }
-    }
-
     // Start loading resources referenced by this style.
     m_styleResourceLoader.loadPendingResources(state.style(), state.elementStyleResources());
 
-    ASSERT(!state.fontBuilder().fontDirty());
+    if (!cachedMatchedProperties && cacheHash && MatchedPropertiesCache::isCacheable(element, state.style(), state.parentStyle())) {
+        INCREMENT_STYLE_STATS_COUNTER(*this, matchedPropertyCacheAdded);
+        m_matchedPropertiesCache.add(state.style(), state.parentStyle(), cacheHash, matchResult);
+    }
 
-    if (cachedMatchedProperties || !cacheHash)
-        return;
-    if (!MatchedPropertiesCache::isCacheable(element, state.style(), state.parentStyle()))
-        return;
-    INCREMENT_STYLE_STATS_COUNTER(*this, matchedPropertyCacheAdded);
-    m_matchedPropertiesCache.add(state.style(), state.parentStyle(), cacheHash, matchResult);
+    applyAnimatedProperties(state, animatingElement);
+
+    ASSERT(!state.fontBuilder().fontDirty());

[Steve Block, 2013/11/14 03:31:18]

Did you intend to move this assert below the caching?

[Timothy Loh, 2013/11/14 03:54:01]

Yep; in particular below applying animated properties.

[Steve Block, 2013/11/14 04:07:33]

OK, makes sense

 }
 
 CSSPropertyValue::CSSPropertyValue(CSSPropertyID id, const StylePropertySet& propertySet)