src: return MaybeLocal in AsyncWrap::MakeCallback

PR-URL: #14549 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
nodejs · Aug 2, 2017 · fa3aa2e · fa3aa2e
1 parent 51c1afa
commit fa3aa2e
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 35 deletions.
diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h
@@ -51,7 +51,7 @@ inline double AsyncWrap::get_trigger_id() const {
 }
 
 
-inline v8::Local<v8::Value> AsyncWrap::MakeCallback(
+inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
     const v8::Local<v8::String> symbol,
     int argc,
     v8::Local<v8::Value>* argv) {
@@ -61,7 +61,7 @@ inline v8::Local<v8::Value> AsyncWrap::MakeCallback(
 }
 
 
-inline v8::Local<v8::Value> AsyncWrap::MakeCallback(
+inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
     uint32_t index,
     int argc,
     v8::Local<v8::Value>* argv) {

diff --git a/src/async-wrap.cc b/src/async-wrap.cc
@@ -674,9 +674,9 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
 }
 
 
-Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
-                                     int argc,
-                                     Local<Value>* argv) {
+MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
+                                          int argc,
+                                          Local<Value>* argv) {
   CHECK(env()->context() == env()->isolate()->GetCurrentContext());
 
   Environment::AsyncCallbackScope callback_scope(env());
@@ -686,15 +686,14 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
                                                 get_trigger_id());
 
   if (!PreCallbackExecution(this, true)) {
-    return Local<Value>();
+    return MaybeLocal<Value>();
   }
 
   // Finally... Get to running the user's callback.
   MaybeLocal<Value> ret = cb->Call(env()->context(), object(), argc, argv);
 
-  Local<Value> ret_v;
-  if (!ret.ToLocal(&ret_v)) {
-    return Local<Value>();
+  if (ret.IsEmpty()) {
+    return ret;
   }
 
   if (!PostCallbackExecution(this, true)) {
@@ -704,7 +703,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   exec_scope.Dispose();
 
   if (callback_scope.in_makecallback()) {
-    return ret_v;
+    return ret;
   }
 
   Environment::TickInfo* tick_info = env()->tick_info();
@@ -722,7 +721,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
 
   if (tick_info->length() == 0) {
     tick_info->set_index(0);
-    return ret_v;
+    return ret;
   }
 
   MaybeLocal<Value> rcheck =
@@ -735,7 +734,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   CHECK_EQ(env()->current_async_id(), 0);
   CHECK_EQ(env()->trigger_id(), 0);
 
-  return rcheck.IsEmpty() ? Local<Value>() : ret_v;
+  return rcheck.IsEmpty() ? MaybeLocal<Value>() : ret;
 }
 
 

diff --git a/src/async-wrap.h b/src/async-wrap.h
@@ -122,16 +122,16 @@ class AsyncWrap : public BaseObject {
   void AsyncReset(bool silent = false);
 
   // Only call these within a valid HandleScope.
-  // TODO(trevnorris): These should return a MaybeLocal.
-  v8::Local<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
-                                    int argc,
-                                    v8::Local<v8::Value>* argv);
-  inline v8::Local<v8::Value> MakeCallback(const v8::Local<v8::String> symbol,
-                                           int argc,
-                                           v8::Local<v8::Value>* argv);
-  inline v8::Local<v8::Value> MakeCallback(uint32_t index,
-                                           int argc,
-                                           v8::Local<v8::Value>* argv);
+  v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
+                                         int argc,
+                                         v8::Local<v8::Value>* argv);
+  inline v8::MaybeLocal<v8::Value> MakeCallback(
+      const v8::Local<v8::String> symbol,
+      int argc,
+      v8::Local<v8::Value>* argv);
+  inline v8::MaybeLocal<v8::Value> MakeCallback(uint32_t index,
+                                                int argc,
+                                                v8::Local<v8::Value>* argv);
 
   virtual size_t self_size() const = 0;
 

diff --git a/src/js_stream.cc b/src/js_stream.cc
@@ -16,6 +16,7 @@ using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Object;
 using v8::Value;
 
@@ -46,22 +47,26 @@ bool JSStream::IsAlive() {
   v8::Local<v8::Value> fn = object()->Get(env()->isalive_string());
   if (!fn->IsFunction())
     return false;
-  return MakeCallback(fn.As<v8::Function>(), 0, nullptr)->IsTrue();
+  return MakeCallback(fn.As<v8::Function>(), 0, nullptr)
+      .ToLocalChecked()->IsTrue();
 }
 
 
 bool JSStream::IsClosing() {
-  return MakeCallback(env()->isclosing_string(), 0, nullptr)->IsTrue();
+  return MakeCallback(env()->isclosing_string(), 0, nullptr)
+      .ToLocalChecked()->IsTrue();
 }
 
 
 int JSStream::ReadStart() {
-  return MakeCallback(env()->onreadstart_string(), 0, nullptr)->Int32Value();
+  return MakeCallback(env()->onreadstart_string(), 0, nullptr)
+      .ToLocalChecked()->Int32Value();
 }
 
 
 int JSStream::ReadStop() {
-  return MakeCallback(env()->onreadstop_string(), 0, nullptr)->Int32Value();
+  return MakeCallback(env()->onreadstop_string(), 0, nullptr)
+      .ToLocalChecked()->Int32Value();
 }
 
 
@@ -73,10 +78,10 @@ int JSStream::DoShutdown(ShutdownWrap* req_wrap) {
   };
 
   req_wrap->Dispatched();
-  Local<Value> res =
+  MaybeLocal<Value> res =
       MakeCallback(env()->onshutdown_string(), arraysize(argv), argv);
 
-  return res->Int32Value();
+  return res.ToLocalChecked()->Int32Value();
 }
 
 
@@ -101,10 +106,10 @@ int JSStream::DoWrite(WriteWrap* w,
   };
 
   w->Dispatched();
-  Local<Value> res =
+  MaybeLocal<Value> res =
       MakeCallback(env()->onwrite_string(), arraysize(argv), argv);
 
-  return res->Int32Value();
+  return res.ToLocalChecked()->Int32Value();
 }
 
 

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -62,6 +62,7 @@ using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
@@ -308,15 +309,15 @@ class Parser : public AsyncWrap {
 
     Environment::AsyncCallbackScope callback_scope(env());
 
-    Local<Value> head_response =
+    MaybeLocal<Value> head_response =
         MakeCallback(cb.As<Function>(), arraysize(argv), argv);
 
     if (head_response.IsEmpty()) {
       got_exception_ = true;
       return -1;
     }
 
-    return head_response->IntegerValue();
+    return head_response.ToLocalChecked()->IntegerValue();
   }
 
 
@@ -344,7 +345,9 @@ class Parser : public AsyncWrap {
       Integer::NewFromUnsigned(env()->isolate(), length)
     };
 
-    Local<Value> r = MakeCallback(cb.As<Function>(), arraysize(argv), argv);
+    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(),
+                                       arraysize(argv),
+                                       argv);
 
     if (r.IsEmpty()) {
       got_exception_ = true;
@@ -369,7 +372,7 @@ class Parser : public AsyncWrap {
 
     Environment::AsyncCallbackScope callback_scope(env());
 
-    Local<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
+    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
 
     if (r.IsEmpty()) {
       got_exception_ = true;
@@ -702,7 +705,9 @@ class Parser : public AsyncWrap {
       url_.ToString(env())
     };
 
-    Local<Value> r = MakeCallback(cb.As<Function>(), arraysize(argv), argv);
+    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(),
+                                       arraysize(argv),
+                                       argv);
 
     if (r.IsEmpty())
       got_exception_ = true;