Bug 1278136 - Part 5: Create a stacking context for opacity/transform animations even if it's in delay phase and even if the property is overridden by !important rules. r=birtles
☠☠ backed out by fdf29e912685 ☠ ☠
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Wed, 12 Oct 2016 09:59:03 +0900
changeset 360432 62cf4a7d6007e3f0b1da0f0590c81a17791ec898
parent 360431 5f2db29e67ca5e8542675bf41b795eb107fda262
child 360433 2af033c15b03b9a16605fb900f1c958797b2dfed
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1278136
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 1278136 - Part 5: Create a stacking context for opacity/transform animations even if it's in delay phase and even if the property is overridden by !important rules. r=birtles This patch introduces a new functions named HasEffectiveAnimationOfProperty. This function checks that a given CSS property is overridden by !important rules. On the other hand, now KeyframeEffetReadOnly::HasAnimationOfProperty() does just check that the effect has a given CSS property. This is used to create a stacking context because we should create a stacking context for opacity or transform animations even if the property is overridden by !important rules. MozReview-Commit-ID: AG1Y0IgoB3U
dom/animation/EffectCompositor.cpp
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
layout/base/RestyleManager.cpp
layout/base/nsDisplayList.cpp
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsFrame.cpp
layout/reftests/css-animations/reftest.list
layout/reftests/css-transitions/reftest.list
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -143,17 +143,17 @@ FindAnimationsForCompositor(const nsIFra
         aMatches->Clear();
       }
       EffectCompositor::SetPerformanceWarning(
         aFrame, aProperty,
         AnimationPerformanceWarning(warningType));
       return false;
     }
 
-    if (!effect->HasAnimationOfProperty(aProperty)) {
+    if (!effect->HasEffectiveAnimationOfProperty(aProperty)) {
       continue;
     }
 
     if (aMatches) {
       aMatches->AppendElement(animation);
     }
     foundSome = true;
   }
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -187,17 +187,18 @@ KeyframeEffectReadOnly::SetKeyframes(nsT
 
   if (aStyleContext) {
     UpdateProperties(aStyleContext);
     MaybeUpdateFrameForCompositor();
   }
 }
 
 const AnimationProperty*
-KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSPropertyID aProperty) const
+KeyframeEffectReadOnly::GetEffectiveAnimationOfProperty(
+  nsCSSPropertyID aProperty) const
 {
   if (!IsInEffect()) {
     return nullptr;
   }
 
   EffectSet* effectSet =
     EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
   for (size_t propIdx = 0, propEnd = mProperties.Length();
@@ -214,16 +215,27 @@ KeyframeEffectReadOnly::GetAnimationOfPr
         result = nullptr;
       }
       return result;
     }
   }
   return nullptr;
 }
 
+bool
+KeyframeEffectReadOnly::HasAnimationOfProperty(nsCSSPropertyID aProperty) const
+{
+  for (const AnimationProperty& property : mProperties) {
+    if (property.mProperty == aProperty) {
+      return true;
+    }
+  }
+  return false;
+}
+
 #ifdef DEBUG
 bool
 SpecifiedKeyframeArraysAreEqual(const nsTArray<Keyframe>& aA,
                                 const nsTArray<Keyframe>& aB)
 {
   if (aA.Length() != aB.Length()) {
     return false;
   }
@@ -921,20 +933,22 @@ KeyframeEffectReadOnly::CanThrottle() co
   }
 
   // First we need to check layer generation and transform overflow
   // prior to the property.mIsRunningOnCompositor check because we should
   // occasionally unthrottle these animations even if the animations are
   // already running on compositor.
   for (const LayerAnimationInfo::Record& record :
         LayerAnimationInfo::sRecords) {
-    // Skip properties that are overridden in the cascade.
-    // (GetAnimationOfProperty, as called by HasAnimationOfProperty,
-    // only returns an animation if it currently wins in the cascade.)
-    if (!HasAnimationOfProperty(record.mProperty)) {
+    // Skip properties that are overridden by !important rules.
+    // (GetEffectiveAnimationOfProperty, as called by
+    // HasEffectiveAnimationOfProperty, only returns a property which is
+    // neither overridden by !important rules nor overridden by other
+    // animation.)
+    if (!HasEffectiveAnimationOfProperty(record.mProperty)) {
       continue;
     }
 
     EffectSet* effectSet = EffectSet::GetEffectSet(mTarget->mElement,
                                                    mTarget->mPseudoType);
     MOZ_ASSERT(effectSet, "CanThrottle should be called on an effect "
                           "associated with a target element");
     layers::Layer* layer =
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -230,22 +230,35 @@ public:
   void NotifyAnimationTimingUpdated();
 
   void SetAnimation(Animation* aAnimation) override;
 
   void SetKeyframes(JSContext* aContext, JS::Handle<JSObject*> aKeyframes,
                     ErrorResult& aRv);
   void SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
                     nsStyleContext* aStyleContext);
-  const AnimationProperty*
-  GetAnimationOfProperty(nsCSSPropertyID aProperty) const;
-  bool HasAnimationOfProperty(nsCSSPropertyID aProperty) const
+
+  // Returns true if the effect includes |aProperty| regardless of whether the
+  // property is overridden by !important rule.
+  bool HasAnimationOfProperty(nsCSSPropertyID aProperty) const;
+
+  // GetEffectiveAnimationOfProperty returns AnimationProperty corresponding
+  // to a given CSS property if the effect includes the property and the
+  // property is not overridden by !important rules.
+  // Also EffectiveAnimationOfProperty returns true under the same condition.
+  //
+  // NOTE: We don't currently check for !important rules for properties that
+  // can't run on the compositor.
+  bool HasEffectiveAnimationOfProperty(nsCSSPropertyID aProperty) const
   {
-    return GetAnimationOfProperty(aProperty) != nullptr;
+    return GetEffectiveAnimationOfProperty(aProperty) != nullptr;
   }
+  const AnimationProperty* GetEffectiveAnimationOfProperty(
+    nsCSSPropertyID aProperty) const;
+
   const InfallibleTArray<AnimationProperty>& Properties() const
   {
     return mProperties;
   }
 
   // Update |mProperties| by recalculating from |mKeyframes| using
   // |aStyleContext| to resolve specified values.
   void UpdateProperties(nsStyleContext* aStyleContext);
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1564,18 +1564,17 @@ ElementRestyler::AddLayerChangesForAnima
     // hint for such animations in each tick, i.e. restyles in each tick. As
     // a result, we usually do restyles for such animations in every tick on
     // the main-thread.  The only animations which will be affected by this
     // explicit change hint are animations that have opacity/transform but did
     // not have those properies just before. e.g,  setting transform by
     // setKeyframes or changing target element from other target which prevents
     // running on the compositor, etc.
     if (!layer &&
-        nsLayoutUtils::HasRelevantAnimationOfProperty(mFrame,
-                                                      layerInfo.mProperty)) {
+        nsLayoutUtils::HasEffectiveAnimation(mFrame, layerInfo.mProperty)) {
       hint |= layerInfo.mChangeHint;
     }
   }
   if (hint) {
     mChangeList->AppendChange(mFrame, mContent, hint);
   }
 }
 
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -482,31 +482,31 @@ AddAnimationsForProperty(nsIFrame* aFram
       continue;
     }
 
     dom::KeyframeEffectReadOnly* keyframeEffect =
       anim->GetEffect() ? anim->GetEffect()->AsKeyframeEffect() : nullptr;
     MOZ_ASSERT(keyframeEffect,
                "A playing animation should have a keyframe effect");
     const AnimationProperty* property =
-      keyframeEffect->GetAnimationOfProperty(aProperty);
+      keyframeEffect->GetEffectiveAnimationOfProperty(aProperty);
     if (!property) {
       continue;
     }
 
     // Note that if the property is overridden by !important rules,
-    // GetAnimationOfProperty returns null instead.
+    // GetEffectiveAnimationOfProperty returns null instead.
     // 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");
+               "GetEffectiveAnimationOfProperty 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.
     //
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -478,17 +478,17 @@ HasMatchingAnimations(const nsIFrame* aF
 bool
 nsLayoutUtils::HasActiveAnimationOfProperty(const nsIFrame* aFrame,
                                             nsCSSPropertyID aProperty)
 {
   return HasMatchingAnimations(aFrame,
     [&aProperty](KeyframeEffectReadOnly& aEffect)
     {
       return aEffect.IsCurrent() && aEffect.IsInEffect() &&
-        aEffect.HasAnimationOfProperty(aProperty);
+        aEffect.HasEffectiveAnimationOfProperty(aProperty);
     }
   );
 }
 
 bool
 nsLayoutUtils::HasCurrentTransitions(const nsIFrame* aFrame)
 {
   return HasMatchingAnimations(aFrame,
@@ -497,28 +497,41 @@ nsLayoutUtils::HasCurrentTransitions(con
       // Since |aEffect| is current, it must have an associated Animation
       // so we don't need to null-check the result of GetAnimation().
       return aEffect.IsCurrent() && aEffect.GetAnimation()->AsCSSTransition();
     }
   );
 }
 
 bool
-nsLayoutUtils::HasRelevantAnimationOfProperty(const nsIFrame* aFrame,
-                                              nsCSSPropertyID aProperty)
+nsLayoutUtils::HasAnimationOfProperty(const nsIFrame* aFrame,
+                                      nsCSSPropertyID aProperty)
 {
   return HasMatchingAnimations(aFrame,
     [&aProperty](KeyframeEffectReadOnly& aEffect)
     {
       return (aEffect.IsInEffect() || aEffect.IsCurrent()) &&
              aEffect.HasAnimationOfProperty(aProperty);
     }
   );
 }
 
+bool
+nsLayoutUtils::HasEffectiveAnimation(const nsIFrame* aFrame,
+                                     nsCSSPropertyID aProperty)
+{
+  return HasMatchingAnimations(aFrame,
+    [&aProperty](KeyframeEffectReadOnly& aEffect)
+    {
+      return (aEffect.IsInEffect() || aEffect.IsCurrent()) &&
+             aEffect.HasEffectiveAnimationOfProperty(aProperty);
+    }
+  );
+}
+
 static float
 GetSuitableScale(float aMaxScale, float aMinScale,
                  nscoord aVisibleDimension, nscoord aDisplayDimension)
 {
   float displayVisibleRatio = float(aDisplayDimension) /
                               float(aVisibleDimension);
   // We want to rasterize based on the largest scale used during the
   // transform animation, unless that would make us rasterize something
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2239,22 +2239,28 @@ public:
   /**
    * Returns true if the frame has any current CSS transitions.
    * A current transition is any transition that has not yet finished playing
    * including paused transitions.
    */
   static bool HasCurrentTransitions(const nsIFrame* aFrame);
 
   /**
-   * Returns true if the frame has current or in-effect (i.e. in before phase,
-   * running or filling) animations or transitions for the
-   * property.
+   * Returns true if |aFrame| has an animation of |aProperty| regardless of
+   * whether the property is overridden by !important rule.
    */
-  static bool HasRelevantAnimationOfProperty(const nsIFrame* aFrame,
-                                             nsCSSPropertyID aProperty);
+  static bool HasAnimationOfProperty(const nsIFrame* aFrame,
+                                     nsCSSPropertyID aProperty);
+
+  /**
+   * Returns true if |aFrame| has an animation of |aProperty| which is
+   * not overridden by !important rules.
+   */
+  static bool HasEffectiveAnimation(const nsIFrame* aFrame,
+                                    nsCSSPropertyID aProperty);
 
   /**
    * Checks if off-main-thread animations are enabled.
    */
   static bool AreAsyncAnimationsEnabled();
 
   /**
    * Checks if we should warn about animations that can't be async
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -549,18 +549,17 @@ nsFrame::Init(nsIContent*       aContent
 
     if (HasAnyStateBits(NS_FRAME_IN_POPUP) && TrackingVisibility()) {
       // Assume all frames in popups are visible.
       IncApproximateVisibleCount();
     }
   }
   const nsStyleDisplay *disp = StyleDisplay();
   if (disp->HasTransform(this) ||
-      nsLayoutUtils::HasRelevantAnimationOfProperty(this,
-                                                    eCSSProperty_transform)) {
+      nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_transform)) {
     // The frame gets reconstructed if we toggle the -moz-transform
     // property, so we can set this bit here and then ignore it.
     mState |= NS_FRAME_MAY_BE_TRANSFORMED;
   }
   if (disp->mPosition == NS_STYLE_POSITION_STICKY &&
       !aPrevInFlow &&
       !(mState & NS_FRAME_IS_NONDISPLAY) &&
       !disp->IsInnerTableStyle()) {
@@ -1135,31 +1134,30 @@ nsIFrame::GetMarginRectRelativeToSelf() 
 
 bool
 nsIFrame::IsTransformed() const
 {
   return ((mState & NS_FRAME_MAY_BE_TRANSFORMED) &&
           (StyleDisplay()->HasTransform(this) ||
            IsSVGTransformed() ||
            (mContent &&
-            nsLayoutUtils::HasRelevantAnimationOfProperty(
-              this, eCSSProperty_transform) &&
+            nsLayoutUtils::HasAnimationOfProperty(this,
+                                                  eCSSProperty_transform) &&
             IsFrameOfType(eSupportsCSSTransforms) &&
             mContent->GetPrimaryFrame() == this)));
 }
 
 bool
 nsIFrame::HasOpacityInternal(float aThreshold) const
 {
   MOZ_ASSERT(0.0 <= aThreshold && aThreshold <= 1.0, "Invalid argument");
   return StyleEffects()->mOpacity < aThreshold ||
          (StyleDisplay()->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) ||
          (mContent &&
-           nsLayoutUtils::HasRelevantAnimationOfProperty(
-             this, eCSSProperty_opacity) &&
+           nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_opacity) &&
            mContent->GetPrimaryFrame() == this);
 }
 
 bool
 nsIFrame::IsSVGTransformed(gfx::Matrix *aOwnTransforms,
                            gfx::Matrix *aFromParentTransforms) const
 {
   return false;
@@ -2110,18 +2108,17 @@ nsIFrame::BuildDisplayListForStackingCon
   // need to have display items built for them.
   bool needEventRegions =
     aBuilder->IsBuildingLayerEventRegions() &&
     StyleUserInterface()->GetEffectivePointerEvents(this) !=
       NS_STYLE_POINTER_EVENTS_NONE;
   bool opacityItemForEventsAndPluginsOnly = false;
   if (effects->mOpacity == 0.0 && aBuilder->IsForPainting() &&
       !(disp->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) &&
-      !nsLayoutUtils::HasActiveAnimationOfProperty(this,
-                                                   eCSSProperty_opacity)) {
+      !nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_opacity)) {
     if (needEventRegions ||
         aBuilder->WillComputePluginGeometry()) {
       opacityItemForEventsAndPluginsOnly = true;
     } else {
       return;
     }
   }
 
--- a/layout/reftests/css-animations/reftest.list
+++ b/layout/reftests/css-animations/reftest.list
@@ -9,18 +9,18 @@ fails != print-no-animations.html print-
 == partially-out-of-view-animation.html partially-out-of-view-animation-ref.html
 == animate-display-table-opacity.html animate-display-table-opacity-ref.html
 # We need to run 100% opacity test case when OMTA is disabled to check that the animation creates a stacking context even if the animation is not running on the compositor
 test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-context-opacity-1-animation.html stacking-context-animation-ref.html
 # We need to run transform:none test case when OMTA is disabled to check that the animation creates a stacking context even if the animation is not running on the compositor
 test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-context-transform-none-animation.html stacking-context-animation-ref.html
 == no-stacking-context-opacity-removing-animation-in-delay.html no-stacking-context-animation-ref.html
 == no-stacking-context-transform-removing-animation-in-delay.html no-stacking-context-animation-ref.html
-fails == stacking-context-lose-opacity-1.html stacking-context-animation-ref.html
-fails == stacking-context-lose-transform-none.html stacking-context-animation-ref.html
+== stacking-context-lose-opacity-1.html stacking-context-animation-ref.html
+== stacking-context-lose-transform-none.html stacking-context-animation-ref.html
 == stacking-context-opacity-win-in-delay.html stacking-context-animation-ref.html
 == stacking-context-opacity-win-in-delay-on-main-thread.html stacking-context-animation-ref.html
 == stacking-context-opacity-wins-over-transition.html stacking-context-animation-ref.html
 == stacking-context-transform-win-in-delay.html stacking-context-animation-ref.html
 == stacking-context-transform-win-in-delay-on-main-thread.html stacking-context-animation-ref.html
 == stacking-context-transform-wins-over-transition.html stacking-context-animation-ref.html
 == stacking-context-opacity-1-animation.html stacking-context-animation-ref.html
 == stacking-context-opacity-1-with-fill-backwards.html stacking-context-animation-ref.html
@@ -28,16 +28,16 @@ fails == stacking-context-lose-transform
 == stacking-context-paused-on-opacity-1.html stacking-context-animation-ref.html
 == stacking-context-paused-on-transform-none.html stacking-context-animation-ref.html
 == stacking-context-transform-none-animation.html stacking-context-animation-ref.html
 == stacking-context-transform-none-animation-on-svg.html  stacking-context-animation-ref.html
 == stacking-context-transform-none-animation-with-backface-visibility.html stacking-context-animation-ref.html
 == stacking-context-transform-none-animation-with-preserve-3d.html stacking-context-animation-ref.html
 == stacking-context-transform-none-with-fill-backwards.html stacking-context-animation-ref.html
 == stacking-context-transform-none-with-fill-forwards.html stacking-context-animation-ref.html
-fails == stacking-context-opacity-1-in-delay.html stacking-context-animation-ref.html # bug 1278136 and bug 1279403
-fails == stacking-context-opacity-removing-important-in-delay.html stacking-context-animation-ref.html
-fails == stacking-context-transform-none-in-delay.html stacking-context-animation-ref.html # bug 1278136 and bug 1279403
-fails == stacking-context-transform-removing-important-in-delay.html stacking-context-animation-ref.html
+== stacking-context-opacity-1-in-delay.html stacking-context-animation-ref.html
+== stacking-context-opacity-removing-important-in-delay.html stacking-context-animation-ref.html
+== stacking-context-transform-none-in-delay.html stacking-context-animation-ref.html
+== stacking-context-transform-removing-important-in-delay.html stacking-context-animation-ref.html
 == background-position-in-delay.html background-position-ref.html
 == background-position-after-finish.html background-position-ref.html
 fails == background-position-running.html background-position-ref.html # This test fails the reftest-opaque-layer check since animating background-position currently creates an active layer, and reftest-opaque-layer only handles items assigned to PaintedLayers.
 fails == background-position-important.html background-position-ref.html # This test fails the reftest-opaque-layer check since animating background-position overridden by a non-animated !important style also creates an active layer, and reftest-opaque-layer only handles items that are assigned to PaintedLayers.
--- a/layout/reftests/css-transitions/reftest.list
+++ b/layout/reftests/css-transitions/reftest.list
@@ -1,8 +1,8 @@
 == transitions-inline-already-wrapped-1.html transitions-inline-ref.html
 == transitions-inline-already-wrapped-2.html transitions-inline-ref.html
 == transitions-inline-rewrap-1.html transitions-inline-ref.html
 == transitions-inline-rewrap-2.html transitions-inline-ref.html
-fails == stacking-context-opacity-lose-to-animation.html stacking-context-transition-ref.html
-fails == stacking-context-transform-lose-to-animation.html stacking-context-transition-ref.html
+== stacking-context-opacity-lose-to-animation.html stacking-context-transition-ref.html
+== stacking-context-transform-lose-to-animation.html stacking-context-transition-ref.html
 == stacking-context-opacity-wins-over-important-style.html stacking-context-transition-ref.html
 == stacking-context-transform-wins-over-important-style.html stacking-context-transition-ref.html