From 476ff6e639076538dd80e6d9442808136014ccae Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Fri, 11 Oct 2024 14:42:57 +1030 Subject: [PATCH] lib: convert transfer sequence to array in js This commit lets `tranfer` passed to `structuredClone` get validated at JS layer by doing webidl conversion. This avoids the C++ to JS function call overhead in the native implementaiton of `structuredClone` --- .../bootstrap/web/exposed-window-or-worker.js | 7 +++- lib/internal/perf/usertiming.js | 2 +- lib/internal/webidl.js | 12 ++++++ lib/internal/webstreams/readablestream.js | 3 +- lib/internal/worker/js_transferable.js | 39 +++++++++++++++++++ src/node_messaging.cc | 29 ++++++-------- test/parallel/test-structuredClone-global.js | 23 ++++++++++- 7 files changed, 91 insertions(+), 24 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-window-or-worker.js b/lib/internal/bootstrap/web/exposed-window-or-worker.js index d5d844b229003d..d7d9b60b1c8e94 100644 --- a/lib/internal/bootstrap/web/exposed-window-or-worker.js +++ b/lib/internal/bootstrap/web/exposed-window-or-worker.js @@ -38,8 +38,11 @@ const { } = require('internal/process/task_queues'); defineOperation(globalThis, 'queueMicrotask', queueMicrotask); -const { structuredClone } = internalBinding('messaging'); -defineOperation(globalThis, 'structuredClone', structuredClone); +defineLazyProperties( + globalThis, + 'internal/worker/js_transferable', + ['structuredClone'], +); defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']); // https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts diff --git a/lib/internal/perf/usertiming.js b/lib/internal/perf/usertiming.js index dfbbcbaf36c538..b2783799067dde 100644 --- a/lib/internal/perf/usertiming.js +++ b/lib/internal/perf/usertiming.js @@ -31,7 +31,7 @@ const { }, } = require('internal/errors'); -const { structuredClone } = internalBinding('messaging'); +const { structuredClone } = require('internal/worker/js_transferable'); const { lazyDOMException, kEnumerableProperty, diff --git a/lib/internal/webidl.js b/lib/internal/webidl.js index 1278f4cac7fb81..351c1398c6d49c 100644 --- a/lib/internal/webidl.js +++ b/lib/internal/webidl.js @@ -38,6 +38,16 @@ converters.any = (V) => { return V; }; +converters.object = (V, opts = kEmptyObject) => { + if (type(V) !== 'Object') { + throw makeException( + 'is not an object', + kEmptyObject, + ); + } + return V; +}; + // https://webidl.spec.whatwg.org/#abstract-opdef-integerpart const integerPart = MathTrunc; @@ -189,6 +199,8 @@ converters.DOMString = function DOMString(V) { return String(V); }; +converters['sequence'] = createSequenceConverter(converters.object); + function codedTypeError(message, errorProperties = kEmptyObject) { // eslint-disable-next-line no-restricted-syntax const err = new TypeError(message); diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 10179321c10366..5d9379287e9605 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -74,6 +74,7 @@ const { kTransfer, kTransferList, markTransferMode, + structuredClone, } = require('internal/worker/js_transferable'); const { @@ -88,8 +89,6 @@ const { kControllerErrorFunction, } = require('internal/streams/utils'); -const { structuredClone } = internalBinding('messaging'); - const { ArrayBufferViewGetBuffer, ArrayBufferViewGetByteLength, diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js index 083702149f29d9..0812a462bb7caa 100644 --- a/lib/internal/worker/js_transferable.js +++ b/lib/internal/worker/js_transferable.js @@ -3,6 +3,16 @@ const { Error, StringPrototypeSplit, } = primordials; +const { + codes: { + ERR_INVALID_ARG_TYPE, + ERR_MISSING_ARGS, + }, +} = require('internal/errors'); +const webidl = require('internal/webidl'); +const { + structuredClone: nativeStructuredClone, +} = internalBinding('messaging'); const { messaging_deserialize_symbol, messaging_transfer_symbol, @@ -90,9 +100,38 @@ function markTransferMode(obj, cloneable = false, transferable = false) { obj[transfer_mode_private_symbol] = mode; } +function structuredClone(value, options) { + if (arguments.length === 0) { + throw new ERR_MISSING_ARGS('The value argument must be specified'); + } + + // TODO(jazelly): implement generic webidl dictionary converter + const prefix = 'Options'; + const optionsType = webidl.type(options); + if (optionsType !== 'Undefined' && optionsType !== 'Null' && optionsType !== 'Object') { + throw new ERR_INVALID_ARG_TYPE( + prefix, + ['object', 'null', 'undefined'], + options, + ); + } + const key = 'transfer'; + const idlOptions = { __proto__: null, [key]: [] }; + if (options != null && key in options && options[key] !== undefined) { + idlOptions[key] = webidl.converters['sequence'](options[key], { + __proto__: null, + context: 'Transfer', + }); + } + + const serializedData = nativeStructuredClone(value, idlOptions); + return serializedData; +} + module.exports = { markTransferMode, setup, + structuredClone, kClone: messaging_clone_symbol, kDeserialize: messaging_deserialize_symbol, kTransfer: messaging_transfer_symbol, diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 9d9e27c3624e6b..a1c22cf5005121 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1578,28 +1578,21 @@ static void StructuredClone(const FunctionCallbackInfo& args) { Realm* realm = Realm::GetCurrent(context); Environment* env = realm->env(); - if (args.Length() == 0) { - return THROW_ERR_MISSING_ARGS(env, "The value argument must be specified"); - } - Local value = args[0]; TransferList transfer_list; - if (!args[1]->IsNullOrUndefined()) { - if (!args[1]->IsObject()) { - return THROW_ERR_INVALID_ARG_TYPE( - env, "The options argument must be either an object or undefined"); - } - Local options = args[1].As(); - Local transfer_list_v; - if (!options->Get(context, env->transfer_string()) - .ToLocal(&transfer_list_v)) { - return; - } + Local options = args[1].As(); + Local transfer_list_v; + if (!options->Get(context, env->transfer_string()) + .ToLocal(&transfer_list_v)) { + return; + } - // TODO(joyeecheung): implement this in JS land to avoid the C++ -> JS - // cost to convert a sequence into an array. - if (!GetTransferList(env, context, transfer_list_v, &transfer_list)) { + Local arr = transfer_list_v.As(); + size_t length = arr->Length(); + transfer_list.AllocateSufficientStorage(length); + for (size_t i = 0; i < length; i++) { + if (!arr->Get(context, i).ToLocal(&transfer_list[i])) { return; } } diff --git a/test/parallel/test-structuredClone-global.js b/test/parallel/test-structuredClone-global.js index 34b6abe32d3fcf..ef6ddc56a73cca 100644 --- a/test/parallel/test-structuredClone-global.js +++ b/test/parallel/test-structuredClone-global.js @@ -8,12 +8,12 @@ assert.throws(() => structuredClone(undefined, ''), { code: 'ERR_INVALID_ARG_TYP assert.throws(() => structuredClone(undefined, 1), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => structuredClone(undefined, { transfer: 1 }), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => structuredClone(undefined, { transfer: '' }), { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => structuredClone(undefined, { transfer: null }), { code: 'ERR_INVALID_ARG_TYPE' }); // Options can be null or undefined. assert.strictEqual(structuredClone(undefined), undefined); assert.strictEqual(structuredClone(undefined, null), undefined); // Transfer can be null or undefined. -assert.strictEqual(structuredClone(undefined, { transfer: null }), undefined); assert.strictEqual(structuredClone(undefined, { }), undefined); // Transferables or its subclasses should be received with its closest transferable superclass @@ -43,6 +43,27 @@ for (const Transferrable of [File, Blob]) { assert.ok(extendedTransfer instanceof Transferrable); } +// Transfer can be iterable +{ + const value = { + a: new ReadableStream(), + b: new WritableStream(), + }; + const cloned = structuredClone(value, { + transfer: { + *[Symbol.iterator]() { + for (const key in value) { + yield value[key]; + } + } + } + }); + for (const key in value) { + assert.ok(value[key].locked); + assert.ok(!cloned[key].locked); + } +} + { // See: https://github.com/nodejs/node/issues/49940 const cloned = structuredClone({}, {