Bug 1273784 - Part 6: Factor out BuildProperties. r=hiro
authorBoris Chiou <boris.chiou@gmail.com>
Mon, 07 Nov 2016 17:01:39 +0800
changeset 435609 48ee0a796817c352ea4e8138fe89766bc17da6d0
parent 435608 2d93758d64e97e4b5bebd873feb7dbd36fe778bc
child 435610 3879ad77bbcfa7bbb89938d8f64151d54a21258e
push id35078
push usermbrubeck@mozilla.com
push dateTue, 08 Nov 2016 22:02:54 +0000
reviewershiro
bugs1273784
milestone52.0a1
Bug 1273784 - Part 6: Factor out BuildProperties. r=hiro Factor out BuildProperties to have a better readability of UpdateProperties. MozReview-Commit-ID: A3cIS65STAx
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -268,50 +268,17 @@ SpecifiedKeyframeArraysAreEqual(const ns
 }
 #endif
 
 void
 KeyframeEffectReadOnly::UpdateProperties(nsStyleContext* aStyleContext)
 {
   MOZ_ASSERT(aStyleContext);
 
-  nsTArray<AnimationProperty> properties;
-  if (mTarget) {
-    // When GetComputedKeyframeValues or GetAnimationPropertiesFromKeyframes
-    // calculate computed values from |mKeyframes|, they could possibly
-    // trigger a subsequent restyle in which we rebuild animations. If that
-    // happens we could find that |mKeyframes| is overwritten while it is
-    // being iterated over. Normally that shouldn't happen but just in case we
-    // make a copy of |mKeyframes| first and iterate over that instead.
-    auto keyframesCopy(mKeyframes);
-
-    nsTArray<ComputedKeyframeValues> computedValues =
-      KeyframeUtils::GetComputedKeyframeValues(keyframesCopy,
-                                               mTarget->mElement,
-                                               aStyleContext);
-
-    if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
-      KeyframeUtils::ApplySpacing(keyframesCopy, SpacingMode::paced,
-                                  mEffectOptions.mPacedProperty,
-                                  computedValues, aStyleContext);
-    }
-
-    properties =
-      KeyframeUtils::GetAnimationPropertiesFromKeyframes(keyframesCopy,
-                                                         computedValues,
-                                                         aStyleContext);
-
-#ifdef DEBUG
-    MOZ_ASSERT(SpecifiedKeyframeArraysAreEqual(mKeyframes, keyframesCopy),
-               "Apart from the computed offset members, the keyframes array"
-               " should not be modified");
-#endif
-
-    mKeyframes.SwapElements(keyframesCopy);
-  }
+  nsTArray<AnimationProperty> properties = BuildProperties(aStyleContext);
 
   if (mProperties == properties) {
     return;
   }
 
   // Preserve the state of the mIsRunningOnCompositor flag.
   nsCSSPropertyIDSet runningOnCompositorProperties;
 
@@ -645,16 +612,60 @@ KeyframeEffectReadOnly::ConstructKeyfram
   //       computed offsets and rebuild the animation properties.
   // FIXME: Bug 1314537: We have to make sure SharedKeyframeList is handled
   //        properly.
   effect->mKeyframes = aSource.mKeyframes;
   effect->mProperties = aSource.mProperties;
   return effect.forget();
 }
 
+nsTArray<AnimationProperty>
+KeyframeEffectReadOnly::BuildProperties(nsStyleContext* aStyleContext)
+{
+  MOZ_ASSERT(aStyleContext);
+
+  nsTArray<AnimationProperty> result;
+  // If mTarget is null, return an empty property array.
+  if (!mTarget) {
+    return result;
+  }
+
+  // When GetComputedKeyframeValues or GetAnimationPropertiesFromKeyframes
+  // calculate computed values from |mKeyframes|, they could possibly
+  // trigger a subsequent restyle in which we rebuild animations. If that
+  // happens we could find that |mKeyframes| is overwritten while it is
+  // being iterated over. Normally that shouldn't happen but just in case we
+  // make a copy of |mKeyframes| first and iterate over that instead.
+  auto keyframesCopy(mKeyframes);
+
+  nsTArray<ComputedKeyframeValues> computedValues =
+    KeyframeUtils::GetComputedKeyframeValues(keyframesCopy,
+                                             mTarget->mElement,
+                                             aStyleContext);
+
+  if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
+    KeyframeUtils::ApplySpacing(keyframesCopy, SpacingMode::paced,
+                                mEffectOptions.mPacedProperty,
+                                computedValues, aStyleContext);
+  }
+
+  result = KeyframeUtils::GetAnimationPropertiesFromKeyframes(keyframesCopy,
+                                                              computedValues,
+                                                              aStyleContext);
+
+#ifdef DEBUG
+  MOZ_ASSERT(SpecifiedKeyframeArraysAreEqual(mKeyframes, keyframesCopy),
+             "Apart from the computed offset members, the keyframes array"
+             " should not be modified");
+#endif
+
+  mKeyframes.SwapElements(keyframesCopy);
+  return result;
+}
+
 void
 KeyframeEffectReadOnly::UpdateTargetRegistration()
 {
   if (!mTarget) {
     return;
   }
 
   bool isRelevant = mAnimation && mAnimation->IsRelevant();
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -342,16 +342,21 @@ protected:
                           ErrorResult& aRv);
 
   template<class KeyframeEffectType>
   static already_AddRefed<KeyframeEffectType>
   ConstructKeyframeEffect(const GlobalObject& aGlobal,
                           KeyframeEffectReadOnly& aSource,
                           ErrorResult& aRv);
 
+  // Build properties by recalculating from |mKeyframes| using |aStyleContext|
+  // to resolve specified values. This function also applies paced spacing if
+  // needed.
+  nsTArray<AnimationProperty> BuildProperties(nsStyleContext* aStyleContext);
+
   // This effect is registered with its target element so long as:
   //
   // (a) It has a target element, and
   // (b) It is "relevant" (i.e. yet to finish but not idle, or finished but
   //     filling forwards)
   //
   // As a result, we need to make sure this gets called whenever anything
   // changes with regards to this effects's timing including changes to the