Bug 995278 - JS_NewArrayBufferContents frees user data on error. r=sfink

content/base/src/nsXMLHttpRequest.cpp

@@ -3930,13 +3930,12 @@ ArrayBufferBuilder::getArrayBuffer(JSContext* aCx)
   }
 
   JSObject* obj = JS_NewArrayBufferWithContents(aCx, mLength, mDataPtr);
-  if (!obj) {
-    return nullptr;
-  }
-
   mDataPtr = nullptr;
   mLength = mCapacity = 0;
-
+  if (!obj) {
+    js_free(mDataPtr);
+    return nullptr;
+  }
   return obj;
 }
 

js/src/jsapi-tests/testMappedArrayBuffer.cpp

@@ -65,7 +65,10 @@ JSObject *CreateNewObject(const int offset, const int length)
     if (!ptr)
         return nullptr;
     JSObject *obj = JS_NewMappedArrayBufferWithContents(cx, length, ptr);
-
+    if (!obj) {
+        JS_ReleaseMappedArrayBufferContents(ptr, length);
+        return nullptr;
+    }
     return obj;
 }
 

js/src/jsapi.h

@@ -3171,9 +3171,8 @@ JS_PUBLIC_API(void)
 JS_SetAllNonReservedSlotsToUndefined(JSContext *cx, JSObject *objArg);
 
 /*
- * Create a new array buffer with the given contents. The new array buffer
- * takes ownership: after calling this function, do not free |contents| or use
- * |contents| from another thread.
+ * Create a new array buffer with the given contents. On success, the ownership
+ * is transferred to the new array buffer.
  */
 extern JS_PUBLIC_API(JSObject *)
 JS_NewArrayBufferWithContents(JSContext *cx, size_t nbytes, void *contents);
@@ -3205,7 +3204,8 @@ extern JS_PUBLIC_API(void *)
 JS_ReallocateArrayBufferContents(JSContext *cx, uint32_t nbytes, void *oldContents, uint32_t oldNbytes);
 
 /*
- * Create a new mapped array buffer with the given memory mapped contents.
+ * Create a new mapped array buffer with the given memory mapped contents. On success,
+ * the ownership is transferred to the new mapped array buffer.
  */
 extern JS_PUBLIC_API(JSObject *)
 JS_NewMappedArrayBufferWithContents(JSContext *cx, size_t nbytes, void *contents);

js/src/vm/ArrayBufferObject.cpp

@@ -651,11 +651,9 @@ ArrayBufferObject::create(JSContext *cx, uint32_t nbytes, void *data /* = nullpt
     gc::AllocKind allocKind = GetGCObjectKind(nslots);
 
     Rooted<ArrayBufferObject*> obj(cx, NewBuiltinClassInstance<ArrayBufferObject>(cx, allocKind, newKind));
-    if (!obj) {
-        if (data)
-            js_free(data);
+    if (!obj)
         return nullptr;
-    }
+
     JS_ASSERT(obj->getClass() == &class_);
 
     JS_ASSERT(!gc::IsInsideNursery(cx->runtime(), obj));

js/src/vm/StructuredClone.cpp

@@ -1572,7 +1572,9 @@ JSStructuredCloneReader::readTransferMap()
             MOZ_ASSERT(obj);
             MOZ_ASSERT(!cx->isExceptionPending());
         }
-
+        
+        // On failure, the buffer will still own the data (since its ownership will not get set to SCTAG_TMO_UNOWNED),
+        // so the data will be freed by ClearStructuredClone
         if (!obj)
             return false;