From 1e669b2e2ea92672813060ab65287b4b6775604c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 7 Mar 2019 15:45:31 +0100 Subject: [PATCH] src,lib: make DOMException available in all Contexts This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: https://github.com/nodejs/node/pull/26497 Reviewed-By: James M Snell --- lib/internal/bootstrap/cache.js | 1 + lib/internal/bootstrap/node.js | 9 ---- .../{ => per_context}/domexception.js | 10 ++++- node.gyp | 2 +- src/api/environment.cc | 36 +++++++++++++++- src/node_internals.h | 2 + src/node_messaging.cc | 42 ++++++++++--------- test/parallel/test-bootstrap-modules.js | 3 +- test/wpt/test-url.js | 16 ++++++- 9 files changed, 84 insertions(+), 37 deletions(-) rename lib/internal/{ => per_context}/domexception.js (93%) diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index f1386eaf460cc7..a68060c19643df 100644 --- 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 index bb076ba9694330..f4ddc48b4a49e0 100644 --- 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 b/lib/internal/per_context/domexception.js similarity index 93% rename from lib/internal/domexception.js rename to lib/internal/per_context/domexception.js index 9845919e498dae..795acf76c09303 100644 --- a/lib/internal/domexception.js +++ b/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 index e8f89139c316e6..8bc6c3d00de15b 100644 --- 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 index 4df339b2e69c8a..3476f4355b3084 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -15,6 +15,7 @@ namespace node { using v8::Context; +using v8::EscapableHandleScope; using v8::Function; using v8::HandleScope; using v8::Isolate; @@ -22,7 +23,9 @@ 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 GetPerContextExports(Local context) { + Isolate* isolate = context->GetIsolate(); + EscapableHandleScope handle_scope(isolate); + + Local global = context->Global(); + Local key = Private::ForApi(isolate, + FIXED_ONE_BYTE_STRING(isolate, "node:per_context_binding_exports")); + + Local existing_value; + if (!global->GetPrivate(context, key).ToLocal(&existing_value)) + return MaybeLocal(); + if (existing_value->IsObject()) + return handle_scope.Escape(existing_value.As()); + + Local exports = Object::New(isolate); + if (context->Global()->SetPrivate(context, key, exports).IsNothing()) + return MaybeLocal(); + return handle_scope.Escape(exports); +} + Local NewContext(Isolate* isolate, Local object_template) { auto context = Context::New(isolate, nullptr, object_template); @@ -261,16 +284,25 @@ Local NewContext(Isolate* isolate, { // Run per-context JS files. Context::Scope context_scope(context); + Local exports; + if (!GetPerContextExports(context).ToLocal(&exports)) + return Local(); + + Local global_string = FIXED_ONE_BYTE_STRING(isolate, "global"); + Local 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> parameters = { - FIXED_ONE_BYTE_STRING(isolate, "global")}; - Local arguments[] = {context->Global()}; + global_string, + exports_string + }; + Local arguments[] = {context->Global(), exports}; MaybeLocal maybe_fn = per_process::native_module_loader.LookupAndCompile( context, *module, ¶meters, nullptr); diff --git a/src/node_internals.h b/src/node_internals.h index 59e95dca8e6e5b..11ad197ab51a0a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -294,6 +294,8 @@ void DefineZlibConstants(v8::Local target); v8::MaybeLocal RunBootstrapping(Environment* env); v8::MaybeLocal StartExecution(Environment* env, const char* main_script_id); +v8::MaybeLocal GetPerContextExports(v8::Local context); + namespace coverage { bool StartCoverageCollection(Environment* env); } diff --git a/src/node_messaging.cc b/src/node_messaging.cc index e6e7f743de27ad..8e7e25ae567288 100644 --- 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 message) { +void ThrowDataCloneException(Local context, Local message) { + Isolate* isolate = context->GetIsolate(); Local argv[] = { message, - FIXED_ONE_BYTE_STRING(env->isolate(), "DataCloneError") + FIXED_ONE_BYTE_STRING(isolate, "DataCloneError") }; Local exception; - Local domexception_ctor = env->domexception_function(); - CHECK(!domexception_ctor.IsEmpty()); - if (!domexception_ctor->NewInstance(env->context(), arraysize(argv), argv) + + Local per_context_bindings; + Local 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 domexception_ctor = domexception_ctor_val.As(); + 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 message) override { - ThrowDataCloneException(env_, message); + ThrowDataCloneException(context_, message); } Maybe WriteHostObject(Isolate* isolate, Local object) override { @@ -309,7 +320,7 @@ Maybe 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,7 +337,7 @@ Maybe 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(); @@ -334,7 +345,7 @@ Maybe Message::Serialize(Environment* env, MessagePort* port = Unwrap(entry.As()); 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 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& args) { .FromJust(); } -static void RegisterDOMException(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsFunction()); - env->set_domexception_function(args[0].As()); -} - static void InitMessaging(Local target, Local unused, Local context, @@ -839,8 +843,6 @@ static void InitMessaging(Local 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 index b3ccc14be8c53f..4cb541358965a5 100644 --- 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 index 8734452940e84e..4b909988ddb5ee 100644 --- a/test/wpt/test-url.js +++ b/test/wpt/test-url.js @@ -3,6 +3,7 @@ // Flags: --expose-internals require('../common'); +const assert = require('assert'); const { WPTRunner } = require('../common/wpt'); const runner = new WPTRunner('url'); @@ -10,9 +11,22 @@ 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; } });