Bug 668915 - JSAutoStructuredCloneBuffer shouldn't require a 'cx' (r=bent,jorendorff)

--HG--
extra : rebase_source : f2fe9ab1e25188223f75e8d2ddcbdf977e62782b

dom/base/nsGlobalWindow.cpp

@@ -5986,7 +5986,7 @@ PostMessageEvent::Run()
 
   // Ensure that the buffer is freed even if we fail to post the message
   JSAutoStructuredCloneBuffer buffer;
-  buffer.adopt(cx, mMessage, mMessageLen);
+  buffer.adopt(mMessage, mMessageLen);
   mMessage = nsnull;
   mMessageLen = 0;
 
@@ -6040,7 +6040,7 @@ PostMessageEvent::Run()
   {
     JSAutoRequest ar(cx);
 
-    if (!buffer.read(&messageData, cx, nsnull))
+    if (!buffer.read(cx, &messageData, nsnull))
       return NS_ERROR_DOM_DATA_CLONE_ERR;
   }
 

dom/indexedDB/AsyncConnectionHelper.cpp

@@ -506,7 +506,7 @@ AsyncConnectionHelper::ConvertCloneBuffersToArray(
   nsresult rv = ConvertCloneBuffersToArrayInternal(aCx, aBuffers, aResult);
 
   for (PRUint32 index = 0; index < aBuffers.Length(); index++) {
-    aBuffers[index].clear(aCx);
+    aBuffers[index].clear();
   }
   aBuffers.Clear();
 

dom/indexedDB/IDBCursor.cpp

@@ -475,7 +475,7 @@ IDBCursor::GetValue(JSContext* aCx,
       return NS_ERROR_DOM_DATA_CLONE_ERR;
     }
 
-    mCloneBuffer.clear(aCx);
+    mCloneBuffer.clear();
     mHaveCachedValue = true;
   }
 
@@ -720,7 +720,7 @@ ContinueHelper::GetSuccessResult(JSContext* aCx,
     mCursor->mContinueToKey = Key::UNSETKEY;
 
     mCursor->mCloneBuffer.swap(mCloneBuffer);
-    mCloneBuffer.clear(aCx);
+    mCloneBuffer.clear();
 
     nsresult rv = WrapNative(aCx, mCursor, aVal);
     NS_ENSURE_SUCCESS(rv, rv);

dom/indexedDB/IDBIndex.cpp

@@ -782,7 +782,7 @@ GetHelper::GetSuccessResult(JSContext* aCx,
 {
   bool result = IDBObjectStore::DeserializeValue(aCx, mCloneBuffer, aVal);
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
   return NS_OK;
@@ -1056,7 +1056,7 @@ GetAllHelper::GetSuccessResult(JSContext* aCx,
   nsresult rv = ConvertCloneBuffersToArray(aCx, mCloneBuffers, aVal);
 
   for (PRUint32 index = 0; index < mCloneBuffers.Length(); index++) {
-    mCloneBuffers[index].clear(aCx);
+    mCloneBuffers[index].clear();
   }
 
   NS_ENSURE_SUCCESS(rv, rv);

dom/indexedDB/IDBObjectStore.cpp

@@ -839,19 +839,9 @@ IDBObjectStore::GetStructuredCloneDataFromStatement(
   nsresult rv = aStatement->GetSharedBlob(aIndex, &dataLength, &data);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
-  JSContext* cx;
-  rv = nsContentUtils::ThreadJSContextStack()->GetSafeJSContext(&cx);
-  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-  JSAutoRequest ar(cx);
-
-  uint64* newData = static_cast<uint64*>(JS_malloc(cx, dataLength));
-  NS_ENSURE_TRUE(newData, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-  memcpy(newData, data, dataLength);
-  aBuffer.adopt(cx, newData, dataLength);
-
-  return NS_OK;
+  return aBuffer.copy(reinterpret_cast<const uint64 *>(data), dataLength) ?
+         NS_OK :
+         NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
 }
 
 // static
@@ -859,18 +849,7 @@ void
 IDBObjectStore::ClearStructuredCloneBuffer(JSAutoStructuredCloneBuffer& aBuffer)
 {
   if (aBuffer.data()) {
-    JSContext* cx;
-    if (NS_SUCCEEDED(nsContentUtils::ThreadJSContextStack()->
-                     GetSafeJSContext(&cx))) {
-      JSAutoRequest ar(cx);
-      aBuffer.clear(cx);
-    }
-    else {
-      NS_WARNING("Couldn't get safe JSContext! Leaking data!");
-      uint64* data;
-      size_t length;
-      aBuffer.steal(&data, &length);
-    }
+    aBuffer.clear();
   }
 }
 
@@ -893,7 +872,7 @@ IDBObjectStore::DeserializeValue(JSContext* aCx,
 
   JSAutoRequest ar(aCx);
 
-  return aBuffer.read(aValue, aCx, aCallbacks, aClosure);
+  return aBuffer.read(aCx, aValue, aCallbacks, aClosure);
 }
 
 // static
@@ -1900,7 +1879,7 @@ AddHelper::GetSuccessResult(JSContext* aCx,
 {
   NS_ASSERTION(!mKey.IsUnset(), "Badness!");
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   return IDBObjectStore::GetJSValFromKey(mKey, aCx, aVal);
 }
@@ -1955,7 +1934,7 @@ GetHelper::GetSuccessResult(JSContext* aCx,
 {
   bool result = IDBObjectStore::DeserializeValue(aCx, mCloneBuffer, aVal);
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
   return NS_OK;
@@ -2530,7 +2509,7 @@ GetAllHelper::GetSuccessResult(JSContext* aCx,
   nsresult rv = ConvertCloneBuffersToArray(aCx, mCloneBuffers, aVal);
 
   for (PRUint32 index = 0; index < mCloneBuffers.Length(); index++) {
-    mCloneBuffers[index].clear(aCx);
+    mCloneBuffers[index].clear();
   }
 
   NS_ENSURE_SUCCESS(rv, rv);

dom/workers/Events.cpp

@@ -565,13 +565,13 @@ private:
     // Deserialize and save the data value if we can.
     if (slot == SLOT_data && event->mData) {
       JSAutoStructuredCloneBuffer buffer;
-      buffer.adopt(aCx, event->mData, event->mDataByteCount);
+      buffer.adopt(event->mData, event->mDataByteCount);
 
       event->mData = NULL;
       event->mDataByteCount = 0;
 
       jsval data;
-      if (!buffer.read(&data) ||
+      if (!buffer.read(aCx, &data) ||
           !JS_SetReservedSlot(aCx, aObj, slot, data)) {
         return false;
       }

dom/workers/WorkerPrivate.cpp

@@ -532,7 +532,7 @@ public:
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
   {
     JSAutoStructuredCloneBuffer buffer;
-    buffer.adopt(aCx, mData, mDataByteCount);
+    buffer.adopt(mData, mDataByteCount);
 
     mData = nsnull;
     mDataByteCount = 0;

dom/workers/XMLHttpRequestPrivate.cpp

@@ -955,7 +955,7 @@ public:
       intN error = 0;
 
       jsval body;
-      if (mBody.read(&body, cx)) {
+      if (mBody.read(cx, &body)) {
         if (NS_FAILED(xpc->JSValToVariant(cx, &body,
                                           getter_AddRefs(variant)))) {
           error = INVALID_STATE_ERR;
@@ -965,7 +965,7 @@ public:
         error = DATA_CLONE_ERR;
       }
 
-      mBody.clear(cx);
+      mBody.clear();
 
       if (error) {
         return error;

js/src/Makefile.in

@@ -675,7 +675,7 @@ check-malloc-function-usage: $(filter-out %jsalloc.h %jscntxt.h %jsutil.h, $(ALL
 
 	# We desire these numbers to go down, not up. See "User guide to memory
 	# management within SpiderMonkey" in jsutil.h.
-	$(srcdir)/config/check_source_count.py OffTheBooks:: 58 \
+	$(srcdir)/config/check_source_count.py OffTheBooks:: 59 \
 		"in Makefile.in" "{cx,rt}->{new_,new_array,malloc_,calloc_,realloc_}" $^
 	# This should go to zero, if possible.
 	$(srcdir)/config/check_source_count.py UnwantedForeground:: 31 \

js/src/jsapi.cpp

@@ -5598,7 +5598,98 @@ JS_StructuredClone(JSContext *cx, jsval v, jsval *vp,
         cx->runtime->structuredCloneCallbacks;
     JSAutoStructuredCloneBuffer buf;
     return buf.write(cx, v, callbacks, closure) &&
-           buf.read(vp, cx, callbacks, closure);
+           buf.read(cx, vp, callbacks, closure);
+}
+
+void
+JSAutoStructuredCloneBuffer::clear()
+{
+    if (data_) {
+        Foreground::free_(data_);
+        data_ = NULL;
+        nbytes_ = 0;
+        version_ = 0;
+    }
+}
+
+void
+JSAutoStructuredCloneBuffer::adopt(JSUint64 *data, size_t nbytes, JSUint32 version)
+{
+    clear();
+    data_ = data;
+    nbytes_ = nbytes;
+    version_ = version;
+}
+
+bool
+JSAutoStructuredCloneBuffer::copy(const JSUint64 *srcData, size_t nbytes, JSUint32 version)
+{
+    JSUint64 *newData = static_cast<JSUint64 *>(OffTheBooks::malloc_(nbytes));
+    if (!newData)
+        return false;
+
+    memcpy(newData, srcData, nbytes);
+
+    clear();
+    data_ = newData;
+    nbytes_ = nbytes;
+    version_ = version;
+    return true;
+}
+void
+JSAutoStructuredCloneBuffer::steal(JSUint64 **datap, size_t *nbytesp, JSUint32 *versionp)
+{
+    *datap = data_;
+    *nbytesp = nbytes_;
+    if (versionp)
+        *versionp = version_;
+
+    data_ = NULL;
+    nbytes_ = 0;
+    version_ = 0;
+}
+
+bool
+JSAutoStructuredCloneBuffer::read(JSContext *cx, jsval *vp,
+                                  const JSStructuredCloneCallbacks *optionalCallbacks,
+                                  void *closure) const
+{
+    JS_ASSERT(cx);
+    JS_ASSERT(data_);
+    return !!JS_ReadStructuredClone(cx, data_, nbytes_, version_, vp,
+                                    optionalCallbacks, closure);
+}
+
+bool
+JSAutoStructuredCloneBuffer::write(JSContext *cx, jsval v,
+                                   const JSStructuredCloneCallbacks *optionalCallbacks,
+                                   void *closure)
+{
+    clear();
+    bool ok = !!JS_WriteStructuredClone(cx, v, &data_, &nbytes_,
+                                        optionalCallbacks, closure);
+    if (!ok) {
+        data_ = NULL;
+        nbytes_ = 0;
+        version_ = JS_STRUCTURED_CLONE_VERSION;
+    }
+    return ok;
+}
+
+void
+JSAutoStructuredCloneBuffer::swap(JSAutoStructuredCloneBuffer &other)
+{
+    JSUint64 *data = other.data_;
+    size_t nbytes = other.nbytes_;
+    JSUint32 version = other.version_;
+
+    other.data_ = this->data_;
+    other.nbytes_ = this->nbytes_;
+    other.version_ = this->version_;
+
+    this->data_ = data;
+    this->nbytes_ = nbytes;
+    this->version_ = version;
 }
 
 JS_PUBLIC_API(void)

js/src/jsapi.h

@@ -3374,119 +3374,62 @@ JS_StructuredClone(JSContext *cx, jsval v, jsval *vp,
                    void *closure);
 
 #ifdef __cplusplus
+JS_END_EXTERN_C
+
 /* RAII sugar for JS_WriteStructuredClone. */
-class JSAutoStructuredCloneBuffer {
-    JSContext *cx_;
-    uint64 *data_;
+class JS_PUBLIC_API(JSAutoStructuredCloneBuffer) {
+    JSUint64 *data_;
     size_t nbytes_;
-    uint32 version_;
+    JSUint32 version_;
 
   public:
     JSAutoStructuredCloneBuffer()
-        : cx_(NULL), data_(NULL), nbytes_(0), version_(JS_STRUCTURED_CLONE_VERSION) {}
+        : data_(NULL), nbytes_(0), version_(JS_STRUCTURED_CLONE_VERSION) {}
 
     ~JSAutoStructuredCloneBuffer() { clear(); }
 
-    JSContext *cx() const { return cx_; }
-    uint64 *data() const { return data_; }
+    JSUint64 *data() const { return data_; }
     size_t nbytes() const { return nbytes_; }
 
-    void clear(JSContext *cx=NULL) {
-        if (data_) {
-            if (!cx)
-                cx = cx_;
-            JS_ASSERT(cx);
-            JS_free(cx, data_);
-            cx_ = NULL;
-            data_ = NULL;
-            nbytes_ = 0;
-            version_ = 0;
-        }
-    }
+    void clear();
+
+    /* Copy some memory. It will be automatically freed by the destructor. */
+    bool copy(const JSUint64 *data, size_t nbytes, JSUint32 version=JS_STRUCTURED_CLONE_VERSION);
 
     /*
      * Adopt some memory. It will be automatically freed by the destructor.
-     * data must have been allocated using JS_malloc.
+     * data must have been allocated by the JS engine (e.g., extracted via
+     * JSAutoStructuredCloneBuffer::steal).
      */
-    void adopt(JSContext *cx, uint64 *data, size_t nbytes,
-               uint32 version=JS_STRUCTURED_CLONE_VERSION) {
-        clear(cx);
-        cx_ = cx;
-        data_ = data;
-        nbytes_ = nbytes;
-        version_ = version;
-    }
+    void adopt(JSUint64 *data, size_t nbytes, JSUint32 version=JS_STRUCTURED_CLONE_VERSION);
 
     /*
      * Remove the buffer so that it will not be automatically freed.
-     * After this, the caller is responsible for calling JS_free(*datap).
+     * After this, the caller is responsible for feeding the memory back to
+     * JSAutoStructuredCloneBuffer::adopt.
      */
-    void steal(uint64 **datap, size_t *nbytesp, JSContext **cxp=NULL,
-               uint32 *versionp=NULL) {
-        *datap = data_;
-        *nbytesp = nbytes_;
-        if (cxp)
-            *cxp = cx_;
-        if (versionp)
-            *versionp = version_;
+    void steal(JSUint64 **datap, size_t *nbytesp, JSUint32 *versionp=NULL);
 
-        cx_ = NULL;
-        data_ = NULL;
-        nbytes_ = 0;
-        version_ = 0;
-    }
-
-    bool read(jsval *vp, JSContext *cx=NULL,
+    bool read(JSContext *cx, jsval *vp,
               const JSStructuredCloneCallbacks *optionalCallbacks=NULL,
-              void *closure=NULL) const {
-        if (!cx)
-            cx = cx_;
-        JS_ASSERT(cx);
-        JS_ASSERT(data_);
-        return !!JS_ReadStructuredClone(cx, data_, nbytes_, version_, vp,
-                                        optionalCallbacks, closure);
-    }
+              void *closure=NULL) const;
 
     bool write(JSContext *cx, jsval v,
                const JSStructuredCloneCallbacks *optionalCallbacks=NULL,
-               void *closure=NULL) {
-        clear(cx);
-        cx_ = cx;
-        bool ok = !!JS_WriteStructuredClone(cx, v, &data_, &nbytes_,
-                                            optionalCallbacks, closure);
-        if (!ok) {
-            data_ = NULL;
-            nbytes_ = 0;
-            version_ = JS_STRUCTURED_CLONE_VERSION;
-        }
-        return ok;
-    }
+               void *closure=NULL);
 
     /**
      * Swap ownership with another JSAutoStructuredCloneBuffer.
      */
-    void swap(JSAutoStructuredCloneBuffer &other) {
-        JSContext *cx = other.cx_;
-        uint64 *data = other.data_;
-        size_t nbytes = other.nbytes_;
-        uint32 version = other.version_;
-
-        other.cx_ = this->cx_;
-        other.data_ = this->data_;
-        other.nbytes_ = this->nbytes_;
-        other.version_ = this->version_;
-
-        this->cx_ = cx;
-        this->data_ = data;
-        this->nbytes_ = nbytes;
-        this->version_ = version;
-    }
+    void swap(JSAutoStructuredCloneBuffer &other);
 
   private:
     /* Copy and assignment are not supported. */
     JSAutoStructuredCloneBuffer(const JSAutoStructuredCloneBuffer &other);
     JSAutoStructuredCloneBuffer &operator=(const JSAutoStructuredCloneBuffer &other);
 };
+
+JS_BEGIN_EXTERN_C
 #endif
 
 /* API for implementing custom serialization behavior (for ImageData, File, etc.) */