Bug 1273784 - Part 5: Avoid re-building the animation properties and re-calculating computed offsets. r=hiro
authorBoris Chiou <boris.chiou@gmail.com>
Thu, 03 Nov 2016 11:36:28 +0800
changeset 351567 2d93758d64e97e4b5bebd873feb7dbd36fe778bc
parent 351566 c543f2472f40a7f8c18f92087857034f7bed9aa7
child 351568 48ee0a796817c352ea4e8138fe89766bc17da6d0
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1273784
milestone52.0a1
Bug 1273784 - Part 5: Avoid re-building the animation properties and re-calculating computed offsets. r=hiro We don't need to rebuild the animation properties and recalculate the computed offsets of Keyframes while copy-constructing a new KeyframeEffect(ReadOnly) object, so avoid calling SetKeyframes() directly. And we also need a customized copy constructor for AnimationProperty to avoid copy mIsRunningOnCompositor. MozReview-Commit-ID: CIF3Ibgc1tM
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -635,22 +635,23 @@ KeyframeEffectReadOnly::ConstructKeyfram
     new KeyframeEffectType(doc,
                            aSource.mTarget,
                            aSource.SpecifiedTiming(),
                            aSource.mEffectOptions);
   // Copy cumulative change hint. mCumulativeChangeHint should be the same as
   // the source one because both of targets are the same.
   effect->mCumulativeChangeHint = aSource.mCumulativeChangeHint;
 
-  // Clone aSource's keyframes and then move it into the new effect.
+  // Copy aSource's keyframes and animation properties.
+  // Note: We don't call SetKeyframes directly, which might revise the
+  //       computed offsets and rebuild the animation properties.
   // FIXME: Bug 1314537: We have to make sure SharedKeyframeList is handled
   //        properly.
-  RefPtr<nsStyleContext> styleContext = effect->GetTargetStyleContext();
-  nsTArray<Keyframe> keyframes(aSource.mKeyframes);
-  effect->SetKeyframes(Move(keyframes), styleContext);
+  effect->mKeyframes = aSource.mKeyframes;
+  effect->mProperties = aSource.mProperties;
   return effect.forget();
 }
 
 void
 KeyframeEffectReadOnly::UpdateTargetRegistration()
 {
   if (!mTarget) {
     return;
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -116,24 +116,26 @@ struct Keyframe
 };
 
 struct AnimationPropertySegment
 {
   float mFromKey, mToKey;
   StyleAnimationValue mFromValue, mToValue;
   Maybe<ComputedTimingFunction> mTimingFunction;
 
-  bool operator==(const AnimationPropertySegment& aOther) const {
+  bool operator==(const AnimationPropertySegment& aOther) const
+  {
     return mFromKey == aOther.mFromKey &&
            mToKey == aOther.mToKey &&
            mFromValue == aOther.mFromValue &&
            mToValue == aOther.mToValue &&
            mTimingFunction == aOther.mTimingFunction;
   }
-  bool operator!=(const AnimationPropertySegment& aOther) const {
+  bool operator!=(const AnimationPropertySegment& aOther) const
+  {
     return !(*this == aOther);
   }
 };
 
 struct AnimationProperty
 {
   nsCSSPropertyID mProperty = eCSSProperty_UNKNOWN;
 
@@ -147,27 +149,41 @@ struct AnimationProperty
   // **NOTE**: This member is not included when comparing AnimationProperty
   // objects for equality.
   bool mIsRunningOnCompositor = false;
 
   Maybe<AnimationPerformanceWarning> mPerformanceWarning;
 
   InfallibleTArray<AnimationPropertySegment> mSegments;
 
+  // The copy constructor/assignment doesn't copy mIsRunningOnCompositor and
+  // mPerformanceWarning.
+  AnimationProperty() = default;
+  AnimationProperty(const AnimationProperty& aOther)
+    : mProperty(aOther.mProperty), mSegments(aOther.mSegments) { }
+  AnimationProperty& operator=(const AnimationProperty& aOther)
+  {
+    mProperty = aOther.mProperty;
+    mSegments = aOther.mSegments;
+    return *this;
+  }
+
   // NOTE: This operator does *not* compare the mIsRunningOnCompositor member.
   // This is because AnimationProperty objects are compared when recreating
   // CSS animations to determine if mutation observer change records need to
   // be created or not. However, at the point when these objects are compared
   // the mIsRunningOnCompositor will not have been set on the new objects so
   // we ignore this member to avoid generating spurious change records.
-  bool operator==(const AnimationProperty& aOther) const {
+  bool operator==(const AnimationProperty& aOther) const
+  {
     return mProperty == aOther.mProperty &&
            mSegments == aOther.mSegments;
   }
-  bool operator!=(const AnimationProperty& aOther) const {
+  bool operator!=(const AnimationProperty& aOther) const
+  {
     return !(*this == aOther);
   }
 };
 
 struct ElementPropertyTransition;
 
 namespace dom {