From b0de48e85441ff710aab240fdfa8a34adbbee976 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 7 Mar 2019 15:45:31 +0100 Subject: 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 ---- lib/internal/domexception.js | 86 ----------------------------- lib/internal/per_context/domexception.js | 92 ++++++++++++++++++++++++++++++++ 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 +++++- 10 files changed, 168 insertions(+), 121 deletions(-) delete mode 100644 lib/internal/domexception.js create mode 100644 lib/internal/per_context/domexception.js diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index 80073ebe61..1b07fa5a81 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 0bd1a3f363..f8cddb3153 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -214,8 +214,6 @@ if (!config.noBrowserGlobals) { defineOperation(global, 'setImmediate', timers.setImmediate); } -setupDOMException(); - // process.allowedNodeEnvironmentFlags Object.defineProperty(process, 'allowedNodeEnvironmentFlags', { get() { @@ -394,13 +392,6 @@ function createGlobalConsole(consoleFromVM) { return consoleFromNode; } -function setupDOMException() { - // Registers the constructor with C++. - const DOMException = NativeModule.require('internal/domexception'); - const { registerDOMException } = internalBinding('messaging'); - registerDOMException(DOMException); -} - // https://heycam.github.io/webidl/#es-namespaces function exposeNamespace(target, name, namespaceObject) { Object.defineProperty(target, name, { diff --git a/lib/internal/domexception.js b/lib/internal/domexception.js deleted file mode 100644 index 9845919e49..0000000000 --- a/lib/internal/domexception.js +++ /dev/null @@ -1,86 +0,0 @@ -'use strict'; - -const { ERR_INVALID_THIS } = require('internal/errors').codes; - -const internalsMap = new WeakMap(); - -const nameToCodeMap = new Map(); - -class DOMException extends Error { - constructor(message = '', name = 'Error') { - super(); - internalsMap.set(this, { - message: `${message}`, - name: `${name}` - }); - } - - get name() { - const internals = internalsMap.get(this); - if (internals === undefined) { - throw new ERR_INVALID_THIS('DOMException'); - } - return internals.name; - } - - get message() { - const internals = internalsMap.get(this); - if (internals === undefined) { - throw new ERR_INVALID_THIS('DOMException'); - } - return internals.message; - } - - get code() { - const internals = internalsMap.get(this); - if (internals === undefined) { - throw new ERR_INVALID_THIS('DOMException'); - } - const code = nameToCodeMap.get(internals.name); - return code === undefined ? 0 : code; - } -} - -Object.defineProperties(DOMException.prototype, { - [Symbol.toStringTag]: { configurable: true, value: 'DOMException' }, - name: { enumerable: true, configurable: true }, - message: { enumerable: true, configurable: true }, - code: { enumerable: true, configurable: true } -}); - -for (const [name, codeName, value] of [ - ['IndexSizeError', 'INDEX_SIZE_ERR', 1], - ['DOMStringSizeError', 'DOMSTRING_SIZE_ERR', 2], - ['HierarchyRequestError', 'HIERARCHY_REQUEST_ERR', 3], - ['WrongDocumentError', 'WRONG_DOCUMENT_ERR', 4], - ['InvalidCharacterError', 'INVALID_CHARACTER_ERR', 5], - ['NoDataAllowedError', 'NO_DATA_ALLOWED_ERR', 6], - ['NoModificationAllowedError', 'NO_MODIFICATION_ALLOWED_ERR', 7], - ['NotFoundError', 'NOT_FOUND_ERR', 8], - ['NotSupportedError', 'NOT_SUPPORTED_ERR', 9], - ['InUseAttributeError', 'INUSE_ATTRIBUTE_ERR', 10], - ['InvalidStateError', 'INVALID_STATE_ERR', 11], - ['SyntaxError', 'SYNTAX_ERR', 12], - ['InvalidModificationError', 'INVALID_MODIFICATION_ERR', 13], - ['NamespaceError', 'NAMESPACE_ERR', 14], - ['InvalidAccessError', 'INVALID_ACCESS_ERR', 15], - ['ValidationError', 'VALIDATION_ERR', 16], - ['TypeMismatchError', 'TYPE_MISMATCH_ERR', 17], - ['SecurityError', 'SECURITY_ERR', 18], - ['NetworkError', 'NETWORK_ERR', 19], - ['AbortError', 'ABORT_ERR', 20], - ['URLMismatchError', 'URL_MISMATCH_ERR', 21], - ['QuotaExceededError', 'QUOTA_EXCEEDED_ERR', 22], - ['TimeoutError', 'TIMEOUT_ERR', 23], - ['InvalidNodeTypeError', 'INVALID_NODE_TYPE_ERR', 24], - ['DataCloneError', 'DATA_CLONE_ERR', 25] - // There are some more error names, but since they don't have codes assigned, - // we don't need to care about them. -]) { - const desc = { enumerable: true, value }; - Object.defineProperty(DOMException, codeName, desc); - Object.defineProperty(DOMException.prototype, codeName, desc); - nameToCodeMap.set(name, value); -} - -module.exports = DOMException; diff --git a/lib/internal/per_context/domexception.js b/lib/internal/per_context/domexception.js new file mode 100644 index 0000000000..795acf76c0 --- /dev/null +++ b/lib/internal/per_context/domexception.js @@ -0,0 +1,92 @@ +'use strict'; + +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(); + +const nameToCodeMap = new Map(); + +class DOMException extends Error { + constructor(message = '', name = 'Error') { + super(); + internalsMap.set(this, { + message: `${message}`, + name: `${name}` + }); + } + + get name() { + const internals = internalsMap.get(this); + if (internals === undefined) { + throw new ERR_INVALID_THIS('DOMException'); + } + return internals.name; + } + + get message() { + const internals = internalsMap.get(this); + if (internals === undefined) { + throw new ERR_INVALID_THIS('DOMException'); + } + return internals.message; + } + + get code() { + const internals = internalsMap.get(this); + if (internals === undefined) { + throw new ERR_INVALID_THIS('DOMException'); + } + const code = nameToCodeMap.get(internals.name); + return code === undefined ? 0 : code; + } +} + +Object.defineProperties(DOMException.prototype, { + [Symbol.toStringTag]: { configurable: true, value: 'DOMException' }, + name: { enumerable: true, configurable: true }, + message: { enumerable: true, configurable: true }, + code: { enumerable: true, configurable: true } +}); + +for (const [name, codeName, value] of [ + ['IndexSizeError', 'INDEX_SIZE_ERR', 1], + ['DOMStringSizeError', 'DOMSTRING_SIZE_ERR', 2], + ['HierarchyRequestError', 'HIERARCHY_REQUEST_ERR', 3], + ['WrongDocumentError', 'WRONG_DOCUMENT_ERR', 4], + ['InvalidCharacterError', 'INVALID_CHARACTER_ERR', 5], + ['NoDataAllowedError', 'NO_DATA_ALLOWED_ERR', 6], + ['NoModificationAllowedError', 'NO_MODIFICATION_ALLOWED_ERR', 7], + ['NotFoundError', 'NOT_FOUND_ERR', 8], + ['NotSupportedError', 'NOT_SUPPORTED_ERR', 9], + ['InUseAttributeError', 'INUSE_ATTRIBUTE_ERR', 10], + ['InvalidStateError', 'INVALID_STATE_ERR', 11], + ['SyntaxError', 'SYNTAX_ERR', 12], + ['InvalidModificationError', 'INVALID_MODIFICATION_ERR', 13], + ['NamespaceError', 'NAMESPACE_ERR', 14], + ['InvalidAccessError', 'INVALID_ACCESS_ERR', 15], + ['ValidationError', 'VALIDATION_ERR', 16], + ['TypeMismatchError', 'TYPE_MISMATCH_ERR', 17], + ['SecurityError', 'SECURITY_ERR', 18], + ['NetworkError', 'NETWORK_ERR', 19], + ['AbortError', 'ABORT_ERR', 20], + ['URLMismatchError', 'URL_MISMATCH_ERR', 21], + ['QuotaExceededError', 'QUOTA_EXCEEDED_ERR', 22], + ['TimeoutError', 'TIMEOUT_ERR', 23], + ['InvalidNodeTypeError', 'INVALID_NODE_TYPE_ERR', 24], + ['DataCloneError', 'DATA_CLONE_ERR', 25] + // There are some more error names, but since they don't have codes assigned, + // we don't need to care about them. +]) { + const desc = { enumerable: true, value }; + Object.defineProperty(DOMException, codeName, desc); + Object.defineProperty(DOMException.prototype, codeName, desc); + nameToCodeMap.set(name, value); +} + +exports.DOMException = DOMException; diff --git a/node.gyp b/node.gyp index 4fbdb58183..0c6aee7f80 100644 --- a/node.gyp +++ b/node.gyp @@ -32,6 +32,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', @@ -115,7 +116,6 @@ 'lib/internal/dgram.js', 'lib/internal/dns/promises.js', 'lib/internal/dns/utils.js', - 'lib/internal/domexception.js', 'lib/internal/dtrace.js', 'lib/internal/encoding.js', 'lib/internal/errors.js', diff --git a/src/api/environment.cc b/src/api/environment.cc index c4a8120d1c..6e3c61fac1 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; @@ -279,6 +282,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); @@ -291,16 +314,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 d2eadf5109..bc6a36d9db 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -296,6 +296,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 profiler { void StartCoverageCollection(Environment* env); } diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 650d64b395..9b9bab0814 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -177,19 +177,30 @@ uint32_t Message::AddWASMModule(WasmModuleObject::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 efae4725cf..e65098b922 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 8734452940..4b909988dd 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; } }); -- cgit v1.2.3