src: use Environment* as the first arg of CallbackScope

Since we are using the first argument, an Isolate*, to figure out the current Environment* again, we should pass the Environment* directly. Signed-off-by: Darshan Sen <darshan.sen@postman.com>
nodejs · Aug 14, 2021 · e076b5d · e076b5d
1 parent ca19775
commit e076b5d
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 14 deletions.
diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc
@@ -62,10 +62,8 @@ async_id AsyncResource::get_trigger_async_id() const {
   return async_context_.trigger_async_id;
 }
 
-// TODO(addaleax): We shouldn’t need to use env_->isolate() if we’re just going
-// to end up using the Isolate* to figure out the Environment* again.
 AsyncResource::CallbackScope::CallbackScope(AsyncResource* res)
-    : node::CallbackScope(res->env_->isolate(),
+    : node::CallbackScope(res->env_,
                           res->resource_.Get(res->env_->isolate()),
                           res->async_context_) {}
 

diff --git a/src/api/callback.cc b/src/api/callback.cc
@@ -16,13 +16,13 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
-CallbackScope::CallbackScope(Isolate* isolate,
+CallbackScope::CallbackScope(Environment* env,
                              Local<Object> object,
                              async_context asyncContext)
-  : private_(new InternalCallbackScope(Environment::GetCurrent(isolate),
+  : private_(new InternalCallbackScope(env,
                                        object,
                                        asyncContext)),
-    try_catch_(isolate) {
+    try_catch_(env->isolate()) {
   try_catch_.SetVerbose(true);
 }
 

diff --git a/src/node.h b/src/node.h
@@ -1002,7 +1002,7 @@ class InternalCallbackScope;
  */
 class NODE_EXTERN CallbackScope {
  public:
-  CallbackScope(v8::Isolate* isolate,
+  CallbackScope(Environment* env,
                 v8::Local<v8::Object> resource,
                 async_context asyncContext);
   ~CallbackScope();

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -539,7 +539,7 @@ class AsyncContext {
   class CallbackScope : public node::CallbackScope {
    public:
     explicit CallbackScope(AsyncContext* async_context)
-        : node::CallbackScope(async_context->node_env()->isolate(),
+        : node::CallbackScope(async_context->node_env(),
                               async_context->resource_.Get(
                                   async_context->node_env()->isolate()),
                               async_context->async_context()) {}

diff --git a/test/addons/callback-scope/binding.cc b/test/addons/callback-scope/binding.cc
@@ -1,15 +1,17 @@
 #include "node.h"
-#include "v8.h"
 #include "uv.h"
+#include "v8.h"
 
 #include <assert.h>
-#include <vector>
 #include <memory>
+#include <vector>
 
 namespace {
 
 void RunInCallbackScope(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  node::Environment* env = node::GetCurrentEnvironment(context);
 
   assert(args.Length() == 4);
   assert(args[0]->IsObject());
@@ -22,11 +24,11 @@ void RunInCallbackScope(const v8::FunctionCallbackInfo<v8::Value>& args) {
     args[2].As<v8::Number>()->Value()
   };
 
-  node::CallbackScope scope(isolate, args[0].As<v8::Object>(), asyncContext);
+  node::CallbackScope scope(env, args[0].As<v8::Object>(), asyncContext);
   v8::Local<v8::Function> fn = args[3].As<v8::Function>();
 
   v8::MaybeLocal<v8::Value> ret =
-      fn->Call(isolate->GetCurrentContext(), args[0], 0, nullptr);
+      fn->Call(context, args[0], 0, nullptr);
 
   if (!ret.IsEmpty())
     args.GetReturnValue().Set(ret.ToLocalChecked());
@@ -35,12 +37,14 @@ void RunInCallbackScope(const v8::FunctionCallbackInfo<v8::Value>& args) {
 static void Callback(uv_work_t* req, int ignored) {
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
   v8::HandleScope scope(isolate);
-  node::CallbackScope callback_scope(isolate, v8::Object::New(isolate),
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  node::Environment* env = node::GetCurrentEnvironment(context);
+  node::CallbackScope callback_scope(env, v8::Object::New(isolate),
                                      node::async_context{0, 0});
   std::unique_ptr<v8::Global<v8::Promise::Resolver>> persistent {
       static_cast<v8::Global<v8::Promise::Resolver>*>(req->data) };
   v8::Local<v8::Promise::Resolver> local = persistent->Get(isolate);
-  local->Resolve(isolate->GetCurrentContext(),
+  local->Resolve(context,
                  v8::Undefined(isolate)).ToChecked();
   delete req;
 }