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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Feb 28, 2019 · 24debc9 · 24debc9
1 parent 0408966
commit 24debc9
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 17 deletions.
diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js
@@ -6,7 +6,9 @@ const {
 } = internalBinding('symbols');
 const {
   MessagePort,
-  MessageChannel
+  MessageChannel,
+  drainMessagePort,
+  stopMessagePort
 } = internalBinding('messaging');
 const { threadId } = internalBinding('worker');
 
@@ -33,13 +35,6 @@ const messageTypes = {
   LOAD_SCRIPT: 'loadScript'
 };
 
-// Original drain from C++
-const originalDrain = MessagePort.prototype.drain;
-
-function drainMessagePort(port) {
-  return originalDrain.call(port);
-}
-
 // We have to mess with the MessagePort prototype a bit, so that a) we can make
 // it inherit from EventEmitter, even though it is a C++ class, and b) we do
 // not provide methods that are not present in the Browser and not documented
@@ -51,9 +46,8 @@ const MessagePortPrototype = Object.create(
 // Set up the new inheritance chain.
 Object.setPrototypeOf(MessagePort, EventEmitter);
 Object.setPrototypeOf(MessagePort.prototype, EventEmitter.prototype);
-// Finally, purge methods we don't want to be public.
-delete MessagePort.prototype.stop;
-delete MessagePort.prototype.drain;
+// Copy methods that are inherited from HandleWrap, because
+// changing the prototype of MessagePort.prototype implicitly removed them.
 MessagePort.prototype.ref = MessagePortPrototype.ref;
 MessagePort.prototype.unref = MessagePortPrototype.unref;
 
@@ -84,7 +78,7 @@ Object.defineProperty(MessagePort.prototype, 'onmessage', {
       MessagePortPrototype.start.call(this);
     } else {
       this.unref();
-      MessagePortPrototype.stop.call(this);
+      stopMessagePort(this);
     }
   }
 });
@@ -152,7 +146,7 @@ function setupPortReferencing(port, eventEmitter, eventName) {
   });
   eventEmitter.on('removeListener', (name) => {
     if (name === eventName && eventEmitter.listenerCount(eventName) === 0) {
-      MessagePortPrototype.stop.call(port);
+      stopMessagePort(port);
       port.unref();
     }
   });

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -741,7 +741,8 @@ void MessagePort::Start(const FunctionCallbackInfo<Value>& args) {
 void MessagePort::Stop(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   MessagePort* port;
-  ASSIGN_OR_RETURN_UNWRAP(&port, args.This());
+  CHECK(args[0]->IsObject());
+  ASSIGN_OR_RETURN_UNWRAP(&port, args[0].As<Object>());
   if (!port->data_) {
     THROW_ERR_CLOSED_MESSAGE_PORT(env);
     return;
@@ -751,7 +752,8 @@ void MessagePort::Stop(const FunctionCallbackInfo<Value>& args) {
 
 void MessagePort::Drain(const FunctionCallbackInfo<Value>& args) {
   MessagePort* port;
-  ASSIGN_OR_RETURN_UNWRAP(&port, args.This());
+  CHECK(args[0]->IsObject());
+  ASSIGN_OR_RETURN_UNWRAP(&port, args[0].As<Object>());
   port->OnMessage();
 }
 
@@ -781,8 +783,6 @@ MaybeLocal<Function> GetMessagePortConstructor(
 
     env->SetProtoMethod(m, "postMessage", MessagePort::PostMessage);
     env->SetProtoMethod(m, "start", MessagePort::Start);
-    env->SetProtoMethod(m, "stop", MessagePort::Stop);
-    env->SetProtoMethod(m, "drain", MessagePort::Drain);
 
     env->set_message_port_constructor_template(m);
 
@@ -848,6 +848,11 @@ static void InitMessaging(Local<Object> target,
                   .FromJust();
 
   env->SetMethod(target, "registerDOMException", RegisterDOMException);
+
+  // These are not methods on the MessagePort prototype, because
+  // the browser equivalents do not provide them.
+  env->SetMethod(target, "stopMessagePort", MessagePort::Stop);
+  env->SetMethod(target, "drainMessagePort", MessagePort::Drain);
 }
 
 }  // anonymous namespace