Bug 1518816 - Rework AnimationUtils::EffectSetContainsAnimatedScale to handle looking up the effect set correctly; r=hiro
authorBrian Birtles <birtles@gmail.com>
Mon, 18 Mar 2019 04:12:10 +0000
changeset 464774 e5bf7eaa0189769d3bbafaf04ded15e627bff9ce
parent 464773 a87a6c5b555072e97cb884326907eb70bd5b7b39
child 464775 aa46ab77475694e771a6988154b263bb56502b47
push id112481
push userrgurzau@mozilla.com
push dateMon, 18 Mar 2019 21:51:19 +0000
treeherdermozilla-inbound@6ed5e5a3e39e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1518816
milestone68.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 1518816 - Rework AnimationUtils::EffectSetContainsAnimatedScale to handle looking up the effect set correctly; r=hiro Differential Revision: https://phabricator.services.mozilla.com/D23284
dom/animation/AnimationUtils.cpp
dom/animation/AnimationUtils.h
dom/animation/KeyframeEffect.cpp
layout/painting/ActiveLayerTracker.cpp
--- a/dom/animation/AnimationUtils.cpp
+++ b/dom/animation/AnimationUtils.cpp
@@ -68,19 +68,24 @@ bool AnimationUtils::IsOffscreenThrottli
     Preferences::AddBoolVarCache(&sOffscreenThrottlingEnabled,
                                  "dom.animations.offscreen-throttling");
   }
 
   return sOffscreenThrottlingEnabled;
 }
 
 /* static */
-bool AnimationUtils::EffectSetContainsAnimatedScale(EffectSet& aEffects,
-                                                    const nsIFrame* aFrame) {
-  for (const dom::KeyframeEffect* effect : aEffects) {
+bool AnimationUtils::FrameHasAnimatedScale(const nsIFrame* aFrame) {
+  EffectSet* effectSet = EffectSet::GetEffectSetForFrame(
+      aFrame, nsCSSPropertyIDSet::TransformLikeProperties());
+  if (!effectSet) {
+    return false;
+  }
+
+  for (const dom::KeyframeEffect* effect : *effectSet) {
     if (effect->ContainsAnimatedScale(aFrame)) {
       return true;
     }
   }
 
   return false;
 }
 
--- a/dom/animation/AnimationUtils.h
+++ b/dom/animation/AnimationUtils.h
@@ -73,21 +73,19 @@ class AnimationUtils {
   static Document* GetDocumentFromGlobal(JSObject* aGlobalObject);
 
   /**
    * Checks if offscreen animation throttling is enabled.
    */
   static bool IsOffscreenThrottlingEnabled();
 
   /**
-   * Returns true if the given EffectSet contains a current effect that animates
-   * scale. |aFrame| is used for calculation of scale values.
+   * Returns true if the given frame has an animated scale.
    */
-  static bool EffectSetContainsAnimatedScale(EffectSet& aEffects,
-                                             const nsIFrame* aFrame);
+  static bool FrameHasAnimatedScale(const nsIFrame* aFrame);
 
   /**
    * Returns true if the given (pseudo-)element has any transitions that are
    * current (playing or waiting to play) or in effect (e.g. filling forwards).
    */
   static bool HasCurrentTransitions(const dom::Element* aElement,
                                     PseudoStyleType aPseudoType);
 };
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1696,16 +1696,23 @@ bool KeyframeEffect::HasComputedTimingCh
 bool KeyframeEffect::HasComputedTimingChanged() const {
   ComputedTiming computedTiming = GetComputedTiming();
   return HasComputedTimingChanged(
       computedTiming, mEffectOptions.mIterationComposite,
       mProgressOnLastCompose, mCurrentIterationOnLastCompose);
 }
 
 bool KeyframeEffect::ContainsAnimatedScale(const nsIFrame* aFrame) const {
+  // For display:table content, transform animations run on the table wrapper
+  // frame. If we are being passed a frame that doesn't support transforms
+  // (i.e. the inner table frame) we could just return false, but it possibly
+  // means we looked up the wrong EffectSet so for now we just assert instead.
+  MOZ_ASSERT(aFrame && aFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms),
+             "We should be passed a frame that supports transforms");
+
   if (!IsCurrent()) {
     return false;
   }
 
   for (const AnimationProperty& prop : mProperties) {
     if (prop.mProperty != eCSSProperty_transform &&
         prop.mProperty != eCSSProperty_scale &&
         prop.mProperty != eCSSProperty_rotate) {
--- a/layout/painting/ActiveLayerTracker.cpp
+++ b/layout/painting/ActiveLayerTracker.cpp
@@ -550,25 +550,17 @@ bool ActiveLayerTracker::IsOffsetStyleAn
 bool ActiveLayerTracker::IsScaleSubjectToAnimation(nsIFrame* aFrame) {
   // Check whether JavaScript is animating this frame's scale.
   LayerActivity* layerActivity = GetLayerActivity(aFrame);
   if (layerActivity &&
       layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_SCALE] >= 2) {
     return true;
   }
 
-  // Check if any animations, transitions, etc. associated with this frame may
-  // animate its scale.
-  EffectSet* effects = EffectSet::GetEffectSet(aFrame);
-  if (effects &&
-      AnimationUtils::EffectSetContainsAnimatedScale(*effects, aFrame)) {
-    return true;
-  }
-
-  return false;
+  return AnimationUtils::FrameHasAnimatedScale(aFrame);
 }
 
 /* static */
 void ActiveLayerTracker::NotifyContentChange(nsIFrame* aFrame) {
   LayerActivity* layerActivity = GetLayerActivityForUpdate(aFrame);
   layerActivity->mContentActive = true;
 }