Refactor structured clone JSAPI to prevent mismatched scopes.

Roll-up of bugs 1442722, 1455071, 1433642, 1456604 and 1458320.

5 files changed, 385 insertions, 254 deletions

diff --git a/js/public/StructuredClone.h b/js/public/StructuredClone.h
index c48975cb95..ebff84387e 100644
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@@ -9,6 +9,7 @@
 
 #include "mozilla/Attributes.h"
 #include "mozilla/BufferList.h"
+#include "mozilla/Move.h"
 
 #include <stdint.h>
 
@@ -29,7 +30,35 @@ namespace JS {
 enum class StructuredCloneScope : uint32_t {
     SameProcessSameThread,
     SameProcessDifferentThread,
-    DifferentProcess
+
+    /**
+     * When writing, this means we're writing for an audience in a different
+     * process. Produce serialized data that can be sent to other processes,
+     * bitwise copied, or even stored as bytes in a database and read by later
+     * versions of Firefox years from now. The HTML5 spec refers to this as
+     * "ForStorage" as in StructuredSerializeForStorage, though we use
+     * DifferentProcess for IPC as well as storage.
+     *
+     * Transferable objects are limited to ArrayBuffers, whose contents are
+     * copied into the serialized data (rather than just writing a pointer).
+     */
+    DifferentProcess,
+
+    /**
+     * Handle a backwards-compatibility case with IndexedDB (bug 1434308): when
+     * reading, this means to treat legacy SameProcessSameThread data as if it
+     * were DifferentProcess.
+     *
+     * Do not use this for writing; use DifferentProcess instead.
+     */
+    DifferentProcessForIndexedDB,
+
+    /**
+     * Existing code wants to be able to create an uninitialized
+     * JSStructuredCloneData without knowing the scope, then populate it with
+     * data (at which point the scope *is* known.)
+     */
+    Unassigned
 };
 
 enum TransferableOwnership {
@@ -89,6 +118,10 @@ class CloneDataPolicy
 
 } /* namespace JS */
 
+namespace js {
+template <typename T, typename AllocPolicy> struct BufferIterator;
+}
+
 /**
  * Read structured data from the reader r. This hook is used to read a value
  * previously serialized by a call to the WriteStructuredCloneOp hook.
@@ -188,49 +221,152 @@ enum OwnTransferablePolicy {
     NoTransferables
 };
 
-class MOZ_NON_MEMMOVABLE JS_PUBLIC_API(JSStructuredCloneData) :
-    public mozilla::BufferList<js::SystemAllocPolicy>
-{
-    typedef js::SystemAllocPolicy AllocPolicy;
-    typedef mozilla::BufferList<js::SystemAllocPolicy> BufferList;
+/**
+ * JSStructuredCloneData represents structured clone data together with the
+ * information needed to read/write/transfer/free the records within it, in the
+ * form of a set of callbacks.
+ */
+class MOZ_NON_MEMMOVABLE JS_PUBLIC_API(JSStructuredCloneData) {
+  public:
+    using BufferList = mozilla::BufferList<js::SystemAllocPolicy>;
+    using Iterator = BufferList::IterImpl;
 
-    static const size_t kInitialSize = 0;
-    static const size_t kInitialCapacity = 4096;
+  private:
     static const size_t kStandardCapacity = 4096;
 
+    BufferList bufList_;
+
+    // The (address space, thread) scope within which this clone is valid. Note
+    // that this must be either set during construction, or start out as
+    // Unassigned and transition once to something else.
+    JS::StructuredCloneScope scope_;
+
     const JSStructuredCloneCallbacks* callbacks_;
     void* closure_;
     OwnTransferablePolicy ownTransferables_;
 
-    void setOptionalCallbacks(const JSStructuredCloneCallbacks* callbacks,
-                              void* closure,
-                              OwnTransferablePolicy policy) {
-        callbacks_ = callbacks;
-        closure_ = closure;
-        ownTransferables_ = policy;
-    }
-
     friend struct JSStructuredCloneWriter;
     friend class JS_PUBLIC_API(JSAutoStructuredCloneBuffer);
+    template <typename T, typename AllocPolicy> friend struct js::BufferIterator;
 
-public:
-    explicit JSStructuredCloneData(AllocPolicy aAP = AllocPolicy())
-        : BufferList(kInitialSize, kInitialCapacity, kStandardCapacity, aAP)
+  public:
+    // The constructor must be infallible but SystemAllocPolicy is not, so both
+    // the initial size and initial capacity of the BufferList must be zero.
+    explicit JSStructuredCloneData(JS::StructuredCloneScope aScope)
+        : bufList_(0, 0, kStandardCapacity, js::SystemAllocPolicy())
+        , scope_(aScope)
         , callbacks_(nullptr)
         , closure_(nullptr)
         , ownTransferables_(OwnTransferablePolicy::NoTransferables)
     {}
-    MOZ_IMPLICIT JSStructuredCloneData(BufferList&& buffers)
-        : BufferList(Move(buffers))
+
+    // Steal the raw data from a BufferList. In this case, we don't know the
+    // scope and none of the callback info is assigned yet.
+    JSStructuredCloneData(BufferList&& buffers, JS::StructuredCloneScope aScope)
+        : bufList_(mozilla::Move(buffers))
+        , scope_(aScope)
         , callbacks_(nullptr)
         , closure_(nullptr)
         , ownTransferables_(OwnTransferablePolicy::NoTransferables)
     {}
+    MOZ_IMPLICIT JSStructuredCloneData(BufferList&& buffers)
+        : JSStructuredCloneData(mozilla::Move(buffers), JS::StructuredCloneScope::Unassigned)
+    {}
     JSStructuredCloneData(JSStructuredCloneData&& other) = default;
     JSStructuredCloneData& operator=(JSStructuredCloneData&& other) = default;
-    ~JSStructuredCloneData();
+    ~JSStructuredCloneData() { discardTransferables(); }
+
+    void setCallbacks(const JSStructuredCloneCallbacks* callbacks,
+                      void* closure,
+                      OwnTransferablePolicy policy)
+    {
+        callbacks_ = callbacks;
+        closure_ = closure;
+        ownTransferables_ = policy;
+    }
+
+    JS::StructuredCloneScope scope() const { return scope_; }
+
+    void initScope(JS::StructuredCloneScope aScope) {
+        MOZ_ASSERT(Size() == 0, "initScope() of nonempty JSStructuredCloneData");
+        if (scope_ != JS::StructuredCloneScope::Unassigned)
+            MOZ_ASSERT(scope_ == aScope, "Cannot change scope after it has been initialized");
+        scope_ = aScope;
+    }
+
+    size_t Size() const { return bufList_.Size(); }
+
+    const Iterator Start() const { return bufList_.Iter(); }
+
+    bool Advance(Iterator& iter, size_t distance) const {
+        return iter.AdvanceAcrossSegments(bufList_, distance);
+    }
+
+    bool ReadBytes(Iterator& iter, char* buffer, size_t size) const {
+        return bufList_.ReadBytes(iter, buffer, size);
+    }
+
+    // Append new data to the end of the buffer.
+    bool AppendBytes(const char* data, size_t size) {
+        MOZ_ASSERT(scope_ != JS::StructuredCloneScope::Unassigned);
+        return bufList_.WriteBytes(data, size);
+    }
 
-    using BufferList::BufferList;
+    // Update data stored within the existing buffer. There must be at least
+    // 'size' bytes between the position of 'iter' and the end of the buffer.
+    bool UpdateBytes(Iterator& iter, const char* data, size_t size) const {
+        MOZ_ASSERT(scope_ != JS::StructuredCloneScope::Unassigned);
+        while (size > 0) {
+            size_t remaining = iter.RemainingInSegment();
+            size_t nbytes = std::min(remaining, size);
+            memcpy(iter.Data(), data, nbytes);
+            data += nbytes;
+            size -= nbytes;
+            iter.Advance(bufList_, nbytes);
+        }
+        return true;
+    }
+
+    void Clear() {
+        discardTransferables();
+        bufList_.Clear();
+    }
+
+    // Return a new read-only JSStructuredCloneData that "borrows" the contents
+    // of |this|. Its lifetime should not exceed the donor's. This is only
+    // allowed for DifferentProcess clones, so finalization of the borrowing
+    // clone will do nothing.
+    JSStructuredCloneData Borrow(Iterator& iter, size_t size, bool* success) const
+    {
+        MOZ_ASSERT(scope_ == JS::StructuredCloneScope::DifferentProcess);
+        return JSStructuredCloneData(bufList_.Borrow<js::SystemAllocPolicy>(iter, size, success),
+                                     scope_);
+    }
+
+    // Iterate over all contained data, one BufferList segment's worth at a
+    // time, and invoke the given FunctionToApply with the data pointer and
+    // size. The function should return a bool value, and this loop will exit
+    // with false if the function ever returns false.
+    template <typename FunctionToApply>
+    bool ForEachDataChunk(FunctionToApply&& function) const {
+        Iterator iter = bufList_.Iter();
+        while (!iter.Done()) {
+            if (!function(iter.Data(), iter.RemainingInSegment()))
+                return false;
+            iter.Advance(bufList_, iter.RemainingInSegment());
+        }
+        return true;
+    }
+
+    // Append the entire contents of other's bufList_ to our own.
+    bool Append(const JSStructuredCloneData& other) {
+        MOZ_ASSERT(scope_ == other.scope_);
+        return other.ForEachDataChunk([&](const char* data, size_t size) {
+            return AppendBytes(data, size);
+        });
+    }
+
+    void discardTransferables();
 };
 
 /** Note: if the *data contains transferable objects, it can be read only once. */
@@ -254,18 +390,29 @@ JS_PUBLIC_API(bool)
 JS_StructuredClone(JSContext* cx, JS::HandleValue v, JS::MutableHandleValue vp,
                    const JSStructuredCloneCallbacks* optionalCallbacks, void* closure);
 
-/** RAII sugar for JS_WriteStructuredClone. */
+/**
+ * The C-style API calls to read and write structured clones are fragile --
+ * they rely on the caller to properly handle ownership of the clone data, and
+ * the handling of the input data as well as the interpretation of the contents
+ * of the clone buffer are dependent on the callbacks passed in. If you
+ * serialize and deserialize with different callbacks, the results are
+ * questionable.
+ *
+ * JSAutoStructuredCloneBuffer wraps things up in an RAII class for data
+ * management, and uses the same callbacks for both writing and reading
+ * (serializing and deserializing).
+ */
 class JS_PUBLIC_API(JSAutoStructuredCloneBuffer) {
     const JS::StructuredCloneScope scope_;
     JSStructuredCloneData data_;
     uint32_t version_;
 
   public:
-    JSAutoStructuredCloneBuffer(JS::StructuredCloneScope scope,
+    JSAutoStructuredCloneBuffer(JS::StructuredCloneScope aScope,
                                 const JSStructuredCloneCallbacks* callbacks, void* closure)
-        : scope_(scope), version_(JS_STRUCTURED_CLONE_VERSION)
+        : scope_(aScope), data_(aScope), version_(JS_STRUCTURED_CLONE_VERSION)
     {
-        data_.setOptionalCallbacks(callbacks, closure, OwnTransferablePolicy::NoTransferables);
+        data_.setCallbacks(callbacks, closure, OwnTransferablePolicy::NoTransferables);
     }
 
     JSAutoStructuredCloneBuffer(JSAutoStructuredCloneBuffer&& other);
@@ -276,11 +423,9 @@ class JS_PUBLIC_API(JSAutoStructuredCloneBuffer) {
     JSStructuredCloneData& data() { return data_; }
     bool empty() const { return !data_.Size(); }
 
-    void clear(const JSStructuredCloneCallbacks* optionalCallbacks=nullptr, void* closure=nullptr);
+    void clear();
 
-    /** Copy some memory. It will be automatically freed by the destructor. */
-    bool copy(const JSStructuredCloneData& data, uint32_t version=JS_STRUCTURED_CLONE_VERSION,
-              const JSStructuredCloneCallbacks* callbacks=nullptr, void* closure=nullptr);
+    JS::StructuredCloneScope scope() const { return scope_; }
 
     /**
      * Adopt some memory. It will be automatically freed by the destructor.
diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp
index 00637a7a5d..373b6c9edc 100644
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2088,7 +2088,7 @@ class CloneBufferObject : public NativeObject {
         Rooted<CloneBufferObject*> obj(cx, Create(cx));
         if (!obj)
             return nullptr;
-        auto data = js::MakeUnique<JSStructuredCloneData>();
+        auto data = js::MakeUnique<JSStructuredCloneData>(buffer->scope());
         if (!data) {
             ReportOutOfMemory(cx);
             return nullptr;
@@ -2141,8 +2141,11 @@ class CloneBufferObject : public NativeObject {
             return false;
         size_t nbytes = JS_GetStringLength(args[0].toString());
         MOZ_ASSERT(nbytes % sizeof(uint64_t) == 0);
-        auto buf = js::MakeUnique<JSStructuredCloneData>(nbytes, nbytes, nbytes);
-        js_memcpy(buf->Start(), str, nbytes);
+        auto buf = js::MakeUnique<JSStructuredCloneData>(JS::StructuredCloneScope::DifferentProcess);
+        if (!buf->AppendBytes(str, nbytes)) {
+            ReportOutOfMemory(cx);
+            return false;
+        }
         JS_free(cx, str);
         obj->setData(buf.release());
 
@@ -2186,7 +2189,7 @@ class CloneBufferObject : public NativeObject {
             ReportOutOfMemory(cx);
             return false;
         }
-        auto iter = obj->data()->Iter();
+        auto iter = obj->data()->Start();
         obj->data()->ReadBytes(iter, buffer.get(), size);
         JSString* str = JS_NewStringCopyN(cx, buffer.get(), size);
         if (!str)
@@ -2244,6 +2247,8 @@ ParseCloneScope(JSContext* cx, HandleString str)
         scope.emplace(JS::StructuredCloneScope::SameProcessDifferentThread);
     else if (strcmp(scopeStr.ptr(), "DifferentProcess") == 0)
         scope.emplace(JS::StructuredCloneScope::DifferentProcess);
+    else if (strcmp(scopeStr.ptr(), "DifferentProcessForIndexedDB") == 0)
+        scope.emplace(JS::StructuredCloneScope::DifferentProcessForIndexedDB);
 
     return scope;
 }
@@ -4370,19 +4375,22 @@ JS_FN_HELP("rejectPromise", RejectPromise, 2, 0,
 "  clone buffer object. 'policy' may be an options hash. Valid keys:\n"
 "    'SharedArrayBuffer' - either 'allow' (the default) or 'deny'\n"
 "    to specify whether SharedArrayBuffers may be serialized.\n"
-"\n"
-"    'scope' - SameProcessSameThread, SameProcessDifferentThread, or\n"
-"    DifferentProcess. Determines how some values will be serialized.\n"
-"    Clone buffers may only be deserialized with a compatible scope."),
+"    'scope' - SameProcessSameThread, SameProcessDifferentThread,\n"
+"      DifferentProcess, or DifferentProcessForIndexedDB. Determines how some\n"
+"      values will be serialized. Clone buffers may only be deserialized with a\n"
+"      compatible scope. NOTE - For DifferentProcess/DifferentProcessForIndexedDB,\n"
+"      must also set SharedArrayBuffer:'deny' if data contains any shared memory\n"
+"      object."),
 
     JS_FN_HELP("deserialize", Deserialize, 1, 0,
 "deserialize(clonebuffer[, opts])",
 "  Deserialize data generated by serialize. 'opts' is an options hash with one\n"
 "  recognized key 'scope', which limits the clone buffers that are considered\n"
 "  valid. Allowed values: 'SameProcessSameThread', 'SameProcessDifferentThread',\n"
-"  and 'DifferentProcess'. So for example, a DifferentProcess clone buffer\n"
-"  may be deserialized in any scope, but a SameProcessSameThread clone buffer\n"
-"  cannot be deserialized in a DifferentProcess scope."),
+"  'DifferentProcess', and 'DifferentProcessForIndexedDB'. So for example, a\n"
+"  DifferentProcessForIndexedDB clone buffer may be deserialized in any scope, but\n"
+"  a SameProcessSameThread clone buffer cannot be deserialized in a\n"
+"  DifferentProcess scope."),
 
     JS_FN_HELP("detachArrayBuffer", DetachArrayBuffer, 1, 0,
 "detachArrayBuffer(buffer)",
diff --git a/js/src/tests/js1_8_5/extensions/clone-errors.js b/js/src/tests/js1_8_5/extensions/clone-errors.js
index f65578a06b..d2ccea2e8c 100644
--- a/js/src/tests/js1_8_5/extensions/clone-errors.js
+++ b/js/src/tests/js1_8_5/extensions/clone-errors.js
@@ -25,6 +25,7 @@ check({get x() { throw new Error("fail"); }});
 // Mismatched scopes.
 for (let [write_scope, read_scope] of [['SameProcessSameThread', 'SameProcessDifferentThread'],
                                        ['SameProcessSameThread', 'DifferentProcess'],
+                                       ['SameProcessDifferentThread', 'DifferentProcessForIndexedDB'],
                                        ['SameProcessDifferentThread', 'DifferentProcess']])
 {
   var ab = new ArrayBuffer(12);
diff --git a/js/src/tests/js1_8_5/extensions/clone-transferables.js b/js/src/tests/js1_8_5/extensions/clone-transferables.js
index 673684b954..9aad27208b 100644
--- a/js/src/tests/js1_8_5/extensions/clone-transferables.js
+++ b/js/src/tests/js1_8_5/extensions/clone-transferables.js
@@ -3,11 +3,15 @@
 // http://creativecommons.org/licenses/publicdomain/
 
 function* buffer_options() {
-  for (var scope of ["SameProcessSameThread", "SameProcessDifferentThread", "DifferentProcess"]) {
-    for (var size of [0, 8, 16, 200, 1000, 4096, 8192, 65536]) {
-      yield { scope, size };
+    for (var scope of ["SameProcessSameThread",
+                       "SameProcessDifferentThread",
+                       "DifferentProcess",
+                       "DifferentProcessForIndexedDB"])
+    {
+        for (var size of [0, 8, 16, 200, 1000, 4096, 8192, 65536]) {
+            yield { scope, size };
+        }
     }
-  }
 }
 
 
diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp
index 3a062c3b8b..42e9090004 100644
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -160,16 +160,16 @@ template<typename T, typename AllocPolicy>
 struct BufferIterator {
     typedef mozilla::BufferList<AllocPolicy> BufferList;
 
-    explicit BufferIterator(BufferList& buffer)
+    explicit BufferIterator(const BufferList& buffer)
         : mBuffer(buffer)
         , mIter(buffer.Iter())
     {
         JS_STATIC_ASSERT(8 % sizeof(T) == 0);
     }
 
-    BufferIterator(const BufferIterator& other)
-        : mBuffer(other.mBuffer)
-        , mIter(other.mIter)
+    explicit BufferIterator(const JSStructuredCloneData& data)
+      : mBuffer(data.bufList_)
+      , mIter(data.Start())
     {
     }
 
@@ -228,17 +228,26 @@ struct BufferIterator {
         return mIter.HasRoomFor(sizeof(T));
     }
 
-    BufferList& mBuffer;
+    const BufferList& mBuffer;
     typename BufferList::IterImpl mIter;
 };
 
+// SCOutput provides an interface to write raw data -- eg uint64_ts, doubles,
+// arrays of bytes -- into a structured clone data output stream. It also knows
+// how to free any transferable data within that stream.
+//
+// Note that it contains a full JSStructuredCloneData object, which holds the
+// callbacks necessary to read/write/transfer/free the data. For the purpose of
+// this class, only the freeTransfer callback is relevant; the rest of the callbacks
+// are used by the higher-level JSStructuredCloneWriter interface.
 struct SCOutput {
   public:
-    using Iter = BufferIterator<uint64_t, TempAllocPolicy>;
+    using Iter = BufferIterator<uint64_t, SystemAllocPolicy>;
 
-    explicit SCOutput(JSContext* cx);
+    SCOutput(JSContext* cx, JS::StructuredCloneScope scope);
 
     JSContext* context() const { return cx; }
+    JS::StructuredCloneScope scope() const { return buf.scope(); }
 
     bool write(uint64_t u);
     bool writePair(uint32_t tag, uint32_t data);
@@ -251,22 +260,25 @@ struct SCOutput {
     template <class T>
     bool writeArray(const T* p, size_t nbytes);
 
-    bool extractBuffer(JSStructuredCloneData* data);
-    void discardTransferables(const JSStructuredCloneCallbacks* cb, void* cbClosure);
+    void setCallbacks(const JSStructuredCloneCallbacks* callbacks,
+                      void* closure,
+                      OwnTransferablePolicy policy)
+    {
+        buf.setCallbacks(callbacks, closure, policy);
+    }
+    void extractBuffer(JSStructuredCloneData* data) { *data = Move(buf); }
+    void discardTransferables();
 
     uint64_t tell() const { return buf.Size(); }
     uint64_t count() const { return buf.Size() / sizeof(uint64_t); }
-    Iter iter() {
-        return BufferIterator<uint64_t, TempAllocPolicy>(buf);
-    }
+    Iter iter() { return Iter(buf); }
 
     size_t offset(Iter dest) {
         return dest - iter();
     }
 
-  private:
     JSContext* cx;
-    mozilla::BufferList<TempAllocPolicy> buf;
+    JSStructuredCloneData buf;
 };
 
 class SCInput {
@@ -356,13 +368,6 @@ struct JSStructuredCloneReader {
     // be valid cross-process.)
     JS::StructuredCloneScope allowedScope;
 
-    // The scope the buffer was generated for (what sort of buffer it is.) The
-    // scope is not just a permissions thing; it also affects the storage
-    // format (eg a Transferred ArrayBuffer can be stored as a pointer for
-    // SameProcessSameThread but must have its contents in the clone buffer for
-    // DifferentProcess.)
-    JS::StructuredCloneScope storedScope;
-
     // Stack of objects with properties remaining to be read.
     AutoValueVector objs;
 
@@ -386,13 +391,15 @@ struct JSStructuredCloneWriter {
                                      const JSStructuredCloneCallbacks* cb,
                                      void* cbClosure,
                                      const Value& tVal)
-        : out(cx), scope(scope), objs(out.context()),
+        : out(cx, scope), objs(out.context()),
           counts(out.context()), entries(out.context()),
-          memory(out.context()), callbacks(cb),
-          closure(cbClosure), transferable(out.context(), tVal),
+          memory(out.context()),
+          transferable(out.context(), tVal),
           transferableObjects(out.context(), GCHashSet<JSObject*>(cx)),
           cloneDataPolicy(cloneDataPolicy)
-    {}
+    {
+        out.setCallbacks(cb, cbClosure, OwnTransferablePolicy::NoTransferables);
+    }
 
     ~JSStructuredCloneWriter();
 
@@ -408,17 +415,10 @@ struct JSStructuredCloneWriter {
 
     SCOutput& output() { return out; }
 
-    bool extractBuffer(JSStructuredCloneData* data) {
-        bool success = out.extractBuffer(data);
-        if (success) {
-            data->setOptionalCallbacks(callbacks, closure,
-                                       OwnTransferablePolicy::OwnsTransferablesIfAny);
-        }
-        return success;
+    void extractBuffer(JSStructuredCloneData* newData) {
+        out.extractBuffer(newData);
     }
 
-    JS::StructuredCloneScope cloneScope() const { return scope; }
-
   private:
     JSStructuredCloneWriter() = delete;
     JSStructuredCloneWriter(const JSStructuredCloneWriter&) = delete;
@@ -449,9 +449,6 @@ struct JSStructuredCloneWriter {
 
     SCOutput out;
 
-    // The (address space, thread) scope within which this clone is valid.
-    JS::StructuredCloneScope scope;
-
     // Vector of objects with properties remaining to be written.
     //
     // NB: These can span multiple compartments, so the compartment must be
@@ -477,12 +474,6 @@ struct JSStructuredCloneWriter {
                                   SystemAllocPolicy>;
     Rooted<CloneMemory> memory;
 
-    // The user defined callbacks that will be used for cloning.
-    const JSStructuredCloneCallbacks* callbacks;
-
-    // Any value passed to JS_WriteStructuredClone.
-    void* closure;
-
     // Set of transferable objects
     RootedValue transferable;
     Rooted<GCHashSet<JSObject*>> transferableObjects;
@@ -542,7 +533,12 @@ WriteStructuredClone(JSContext* cx, HandleValue v, JSStructuredCloneData* bufp,
                      const Value& transferable)
 {
     JSStructuredCloneWriter w(cx, scope, cloneDataPolicy, cb, cbClosure, transferable);
-    return w.init() && w.write(v) && w.extractBuffer(bufp);
+    if (!w.init())
+        return false;
+    if (!w.write(v))
+        return false;
+    w.extractBuffer(bufp);
+    return true;
 }
 
 bool
@@ -555,91 +551,15 @@ ReadStructuredClone(JSContext* cx, JSStructuredCloneData& data,
     return r.read(vp);
 }
 
-// If the given buffer contains Transferables, free them. Note that custom
-// Transferables will use the JSStructuredCloneCallbacks::freeTransfer() to
-// delete their transferables.
-template<typename AllocPolicy>
-static void
-DiscardTransferables(mozilla::BufferList<AllocPolicy>& buffer,
-                     const JSStructuredCloneCallbacks* cb, void* cbClosure)
-{
-    auto point = BufferIterator<uint64_t, AllocPolicy>(buffer);
-    if (point.done())
-        return; // Empty buffer
-
-    uint32_t tag, data;
-    MOZ_RELEASE_ASSERT(point.canPeek());
-    SCInput::getPair(point.peek(), &tag, &data);
-    point.next();
-
-    if (tag == SCTAG_HEADER) {
-        if (point.done())
-            return;
-
-        MOZ_RELEASE_ASSERT(point.canPeek());
-        SCInput::getPair(point.peek(), &tag, &data);
-        point.next();
-    }
-
-    if (tag != SCTAG_TRANSFER_MAP_HEADER)
-        return;
-
-    if (TransferableMapHeader(data) == SCTAG_TM_TRANSFERRED)
-        return;
-
-    // freeTransfer should not GC
-    JS::AutoSuppressGCAnalysis nogc;
-
-    if (point.done())
-        return;
-
-    uint64_t numTransferables = NativeEndian::swapFromLittleEndian(point.peek());
-    point.next();
-    while (numTransferables--) {
-        if (!point.canPeek())
-            return;
-
-        uint32_t ownership;
-        SCInput::getPair(point.peek(), &tag, &ownership);
-        point.next();
-        MOZ_ASSERT(tag >= SCTAG_TRANSFER_MAP_PENDING_ENTRY);
-        if (!point.canPeek())
-            return;
-
-        void* content;
-        SCInput::getPtr(point.peek(), &content);
-        point.next();
-        if (!point.canPeek())
-            return;
-
-        uint64_t extraData = NativeEndian::swapFromLittleEndian(point.peek());
-        point.next();
-
-        if (ownership < JS::SCTAG_TMO_FIRST_OWNED)
-            continue;
-
-        if (ownership == JS::SCTAG_TMO_ALLOC_DATA) {
-            js_free(content);
-        } else if (ownership == JS::SCTAG_TMO_MAPPED_DATA) {
-            JS_ReleaseMappedArrayBufferContents(content, extraData);
-        } else if (cb && cb->freeTransfer) {
-            cb->freeTransfer(tag, JS::TransferableOwnership(ownership), content, extraData, cbClosure);
-        } else {
-            MOZ_ASSERT(false, "unknown ownership");
-        }
-    }
-}
-
 static bool
 StructuredCloneHasTransferObjects(const JSStructuredCloneData& data)
 {
-    auto iter = data.Iter();
-
     if (data.Size() < sizeof(uint64_t))
         return false;
 
     uint64_t u;
-    data.ReadBytes(iter, reinterpret_cast<char*>(&u), sizeof(u));
+    BufferIterator<uint64_t, SystemAllocPolicy> iter(data);
+    MOZ_ALWAYS_TRUE(iter.readBytes(reinterpret_cast<char*>(&u), sizeof(u)));
     uint32_t tag = uint32_t(u >> 32);
     return (tag == SCTAG_TRANSFER_MAP_HEADER);
 }
@@ -650,7 +570,7 @@ SCInput::SCInput(JSContext* cx, JSStructuredCloneData& data)
     : cx(cx), point(data)
 {
 
-    static_assert(JSStructuredCloneData::kSegmentAlignment % 8 == 0,
+    static_assert(JSStructuredCloneData::BufferList::kSegmentAlignment % 8 == 0,
                   "structured clone buffer reads should be aligned");
     MOZ_ASSERT(data.Size() % 8 == 0);
 }
@@ -812,9 +732,8 @@ SCInput::readPtr(void** p)
     return true;
 }
 
-SCOutput::SCOutput(JSContext* cx)
-    : cx(cx)
-    , buf(0, 0, 4096, cx)
+SCOutput::SCOutput(JSContext* cx, JS::StructuredCloneScope scope)
+  : cx(cx), buf(scope)
 {
 }
 
@@ -822,7 +741,11 @@ bool
 SCOutput::write(uint64_t u)
 {
     uint64_t v = NativeEndian::swapToLittleEndian(u);
-    return buf.WriteBytes(reinterpret_cast<char*>(&v), sizeof(u));
+    if (!buf.AppendBytes(reinterpret_cast<char*>(&v), sizeof(u))) {
+        ReportOutOfMemory(context());
+        return false;
+    }
+    return true;
 }
 
 bool
@@ -883,7 +806,7 @@ SCOutput::writeArray(const T* p, size_t nelems)
 
     for (size_t i = 0; i < nelems; i++) {
         T value = swapToLittleEndian(p[i]);
-        if (!buf.WriteBytes(reinterpret_cast<char*>(&value), sizeof(value)))
+        if (!buf.AppendBytes(reinterpret_cast<char*>(&value), sizeof(value)))
             return false;
     }
 
@@ -892,7 +815,7 @@ SCOutput::writeArray(const T* p, size_t nelems)
     size_t padbytes = sizeof(uint64_t) * nwords - sizeof(T) * nelems;
     char zero = 0;
     for (size_t i = 0; i < padbytes; i++) {
-        if (!buf.WriteBytes(&zero, sizeof(zero)))
+        if (!buf.AppendBytes(&zero, sizeof(zero)))
             return false;
     }
 
@@ -927,34 +850,101 @@ SCOutput::writePtr(const void* p)
     return write(reinterpret_cast<uint64_t>(p));
 }
 
-bool
-SCOutput::extractBuffer(JSStructuredCloneData* data)
-{
-    bool success;
-    mozilla::BufferList<SystemAllocPolicy> out =
-        buf.MoveFallible<SystemAllocPolicy>(&success);
-    if (!success) {
-        ReportOutOfMemory(cx);
-        return false;
-    }
-    *data = JSStructuredCloneData(Move(out));
-    return true;
-}
-
 void
-SCOutput::discardTransferables(const JSStructuredCloneCallbacks* cb, void* cbClosure)
+SCOutput::discardTransferables()
 {
-    DiscardTransferables(buf, cb, cbClosure);
+    buf.discardTransferables();
 }
 
 } /* namespace js */
 
-JSStructuredCloneData::~JSStructuredCloneData()
+
+// If the buffer contains Transferables, free them. Note that custom
+// Transferables will use the JSStructuredCloneCallbacks::freeTransfer() to
+// delete their transferables.
+void
+JSStructuredCloneData::discardTransferables()
 {
     if (!Size())
         return;
-    if (ownTransferables_ == OwnTransferablePolicy::OwnsTransferablesIfAny)
-        DiscardTransferables(*this, callbacks_, closure_);
+
+    if (ownTransferables_ != OwnTransferablePolicy::OwnsTransferablesIfAny)
+        return;
+
+    // DifferentProcess clones cannot contain pointers, so nothing needs to be
+    // released.
+    if (scope_ == JS::StructuredCloneScope::DifferentProcess)
+        return;
+
+    FreeTransferStructuredCloneOp freeTransfer = nullptr;
+    if (callbacks_)
+        freeTransfer = callbacks_->freeTransfer;
+
+    auto point = BufferIterator<uint64_t, SystemAllocPolicy>(*this);
+    if (point.done())
+        return; // Empty buffer
+
+    uint32_t tag, data;
+    MOZ_RELEASE_ASSERT(point.canPeek());
+    SCInput::getPair(point.peek(), &tag, &data);
+    point.next();
+
+    if (tag == SCTAG_HEADER) {
+        if (point.done())
+            return;
+
+        MOZ_RELEASE_ASSERT(point.canPeek());
+        SCInput::getPair(point.peek(), &tag, &data);
+        point.next();
+    }
+
+    if (tag != SCTAG_TRANSFER_MAP_HEADER)
+        return;
+
+    if (TransferableMapHeader(data) == SCTAG_TM_TRANSFERRED)
+        return;
+
+    // freeTransfer should not GC
+    JS::AutoSuppressGCAnalysis nogc;
+
+    if (point.done())
+        return;
+
+    uint64_t numTransferables = NativeEndian::swapFromLittleEndian(point.peek());
+    point.next();
+    while (numTransferables--) {
+        if (!point.canPeek())
+            return;
+
+        uint32_t ownership;
+        SCInput::getPair(point.peek(), &tag, &ownership);
+        point.next();
+        MOZ_ASSERT(tag >= SCTAG_TRANSFER_MAP_PENDING_ENTRY);
+        if (!point.canPeek())
+            return;
+
+        void* content;
+        SCInput::getPtr(point.peek(), &content);
+        point.next();
+        if (!point.canPeek())
+            return;
+
+        uint64_t extraData = NativeEndian::swapFromLittleEndian(point.peek());
+        point.next();
+
+        if (ownership < JS::SCTAG_TMO_FIRST_OWNED)
+            continue;
+
+        if (ownership == JS::SCTAG_TMO_ALLOC_DATA) {
+            js_free(content);
+        } else if (ownership == JS::SCTAG_TMO_MAPPED_DATA) {
+            JS_ReleaseMappedArrayBufferContents(content, extraData);
+        } else if (freeTransfer) {
+            freeTransfer(tag, JS::TransferableOwnership(ownership), content, extraData, closure_);
+        } else {
+            MOZ_ASSERT(false, "unknown ownership");
+        }
+    }
 }
 
 JS_STATIC_ASSERT(JSString::MAX_LENGTH < UINT32_MAX);
@@ -962,9 +952,8 @@ JS_STATIC_ASSERT(JSString::MAX_LENGTH < UINT32_MAX);
 JSStructuredCloneWriter::~JSStructuredCloneWriter()
 {
     // Free any transferable data left lying around in the buffer
-    if (out.count()) {
-        out.discardTransferables(callbacks, closure);
-    }
+    if (out.count())
+        out.discardTransferables();
 }
 
 bool
@@ -1038,7 +1027,7 @@ JSStructuredCloneWriter::parseTransferable()
 bool
 JSStructuredCloneWriter::reportDataCloneError(uint32_t errorId)
 {
-    ReportDataCloneError(context(), callbacks, errorId);
+    ReportDataCloneError(context(), out.buf.callbacks_, errorId);
     return false;
 }
 
@@ -1454,8 +1443,8 @@ JSStructuredCloneWriter::startWrite(HandleValue v)
             return traverseSavedFrame(obj);
         }
 
-        if (callbacks && callbacks->write)
-            return callbacks->write(context(), this, obj, closure);
+        if (out.buf.callbacks_ && out.buf.callbacks_->write)
+            return out.buf.callbacks_->write(context(), this, obj, out.buf.closure_);
         /* else fall through */
     }
 
@@ -1465,7 +1454,7 @@ JSStructuredCloneWriter::startWrite(HandleValue v)
 bool
 JSStructuredCloneWriter::writeHeader()
 {
-    return out.writePair(SCTAG_HEADER, (uint32_t)scope);
+    return out.writePair(SCTAG_HEADER, (uint32_t)output().scope());
 }
 
 bool
@@ -1523,6 +1512,7 @@ JSStructuredCloneWriter::transferOwnership()
 
     JSContext* cx = context();
     RootedObject obj(cx);
+    JS::StructuredCloneScope scope = output().scope();
     for (auto tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
         obj = tr.front();
 
@@ -1555,7 +1545,9 @@ JSStructuredCloneWriter::transferOwnership()
                 return false;
             }
 
-            if (scope == JS::StructuredCloneScope::DifferentProcess) {
+            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
@@ -1592,9 +1584,9 @@ JSStructuredCloneWriter::transferOwnership()
                 extraData = nbytes;
             }
         } else {
-            if (!callbacks || !callbacks->writeTransfer)
+            if (!out.buf.callbacks_ || !out.buf.callbacks_->writeTransfer)
                 return reportDataCloneError(JS_SCERR_TRANSFERABLE);
-            if (!callbacks->writeTransfer(cx, obj, closure, &tag, &ownership, &content, &extraData))
+            if (!out.buf.callbacks_->writeTransfer(cx, obj, out.buf.closure_, &tag, &ownership, &content, &extraData))
                 return false;
             MOZ_ASSERT(tag > SCTAG_TRANSFER_MAP_PENDING_ENTRY);
         }
@@ -2187,25 +2179,33 @@ JSStructuredCloneReader::readHeader()
     if (!in.getPair(&tag, &data))
         return in.reportTruncated();
 
-    if (tag != SCTAG_HEADER) {
+    JS::StructuredCloneScope storedScope;
+    if (tag == SCTAG_HEADER) {
+        MOZ_ALWAYS_TRUE(in.readPair(&tag, &data));
+        storedScope = JS::StructuredCloneScope(data);
+    } else {
         // Old structured clone buffer. We must have read it from disk.
-        storedScope = JS::StructuredCloneScope::DifferentProcess;
-        return true;
+        storedScope = JS::StructuredCloneScope::DifferentProcessForIndexedDB;
     }
 
-    MOZ_ALWAYS_TRUE(in.readPair(&tag, &data));
-    storedScope = JS::StructuredCloneScope(data);
-
-    if (data != uint32_t(JS::StructuredCloneScope::SameProcessSameThread) &&
-        data != uint32_t(JS::StructuredCloneScope::SameProcessDifferentThread) &&
-        data != uint32_t(JS::StructuredCloneScope::DifferentProcess))
+    if (storedScope < JS::StructuredCloneScope::SameProcessSameThread ||
+        storedScope > JS::StructuredCloneScope::DifferentProcessForIndexedDB)
     {
         JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA,
                                   "invalid structured clone scope");
         return false;
     }
+
+    if (allowedScope == JS::StructuredCloneScope::DifferentProcessForIndexedDB) {
+        // Bug 1434308 and bug 1458320 - the scopes stored in old IndexedDB
+        // clones are incorrect. Treat them as if they were DifferentProcess.
+        allowedScope = JS::StructuredCloneScope::DifferentProcess;
+        return true;
+    }
+
     if (storedScope < allowedScope) {
-        JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA,
+        JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr,
+                                  JSMSG_SC_BAD_SERIALIZED_DATA,
                                   "incompatible structured clone scope");
         return false;
     }
@@ -2249,10 +2249,14 @@ JSStructuredCloneReader::readTransferMap()
             return false;
 
         if (tag == SCTAG_TRANSFER_MAP_ARRAY_BUFFER) {
-            if (storedScope == JS::StructuredCloneScope::DifferentProcess) {
+            if (allowedScope == JS::StructuredCloneScope::DifferentProcess ||
+                allowedScope == JS::StructuredCloneScope::DifferentProcessForIndexedDB)
+            {
                 // Transferred ArrayBuffers in a DifferentProcess clone buffer
-                // are treated as if they weren't Transferred at all.
-                continue;
+                // are treated as if they weren't Transferred at all. We should
+                // only see SCTAG_TRANSFER_MAP_STORED_ARRAY_BUFFER.
+                ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
+                return false;
             }
 
             size_t nbytes = extraData;
@@ -2586,7 +2590,7 @@ JS_StructuredClone(JSContext* cx, HandleValue value, MutableHandleValue vp,
 }
 
 JSAutoStructuredCloneBuffer::JSAutoStructuredCloneBuffer(JSAutoStructuredCloneBuffer&& other)
-    : scope_(other.scope_)
+  : scope_(other.scope()), data_(other.scope())
 {
     data_.ownTransferables_ = other.data_.ownTransferables_;
     other.steal(&data_, &version_, &data_.callbacks_, &data_.closure_);
@@ -2604,45 +2608,14 @@ JSAutoStructuredCloneBuffer::operator=(JSAutoStructuredCloneBuffer&& other)
 }
 
 void
-JSAutoStructuredCloneBuffer::clear(const JSStructuredCloneCallbacks* optionalCallbacks,
-                                   void* optionalClosure)
+JSAutoStructuredCloneBuffer::clear()
 {
-    if (!data_.Size())
-        return;
-
-    const JSStructuredCloneCallbacks* callbacks =
-        optionalCallbacks ?  optionalCallbacks : data_.callbacks_;
-    void* closure = optionalClosure ?  optionalClosure : data_.closure_;
-
-    if (data_.ownTransferables_ == OwnTransferablePolicy::OwnsTransferablesIfAny)
-        DiscardTransferables(data_, callbacks, closure);
+    data_.discardTransferables();
     data_.ownTransferables_ = OwnTransferablePolicy::NoTransferables;
     data_.Clear();
     version_ = 0;
 }
 
-bool
-JSAutoStructuredCloneBuffer::copy(const JSStructuredCloneData& srcData, uint32_t version,
-                                  const JSStructuredCloneCallbacks* callbacks,
-                                  void* closure)
-{
-    // transferable objects cannot be copied
-    if (StructuredCloneHasTransferObjects(srcData))
-        return false;
-
-    clear();
-
-    auto iter = srcData.Iter();
-    while (!iter.Done()) {
-            data_.WriteBytes(iter.Data(), iter.RemainingInSegment());
-            iter.Advance(srcData, iter.RemainingInSegment());
-    }
-
-    version_ = version;
-    data_.setOptionalCallbacks(callbacks, closure, OwnTransferablePolicy::NoTransferables);
-    return true;
-}
-
 void
 JSAutoStructuredCloneBuffer::adopt(JSStructuredCloneData&& data, uint32_t version,
                                    const JSStructuredCloneCallbacks* callbacks,
@@ -2651,7 +2624,7 @@ JSAutoStructuredCloneBuffer::adopt(JSStructuredCloneData&& data, uint32_t versio
     clear();
     data_ = Move(data);
     version_ = version;
-    data_.setOptionalCallbacks(callbacks, closure, OwnTransferablePolicy::OwnsTransferablesIfAny);
+    data_.setCallbacks(callbacks, closure, OwnTransferablePolicy::OwnsTransferablesIfAny);
 }
 
 void
@@ -2668,7 +2641,7 @@ JSAutoStructuredCloneBuffer::steal(JSStructuredCloneData* data, uint32_t* versio
     *data = Move(data_);
 
     version_ = 0;
-    data_.setOptionalCallbacks(nullptr, nullptr, OwnTransferablePolicy::NoTransferables);
+    data_.setCallbacks(nullptr, nullptr, OwnTransferablePolicy::NoTransferables);
 }
 
 bool
@@ -2782,5 +2755,5 @@ JS_ObjectNotWritten(JSStructuredCloneWriter* w, HandleObject obj)
 JS_PUBLIC_API(JS::StructuredCloneScope)
 JS_GetStructuredCloneScope(JSStructuredCloneWriter* w)
 {
-    return w->cloneScope();
+    return w->output().scope();
 }