From 98bae2930453d40dadf3f36f5ad89eb73b23ef3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 31 Jul 2017 02:05:20 +0200 Subject: [PATCH] src: return MaybeLocal in AsyncWrap::MakeCallback PR-URL: https://github.com/nodejs/node/pull/14549 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Timothy Gu Reviewed-By: Colin Ihrig --- src/async-wrap-inl.h | 4 ++-- src/async-wrap.cc | 19 +++++++++---------- src/async-wrap.h | 20 ++++++++++---------- src/js_stream.cc | 21 +++++++++++++-------- src/node_http_parser.cc | 15 ++++++++++----- 5 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 75306a3b0dd..4f8b7c3f8dd 100644 --- 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 AsyncWrap::MakeCallback( +inline v8::MaybeLocal AsyncWrap::MakeCallback( const v8::Local symbol, int argc, v8::Local* argv) { @@ -61,7 +61,7 @@ inline v8::Local AsyncWrap::MakeCallback( } -inline v8::Local AsyncWrap::MakeCallback( +inline v8::MaybeLocal AsyncWrap::MakeCallback( uint32_t index, int argc, v8::Local* argv) { diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 038e1488efe..4b4a9b9f6cb 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -673,9 +673,9 @@ void AsyncWrap::EmitAsyncInit(Environment* env, } -Local AsyncWrap::MakeCallback(const Local cb, - int argc, - Local* argv) { +MaybeLocal AsyncWrap::MakeCallback(const Local cb, + int argc, + Local* argv) { CHECK(env()->context() == env()->isolate()->GetCurrentContext()); Environment::AsyncCallbackScope callback_scope(env()); @@ -685,15 +685,14 @@ Local AsyncWrap::MakeCallback(const Local cb, get_trigger_id()); if (!PreCallbackExecution(this, true)) { - return Local(); + return MaybeLocal(); } // Finally... Get to running the user's callback. MaybeLocal ret = cb->Call(env()->context(), object(), argc, argv); - Local ret_v; - if (!ret.ToLocal(&ret_v)) { - return Local(); + if (ret.IsEmpty()) { + return ret; } if (!PostCallbackExecution(this, true)) { @@ -703,7 +702,7 @@ Local AsyncWrap::MakeCallback(const Local cb, exec_scope.Dispose(); if (callback_scope.in_makecallback()) { - return ret_v; + return ret; } Environment::TickInfo* tick_info = env()->tick_info(); @@ -721,7 +720,7 @@ Local AsyncWrap::MakeCallback(const Local cb, if (tick_info->length() == 0) { tick_info->set_index(0); - return ret_v; + return ret; } MaybeLocal rcheck = @@ -734,7 +733,7 @@ Local AsyncWrap::MakeCallback(const Local cb, CHECK_EQ(env()->current_async_id(), 0); CHECK_EQ(env()->trigger_id(), 0); - return rcheck.IsEmpty() ? Local() : ret_v; + return rcheck.IsEmpty() ? MaybeLocal() : ret; } diff --git a/src/async-wrap.h b/src/async-wrap.h index 3a6bea276f5..79332d9a42f 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -121,16 +121,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 MakeCallback(const v8::Local cb, - int argc, - v8::Local* argv); - inline v8::Local MakeCallback(const v8::Local symbol, - int argc, - v8::Local* argv); - inline v8::Local MakeCallback(uint32_t index, - int argc, - v8::Local* argv); + v8::MaybeLocal MakeCallback(const v8::Local cb, + int argc, + v8::Local* argv); + inline v8::MaybeLocal MakeCallback( + const v8::Local symbol, + int argc, + v8::Local* argv); + inline v8::MaybeLocal MakeCallback(uint32_t index, + int argc, + v8::Local* argv); virtual size_t self_size() const = 0; diff --git a/src/js_stream.cc b/src/js_stream.cc index 2644a6a451a..d88cc853c80 100644 --- 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 fn = object()->Get(env()->isalive_string()); if (!fn->IsFunction()) return false; - return MakeCallback(fn.As(), 0, nullptr)->IsTrue(); + return MakeCallback(fn.As(), 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 res = + MaybeLocal 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 res = + MaybeLocal 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 index 90fc3b41b52..c897a771a52 100644 --- 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,7 +309,7 @@ class Parser : public AsyncWrap { Environment::AsyncCallbackScope callback_scope(env()); - Local head_response = + MaybeLocal head_response = MakeCallback(cb.As(), arraysize(argv), argv); if (head_response.IsEmpty()) { @@ -316,7 +317,7 @@ class Parser : public AsyncWrap { 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 r = MakeCallback(cb.As(), arraysize(argv), argv); + MaybeLocal r = MakeCallback(cb.As(), + arraysize(argv), + argv); if (r.IsEmpty()) { got_exception_ = true; @@ -369,7 +372,7 @@ class Parser : public AsyncWrap { Environment::AsyncCallbackScope callback_scope(env()); - Local r = MakeCallback(cb.As(), 0, nullptr); + MaybeLocal r = MakeCallback(cb.As(), 0, nullptr); if (r.IsEmpty()) { got_exception_ = true; @@ -702,7 +705,9 @@ class Parser : public AsyncWrap { url_.ToString(env()) }; - Local r = MakeCallback(cb.As(), arraysize(argv), argv); + MaybeLocal r = MakeCallback(cb.As(), + arraysize(argv), + argv); if (r.IsEmpty()) got_exception_ = true;