Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: use CreateEnvironment instead of inlining its code where possible #45886

Merged
merged 3 commits into from Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/api/environment.cc
Expand Up @@ -375,23 +375,33 @@ Environment* CreateEnvironment(
// options than the global parse call.
Environment* env = new Environment(
isolate_data, context, args, exec_args, nullptr, flags, thread_id);

auto initialize_inspector = [&]() {
#if HAVE_INSPECTOR
if (env->should_create_inspector()) {
if (inspector_parent_handle) {
env->InitializeInspector(
std::move(static_cast<InspectorParentHandleImpl*>(
inspector_parent_handle.get())->impl));
} else {
env->InitializeInspector({});
if (env->should_create_inspector()) {
if (inspector_parent_handle) {
env->InitializeInspector(
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
std::move(static_cast<InspectorParentHandleImpl*>(
inspector_parent_handle.get())
->impl));
} else {
env->InitializeInspector({});
}
}
}
#endif
};

if (!(flags & EnvironmentFlags::kInspectorOnlyAfterBootstrap))
initialize_inspector();

if (env->principal_realm()->RunBootstrapping().IsEmpty()) {
FreeEnvironment(env);
return nullptr;
}

if (flags & EnvironmentFlags::kInspectorOnlyAfterBootstrap)
initialize_inspector();

return env;
}

Expand Down
5 changes: 4 additions & 1 deletion src/node.h
Expand Up @@ -560,7 +560,10 @@ enum Flags : uint64_t {
// This control is needed by embedders who may not want to initialize the V8
// inspector in situations where one has already been created,
// e.g. Blink's in Chromium.
kNoCreateInspector = 1 << 9
kNoCreateInspector = 1 << 9,
// Whether to enable the V8 inspector during Node.js bootstrap or not.
// This has no effect if kNoCreateInspector is set.
kInspectorOnlyAfterBootstrap = 1 << 10
};
} // namespace EnvironmentFlags

Expand Down
15 changes: 2 additions & 13 deletions src/node_main_instance.cc
Expand Up @@ -191,19 +191,8 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* 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
27 changes: 14 additions & 13 deletions src/node_snapshotable.cc
Expand Up @@ -1151,16 +1151,20 @@ ExitCode 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()) {
uint64_t env_flags = EnvironmentFlags::kDefaultFlags |
EnvironmentFlags::kInspectorOnlyAfterBootstrap;
if (snapshot_type != SnapshotMetadata::Type::kFullyCustomized) {
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
env_flags |= 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 ExitCode::kBootstrapFailure;
}
// If --build-snapshot is true, lib/internal/main/mksnapshot.js would be
Expand All @@ -1169,9 +1173,6 @@ ExitCode SnapshotBuilder::Generate(SnapshotData* out,
// could also explore snapshotting other kinds of execution modes
// in the future).
if (snapshot_type == SnapshotMetadata::Type::kFullyCustomized) {
#if HAVE_INSPECTOR
env->InitializeInspector({});
#endif
if (LoadEnvironment(env, StartExecutionCallback{}).IsEmpty()) {
return ExitCode::kGenericUserError;
}
Expand Down