src: bundle persistent-to-local methods as class

Create a class `PersistentToLocal` which contains three methods, `Strong`, `Weak`, and `Default`: * `Strong` returns a `Local` from a strong persistent reference, * `Weak` returns a `Local` from a weak persistent reference, and * `Default` decides based on `IsWeak()` which of the above two to call. These replace `node::StrongPersistentToLocal()`, `node::WeakPersistentToLocal()`, and `node::PersistentToLocal()`, respectively. PR-URL: #24276 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
nodejs · Nov 13, 2018 · 0603c0a · 0603c0a
1 parent bda4643
commit 0603c0a
Show file tree

Hide file tree

Showing 14 changed files with 50 additions and 67 deletions.
diff --git a/src/async_wrap.cc b/src/async_wrap.cc
@@ -346,7 +346,8 @@ void AsyncWrap::WeakCallback(const v8::WeakCallbackInfo<DestroyParam>& info) {
   HandleScope scope(info.GetIsolate());
 
   std::unique_ptr<DestroyParam> p{info.GetParameter()};
-  Local<Object> prop_bag = PersistentToLocal(info.GetIsolate(), p->propBag);
+  Local<Object> prop_bag = PersistentToLocal::Default(info.GetIsolate(),
+                                                      p->propBag);
   Local<Value> val;
 
   if (!prop_bag->Get(p->env->context(), p->env->destroyed_string())

diff --git a/src/base_object-inl.h b/src/base_object-inl.h
@@ -62,7 +62,7 @@ Persistent<v8::Object>& BaseObject::persistent() {
 
 
 v8::Local<v8::Object> BaseObject::object() const {
-  return PersistentToLocal(env_->isolate(), persistent_handle_);
+  return PersistentToLocal::Default(env_->isolate(), persistent_handle_);
 }
 
 v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const {

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -895,7 +895,7 @@ void Environment::ForEachBaseObject(T&& iterator) {
 
 #define V(PropertyName, TypeName)                                             \
   inline v8::Local<TypeName> Environment::PropertyName() const {              \
-    return StrongPersistentToLocal(PropertyName ## _);                        \
+    return PersistentToLocal::Strong(PropertyName ## _);                      \
   }                                                                           \
   inline void Environment::set_ ## PropertyName(v8::Local<TypeName> value) {  \
     PropertyName ## _.Reset(isolate(), value);                                \

diff --git a/src/heap_utils.cc b/src/heap_utils.cc
@@ -26,7 +26,7 @@ class JSGraphJSNode : public EmbedderGraph::Node {
   const char* Name() override { return "<JS Node>"; }
   size_t SizeInBytes() override { return 0; }
   bool IsEmbedderNode() override { return false; }
-  Local<Value> JSValue() { return StrongPersistentToLocal(persistent_); }
+  Local<Value> JSValue() { return PersistentToLocal::Strong(persistent_); }
 
   int IdentityHash() {
     Local<Value> v = JSValue();

diff --git a/src/memory_tracker.h b/src/memory_tracker.h
@@ -69,7 +69,7 @@ class NodeBIO;
  *     // a BaseObject or an AsyncWrap class
  *     bool IsRootNode() const override { return !wrapped_.IsWeak(); }
  *     v8::Local<v8::Object> WrappedObject() const override {
- *       return node::PersistentToLocal(wrapped_);
+ *       return node::PersistentToLocal::Default(wrapped_);
  *     }
  *   private:
  *     AnotherRetainerClass another_retainer_;

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -30,7 +30,7 @@ struct napi_env__ {
   node::Persistent<v8::Context> context_persistent;
 
   inline v8::Local<v8::Context> context() const {
-    return StrongPersistentToLocal(context_persistent);
+    return node::PersistentToLocal::Strong(context_persistent);
   }
 
   inline node::Environment* node_env() const {

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -767,7 +767,7 @@ void ContextifyScript::CreateCachedData(
   ContextifyScript* wrapped_script;
   ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
   Local<UnboundScript> unbound_script =
-      PersistentToLocal(env->isolate(), wrapped_script->script_);
+      PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
   std::unique_ptr<ScriptCompiler::CachedData> cached_data(
       ScriptCompiler::CreateCodeCache(unbound_script));
   if (!cached_data) {
@@ -867,7 +867,7 @@ bool ContextifyScript::EvalMachine(Environment* env,
   ContextifyScript* wrapped_script;
   ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
   Local<UnboundScript> unbound_script =
-      PersistentToLocal(env->isolate(), wrapped_script->script_);
+      PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
   Local<Script> script = unbound_script->BindToCurrentContext();
 
   MaybeLocal<Value> result;

diff --git a/src/node_contextify.h b/src/node_contextify.h
@@ -40,7 +40,7 @@ class ContextifyContext {
   }
 
   inline v8::Local<v8::Context> context() const {
-    return PersistentToLocal(env()->isolate(), context_);
+    return PersistentToLocal::Default(env()->isolate(), context_);
   }
 
   inline v8::Local<v8::Object> global_proxy() const {

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -2348,7 +2348,8 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
     if (w->ocsp_response_.IsEmpty())
       return SSL_TLSEXT_ERR_NOACK;
 
-    Local<Object> obj = PersistentToLocal(env->isolate(), w->ocsp_response_);
+    Local<Object> obj = PersistentToLocal::Default(env->isolate(),
+                                                   w->ocsp_response_);
     char* resp = Buffer::Data(obj);
     size_t len = Buffer::Length(obj);
 

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -182,14 +182,6 @@ extern std::shared_ptr<PerProcessOptions> per_process_opts;
 // Forward declaration
 class Environment;
 
-// If persistent.IsWeak() == false, then do not call persistent.Reset()
-// while the returned Local<T> is still in scope, it will destroy the
-// reference to the object.
-template <class TypeName>
-inline v8::Local<TypeName> PersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent);
-
 // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object.
 // Sets address and port properties on the info object and returns it.
 // If |info| is omitted, a new object is returned.

diff --git a/src/node_persistent.h b/src/node_persistent.h
@@ -23,6 +23,42 @@ struct ResetInDestructorPersistentTraits {
 template <typename T>
 using Persistent = v8::Persistent<T, ResetInDestructorPersistentTraits<T>>;
 
+class PersistentToLocal {
+ public:
+  // If persistent.IsWeak() == false, then do not call persistent.Reset()
+  // while the returned Local<T> is still in scope, it will destroy the
+  // reference to the object.
+  template <class TypeName>
+  static inline v8::Local<TypeName> Default(
+      v8::Isolate* isolate,
+      const Persistent<TypeName>& persistent) {
+    if (persistent.IsWeak()) {
+      return PersistentToLocal::Weak(isolate, persistent);
+    } else {
+      return PersistentToLocal::Strong(persistent);
+    }
+  }
+
+  // Unchecked conversion from a non-weak Persistent<T> to Local<T>,
+  // use with care!
+  //
+  // Do not call persistent.Reset() while the returned Local<T> is still in
+  // scope, it will destroy the reference to the object.
+  template <class TypeName>
+  static inline v8::Local<TypeName> Strong(
+      const Persistent<TypeName>& persistent) {
+    return *reinterpret_cast<v8::Local<TypeName>*>(
+        const_cast<Persistent<TypeName>*>(&persistent));
+  }
+
+  template <class TypeName>
+  static inline v8::Local<TypeName> Weak(
+      v8::Isolate* isolate,
+      const Persistent<TypeName>& persistent) {
+    return v8::Local<TypeName>::New(isolate, persistent);
+  }
+};
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/node_zlib.cc b/src/node_zlib.cc
@@ -313,8 +313,8 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
     UpdateWriteResult();
 
     // call the write() cb
-    Local<Function> cb = PersistentToLocal(env()->isolate(),
-                                           write_js_callback_);
+    Local<Function> cb = PersistentToLocal::Default(env()->isolate(),
+                                                    write_js_callback_);
     MakeCallback(cb, 0, nullptr);
 
     if (pending_close_)

diff --git a/src/util-inl.h b/src/util-inl.h
@@ -165,31 +165,6 @@ constexpr ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field,
   return ContainerOfHelper<Inner, Outer>(field, pointer);
 }
 
-template <class TypeName>
-inline v8::Local<TypeName> PersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent) {
-  if (persistent.IsWeak()) {
-    return WeakPersistentToLocal(isolate, persistent);
-  } else {
-    return StrongPersistentToLocal(persistent);
-  }
-}
-
-template <class TypeName>
-inline v8::Local<TypeName> StrongPersistentToLocal(
-    const Persistent<TypeName>& persistent) {
-  return *reinterpret_cast<v8::Local<TypeName>*>(
-      const_cast<Persistent<TypeName>*>(&persistent));
-}
-
-template <class TypeName>
-inline v8::Local<TypeName> WeakPersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent) {
-  return v8::Local<TypeName>::New(isolate, persistent);
-}
-
 inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
                                            const char* data,
                                            int length) {

diff --git a/src/util.h b/src/util.h
@@ -201,28 +201,6 @@ template <typename Inner, typename Outer>
 constexpr ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field,
                                                       Inner* pointer);
 
-// If persistent.IsWeak() == false, then do not call persistent.Reset()
-// while the returned Local<T> is still in scope, it will destroy the
-// reference to the object.
-template <class TypeName>
-inline v8::Local<TypeName> PersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent);
-
-// Unchecked conversion from a non-weak Persistent<T> to Local<T>,
-// use with care!
-//
-// Do not call persistent.Reset() while the returned Local<T> is still in
-// scope, it will destroy the reference to the object.
-template <class TypeName>
-inline v8::Local<TypeName> StrongPersistentToLocal(
-    const Persistent<TypeName>& persistent);
-
-template <class TypeName>
-inline v8::Local<TypeName> WeakPersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent);
-
 // Convenience wrapper around v8::String::NewFromOneByte().
 inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
                                            const char* data,