Bug 1301305 - Factor out check for main-thread synchronization to a method on Animation; r=hiro
authorBrian Birtles <birtles@gmail.com>
Fri, 02 Dec 2016 10:13:06 +0900
changeset 325332 0a8a06811d79b1f8b8b7b961be055a1518bdb376
parent 325331 49a60bdadd4eeb1ac0594b095456f5566e499314
child 325333 dc0424e6e32de0cfca4cd5a218f8b11c129ae388
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewershiro
bugs1301305
milestone53.0a1
Bug 1301305 - Factor out check for main-thread synchronization to a method on Animation; r=hiro This should be easier to read and provide us a convenient place to check for other cases where we need to synchronize with the main thread (such as the change introduced in this bug where we synchronize with other animations started at the same time). MozReview-Commit-ID: 8iuA7P4ycwM
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/AnimationPerformanceWarning.h
dom/animation/EffectCompositor.cpp
dom/animation/KeyframeEffectReadOnly.cpp
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -775,16 +775,45 @@ Animation::CancelNoUpdate()
 
   UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   if (mTimeline) {
     mTimeline->RemoveAnimation(this);
   }
 }
 
+bool
+Animation::ShouldBeSynchronizedWithMainThread(
+  nsCSSPropertyID aProperty,
+  const nsIFrame* aFrame,
+  AnimationPerformanceWarning::Type& aPerformanceWarning) const
+{
+  // Only synchronize playing animations
+  if (!IsPlaying()) {
+    return false;
+  }
+
+  // Currently only transform animations need to be synchronized
+  if (aProperty != eCSSProperty_transform) {
+    return false;
+  }
+
+  // Is there anything about our effect itself that means it should be
+  // main-thread bound?
+  KeyframeEffectReadOnly* keyframeEffect = mEffect
+                                           ? mEffect->AsKeyframeEffect()
+                                           : nullptr;
+  if (!keyframeEffect) {
+    return false;
+  }
+
+  return keyframeEffect->
+           ShouldBlockAsyncTransformAnimations(aFrame, aPerformanceWarning);
+}
+
 void
 Animation::UpdateRelevance()
 {
   bool wasRelevant = mIsRelevant;
   mIsRelevant = HasCurrentEffect() || IsInEffect();
 
   // Notify animation observers.
   if (wasRelevant && !mIsRelevant) {
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_Animation_h
 #define mozilla_dom_Animation_h
 
 #include "nsWrapperCache.h"
 #include "nsCycleCollectionParticipant.h"
+#include "mozilla/AnimationPerformanceWarning.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/EffectCompositor.h" // For EffectCompositor::CascadeLevel
 #include "mozilla/LinkedList.h"
 #include "mozilla/TimeStamp.h" // for TimeStamp, TimeDuration
 #include "mozilla/dom/AnimationBinding.h" // for AnimationPlayState
 #include "mozilla/dom/AnimationEffectReadOnly.h"
 #include "mozilla/dom/AnimationTimeline.h"
@@ -30,17 +31,17 @@
 // GetTickCount().
 #ifdef GetCurrentTime
 #undef GetCurrentTime
 #endif
 
 struct JSContext;
 class nsCSSPropertyIDSet;
 class nsIDocument;
-class nsPresContext;
+class nsIFrame;
 
 namespace mozilla {
 
 class AnimValuesStyleRule;
 
 namespace dom {
 
 class CSSAnimation;
@@ -276,16 +277,21 @@ public:
 
   bool IsPlaying() const
   {
     return mPlaybackRate != 0.0 &&
            (PlayState() == AnimationPlayState::Running ||
             mPendingState == PendingState::PlayPending);
   }
 
+  bool ShouldBeSynchronizedWithMainThread(
+    nsCSSPropertyID aProperty,
+    const nsIFrame* aFrame,
+    AnimationPerformanceWarning::Type& aPerformanceWarning) const;
+
   bool IsRelevant() const { return mIsRelevant; }
   void UpdateRelevance();
 
   /**
    * Returns true if this Animation has a lower composite order than aOther.
    */
   bool HasLowerCompositeOrderThan(const Animation& aOther) const;
 
--- a/dom/animation/AnimationPerformanceWarning.h
+++ b/dom/animation/AnimationPerformanceWarning.h
@@ -4,16 +4,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_AnimationPerformanceWarning_h
 #define mozilla_dom_AnimationPerformanceWarning_h
 
 #include <initializer_list>
 
+#include "mozilla/Maybe.h"
+#include "nsTArray.h"
+
 class nsXPIDLString;
 
 namespace mozilla {
 
 // Represents the reason why we can't run the CSS property on the compositor.
 struct AnimationPerformanceWarning
 {
   enum class Type : uint8_t {
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -80,37 +80,35 @@ IsMatchForCompositor(const KeyframeEffec
 {
   const Animation* animation = aEffect.GetAnimation();
   MOZ_ASSERT(animation);
 
   if (!animation->IsRelevant()) {
     return MatchForCompositor::No;
   }
 
-  bool isPlaying = animation->IsPlaying();
-
-  // If we are finding animations for transform, check if there are other
-  // animations that should block the transform animation. e.g. geometric
-  // properties' animation. This check should be done regardless of whether
-  // the effect has the target property |aProperty| or not.
   AnimationPerformanceWarning::Type warningType;
-  if (aProperty == eCSSProperty_transform &&
-      isPlaying &&
-      aEffect.ShouldBlockAsyncTransformAnimations(aFrame, warningType)) {
+  if (animation->ShouldBeSynchronizedWithMainThread(aProperty, aFrame,
+                                                    warningType)) {
     EffectCompositor::SetPerformanceWarning(
       aFrame, aProperty,
       AnimationPerformanceWarning(warningType));
+    // For a given |aFrame|, we don't want some animations of |aProperty| to
+    // run on the compositor and others to run on the main thread, so if any
+    // need to be synchronized with the main thread, run them all there.
     return MatchForCompositor::NoAndBlockThisProperty;
   }
 
   if (!aEffect.HasEffectiveAnimationOfProperty(aProperty)) {
     return MatchForCompositor::No;
   }
 
-  return isPlaying ? MatchForCompositor::Yes : MatchForCompositor::IfNeeded;
+  return animation->IsPlaying()
+         ? MatchForCompositor::Yes
+         : MatchForCompositor::IfNeeded;
 }
 
 // Helper function to factor out the common logic from
 // GetAnimationsForCompositor and HasAnimationsForCompositor.
 //
 // Takes an optional array to fill with eligible animations.
 //
 // Returns true if there are eligible animations, false otherwise.
@@ -185,16 +183,19 @@ FindAnimationsForCompositor(const nsIFra
   }
 
   bool foundRunningAnimations = false;
   for (KeyframeEffectReadOnly* effect : *effects) {
     MatchForCompositor matchResult =
       IsMatchForCompositor(*effect, aProperty, aFrame);
 
     if (matchResult == MatchForCompositor::NoAndBlockThisProperty) {
+      // For a given |aFrame|, we don't want some animations of |aProperty| to
+      // run on the compositor and others to run on the main thread, so if any
+      // need to be synchronized with the main thread, run them all there.
       if (aMatches) {
         aMatches->Clear();
       }
       return false;
     }
 
     if (matchResult == MatchForCompositor::No) {
       continue;
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -1311,22 +1311,16 @@ KeyframeEffectReadOnly::CanAnimateTransf
   return true;
 }
 
 bool
 KeyframeEffectReadOnly::ShouldBlockAsyncTransformAnimations(
   const nsIFrame* aFrame,
   AnimationPerformanceWarning::Type& aPerformanceWarning) const
 {
-  // We currently only expect this method to be called for effects whose
-  // animations are eligible for the compositor since, Animations that are
-  // paused, zero-duration, finished etc. should not block other animations from
-  // running on the compositor.
-  MOZ_ASSERT(mAnimation && mAnimation->IsPlaying());
-
   EffectSet* effectSet =
     EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
   for (const AnimationProperty& property : mProperties) {
     // 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)