Skip to content

Commit

Permalink
src: bundle persistent-to-local methods as class
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Gabriel Schulhof authored and BridgeAR committed Nov 13, 2018
1 parent f5945c9 commit 7601cdf
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 67 deletions.
3 changes: 2 additions & 1 deletion src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion src/base_object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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); \
Expand Down
2 changes: 1 addition & 1 deletion src/heap_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/memory_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
2 changes: 1 addition & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
8 changes: 0 additions & 8 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
36 changes: 36 additions & 0 deletions src/node_persistent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_)
Expand Down
25 changes: 0 additions & 25 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 0 additions & 22 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 7601cdf

Please sign in to comment.