Bug 1529298 - Implement an ArrayBufferObject::extractStructuredCloneContents for structured-cloning an ArrayBuffer. r=sfink
authorJeff Walden <jwalden@mit.edu>
Wed, 20 Feb 2019 13:33:16 -0800
changeset 519498 63536a044a29b3f723e3ee829f260dafc41feb54
parent 519497 0df9a1e522afbc4fedd23fa72bc307aac7d06185
child 519499 5cb592de5e03d0df8254bbd779d84760b0d16eef
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1529298
milestone67.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 1529298 - Implement an ArrayBufferObject::extractStructuredCloneContents for structured-cloning an ArrayBuffer. r=sfink
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/vm/StructuredClone.cpp
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1444,16 +1444,69 @@ ArrayBufferObject* ArrayBufferObject::cr
       MOZ_ASSERT_UNREACHABLE("bad kind when stealing malloc'd data");
       return nullptr;
   }
 
   MOZ_ASSERT_UNREACHABLE("garbage kind computed");
   return nullptr;
 }
 
+/* static */ ArrayBufferObject::BufferContents
+ArrayBufferObject::extractStructuredCloneContents(
+    JSContext* cx, Handle<ArrayBufferObject*> buffer) {
+  CheckStealPreconditions(buffer, cx);
+
+  BufferContents contents = buffer->contents();
+
+  switch (contents.kind()) {
+    case INLINE_DATA:
+    case NO_DATA:
+    case USER_OWNED: {
+      uint8_t* copiedData = NewCopiedBufferContents(cx, buffer);
+      if (!copiedData) {
+        return BufferContents::createFailed();
+      }
+
+      ArrayBufferObject::detach(cx, buffer, BufferContents::createNoData());
+      return BufferContents::createMalloced(copiedData);
+    }
+
+    case MALLOCED:
+    case MAPPED: {
+      MOZ_ASSERT(contents);
+
+      // Overwrite the old data pointer *without* releasing old data.
+      BufferContents newContents = BufferContents::createNoData();
+      buffer->setDataPointer(newContents, OwnsData);
+
+      // Detach |buffer| now that doing so won't release |oldContents|.
+      ArrayBufferObject::detach(cx, buffer, newContents);
+      return contents;
+    }
+
+    case WASM:
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_WASM_NO_TRANSFER);
+      return BufferContents::createFailed();
+
+    case EXTERNAL:
+      MOZ_ASSERT_UNREACHABLE(
+          "external ArrayBuffer shouldn't have passed the "
+          "structured-clone preflighting");
+      break;
+
+    case BAD1:
+      MOZ_ASSERT_UNREACHABLE("bad kind when stealing malloc'd data");
+      break;
+  }
+
+  MOZ_ASSERT_UNREACHABLE("garbage kind computed");
+  return BufferContents::createFailed();
+}
+
 /* static */ ArrayBufferObject::BufferContents ArrayBufferObject::stealContents(
     JSContext* cx, Handle<ArrayBufferObject*> buffer,
     bool hasStealableContents) {
   CheckStealPreconditions(buffer, cx);
 
 #ifdef DEBUG
   if (hasStealableContents) {
     MOZ_ASSERT(!buffer->isInlineData(),
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -353,16 +353,19 @@ class ArrayBufferObject : public ArrayBu
 
   static uint8_t* stealMallocedContents(JSContext* cx,
                                         Handle<ArrayBufferObject*> buffer);
 
   static BufferContents stealContents(JSContext* cx,
                                       Handle<ArrayBufferObject*> buffer,
                                       bool hasStealableContents);
 
+  static BufferContents extractStructuredCloneContents(
+      JSContext* cx, Handle<ArrayBufferObject*> buffer);
+
   static void addSizeOfExcludingThis(JSObject* obj,
                                      mozilla::MallocSizeOf mallocSizeOf,
                                      JS::ClassInfo* info);
 
   // ArrayBufferObjects (strongly) store the first view added to them, while
   // later views are (weakly) stored in the compartment's InnerViewTable
   // below. Buffers usually only have one view, so this slot optimizes for
   // the common case. Avoiding entries in the InnerViewTable saves memory and
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -1828,30 +1828,22 @@ bool JSStructuredCloneWriter::transferOw
       JSAutoRealm ar(cx, arrayBuffer);
 
       if (arrayBuffer->isDetached()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                                   JSMSG_TYPED_ARRAY_DETACHED);
         return false;
       }
 
-      size_t nbytes = arrayBuffer->byteLength();
-
-      if (arrayBuffer->isWasm() || arrayBuffer->isPreparedForAsmJS()) {
+      if (arrayBuffer->isPreparedForAsmJS()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                                   JSMSG_WASM_NO_TRANSFER);
         return false;
       }
 
-      if (arrayBuffer->isDetached()) {
-        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                  JSMSG_TYPED_ARRAY_DETACHED);
-        return false;
-      }
-
       if (scope == JS::StructuredCloneScope::DifferentProcess ||
           scope == JS::StructuredCloneScope::DifferentProcessForIndexedDB) {
         // Write Transferred ArrayBuffers in DifferentProcess scope at
         // the end of the clone buffer, and store the offset within the
         // buffer to where the ArrayBuffer was written. Note that this
         // will invalidate the current position iterator.
 
         size_t pointOffset = out.offset(point);
@@ -1869,37 +1861,22 @@ bool JSStructuredCloneWriter::transferOw
         // been modified.
         point = out.iter();
         point += pointOffset;
 
         if (!JS_DetachArrayBuffer(cx, arrayBuffer)) {
           return false;
         }
       } else {
-        // No data isn't stealable because it would fail |!bufContents|: actual
-        // malloc'd data must be created to return.
-        //
-        // XXX This extra caution *may* not actually be necessary: no-data
-        //     ArrayBuffers exist only as the result of detaching ArrayBuffers
-        //     (but these are excluded by the test above) or as the result of a
-        //     wasm memory.grow operation (but contents are then immediately
-        //     replaced with wasm contents).  But it seems better to play it
-        //     safe and deal with *all* states, not just the ones that appear
-        //     possible here.
-        //
-        // Inline data obviously has to be copied.  So too user-owned data
-        bool hasStealableContents =
-          arrayBuffer->ownsData() &&
-          !arrayBuffer->isInlineData() &&
-          !arrayBuffer->isNoData() &&
-          !arrayBuffer->hasUserOwnedData();
-
-        ArrayBufferObject::BufferContents bufContents =
-            ArrayBufferObject::stealContents(cx, arrayBuffer,
-                                             hasStealableContents);
+        size_t nbytes = arrayBuffer->byteLength();
+
+        using BufferContents = ArrayBufferObject::BufferContents;
+
+        BufferContents bufContents =
+            ArrayBufferObject::extractStructuredCloneContents(cx, arrayBuffer);
         if (!bufContents) {
           return false;  // out of memory
         }
 
         content = bufContents.data();
         if (bufContents.kind() == ArrayBufferObject::MAPPED) {
           ownership = JS::SCTAG_TMO_MAPPED_DATA;
         } else {