Bug 1841314 - Part 3: Prefer JS::NewArrayBufferWithContents with UniquePtr. r=sfink,saschanaz

Replace the existing callers of `JS::NewArrayBufferWithContents` with the new
`UniquePtr` alternative.

Three callers to the old `JS::NewArrayBufferWithContents` function were left
unchanged:
- `mozilla::dom::FileReader::OnLoadEndArrayBuffer()` and
  `mozilla::dom::ArrayBufferBuilder::TakeArrayBuffer()` both store the data
  buffer as members and therefore have a more complicated lifetime.
- `JSStructuredCloneReader::readTransferMap()` because it's not clear if the
  data can be free'ed when `ArrayBuffectObject` allocation fails.

Differential Revision: https://phabricator.services.mozilla.com/D182588

dom/base/BodyConsumer.cpp

@@ -651,7 +651,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
   AssertIsOnTargetThread();
 
   // This makes sure that we free the data correctly.
-  auto autoFree = mozilla::MakeScopeExit([&] { free(aResult); });
+  UniquePtr<uint8_t[], JS::FreePolicy> resultPtr{aResult};
 
   if (mBodyConsumed) {
     return;
@@ -689,7 +689,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
   }
 
   // Finish successfully consuming body according to type.
-  MOZ_ASSERT(aResult);
+  MOZ_ASSERT(resultPtr);
 
   AutoJSAPI jsapi;
   if (!jsapi.Init(mGlobal)) {
@@ -703,16 +703,14 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
   switch (mConsumeType) {
     case CONSUME_ARRAYBUFFER: {
       JS::Rooted<JSObject*> arrayBuffer(cx);
-      BodyUtil::ConsumeArrayBuffer(cx, &arrayBuffer, aResultLength, aResult,
-                                   error);
+      BodyUtil::ConsumeArrayBuffer(cx, &arrayBuffer, aResultLength,
+                                   std::move(resultPtr), error);
 
       if (!error.Failed()) {
         JS::Rooted<JS::Value> val(cx);
         val.setObjectOrNull(arrayBuffer);
 
         localPromise->MaybeResolve(val);
-        // ArrayBuffer takes over ownership.
-        aResult = nullptr;
       }
       break;
     }
@@ -722,8 +720,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
     }
     case CONSUME_FORMDATA: {
       nsCString data;
-      data.Adopt(reinterpret_cast<char*>(aResult), aResultLength);
-      aResult = nullptr;
+      data.Adopt(reinterpret_cast<char*>(resultPtr.release()), aResultLength);
 
       RefPtr<dom::FormData> fd = BodyUtil::ConsumeFormData(
           mGlobal, mBodyMimeType, mMixedCaseMimeType, data, error);
@@ -737,7 +734,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
     case CONSUME_JSON: {
       nsString decoded;
       if (NS_SUCCEEDED(
-              BodyUtil::ConsumeText(aResultLength, aResult, decoded))) {
+              BodyUtil::ConsumeText(aResultLength, resultPtr.get(), decoded))) {
         if (mConsumeType == CONSUME_TEXT) {
           localPromise->MaybeResolve(decoded);
         } else {

dom/base/BodyUtil.cpp

@@ -353,11 +353,13 @@ class MOZ_STACK_CLASS FormDataParser {
 // static
 void BodyUtil::ConsumeArrayBuffer(JSContext* aCx,
                                   JS::MutableHandle<JSObject*> aValue,
-                                  uint32_t aInputLength, uint8_t* aInput,
+                                  uint32_t aInputLength,
+                                  UniquePtr<uint8_t[], JS::FreePolicy> aInput,
                                   ErrorResult& aRv) {
+  UniquePtr<void, JS::FreePolicy> dataPtr{aInput.release()};
   JS::Rooted<JSObject*> arrayBuffer(aCx);
-  arrayBuffer = JS::NewArrayBufferWithContents(aCx, aInputLength,
-                                               reinterpret_cast<void*>(aInput));
+  arrayBuffer =
+      JS::NewArrayBufferWithContents(aCx, aInputLength, std::move(dataPtr));
   if (!arrayBuffer) {
     JS_ClearPendingException(aCx);
     aRv.Throw(NS_ERROR_OUT_OF_MEMORY);

dom/base/BodyUtil.h

@@ -13,6 +13,8 @@
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FormData.h"
 
+#include "js/Utility.h"  // JS::FreePolicy
+
 namespace mozilla {
 class ErrorResult;
 
@@ -30,7 +32,8 @@ class BodyUtil final {
    */
   static void ConsumeArrayBuffer(JSContext* aCx,
                                  JS::MutableHandle<JSObject*> aValue,
-                                 uint32_t aInputLength, uint8_t* aInput,
+                                 uint32_t aInputLength,
+                                 UniquePtr<uint8_t[], JS::FreePolicy> aInput,
                                  ErrorResult& aRv);
 
   /**

dom/base/CompressionStream.cpp

@@ -108,7 +108,7 @@ class CompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
 
     do {
       static uint16_t kBufferSize = 16384;
-      UniquePtr<uint8_t> buffer(
+      UniquePtr<uint8_t[], JS::FreePolicy> buffer(
           static_cast<uint8_t*>(JS_malloc(aCx, kBufferSize)));
       if (!buffer) {
         aRv.ThrowTypeError("Out of memory");
@@ -164,8 +164,8 @@ class CompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
       // into Uint8Arrays.
       // (The buffer is 'split' by having a fixed sized buffer above.)
 
-      JS::Rooted<JSObject*> view(
-          aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, written, buffer));
+      JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
+                                          aCx, written, std::move(buffer)));
       if (!view || !array.append(view)) {
         JS_ClearPendingException(aCx);
         aRv.ThrowTypeError("Out of memory");

dom/base/DecompressionStream.cpp

@@ -107,7 +107,7 @@ class DecompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
 
     do {
       static uint16_t kBufferSize = 16384;
-      UniquePtr<uint8_t> buffer(
+      UniquePtr<uint8_t[], JS::FreePolicy> buffer(
           static_cast<uint8_t*>(JS_malloc(aCx, kBufferSize)));
       if (!buffer) {
         aRv.ThrowTypeError("Out of memory");
@@ -194,8 +194,8 @@ class DecompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
       // into Uint8Arrays.
       // (The buffer is 'split' by having a fixed sized buffer above.)
 
-      JS::Rooted<JSObject*> view(
-          aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, written, buffer));
+      JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
+                                          aCx, written, std::move(buffer)));
       if (!view || !array.append(view)) {
         JS_ClearPendingException(aCx);
         aRv.ThrowTypeError("Out of memory");

dom/base/nsJSUtils.cpp

@@ -186,17 +186,16 @@ bool nsJSUtils::DumpEnabled() {
 #endif
 }
 
-JSObject* nsJSUtils::MoveBufferAsUint8Array(JSContext* aCx, size_t aSize,
-                                            UniquePtr<uint8_t>& aBuffer) {
+JSObject* nsJSUtils::MoveBufferAsUint8Array(
+    JSContext* aCx, size_t aSize,
+    UniquePtr<uint8_t[], JS::FreePolicy> aBuffer) {
+  UniquePtr<void, JS::FreePolicy> dataPtr{aBuffer.release()};
   JS::Rooted<JSObject*> arrayBuffer(
-      aCx, JS::NewArrayBufferWithContents(aCx, aSize, aBuffer.get()));
+      aCx, JS::NewArrayBufferWithContents(aCx, aSize, std::move(dataPtr)));
   if (!arrayBuffer) {
     return nullptr;
   }
 
-  // Now the ArrayBuffer owns the buffer, so let's release our ownership
-  (void)aBuffer.release();
-
   return JS_NewUint8ArrayWithBuffer(aCx, arrayBuffer, 0,
                                     static_cast<int64_t>(aSize));
 }

dom/base/nsJSUtils.h

@@ -21,6 +21,7 @@
 #include "js/Conversions.h"
 #include "js/SourceText.h"
 #include "js/String.h"  // JS::{,Lossy}CopyLinearStringChars, JS::CopyStringChars, JS::Get{,Linear}StringLength, JS::MaxStringLength, JS::StringHasLatin1Chars
+#include "js/Utility.h"  // JS::FreePolicy
 #include "nsString.h"
 #include "xpcpublic.h"
 
@@ -87,8 +88,9 @@ class nsJSUtils {
   // Note that the buffer needs to be created by JS_malloc (or at least can be
   // freed by JS_free), as the resulting Uint8Array takes the ownership of the
   // buffer.
-  static JSObject* MoveBufferAsUint8Array(JSContext* aCx, size_t aSize,
-                                          mozilla::UniquePtr<uint8_t>& aBuffer);
+  static JSObject* MoveBufferAsUint8Array(
+      JSContext* aCx, size_t aSize,
+      mozilla::UniquePtr<uint8_t[], JS::FreePolicy> aBuffer);
 };
 
 inline void AssignFromStringBuffer(nsStringBuffer* buffer, size_t len,

dom/encoding/TextEncoderStream.cpp

@@ -68,7 +68,7 @@ static void EncodeNative(JSContext* aCx, mozilla::Decoder* aDecoder,
     return;
   }
 
-  UniquePtr<uint8_t> buffer(
+  UniquePtr<uint8_t[], JS::FreePolicy> buffer(
       static_cast<uint8_t*>(JS_malloc(aCx, needed.value())));
   if (!buffer) {
     aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
@@ -98,8 +98,9 @@ static void EncodeNative(JSContext* aCx, mozilla::Decoder* aDecoder,
   // https://encoding.spec.whatwg.org/#encode-and-enqueue-a-chunk
   // Step 4.2.2.1. Let chunk be a Uint8Array object wrapping an ArrayBuffer
   // containing output.
+  UniquePtr<void, JS::FreePolicy> dataPtr{buffer.release()};
   JS::Rooted<JSObject*> arrayBuffer(
-      aCx, JS::NewArrayBufferWithContents(aCx, written, buffer.release()));
+      aCx, JS::NewArrayBufferWithContents(aCx, written, std::move(dataPtr)));
   if (!arrayBuffer.get()) {
     JS_ClearPendingException(aCx);
     aRv.Throw(NS_ERROR_OUT_OF_MEMORY);

dom/file/FileReaderSync.cpp

@@ -84,15 +84,13 @@ void FileReaderSync::ReadAsArrayBuffer(JSContext* aCx,
     return;
   }
 
+  UniquePtr<void, JS::FreePolicy> dataPtr{bufferData.release()};
   JSObject* arrayBuffer =
-      JS::NewArrayBufferWithContents(aCx, blobSize, bufferData.get());
+      JS::NewArrayBufferWithContents(aCx, blobSize, std::move(dataPtr));
   if (!arrayBuffer) {
     aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
     return;
   }
-  // arrayBuffer takes the ownership when it is not null. Otherwise we
-  // need to release it explicitly.
-  (void)bufferData.release();
 
   aRetval.set(arrayBuffer);
 }

dom/indexedDB/Key.cpp

@@ -855,12 +855,19 @@ JSObject* Key::DecodeBinary(const EncodedDataType*& aPos,
   DecodeStringy<eBinary, uint8_t>(
       aPos, aEnd,
       [&rv, aCx](uint8_t** out, uint32_t decodedSize) {
-        *out = static_cast<uint8_t*>(JS_malloc(aCx, decodedSize));
-        if (NS_WARN_IF(!*out)) {
+        UniquePtr<void, JS::FreePolicy> ptr{JS_malloc(aCx, decodedSize)};
+        if (NS_WARN_IF(!ptr)) {
+          *out = nullptr;
           rv = nullptr;
           return false;
         }
-        rv = JS::NewArrayBufferWithContents(aCx, decodedSize, *out);
+
+        *out = static_cast<uint8_t*>(ptr.get());
+        rv = JS::NewArrayBufferWithContents(aCx, decodedSize, std::move(ptr));
+        if (NS_WARN_IF(!rv)) {
+          *out = nullptr;
+          return false;
+        }
         return true;
       },
       [&rv, aCx] { rv = JS::NewArrayBuffer(aCx, 0); });

dom/indexedDB/test/gtest/TestKey.cpp

@@ -238,8 +238,9 @@ INSTANTIATE_TEST_SUITE_P(DOM_IndexedDB_Key, TestWithParam_LiteralString,
 
 static JS::Value CreateArrayBufferValue(JSContext* const aContext,
                                         const size_t aSize, char* const aData) {
-  Rooted<JSObject*> arrayBuffer{
-      aContext, JS::NewArrayBufferWithContents(aContext, aSize, aData)};
+  mozilla::UniquePtr<void, JS::FreePolicy> ptr{aData};
+  Rooted<JSObject*> arrayBuffer{aContext, JS::NewArrayBufferWithContents(
+                                              aContext, aSize, std::move(ptr))};
   EXPECT_TRUE(arrayBuffer);
   return JS::ObjectValue(*arrayBuffer);
 }

dom/media/webrtc/jsapi/PeerConnectionImpl.cpp

@@ -2166,9 +2166,10 @@ void PeerConnectionImpl::DumpPacket_m(size_t level, dom::mozPacketDumpType type,
     return;
   }
 
+  UniquePtr<void, JS::FreePolicy> packetPtr{packet.release()};
   JS::Rooted<JSObject*> jsobj(
       jsapi.cx(),
-      JS::NewArrayBufferWithContents(jsapi.cx(), size, packet.release()));
+      JS::NewArrayBufferWithContents(jsapi.cx(), size, std::move(packetPtr)));
 
   RootedSpiderMonkeyInterface<ArrayBuffer> arrayBuffer(jsapi.cx());
   if (!arrayBuffer.Init(jsobj)) {

dom/network/TCPSocketChild.cpp

@@ -32,12 +32,10 @@ bool DeserializeArrayBuffer(JSContext* cx, const nsTArray<uint8_t>& aBuffer,
   if (!data) return false;
   memcpy(data.get(), aBuffer.Elements(), aBuffer.Length());
 
+  mozilla::UniquePtr<void, JS::FreePolicy> dataPtr{data.release()};
   JSObject* obj =
-      JS::NewArrayBufferWithContents(cx, aBuffer.Length(), data.get());
+      JS::NewArrayBufferWithContents(cx, aBuffer.Length(), std::move(dataPtr));
   if (!obj) return false;
-  // If JS::NewArrayBufferWithContents returns non-null, the ownership of
-  // the data is transfered to obj, so we release the ownership here.
-  mozilla::Unused << data.release();
 
   aVal.setObject(*obj);
   return true;

dom/serviceworkers/ServiceWorkerEvents.cpp

@@ -1093,7 +1093,9 @@ void PushMessageData::ArrayBuffer(JSContext* cx,
                                   ErrorResult& aRv) {
   uint8_t* data = GetContentsCopy();
   if (data) {
-    BodyUtil::ConsumeArrayBuffer(cx, aRetval, mBytes.Length(), data, aRv);
+    UniquePtr<uint8_t[], JS::FreePolicy> dataPtr(data);
+    BodyUtil::ConsumeArrayBuffer(cx, aRetval, mBytes.Length(),
+                                 std::move(dataPtr), aRv);
   }
 }
 

dom/streams/ByteStreamHelpers.cpp

@@ -26,7 +26,8 @@ JSObject* TransferArrayBuffer(JSContext* aCx, JS::Handle<JSObject*> aObject) {
   size_t bufferLength = JS::GetArrayBufferByteLength(aObject);
 
   // Step 2 (Reordered)
-  void* bufferData = JS::StealArrayBufferContents(aCx, aObject);
+  UniquePtr<void, JS::FreePolicy> bufferData{
+      JS::StealArrayBufferContents(aCx, aObject)};
 
   // Step 4.
   if (!JS::DetachArrayBuffer(aCx, aObject)) {
@@ -34,7 +35,8 @@ JSObject* TransferArrayBuffer(JSContext* aCx, JS::Handle<JSObject*> aObject) {
   }
 
   // Step 5.
-  return JS::NewArrayBufferWithContents(aCx, bufferLength, bufferData);
+  return JS::NewArrayBufferWithContents(aCx, bufferLength,
+                                        std::move(bufferData));
 }
 
 // https://streams.spec.whatwg.org/#can-transfer-array-buffer

dom/streams/UnderlyingSourceCallbackHelpers.cpp

@@ -429,7 +429,8 @@ void InputToReadableStreamAlgorithms::PullFromInputStream(JSContext* aCx,
   else {
     // Step 9.1. Set view to the result of creating a Uint8Array from pulled in
     // stream’s relevant Realm.
-    UniquePtr<uint8_t> buffer(static_cast<uint8_t*>(JS_malloc(aCx, pullSize)));
+    UniquePtr<uint8_t[], JS::FreePolicy> buffer(
+        static_cast<uint8_t*>(JS_malloc(aCx, pullSize)));
     if (!buffer) {
       aRv.ThrowTypeError("Out of memory");
       return;
@@ -446,8 +447,8 @@ void InputToReadableStreamAlgorithms::PullFromInputStream(JSContext* aCx,
     }
 
     MOZ_DIAGNOSTIC_ASSERT(pullSize == bytesWritten);
-    JS::Rooted<JSObject*> view(
-        aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, bytesWritten, buffer));
+    JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
+                                        aCx, bytesWritten, std::move(buffer)));
     if (!view) {
       JS_ClearPendingException(aCx);
       aRv.ThrowTypeError("Out of memory");

dom/system/IOUtils.cpp

@@ -2708,20 +2708,16 @@ JSObject* IOUtils::JsBuffer::IntoUint8Array(JSContext* aCx, JsBuffer aBuffer) {
     return JS_NewUint8Array(aCx, 0);
   }
 
-  char* rawBuffer = aBuffer.mBuffer.release();
+  UniquePtr<void, JS::FreePolicy> rawBuffer{aBuffer.mBuffer.release()};
   MOZ_RELEASE_ASSERT(rawBuffer);
   JS::Rooted<JSObject*> arrayBuffer(
       aCx, JS::NewArrayBufferWithContents(aCx, aBuffer.mLength,
-                                          reinterpret_cast<void*>(rawBuffer)));
+                                          std::move(rawBuffer)));
 
   if (!arrayBuffer) {
-    // The array buffer does not take ownership of the data pointer unless
-    // creation succeeds. We are still on the hook to free it.
-    //
     // aBuffer will be destructed at end of scope, but its destructor does not
     // take into account |mCapacity| or |mLength|, so it is OK for them to be
     // non-zero here with a null |mBuffer|.
-    js_free(rawBuffer);
     return nullptr;
   }
 

js/src/builtin/TestingFunctions.cpp

@@ -4989,10 +4989,10 @@ class CloneBufferObject : public NativeObject {
       return false;
     }
 
-    auto* rawBuffer = buffer.release();
-    JSObject* arrayBuffer = JS::NewArrayBufferWithContents(cx, size, rawBuffer);
+    UniquePtr<void, JS::FreePolicy> dataBuffer{buffer.release()};
+    JSObject* arrayBuffer =
+        JS::NewArrayBufferWithContents(cx, size, std::move(dataBuffer));
     if (!arrayBuffer) {
-      js_free(rawBuffer);
       return false;
     }
 

js/src/jsapi-tests/testArrayBuffer.cpp

@@ -66,14 +66,15 @@ BEGIN_TEST(testArrayBuffer_bug720949_steal) {
     CHECK(v.isInt32(MAGIC_VALUE_2));
 
     // Steal the contents
-    void* contents = JS::StealArrayBufferContents(cx, obj);
+    mozilla::UniquePtr<void, JS::FreePolicy> contents{
+        JS::StealArrayBufferContents(cx, obj)};
     CHECK(contents != nullptr);
 
     CHECK(JS::IsDetachedArrayBufferObject(obj));
 
     // Transfer to a new ArrayBuffer
-    JS::RootedObject dst(cx,
-                         JS::NewArrayBufferWithContents(cx, size, contents));
+    JS::RootedObject dst(
+        cx, JS::NewArrayBufferWithContents(cx, size, std::move(contents)));
     CHECK(JS::IsArrayBufferObject(dst));
     {
       JS::AutoCheckCannotGC nogc;

mfbt/Span.h

@@ -490,8 +490,9 @@ class Span {
   /**
    * Constructor for mozilla::UniquePtr holding an array and length.
    */
-  template <class ArrayElementType = std::add_pointer<element_type>>
-  constexpr Span(const mozilla::UniquePtr<ArrayElementType>& aPtr,
+  template <class ArrayElementType = std::add_pointer<element_type>,
+            class DeleterType>
+  constexpr Span(const mozilla::UniquePtr<ArrayElementType, DeleterType>& aPtr,
                  index_type aLength)
       : storage_(aPtr.get(), aLength) {}