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
--- 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)