Skip to content

Commit

Permalink
src: return Maybe from a couple of functions
Browse files Browse the repository at this point in the history
Functions affected:
* InitializeContext()
* InitializeContextForSnapshot()
* InitializePrimordials()

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #39603
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
RaisinTen authored and jasnell committed Aug 6, 2021
1 parent 26632c9 commit 0a7f850
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 18 deletions.
34 changes: 19 additions & 15 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Nothing;
using v8::Null;
using v8::Object;
using v8::ObjectTemplate;
Expand Down Expand Up @@ -501,7 +504,7 @@ MaybeLocal<Object> GetPerContextExports(Local<Context> context) {

Local<Object> exports = Object::New(isolate);
if (context->Global()->SetPrivate(context, key, exports).IsNothing() ||
!InitializePrimordials(context))
InitializePrimordials(context).IsNothing())
return MaybeLocal<Object>();
return handle_scope.Escape(exports);
}
Expand All @@ -514,7 +517,7 @@ Local<Context> NewContext(Isolate* isolate,
auto context = Context::New(isolate, nullptr, object_template);
if (context.IsEmpty()) return context;

if (!InitializeContext(context)) {
if (InitializeContext(context).IsNothing()) {
return Local<Context>();
}

Expand Down Expand Up @@ -581,16 +584,17 @@ void InitializeContextRuntime(Local<Context> context) {
}
}

bool InitializeContextForSnapshot(Local<Context> context) {
Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);

context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
True(isolate));

return InitializePrimordials(context);
}

bool InitializePrimordials(Local<Context> context) {
Maybe<bool> InitializePrimordials(Local<Context> context) {
// Run per-context JS files.
Isolate* isolate = context->GetIsolate();
Context::Scope context_scope(context);
Expand All @@ -603,10 +607,10 @@ bool InitializePrimordials(Local<Context> context) {

// Create primordials first and make it available to per-context scripts.
Local<Object> primordials = Object::New(isolate);
if (!primordials->SetPrototype(context, Null(isolate)).FromJust() ||
if (primordials->SetPrototype(context, Null(isolate)).IsNothing() ||
!GetPerContextExports(context).ToLocal(&exports) ||
!exports->Set(context, primordials_string, primordials).FromJust()) {
return false;
exports->Set(context, primordials_string, primordials).IsNothing()) {
return Nothing<bool>();
}

static const char* context_files[] = {"internal/per_context/primordials",
Expand All @@ -623,27 +627,27 @@ bool InitializePrimordials(Local<Context> context) {
context, *module, &parameters, nullptr);
Local<Function> fn;
if (!maybe_fn.ToLocal(&fn)) {
return false;
return Nothing<bool>();
}
MaybeLocal<Value> result =
fn->Call(context, Undefined(isolate), arraysize(arguments), arguments);
// Execution failed during context creation.
// TODO(joyeecheung): deprecate this signature and return a MaybeLocal.
if (result.IsEmpty()) {
return false;
return Nothing<bool>();
}
}

return true;
return Just(true);
}

bool InitializeContext(Local<Context> context) {
if (!InitializeContextForSnapshot(context)) {
return false;
Maybe<bool> InitializeContext(Local<Context> context) {
if (InitializeContextForSnapshot(context).IsNothing()) {
return Nothing<bool>();
}

InitializeContextRuntime(context);
return true;

return Just(true);
}

uv_loop_t* GetCurrentEventLoop(Isolate* isolate) {
Expand Down
2 changes: 1 addition & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ NODE_EXTERN v8::Local<v8::Context> NewContext(

// Runs Node.js-specific tweaks on an already constructed context
// Return value indicates success of operation
NODE_EXTERN bool InitializeContext(v8::Local<v8::Context> context);
NODE_EXTERN v8::Maybe<bool> InitializeContext(v8::Local<v8::Context> context);

// If `platform` is passed, it will be used to register new Worker instances.
// It can be `nullptr`, in which case creating new Workers inside of
Expand Down
5 changes: 3 additions & 2 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ void SignalExit(int signal, siginfo_t* info, void* ucontext);
std::string GetProcessTitle(const char* default_title);
std::string GetHumanReadableProcessName();

void InitializeContextRuntime(v8::Local<v8::Context>);
bool InitializePrimordials(v8::Local<v8::Context> context);
// TODO(RaisinTen): return a v8::Maybe<bool>.
void InitializeContextRuntime(v8::Local<v8::Context> context);
v8::Maybe<bool> InitializePrimordials(v8::Local<v8::Context> context);

class NodeArrayBufferAllocator : public ArrayBufferAllocator {
public:
Expand Down

0 comments on commit 0a7f850

Please sign in to comment.