Bug 1260572 - Replace AnimValuesStyleRule::AddEmptyValue with an overload of AddValue that takes an rvalue reference; r=heycam
authorBrian Birtles <birtles@gmail.com>
Wed, 30 Mar 2016 08:59:01 +0900
changeset 316330 debc00d94145d59fe5ddc6bc4f5526508745e24c
parent 316329 3f521e50602914247d3a2ac977283dc4f6d54f60
child 316331 3baed5c339e31188949641cb7bc7178a71070718
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1260572
milestone48.0a1
Bug 1260572 - Replace AnimValuesStyleRule::AddEmptyValue with an overload of AddValue that takes an rvalue reference; r=heycam In the next patch in this series, we would like to update the error handling of the call to StyleAnimationValue::Interpolate in KeyframeEffectReadOnly::ComposeStyle. Using AnimValuesStyleRule::AddEmptyValue there, however, makes handling the error case difficult because we need a means of clearing the allocated StyleAnimationValue. However, simply using AnimationValuesStyleRule::AddValue means we will end up doing needless allocations for StyleAnimationValue objects (the copy constructor for which can result in performing potentially expensive heap allocations, such as when lists are deep-copied). Instead, we add a Move constructor to StyleAnimationValue and add an overload of AnimValuesStyleRule::AddValue that takes an rvalue reference. This provides a more consistent interface to AnimValuesStyleRule and avoids the unnecessary allocations from copying StyleAnimationValue objects. MozReview-Commit-ID: CaP1uPAgNnm
dom/animation/AnimValuesStyleRule.h
dom/animation/KeyframeEffect.cpp
layout/style/StyleAnimationValue.h
--- a/dom/animation/AnimValuesStyleRule.h
+++ b/dom/animation/AnimValuesStyleRule.h
@@ -31,32 +31,31 @@ public:
 
   // nsIStyleRule implementation
   void MapRuleInfoInto(nsRuleData* aRuleData) override;
   bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
 
-  void AddValue(nsCSSProperty aProperty, StyleAnimationValue &aStartValue)
+  void AddValue(nsCSSProperty aProperty, const StyleAnimationValue &aStartValue)
   {
     PropertyValuePair v = { aProperty, aStartValue };
     mPropertyValuePairs.AppendElement(v);
     mStyleBits |=
       nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[aProperty]);
   }
 
-  // Caller must fill in returned value.
-  StyleAnimationValue* AddEmptyValue(nsCSSProperty aProperty)
+  void AddValue(nsCSSProperty aProperty, StyleAnimationValue&& aStartValue)
   {
     PropertyValuePair *p = mPropertyValuePairs.AppendElement();
     p->mProperty = aProperty;
+    p->mValue = Move(aStartValue);
     mStyleBits |=
       nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[aProperty]);
-    return &p->mValue;
   }
 
   struct PropertyValuePair {
     nsCSSProperty mProperty;
     StyleAnimationValue mValue;
   };
 
 private:
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -593,44 +593,45 @@ KeyframeEffectReadOnly::ComposeStyle(Ref
                  prop.mSegments.Length(),
                "out of array bounds");
 
     if (!aStyleRule) {
       // Allocate the style rule now that we know we have animation data.
       aStyleRule = new AnimValuesStyleRule();
     }
 
-    StyleAnimationValue* val = aStyleRule->AddEmptyValue(prop.mProperty);
-
     // Special handling for zero-length segments
     if (segment->mToKey == segment->mFromKey) {
       if (computedTiming.mProgress.Value() < 0) {
-        *val = segment->mFromValue;
+        aStyleRule->AddValue(prop.mProperty, segment->mFromValue);
       } else {
-        *val = segment->mToValue;
+        aStyleRule->AddValue(prop.mProperty, segment->mToValue);
       }
       continue;
     }
 
     double positionInSegment =
       (computedTiming.mProgress.Value() - segment->mFromKey) /
       (segment->mToKey - segment->mFromKey);
     double valuePosition =
       ComputedTimingFunction::GetPortion(segment->mTimingFunction,
                                          positionInSegment,
                                          computedTiming.mBeforeFlag);
 
+    StyleAnimationValue val;
 #ifdef DEBUG
     bool result =
 #endif
       StyleAnimationValue::Interpolate(prop.mProperty,
                                        segment->mFromValue,
                                        segment->mToValue,
-                                       valuePosition, *val);
+                                       valuePosition, val);
     MOZ_ASSERT(result, "interpolate must succeed now");
+
+    aStyleRule->AddValue(prop.mProperty, Move(val));
   }
 }
 
 bool
 KeyframeEffectReadOnly::IsRunningOnCompositor() const
 {
   // We consider animation is running on compositor if there is at least
   // one property running on compositor.
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -368,16 +368,22 @@ public:
 
   explicit StyleAnimationValue(Unit aUnit = eUnit_Null) : mUnit(aUnit) {
     NS_ASSERTION(aUnit == eUnit_Null || aUnit == eUnit_Normal ||
                  aUnit == eUnit_Auto || aUnit == eUnit_None,
                  "must be valueless unit");
   }
   StyleAnimationValue(const StyleAnimationValue& aOther)
     : mUnit(eUnit_Null) { *this = aOther; }
+  StyleAnimationValue(StyleAnimationValue&& aOther)
+    : mUnit(aOther.mUnit)
+    , mValue(aOther.mValue)
+  {
+    aOther.mUnit = eUnit_Null;
+  }
   enum IntegerConstructorType { IntegerConstructor };
   StyleAnimationValue(int32_t aInt, Unit aUnit, IntegerConstructorType);
   enum CoordConstructorType { CoordConstructor };
   StyleAnimationValue(nscoord aLength, CoordConstructorType);
   enum PercentConstructorType { PercentConstructor };
   StyleAnimationValue(float aPercent, PercentConstructorType);
   enum FloatConstructorType { FloatConstructor };
   StyleAnimationValue(float aFloat, FloatConstructorType);