Skip to content

Commit 24debc9

Browse files
addaleaxrvagg
authored andcommitted
worker: do not add removed methods to MessagePort
Do not put the `.stop()` and `.drain()` methods on the `MessagePort` prototype if we are going to remove them later on anyway. PR-URL: #26109 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0408966 commit 24debc9

File tree

2 files changed

+16
-17
lines changed

2 files changed

+16
-17
lines changed

lib/internal/worker/io.js

+7-13
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ const {
66
} = internalBinding('symbols');
77
const {
88
MessagePort,
9-
MessageChannel
9+
MessageChannel,
10+
drainMessagePort,
11+
stopMessagePort
1012
} = internalBinding('messaging');
1113
const { threadId } = internalBinding('worker');
1214

@@ -33,13 +35,6 @@ const messageTypes = {
3335
LOAD_SCRIPT: 'loadScript'
3436
};
3537

36-
// Original drain from C++
37-
const originalDrain = MessagePort.prototype.drain;
38-
39-
function drainMessagePort(port) {
40-
return originalDrain.call(port);
41-
}
42-
4338
// We have to mess with the MessagePort prototype a bit, so that a) we can make
4439
// it inherit from EventEmitter, even though it is a C++ class, and b) we do
4540
// not provide methods that are not present in the Browser and not documented
@@ -51,9 +46,8 @@ const MessagePortPrototype = Object.create(
5146
// Set up the new inheritance chain.
5247
Object.setPrototypeOf(MessagePort, EventEmitter);
5348
Object.setPrototypeOf(MessagePort.prototype, EventEmitter.prototype);
54-
// Finally, purge methods we don't want to be public.
55-
delete MessagePort.prototype.stop;
56-
delete MessagePort.prototype.drain;
49+
// Copy methods that are inherited from HandleWrap, because
50+
// changing the prototype of MessagePort.prototype implicitly removed them.
5751
MessagePort.prototype.ref = MessagePortPrototype.ref;
5852
MessagePort.prototype.unref = MessagePortPrototype.unref;
5953

@@ -84,7 +78,7 @@ Object.defineProperty(MessagePort.prototype, 'onmessage', {
8478
MessagePortPrototype.start.call(this);
8579
} else {
8680
this.unref();
87-
MessagePortPrototype.stop.call(this);
81+
stopMessagePort(this);
8882
}
8983
}
9084
});
@@ -152,7 +146,7 @@ function setupPortReferencing(port, eventEmitter, eventName) {
152146
});
153147
eventEmitter.on('removeListener', (name) => {
154148
if (name === eventName && eventEmitter.listenerCount(eventName) === 0) {
155-
MessagePortPrototype.stop.call(port);
149+
stopMessagePort(port);
156150
port.unref();
157151
}
158152
});

src/node_messaging.cc

+9-4
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,8 @@ void MessagePort::Start(const FunctionCallbackInfo<Value>& args) {
741741
void MessagePort::Stop(const FunctionCallbackInfo<Value>& args) {
742742
Environment* env = Environment::GetCurrent(args);
743743
MessagePort* port;
744-
ASSIGN_OR_RETURN_UNWRAP(&port, args.This());
744+
CHECK(args[0]->IsObject());
745+
ASSIGN_OR_RETURN_UNWRAP(&port, args[0].As<Object>());
745746
if (!port->data_) {
746747
THROW_ERR_CLOSED_MESSAGE_PORT(env);
747748
return;
@@ -751,7 +752,8 @@ void MessagePort::Stop(const FunctionCallbackInfo<Value>& args) {
751752

752753
void MessagePort::Drain(const FunctionCallbackInfo<Value>& args) {
753754
MessagePort* port;
754-
ASSIGN_OR_RETURN_UNWRAP(&port, args.This());
755+
CHECK(args[0]->IsObject());
756+
ASSIGN_OR_RETURN_UNWRAP(&port, args[0].As<Object>());
755757
port->OnMessage();
756758
}
757759

@@ -781,8 +783,6 @@ MaybeLocal<Function> GetMessagePortConstructor(
781783

782784
env->SetProtoMethod(m, "postMessage", MessagePort::PostMessage);
783785
env->SetProtoMethod(m, "start", MessagePort::Start);
784-
env->SetProtoMethod(m, "stop", MessagePort::Stop);
785-
env->SetProtoMethod(m, "drain", MessagePort::Drain);
786786

787787
env->set_message_port_constructor_template(m);
788788

@@ -848,6 +848,11 @@ static void InitMessaging(Local<Object> target,
848848
.FromJust();
849849

850850
env->SetMethod(target, "registerDOMException", RegisterDOMException);
851+
852+
// These are not methods on the MessagePort prototype, because
853+
// the browser equivalents do not provide them.
854+
env->SetMethod(target, "stopMessagePort", MessagePort::Stop);
855+
env->SetMethod(target, "drainMessagePort", MessagePort::Drain);
851856
}
852857

853858
} // anonymous namespace

0 commit comments

Comments
 (0)