From 7601cdfe8bd47a1eee0851029258d24553be5364 Mon Sep 17 00:00:00 2001
From: Gabriel Schulhof <gabriel.schulhof@intel.com>
Date: Wed, 7 Nov 2018 07:09:40 -0500
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/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>
---
 src/async_wrap.cc      |  3 ++-
 src/base_object-inl.h  |  2 +-
 src/env-inl.h          |  2 +-
 src/heap_utils.cc      |  2 +-
 src/memory_tracker.h   |  2 +-
 src/node_api.cc        |  2 +-
 src/node_contextify.cc |  4 ++--
 src/node_contextify.h  |  2 +-
 src/node_crypto.cc     |  3 ++-
 src/node_internals.h   |  8 --------
 src/node_persistent.h  | 36 ++++++++++++++++++++++++++++++++++++
 src/node_zlib.cc       |  4 ++--
 src/util-inl.h         | 25 -------------------------
 src/util.h             | 22 ----------------------
 14 files changed, 50 insertions(+), 67 deletions(-)

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index d23e59f69e721c..21206e7208ea68 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -358,7 +358,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
index 8c8fa1699c3069..0b8fbb8520c283 100644
--- 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
index 30fed57b71fbed..70a8e052258531 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -899,7 +899,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
index 72ad33c99a1e80..d1e3fad0980280 100644
--- 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
index 17992792128809..11dd2be7af35c9 100644
--- 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
index c023920da41f74..500312750974ef 100644
--- 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
index 37afead80896d0..bc08e31a065306 100644
--- 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
index 1221233bb8bfe5..68ba2707f8d53b 100644
--- 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
index baa0fbe635cd8b..209b4c7dcb183b 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2339,7 +2339,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
index cfd42daa7bd4ae..32b8ee6115dfe0 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -183,14 +183,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
index 762842dd4bd373..6126ebc6c20fd5 100644
--- 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
index 6b050fcb40289a..f476c554d4477b 100644
--- 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
index e945c614d49e81..9751f9babe1189 100644
--- a/src/util-inl.h
+++ b/src/util-inl.h
@@ -165,31 +165,6 @@ inline 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
index 880408df4d57fd..f0d3189d1af2f2 100644
--- a/src/util.h
+++ b/src/util.h
@@ -201,28 +201,6 @@ template <typename Inner, typename Outer>
 inline 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,