Update jsg::Serializer/Deserializer to use JsValue

src/workerd/api/actor-state.c++

@@ -952,7 +952,7 @@ jsg::JsValue deserializeV8Value(jsg::Lock& js,
 
       jsg::Deserializer deserializer(js, buf, nullptr, nullptr, options);
 
-      return jsg::JsValue(deserializer.readValue(js));
+      return deserializer.readValue(js);
     }, [&](jsg::Value&& exception) mutable -> jsg::JsValue {
       // If we do hit a deserialization error, we log information that will be helpful in
       // understanding the problem but that won't leak too much about the customer's data. We

src/workerd/api/global-scope.c++

@@ -632,13 +632,15 @@ jsg::JsValue ServiceWorkerGlobalScope::structuredClone(
     jsg::Lock& js,
     jsg::JsValue value,
     jsg::Optional<StructuredCloneOptions> maybeOptions) {
-  kj::Maybe<kj::ArrayPtr<jsg::Value>> transfers;
   KJ_IF_MAYBE(options, maybeOptions) {
-    transfers = options->transfer.map([&](kj::Array<jsg::Value>& transfer) {
-      return transfer.asPtr();
-    });
+    KJ_IF_MAYBE(transfer, options->transfer) {
+      auto transfers = KJ_MAP(i, *transfer) {
+        return i.getHandle(js);
+      };
+      return value.structuredClone(js, kj::mv(transfers));
+    }
   }
-  return jsg::JsValue(jsg::structuredClone(js, value, transfers));
+  return value.structuredClone(js);
 }
 
 TimeoutId::NumberType ServiceWorkerGlobalScope::setTimeoutInternal(

src/workerd/api/global-scope.h

@@ -316,7 +316,7 @@ class ServiceWorkerGlobalScope: public WorkerGlobalScope {
   void queueMicrotask(jsg::Lock& js, v8::Local<v8::Function> task);
 
   struct StructuredCloneOptions {
-    jsg::Optional<kj::Array<jsg::Value>> transfer;
+    jsg::Optional<kj::Array<jsg::JsRef<jsg::JsValue>>> transfer;
     JSG_STRUCT(transfer);
     JSG_STRUCT_TS_OVERRIDE(StructuredSerializeOptions);
   };

src/workerd/api/http.c++

@@ -2000,7 +2000,7 @@ jsg::Promise<Fetcher::QueueResult> Fetcher::queue(
           .version = 15,
           .omitHeader = false,
       });
-      serializer.write(js, b->getHandle(js));
+      serializer.write(js, jsg::JsValue(b->getHandle(js)));
       encodedMessages.add(IncomingQueueMessage{
           .id=kj::mv(msg.id),
           .timestamp=msg.timestamp,

src/workerd/api/node/diagnostics-channel.c++

@@ -27,7 +27,7 @@ void Channel::publish(jsg::Lock& js, jsg::Value message) {
     jsg::Serializer ser(js, jsg::Serializer::Options {
       .omitHeader = false,
     });
-    ser.write(js, message.getHandle(js));
+    ser.write(js, jsg::JsValue(message.getHandle(js)));
     auto tmp = ser.release();
     JSG_REQUIRE(tmp.sharedArrayBuffers.size() == 0 &&
                 tmp.transferedArrayBuffers.size() == 0, Error,

src/workerd/api/queue.c++

@@ -44,7 +44,7 @@ Serialized serializeV8(jsg::Lock& js, v8::Local<v8::Value> body) {
     .version = 15,
     .omitHeader = false,
   });
-  serializer.write(js, body);
+  serializer.write(js, jsg::JsValue(body));
   kj::Array<kj::byte> bytes = serializer.release().data;
   Serialized result;
   result.data = bytes;
@@ -277,7 +277,7 @@ jsg::Value deserialize(jsg::Lock& js, kj::Array<kj::byte> body, kj::Maybe<kj::St
   } else if (type == IncomingQueueMessage::ContentType::JSON) {
     return js.parseJson(body.asChars());
   } else if (type == IncomingQueueMessage::ContentType::V8) {
-    return js.v8Ref(jsg::Deserializer(js, body.asPtr()).readValue(js));
+    return js.v8Ref(v8::Local<v8::Value>(jsg::Deserializer(js, body.asPtr()).readValue(js)));
   } else {
     JSG_FAIL_REQUIRE(TypeError, kj::str("Unsupported queue message content type: ", type));
   }
@@ -298,7 +298,7 @@ jsg::Value deserialize(jsg::Lock& js, rpc::QueueMessage::Reader message) {
   } else if (type == IncomingQueueMessage::ContentType::JSON) {
     return js.parseJson(message.getData().asChars());
   } else if (type == IncomingQueueMessage::ContentType::V8) {
-    return js.v8Ref(jsg::Deserializer(js, message.getData()).readValue(js));
+    return js.v8Ref(v8::Local<v8::Value>(jsg::Deserializer(js, message.getData()).readValue(js)));
   } else {
     JSG_FAIL_REQUIRE(TypeError, kj::str("Unsupported queue message content type: ", type));
   }

src/workerd/api/web-socket.c++

@@ -645,7 +645,8 @@ kj::Maybe<kj::StringPtr> WebSocket::getExtensions() {
 }
 
 kj::Maybe<v8::Local<v8::Value>> WebSocket::deserializeAttachment(jsg::Lock& js) {
-  return serializedAttachment.map([&](kj::ArrayPtr<byte> attachment) {
+  return serializedAttachment.map([&](kj::ArrayPtr<byte> attachment)
+      -> v8::Local<v8::Value> {
     jsg::Deserializer deserializer(js, attachment, nullptr, nullptr,
         jsg::Deserializer::Options {
       .version = 15,
@@ -661,7 +662,7 @@ void WebSocket::serializeAttachment(jsg::Lock& js, v8::Local<v8::Value> attachme
     .version = 15,
     .omitHeader = false,
   });
-  serializer.write(js, attachment);
+  serializer.write(js, jsg::JsValue(attachment));
   auto released = serializer.release();
   JSG_REQUIRE(released.data.size() <= MAX_ATTACHMENT_SIZE, Error,
       "A WebSocket 'attachment' cannot be larger than ",  MAX_ATTACHMENT_SIZE, " bytes." \

src/workerd/jsg/jsvalue.c++

@@ -1,5 +1,6 @@
 #include "jsvalue.h"
 #include "buffersource.h"
+#include "ser.h"
 
 namespace workerd::jsg {
 
@@ -400,4 +401,9 @@ JsRef<JsValue> JsValue::addRef(Lock& js) {
   return JsRef<JsValue>(js, *this);
 }
 
+JsValue JsValue::structuredClone(Lock& js, kj::Maybe<kj::Array<JsValue>> maybeTransfers) {
+  return jsg::structuredClone(js, *this, kj::mv(maybeTransfers));
+}
+
+
 }  // namespace workerd::jsg

src/workerd/jsg/jsvalue.h

@@ -137,6 +137,9 @@ class JsValue final {
 
   JsRef<JsValue> addRef(Lock& js) KJ_WARN_UNUSED_RESULT;
 
+  JsValue structuredClone(Lock& js, kj::Maybe<kj::Array<JsValue>> maybeTransfers = nullptr)
+      KJ_WARN_UNUSED_RESULT;
+
   template <typename T>
   static kj::Maybe<T&> tryGetExternal(Lock& js, const JsValue& value) KJ_WARN_UNUSED_RESULT;
 

src/workerd/jsg/ser.c++

@@ -25,13 +25,14 @@ v8::Maybe<uint32_t> Serializer::GetSharedArrayBufferId(
     v8::Isolate *isolate,
     v8::Local<v8::SharedArrayBuffer> sab) {
   uint32_t n;
+  auto value = JsValue(sab);
   for (n = 0; n < sharedArrayBuffers.size(); n++) {
     // If the SharedArrayBuffer has already been added, return the existing ID for it.
-    if (sharedArrayBuffers[n].getHandle(isolate) == sab) {
+    if (sharedArrayBuffers[n] == value) {
       return v8::Just(n);
     }
   }
-  sharedArrayBuffers.add(jsg::V8Ref(isolate, sab));
+  sharedArrayBuffers.add(value);
   sharedBackingStores.add(sab->GetBackingStore());
   return v8::Just(n);
 }
@@ -55,23 +56,34 @@ Serializer::Released Serializer::release() {
   };
 }
 
-void Serializer::transfer(Lock& js, v8::Local<v8::ArrayBuffer> arrayBuffer) {
+void Serializer::transfer(Lock& js, const JsValue& value) {
   KJ_ASSERT(!released, "The data has already been released.");
+  // Currently we only allow transfer of ArrayBuffers
+  v8::Local<v8::ArrayBuffer> arrayBuffer;
+  if (value.isArrayBufferView()) {
+    auto view = v8::Local<v8::Value>(value).As<v8::ArrayBufferView>();
+    arrayBuffer = view->Buffer();
+  } else if (value.isArrayBuffer()) {
+    arrayBuffer = v8::Local<v8::Value>(value).As<v8::ArrayBuffer>();
+  } else {
+    JSG_FAIL_REQUIRE(TypeError, "Object is not transferable");
+  }
+
   uint32_t n;
   for (n = 0; n < arrayBuffers.size(); n++) {
     // If the ArrayBuffer has already been added, we do not want to try adding it again.
-    if (arrayBuffers[n].getHandle(js) == arrayBuffer) {
+    if (arrayBuffers[n] == value) {
       return;
     }
   }
+  arrayBuffers.add(value);
 
-  arrayBuffers.add(js.v8Ref(arrayBuffer));
   backingStores.add(arrayBuffer->GetBackingStore());
   check(arrayBuffer->Detach(v8::Local<v8::Value>()));
   ser.TransferArrayBuffer(n, arrayBuffer);
 }
 
-void Serializer::write(Lock& js, v8::Local<v8::Value> value) {
+void Serializer::write(Lock& js, const JsValue& value) {
   KJ_ASSERT(!released, "The data has already been released.");
   KJ_ASSERT(check(ser.WriteValue(js.v8Context(), value)));
 }
@@ -121,8 +133,8 @@ void Deserializer::init(
   }
 }
 
-v8::Local<v8::Value> Deserializer::readValue(Lock& js) {
-  return check(deser.ReadValue(js.v8Context()));
+JsValue Deserializer::readValue(Lock& js) {
+  return JsValue(check(deser.ReadValue(js.v8Context())));
 }
 
 v8::MaybeLocal<v8::SharedArrayBuffer> Deserializer::GetSharedArrayBufferFromId(
@@ -144,16 +156,14 @@ void SerializedBufferDisposer::disposeImpl(
   free(firstElement);
 }
 
-v8::Local<v8::Value> structuredClone(
+JsValue structuredClone(
     Lock& js,
-    v8::Local<v8::Value> value,
-    kj::Maybe<kj::ArrayPtr<jsg::Value>> maybeTransfer) {
+    const JsValue& value,
+    kj::Maybe<kj::Array<JsValue>> maybeTransfer) {
   Serializer ser(js, nullptr);
   KJ_IF_MAYBE(transfers, maybeTransfer) {
     for (auto& item : *transfers) {
-      auto val = item.getHandle(js);
-      JSG_REQUIRE(val->IsArrayBuffer(), TypeError, "Object is not transferable");
-      ser.transfer(js, val.As<v8::ArrayBuffer>());
+      ser.transfer(js, item);
     }
   }
   ser.write(js, value);

src/workerd/jsg/ser.h

@@ -5,18 +5,21 @@
 #pragma once
 
 #include <workerd/jsg/jsg.h>
+#include <workerd/jsg/buffersource.h>
 #include <kj/vector.h>
 
 namespace workerd::jsg {
 
+// Wraps the v8::ValueSerializer and v8::ValueSerializer::Delegate implementation.
+// Must be allocated on the stack, and requires that a v8::HandleScope exist in
+// the stack.
 class Serializer final: v8::ValueSerializer::Delegate {
-  // Wraps the v8::ValueSerializer and v8::ValueSerializer::Delegate implementation.
 public:
   struct Options {
-    kj::Maybe<uint32_t> version;
     // When set, overrides the default wire format version with the one provided.
-    bool omitHeader = false;
+    kj::Maybe<uint32_t> version;
     // When set to true, the serialization header is not written to the output buffer.
+    bool omitHeader = false;
   };
 
   struct Released {
@@ -30,14 +33,9 @@ class Serializer final: v8::ValueSerializer::Delegate {
 
   KJ_DISALLOW_COPY_AND_MOVE(Serializer);
 
-  void write(Lock& js, v8::Local<v8::Value> value);
-
-  inline void write(Lock& js, Value value) { write(js, value.getHandle(js)); }
-
-  template <typename T>
-  inline void write(Lock& js, V8Ref<T> value) { write(js, value.getHandle(js)); }
+  void write(Lock& js, const JsValue& value);
 
-  void transfer(Lock& js, v8::Local<v8::ArrayBuffer> arrayBuffer);
+  void transfer(Lock& js, const JsValue& value);
 
   Released release();
 
@@ -49,14 +47,17 @@ class Serializer final: v8::ValueSerializer::Delegate {
       v8::Isolate* isolate,
       v8::Local<v8::SharedArrayBuffer> sab) override;
 
-  kj::Vector<V8Ref<v8::SharedArrayBuffer>> sharedArrayBuffers;
-  kj::Vector<V8Ref<v8::ArrayBuffer>> arrayBuffers;
+  kj::Vector<JsValue> sharedArrayBuffers;
+  kj::Vector<JsValue> arrayBuffers;
   kj::Vector<std::shared_ptr<v8::BackingStore>> sharedBackingStores;
   kj::Vector<std::shared_ptr<v8::BackingStore>> backingStores;
   v8::ValueSerializer ser;
   bool released = false;
 };
 
+// Wraps the v8::ValueDeserializer and v8::ValueDeserializer::Delegate implementation.
+// Must be allocated on the stack, and requires that a v8::HandleScope exist in
+// the stack.
 class Deserializer final: v8::ValueDeserializer::Delegate {
 public:
   struct Options {
@@ -80,7 +81,7 @@ class Deserializer final: v8::ValueDeserializer::Delegate {
 
   KJ_DISALLOW_COPY_AND_MOVE(Deserializer);
 
-  v8::Local<v8::Value> readValue(Lock& js);
+  JsValue readValue(Lock& js);
 
   inline uint32_t getVersion() const { return deser.GetWireFormatVersion(); }
 
@@ -98,17 +99,17 @@ class Deserializer final: v8::ValueDeserializer::Delegate {
   kj::Maybe<kj::ArrayPtr<std::shared_ptr<v8::BackingStore>>> sharedBackingStores;
 };
 
+// Intended for use with v8::ValueSerializer data released into a kj::Array.
 class SerializedBufferDisposer: public kj::ArrayDisposer {
-  // Intended for use with v8::ValueSerializer data released into a kj::Array.
 protected:
   void disposeImpl(void* firstElement, size_t elementSize, size_t elementCount,
                    size_t capacity, void (*destroyElement)(void*)) const override;
 };
 constexpr SerializedBufferDisposer SERIALIZED_BUFFER_DISPOSER;
 
-v8::Local<v8::Value> structuredClone(
+JsValue structuredClone(
     Lock& js,
-    v8::Local<v8::Value> value,
-    kj::Maybe<kj::ArrayPtr<jsg::Value>> maybeTransfer = nullptr);
+    const JsValue& value,
+    kj::Maybe<kj::Array<JsValue>> maybeTransfer = nullptr);
 
 }  // namespace workerd::jsg