Bug 668915 - JSAutoStructuredCloneBuffer shouldn't require a 'cx' (r=bent,jorendorff)
authorLuke Wagner <luke@mozilla.com>
Tue, 12 Jul 2011 10:42:24 -0700
changeset 73524 eaf738a0e1b331e72aad1941425860af311c9824
parent 73523 567a0c458ae00e8002368e0ea8eace49141924d6
child 73525 74a4b995cfe95fde5d2a842dbca5168ad265ec90
push idunknown
push userunknown
push dateunknown
reviewersbent, jorendorff
bugs668915
milestone8.0a1
Bug 668915 - JSAutoStructuredCloneBuffer shouldn't require a 'cx' (r=bent,jorendorff)
dom/base/nsGlobalWindow.cpp
dom/indexedDB/AsyncConnectionHelper.cpp
dom/indexedDB/IDBCursor.cpp
dom/indexedDB/IDBIndex.cpp
dom/indexedDB/IDBObjectStore.cpp
dom/workers/Events.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/XMLHttpRequestPrivate.cpp
js/src/Makefile.in
js/src/jsapi.cpp
js/src/jsapi.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -5981,17 +5981,17 @@ PostMessageEvent::Run()
     }
   }
 
   // If we bailed before this point we're going to leak mMessage, but
   // that's probably better than crashing.
 
   // 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;
 
   nsRefPtr<nsGlobalWindow> targetWindow;
   if (mTargetWindow->IsClosedOrClosing() ||
       !(targetWindow = mTargetWindow->GetCurrentInnerWindowInternal()) ||
       targetWindow->IsClosedOrClosing())
     return NS_OK;
@@ -6035,17 +6035,17 @@ PostMessageEvent::Run()
       return NS_OK;
   }
 
   // Deserialize the structured clone data
   jsval messageData;
   {
     JSAutoRequest ar(cx);
 
-    if (!buffer.read(&messageData, cx, nsnull))
+    if (!buffer.read(cx, &messageData, nsnull))
       return NS_ERROR_DOM_DATA_CLONE_ERR;
   }
 
   // Create the event
   nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(targetWindow->mDocument);
   if (!domDoc)
     return NS_OK;
   nsCOMPtr<nsIDOMEvent> event;
--- a/dom/indexedDB/AsyncConnectionHelper.cpp
+++ b/dom/indexedDB/AsyncConnectionHelper.cpp
@@ -501,17 +501,17 @@ AsyncConnectionHelper::ConvertCloneBuffe
   NS_ASSERTION(aCx, "Null context!");
   NS_ASSERTION(aResult, "Null pointer!");
 
   JSAutoRequest ar(aCx);
 
   nsresult rv = ConvertCloneBuffersToArrayInternal(aCx, aBuffers, aResult);
 
   for (PRUint32 index = 0; index < aBuffers.Length(); index++) {
-    aBuffers[index].clear(aCx);
+    aBuffers[index].clear();
   }
   aBuffers.Clear();
 
   return rv;
 }
 
 NS_IMETHODIMP_(nsrefcnt)
 TransactionPoolEventTarget::AddRef()
--- a/dom/indexedDB/IDBCursor.cpp
+++ b/dom/indexedDB/IDBCursor.cpp
@@ -470,17 +470,17 @@ IDBCursor::GetValue(JSContext* aCx,
       mRooted = true;
     }
 
     if (!IDBObjectStore::DeserializeValue(aCx, mCloneBuffer, &mCachedValue)) {
       mCachedValue = JSVAL_VOID;
       return NS_ERROR_DOM_DATA_CLONE_ERR;
     }
 
-    mCloneBuffer.clear(aCx);
+    mCloneBuffer.clear();
     mHaveCachedValue = true;
   }
 
   *aValue = mCachedValue;
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -715,17 +715,17 @@ ContinueHelper::GetSuccessResult(JSConte
                  !mObjectKey.IsUnset(), "Bad key!");
 
     // Set new values.
     mCursor->mKey = mKey;
     mCursor->mObjectKey = mObjectKey;
     mCursor->mContinueToKey = Key::UNSETKEY;
 
     mCursor->mCloneBuffer.swap(mCloneBuffer);
-    mCloneBuffer.clear(aCx);
+    mCloneBuffer.clear();
 
     nsresult rv = WrapNative(aCx, mCursor, aVal);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
--- a/dom/indexedDB/IDBIndex.cpp
+++ b/dom/indexedDB/IDBIndex.cpp
@@ -777,17 +777,17 @@ GetHelper::DoDatabaseWork(mozIStorageCon
 }
 
 nsresult
 GetHelper::GetSuccessResult(JSContext* aCx,
                             jsval* aVal)
 {
   bool result = IDBObjectStore::DeserializeValue(aCx, mCloneBuffer, aVal);
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
   return NS_OK;
 }
 
 nsresult
 GetAllKeysHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
@@ -1051,17 +1051,17 @@ nsresult
 GetAllHelper::GetSuccessResult(JSContext* aCx,
                                jsval* aVal)
 {
   NS_ASSERTION(mCloneBuffers.Length() <= mLimit, "Too many results!");
 
   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);
   return NS_OK;
 }
 
 nsresult
 OpenKeyCursorHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -834,48 +834,27 @@ IDBObjectStore::GetStructuredCloneDataFr
   }
 #endif
 
   const PRUint8* data;
   PRUint32 dataLength;
   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
 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();
   }
 }
 
 // static
 bool
 IDBObjectStore::DeserializeValue(JSContext* aCx,
                                  JSAutoStructuredCloneBuffer& aBuffer,
                                  jsval* aValue,
@@ -888,17 +867,17 @@ IDBObjectStore::DeserializeValue(JSConte
 
   if (!aBuffer.data()) {
     *aValue = JSVAL_VOID;
     return true;
   }
 
   JSAutoRequest ar(aCx);
 
-  return aBuffer.read(aValue, aCx, aCallbacks, aClosure);
+  return aBuffer.read(aCx, aValue, aCallbacks, aClosure);
 }
 
 // static
 bool
 IDBObjectStore::SerializeValue(JSContext* aCx,
                                JSAutoStructuredCloneBuffer& aBuffer,
                                jsval aValue,
                                JSStructuredCloneCallbacks* aCallbacks,
@@ -1895,17 +1874,17 @@ AddHelper::DoDatabaseWork(mozIStorageCon
 }
 
 nsresult
 AddHelper::GetSuccessResult(JSContext* aCx,
                             jsval* aVal)
 {
   NS_ASSERTION(!mKey.IsUnset(), "Badness!");
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   return IDBObjectStore::GetJSValFromKey(mKey, aCx, aVal);
 }
 
 nsresult
 GetHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
   NS_PRECONDITION(aConnection, "Passed a null connection!");
@@ -1950,17 +1929,17 @@ GetHelper::DoDatabaseWork(mozIStorageCon
 }
 
 nsresult
 GetHelper::GetSuccessResult(JSContext* aCx,
                             jsval* aVal)
 {
   bool result = IDBObjectStore::DeserializeValue(aCx, mCloneBuffer, aVal);
 
-  mCloneBuffer.clear(aCx);
+  mCloneBuffer.clear();
 
   NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
   return NS_OK;
 }
 
 nsresult
 DeleteHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
@@ -2525,14 +2504,14 @@ nsresult
 GetAllHelper::GetSuccessResult(JSContext* aCx,
                                jsval* aVal)
 {
   NS_ASSERTION(mCloneBuffers.Length() <= mLimit, "Too many results!");
 
   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);
   return NS_OK;
 }
--- a/dom/workers/Events.cpp
+++ b/dom/workers/Events.cpp
@@ -560,23 +560,23 @@ private:
     MessageEvent* event = GetInstancePrivate(aCx, aObj, name);
     if (!event) {
       return false;
     }
 
     // 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;
       }
 
       *aVp = data;
       return true;
     }
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -527,17 +527,17 @@ public:
   {
     aData.steal(&mData, &mDataByteCount);
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
   {
     JSAutoStructuredCloneBuffer buffer;
-    buffer.adopt(aCx, mData, mDataByteCount);
+    buffer.adopt(mData, mDataByteCount);
 
     mData = nsnull;
     mDataByteCount = 0;
 
     bool mainRuntime;
     JSObject* target;
     if (mTarget == ParentThread) {
       mainRuntime = !aWorkerPrivate->GetParent();
--- a/dom/workers/XMLHttpRequestPrivate.cpp
+++ b/dom/workers/XMLHttpRequestPrivate.cpp
@@ -950,27 +950,27 @@ public:
       nsIXPConnect* xpc = nsContentUtils::XPConnect();
       NS_ASSERTION(xpc, "This should never be null!");
 
       RuntimeService::AutoSafeJSContext cx;
 
       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;
         }
       }
       else {
         error = DATA_CLONE_ERR;
       }
 
-      mBody.clear(cx);
+      mBody.clear();
 
       if (error) {
         return error;
       }
     }
 
     if (mHasUploadListeners) {
       NS_ASSERTION(!mProxy->mUploadEventListenersAttached, "Huh?!");
--- a/js/src/Makefile.in
+++ b/js/src/Makefile.in
@@ -670,17 +670,17 @@ check-malloc-function-usage: $(filter-ou
 		"in Makefile.in" "cx->calloc_ or rt->calloc_" $^
 	$(srcdir)/config/check_source_count.py "\bjs_realloc\b" 0 \
 		"in Makefile.in" "cx->realloc_ or rt->realloc_" $^
 	$(srcdir)/config/check_source_count.py "\bjs_free\b" 0 \
 		"in Makefile.in" "cx->free_" $^
 
 	# 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 \
 		"in Makefile.in" "{cx,rt}->{free_,delete_,array_delete}" $^
 
 ifneq ($(OS_ARCH),WINNT) # FIXME: this should be made work on Windows too.
 check:: check-malloc-function-usage
 endif
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -5593,17 +5593,108 @@ JS_StructuredClone(JSContext *cx, jsval 
                    void *closure)
 {
     const JSStructuredCloneCallbacks *callbacks =
         optionalCallbacks ?
         optionalCallbacks :
         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_SetStructuredCloneCallbacks(JSRuntime *rt, const JSStructuredCloneCallbacks *callbacks)
 {
     rt->structuredCloneCallbacks = callbacks;
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3369,129 +3369,72 @@ JS_WriteStructuredClone(JSContext *cx, j
                         void *closure);
 
 JS_PUBLIC_API(JSBool)
 JS_StructuredClone(JSContext *cx, jsval v, jsval *vp,
                    const JSStructuredCloneCallbacks *optionalCallbacks,
                    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_;
-
-        cx_ = NULL;
-        data_ = NULL;
-        nbytes_ = 0;
-        version_ = 0;
-    }
-
-    bool read(jsval *vp, JSContext *cx=NULL,
+    void steal(JSUint64 **datap, size_t *nbytesp, JSUint32 *versionp=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.) */
 
 /* The range of tag values the application may use for its own custom object types. */
 #define JS_SCTAG_USER_MIN  ((uint32) 0xFFFF8000)
 #define JS_SCTAG_USER_MAX  ((uint32) 0xFFFFFFFF)