Bug 1298722 - Use MOZ_MUST_USE in StyleAnimationValue. r=birtles.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 30 Aug 2016 16:10:59 +1000
changeset 311940 cfcfff96e4ffd245631e59cacb187410ef20362c
parent 311939 4e837c5be1137927f8a9400beab563c6530cf23c
child 311941 4b36234946e31cdeea12099ce12e6fd697601e5e
push id81246
push usernnethercote@mozilla.com
push dateWed, 31 Aug 2016 00:43:21 +0000
treeherdermozilla-inbound@cfcfff96e4ff [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1298722
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1298722 - Use MOZ_MUST_USE in StyleAnimationValue. r=birtles.
dom/animation/AnimValuesStyleRule.cpp
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeUtils.cpp
gfx/layers/composite/AsyncCompositionManager.cpp
layout/style/StyleAnimationValue.cpp
layout/style/StyleAnimationValue.h
layout/style/nsAnimationManager.cpp
--- a/dom/animation/AnimValuesStyleRule.cpp
+++ b/dom/animation/AnimValuesStyleRule.cpp
@@ -34,19 +34,17 @@ AnimValuesStyleRule::MapRuleInfoInto(nsR
 
   for (uint32_t i = 0, i_end = mPropertyValuePairs.Length(); i < i_end; ++i) {
     PropertyStyleAnimationValuePair& pair = mPropertyValuePairs[i];
     if (aRuleData->mSIDs & nsCachedStyleData::GetBitForSID(
                              nsCSSProps::kSIDTable[pair.mProperty]))
     {
       nsCSSValue *prop = aRuleData->ValueFor(pair.mProperty);
       if (prop->GetUnit() == eCSSUnit_Null) {
-#ifdef DEBUG
-        bool ok =
-#endif
+        DebugOnly<bool> ok =
           StyleAnimationValue::UncomputeValue(pair.mProperty, pair.mValue,
                                               *prop);
         MOZ_ASSERT(ok, "could not store computed value");
       }
     }
   }
 }
 
@@ -73,17 +71,18 @@ AnimValuesStyleRule::List(FILE* out, int
     str.AppendLiteral("  ");
   }
   str.AppendLiteral("[anim values] { ");
   for (uint32_t i = 0, i_end = mPropertyValuePairs.Length(); i < i_end; ++i) {
     const PropertyStyleAnimationValuePair& pair = mPropertyValuePairs[i];
     str.Append(nsCSSProps::GetStringValue(pair.mProperty));
     str.AppendLiteral(": ");
     nsAutoString value;
-    StyleAnimationValue::UncomputeValue(pair.mProperty, pair.mValue, value);
+    Unused <<
+      StyleAnimationValue::UncomputeValue(pair.mProperty, pair.mValue, value);
     AppendUTF16toUTF8(value, str);
     str.AppendLiteral("; ");
   }
   str.AppendLiteral("}\n");
   fprintf_stderr(out, "%s", str.get());
 }
 #endif
 
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -648,22 +648,22 @@ KeyframeEffectReadOnly::GetTargetStyleCo
 #ifdef DEBUG
 void
 DumpAnimationProperties(nsTArray<AnimationProperty>& aAnimationProperties)
 {
   for (auto& p : aAnimationProperties) {
     printf("%s\n", nsCSSProps::GetStringValue(p.mProperty).get());
     for (auto& s : p.mSegments) {
       nsString fromValue, toValue;
-      StyleAnimationValue::UncomputeValue(p.mProperty,
-                                          s.mFromValue,
-                                          fromValue);
-      StyleAnimationValue::UncomputeValue(p.mProperty,
-                                          s.mToValue,
-                                          toValue);
+      Unused << StyleAnimationValue::UncomputeValue(p.mProperty,
+                                                    s.mFromValue,
+                                                    fromValue);
+      Unused << StyleAnimationValue::UncomputeValue(p.mProperty,
+                                                    s.mToValue,
+                                                    toValue);
       printf("  %f..%f: %s..%s\n", s.mFromKey, s.mToKey,
              NS_ConvertUTF16toUTF8(fromValue).get(),
              NS_ConvertUTF16toUTF8(toValue).get());
     }
   }
 }
 #endif
 
@@ -712,17 +712,19 @@ CreatePropertyValue(nsCSSPropertyID aPro
                     float aOffset,
                     const Maybe<ComputedTimingFunction>& aTimingFunction,
                     const StyleAnimationValue& aValue,
                     AnimationPropertyValueDetails& aResult)
 {
   aResult.mOffset = aOffset;
 
   nsString stringValue;
-  StyleAnimationValue::UncomputeValue(aProperty, aValue, stringValue);
+  DebugOnly<bool> uncomputeResult =
+    StyleAnimationValue::UncomputeValue(aProperty, aValue, stringValue);
+  MOZ_ASSERT(uncomputeResult, "failed to uncompute value");
   aResult.mValue = stringValue;
 
   if (aTimingFunction) {
     aResult.mEasing.Construct();
     aTimingFunction->AppendToString(aResult.mEasing.Value());
   } else {
     aResult.mEasing.Construct(NS_LITERAL_STRING("linear"));
   }
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -1570,20 +1570,21 @@ GetCumulativeDistances(const nsTArray<Co
             dist += componentDistance * componentDistance;
           }
         }
         dist = sqrt(dist);
       } else {
         // If the property is longhand, we just use the 1st value.
         // If ComputeDistance() fails, |dist| will remain zero so there will be
         // no distance between the previous paced value and this value.
-        StyleAnimationValue::ComputeDistance(aPacedProperty,
-                                             prevPacedValues[0].mValue,
-                                             pacedValues[0].mValue,
-                                             dist);
+        Unused <<
+          StyleAnimationValue::ComputeDistance(aPacedProperty,
+                                               prevPacedValues[0].mValue,
+                                               pacedValues[0].mValue,
+                                               dist);
       }
       cumulativeDistances[i] = cumulativeDistances[preIdx] + dist;
     }
     prevPacedValues.SwapElements(pacedValues);
     preIdx = i;
   }
   return cumulativeDistances;
 }
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -556,18 +556,23 @@ static void
 SampleValue(float aPortion, Animation& aAnimation, StyleAnimationValue& aStart,
             StyleAnimationValue& aEnd, Animatable* aValue, Layer* aLayer)
 {
   StyleAnimationValue interpolatedValue;
   NS_ASSERTION(aStart.GetUnit() == aEnd.GetUnit() ||
                aStart.GetUnit() == StyleAnimationValue::eUnit_None ||
                aEnd.GetUnit() == StyleAnimationValue::eUnit_None,
                "Must have same unit");
-  StyleAnimationValue::Interpolate(aAnimation.property(), aStart, aEnd,
-                                aPortion, interpolatedValue);
+  // This should never fail because we only pass transform and opacity values
+  // to the compositor and they should never fail to interpolate.
+  DebugOnly<bool> uncomputeResult =
+    StyleAnimationValue::Interpolate(aAnimation.property(), aStart, aEnd,
+                                     aPortion, interpolatedValue);
+  MOZ_ASSERT(uncomputeResult, "could not uncompute value");
+
   if (aAnimation.property() == eCSSProperty_opacity) {
     *aValue = interpolatedValue.GetFloatValue();
     return;
   }
 
   nsCSSValueSharedList* interpolatedList =
     interpolatedValue.GetCSSValueSharedListValue();
 
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -844,19 +844,17 @@ StyleAnimationValue::ComputeDistance(nsC
 
         if (color1.GetUnit() != eCSSUnit_Null) {
           StyleAnimationValue color1Value
             (color1.GetColorValue(), StyleAnimationValue::ColorConstructor);
           StyleAnimationValue color2Value
             (color2.GetColorValue(), StyleAnimationValue::ColorConstructor);
           double colorDistance;
 
-        #ifdef DEBUG
-          bool ok =
-        #endif
+          DebugOnly<bool> ok =
             StyleAnimationValue::ComputeDistance(eCSSProperty_color,
                                                  color1Value, color2Value,
                                                  colorDistance);
           MOZ_ASSERT(ok, "should not fail");
           squareDistance += colorDistance * colorDistance;
         }
 
         shadow1 = shadow1->mNext;
@@ -1255,19 +1253,17 @@ AddShadowItems(double aCoeff1, const nsC
   }
 
   if (color1.GetUnit() != eCSSUnit_Null) {
     StyleAnimationValue color1Value
       (color1.GetColorValue(), StyleAnimationValue::ColorConstructor);
     StyleAnimationValue color2Value
       (color2.GetColorValue(), StyleAnimationValue::ColorConstructor);
     StyleAnimationValue resultColorValue;
-  #ifdef DEBUG
-    bool ok =
-  #endif
+    DebugOnly<bool> ok =
       StyleAnimationValue::AddWeighted(eCSSProperty_color,
                                        aCoeff1, color1Value,
                                        aCoeff2, color2Value,
                                        resultColorValue);
     MOZ_ASSERT(ok, "should not fail");
     resultArray->Item(4).SetColorValue(resultColorValue.GetColorValue());
   }
 
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -48,18 +48,19 @@ public:
    * Note that if |aCount| is 0, then |aDest| will be unchanged.  Also, if
    * this method fails, then |aDest| will be unchanged.
    *
    * @param aDest       The value to add to.
    * @param aValueToAdd The value to add.
    * @param aCount      The number of times to add aValueToAdd.
    * @return true on success, false on failure.
    */
-  static bool Add(nsCSSPropertyID aProperty, StyleAnimationValue& aDest,
-                  const StyleAnimationValue& aValueToAdd, uint32_t aCount) {
+  static MOZ_MUST_USE bool
+  Add(nsCSSPropertyID aProperty, StyleAnimationValue& aDest,
+      const StyleAnimationValue& aValueToAdd, uint32_t aCount) {
     return AddWeighted(aProperty, 1.0, aDest, aCount, aValueToAdd, aDest);
   }
 
   /**
    * Calculates a measure of 'distance' between two values.
    *
    * This measure of Distance is guaranteed to be proportional to
    * portions passed to Interpolate, Add, or AddWeighted.  However, for
@@ -71,20 +72,21 @@ public:
    *
    * @param aStartValue The start of the interval for which the distance
    *                    should be calculated.
    * @param aEndValue   The end of the interval for which the distance
    *                    should be calculated.
    * @param aDistance   The result of the calculation.
    * @return true on success, false on failure.
    */
-  static bool ComputeDistance(nsCSSPropertyID aProperty,
-                              const StyleAnimationValue& aStartValue,
-                              const StyleAnimationValue& aEndValue,
-                              double& aDistance);
+  static MOZ_MUST_USE bool
+  ComputeDistance(nsCSSPropertyID aProperty,
+                  const StyleAnimationValue& aStartValue,
+                  const StyleAnimationValue& aEndValue,
+                  double& aDistance);
 
   /**
    * Calculates an interpolated value that is the specified |aPortion| between
    * the two given values.
    *
    * This really just does the following calculation:
    *   aResultValue = (1.0 - aPortion) * aStartValue + aPortion * aEndValue
    *
@@ -92,21 +94,22 @@ public:
    *                    interpolation.
    * @param aEndValue   The value defining the end of the interval of
    *                    interpolation.
    * @param aPortion    A number in the range [0.0, 1.0] defining the
    *                    distance of the interpolated value in the interval.
    * @param [out] aResultValue The resulting interpolated value.
    * @return true on success, false on failure.
    */
-  static bool Interpolate(nsCSSPropertyID aProperty,
-                          const StyleAnimationValue& aStartValue,
-                          const StyleAnimationValue& aEndValue,
-                          double aPortion,
-                          StyleAnimationValue& aResultValue) {
+  static MOZ_MUST_USE bool
+  Interpolate(nsCSSPropertyID aProperty,
+              const StyleAnimationValue& aStartValue,
+              const StyleAnimationValue& aEndValue,
+              double aPortion,
+              StyleAnimationValue& aResultValue) {
     return AddWeighted(aProperty, 1.0 - aPortion, aStartValue,
                        aPortion, aEndValue, aResultValue);
   }
 
   /**
    * Does the calculation:
    *   aResultValue = aCoeff1 * aValue1 + aCoeff2 * aValue2
    *
@@ -115,20 +118,21 @@ public:
    * @return true on success, false on failure.
    *
    * NOTE: Current callers always pass aCoeff1 and aCoeff2 >= 0.  They
    * are currently permitted to be negative; however, if, as we add
    * support more value types types, we find that this causes
    * difficulty, we might change this to restrict them to being
    * positive.
    */
-  static bool AddWeighted(nsCSSPropertyID aProperty,
-                          double aCoeff1, const StyleAnimationValue& aValue1,
-                          double aCoeff2, const StyleAnimationValue& aValue2,
-                          StyleAnimationValue& aResultValue);
+  static MOZ_MUST_USE bool
+  AddWeighted(nsCSSPropertyID aProperty,
+              double aCoeff1, const StyleAnimationValue& aValue1,
+              double aCoeff2, const StyleAnimationValue& aValue2,
+              StyleAnimationValue& aResultValue);
 
   // Type-conversion methods
   // -----------------------
   /**
    * Creates a computed value for the given specified value
    * (property ID + string).  A style context is needed in case the
    * specified value depends on inherited style or on the values of other
    * properties.
@@ -152,53 +156,56 @@ public:
    *                        a different |aComputedValue| depending on other CSS
    *                        properties on |aTargetElement| or its ancestors.
    *                        false otherwise.
    *                        Note that the operation of this method is
    *                        significantly faster when |aIsContextSensitive| is
    *                        nullptr.
    * @return true on success, false on failure.
    */
-  static bool ComputeValue(nsCSSPropertyID aProperty,
-                           mozilla::dom::Element* aTargetElement,
-                           nsStyleContext* aStyleContext,
-                           const nsAString& aSpecifiedValue,
-                           bool aUseSVGMode,
-                           StyleAnimationValue& aComputedValue,
-                           bool* aIsContextSensitive = nullptr);
+  static MOZ_MUST_USE bool
+  ComputeValue(nsCSSPropertyID aProperty,
+               mozilla::dom::Element* aTargetElement,
+               nsStyleContext* aStyleContext,
+               const nsAString& aSpecifiedValue,
+               bool aUseSVGMode,
+               StyleAnimationValue& aComputedValue,
+               bool* aIsContextSensitive = nullptr);
 
   /**
    * Like ComputeValue, but returns an array of StyleAnimationValues.
    *
    * On success, when aProperty is a longhand, aResult will have a single
    * value in it.  When aProperty is a shorthand, aResult will be filled with
    * values for all of aProperty's longhand components.  aEnabledState
    * is used to filter the longhand components that will be appended
    * to aResult.  On failure, aResult might still have partial results
    * in it.
    */
-  static bool ComputeValues(nsCSSPropertyID aProperty,
-                            mozilla::CSSEnabledState aEnabledState,
-                            mozilla::dom::Element* aTargetElement,
-                            nsStyleContext* aStyleContext,
-                            const nsAString& aSpecifiedValue,
-                            bool aUseSVGMode,
-                            nsTArray<PropertyStyleAnimationValuePair>& aResult);
+  static MOZ_MUST_USE bool
+  ComputeValues(nsCSSPropertyID aProperty,
+                mozilla::CSSEnabledState aEnabledState,
+                mozilla::dom::Element* aTargetElement,
+                nsStyleContext* aStyleContext,
+                const nsAString& aSpecifiedValue,
+                bool aUseSVGMode,
+                nsTArray<PropertyStyleAnimationValuePair>& aResult);
 
   /**
    * A variant on ComputeValues that takes an nsCSSValue as the specified
    * value. Only longhand properties are supported.
    */
-  static bool ComputeValues(nsCSSPropertyID aProperty,
-                            mozilla::CSSEnabledState aEnabledState,
-                            mozilla::dom::Element* aTargetElement,
-                            nsStyleContext* aStyleContext,
-                            const nsCSSValue& aSpecifiedValue,
-                            bool aUseSVGMode,
-                            nsTArray<PropertyStyleAnimationValuePair>& aResult);
+  static MOZ_MUST_USE bool
+  ComputeValues(nsCSSPropertyID aProperty,
+                mozilla::CSSEnabledState aEnabledState,
+                mozilla::dom::Element* aTargetElement,
+                nsStyleContext* aStyleContext,
+                const nsCSSValue& aSpecifiedValue,
+                bool aUseSVGMode,
+                nsTArray<PropertyStyleAnimationValuePair>& aResult);
 
   /**
    * Creates a specified value for the given computed value.
    *
    * The first two overloads fill in an nsCSSValue object; the third
    * produces a string.  For the overload that takes a const
    * StyleAnimationValue& reference, the nsCSSValue result may depend on
    * objects owned by the |aComputedValue| object, so users of that variant
@@ -206,44 +213,51 @@ public:
    * The overload that takes an rvalue StyleAnimationValue reference
    * transfers ownership for some resources such that the |aComputedValue|
    * does not depend on the lifetime of |aSpecifiedValue|.
    *
    * @param aProperty      The property whose value we're uncomputing.
    * @param aComputedValue The computed value to be converted.
    * @param [out] aSpecifiedValue The resulting specified value.
    * @return true on success, false on failure.
+   *
+   * These functions are not MOZ_MUST_USE because failing to check the return
+   * value is common and reasonable.
    */
-  static bool UncomputeValue(nsCSSPropertyID aProperty,
-                             const StyleAnimationValue& aComputedValue,
-                             nsCSSValue& aSpecifiedValue);
-  static bool UncomputeValue(nsCSSPropertyID aProperty,
-                             StyleAnimationValue&& aComputedValue,
-                             nsCSSValue& aSpecifiedValue);
-  static bool UncomputeValue(nsCSSPropertyID aProperty,
-                             const StyleAnimationValue& aComputedValue,
-                             nsAString& aSpecifiedValue);
+  static MOZ_MUST_USE bool
+  UncomputeValue(nsCSSPropertyID aProperty,
+                 const StyleAnimationValue& aComputedValue,
+                 nsCSSValue& aSpecifiedValue);
+  static MOZ_MUST_USE bool
+  UncomputeValue(nsCSSPropertyID aProperty,
+                 StyleAnimationValue&& aComputedValue,
+                 nsCSSValue& aSpecifiedValue);
+  static MOZ_MUST_USE bool
+  UncomputeValue(nsCSSPropertyID aProperty,
+                 const StyleAnimationValue& aComputedValue,
+                 nsAString& aSpecifiedValue);
 
   /**
    * Gets the computed value for the given property from the given style
    * context.
    *
    * Obtaining the computed value allows us to animate properties when the
    * content author has specified a value like "inherit" or "initial" or some
    * other keyword that isn't directly interpolatable, but which *computes* to
    * something interpolatable.
    *
    * @param aProperty     The property whose value we're looking up.
    * @param aStyleContext The style context to check for the computed value.
    * @param [out] aComputedValue The resulting computed value.
    * @return true on success, false on failure.
    */
-  static bool ExtractComputedValue(nsCSSPropertyID aProperty,
-                                   nsStyleContext* aStyleContext,
-                                   StyleAnimationValue& aComputedValue);
+  static MOZ_MUST_USE bool ExtractComputedValue(
+    nsCSSPropertyID aProperty,
+    nsStyleContext* aStyleContext,
+    StyleAnimationValue& aComputedValue);
 
   /**
    * Interpolates between 2 matrices by decomposing them.
    *
    * @param aMatrix1   First matrix, using CSS pixel units.
    * @param aMatrix2   Second matrix, using CSS pixel units.
    * @param aProgress  Interpolation value in the range [0.0, 1.0]
    */
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -1031,17 +1031,21 @@ CSSAnimationBuilder::GetComputedValue(ns
     mStyleWithoutAnimation = aPresContext->StyleSet()->AsGecko()->
       ResolveStyleWithoutAnimation(mTarget, mStyleContext,
                                    eRestyle_AllHintsWithAnimations);
   }
 
   if (StyleAnimationValue::ExtractComputedValue(aProperty,
                                                 mStyleWithoutAnimation,
                                                 computedValue)) {
-    StyleAnimationValue::UncomputeValue(aProperty, Move(computedValue), result);
+    DebugOnly<bool> uncomputeResult =
+      StyleAnimationValue::UncomputeValue(aProperty, Move(computedValue),
+                                          result);
+    MOZ_ASSERT(uncomputeResult,
+               "Unable to get specified value from computed value");
   }
 
   // If we hit this assertion, it probably means we are fetching a value from
   // the computed style that we don't know how to represent as
   // a StyleAnimationValue.
   MOZ_ASSERT(result.GetUnit() != eCSSUnit_Null, "Got null computed value");
 
   return result;