Bug 1304922 - Part 6: Introduce mPropertiesWithImportantRules and mPropertiesForAnimationsLevel and use it to detect conditions that we need to update layers. r=birtles
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Wed, 05 Oct 2016 14:42:56 +0900
changeset 316581 01bdca9df2c7f693ede320a0d5bd730921eb4907
parent 316580 04b7daffe4383dfa772201b4d2e0281409ea6e16
child 316582 fa352a014eb4b1034aec95196fd91cf1f79d9163
push id30778
push usercbook@mozilla.com
push dateThu, 06 Oct 2016 09:56:14 +0000
treeherdermozilla-central@cc3ee8d499c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1304922
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1304922 - Part 6: Introduce mPropertiesWithImportantRules and mPropertiesForAnimationsLevel and use it to detect conditions that we need to update layers. r=birtles This patch also makes composite order lowest to highest, as a result we also need to replace mWinsInCascade checks with the the properties. The mWinsInCascade membed itself will be removed in a subsequent patch. Now we call RequestRestyle(Layer) respectively for transition and animation, so a test case in test_restyles.html works as expected. And now lower-priority animations are also sent to the compositor so this patch fixed some tests in test_running_on_compositor.html and test_animation_performance_warning.html MozReview-Commit-ID: BchUsJbmatg
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/EffectCompositor.cpp
dom/animation/EffectCompositor.h
dom/animation/EffectSet.h
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
dom/animation/test/chrome/test_animation_performance_warning.html
dom/animation/test/chrome/test_restyles.html
dom/animation/test/chrome/test_running_on_compositor.html
layout/base/nsDisplayList.cpp
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -831,17 +831,17 @@ Animation::HasLowerCompositeOrderThan(co
 
   // 3. Finally, generic animations sort by their position in the global
   // animation array.
   return mAnimationIndex < aOther.mAnimationIndex;
 }
 
 void
 Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                        nsCSSPropertyIDSet& aSetProperties)
+                        const nsCSSPropertyIDSet& aPropertiesToSkip)
 {
   if (!mEffect) {
     return;
   }
 
   if (!IsInEffect()) {
     return;
   }
@@ -896,17 +896,17 @@ Animation::ComposeStyle(RefPtr<AnimValue
       if (!timeToUse.IsNull()) {
         mHoldTime.SetValue((timeToUse.Value() - mStartTime.Value())
                             .MultDouble(mPlaybackRate));
       }
     }
 
     KeyframeEffectReadOnly* keyframeEffect = mEffect->AsKeyframeEffect();
     if (keyframeEffect) {
-      keyframeEffect->ComposeStyle(aStyleRule, aSetProperties);
+      keyframeEffect->ComposeStyle(aStyleRule, aPropertiesToSkip);
     }
   }
 
   MOZ_ASSERT(playState == PlayState(),
              "Play state should not change during the course of compositing");
   mFinishedAtLastComposeStyle = (playState == AnimationPlayState::Finished);
 }
 
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -305,21 +305,21 @@ public:
    * Returns true if this animation does not currently need to update
    * style on the main thread (e.g. because it is empty, or is
    * running on the compositor).
    */
   bool CanThrottle() const;
   /**
    * Updates |aStyleRule| with the animation values of this animation's effect,
    * if any.
-   * Any properties already contained in |aSetProperties| are not changed. Any
-   * properties that are changed are added to |aSetProperties|.
+   * Any properties contained in |aPropertiesToSkip| will not be added or
+   * updated in |aStyleRule|.
    */
   void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                    nsCSSPropertyIDSet& aSetProperties);
+                    const nsCSSPropertyIDSet& aPropertiesToSkip);
 
   void NotifyEffectTimingUpdated();
 
 protected:
   void SilentlySetCurrentTime(const TimeDuration& aNewCurrentTime);
   void SilentlySetPlaybackRate(double aPlaybackRate);
   void CancelNoUpdate();
   void PlayNoUpdate(ErrorResult& aRv, LimitBehavior aLimitBehavior);
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -21,16 +21,17 @@
 #include "nsComputedDOMStyle.h" // nsComputedDOMStyle::GetPresShellForContent
 #include "nsCSSPropertyIDSet.h"
 #include "nsCSSProps.h"
 #include "nsIPresShell.h"
 #include "nsLayoutUtils.h"
 #include "nsRuleNode.h" // For nsRuleNode::ComputePropertiesOverridingAnimation
 #include "nsRuleProcessorData.h" // For ElementRuleProcessorData etc.
 #include "nsTArray.h"
+#include <bitset>
 
 using mozilla::dom::Animation;
 using mozilla::dom::Element;
 using mozilla::dom::KeyframeEffectReadOnly;
 
 namespace mozilla {
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(EffectCompositor)
@@ -580,38 +581,36 @@ EffectCompositor::ComposeAnimationRule(d
   if (!effects) {
     return;
   }
 
   // The caller is responsible for calling MaybeUpdateCascadeResults first.
   MOZ_ASSERT(!effects->CascadeNeedsUpdate(),
              "Animation cascade out of date when composing animation rule");
 
-  // Get a list of effects for the current level sorted by composite order.
-  nsTArray<KeyframeEffectReadOnly*> sortedEffectList;
+  // Get a list of effects sorted by composite order.
+  nsTArray<KeyframeEffectReadOnly*> sortedEffectList(effects->Count());
   for (KeyframeEffectReadOnly* effect : *effects) {
-    MOZ_ASSERT(effect->GetAnimation());
-    if (effect->GetAnimation()->CascadeLevel() == aCascadeLevel) {
-      sortedEffectList.AppendElement(effect);
-    }
+    sortedEffectList.AppendElement(effect);
   }
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
   RefPtr<AnimValuesStyleRule>& animationRule =
     effects->AnimationRule(aCascadeLevel);
   animationRule = nullptr;
 
-  // If multiple animations specify behavior for the same property the
-  // animation with the *highest* composite order wins.
-  // As a result, we iterate from last animation to first and, if a
-  // property has already been set, we don't change it.
-  nsCSSPropertyIDSet properties;
-
-  for (KeyframeEffectReadOnly* effect : Reversed(sortedEffectList)) {
-    effect->GetAnimation()->ComposeStyle(animationRule, properties);
+  // If multiple animations affect the same property, animations with higher
+  // composite order (priority) override or add or animations with lower
+  // priority except properties in propertiesToSkip.
+  const nsCSSPropertyIDSet& propertiesToSkip =
+    aCascadeLevel == CascadeLevel::Animations
+    ? nsCSSPropertyIDSet()
+    : effects->PropertiesForAnimationsLevel();
+  for (KeyframeEffectReadOnly* effect : sortedEffectList) {
+    effect->GetAnimation()->ComposeStyle(animationRule, propertiesToSkip);
   }
 
   MOZ_ASSERT(effects == EffectSet::GetEffectSet(aElement, aPseudoType),
              "EffectSet should not change while composing style");
 
   effects->UpdateAnimationRuleRefreshTime(aCascadeLevel, aRefreshTime);
 }
 
@@ -659,45 +658,91 @@ EffectCompositor::UpdateCascadeResults(E
   MOZ_ASSERT(EffectSet::GetEffectSet(aElement, aPseudoType) == &aEffectSet,
              "Effect set should correspond to the specified (pseudo-)element");
   if (aEffectSet.IsEmpty()) {
     aEffectSet.MarkCascadeUpdated();
     return;
   }
 
   // Get a list of effects sorted by composite order.
-  nsTArray<KeyframeEffectReadOnly*> sortedEffectList;
+  nsTArray<KeyframeEffectReadOnly*> sortedEffectList(aEffectSet.Count());
   for (KeyframeEffectReadOnly* effect : aEffectSet) {
     sortedEffectList.AppendElement(effect);
   }
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
   // Get properties that override the *animations* level of the cascade.
   //
   // We only do this for properties that we can animate on the compositor
   // since we will apply other properties on the main thread where the usual
   // cascade applies.
   nsCSSPropertyIDSet overriddenProperties;
   if (aStyleContext) {
     GetOverriddenProperties(aStyleContext, aEffectSet, overriddenProperties);
   }
 
-  bool changed = false;
-  nsCSSPropertyIDSet animatedProperties;
+  // Returns a bitset the represents which properties from
+  // LayerAnimationInfo::sRecords are present in |aPropertySet|.
+  auto compositorPropertiesInSet =
+    [](nsCSSPropertyIDSet& aPropertySet) ->
+      std::bitset<LayerAnimationInfo::kRecords> {
+        std::bitset<LayerAnimationInfo::kRecords> result;
+        for (size_t i = 0; i < LayerAnimationInfo::kRecords; i++) {
+          if (aPropertySet.HasProperty(
+                LayerAnimationInfo::sRecords[i].mProperty)) {
+            result.set(i);
+          }
+        }
+      return result;
+    };
 
-  // Iterate from highest to lowest composite order.
-  for (KeyframeEffectReadOnly* effect : Reversed(sortedEffectList)) {
+  nsCSSPropertyIDSet& propertiesWithImportantRules =
+    aEffectSet.PropertiesWithImportantRules();
+  nsCSSPropertyIDSet& propertiesForAnimationsLevel =
+    aEffectSet.PropertiesForAnimationsLevel();
+
+  // Record which compositor-animatable properties were originally set so we can
+  // compare for changes later.
+  std::bitset<LayerAnimationInfo::kRecords>
+    prevCompositorPropertiesWithImportantRules =
+      compositorPropertiesInSet(propertiesWithImportantRules);
+  std::bitset<LayerAnimationInfo::kRecords>
+    prevCompositorPropertiesForAnimationsLevel =
+      compositorPropertiesInSet(propertiesForAnimationsLevel);
+
+  propertiesWithImportantRules.Empty();
+  propertiesForAnimationsLevel.Empty();
+
+  nsCSSPropertyIDSet animatedProperties;
+  bool hasCompositorPropertiesForTransition = false;
+
+  for (KeyframeEffectReadOnly* effect : sortedEffectList) {
     MOZ_ASSERT(effect->GetAnimation(),
                "Effects on a target element should have an Animation");
     bool inEffect = effect->IsInEffect();
+    CascadeLevel cascadeLevel = effect->GetAnimation()->CascadeLevel();
+
     for (AnimationProperty& prop : effect->Properties()) {
 
       bool winsInCascade = !animatedProperties.HasProperty(prop.mProperty) &&
                            inEffect;
 
+      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;
+      }
+
       // If this property wins in the cascade, add it to the set of animated
       // properties. We need to do this even if the property is overridden
       // (in which case we set winsInCascade to false below) since we don't
       // want to fire transitions on these properties.
       if (winsInCascade) {
         animatedProperties.AddProperty(prop.mProperty);
       }
 
@@ -709,38 +754,48 @@ EffectCompositor::UpdateCascadeResults(E
       // the compositor. For properties animated on the main thread the usual
       // cascade ensures these animations will be correctly overridden.
       if (winsInCascade &&
           effect->GetAnimation()->CascadeLevel() == CascadeLevel::Animations &&
           overriddenProperties.HasProperty(prop.mProperty)) {
         winsInCascade = false;
       }
 
-      if (winsInCascade != prop.mWinsInCascade) {
-        changed = true;
-      }
       prop.mWinsInCascade = winsInCascade;
     }
   }
 
   aEffectSet.MarkCascadeUpdated();
 
-  // If there is any change in the cascade result, update animations on
-  // layers with the winning animations.
   nsPresContext* presContext = GetPresContext(aElement);
-  if (changed && presContext) {
-    // Update both transitions and animations. We could detect *which* levels
-    // actually changed and only update them, but that's probably unnecessary.
-    for (auto level : { CascadeLevel::Animations,
-                        CascadeLevel::Transitions }) {
-      presContext->EffectCompositor()->RequestRestyle(aElement,
-                                                      aPseudoType,
-                                                      RestyleType::Layer,
-                                                      level);
-    }
+  if (!presContext) {
+    return;
+  }
+
+  // If properties for compositor are newly overridden by !important rules, or
+  // released from being overridden by !important rules, we need to update
+  // layers for animations level because it's a trigger to send animations to
+  // the compositor or pull animations back from the compositor.
+  if (prevCompositorPropertiesWithImportantRules !=
+        compositorPropertiesInSet(propertiesWithImportantRules)) {
+    presContext->EffectCompositor()->
+      RequestRestyle(aElement, aPseudoType,
+                     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)) {
+    presContext->EffectCompositor()->
+      RequestRestyle(aElement, aPseudoType,
+                     EffectCompositor::RestyleType::Layer,
+                     EffectCompositor::CascadeLevel::Transitions);
   }
 }
 
 /* static */ nsPresContext*
 EffectCompositor::GetPresContext(Element* aElement)
 {
   MOZ_ASSERT(aElement);
   nsIPresShell* shell = nsComputedDOMStyle::GetPresShellForContent(aElement);
--- a/dom/animation/EffectCompositor.h
+++ b/dom/animation/EffectCompositor.h
@@ -180,18 +180,20 @@ public:
   //
   // This method does NOT detect if other styles that apply above the
   // animation level of the cascade have changed.
   static void
   MaybeUpdateCascadeResults(dom::Element* aElement,
                             CSSPseudoElementType aPseudoType,
                             nsStyleContext* aStyleContext);
 
-  // Update the mWinsInCascade member for each property in effects targetting
-  // the specified (pseudo-)element.
+  // Update the mWinsInCascade member for each property and in effects
+  // targetting the specified (pseudo-)element. Also updates the
+  // mPropertiesWithImportantRules and mPropertiesForAnimationsLevel members
+  // of the corresponding EffectSet.
   //
   // 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
   // other cases we should call MaybeUpdateCascadeResults.
   static void
   UpdateCascadeResults(dom::Element* aElement,
                        CSSPseudoElementType aPseudoType,
                        nsStyleContext* aStyleContext);
--- a/dom/animation/EffectSet.h
+++ b/dom/animation/EffectSet.h
@@ -185,16 +185,25 @@ public:
   void MarkCascadeNeedsUpdate() { mCascadeNeedsUpdate = true; }
   void MarkCascadeUpdated() { mCascadeNeedsUpdate = false; }
 
   void UpdateAnimationGeneration(nsPresContext* aPresContext);
   uint64_t GetAnimationGeneration() const { return mAnimationGeneration; }
 
   static nsIAtom** GetEffectSetPropertyAtoms();
 
+  nsCSSPropertyIDSet& PropertiesWithImportantRules()
+  {
+    return mPropertiesWithImportantRules;
+  }
+  nsCSSPropertyIDSet& PropertiesForAnimationsLevel()
+  {
+    return mPropertiesForAnimationsLevel;
+  }
+
 private:
   static nsIAtom* GetEffectSetPropertyAtom(CSSPseudoElementType aPseudoType);
 
   OwningEffectSet mEffects;
 
   // These style rules contain the style data for currently animating
   // values.  They only match when styling with animation.  When we
   // style without animation, we need to not use them so that we can
@@ -224,16 +233,24 @@ private:
   // RestyleManager keeps track of the number of animation restyles.
   // 'mini-flushes' (see nsTransitionManager::UpdateAllThrottledStyles()).
   // mAnimationGeneration is the sequence number of the last flush where a
   // transition/animation changed.  We keep a similar count on the
   // corresponding layer so we can check that the layer is up to date with
   // the animation manager.
   uint64_t mAnimationGeneration;
 
+  // Specifies the compositor-animatable properties that are overridden by
+  // !important rules.
+  nsCSSPropertyIDSet mPropertiesWithImportantRules;
+  // Specifies the properties for which the result will be added to the
+  // animations level of the cascade and hence should be skipped when we are
+  // composing the animation style for the transitions level of the cascede.
+  nsCSSPropertyIDSet mPropertiesForAnimationsLevel;
+
 #ifdef DEBUG
   // Track how many iterators are referencing this effect set when we are
   // destroyed, we can assert that nothing is still pointing to us.
   uint64_t mActiveIterators;
 
   bool mCalledPropertyDtor;
 #endif
 };
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -189,21 +189,33 @@ KeyframeEffectReadOnly::SetKeyframes(nsT
     UpdateProperties(aStyleContext);
     MaybeUpdateFrameForCompositor();
   }
 }
 
 const AnimationProperty*
 KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSPropertyID aProperty) const
 {
+  if (!IsInEffect()) {
+    return nullptr;
+  }
+
+  EffectSet* effectSet =
+    EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx) {
     if (aProperty == mProperties[propIdx].mProperty) {
       const AnimationProperty* result = &mProperties[propIdx];
-      if (!result->mWinsInCascade) {
+      // Skip if there is a property of animation level that is overridden
+      // by !important rules.
+      if (effectSet &&
+          effectSet->PropertiesWithImportantRules()
+            .HasProperty(result->mProperty) &&
+          effectSet->PropertiesForAnimationsLevel()
+            .HasProperty(result->mProperty)) {
         result = nullptr;
       }
       return result;
     }
   }
   return nullptr;
 }
 
@@ -299,18 +311,19 @@ KeyframeEffectReadOnly::UpdateProperties
   CalculateCumulativeChangeHint(aStyleContext);
 
   MarkCascadeNeedsUpdate();
 
   RequestRestyle(EffectCompositor::RestyleType::Layer);
 }
 
 void
-KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                                     nsCSSPropertyIDSet& aSetProperties)
+KeyframeEffectReadOnly::ComposeStyle(
+  RefPtr<AnimValuesStyleRule>& aStyleRule,
+  const nsCSSPropertyIDSet& aPropertiesToSkip)
 {
   ComputedTiming computedTiming = GetComputedTiming();
   mProgressOnLastCompose = computedTiming.mProgress;
   mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
   if (computedTiming.mProgress.IsNull()) {
@@ -321,36 +334,20 @@ KeyframeEffectReadOnly::ComposeStyle(Ref
        propIdx != propEnd; ++propIdx)
   {
     const AnimationProperty& prop = mProperties[propIdx];
 
     MOZ_ASSERT(prop.mSegments[0].mFromKey == 0.0, "incorrect first from key");
     MOZ_ASSERT(prop.mSegments[prop.mSegments.Length() - 1].mToKey == 1.0,
                "incorrect last to key");
 
-    if (aSetProperties.HasProperty(prop.mProperty)) {
-      // Animations are composed by EffectCompositor by iterating
-      // from the last animation to first. For animations targetting the
-      // same property, the later one wins. So if this property is already set,
-      // we should not override it.
+    if (aPropertiesToSkip.HasProperty(prop.mProperty)) {
       continue;
     }
 
-    if (!prop.mWinsInCascade) {
-      // This isn't the winning declaration, so don't add it to style.
-      // For transitions, this is important, because it's how we
-      // implement the rule that CSS transitions don't run when a CSS
-      // animation is running on the same property and element.  For
-      // animations, this is only skipping things that will otherwise be
-      // overridden.
-      continue;
-    }
-
-    aSetProperties.AddProperty(prop.mProperty);
-
     MOZ_ASSERT(prop.mSegments.Length() > 0,
                "property should not be in animations if it has no segments");
 
     // FIXME: Maybe cache the current segment?
     const AnimationPropertySegment *segment = prop.mSegments.Elements(),
                                 *segmentEnd = segment + prop.mSegments.Length();
     while (segment->mToKey <= computedTiming.mProgress.Value()) {
       MOZ_ASSERT(segment->mFromKey <= segment->mToKey, "incorrect keys");
@@ -1127,20 +1124,36 @@ KeyframeEffectReadOnly::ShouldBlockAsync
 {
   // We currently only expect this method to be called when this effect
   // is attached to a playing Animation. If that ever changes we'll need
   // to update this to only return true when that is the case since paused,
   // filling, cancelled Animations etc. shouldn't stop other Animations from
   // running on the compositor.
   MOZ_ASSERT(mAnimation && mAnimation->IsPlaying());
 
+  // Should not block other animations if this effect is not in-effect.
+  if (!IsInEffect()) {
+    return false;
+  }
+
+  EffectSet* effectSet =
+    EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
   for (const AnimationProperty& property : mProperties) {
-    // If a property is overridden in the CSS cascade, it should not block other
-    // animations from running on the compositor.
-    if (!property.mWinsInCascade) {
+    // If there is a property for animations level that is overridden by
+    // !important rules, it should not block other animations from running
+    // on the compositor.
+    // NOTE: We don't currently check for !important rules for properties that
+    // don't run on the compositor. As result such properties (e.g. margin-left)
+    // can still block async animations even if they are overridden by
+    // !important rules.
+    if (effectSet &&
+        effectSet->PropertiesWithImportantRules()
+          .HasProperty(property.mProperty) &&
+        effectSet->PropertiesForAnimationsLevel()
+          .HasProperty(property.mProperty)) {
       continue;
     }
     // Check for geometric properties
     if (IsGeometricProperty(property.mProperty)) {
       aPerformanceWarning =
         AnimationPerformanceWarning::Type::TransformWithGeometricProperties;
       return true;
     }
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -257,21 +257,20 @@ public:
     return mProperties;
   }
 
   // Update |mProperties| by recalculating from |mKeyframes| using
   // |aStyleContext| to resolve specified values.
   void UpdateProperties(nsStyleContext* aStyleContext);
 
   // Updates |aStyleRule| with the animation values produced by this
-  // AnimationEffect for the current time except any properties already
-  // contained in |aSetProperties|.
-  // Any updated properties are added to |aSetProperties|.
+  // AnimationEffect for the current time except any properties contained
+  // in |aPropertiesToSkip|.
   void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                    nsCSSPropertyIDSet& aSetProperties);
+                    const nsCSSPropertyIDSet& aPropertiesToSkip);
   // Returns true if at least one property is being animated on compositor.
   bool IsRunningOnCompositor() const;
   void SetIsRunningOnCompositor(nsCSSPropertyID aProperty, bool aIsRunning);
   void ResetIsRunningOnCompositor();
 
   // Returns true if this effect, applied to |aFrame|, contains properties
   // that mean we shouldn't run transform compositor animations on this element.
   //
--- a/dom/animation/test/chrome/test_animation_performance_warning.html
+++ b/dom/animation/test/chrome/test_animation_performance_warning.html
@@ -937,17 +937,17 @@ function start() {
   promise_test(function(t) {
     var div = addDiv(t, { class: 'compositable',
                           style: 'animation: fade 100s' });
     var cssAnimation = div.getAnimations()[0];
     var scriptAnimation = div.animate({ opacity: [ 1, 0 ] }, 100 * MS_PER_SEC);
     return scriptAnimation.ready.then(function() {
       assert_animation_property_state_equals(
         cssAnimation.effect.getProperties(),
-        [ { property: 'opacity', runningOnCompositor: false } ]);
+        [ { property: 'opacity', runningOnCompositor: true } ]);
       assert_animation_property_state_equals(
         scriptAnimation.effect.getProperties(),
         [ { property: 'opacity', runningOnCompositor: true } ]);
     });
   }, 'overridden animation');
 }
 
 </script>
--- a/dom/animation/test/chrome/test_restyles.html
+++ b/dom/animation/test/chrome/test_restyles.html
@@ -672,20 +672,17 @@ waitForAllPaints(function() {
     var markers = yield observeStyling(5);
 
     is(markers.length, 0,
        'Animations with no keyframes should not cause restyles');
 
     animation.effect.setKeyframes({ backgroundColor: ['red', 'blue'] });
     markers = yield observeStyling(5);
 
-    // There are 5 eRestyle_CSSAnimations and 1 eRestyle_CSSTransitions.
-    // 1 eRestyle_CSSTransitions comes from
-    // EffectCompositor::UpdateCascadeResults.
-    ok(markers.length == 6,
+    is(markers.length, 5,
        'Setting valid keyframes should cause regular animation restyles to ' +
        'occur');
 
     animation.effect.setKeyframes({ });
     markers = yield observeStyling(5);
 
     is(markers.length, 1,
        'Setting an empty set of keyframes should trigger a single restyle ' +
--- a/dom/animation/test/chrome/test_running_on_compositor.html
+++ b/dom/animation/test/chrome/test_running_on_compositor.html
@@ -567,105 +567,50 @@ promise_test(function(t) {
       'Opacity animation reports that it is running on the compositor after '
       + 'clearing the !important flag');
   });
 }, 'Clearing *important* opacity style on the target element sends the ' +
    'animation to the compositor');
 
 promise_test(function(t) {
   var div = addDiv(t);
-  var firstAnimation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
-  var secondAnimation = div.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
-
-  var another = addDiv(t);
-
-  return Promise.all([firstAnimation.ready, secondAnimation.ready]).then(function() {
-    assert_animation_is_running_on_compositor(secondAnimation,
-                  'The second opacity animation on an element reports that ' +
-                  'it is running on the compositor');
-    assert_animation_is_not_running_on_compositor(firstAnimation,
-                  'The first opacity animation on the same element reports ' +
-                  'that it is NOT running on the compositor');
+  var lowerAnimation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
+  var higherAnimation = div.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
 
-    firstAnimation.effect.target = another;
-    return waitForFrame();
-  }).then(function() {
-    assert_animation_is_running_on_compositor(secondAnimation,
-                  'The second opacity animation on the element keeps ' +
-                  'running on the compositor after the preiously overridden ' +
-                  'animation is applied to a different element');
-    assert_animation_is_running_on_compositor(firstAnimation,
-                  'The previously overridden opacity animation reports that ' +
-                  'it it running on the compositor after being applied to a ' +
-                  'different element');
+  return Promise.all([lowerAnimation.ready, higherAnimation.ready]).then(function() {
+    assert_animation_is_running_on_compositor(higherAnimation,
+                  'A higher-priority opacity animation on an element ' +
+                  'reports that it is running on the compositor');
+    assert_animation_is_running_on_compositor(lowerAnimation,
+                  'A lower-priority opacity animation on the same ' +
+                  'element also reports that it is running on the compositor');
   });
-}, 'Active animation which was not running on the compositor due to ' +
-   'composite order starts running on the compositor after changing ' +
-   'the target element');
+}, 'Opacity animations on the same element run on the compositor');
 
 promise_test(function(t) {
-  var div = addDiv(t);
-  var firstAnimation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
-  var secondAnimation = div.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
-
-  var another = addDiv(t);
+  var div = addDiv(t, { style: 'transition: opacity 100s; opacity: 1' });
 
-  return Promise.all([firstAnimation.ready, secondAnimation.ready]).then(function() {
-    assert_animation_is_running_on_compositor(secondAnimation,
-                  'The second opacity animation on an element reports that ' +
-                  'it is running on the compositor');
-    assert_animation_is_not_running_on_compositor(firstAnimation,
-                  'The first opacity animation on the same element reports ' +
-                  'that it is NOT running on the compositor');
+  getComputedStyle(div).opacity;
 
-    secondAnimation.effect.target = another;
-    return waitForFrame();
-  }).then(function() {
-    assert_animation_is_running_on_compositor(secondAnimation,
-                  'The second opacity animation continues to run on the ' +
-                  'compositor after being applied to a different element');
-    assert_animation_is_running_on_compositor(firstAnimation,
-                  'The previously overridden opacity animation now reports ' +
-                  'that it is running on the compositor after the animation ' +
-                  'that was overridding it is applied to a different element');
-  });
-}, 'Animation which was overridden and, as a result, not running on the ' +
-   'compositor begins running on the compositor after higher-priority ' +
-   'animation is applied to a different element');
+  div.style.opacity = 0;
+  getComputedStyle(div).opacity;
+
+  var transition = div.getAnimations()[0];
+  var animation = div.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
 
-promise_test(function(t) {
-  var div = addDiv(t);
-  var another = addDiv(t);
-
-  var animation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
-  var anotherAnimation = another.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
-
-  return Promise.all([animation.ready, anotherAnimation.ready]).then(function() {
-    assert_animation_is_running_on_compositor(anotherAnimation,
-                  'An opacity animation on an element reports that ' +
-                  'it is running on the compositor');
+  return Promise.all([transition.ready, animation.ready]).then(function() {
     assert_animation_is_running_on_compositor(animation,
-                  'Opacity animation running on a different element reports ' +
+                  'An opacity animation on an element reports that' +
                   'that it is running on the compositor');
-
-    anotherAnimation.effect.target = div;
-    return waitForFrame();
-  }).then(function() {
-    assert_animation_is_running_on_compositor(anotherAnimation,
-                  'Animation continues to run on the compositor after ' +
-                  'being applied to a different element with a ' +
-                  'lower-priority animation');
-    assert_animation_is_not_running_on_compositor(animation,
-                  'Animation stops running on the compositor after ' +
-                  'a higher-priority animation originally applied to ' +
-                  'a different element is applied to the same element');
+    assert_animation_is_running_on_compositor(transition,
+                  'An opacity transition on the same element reports that ' +
+                  'it is running on the compositor');
   });
-}, 'Moving a higher-priority animation to an element which already has the ' +
-   'same property animation running on the compositor makes the initial ' +
-   'animation stop running on the compositor');
+}, 'Both of transition and script animation on the same element run on the ' +
+   'compositor');
 
 promise_test(function(t) {
   var div = addDiv(t);
   var importantOpacityElement = addDiv(t, { style: "opacity: 1 ! important" });
 
   var animation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
 
   return animation.ready.then(function() {
@@ -710,28 +655,28 @@ promise_test(function(t) {
     return waitForFrame();
   }).then(function() {
     assert_animation_is_not_running_on_compositor(lowerAnimation,
                   'Animation is no longer running on the compositor after ' +
                   'being removed from the element');
     lowerAnimation.effect.target = another;
     return waitForFrame();
   }).then(function() {
-    assert_animation_is_not_running_on_compositor(lowerAnimation,
-                  'A lower-priority animation does NOT begin running ' +
+    assert_animation_is_running_on_compositor(lowerAnimation,
+                  'A lower-priority animation begins running ' +
                   'on the compositor after being applied to an element ' +
                   'which has a higher-priority animation');
     assert_animation_is_running_on_compositor(higherAnimation,
                   'A higher-priority animation continues to run on the ' +
                   'compositor even after a lower-priority animation is ' +
                   'applied to the same element');
   });
-}, 'Animation continues not running on the compositor after being applied ' +
+}, 'Animation begins running on the compositor after being applied ' +
    'to an element which has a higher-priority animation and after ' +
-   'being remporarily associated with no target element');
+   'being temporarily associated with no target element');
 
 promise_test(function(t) {
   var div = addDiv(t);
   var another = addDiv(t);
 
   var lowerAnimation = div.animate({ opacity: [1, 0] }, 100 * MS_PER_SEC);
   var higherAnimation = another.animate({ opacity: [0, 1] }, 100 * MS_PER_SEC);
 
@@ -747,22 +692,62 @@ promise_test(function(t) {
     return waitForFrame();
   }).then(function() {
     assert_animation_is_not_running_on_compositor(higherAnimation,
                   'Animation is no longer running on the compositor after ' +
                   'being removed from the element');
     higherAnimation.effect.target = div;
     return waitForFrame();
   }).then(function() {
-    assert_animation_is_not_running_on_compositor(lowerAnimation,
-                  'Animation stops running on the compositor after ' +
+    assert_animation_is_running_on_compositor(lowerAnimation,
+                  'Animation continues running on the compositor after ' +
                   'a higher-priority animation applied to the same element');
     assert_animation_is_running_on_compositor(higherAnimation,
                   'A higher-priority animation begins to running on the ' +
                   'compositor after being applied to an element which has ' +
                   'a lower-priority-animation');
   });
 }, 'Animation begins running on the compositor after being applied ' +
    'to an element which has a lower-priority animation once after ' +
    'disassociating with an element');
 
+promise_test(function(t) {
+  var div = addDiv(t, { style: 'transition: opacity 100s; ' +
+                               'opacity: 0 !important' });
+  getComputedStyle(div).opacity;
+
+  div.style.setProperty('opacity', '1', 'important');
+  getComputedStyle(div).opacity;
+
+  var animation = div.getAnimations()[0];
+
+  return animation.ready.then(function() {
+    assert_animation_is_running_on_compositor(animation,
+       'Transition reports that it is running on the compositor even if the ' +
+       'property is overridden by an !important rule');
+  });
+}, 'Transitions override important rules');
+
+promise_test(function(t) {
+  var div = addDiv(t, { style: 'transition: opacity 100s; ' +
+                               'opacity: 0 !important' });
+  getComputedStyle(div).opacity;
+
+  div.animate({ opacity: [ 0, 1 ] }, 10 * MS_PER_SEC);
+
+  div.style.setProperty('opacity', '1', 'important');
+  getComputedStyle(div).opacity;
+
+  var [transition, animation] = div.getAnimations();
+
+  return Promise.all([transition.ready, animation.ready]).then(function() {
+    assert_animation_is_not_running_on_compositor(transition,
+       'Transition suppressed by an animation which is overridden by an ' +
+       '!important rule reports that it is NOT running on the compositor');
+    assert_animation_is_not_running_on_compositor(animation,
+       'Animation overridden by an !important rule reports that it is ' +
+       'NOT running on the compositor');
+  });
+}, 'Neither transition nor animation does run on the compositor if the ' +
+   'property is overridden by an !important rule');
+
 </script>
 </body>
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -50,16 +50,17 @@
 #include "nsViewManager.h"
 #include "ImageLayers.h"
 #include "ImageContainer.h"
 #include "nsCanvasFrame.h"
 #include "StickyScrollContainer.h"
 #include "mozilla/AnimationPerformanceWarning.h"
 #include "mozilla/AnimationUtils.h"
 #include "mozilla/EffectCompositor.h"
+#include "mozilla/EffectSet.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/OperatorNewExtensions.h"
 #include "mozilla/PendingAnimationTracker.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "ActiveLayerTracker.h"
@@ -466,16 +467,19 @@ AddAnimationsForProperty(nsIFrame* aFram
                          nsTArray<RefPtr<dom::Animation>>& aAnimations,
                          Layer* aLayer, AnimationData& aData,
                          bool aPending)
 {
   MOZ_ASSERT(nsCSSProps::PropHasFlags(aProperty,
                                       CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR),
              "inconsistent property flags");
 
+  DebugOnly<EffectSet*> effects = EffectSet::GetEffectSet(aFrame);
+  MOZ_ASSERT(effects);
+
   // Add from first to last (since last overrides)
   for (size_t animIdx = 0; animIdx < aAnimations.Length(); animIdx++) {
     dom::Animation* anim = aAnimations[animIdx];
     if (!anim->IsPlaying()) {
       continue;
     }
 
     dom::KeyframeEffectReadOnly* keyframeEffect =
@@ -483,27 +487,26 @@ AddAnimationsForProperty(nsIFrame* aFram
     MOZ_ASSERT(keyframeEffect,
                "A playing animation should have a keyframe effect");
     const AnimationProperty* property =
       keyframeEffect->GetAnimationOfProperty(aProperty);
     if (!property) {
       continue;
     }
 
-    // Note that if mWinsInCascade on property was  false,
+    // Note that if the property is overridden by !important rules,
     // GetAnimationOfProperty returns null instead.
-    // This is what we want, since if we have an animation or transition
-    // that isn't actually winning in the CSS cascade, we don't want to
-    // send it to the compositor.
-    // I believe that anything that changes mWinsInCascade should
-    // trigger this code again, either because of a restyle that changes
-    // the properties in question, or because of the main-thread style
-    // update that results when an animation stops being in effect.
-    MOZ_ASSERT(property->mWinsInCascade,
-               "GetAnimationOfProperty already tested mWinsInCascade");
+    // This is what we want, since if we have animations overridden by
+    // !important rules, we don't want to send them to the compositor.
+    MOZ_ASSERT(anim->CascadeLevel() !=
+                 EffectCompositor::CascadeLevel::Animations ||
+               !effects->PropertiesWithImportantRules()
+                  .HasProperty(aProperty),
+               "GetAnimationOfProperty already tested the property is not "
+               "overridden by !important rules");
 
     // Don't add animations that are pending if their timeline does not
     // track wallclock time. This is because any pending animations on layers
     // will have their start time updated with the current wallclock time.
     // If we can't convert that wallclock time back to an equivalent timeline
     // time, we won't be able to update the content animation and it will end
     // up being out of sync with the layer animation.
     //