Skip to content

Commit

Permalink
src: use CreateEnvironment instead of inlining its code where possible
Browse files Browse the repository at this point in the history
We had a number of places in which we created an `Environment` instance
by performing each step in `CreateEnvironment` manually. Instead,
just call the function itself.

PR-URL: #45886
Backport-PR-URL: #46330
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
addaleax authored and juanarbol committed Mar 5, 2023
1 parent 5019b54 commit 8adaa13
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 26 deletions.
7 changes: 4 additions & 3 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -392,12 +392,13 @@ Environment* CreateEnvironment(
// options than the global parse call.
Environment* env = new Environment(
isolate_data, context, args, exec_args, nullptr, flags, thread_id);

#if HAVE_INSPECTOR
if (env->should_create_inspector()) {
if (inspector_parent_handle) {
env->InitializeInspector(
std::move(static_cast<InspectorParentHandleImpl*>(
inspector_parent_handle.get())->impl));
env->InitializeInspector(std::move(
static_cast<InspectorParentHandleImpl*>(inspector_parent_handle.get())
->impl));
} else {
env->InitializeInspector({});
}
Expand Down
15 changes: 2 additions & 13 deletions src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,19 +180,8 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code) {
context = NewContext(isolate_);
CHECK(!context.IsEmpty());
Context::Scope context_scope(context);
env.reset(new Environment(isolate_data_.get(),
context,
args_,
exec_args_,
nullptr,
EnvironmentFlags::kDefaultFlags,
{}));
#if HAVE_INSPECTOR
env->InitializeInspector({});
#endif
if (env->principal_realm()->RunBootstrapping().IsEmpty()) {
return nullptr;
}
env.reset(
CreateEnvironment(isolate_data_.get(), context, args_, exec_args_));
}

return env;
Expand Down
25 changes: 15 additions & 10 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1159,16 +1159,21 @@ int SnapshotBuilder::Generate(SnapshotData* out,
Context::Scope context_scope(main_context);

// Create the environment.
env = new Environment(main_instance->isolate_data(),
main_context,
args,
exec_args,
nullptr,
node::EnvironmentFlags::kDefaultFlags,
{});

// Run scripts in lib/internal/bootstrap/
if (env->principal_realm()->RunBootstrapping().IsEmpty()) {
// It's not guaranteed that a context that goes through
// v8_inspector::V8Inspector::contextCreated() is runtime-independent,
// so do not start the inspector on the main context when building
// the default snapshot.
uint64_t env_flags = EnvironmentFlags::kDefaultFlags |
EnvironmentFlags::kNoCreateInspector;

env = CreateEnvironment(main_instance->isolate_data(),
main_context,
args,
exec_args,
static_cast<EnvironmentFlags::Flags>(env_flags));

// This already ran scripts in lib/internal/bootstrap/, if it fails return
if (env == nullptr) {
return BOOTSTRAP_ERROR;
}
// If --build-snapshot is true, lib/internal/main/mksnapshot.js would be
Expand Down

0 comments on commit 8adaa13

Please sign in to comment.