diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c0e84dbca0f7a1..6d13a62b7feb7d 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -365,13 +365,11 @@ void ContextifyContext::CreatePerIsolateProperties( IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); SetMethod(isolate, target, "makeContext", MakeContext); - SetMethod(isolate, target, "compileFunction", CompileFunction); } void ContextifyContext::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(MakeContext); - registry->Register(CompileFunction); registry->Register(PropertyQueryCallback); registry->Register(PropertyGetterCallback); registry->Register(PropertySetterCallback); @@ -1163,22 +1161,6 @@ Maybe StoreCodeCacheResult( return JustVoid(); } -// TODO(RaisinTen): Reuse in ContextifyContext::CompileFunction(). -MaybeLocal CompileFunction(Local context, - Local filename, - Local content, - LocalVector* parameters) { - ScriptOrigin script_origin(filename, 0, 0, true); - ScriptCompiler::Source script_source(content, script_origin); - - return ScriptCompiler::CompileFunction(context, - &script_source, - parameters->size(), - parameters->data(), - 0, - nullptr); -} - bool ContextifyScript::InstanceOf(Environment* env, const Local& value) { return !value.IsEmpty() && @@ -1392,7 +1374,19 @@ ContextifyScript::ContextifyScript(Environment* env, Local object) { ContextifyScript::~ContextifyScript() {} -void ContextifyContext::CompileFunction( +void ContextifyFunction::RegisterExternalReferences( + ExternalReferenceRegistry* registry) { + registry->Register(CompileFunction); +} + +void ContextifyFunction::CreatePerIsolateProperties( + IsolateData* isolate_data, Local target) { + Isolate* isolate = isolate_data->isolate(); + + SetMethod(isolate, target, "compileFunction", CompileFunction); +} + +void ContextifyFunction::CompileFunction( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -1511,24 +1505,22 @@ void ContextifyContext::CompileFunction( } TryCatchScope try_catch(env); - Local result = CompileFunctionAndCacheResult(env, - parsing_context, - &source, - params, - context_extensions, - options, - produce_cached_data, - id_symbol, - try_catch); - - if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + MaybeLocal maybe_result = + CompileFunctionAndCacheResult(env, + parsing_context, + &source, + params, + context_extensions, + options, + produce_cached_data, + id_symbol, + try_catch); + Local result; + if (!maybe_result.ToLocal(&result)) { + CHECK(try_catch.HasCaught()); try_catch.ReThrow(); return; } - - if (result.IsEmpty()) { - return; - } args.GetReturnValue().Set(result); } @@ -1544,7 +1536,7 @@ static LocalVector GetCJSParameters(IsolateData* data) { return result; } -Local ContextifyContext::CompileFunctionAndCacheResult( +MaybeLocal ContextifyFunction::CompileFunctionAndCacheResult( Environment* env, Local parsing_context, ScriptCompiler::Source* source, @@ -1566,28 +1558,29 @@ Local ContextifyContext::CompileFunctionAndCacheResult( Local fn; if (!maybe_fn.ToLocal(&fn)) { - if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + CHECK(try_catch.HasCaught()); + if (!try_catch.HasTerminated()) { errors::DecorateErrorStack(env, try_catch); - return Object::New(env->isolate()); } + return {}; } Local context = env->context(); if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) .IsNothing()) { - return Object::New(env->isolate()); + return {}; } Isolate* isolate = env->isolate(); Local result = Object::New(isolate); if (result->Set(parsing_context, env->function_string(), fn).IsNothing()) - return Object::New(env->isolate()); + return {}; if (result ->Set(parsing_context, env->source_map_url_string(), fn->GetScriptOrigin().SourceMapUrl()) .IsNothing()) - return Object::New(env->isolate()); + return {}; std::unique_ptr new_cached_data; if (produce_cached_data) { @@ -1600,7 +1593,7 @@ Local ContextifyContext::CompileFunctionAndCacheResult( produce_cached_data, std::move(new_cached_data)) .IsNothing()) { - return Object::New(env->isolate()); + return {}; } return result; @@ -1974,6 +1967,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data, ContextifyContext::CreatePerIsolateProperties(isolate_data, target); ContextifyScript::CreatePerIsolateProperties(isolate_data, target); + ContextifyFunction::CreatePerIsolateProperties(isolate_data, target); SetMethod(isolate, target, "startSigintWatchdog", StartSigintWatchdog); SetMethod(isolate, target, "stopSigintWatchdog", StopSigintWatchdog); @@ -2026,6 +2020,7 @@ static void CreatePerContextProperties(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { ContextifyContext::RegisterExternalReferences(registry); ContextifyScript::RegisterExternalReferences(registry); + ContextifyFunction::RegisterExternalReferences(registry); registry->Register(CompileFunctionForCJSLoader); registry->Register(StartSigintWatchdog); diff --git a/src/node_contextify.h b/src/node_contextify.h index 7e5d90459b572f..1066633daf65d4 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -144,18 +144,6 @@ class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) { static bool IsStillInitializing(const ContextifyContext* ctx); static void MakeContext(const v8::FunctionCallbackInfo& args); static void IsContext(const v8::FunctionCallbackInfo& args); - static void CompileFunction( - const v8::FunctionCallbackInfo& args); - static v8::Local CompileFunctionAndCacheResult( - Environment* env, - v8::Local parsing_context, - v8::ScriptCompiler::Source* source, - v8::LocalVector params, - v8::LocalVector context_extensions, - v8::ScriptCompiler::CompileOptions options, - bool produce_cached_data, - v8::Local id_symbol, - const errors::TryCatchScope& try_catch); static v8::Intercepted PropertyQueryCallback( v8::Local property, const v8::PropertyCallbackInfo& args); @@ -234,6 +222,29 @@ class ContextifyScript final : CPPGC_MIXIN(ContextifyScript) { v8::TracedReference script_; }; +class ContextifyFunction final { + public: + static void RegisterExternalReferences(ExternalReferenceRegistry* registry); + static void CreatePerIsolateProperties(IsolateData* isolate_data, + v8::Local target); + + static void CompileFunction(const v8::FunctionCallbackInfo& args); + static v8::MaybeLocal CompileFunctionAndCacheResult( + Environment* env, + v8::Local parsing_context, + v8::ScriptCompiler::Source* source, + v8::LocalVector params, + v8::LocalVector context_extensions, + v8::ScriptCompiler::CompileOptions options, + bool produce_cached_data, + v8::Local id_symbol, + const errors::TryCatchScope& try_catch); + + private: + ContextifyFunction() = delete; + ~ContextifyFunction() = delete; +}; + v8::Maybe StoreCodeCacheResult( Environment* env, v8::Local target, @@ -242,12 +253,6 @@ v8::Maybe StoreCodeCacheResult( bool produce_cached_data, std::unique_ptr new_cached_data); -v8::MaybeLocal CompileFunction( - v8::Local context, - v8::Local filename, - v8::Local content, - v8::LocalVector* parameters); - } // namespace contextify } // namespace node diff --git a/src/node_sea.cc b/src/node_sea.cc index 1f7f3c8a707f4e..65a338e00d4e22 100644 --- a/src/node_sea.cc +++ b/src/node_sea.cc @@ -41,6 +41,7 @@ using v8::MaybeLocal; using v8::NewStringType; using v8::Object; using v8::ScriptCompiler; +using v8::ScriptOrigin; using v8::String; using v8::Value; @@ -460,16 +461,23 @@ std::optional GenerateCodeCache(std::string_view main_path, FIXED_ONE_BYTE_STRING(isolate, "__filename"), FIXED_ONE_BYTE_STRING(isolate, "__dirname"), }); - - // TODO(RaisinTen): Using the V8 code cache prevents us from using `import()` - // in the SEA code. Support it. - // Refs: https://github.com/nodejs/node/pull/48191#discussion_r1213271430 + ScriptOrigin script_origin(filename, 0, 0, true); + ScriptCompiler::Source script_source(content, script_origin); + MaybeLocal maybe_fn = + ScriptCompiler::CompileFunction(context, + &script_source, + parameters.size(), + parameters.data(), + 0, + nullptr); Local fn; - if (!contextify::CompileFunction(context, filename, content, ¶meters) - .ToLocal(&fn)) { + if (!maybe_fn.ToLocal(&fn)) { return std::nullopt; } + // TODO(RaisinTen): Using the V8 code cache prevents us from using `import()` + // in the SEA code. Support it. + // Refs: https://github.com/nodejs/node/pull/48191#discussion_r1213271430 std::unique_ptr cache{ ScriptCompiler::CreateCodeCacheForFunction(fn)}; std::string code_cache(cache->data, cache->data + cache->length); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 01e05beaefa0a9..9bdef1032e4dc9 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -42,8 +42,11 @@ using v8::HandleScope; using v8::Isolate; using v8::Local; using v8::LocalVector; +using v8::MaybeLocal; using v8::Object; using v8::ObjectTemplate; +using v8::ScriptCompiler; +using v8::ScriptOrigin; using v8::SnapshotCreator; using v8::StartupData; using v8::String; @@ -1488,9 +1491,18 @@ void CompileSerializeMain(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(isolate, "__filename"), FIXED_ONE_BYTE_STRING(isolate, "__dirname"), }); + + ScriptOrigin script_origin(filename, 0, 0, true); + ScriptCompiler::Source script_source(source, script_origin); + MaybeLocal maybe_fn = + ScriptCompiler::CompileFunction(context, + &script_source, + parameters.size(), + parameters.data(), + 0, + nullptr); Local fn; - if (contextify::CompileFunction(context, filename, source, ¶meters) - .ToLocal(&fn)) { + if (maybe_fn.ToLocal(&fn)) { args.GetReturnValue().Set(fn); } }