From 0df05684f094dca2b92fc61d037d8c767adb3472 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 00:15:33 +0100 Subject: [PATCH 01/12] src: port `defineLazyProperties` to native code This allows us to have getters not observable from JS side. --- .../bootstrap/web/exposed-wildcard.js | 12 +----- lib/internal/util.js | 41 +------------------ src/node_messaging.cc | 27 ++++++++++++ src/node_util.cc | 38 +++++++++++++++++ test/common/wpt.js | 27 ------------ test/wpt/test-domexception.js | 2 - 6 files changed, 68 insertions(+), 79 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-wildcard.js b/lib/internal/bootstrap/web/exposed-wildcard.js index d8f451edb01205..9d83195db5c95e 100644 --- a/lib/internal/bootstrap/web/exposed-wildcard.js +++ b/lib/internal/bootstrap/web/exposed-wildcard.js @@ -18,6 +18,7 @@ const { exposeNamespace, } = require('internal/util'); const config = internalBinding('config'); +const {exposeLazyDOMExceptionProperty} = internalBinding('messaging'); // https://console.spec.whatwg.org/#console-namespace exposeNamespace(globalThis, 'console', @@ -28,16 +29,7 @@ const { URL, URLSearchParams } = require('internal/url'); exposeInterface(globalThis, 'URL', URL); // https://url.spec.whatwg.org/#urlsearchparams exposeInterface(globalThis, 'URLSearchParams', URLSearchParams); -exposeGetterAndSetter(globalThis, - 'DOMException', - () => { - const DOMException = lazyDOMExceptionClass(); - exposeInterface(globalThis, 'DOMException', DOMException); - return DOMException; - }, - (value) => { - exposeInterface(globalThis, 'DOMException', value); - }); +exposeLazyDOMExceptionProperty(globalThis); // https://dom.spec.whatwg.org/#interface-abortcontroller // Lazy ones. diff --git a/lib/internal/util.js b/lib/internal/util.js index 887ae8874e15fc..e690d7f4323bcc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -55,6 +55,7 @@ const { const { signals } = internalBinding('constants').os; const { guessHandleType: _guessHandleType, + defineLazyProperties, privateSymbols: { arrow_message_private_symbol, decorated_private_symbol, @@ -610,46 +611,6 @@ function exposeGetterAndSetter(target, name, getter, setter = undefined) { }); } -function defineLazyProperties(target, id, keys, enumerable = true) { - const descriptors = { __proto__: null }; - let mod; - for (let i = 0; i < keys.length; i++) { - const key = keys[i]; - let lazyLoadedValue; - function set(value) { - ObjectDefineProperty(target, key, { - __proto__: null, - writable: true, - value, - }); - } - ObjectDefineProperty(set, 'name', { - __proto__: null, - value: `set ${key}`, - }); - function get() { - mod ??= require(id); - if (lazyLoadedValue === undefined) { - lazyLoadedValue = mod[key]; - set(lazyLoadedValue); - } - return lazyLoadedValue; - } - ObjectDefineProperty(get, 'name', { - __proto__: null, - value: `get ${key}`, - }); - descriptors[key] = { - __proto__: null, - configurable: true, - enumerable, - get, - set, - }; - } - ObjectDefineProperties(target, descriptors); -} - function defineReplaceableLazyAttribute(target, id, keys, writable = true, check) { let mod; for (let i = 0; i < keys.length; i++) { diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 73c0c38dc7bf45..eac0e4f62064e1 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1645,6 +1645,32 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } } + +static void ExposeLazyDOMExceptionPropertyGetter(v8::Local name, const v8::PropertyCallbackInfo& info) { + Realm* realm = Realm::GetCurrent(info); + Isolate* isolate = realm->isolate(); + auto context = isolate->GetCurrentContext(); + Local domexception = GetDOMException(context).ToLocalChecked(); + info.GetReturnValue().Set(domexception); +} +static void ExposeLazyDOMExceptionProperty(const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 1); // target[, enumerable = true] + CHECK(args[0]->IsObject()); // target: Object where to define the lazy properties. + // CHECK(args.Length() == 1 || args[1]->IsBoolean()); // enumerable: Whether the property should be enumerable. + + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + auto target = args[0].As(); + // v8::PropertyAttribute attribute = args.Length() == 1 || args[1]->IsTrue() ? v8::None : ; + target->SetLazyDataProperty( + isolate->GetCurrentContext(), + FIXED_ONE_BYTE_STRING(isolate, "DOMException"), + ExposeLazyDOMExceptionPropertyGetter, + Null(isolate), + v8::DontEnum + ).Check(); +} + static void CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); @@ -1671,6 +1697,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, // These are not methods on the MessagePort prototype, because // the browser equivalents do not provide them. + SetMethod(isolate, target, "exposeLazyDOMExceptionProperty", ExposeLazyDOMExceptionProperty); SetMethod(isolate, target, "stopMessagePort", MessagePort::Stop); SetMethod(isolate, target, "checkMessagePort", MessagePort::CheckType); SetMethod(isolate, target, "drainMessagePort", MessagePort::Drain); diff --git a/src/node_util.cc b/src/node_util.cc index c855201d8938fd..25e3189582a43a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -348,6 +348,43 @@ static void IsInsideNodeModules(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } +static void DefineLazyPropertiesGetter(v8::Local name, const v8::PropertyCallbackInfo& info) { + Realm* realm = Realm::GetCurrent(info); + Isolate* isolate = realm->isolate(); + auto context = isolate->GetCurrentContext(); + Local arg = info.Data(); + auto require_result = realm->builtin_module_require() + ->Call(context, Null(isolate), 1, &arg).ToLocalChecked(); + info.GetReturnValue().Set(require_result.As()->Get(context, name).ToLocalChecked()); +} +static void DefineLazyProperties(const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 3); // target, id, keys, enumerable = true + CHECK(args[0]->IsObject()); // target: Object where to define the lazy properties. + CHECK(args[1]->IsString()); // id: Internal module to lazy-load where the API to expose are implemented. + CHECK(args[2]->IsArray()); // keys: Keys to map from `require(id)` and `target`. + CHECK(args.Length() == 3 || args[3]->IsBoolean()); // enumerable: Whether the property should be enumerable. + + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + auto context = isolate->GetCurrentContext(); + + auto target = args[0].As(); + Local id = args[1]; + v8::PropertyAttribute attribute = args.Length() == 3 || args[3]->IsTrue() ? v8::None : v8::DontEnum; + + const Local keys = args[2].As(); + size_t length = keys->Length(); + for(size_t i = 0; i < length; i++) { + Local key; + if (!keys->Get(context, i).ToLocal(&key)) { + // V8 will have scheduled an error to be thrown. + return; + } + CHECK(key->IsString()); + target->SetLazyDataProperty(context, key.As(), DefineLazyPropertiesGetter, id, attribute).Check(); + } +} + void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetPromiseDetails); registry->Register(GetProxyDetails); @@ -448,6 +485,7 @@ void Initialize(Local target, } SetMethod(context, target, "isInsideNodeModules", IsInsideNodeModules); + SetMethod(context, target, "defineLazyProperties", DefineLazyProperties); SetMethodNoSideEffect( context, target, "getPromiseDetails", GetPromiseDetails); SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails); diff --git a/test/common/wpt.js b/test/common/wpt.js index 3ead9cb23b9c46..74b182e5b84296 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -595,7 +595,6 @@ class WPTRunner { switch (name) { case 'Window': { this.globalThisInitScripts.push('globalThis.Window = Object.getPrototypeOf(globalThis).constructor;'); - this.loadLazyGlobals(); break; } @@ -609,32 +608,6 @@ class WPTRunner { } } - loadLazyGlobals() { - const lazyProperties = [ - 'DOMException', - 'Performance', 'PerformanceEntry', 'PerformanceMark', 'PerformanceMeasure', - 'PerformanceObserver', 'PerformanceObserverEntryList', 'PerformanceResourceTiming', - 'Blob', 'atob', 'btoa', - 'MessageChannel', 'MessagePort', 'MessageEvent', - 'EventTarget', 'Event', - 'AbortController', 'AbortSignal', - 'performance', - 'TransformStream', 'TransformStreamDefaultController', - 'WritableStream', 'WritableStreamDefaultController', 'WritableStreamDefaultWriter', - 'ReadableStream', 'ReadableStreamDefaultReader', - 'ReadableStreamBYOBReader', 'ReadableStreamBYOBRequest', - 'ReadableByteStreamController', 'ReadableStreamDefaultController', - 'ByteLengthQueuingStrategy', 'CountQueuingStrategy', - 'TextEncoder', 'TextDecoder', 'TextEncoderStream', 'TextDecoderStream', - 'CompressionStream', 'DecompressionStream', - ]; - if (Boolean(process.versions.openssl) && !process.env.NODE_SKIP_CRYPTO) { - lazyProperties.push('crypto', 'Crypto', 'CryptoKey', 'SubtleCrypto'); - } - const script = lazyProperties.map((name) => `globalThis.${name};`).join('\n'); - this.globalThisInitScripts.push(script); - } - // TODO(joyeecheung): work with the upstream to port more tests in .html // to .js. async runJsTests() { diff --git a/test/wpt/test-domexception.js b/test/wpt/test-domexception.js index 7900d1ca9a79c6..8fa93bc59bdf44 100644 --- a/test/wpt/test-domexception.js +++ b/test/wpt/test-domexception.js @@ -4,6 +4,4 @@ const { WPTRunner } = require('../common/wpt'); const runner = new WPTRunner('webidl/ecmascript-binding/es-exceptions'); -runner.loadLazyGlobals(); - runner.runJsTests(); From 2f9a8b746f68ba9836b449931822c325185c2db5 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 00:50:15 +0100 Subject: [PATCH 02/12] fixup! src: port `defineLazyProperties` to native code --- lib/internal/bootstrap/web/exposed-wildcard.js | 4 +--- src/node_messaging.cc | 18 ++++++++---------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-wildcard.js b/lib/internal/bootstrap/web/exposed-wildcard.js index 9d83195db5c95e..4b3d6be0701ec4 100644 --- a/lib/internal/bootstrap/web/exposed-wildcard.js +++ b/lib/internal/bootstrap/web/exposed-wildcard.js @@ -12,13 +12,11 @@ const { const { exposeInterface, - lazyDOMExceptionClass, exposeLazyInterfaces, - exposeGetterAndSetter, exposeNamespace, } = require('internal/util'); const config = internalBinding('config'); -const {exposeLazyDOMExceptionProperty} = internalBinding('messaging'); +const { exposeLazyDOMExceptionProperty } = internalBinding('messaging'); // https://console.spec.whatwg.org/#console-namespace exposeNamespace(globalThis, 'console', diff --git a/src/node_messaging.cc b/src/node_messaging.cc index eac0e4f62064e1..b374a1234162d1 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1646,22 +1646,20 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } -static void ExposeLazyDOMExceptionPropertyGetter(v8::Local name, const v8::PropertyCallbackInfo& info) { - Realm* realm = Realm::GetCurrent(info); - Isolate* isolate = realm->isolate(); - auto context = isolate->GetCurrentContext(); +static void ExposeLazyDOMExceptionPropertyGetter( + v8::Local name, const v8::PropertyCallbackInfo& info) { + auto context = info.GetIsolate()->GetCurrentContext(); Local domexception = GetDOMException(context).ToLocalChecked(); info.GetReturnValue().Set(domexception); } -static void ExposeLazyDOMExceptionProperty(const FunctionCallbackInfo& args) { - CHECK_GE(args.Length(), 1); // target[, enumerable = true] +static void ExposeLazyDOMExceptionProperty( + const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 1); // target CHECK(args[0]->IsObject()); // target: Object where to define the lazy properties. - // CHECK(args.Length() == 1 || args[1]->IsBoolean()); // enumerable: Whether the property should be enumerable. - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); + Isolate* isolate = args.GetIsolate(); auto target = args[0].As(); - // v8::PropertyAttribute attribute = args.Length() == 1 || args[1]->IsTrue() ? v8::None : ; + target->SetLazyDataProperty( isolate->GetCurrentContext(), FIXED_ONE_BYTE_STRING(isolate, "DOMException"), From cbe1f31f6563c77a096c1863240b7536f01fdc47 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 00:56:07 +0100 Subject: [PATCH 03/12] fixup! src: port `defineLazyProperties` to native code --- src/node_messaging.cc | 28 ++++++++++++++++------------ src/node_util.cc | 40 +++++++++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index b374a1234162d1..6e18b8196cda45 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1645,7 +1645,6 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } } - static void ExposeLazyDOMExceptionPropertyGetter( v8::Local name, const v8::PropertyCallbackInfo& info) { auto context = info.GetIsolate()->GetCurrentContext(); @@ -1654,19 +1653,21 @@ static void ExposeLazyDOMExceptionPropertyGetter( } static void ExposeLazyDOMExceptionProperty( const FunctionCallbackInfo& args) { - CHECK_GE(args.Length(), 1); // target - CHECK(args[0]->IsObject()); // target: Object where to define the lazy properties. + CHECK_GE(args.Length(), 1); // target + CHECK( + args[0] + ->IsObject()); // target: Object where to define the lazy properties. Isolate* isolate = args.GetIsolate(); auto target = args[0].As(); - - target->SetLazyDataProperty( - isolate->GetCurrentContext(), - FIXED_ONE_BYTE_STRING(isolate, "DOMException"), - ExposeLazyDOMExceptionPropertyGetter, - Null(isolate), - v8::DontEnum - ).Check(); + + target + ->SetLazyDataProperty(isolate->GetCurrentContext(), + FIXED_ONE_BYTE_STRING(isolate, "DOMException"), + ExposeLazyDOMExceptionPropertyGetter, + Null(isolate), + v8::DontEnum) + .Check(); } static void CreatePerIsolateProperties(IsolateData* isolate_data, @@ -1693,9 +1694,12 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, isolate_data->message_port_constructor_string(), GetMessagePortConstructorTemplate(isolate_data)); + SetMethod(isolate, + target, + "exposeLazyDOMExceptionProperty", + ExposeLazyDOMExceptionProperty); // These are not methods on the MessagePort prototype, because // the browser equivalents do not provide them. - SetMethod(isolate, target, "exposeLazyDOMExceptionProperty", ExposeLazyDOMExceptionProperty); SetMethod(isolate, target, "stopMessagePort", MessagePort::Stop); SetMethod(isolate, target, "checkMessagePort", MessagePort::CheckType); SetMethod(isolate, target, "drainMessagePort", MessagePort::Drain); diff --git a/src/node_util.cc b/src/node_util.cc index 25e3189582a43a..f004152b3f3167 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -348,21 +348,32 @@ static void IsInsideNodeModules(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } -static void DefineLazyPropertiesGetter(v8::Local name, const v8::PropertyCallbackInfo& info) { +static void DefineLazyPropertiesGetter( + v8::Local name, const v8::PropertyCallbackInfo& info) { Realm* realm = Realm::GetCurrent(info); Isolate* isolate = realm->isolate(); auto context = isolate->GetCurrentContext(); Local arg = info.Data(); auto require_result = realm->builtin_module_require() - ->Call(context, Null(isolate), 1, &arg).ToLocalChecked(); - info.GetReturnValue().Set(require_result.As()->Get(context, name).ToLocalChecked()); + ->Call(context, Null(isolate), 1, &arg) + .ToLocalChecked(); + info.GetReturnValue().Set( + require_result.As()->Get(context, name).ToLocalChecked()); } static void DefineLazyProperties(const FunctionCallbackInfo& args) { - CHECK_GE(args.Length(), 3); // target, id, keys, enumerable = true - CHECK(args[0]->IsObject()); // target: Object where to define the lazy properties. - CHECK(args[1]->IsString()); // id: Internal module to lazy-load where the API to expose are implemented. - CHECK(args[2]->IsArray()); // keys: Keys to map from `require(id)` and `target`. - CHECK(args.Length() == 3 || args[3]->IsBoolean()); // enumerable: Whether the property should be enumerable. + CHECK_GE( + args.Length(), + 3); // target: object, id: string, keys: string[][, enumerable = true] + CHECK( + args[0] + ->IsObject()); // target: Object where to define the lazy properties. + CHECK(args[1]->IsString()); // id: Internal module to lazy-load where the API + // to expose are implemented. + CHECK(args[2] + ->IsArray()); // keys: Keys to map from `require(id)` and `target`. + CHECK(args.Length() == 3 || + args[3]->IsBoolean()); // enumerable: Whether the property should be + // enumerable. Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -370,18 +381,25 @@ static void DefineLazyProperties(const FunctionCallbackInfo& args) { auto target = args[0].As(); Local id = args[1]; - v8::PropertyAttribute attribute = args.Length() == 3 || args[3]->IsTrue() ? v8::None : v8::DontEnum; + v8::PropertyAttribute attribute = + args.Length() == 3 || args[3]->IsTrue() ? v8::None : v8::DontEnum; const Local keys = args[2].As(); size_t length = keys->Length(); - for(size_t i = 0; i < length; i++) { + for (size_t i = 0; i < length; i++) { Local key; if (!keys->Get(context, i).ToLocal(&key)) { // V8 will have scheduled an error to be thrown. return; } CHECK(key->IsString()); - target->SetLazyDataProperty(context, key.As(), DefineLazyPropertiesGetter, id, attribute).Check(); + target + ->SetLazyDataProperty(context, + key.As(), + DefineLazyPropertiesGetter, + id, + attribute) + .Check(); } } From f0cb5d4e6f00b83b4e75bec103239b0ba0a1a592 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 01:09:05 +0100 Subject: [PATCH 04/12] fixup! src: port `defineLazyProperties` to native code --- src/node_messaging.cc | 8 +++----- src/node_util.cc | 23 ++++++++++------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 6e18b8196cda45..f2dd512dbcd844 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1653,10 +1653,8 @@ static void ExposeLazyDOMExceptionPropertyGetter( } static void ExposeLazyDOMExceptionProperty( const FunctionCallbackInfo& args) { - CHECK_GE(args.Length(), 1); // target - CHECK( - args[0] - ->IsObject()); // target: Object where to define the lazy properties. + CHECK_GE(args.Length(), 1); + CHECK(args[0]->IsObject()); Isolate* isolate = args.GetIsolate(); auto target = args[0].As(); @@ -1665,7 +1663,7 @@ static void ExposeLazyDOMExceptionProperty( ->SetLazyDataProperty(isolate->GetCurrentContext(), FIXED_ONE_BYTE_STRING(isolate, "DOMException"), ExposeLazyDOMExceptionPropertyGetter, - Null(isolate), + Local(), v8::DontEnum) .Check(); } diff --git a/src/node_util.cc b/src/node_util.cc index f004152b3f3167..b1b3d110a3048b 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -361,19 +361,16 @@ static void DefineLazyPropertiesGetter( require_result.As()->Get(context, name).ToLocalChecked()); } static void DefineLazyProperties(const FunctionCallbackInfo& args) { - CHECK_GE( - args.Length(), - 3); // target: object, id: string, keys: string[][, enumerable = true] - CHECK( - args[0] - ->IsObject()); // target: Object where to define the lazy properties. - CHECK(args[1]->IsString()); // id: Internal module to lazy-load where the API - // to expose are implemented. - CHECK(args[2] - ->IsArray()); // keys: Keys to map from `require(id)` and `target`. - CHECK(args.Length() == 3 || - args[3]->IsBoolean()); // enumerable: Whether the property should be - // enumerable. + // target: object, id: string, keys: string[][, enumerable = true] + CHECK_GE(args.Length(), 3); + // target: Object where to define the lazy properties. + CHECK(args[0]->IsObject()); + // id: Internal module to lazy-load where the API to expose are implemented. + CHECK(args[1]->IsString()); + // keys: Keys to map from `require(id)` and `target`. + CHECK(args[2]->IsArray()); + // enumerable: Whether the property should be enumerable. + CHECK(args.Length() == 3 || args[3]->IsBoolean()); Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); From b9a85f4e31c11f7f350b4469834ac18f4062014b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 01:20:51 +0100 Subject: [PATCH 05/12] Apply suggestions from code review Co-authored-by: James M Snell --- src/node_util.cc | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index b1b3d110a3048b..feafd10c9b5309 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -354,11 +354,16 @@ static void DefineLazyPropertiesGetter( Isolate* isolate = realm->isolate(); auto context = isolate->GetCurrentContext(); Local arg = info.Data(); - auto require_result = realm->builtin_module_require() - ->Call(context, Null(isolate), 1, &arg) - .ToLocalChecked(); - info.GetReturnValue().Set( - require_result.As()->Get(context, name).ToLocalChecked()); + Local require_result; + if (!realm->builtin_module_require() + ->Call(context, Null(isolate), 1, &arg) + .ToLocal(&require_result)) { + return; + } + Local ret; + if (require_result.As()->Get(context, name).ToLocal(&ret)) { + info.GetReturnValue().Set(ret); + } } static void DefineLazyProperties(const FunctionCallbackInfo& args) { // target: object, id: string, keys: string[][, enumerable = true] From 779253797011f887471a6050ce1afdb522a5fbd7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 01:22:43 +0100 Subject: [PATCH 06/12] Apply suggestions from code review --- src/node_util.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index feafd10c9b5309..61258df2c66d99 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -358,12 +358,15 @@ static void DefineLazyPropertiesGetter( if (!realm->builtin_module_require() ->Call(context, Null(isolate), 1, &arg) .ToLocal(&require_result)) { + // V8 will have scheduled an error to be thrown. return; } Local ret; - if (require_result.As()->Get(context, name).ToLocal(&ret)) { - info.GetReturnValue().Set(ret); + if (!require_result.As()->Get(context, name).ToLocal(&ret)) { + // V8 will have scheduled an error to be thrown. + return; } + info.GetReturnValue().Set(ret); } static void DefineLazyProperties(const FunctionCallbackInfo& args) { // target: object, id: string, keys: string[][, enumerable = true] From 71fec7a6f36e44bb84e2c99258f5f8ace9bfadbc Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 01:27:28 +0100 Subject: [PATCH 07/12] Apply suggestions from code review --- src/node_messaging.cc | 17 ++++++++++------- src/node_util.cc | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index f2dd512dbcd844..c841068377144f 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1659,13 +1659,16 @@ static void ExposeLazyDOMExceptionProperty( Isolate* isolate = args.GetIsolate(); auto target = args[0].As(); - target - ->SetLazyDataProperty(isolate->GetCurrentContext(), - FIXED_ONE_BYTE_STRING(isolate, "DOMException"), - ExposeLazyDOMExceptionPropertyGetter, - Local(), - v8::DontEnum) - .Check(); + if (target + ->SetLazyDataProperty(isolate->GetCurrentContext(), + FIXED_ONE_BYTE_STRING(isolate, "DOMException"), + ExposeLazyDOMExceptionPropertyGetter, + Local(), + v8::DontEnum) + .IsNothing()) { + // V8 will have scheduled an error to be thrown. + return; + } } static void CreatePerIsolateProperties(IsolateData* isolate_data, diff --git a/src/node_util.cc b/src/node_util.cc index 61258df2c66d99..36f45a116389ee 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -398,13 +398,16 @@ static void DefineLazyProperties(const FunctionCallbackInfo& args) { return; } CHECK(key->IsString()); - target - ->SetLazyDataProperty(context, - key.As(), - DefineLazyPropertiesGetter, - id, - attribute) - .Check(); + if (target + ->SetLazyDataProperty(context, + key.As(), + DefineLazyPropertiesGetter, + id, + attribute) + .IsNothing()) { + // V8 will have scheduled an error to be thrown. + return; + }; } } From 019fed5526e896082d639d6ee44d0780d4ab1391 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 10:00:00 +0100 Subject: [PATCH 08/12] f --- src/node_messaging.cc | 4 ++-- src/node_util.cc | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c841068377144f..d5b2dfa53ee001 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1646,7 +1646,7 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } static void ExposeLazyDOMExceptionPropertyGetter( - v8::Local name, const v8::PropertyCallbackInfo& info) { + Local name, const v8::PropertyCallbackInfo& info) { auto context = info.GetIsolate()->GetCurrentContext(); Local domexception = GetDOMException(context).ToLocalChecked(); info.GetReturnValue().Set(domexception); @@ -1657,7 +1657,7 @@ static void ExposeLazyDOMExceptionProperty( CHECK(args[0]->IsObject()); Isolate* isolate = args.GetIsolate(); - auto target = args[0].As(); + auto target = args[0].As(); if (target ->SetLazyDataProperty(isolate->GetCurrentContext(), diff --git a/src/node_util.cc b/src/node_util.cc index 36f45a116389ee..5a6f0d4bb13fc3 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -349,18 +349,18 @@ static void IsInsideNodeModules(const FunctionCallbackInfo& args) { } static void DefineLazyPropertiesGetter( - v8::Local name, const v8::PropertyCallbackInfo& info) { + Local name, const v8::PropertyCallbackInfo& info) { Realm* realm = Realm::GetCurrent(info); Isolate* isolate = realm->isolate(); auto context = isolate->GetCurrentContext(); Local arg = info.Data(); Local require_result; if (!realm->builtin_module_require() - ->Call(context, Null(isolate), 1, &arg) - .ToLocal(&require_result)) { - // V8 will have scheduled an error to be thrown. - return; - } + ->Call(context, Null(isolate), 1, &arg) + .ToLocal(&require_result)) { + // V8 will have scheduled an error to be thrown. + return; + } Local ret; if (!require_result.As()->Get(context, name).ToLocal(&ret)) { // V8 will have scheduled an error to be thrown. @@ -384,7 +384,7 @@ static void DefineLazyProperties(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); auto context = isolate->GetCurrentContext(); - auto target = args[0].As(); + auto target = args[0].As(); Local id = args[1]; v8::PropertyAttribute attribute = args.Length() == 3 || args[3]->IsTrue() ? v8::None : v8::DontEnum; @@ -400,7 +400,7 @@ static void DefineLazyProperties(const FunctionCallbackInfo& args) { CHECK(key->IsString()); if (target ->SetLazyDataProperty(context, - key.As(), + key.As(), DefineLazyPropertiesGetter, id, attribute) From c89ce290d984c83d89e8835f13cc4da22ba20f6a Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 10:08:49 +0100 Subject: [PATCH 09/12] f --- src/node_messaging.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index d5b2dfa53ee001..f2701dd4d03aa6 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1648,7 +1648,11 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { static void ExposeLazyDOMExceptionPropertyGetter( Local name, const v8::PropertyCallbackInfo& info) { auto context = info.GetIsolate()->GetCurrentContext(); - Local domexception = GetDOMException(context).ToLocalChecked(); + Local domexception; + if (!GetDOMException(context).ToLocal(&domexception)) { + // V8 will have scheduled an error to be thrown. + return; + } info.GetReturnValue().Set(domexception); } static void ExposeLazyDOMExceptionProperty( From af7bb75f8c4d411ca43135307feb512bad393c64 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 16 Feb 2025 13:42:17 +0100 Subject: [PATCH 10/12] declare when not building snapshot --- .../bootstrap/web/exposed-wildcard.js | 3 ++- lib/internal/util.js | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-wildcard.js b/lib/internal/bootstrap/web/exposed-wildcard.js index 4b3d6be0701ec4..e3342e1ce2479a 100644 --- a/lib/internal/bootstrap/web/exposed-wildcard.js +++ b/lib/internal/bootstrap/web/exposed-wildcard.js @@ -11,6 +11,7 @@ const { } = primordials; const { + doWhenNotBUildingSnapshot, exposeInterface, exposeLazyInterfaces, exposeNamespace, @@ -27,7 +28,7 @@ const { URL, URLSearchParams } = require('internal/url'); exposeInterface(globalThis, 'URL', URL); // https://url.spec.whatwg.org/#urlsearchparams exposeInterface(globalThis, 'URLSearchParams', URLSearchParams); -exposeLazyDOMExceptionProperty(globalThis); +doWhenNotBUildingSnapshot(() => exposeLazyDOMExceptionProperty(globalThis)); // https://dom.spec.whatwg.org/#interface-abortcontroller // Lazy ones. diff --git a/lib/internal/util.js b/lib/internal/util.js index e690d7f4323bcc..2edb7b2bf61241 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -55,7 +55,7 @@ const { const { signals } = internalBinding('constants').os; const { guessHandleType: _guessHandleType, - defineLazyProperties, + defineLazyProperties: _defineLazyProperties, privateSymbols: { arrow_message_private_symbol, decorated_private_symbol, @@ -66,6 +66,10 @@ const { isNativeError, isPromise } = internalBinding('types'); const { getOptionValue } = require('internal/options'); const assert = require('internal/assert'); const { encodings } = internalBinding('string_decoder'); +const { + isBuildingSnapshot, + addDeserializeCallback, +} = require('internal/v8/startup_snapshot').namespace; const noCrypto = !process.versions.openssl; const noTypeScript = !process.versions.amaro; @@ -611,6 +615,18 @@ function exposeGetterAndSetter(target, name, getter, setter = undefined) { }); } +function doWhenNotBUildingSnapshot(fn) { + if (isBuildingSnapshot()) { + addDeserializeCallback(fn); + } else { + fn(); + } +} + +function defineLazyProperties(...args) { + doWhenNotBUildingSnapshot(() => ReflectApply(_defineLazyProperties, this, args)); +} + function defineReplaceableLazyAttribute(target, id, keys, writable = true, check) { let mod; for (let i = 0; i < keys.length; i++) { @@ -852,6 +868,7 @@ module.exports = { defineReplaceableLazyAttribute, deprecate, deprecateInstantiation, + doWhenNotBUildingSnapshot, emitExperimentalWarning, encodingsMap, exposeInterface, From fde628107400e89ce4a7fe891f830c6ee244f74f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 17 Feb 2025 21:52:22 +0100 Subject: [PATCH 11/12] Revert "declare when not building snapshot" This reverts commit af7bb75f8c4d411ca43135307feb512bad393c64. --- .../bootstrap/web/exposed-wildcard.js | 3 +-- lib/internal/util.js | 19 +------------------ 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-wildcard.js b/lib/internal/bootstrap/web/exposed-wildcard.js index e3342e1ce2479a..4b3d6be0701ec4 100644 --- a/lib/internal/bootstrap/web/exposed-wildcard.js +++ b/lib/internal/bootstrap/web/exposed-wildcard.js @@ -11,7 +11,6 @@ const { } = primordials; const { - doWhenNotBUildingSnapshot, exposeInterface, exposeLazyInterfaces, exposeNamespace, @@ -28,7 +27,7 @@ const { URL, URLSearchParams } = require('internal/url'); exposeInterface(globalThis, 'URL', URL); // https://url.spec.whatwg.org/#urlsearchparams exposeInterface(globalThis, 'URLSearchParams', URLSearchParams); -doWhenNotBUildingSnapshot(() => exposeLazyDOMExceptionProperty(globalThis)); +exposeLazyDOMExceptionProperty(globalThis); // https://dom.spec.whatwg.org/#interface-abortcontroller // Lazy ones. diff --git a/lib/internal/util.js b/lib/internal/util.js index 2edb7b2bf61241..e690d7f4323bcc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -55,7 +55,7 @@ const { const { signals } = internalBinding('constants').os; const { guessHandleType: _guessHandleType, - defineLazyProperties: _defineLazyProperties, + defineLazyProperties, privateSymbols: { arrow_message_private_symbol, decorated_private_symbol, @@ -66,10 +66,6 @@ const { isNativeError, isPromise } = internalBinding('types'); const { getOptionValue } = require('internal/options'); const assert = require('internal/assert'); const { encodings } = internalBinding('string_decoder'); -const { - isBuildingSnapshot, - addDeserializeCallback, -} = require('internal/v8/startup_snapshot').namespace; const noCrypto = !process.versions.openssl; const noTypeScript = !process.versions.amaro; @@ -615,18 +611,6 @@ function exposeGetterAndSetter(target, name, getter, setter = undefined) { }); } -function doWhenNotBUildingSnapshot(fn) { - if (isBuildingSnapshot()) { - addDeserializeCallback(fn); - } else { - fn(); - } -} - -function defineLazyProperties(...args) { - doWhenNotBUildingSnapshot(() => ReflectApply(_defineLazyProperties, this, args)); -} - function defineReplaceableLazyAttribute(target, id, keys, writable = true, check) { let mod; for (let i = 0; i < keys.length; i++) { @@ -868,7 +852,6 @@ module.exports = { defineReplaceableLazyAttribute, deprecate, deprecateInstantiation, - doWhenNotBUildingSnapshot, emitExperimentalWarning, encodingsMap, exposeInterface, From 55cf9bb12436fdf5b3dd979867b4145c7e10c148 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 17 Feb 2025 22:00:47 +0100 Subject: [PATCH 12/12] RegisterExternalReferences --- src/node_messaging.cc | 2 ++ src/node_util.cc | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index f2701dd4d03aa6..1bdd3bfe117240 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1748,6 +1748,8 @@ static void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(MessagePort::MoveToContext); registry->Register(SetDeserializerCreateObjectFunction); registry->Register(StructuredClone); + registry->Register(ExposeLazyDOMExceptionProperty); + registry->Register(ExposeLazyDOMExceptionPropertyGetter); } } // anonymous namespace diff --git a/src/node_util.cc b/src/node_util.cc index 5a6f0d4bb13fc3..844cdb0f440577 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -427,6 +427,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(fast_guess_handle_type_.GetTypeInfo()); registry->Register(ParseEnv); registry->Register(IsInsideNodeModules); + registry->Register(DefineLazyProperties); + registry->Register(DefineLazyPropertiesGetter); } void Initialize(Local target,