Bug 1336772 - Request any restyles required by changes to the cascade result. r=birtles

When an animation is newly created while the same property transition is
running, the transition style rule persists until we call RequestRestyle() for
transitions level. That means if user calls getComputedStyle for the property
right after creating animation, the style obtained by getComputedStyle still
included the transitions level rule. As a result, the transitions level style
overrides newly created animation style until the next normal restyling process
happens (i.e. process transition level restyle request). Vice versa, in the
case where an animation is removed, transitions level style does not appear
until the next normal restyling.

This patch fixes this problem by trigerring a resyle of the transitions level
when an animation is created or removed.

MozReview-Commit-ID: HY6amLmDHTi

--HG--
extra : rebase_source : 67e58dc9a6c695299c3eef684bf7357153c5168b

dom/animation/EffectCompositor.cpp

@@ -881,14 +881,14 @@ EffectCompositor::UpdateCascadeResults(StyleBackendType aBackendType,
   std::bitset<LayerAnimationInfo::kRecords>
     prevCompositorPropertiesWithImportantRules =
       compositorPropertiesInSet(propertiesWithImportantRules);
-  std::bitset<LayerAnimationInfo::kRecords>
-    prevCompositorPropertiesForAnimationsLevel =
-      compositorPropertiesInSet(propertiesForAnimationsLevel);
+
+  nsCSSPropertyIDSet prevPropertiesForAnimationsLevel =
+    propertiesForAnimationsLevel;
 
   propertiesWithImportantRules.Empty();
   propertiesForAnimationsLevel.Empty();
 
-  bool hasCompositorPropertiesForTransition = false;
+  nsCSSPropertyIDSet propertiesForTransitionsLevel;
 
   for (const KeyframeEffectReadOnly* effect : sortedEffectList) {
     MOZ_ASSERT(effect->GetAnimation(),
@@ -899,14 +899,14 @@ EffectCompositor::UpdateCascadeResults(StyleBackendType aBackendType,
       if (overriddenProperties.HasProperty(prop.mProperty)) {
         propertiesWithImportantRules.AddProperty(prop.mProperty);
       }
-      if (cascadeLevel == EffectCompositor::CascadeLevel::Animations) {
-        propertiesForAnimationsLevel.AddProperty(prop.mProperty);
-      }
 
-      if (nsCSSProps::PropHasFlags(prop.mProperty,
-                                   CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR) &&
-          cascadeLevel == EffectCompositor::CascadeLevel::Transitions) {
-        hasCompositorPropertiesForTransition = true;
+      switch (cascadeLevel) {
+        case EffectCompositor::CascadeLevel::Animations:
+          propertiesForAnimationsLevel.AddProperty(prop.mProperty);
+          break;
+        case EffectCompositor::CascadeLevel::Transitions:
+          propertiesForTransitionsLevel.AddProperty(prop.mProperty);
+          break;
       }
     }
   }
@@ -929,15 +929,23 @@ EffectCompositor::UpdateCascadeResults(StyleBackendType aBackendType,
                      EffectCompositor::RestyleType::Layer,
                      EffectCompositor::CascadeLevel::Animations);
   }
-  // If we have transition properties for compositor and if the same propery
-  // for animations level is newly added or removed, we need to update layers
-  // for transitions level because composite order has been changed now.
-  if (hasCompositorPropertiesForTransition &&
-      prevCompositorPropertiesForAnimationsLevel !=
-        compositorPropertiesInSet(propertiesForAnimationsLevel)) {
+
+  // If we have transition properties and if the same propery for animations
+  // level is newly added or removed, we need to update the transition level
+  // rule since the it will be added/removed from the rule tree.
+  nsCSSPropertyIDSet changedPropertiesForAnimationLevel =
+    prevPropertiesForAnimationsLevel.Xor(propertiesForAnimationsLevel);
+  nsCSSPropertyIDSet commonProperties =
+    propertiesForTransitionsLevel.Intersect(
+      changedPropertiesForAnimationLevel);
+  if (!commonProperties.IsEmpty()) {
+    EffectCompositor::RestyleType restyleType =
+      compositorPropertiesInSet(changedPropertiesForAnimationLevel).none()
+      ? EffectCompositor::RestyleType::Standard
+      : EffectCompositor::RestyleType::Layer;
     presContext->EffectCompositor()->
       RequestRestyle(aElement, aPseudoType,
-                     EffectCompositor::RestyleType::Layer,
+                     restyleType,
                      EffectCompositor::CascadeLevel::Transitions);
   }
 }

dom/animation/EffectCompositor.h

@@ -280,7 +280,8 @@ private:
                           nsStyleContext* aStyleContext);
 
   // Update the mPropertiesWithImportantRules and
-  // mPropertiesForAnimationsLevel members of the given EffectSet.
+  // mPropertiesForAnimationsLevel members of the given EffectSet, and also
+  // request any restyles required by changes to the cascade result.
   //
   // This can be expensive so we should only call it if styles that apply
   // above the animation level of the cascade might have changed. For all

dom/animation/test/mochitest.ini

@@ -100,6 +100,7 @@ support-files =
 [css-transitions/test_setting-effect.html]
 [document-timeline/test_document-timeline.html]
 [document-timeline/test_request_animation_frame.html]
+[mozilla/test_cascade.html]
 [mozilla/test_cubic_bezier_limits.html]
 [mozilla/test_deferred_start.html]
 [mozilla/test_disable_animations_api_core.html]

dom/animation/test/mozilla/test_cascade.html

@@ -0,0 +1,37 @@
+<!doctype html>
+<meta charset=utf-8>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../testcommon.js"></script>
+<style>
+@keyframes margin-left {
+  from { margin-left: 20px; }
+  to   { margin-left: 80px; }
+}
+</style>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+test(function(t) {
+  var div = addDiv(t, { style: 'transition: margin-left 100s; ' +
+                               'margin-left: 80px' });
+  var cs = getComputedStyle(div);
+
+  assert_equals(cs.marginLeft, '80px', 'initial margin-left');
+
+  div.style.marginLeft = "20px";
+  assert_equals(cs.marginLeft, '80px', 'margin-left transition at 0s');
+
+  div.style.animation = "margin-left 2s";
+  assert_equals(cs.marginLeft, '20px',
+                'margin-left animation overrides transition at 0s');
+
+  div.style.animation = "none";
+  assert_equals(cs.marginLeft, '80px',
+                'margin-left animation stops overriding transition at 0s');
+}, 'Animation overrides/stops overriding transition immediately');
+
+</script>
+</body>

layout/style/nsCSSPropertyIDSet.h

@@ -67,6 +67,15 @@ public:
       return mozilla::PodEqual(mProperties, aOther.mProperties);
     }
 
+    bool IsEmpty() const {
+      for (size_t i = 0; i < mozilla::ArrayLength(mProperties); ++i) {
+          if (mProperties[i] != 0) {
+            return false;
+          }
+      }
+      return true;
+    }
+
     // Return a new nsCSSPropertyIDSet which is the inverse of this set.
     nsCSSPropertyIDSet Inverse() const {
       nsCSSPropertyIDSet result;
@@ -76,6 +85,26 @@ public:
       return result;
     }
 
+    // Returns a new nsCSSPropertyIDSet with all properties that are both in
+    // this set and |aOther|.
+    nsCSSPropertyIDSet Intersect(const nsCSSPropertyIDSet& aOther) const {
+      nsCSSPropertyIDSet result;
+      for (size_t i = 0; i < mozilla::ArrayLength(mProperties); ++i) {
+        result.mProperties[i] = mProperties[i] & aOther.mProperties[i];
+      }
+      return result;
+    }
+
+    // Return a new nsCSSPropertyIDSet with all properties that are in either
+    // this set or |aOther| but not both.
+    nsCSSPropertyIDSet Xor(const nsCSSPropertyIDSet& aOther) const {
+      nsCSSPropertyIDSet result;
+      for (size_t i = 0; i < mozilla::ArrayLength(mProperties); ++i) {
+        result.mProperties[i] = mProperties[i] ^ aOther.mProperties[i];
+      }
+      return result;
+    }
+
 private:
     typedef unsigned long property_set_type;
 public: