Bug 1479173 - Check mIsRunningOnCompositor flag before iterating LayerAnimationInfo. r=birtles
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 06 Nov 2018 21:01:50 +0000
changeset 444719 389edd54ab0f254aff479f5101627639510f9ee7
parent 444718 42bd72076aca1cc9f855501fbbf9dbf1f11b63bb
child 444720 a3dfabd0a69a45b8bfbbe3a886f15443e63922c3
push id35001
push userncsoregi@mozilla.com
push dateWed, 07 Nov 2018 09:52:11 +0000
treeherdermozilla-central@bc83ec5a338d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1479173
milestone65.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 1479173 - Check mIsRunningOnCompositor flag before iterating LayerAnimationInfo. r=birtles The comment there was wrong. We just bail out from there only if mIsRunningCompositor is false, so it doesn't matter whatever the layer generation check results. (i.e., we don't bail out in the case where mIsRunningCompositor is true). Also, we iterate over mProperties in the LayerAnimationInfo::sRecords loop through HasEffectiveAnimationOfProperty, so it doesn't matter that we iterate mProperties before the loop either. We will avoid the iteration in the sRecords loop in a subsequent patch in this series. Depends on D10692 Differential Revision: https://phabricator.services.mozilla.com/D10693
dom/animation/KeyframeEffect.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1237,20 +1237,22 @@ KeyframeEffect::CanThrottle() const
     // need to update on the main thread.
     return true;
   }
 
   if (CanThrottleIfNotVisible(*frame)) {
     return true;
   }
 
-  // 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 AnimationProperty& property : mProperties) {
+    if (!property.mIsRunningOnCompositor) {
+      return false;
+    }
+  }
+
   for (const LayerAnimationInfo::Record& record :
         LayerAnimationInfo::sRecords) {
     // 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)) {
@@ -1273,22 +1275,16 @@ KeyframeEffect::CanThrottle() const
     // If this is a transform animation that affects the overflow region,
     // we should unthrottle the animation periodically.
     if (HasPropertiesThatMightAffectOverflow() &&
         !CanThrottleOverflowChangesInScrollable(*frame)) {
       return false;
     }
   }
 
-  for (const AnimationProperty& property : mProperties) {
-    if (!property.mIsRunningOnCompositor) {
-      return false;
-    }
-  }
-
   return true;
 }
 
 bool
 KeyframeEffect::CanThrottleOverflowChanges(const nsIFrame& aFrame) const
 {
   TimeStamp now = aFrame.PresContext()->RefreshDriver()->MostRecentRefresh();