Skip to content

Commit d48c5da

Browse files
jazellyruyadorno
authored andcommitted
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` PR-URL: #55317 Fixes: #55280 Refs: #50330 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
1 parent fce8c32 commit d48c5da

File tree

7 files changed

+68
-24
lines changed

7 files changed

+68
-24
lines changed

lib/internal/bootstrap/web/exposed-window-or-worker.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ const {
3232
} = require('internal/process/task_queues');
3333
defineOperation(globalThis, 'queueMicrotask', queueMicrotask);
3434

35-
const { structuredClone } = internalBinding('messaging');
36-
defineOperation(globalThis, 'structuredClone', structuredClone);
35+
defineLazyProperties(
36+
globalThis,
37+
'internal/worker/js_transferable',
38+
['structuredClone'],
39+
);
3740
defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
3841

3942
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts

lib/internal/perf/usertiming.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const {
3131
},
3232
} = require('internal/errors');
3333

34-
const { structuredClone } = internalBinding('messaging');
34+
const { structuredClone } = require('internal/worker/js_transferable');
3535
const {
3636
lazyDOMException,
3737
kEnumerableProperty,

lib/internal/webidl.js

+12
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ converters.any = (V) => {
3636
return V;
3737
};
3838

39+
converters.object = (V, opts = kEmptyObject) => {
40+
if (type(V) !== 'Object') {
41+
throw makeException(
42+
'is not an object',
43+
kEmptyObject,
44+
);
45+
}
46+
return V;
47+
};
48+
3949
// https://webidl.spec.whatwg.org/#abstract-opdef-integerpart
4050
const integerPart = MathTrunc;
4151

@@ -187,6 +197,8 @@ converters.DOMString = function DOMString(V) {
187197
return String(V);
188198
};
189199

200+
converters['sequence<object>'] = createSequenceConverter(converters.object);
201+
190202
function codedTypeError(message, errorProperties = kEmptyObject) {
191203
// eslint-disable-next-line no-restricted-syntax
192204
const err = new TypeError(message);

lib/internal/webstreams/readablestream.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const {
7474
kTransfer,
7575
kTransferList,
7676
markTransferMode,
77+
structuredClone,
7778
} = require('internal/worker/js_transferable');
7879

7980
const {
@@ -88,8 +89,6 @@ const {
8889
kControllerErrorFunction,
8990
} = require('internal/streams/utils');
9091

91-
const { structuredClone } = internalBinding('messaging');
92-
9392
const {
9493
ArrayBufferViewGetBuffer,
9594
ArrayBufferViewGetByteLength,

lib/internal/worker/js_transferable.js

+37
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ const {
33
Error,
44
StringPrototypeSplit,
55
} = primordials;
6+
const {
7+
codes: {
8+
ERR_INVALID_ARG_TYPE,
9+
ERR_MISSING_ARGS,
10+
},
11+
} = require('internal/errors');
12+
const webidl = require('internal/webidl');
613
const {
714
messaging_deserialize_symbol,
815
messaging_transfer_symbol,
@@ -11,6 +18,7 @@ const {
1118
} = internalBinding('symbols');
1219
const {
1320
setDeserializerCreateObjectFunction,
21+
structuredClone: nativeStructuredClone,
1422
} = internalBinding('messaging');
1523
const {
1624
privateSymbols: {
@@ -90,9 +98,38 @@ function markTransferMode(obj, cloneable = false, transferable = false) {
9098
obj[transfer_mode_private_symbol] = mode;
9199
}
92100

101+
function structuredClone(value, options) {
102+
if (arguments.length === 0) {
103+
throw new ERR_MISSING_ARGS('The value argument must be specified');
104+
}
105+
106+
// TODO(jazelly): implement generic webidl dictionary converter
107+
const prefix = 'Options';
108+
const optionsType = webidl.type(options);
109+
if (optionsType !== 'Undefined' && optionsType !== 'Null' && optionsType !== 'Object') {
110+
throw new ERR_INVALID_ARG_TYPE(
111+
prefix,
112+
['object', 'null', 'undefined'],
113+
options,
114+
);
115+
}
116+
const key = 'transfer';
117+
const idlOptions = { __proto__: null, [key]: [] };
118+
if (options != null && key in options && options[key] !== undefined) {
119+
idlOptions[key] = webidl.converters['sequence<object>'](options[key], {
120+
__proto__: null,
121+
context: 'Transfer',
122+
});
123+
}
124+
125+
const serializedData = nativeStructuredClone(value, idlOptions);
126+
return serializedData;
127+
}
128+
93129
module.exports = {
94130
markTransferMode,
95131
setup,
132+
structuredClone,
96133
kClone: messaging_clone_symbol,
97134
kDeserialize: messaging_deserialize_symbol,
98135
kTransfer: messaging_transfer_symbol,

src/node_messaging.cc

+11-18
Original file line numberDiff line numberDiff line change
@@ -1578,28 +1578,21 @@ static void StructuredClone(const FunctionCallbackInfo<Value>& args) {
15781578
Realm* realm = Realm::GetCurrent(context);
15791579
Environment* env = realm->env();
15801580

1581-
if (args.Length() == 0) {
1582-
return THROW_ERR_MISSING_ARGS(env, "The value argument must be specified");
1583-
}
1584-
15851581
Local<Value> value = args[0];
15861582

15871583
TransferList transfer_list;
1588-
if (!args[1]->IsNullOrUndefined()) {
1589-
if (!args[1]->IsObject()) {
1590-
return THROW_ERR_INVALID_ARG_TYPE(
1591-
env, "The options argument must be either an object or undefined");
1592-
}
1593-
Local<Object> options = args[1].As<Object>();
1594-
Local<Value> transfer_list_v;
1595-
if (!options->Get(context, env->transfer_string())
1596-
.ToLocal(&transfer_list_v)) {
1597-
return;
1598-
}
1584+
Local<Object> options = args[1].As<Object>();
1585+
Local<Value> transfer_list_v;
1586+
if (!options->Get(context, env->transfer_string())
1587+
.ToLocal(&transfer_list_v)) {
1588+
return;
1589+
}
15991590

1600-
// TODO(joyeecheung): implement this in JS land to avoid the C++ -> JS
1601-
// cost to convert a sequence into an array.
1602-
if (!GetTransferList(env, context, transfer_list_v, &transfer_list)) {
1591+
Local<Array> arr = transfer_list_v.As<Array>();
1592+
size_t length = arr->Length();
1593+
transfer_list.AllocateSufficientStorage(length);
1594+
for (size_t i = 0; i < length; i++) {
1595+
if (!arr->Get(context, i).ToLocal(&transfer_list[i])) {
16031596
return;
16041597
}
16051598
}

test/parallel/test-structuredClone-global.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ assert.throws(() => structuredClone(undefined, ''), { code: 'ERR_INVALID_ARG_TYP
88
assert.throws(() => structuredClone(undefined, 1), { code: 'ERR_INVALID_ARG_TYPE' });
99
assert.throws(() => structuredClone(undefined, { transfer: 1 }), { code: 'ERR_INVALID_ARG_TYPE' });
1010
assert.throws(() => structuredClone(undefined, { transfer: '' }), { code: 'ERR_INVALID_ARG_TYPE' });
11+
assert.throws(() => structuredClone(undefined, { transfer: null }), { code: 'ERR_INVALID_ARG_TYPE' });
1112

1213
// Options can be null or undefined.
1314
assert.strictEqual(structuredClone(undefined), undefined);
1415
assert.strictEqual(structuredClone(undefined, null), undefined);
1516
// Transfer can be null or undefined.
16-
assert.strictEqual(structuredClone(undefined, { transfer: null }), undefined);
1717
assert.strictEqual(structuredClone(undefined, { }), undefined);
1818

1919
// Transferables or its subclasses should be received with its closest transferable superclass

0 commit comments

Comments
 (0)