Bug 1238660 part 2 - Preserve "wins in cascade" state when updating animations; r=hiro
authorBrian Birtles <birtles@gmail.com>
Wed, 13 Jan 2016 13:38:16 +0900
changeset 321365 fb40d887724cdae040a4abf28317f479071841fc
parent 321364 9955578aabd17893c5db02fd6ab20599af970e3d
child 321366 f58e7ca07205514611d65a65aba46cbf3db5cddf
push id9363
push userjlorenzo@mozilla.com
push dateWed, 13 Jan 2016 13:04:42 +0000
reviewershiro
bugs1238660
milestone46.0a1
Bug 1238660 part 2 - Preserve "wins in cascade" state when updating animations; r=hiro When updating animations, we shouldn't unnecessarily clobber the "wins in cascade" state of their properties since this can lead to unnecessary restyles when we then decide we need to update the cascade.
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
layout/style/nsAnimationManager.cpp
layout/style/nsAnimationManager.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -424,16 +424,41 @@ KeyframeEffectReadOnly::HasAnimationOfPr
     if (HasAnimationOfProperty(aProperties[i])) {
       return true;
     }
   }
   return false;
 }
 
 void
+KeyframeEffectReadOnly::CopyPropertiesFrom(const KeyframeEffectReadOnly& aOther)
+{
+  nsCSSPropertySet winningInCascadeProperties;
+  nsCSSPropertySet runningOnCompositorProperties;
+
+  for (const AnimationProperty& property : mProperties) {
+    if (property.mWinsInCascade) {
+      winningInCascadeProperties.AddProperty(property.mProperty);
+    }
+    if (property.mIsRunningOnCompositor) {
+      runningOnCompositorProperties.AddProperty(property.mProperty);
+    }
+  }
+
+  mProperties = aOther.mProperties;
+
+  for (AnimationProperty& property : mProperties) {
+    property.mWinsInCascade =
+      winningInCascadeProperties.HasProperty(property.mProperty);
+    property.mIsRunningOnCompositor =
+      runningOnCompositorProperties.HasProperty(property.mProperty);
+  }
+}
+
+void
 KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                                      nsCSSPropertySet& aSetProperties)
 {
   ComputedTiming computedTiming = GetComputedTiming();
   mProgressOnLastCompose = computedTiming.mProgress;
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -278,16 +278,20 @@ public:
   bool HasAnimationOfProperties(const nsCSSProperty* aProperties,
                                 size_t aPropertyCount) const;
   const InfallibleTArray<AnimationProperty>& Properties() const {
     return mProperties;
   }
   InfallibleTArray<AnimationProperty>& Properties() {
     return mProperties;
   }
+  // Copies the properties from another keyframe effect whilst preserving
+  // the mWinsInCascade and mIsRunningOnCompositor state of matching
+  // properties.
+  void CopyPropertiesFrom(const KeyframeEffectReadOnly& aOther);
 
   // Updates |aStyleRule| with the animation values produced by this
   // AnimationEffect for the current time except any properties already
   // contained in |aSetProperties|.
   // Any updated properties are added to |aSetProperties|.
   void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                     nsCSSPropertySet& aSetProperties);
   // Returns true if at least one property is being animated on compositor.
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -336,36 +336,16 @@ nsAnimationManager::SizeOfExcludingThis(
 }
 
 /* virtual */ size_t
 nsAnimationManager::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
   return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf);
 }
 
-void
-nsAnimationManager::CopyIsRunningOnCompositor(
-  KeyframeEffectReadOnly& aSourceEffect,
-  KeyframeEffectReadOnly& aDestEffect)
-{
-  nsCSSPropertySet sourceProperties;
-
-  for (AnimationProperty& property : aSourceEffect.Properties()) {
-    if (property.mIsRunningOnCompositor) {
-      sourceProperties.AddProperty(property.mProperty);
-    }
-  }
-
-  for (AnimationProperty& property : aDestEffect.Properties()) {
-    if (sourceProperties.HasProperty(property.mProperty)) {
-      property.mIsRunningOnCompositor = true;
-    }
-  }
-}
-
 nsIStyleRule*
 nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext,
                                        mozilla::dom::Element* aElement)
 {
   // Ignore animations for print or print preview, and for elements
   // that are not attached to the document tree.
   if (!mPresContext->IsDynamic() || !aElement->IsInComposedDoc()) {
     return nullptr;
@@ -460,23 +440,17 @@ nsAnimationManager::CheckAnimationRule(n
         // identity (and any expando properties attached to it).
         if (oldAnim->GetEffect() && newAnim->GetEffect()) {
           KeyframeEffectReadOnly* oldEffect = oldAnim->GetEffect();
           KeyframeEffectReadOnly* newEffect = newAnim->GetEffect();
           animationChanged =
             oldEffect->Timing() != newEffect->Timing() ||
             oldEffect->Properties() != newEffect->Properties();
           oldEffect->SetTiming(newEffect->Timing());
-
-          // To preserve the mIsRunningOnCompositor value on each property,
-          // we copy it from the old effect to the new effect since, in the
-          // following step, we will completely clobber the properties on the
-          // old effect with the values on the new effect.
-          CopyIsRunningOnCompositor(*oldEffect, *newEffect);
-          oldEffect->Properties() = newEffect->Properties();
+          oldEffect->CopyPropertiesFrom(*newEffect);
         }
 
         // Handle changes in play state. If the animation is idle, however,
         // changes to animation-play-state should *not* restart it.
         if (oldAnim->PlayState() != AnimationPlayState::Idle) {
           // CSSAnimation takes care of override behavior so that,
           // for example, if the author has called pause(), that will
           // override the animation-play-state.
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -355,15 +355,11 @@ private:
                        mozilla::AnimationPtrArray& aAnimations);
   bool BuildSegment(InfallibleTArray<mozilla::AnimationPropertySegment>&
                       aSegments,
                     nsCSSProperty aProperty,
                     const mozilla::StyleAnimation& aAnimation,
                     float aFromKey, nsStyleContext* aFromContext,
                     mozilla::css::Declaration* aFromDeclaration,
                     float aToKey, nsStyleContext* aToContext);
-
-  static void CopyIsRunningOnCompositor(
-    mozilla::dom::KeyframeEffectReadOnly& aSourceEffect,
-    mozilla::dom::KeyframeEffectReadOnly& aDestEffect);
 };
 
 #endif /* !defined(nsAnimationManager_h_) */