Bug 1216843 - Part 11: Make AddFilterFunction and AddFilterFunctionImpl return UniquePtr and rename them to AddWeightedFilterXX. r?dholbert draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 12 Sep 2016 14:34:28 +0900
changeset 412524 22a7cb11611d4717f9326b3c74babaffbc058b3c
parent 412523 193383caab19f3360260f440cccf38d7930e282f
child 412525 c1dc7d3b9c819420b0a1752dca4cbfe686fa79e5
push id29192
push userbmo:hiikezoe@mozilla-japan.org
push dateMon, 12 Sep 2016 05:39:50 +0000
reviewersdholbert
bugs1216843
milestone51.0a1
Bug 1216843 - Part 11: Make AddFilterFunction and AddFilterFunctionImpl return UniquePtr and rename them to AddWeightedFilterXX. r?dholbert MozReview-Commit-ID: 3hofHkvcR6G
layout/style/StyleAnimationValue.cpp
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -1798,38 +1798,38 @@ AddDifferentTransformLists(double aCoeff
 }
 
 static bool
 TransformFunctionsMatch(nsCSSKeyword func1, nsCSSKeyword func2)
 {
   return ToPrimitive(func1) == ToPrimitive(func2);
 }
 
-static bool
-AddFilterFunctionImpl(double aCoeff1, const nsCSSValueList* aList1,
-                      double aCoeff2, const nsCSSValueList* aList2,
-                      nsCSSValueList**& aResultTail)
+static UniquePtr<nsCSSValueList>
+AddWeightedFilterFunctionImpl(double aCoeff1, const nsCSSValueList* aList1,
+                              double aCoeff2, const nsCSSValueList* aList2)
 {
-  // AddFilterFunction should be our only caller, and it should ensure that both
-  // args are non-null.
+  // AddWeightedFilterFunction should be our only caller, and it should ensure
+  // that both args are non-null.
   MOZ_ASSERT(aList1, "expected filter list");
   MOZ_ASSERT(aList2, "expected filter list");
   MOZ_ASSERT(aList1->mValue.GetUnit() == eCSSUnit_Function,
              "expected function");
   MOZ_ASSERT(aList2->mValue.GetUnit() == eCSSUnit_Function,
              "expected function");
   RefPtr<nsCSSValue::Array> a1 = aList1->mValue.GetArrayValue(),
                               a2 = aList2->mValue.GetArrayValue();
   nsCSSKeyword filterFunction = a1->Item(0).GetKeywordValue();
-  if (filterFunction != a2->Item(0).GetKeywordValue())
-    return false; // Can't add two filters of different types.
-
-  nsAutoPtr<nsCSSValueList> resultListEntry(new nsCSSValueList);
+  if (filterFunction != a2->Item(0).GetKeywordValue()) {
+    return nullptr; // Can't add two filters of different types.
+  }
+
+  auto resultList = MakeUnique<nsCSSValueList>();
   nsCSSValue::Array* result =
-    resultListEntry->mValue.InitFunction(filterFunction, 1);
+    resultList->mValue.InitFunction(filterFunction, 1);
 
   // "hue-rotate" is the only filter-function that accepts negative values, and
   // we don't use this "restrictions" variable in its clause below.
   const uint32_t restrictions = CSS_PROPERTY_VALUE_NONNEGATIVE;
   const nsCSSValue& funcArg1 = a1->Item(1);
   const nsCSSValue& funcArg2 = a2->Item(1);
   nsCSSValue& resultArg = result->Item(1);
   float initialVal = 1.0f;
@@ -1842,17 +1842,17 @@ AddFilterFunctionImpl(double aCoeff1, co
         // If units differ, we'll just combine them with calc().
         unit = eCSSUnit_Calc;
       }
       if (!AddCSSValuePixelPercentCalc(restrictions,
                                        unit,
                                        aCoeff1, funcArg1,
                                        aCoeff2, funcArg2,
                                        resultArg)) {
-        return false;
+        return nullptr;
       }
       break;
     }
     case eCSSKeyword_grayscale:
     case eCSSKeyword_invert:
     case eCSSKeyword_sepia:
       initialVal = 0.0f;
       MOZ_FALLTHROUGH;
@@ -1877,52 +1877,48 @@ AddFilterFunctionImpl(double aCoeff1, co
                  "drop-shadow filter func doesn't support lists");
       UniquePtr<nsCSSValueList> shadowValue =
         AddWeightedShadowItems(aCoeff1,
                                funcArg1.GetListValue()->mValue,
                                aCoeff2,
                                funcArg2.GetListValue()->mValue,
                                ColorAdditionType::Clamped);
       if (!shadowValue) {
-        return false;
+        return nullptr;
       }
       resultArg.AdoptListValue(Move(shadowValue));
       break;
     }
     default:
       MOZ_ASSERT(false, "unknown filter function");
-      return false;
+      return nullptr;
   }
 
-  *aResultTail = resultListEntry.forget();
-  aResultTail = &(*aResultTail)->mNext;
-
-  return true;
+  return resultList;
 }
 
-static bool
-AddFilterFunction(double aCoeff1, const nsCSSValueList* aList1,
-                  double aCoeff2, const nsCSSValueList* aList2,
-                  nsCSSValueList**& aResultTail)
+static UniquePtr<nsCSSValueList>
+AddWeightedFilterFunction(double aCoeff1, const nsCSSValueList* aList1,
+                          double aCoeff2, const nsCSSValueList* aList2)
 {
   MOZ_ASSERT(aList1 || aList2,
              "one function list item must not be null");
   // Note that one of our arguments could be null, indicating that
   // it's the initial value. Rather than adding special null-handling
   // logic, we just check for null values and replace them with
-  // 0 * the other value. That way, AddFilterFunctionImpl can assume
+  // 0 * the other value. That way, AddWeightedFilterFunctionImpl can assume
   // its args are non-null.
   if (!aList1) {
-    return AddFilterFunctionImpl(aCoeff2, aList2, 0, aList2, aResultTail);
+    return AddWeightedFilterFunctionImpl(aCoeff2, aList2, 0, aList2);
   }
   if (!aList2) {
-    return AddFilterFunctionImpl(aCoeff1, aList1, 0, aList1, aResultTail);
+    return AddWeightedFilterFunctionImpl(aCoeff1, aList1, 0, aList1);
   }
 
-  return AddFilterFunctionImpl(aCoeff1, aList1, aCoeff2, aList2, aResultTail);
+  return AddWeightedFilterFunctionImpl(aCoeff1, aList1, aCoeff2, aList2);
 }
 
 static inline uint32_t
 ShapeArgumentCount(nsCSSKeyword aShapeFunction)
 {
   switch (aShapeFunction) {
     case eCSSKeyword_circle:
       return 2; // radius and center point
@@ -2750,45 +2746,45 @@ StyleAnimationValue::AddWeighted(nsCSSPr
       }
       aResultValue.SetCSSValueArrayValue(result, eUnit_Shape);
       return true;
     }
     case eUnit_Filter: {
       const nsCSSValueList *list1 = aValue1.GetCSSValueListValue();
       const nsCSSValueList *list2 = aValue2.GetCSSValueListValue();
 
-      nsAutoPtr<nsCSSValueList> result;
-      nsCSSValueList **resultTail = getter_Transfers(result);
+      UniquePtr<nsCSSValueList> result;
+      nsCSSValueList* tail = nullptr;
       while (list1 || list2) {
-        MOZ_ASSERT(!*resultTail,
-          "resultTail isn't pointing to the tail (may leak)");
         if ((list1 && list1->mValue.GetUnit() != eCSSUnit_Function) ||
             (list2 && list2->mValue.GetUnit() != eCSSUnit_Function)) {
           // If we don't have filter-functions, we must have filter-URLs, which
           // we can't add or interpolate.
           return false;
         }
 
-        if (!AddFilterFunction(aCoeff1, list1, aCoeff2, list2, resultTail)) {
+        UniquePtr<nsCSSValueList> resultFunction =
+          AddWeightedFilterFunction(aCoeff1, list1, aCoeff2, list2);
+        if (!resultFunction) {
           // filter function mismatch
           return false;
         }
 
+        AppendToCSSValueList(result, Move(resultFunction), &tail);
+
         // move to next list items
         if (list1) {
           list1 = list1->mNext;
         }
         if (list2) {
           list2 = list2->mNext;
         }
       }
-      MOZ_ASSERT(!*resultTail,
-                 "resultTail isn't pointing to the tail (may leak)");
-
-      aResultValue.SetAndAdoptCSSValueListValue(result.forget(),
+
+      aResultValue.SetAndAdoptCSSValueListValue(result.release(),
                                                 eUnit_Filter);
       return true;
     }
 
     case eUnit_Transform: {
       const nsCSSValueList* list1 = aValue1.GetCSSValueSharedListValue()->mHead;
       const nsCSSValueList* list2 = aValue2.GetCSSValueSharedListValue()->mHead;