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

@@ -36,7 +36,7 @@ public:
   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);
@@ -44,14 +44,13 @@ public:
       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 {

dom/animation/KeyframeEffect.cpp

@@ -598,14 +598,12 @@ KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
       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;
     }
@@ -618,14 +616,17 @@ KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                                          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));
   }
 }
 

layout/style/StyleAnimationValue.h

@@ -373,6 +373,12 @@ public:
   }
   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 };