Bug 1483699 - part 2 - make InsertSlotsAt error checking more thorough; r=mccr8 a=pascalc
authorNathan Froyd <froydnj@mozilla.com>
Mon, 08 Oct 2018 10:39:44 -0400
changeset 490241 de127662180d7e2798660e75cee75523fdd056dd
parent 490240 b4e29532f5282a02b819ad2fe1bc89fd6d5b1002
child 490242 253f11fbc824e50ded7fa9d1e6fb5046e0e86188
push id9959
push userarchaeopteryx@coole-files.de
push dateTue, 09 Oct 2018 18:37:44 +0000
treeherdermozilla-beta@f997cf948ca0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8, pascalc
bugs1483699
milestone63.0
Bug 1483699 - part 2 - make InsertSlotsAt error checking more thorough; r=mccr8 a=pascalc I don't know what the existing code was trying to do, but it certainly wasn't clear, and possibly not correct.
xpcom/ds/nsTArray-inl.h
xpcom/ds/nsTArray.h
--- a/xpcom/ds/nsTArray-inl.h
+++ b/xpcom/ds/nsTArray-inl.h
@@ -342,39 +342,34 @@ nsTArray_base<Alloc, Copy>::SwapFromEnd(
   Copy::MoveNonOverlappingRegion(baseAddr + destBytes,
                                  baseAddr + sourceBytes,
                                  relocCount,
                                  aElemSize);
 }
 
 template<class Alloc, class Copy>
 template<typename ActualAlloc>
-bool
+typename ActualAlloc::ResultTypeProxy
 nsTArray_base<Alloc, Copy>::InsertSlotsAt(index_type aIndex, size_type aCount,
                                           size_type aElemSize,
                                           size_t aElemAlign)
 {
   if (MOZ_UNLIKELY(aIndex > Length())) {
     InvalidArrayIndex_CRASH(aIndex, Length());
   }
 
-  size_type newLen = Length() + aCount;
-
-  EnsureCapacity<ActualAlloc>(newLen, aElemSize);
-
-  // Check for out of memory conditions
-  if (Capacity() < newLen) {
-    return false;
+  if (!ActualAlloc::Successful(this->ExtendCapacity<ActualAlloc>(Length(), aCount, aElemSize))) {
+    return ActualAlloc::FailureResult();
   }
 
   // Move the existing elements as needed.  Note that this will
   // change our mLength, so no need to call IncrementLength.
   ShiftData<ActualAlloc>(aIndex, 0, aCount, aElemSize, aElemAlign);
 
-  return true;
+  return ActualAlloc::SuccessResult();
 }
 
 // nsTArray_base::IsAutoArrayRestorer is an RAII class which takes
 // |nsTArray_base &array| in its constructor.  When it's destructed, it ensures
 // that
 //
 //   * array.mIsAutoArray has the same value as it did when we started, and
 //   * if array has an auto buffer and mHdr would otherwise point to
--- a/xpcom/ds/nsTArray.h
+++ b/xpcom/ds/nsTArray.h
@@ -454,18 +454,19 @@ protected:
 
   // This method inserts blank slots into the array.
   // @param aIndex the place to insert the new elements. This must be no
   //               greater than the current length of the array.
   // @param aCount the number of slots to insert
   // @param aElementSize the size of an array element.
   // @param aElemAlign the alignment in bytes of an array element.
   template<typename ActualAlloc>
-  bool InsertSlotsAt(index_type aIndex, size_type aCount,
-                     size_type aElementSize, size_t aElemAlign);
+  typename ActualAlloc::ResultTypeProxy
+  InsertSlotsAt(index_type aIndex, size_type aCount,
+                size_type aElementSize, size_t aElemAlign);
 
   template<typename ActualAlloc, class Allocator>
   typename ActualAlloc::ResultTypeProxy
   SwapArrayElements(nsTArray_base<Allocator, Copy>& aOther,
                     size_type aElemSize,
                     size_t aElemAlign);
 
   // This is an RAII class used in SwapArrayElements.
@@ -2236,19 +2237,18 @@ public:
   // them using elem_type's default constructor.
   // @param aIndex the place to insert the new elements. This must be no
   //               greater than the current length of the array.
   // @param aCount the number of elements to insert
 protected:
   template<typename ActualAlloc = Alloc>
   elem_type* InsertElementsAt(index_type aIndex, size_type aCount)
   {
-    if (!base_type::template InsertSlotsAt<ActualAlloc>(aIndex, aCount,
-                                                        sizeof(elem_type),
-                                                        MOZ_ALIGNOF(elem_type))) {
+    if (!ActualAlloc::Successful(this->template InsertSlotsAt<ActualAlloc>(
+          aIndex, aCount, sizeof(elem_type), MOZ_ALIGNOF(elem_type)))) {
       return nullptr;
     }
 
     // Initialize the extra array elements
     elem_type* iter = Elements() + aIndex;
     elem_type* iend = iter + aCount;
     for (; iter != iend; ++iter) {
       elem_traits::Construct(iter);
@@ -2461,19 +2461,18 @@ nsTArray_Impl<E, Alloc>::RemoveElementsB
 }
 
 template<typename E, class Alloc>
 template<class Item, typename ActualAlloc>
 auto
 nsTArray_Impl<E, Alloc>::InsertElementsAt(index_type aIndex, size_type aCount,
                                           const Item& aItem) -> elem_type*
 {
-  if (!base_type::template InsertSlotsAt<ActualAlloc>(aIndex, aCount,
-                                                      sizeof(elem_type),
-                                                      MOZ_ALIGNOF(elem_type))) {
+  if (!ActualAlloc::Successful(this->template InsertSlotsAt<ActualAlloc>(
+        aIndex, aCount, sizeof(elem_type), MOZ_ALIGNOF(elem_type)))) {
     return nullptr;
   }
 
   // Initialize the extra array elements
   elem_type* iter = Elements() + aIndex;
   elem_type* iend = iter + aCount;
   for (; iter != iend; ++iter) {
     elem_traits::Construct(iter, aItem);