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 id20884
push usermak77@bonardo.net
push dateFri, 29 Jul 2011 09:49:02 +0000
treeherdermozilla-central@f5f1e3822540 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent, jorendorff
bugs668915
milestone8.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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)