Skip to content

Commit

Permalink
src: avoid prototype access in binding templates
Browse files Browse the repository at this point in the history
This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.

Previously the initialization was similar to:

```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```

Now it's similar to:

```
module.exports = { property: ... };
```

PR-URL: #47913
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung committed May 24, 2023
1 parent 21153a8 commit c0365fd
Show file tree
Hide file tree
Showing 27 changed files with 98 additions and 121 deletions.
3 changes: 1 addition & 2 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,8 @@ Local<FunctionTemplate> AsyncWrap::GetConstructorTemplate(
}

void AsyncWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> ctor) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> target = ctor->InstanceTemplate();

SetMethod(isolate, target, "setupHooks", SetupHooks);
SetMethod(isolate, target, "setCallbackTrampoline", SetCallbackTrampoline);
Expand Down
4 changes: 2 additions & 2 deletions src/async_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ class AsyncWrap : public BaseObject {
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context,
void* priv);
static void CreatePerIsolateProperties(
IsolateData* isolate_data, v8::Local<v8::FunctionTemplate> target);
static void CreatePerIsolateProperties(IsolateData* isolate_data,
v8::Local<v8::ObjectTemplate> target);

static void GetAsyncId(const v8::FunctionCallbackInfo<v8::Value>& args);
static void PushAsyncContext(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
4 changes: 1 addition & 3 deletions src/encoding_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ using v8::ArrayBuffer;
using v8::BackingStore;
using v8::Context;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
Expand Down Expand Up @@ -219,9 +218,8 @@ void BindingData::ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args) {
}

void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> ctor) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> target = ctor->InstanceTemplate();
SetMethod(isolate, target, "encodeInto", EncodeInto);
SetMethodNoSideEffect(isolate, target, "encodeUtf8String", EncodeUtf8String);
SetMethodNoSideEffect(isolate, target, "decodeUTF8", DecodeUTF8);
Expand Down
2 changes: 1 addition & 1 deletion src/encoding_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class BindingData : public SnapshotableObject {
static void ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args);

static void CreatePerIsolateProperties(IsolateData* isolate_data,
v8::Local<v8::FunctionTemplate> ctor);
v8::Local<v8::ObjectTemplate> target);
static void CreatePerContextProperties(v8::Local<v8::Object> target,
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context,
Expand Down
2 changes: 1 addition & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ void Environment::set_process_exit_handler(
#undef VY
#undef VP

#define VM(PropertyName) V(PropertyName##_binding, v8::FunctionTemplate)
#define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate)
#define V(PropertyName, TypeName) \
inline v8::Local<TypeName> IsolateData::PropertyName() const { \
return PropertyName##_.Get(isolate_); \
Expand Down
13 changes: 6 additions & 7 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ using v8::Context;
using v8::EmbedderGraph;
using v8::EscapableHandleScope;
using v8::Function;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::HeapProfiler;
using v8::HeapSpaceStatistics;
Expand All @@ -49,6 +48,7 @@ using v8::MaybeLocal;
using v8::NewStringType;
using v8::Number;
using v8::Object;
using v8::ObjectTemplate;
using v8::Private;
using v8::Script;
using v8::SnapshotCreator;
Expand Down Expand Up @@ -326,7 +326,7 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
info.primitive_values.push_back(creator->AddData(async_wrap_provider(i)));

uint32_t id = 0;
#define VM(PropertyName) V(PropertyName##_binding, FunctionTemplate)
#define VM(PropertyName) V(PropertyName##_binding_template, ObjectTemplate)
#define V(PropertyName, TypeName) \
do { \
Local<TypeName> field = PropertyName(); \
Expand Down Expand Up @@ -390,7 +390,7 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
const std::vector<PropInfo>& values = info->template_values;
i = 0; // index to the array
uint32_t id = 0;
#define VM(PropertyName) V(PropertyName##_binding, FunctionTemplate)
#define VM(PropertyName) V(PropertyName##_binding_template, ObjectTemplate)
#define V(PropertyName, TypeName) \
do { \
if (values.size() > i && id == values[i].id) { \
Expand Down Expand Up @@ -491,10 +491,9 @@ void IsolateData::CreateProperties() {
NODE_ASYNC_PROVIDER_TYPES(V)
#undef V

Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
templ->InstanceTemplate()->SetInternalFieldCount(
BaseObject::kInternalFieldCount);
set_binding_data_ctor_template(templ);
Local<ObjectTemplate> templ = ObjectTemplate::New(isolate());
templ->SetInternalFieldCount(BaseObject::kInternalFieldCount);
set_binding_data_default_template(templ);
binding::CreateInternalBindingTemplates(this);

contextify::ContextifyContext::InitializeGlobalTemplates(this);
Expand Down
4 changes: 2 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
#undef VS
#undef VP

#define VM(PropertyName) V(PropertyName##_binding, v8::FunctionTemplate)
#define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate)
#define V(PropertyName, TypeName) \
inline v8::Local<TypeName> PropertyName() const; \
inline void set_##PropertyName(v8::Local<TypeName> value);
Expand Down Expand Up @@ -194,7 +194,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
#define VR(PropertyName, TypeName) V(v8::Private, per_realm_##PropertyName)
#define VM(PropertyName) V(v8::FunctionTemplate, PropertyName##_binding)
#define VM(PropertyName) V(v8::ObjectTemplate, PropertyName##_binding_template)
#define VT(PropertyName, TypeName) V(TypeName, PropertyName)
#define V(TypeName, PropertyName) \
v8::Eternal<TypeName> PropertyName ## _;
Expand Down
2 changes: 1 addition & 1 deletion src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@
#define PER_ISOLATE_TEMPLATE_PROPERTIES(V) \
V(async_wrap_ctor_template, v8::FunctionTemplate) \
V(async_wrap_object_ctor_template, v8::FunctionTemplate) \
V(binding_data_ctor_template, v8::FunctionTemplate) \
V(binding_data_default_template, v8::ObjectTemplate) \
V(blob_constructor_template, v8::FunctionTemplate) \
V(blob_reader_constructor_template, v8::FunctionTemplate) \
V(blocklist_constructor_template, v8::FunctionTemplate) \
Expand Down
26 changes: 12 additions & 14 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ NODE_BUILTIN_BINDINGS(V)

#define V(modname) \
void _register_isolate_##modname(node::IsolateData* isolate_data, \
v8::Local<v8::FunctionTemplate> target);
v8::Local<v8::ObjectTemplate> target);
NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
#undef V

Expand Down Expand Up @@ -235,11 +235,11 @@ using v8::Context;
using v8::EscapableHandleScope;
using v8::Exception;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::Object;
using v8::ObjectTemplate;
using v8::String;
using v8::Value;

Expand Down Expand Up @@ -572,12 +572,11 @@ inline struct node_module* FindModule(struct node_module* list,
void CreateInternalBindingTemplates(IsolateData* isolate_data) {
#define V(modname) \
do { \
Local<FunctionTemplate> templ = \
FunctionTemplate::New(isolate_data->isolate()); \
templ->InstanceTemplate()->SetInternalFieldCount( \
BaseObject::kInternalFieldCount); \
Local<ObjectTemplate> templ = \
ObjectTemplate::New(isolate_data->isolate()); \
templ->SetInternalFieldCount(BaseObject::kInternalFieldCount); \
_register_isolate_##modname(isolate_data, templ); \
isolate_data->set_##modname##_binding(templ); \
isolate_data->set_##modname##_binding_template(templ); \
} while (0);
NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
#undef V
Expand All @@ -586,21 +585,20 @@ void CreateInternalBindingTemplates(IsolateData* isolate_data) {
static Local<Object> GetInternalBindingExportObject(IsolateData* isolate_data,
const char* mod_name,
Local<Context> context) {
Local<FunctionTemplate> ctor;
Local<ObjectTemplate> templ;

#define V(name) \
if (strcmp(mod_name, #name) == 0) { \
ctor = isolate_data->name##_binding(); \
templ = isolate_data->name##_binding_template(); \
} else // NOLINT(readability/braces)
NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
#undef V
{
ctor = isolate_data->binding_data_ctor_template();
// Default template.
templ = isolate_data->binding_data_default_template();
}

Local<Object> obj = ctor->GetFunction(context)
.ToLocalChecked()
->NewInstance(context)
.ToLocalChecked();
Local<Object> obj = templ->NewInstance(context).ToLocalChecked();
return obj;
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ namespace node {
// list.
#define NODE_BINDING_PER_ISOLATE_INIT(modname, per_isolate_func) \
void _register_isolate_##modname(node::IsolateData* isolate_data, \
v8::Local<v8::FunctionTemplate> target) { \
v8::Local<v8::ObjectTemplate> target) { \
per_isolate_func(isolate_data, target); \
}

Expand Down
3 changes: 1 addition & 2 deletions src/node_blob.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,8 @@ void BlobFromFilePath(const FunctionCallbackInfo<Value>& args) {
} // namespace

void Blob::CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> ctor) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> target = ctor->InstanceTemplate();

SetMethod(isolate, target, "createBlob", New);
SetMethod(isolate, target, "storeDataObject", StoreDataObject);
Expand Down
2 changes: 1 addition & 1 deletion src/node_blob.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Blob : public BaseObject {
ExternalReferenceRegistry* registry);

static void CreatePerIsolateProperties(IsolateData* isolate_data,
v8::Local<v8::FunctionTemplate> ctor);
v8::Local<v8::ObjectTemplate> target);
static void CreatePerContextProperties(v8::Local<v8::Object> target,
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context,
Expand Down
62 changes: 30 additions & 32 deletions src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ using v8::DEFAULT;
using v8::EscapableHandleScope;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::IntegrityLevel;
using v8::Isolate;
using v8::Local;
Expand Down Expand Up @@ -663,38 +662,37 @@ void BuiltinLoader::CopySourceAndCodeCacheReferenceFrom(
}

void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> target) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> proto = target->PrototypeTemplate();

proto->SetAccessor(isolate_data->config_string(),
ConfigStringGetter,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

proto->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "builtinIds"),
BuiltinIdsGetter,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

proto->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "builtinCategories"),
GetBuiltinCategories,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

SetMethod(isolate, proto, "getCacheUsage", BuiltinLoader::GetCacheUsage);
SetMethod(isolate, proto, "compileFunction", BuiltinLoader::CompileFunction);
SetMethod(isolate, proto, "hasCachedBuiltins", HasCachedBuiltins);
SetMethod(isolate, proto, "setInternalLoaders", SetInternalLoaders);

target->SetAccessor(isolate_data->config_string(),
ConfigStringGetter,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

target->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "builtinIds"),
BuiltinIdsGetter,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

target->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "builtinCategories"),
GetBuiltinCategories,
nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect);

SetMethod(isolate, target, "getCacheUsage", BuiltinLoader::GetCacheUsage);
SetMethod(isolate, target, "compileFunction", BuiltinLoader::CompileFunction);
SetMethod(isolate, target, "hasCachedBuiltins", HasCachedBuiltins);
SetMethod(isolate, target, "setInternalLoaders", SetInternalLoaders);
}

void BuiltinLoader::CreatePerContextProperties(Local<Object> target,
Expand Down
4 changes: 2 additions & 2 deletions src/node_builtins.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
BuiltinLoader& operator=(const BuiltinLoader&) = delete;

static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
static void CreatePerIsolateProperties(
IsolateData* isolate_data, v8::Local<v8::FunctionTemplate> target);
static void CreatePerIsolateProperties(IsolateData* isolate_data,
v8::Local<v8::ObjectTemplate> target);
static void CreatePerContextProperties(v8::Local<v8::Object> target,
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context,
Expand Down
4 changes: 2 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1393,9 +1393,9 @@ void MicrotaskQueueWrap::RegisterExternalReferences(
}

void CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> ctor) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> target = ctor->InstanceTemplate();

ContextifyContext::CreatePerIsolateProperties(isolate_data, target);
ContextifyScript::CreatePerIsolateProperties(isolate_data, target);
MicrotaskQueueWrap::CreatePerIsolateProperties(isolate_data, target);
Expand Down
5 changes: 2 additions & 3 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2827,9 +2827,8 @@ InternalFieldInfoBase* BindingData::Serialize(int index) {
}

static void CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> ctor) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> target = ctor->InstanceTemplate();

SetMethod(isolate, target, "access", Access);
SetMethod(isolate, target, "close", Close);
Expand Down Expand Up @@ -2873,7 +2872,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,

SetMethod(isolate, target, "mkdtemp", Mkdtemp);

StatWatcher::CreatePerIsolateProperties(isolate_data, ctor);
StatWatcher::CreatePerIsolateProperties(isolate_data, target);

target->Set(
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"),
Expand Down
19 changes: 9 additions & 10 deletions src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -860,17 +860,16 @@ static void GetStringWidth(const FunctionCallbackInfo<Value>& args) {
}

static void CreatePerIsolateProperties(IsolateData* isolate_data,
Local<FunctionTemplate> target) {
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
Local<ObjectTemplate> proto = target->PrototypeTemplate();

SetMethod(isolate, proto, "toUnicode", ToUnicode);
SetMethod(isolate, proto, "toASCII", ToASCII);
SetMethod(isolate, proto, "getStringWidth", GetStringWidth);
SetMethod(isolate, target, "toUnicode", ToUnicode);
SetMethod(isolate, target, "toASCII", ToASCII);
SetMethod(isolate, target, "getStringWidth", GetStringWidth);

// One-shot converters
SetMethod(isolate, proto, "icuErrName", ICUErrorName);
SetMethod(isolate, proto, "transcode", Transcode);
SetMethod(isolate, target, "icuErrName", ICUErrorName);
SetMethod(isolate, target, "transcode", Transcode);

// ConverterObject
{
Expand All @@ -883,9 +882,9 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
isolate_data->set_i18n_converter_template(t->InstanceTemplate());
}

SetMethod(isolate, proto, "getConverter", ConverterObject::Create);
SetMethod(isolate, proto, "decode", ConverterObject::Decode);
SetMethod(isolate, proto, "hasConverter", ConverterObject::Has);
SetMethod(isolate, target, "getConverter", ConverterObject::Create);
SetMethod(isolate, target, "decode", ConverterObject::Decode);
SetMethod(isolate, target, "hasConverter", ConverterObject::Has);
}

void CreatePerContextProperties(Local<Object> target,
Expand Down
Loading

0 comments on commit c0365fd

Please sign in to comment.