Skip to content

Commit 6f06243

Browse files
fix(runtime): deliver a posted undefined as undefined, not null (#477)
Every delivery path built its MessageEvent through the public constructor, whose init dictionary turns an undefined data into null. Delivery now goes through an internal factory that stores the payload as given, and the native messageerror paths pass null so that event keeps its default data.
1 parent e146d4e commit 6f06243

10 files changed

Lines changed: 161 additions & 31 deletions

File tree

‎NativeScript/runtime/Messaging.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,13 @@ void NativeMessagePort::Drain() {
687687
if (tc.HasTerminated() || !tc.CanContinue()) {
688688
return;
689689
}
690-
payload = tc.HasCaught() ? tc.Exception()
691-
: v8::Undefined(isolate).As<Value>();
690+
// Null rather than undefined when there is nothing to carry, which
691+
// includes a thrown undefined: delivery stores the payload as given,
692+
// and an event's `data` defaults to null.
693+
payload = tc.HasCaught() ? tc.Exception() : Local<Value>();
694+
if (payload.IsEmpty() || payload->IsUndefined()) {
695+
payload = v8::Null(isolate);
696+
}
692697
tc.Reset();
693698
}
694699
}

‎NativeScript/runtime/Worker.mm‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,9 +729,11 @@ throw NativeScriptException(
729729
return;
730730
}
731731
// HTML: a message that cannot be read still reaches its target, as a
732-
// `messageerror` event carrying nothing.
732+
// `messageerror` event carrying nothing, which leaves `data` at the
733+
// null it defaults to. Delivery stores what it is given, so an
734+
// undefined here would surface as undefined.
733735
tc.Reset();
734-
data = v8::Undefined(isolate);
736+
data = v8::Null(isolate);
735737
ports = Local<Value>();
736738
type = "messageerror";
737739
}

‎NativeScript/runtime/js/broadcast-channel.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ const { adoptPort } = require("internal/message-channel");
2424
const addEventListener = EventTarget.prototype.addEventListener;
2525
const dispatchEvent = EventTarget.prototype.dispatchEvent;
2626

27-
let MessageEvent;
28-
function getMessageEvent() {
29-
if (MessageEvent === undefined) {
30-
({ MessageEvent } = require("internal/message-event"));
27+
let createMessageEvent;
28+
function getCreateMessageEvent() {
29+
if (createMessageEvent === undefined) {
30+
({ createMessageEvent } = require("internal/message-event"));
3131
}
32-
return MessageEvent;
32+
return createMessageEvent;
3333
}
3434

3535
let DOMException;
@@ -64,7 +64,7 @@ class BroadcastChannel extends EventTarget {
6464
FunctionPrototypeCall(
6565
dispatchEvent,
6666
channel,
67-
new (getMessageEvent())(event.type, { data: event.data })
67+
getCreateMessageEvent()(event.type, event.data)
6868
);
6969
};
7070
FunctionPrototypeCall(addEventListener, port, "message", relay);

‎NativeScript/runtime/js/message-channel.js‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,12 @@ const {
5353
const addEventListener = EventTarget.prototype.addEventListener;
5454
const dispatchEvent = EventTarget.prototype.dispatchEvent;
5555

56-
let MessageEvent;
57-
function getMessageEvent() {
58-
if (MessageEvent === undefined) {
59-
({ MessageEvent } = require("internal/message-event"));
56+
let createMessageEvent;
57+
function getCreateMessageEvent() {
58+
if (createMessageEvent === undefined) {
59+
({ createMessageEvent } = require("internal/message-event"));
6060
}
61-
return MessageEvent;
61+
return createMessageEvent;
6262
}
6363

6464
// WebIDL sequence<object>. Entries are handed to the native transfer-list
@@ -253,11 +253,10 @@ function emitMessage(data, ports, type) {
253253
ArrayPrototypePush(list, adoptPort(ports[i]));
254254
}
255255
}
256-
const MessageEventCtor = getMessageEvent();
257256
FunctionPrototypeCall(
258257
dispatchEvent,
259258
this,
260-
new MessageEventCtor(type, { data, ports: list })
259+
getCreateMessageEvent()(type, data, list)
261260
);
262261
}
263262

‎NativeScript/runtime/js/message-event.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ function toPortSequence(value) {
5353
return list;
5454
}
5555

56+
// Builds the event a delivered message dispatches. The constructor cannot
57+
// serve delivery: per Web IDL its init dictionary treats `data: undefined` as
58+
// absent and defaults it to null, whereas a message that deserialized to
59+
// undefined has to arrive as undefined.
60+
let createMessageEvent;
61+
5662
class MessageEvent extends Event {
5763
#data;
5864
#origin;
@@ -134,6 +140,17 @@ class MessageEvent extends Event {
134140
this.#source = source;
135141
this.#ports = ports === null ? [] : toPortSequence(ports);
136142
}
143+
144+
static {
145+
createMessageEvent = (type, data, ports) => {
146+
// Null-prototype init: the constructor reads every dictionary member,
147+
// and one this object lacks would otherwise be looked up on
148+
// Object.prototype, which app code can change.
149+
const event = new MessageEvent(type, { __proto__: null, ports });
150+
event.#data = data;
151+
return event;
152+
};
153+
}
137154
}
138155

139156
// Class members are non-enumerable; the IDL attributes and operations are not.
@@ -150,4 +167,4 @@ ObjectDefineProperty(MessageEvent.prototype, SymbolToStringTag, {
150167
configurable: true,
151168
});
152169

153-
module.exports = { MessageEvent };
170+
module.exports = { MessageEvent, createMessageEvent };

‎NativeScript/runtime/js/node-worker-threads.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ const {
4848
globalEventTarget,
4949
} = require("internal/events");
5050

51-
let MessageEvent;
52-
function getMessageEvent() {
53-
if (MessageEvent === undefined) {
54-
({ MessageEvent } = require("internal/message-event"));
51+
let createMessageEvent;
52+
function getCreateMessageEvent() {
53+
if (createMessageEvent === undefined) {
54+
({ createMessageEvent } = require("internal/message-event"));
5555
}
56-
return MessageEvent;
56+
return createMessageEvent;
5757
}
5858

5959
const g = globalThis;
@@ -348,7 +348,7 @@ if (!isMainThread) {
348348
FunctionPrototypeCall(
349349
dispatchEvent,
350350
parentPort,
351-
new (getMessageEvent())(event.type, { data: event.data, ports: event.ports })
351+
getCreateMessageEvent()(event.type, event.data, event.ports)
352352
);
353353
};
354354
FunctionPrototypeCall(addEventListener, globalEventTarget, "message", relay);

‎NativeScript/runtime/js/worker-events.js‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ const {
2121

2222
const g = globalThis;
2323

24-
let MessageEvent;
25-
function getMessageEvent() {
26-
if (MessageEvent === undefined) {
27-
({ MessageEvent } = require("internal/message-event"));
24+
let createMessageEvent;
25+
function getCreateMessageEvent() {
26+
if (createMessageEvent === undefined) {
27+
({ createMessageEvent } = require("internal/message-event"));
2828
}
29-
return MessageEvent;
29+
return createMessageEvent;
3030
}
3131

3232
// ErrorEvent is installed by the error-events builtin, which Runtime::Init
@@ -49,8 +49,7 @@ function getErrorEvent() {
4949
// is what feeds the worker's onerror chain — the worker scope's handler
5050
// first, then the parent's — which the cross-runtime worker suite asserts.
5151
function emitMessage(data, ports, type) {
52-
const MessageEventCtor = getMessageEvent();
53-
dispatchEventRethrowing(this, new MessageEventCtor(type, { data, ports }));
52+
dispatchEventRethrowing(this, getCreateMessageEvent()(type, data, ports));
5453
}
5554

5655
// The parent-side error delivery callout, invoked by native with the Worker

‎TestRunner/app/tests/MessagingTests.js‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,100 @@ describe("Messaging runtime edges", function () {
201201
});
202202
});
203203

204+
// The MessageEvent constructor defaults an undefined `data` to null, as Web
205+
// IDL requires of its init dictionary. A delivered message is not built
206+
// from a dictionary: it carries whatever the payload deserialized to.
207+
describe("undefined payloads", function () {
208+
var payloads = [undefined, null, false, 0, ""];
209+
// Closed after every spec, so one that failed or timed out leaves no
210+
// worker or channel behind. terminate() and close() are idempotent.
211+
var cleanups = [];
212+
afterEach(function () {
213+
cleanups.forEach(function (cleanup) { cleanup(); });
214+
cleanups = [];
215+
});
216+
217+
// fail() throws in this runner, which would skip done() when called
218+
// from an event handler.
219+
function failOnWorkerError(done) {
220+
return function (error) {
221+
expect("worker error: " + error.message).toBeNull();
222+
done();
223+
};
224+
}
225+
226+
function expectPayloads(events) {
227+
expect(events.map(function (event) { return event.data; })).toEqual(payloads);
228+
expect(events[0].data).toBeUndefined();
229+
expect("data" in events[0]).toBe(true);
230+
}
231+
232+
it("arrive unchanged on a MessagePort", function (done) {
233+
var channel = new MessageChannel();
234+
// The receiver first: closing only the sender queues the close
235+
// behind the messages, which would still be delivered.
236+
cleanups.push(function () { channel.port2.close(); channel.port1.close(); });
237+
var events = [];
238+
channel.port2.onmessage = function (event) {
239+
events.push(event);
240+
if (events.length === payloads.length) {
241+
expectPayloads(events);
242+
done();
243+
}
244+
};
245+
payloads.forEach(function (payload) { channel.port1.postMessage(payload); });
246+
});
247+
248+
it("arrive unchanged on a BroadcastChannel", function (done) {
249+
var sender = new BroadcastChannel("undefined-payloads");
250+
var receiver = new BroadcastChannel("undefined-payloads");
251+
cleanups.push(function () { sender.close(); receiver.close(); });
252+
var events = [];
253+
receiver.onmessage = function (event) {
254+
events.push(event);
255+
if (events.length === payloads.length) {
256+
expectPayloads(events);
257+
done();
258+
}
259+
};
260+
payloads.forEach(function (payload) { sender.postMessage(payload); });
261+
});
262+
263+
it("arrive unchanged in a worker and back on its Worker object", function (done) {
264+
var worker = new Worker("./messaging/describeDataWorker.js");
265+
cleanups.push(function () { worker.terminate(); });
266+
var events = [];
267+
worker.onmessage = function (event) {
268+
events.push(event);
269+
if (events.length === 2) {
270+
expect(events[0].data).toEqual({ received: "undefined" });
271+
expect(events[1].data).toBeUndefined();
272+
expect("data" in events[1]).toBe(true);
273+
done();
274+
}
275+
};
276+
worker.onerror = failOnWorkerError(done);
277+
worker.postMessage(undefined);
278+
});
279+
280+
it("arrive unchanged on a node:worker_threads parentPort", function (done) {
281+
var wt = require("node:worker_threads");
282+
var worker = new wt.Worker("~/tests/messaging/parentPortDescribeWorker.js");
283+
cleanups.push(function () { worker.terminate(); });
284+
worker.on("message", function (value) {
285+
expect(value).toEqual({ received: "undefined" });
286+
done();
287+
});
288+
worker.on("error", failOnWorkerError(done));
289+
worker.postMessage(undefined);
290+
});
291+
292+
it("still default to null in a constructed MessageEvent", function () {
293+
expect(new MessageEvent("message").data).toBeNull();
294+
expect(new MessageEvent("message", { data: undefined }).data).toBeNull();
295+
});
296+
});
297+
204298
describe("worker error reporting", function () {
205299
// A worker boots on its own thread, so the first error arrives whenever
206300
// the runner gets to it; specs wait for it and only then settle for
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Reports what `event.data` was before echoing it, so the parent can tell a
2+
// value lost on the way in from one lost on the way back.
3+
function describe(value) {
4+
return value === undefined ? "undefined" : value === null ? "null" : typeof value;
5+
}
6+
7+
onmessage = function (event) {
8+
postMessage({ received: describe(event.data) });
9+
postMessage(event.data);
10+
};
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
var parentPort = require("node:worker_threads").parentPort;
2+
parentPort.on("message", function (value) {
3+
parentPort.postMessage({ received: value === undefined ? "undefined" : typeof value });
4+
});

0 commit comments

Comments
 (0)