src,lib: make DOMException available in all Contexts

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: #26497 Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 27, 2019 · 1e669b2 · 1e669b2
1 parent e044563
commit 1e669b2
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 37 deletions.
diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js
@@ -25,6 +25,7 @@ const cannotBeRequired = [
   'internal/bootstrap/node',
 
   'internal/per_context/setup',
+  'internal/per_context/domexception',
 ];
 
 // Skip modules that cannot be required when they are not

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -220,8 +220,6 @@ if (!config.noBrowserGlobals) {
   defineOperation(global, 'setImmediate', timers.setImmediate);
 }
 
-setupDOMException();
-
 Object.defineProperty(process, 'argv0', {
   enumerable: true,
   configurable: false,
@@ -450,13 +448,6 @@ function setupQueueMicrotask() {
   });
 }
 
-function setupDOMException() {
-  // Registers the constructor with C++.
-  const DOMException = NativeModule.require('internal/domexception');
-  const { registerDOMException } = internalBinding('messaging');
-  registerDOMException(DOMException);
-}
-
 function noop() {}
 
 // https://heycam.github.io/webidl/#es-namespaces

diff --git a/lib/internal/domexception.js → lib/internal/per_context/domexception.js b/lib/internal/domexception.js → lib/internal/per_context/domexception.js
@@ -1,6 +1,12 @@
 'use strict';
 
-const { ERR_INVALID_THIS } = require('internal/errors').codes;
+class ERR_INVALID_THIS extends TypeError {
+  constructor(type) {
+    super('Value of "this" must be of ' + type);
+  }
+
+  get code() { return 'ERR_INVALID_THIS'; }
+}
 
 const internalsMap = new WeakMap();
 
@@ -83,4 +89,4 @@ for (const [name, codeName, value] of [
   nameToCodeMap.set(name, value);
 }
 
-module.exports = DOMException;
+exports.DOMException = DOMException;
diff --git a/node.gyp b/node.gyp
@@ -33,6 +33,7 @@
       'lib/internal/bootstrap/node.js',
       'lib/internal/bootstrap/pre_execution.js',
       'lib/internal/per_context/setup.js',
+      'lib/internal/per_context/domexception.js',
       'lib/async_hooks.js',
       'lib/assert.js',
       'lib/buffer.js',
@@ -117,7 +118,6 @@
       'lib/internal/dgram.js',
       'lib/internal/dns/promises.js',
       'lib/internal/dns/utils.js',
-      'lib/internal/domexception.js',
       'lib/internal/encoding.js',
       'lib/internal/errors.js',
       'lib/internal/error-serdes.js',

diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -15,14 +15,17 @@
 
 namespace node {
 using v8::Context;
+using v8::EscapableHandleScope;
 using v8::Function;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
 using v8::Message;
 using v8::MicrotasksPolicy;
+using v8::Object;
 using v8::ObjectTemplate;
+using v8::Private;
 using v8::String;
 using v8::Value;
 
@@ -249,6 +252,26 @@ void FreePlatform(MultiIsolatePlatform* platform) {
   delete platform;
 }
 
+MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
+  Isolate* isolate = context->GetIsolate();
+  EscapableHandleScope handle_scope(isolate);
+
+  Local<Object> global = context->Global();
+  Local<Private> key = Private::ForApi(isolate,
+      FIXED_ONE_BYTE_STRING(isolate, "node:per_context_binding_exports"));
+
+  Local<Value> existing_value;
+  if (!global->GetPrivate(context, key).ToLocal(&existing_value))
+    return MaybeLocal<Object>();
+  if (existing_value->IsObject())
+    return handle_scope.Escape(existing_value.As<Object>());
+
+  Local<Object> exports = Object::New(isolate);
+  if (context->Global()->SetPrivate(context, key, exports).IsNothing())
+    return MaybeLocal<Object>();
+  return handle_scope.Escape(exports);
+}
+
 Local<Context> NewContext(Isolate* isolate,
                           Local<ObjectTemplate> object_template) {
   auto context = Context::New(isolate, nullptr, object_template);
@@ -261,16 +284,25 @@ Local<Context> NewContext(Isolate* isolate,
   {
     // Run per-context JS files.
     Context::Scope context_scope(context);
+    Local<Object> exports;
+    if (!GetPerContextExports(context).ToLocal(&exports))
+      return Local<Context>();
+
+    Local<String> global_string = FIXED_ONE_BYTE_STRING(isolate, "global");
+    Local<String> exports_string = FIXED_ONE_BYTE_STRING(isolate, "exports");
 
     static const char* context_files[] = {
       "internal/per_context/setup",
+      "internal/per_context/domexception",
       nullptr
     };
 
     for (const char** module = context_files; *module != nullptr; module++) {
       std::vector<Local<String>> parameters = {
-          FIXED_ONE_BYTE_STRING(isolate, "global")};
-      Local<Value> arguments[] = {context->Global()};
+        global_string,
+        exports_string
+      };
+      Local<Value> arguments[] = {context->Global(), exports};
       MaybeLocal<Function> maybe_fn =
           per_process::native_module_loader.LookupAndCompile(
               context, *module, &parameters, nullptr);

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -294,6 +294,8 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
 v8::MaybeLocal<v8::Value> RunBootstrapping(Environment* env);
 v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
                                          const char* main_script_id);
+v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
+
 namespace coverage {
 bool StartCoverageCollection(Environment* env);
 }

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -177,19 +177,30 @@ uint32_t Message::AddWASMModule(WasmCompiledModule::TransferrableModule&& mod) {
 
 namespace {
 
-void ThrowDataCloneException(Environment* env, Local<String> message) {
+void ThrowDataCloneException(Local<Context> context, Local<String> message) {
+  Isolate* isolate = context->GetIsolate();
   Local<Value> argv[] = {
     message,
-    FIXED_ONE_BYTE_STRING(env->isolate(), "DataCloneError")
+    FIXED_ONE_BYTE_STRING(isolate, "DataCloneError")
   };
   Local<Value> exception;
-  Local<Function> domexception_ctor = env->domexception_function();
-  CHECK(!domexception_ctor.IsEmpty());
-  if (!domexception_ctor->NewInstance(env->context(), arraysize(argv), argv)
+
+  Local<Object> per_context_bindings;
+  Local<Value> domexception_ctor_val;
+  if (!GetPerContextExports(context).ToLocal(&per_context_bindings) ||
+      !per_context_bindings->Get(context,
+                                FIXED_ONE_BYTE_STRING(isolate, "DOMException"))
+          .ToLocal(&domexception_ctor_val)) {
+    return;
+  }
+
+  CHECK(domexception_ctor_val->IsFunction());
+  Local<Function> domexception_ctor = domexception_ctor_val.As<Function>();
+  if (!domexception_ctor->NewInstance(context, arraysize(argv), argv)
           .ToLocal(&exception)) {
     return;
   }
-  env->isolate()->ThrowException(exception);
+  isolate->ThrowException(exception);
 }
 
 // This tells V8 how to serialize objects that it does not understand
@@ -201,7 +212,7 @@ class SerializerDelegate : public ValueSerializer::Delegate {
       : env_(env), context_(context), msg_(m) {}
 
   void ThrowDataCloneError(Local<String> message) override {
-    ThrowDataCloneException(env_, message);
+    ThrowDataCloneException(context_, message);
   }
 
   Maybe<bool> WriteHostObject(Isolate* isolate, Local<Object> object) override {
@@ -309,7 +320,7 @@ Maybe<bool> Message::Serialize(Environment* env,
         if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
             array_buffers.end()) {
           ThrowDataCloneException(
-              env,
+              context,
               FIXED_ONE_BYTE_STRING(
                   env->isolate(),
                   "Transfer list contains duplicate ArrayBuffer"));
@@ -326,15 +337,15 @@ Maybe<bool> Message::Serialize(Environment* env,
         // Check if the source MessagePort is being transferred.
         if (!source_port.IsEmpty() && entry == source_port) {
           ThrowDataCloneException(
-              env,
+              context,
               FIXED_ONE_BYTE_STRING(env->isolate(),
                                     "Transfer list contains source port"));
           return Nothing<bool>();
         }
         MessagePort* port = Unwrap<MessagePort>(entry.As<Object>());
         if (port == nullptr || port->IsDetached()) {
           ThrowDataCloneException(
-              env,
+              context,
               FIXED_ONE_BYTE_STRING(
                   env->isolate(),
                   "MessagePort in transfer list is already detached"));
@@ -343,7 +354,7 @@ Maybe<bool> Message::Serialize(Environment* env,
         if (std::find(delegate.ports_.begin(), delegate.ports_.end(), port) !=
             delegate.ports_.end()) {
           ThrowDataCloneException(
-              env,
+              context,
               FIXED_ONE_BYTE_STRING(
                   env->isolate(),
                   "Transfer list contains duplicate MessagePort"));
@@ -811,13 +822,6 @@ static void MessageChannel(const FunctionCallbackInfo<Value>& args) {
       .FromJust();
 }
 
-static void RegisterDOMException(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  CHECK_EQ(args.Length(), 1);
-  CHECK(args[0]->IsFunction());
-  env->set_domexception_function(args[0].As<Function>());
-}
-
 static void InitMessaging(Local<Object> target,
                           Local<Value> unused,
                           Local<Context> context,
@@ -839,8 +843,6 @@ static void InitMessaging(Local<Object> target,
               GetMessagePortConstructor(env, context).ToLocalChecked())
                   .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);

diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js
@@ -17,7 +17,6 @@ const expectedModules = new Set([
   'Internal Binding credentials',
   'Internal Binding fs',
   'Internal Binding inspector',
-  'Internal Binding messaging',
   'Internal Binding module_wrap',
   'Internal Binding native_module',
   'Internal Binding options',
@@ -38,7 +37,6 @@ const expectedModules = new Set([
   'NativeModule internal/console/constructor',
   'NativeModule internal/console/global',
   'NativeModule internal/constants',
-  'NativeModule internal/domexception',
   'NativeModule internal/encoding',
   'NativeModule internal/errors',
   'NativeModule internal/fixed_queue',
@@ -74,6 +72,7 @@ if (common.isMainThread) {
   expectedModules.add('NativeModule internal/process/stdio');
 } else {
   expectedModules.add('Internal Binding heap_utils');
+  expectedModules.add('Internal Binding messaging');
   expectedModules.add('Internal Binding serdes');
   expectedModules.add('Internal Binding stream_wrap');
   expectedModules.add('Internal Binding symbols');

diff --git a/test/wpt/test-url.js b/test/wpt/test-url.js
@@ -3,16 +3,30 @@
 // Flags: --expose-internals
 
 require('../common');
+const assert = require('assert');
 const { WPTRunner } = require('../common/wpt');
 
 const runner = new WPTRunner('url');
 
 // Copy global descriptors from the global object
 runner.copyGlobalsFromObject(global, ['URL', 'URLSearchParams']);
 // Needed by urlsearchparams-constructor.any.js
+let DOMException;
 runner.defineGlobal('DOMException', {
   get() {
-    return require('internal/domexception');
+    // A 'hack' to get the DOMException constructor since we don't have it
+    // on the global object.
+    if (DOMException === undefined) {
+      const port = new (require('worker_threads').MessagePort)();
+      const ab = new ArrayBuffer(1);
+      try {
+        port.postMessage(ab, [ab, ab]);
+      } catch (err) {
+        DOMException = err.constructor;
+      }
+      assert.strictEqual(DOMException.name, 'DOMException');
+    }
+    return DOMException;
   }
 });