From ca720a3ed621af4129eb9885dd6590cb27dcfe64 Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Thu, 30 Nov 2017 12:04:24 -0800 Subject: [PATCH] chakrashim: fixing ObjectTemplate implementation It's possible to create an object template with a constructor in V8, but we didn't support it. It's a little weird (even in V8) since objects constructed this way will never trigger the CallHandler of the FunctionTemplate object, it just inherits the prototype from it. PR-URL: https://github.com/nodejs/node-chakracore/pull/439 Reviewed-By: Jimmy Thomson --- deps/chakrashim/include/v8.h | 9 +++-- deps/chakrashim/src/v8functiontemplate.cc | 27 ++++++++----- deps/chakrashim/src/v8objecttemplate.cc | 48 ++++++++++++++--------- 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h index 1dbe9c1df93..77b0eeead48 100644 --- a/deps/chakrashim/include/v8.h +++ b/deps/chakrashim/include/v8.h @@ -2366,13 +2366,13 @@ struct IndexedPropertyHandlerConfiguration { class V8_EXPORT ObjectTemplate : public Template { public: - static Local New(Isolate* isolate); + static Local New( + Isolate* isolate, + Local constructor = Local()); V8_DEPRECATE_SOON("Use maybe version", Local NewInstance()); V8_WARN_UNUSED_RESULT MaybeLocal NewInstance(Local context); - void SetClassName(Handle name); - void SetAccessor(Handle name, AccessorGetterCallback getter, AccessorSetterCallback setter = 0, @@ -2423,12 +2423,13 @@ class V8_EXPORT ObjectTemplate : public Template { Handle data = Handle()); private: + friend class FunctionTemplate; friend class FunctionCallbackData; friend class FunctionTemplateData; friend class Utils; Local NewInstance(Handle prototype); - Handle GetClassName(); + void SetConstructor(Handle constructor); }; class V8_EXPORT External : public Value { diff --git a/deps/chakrashim/src/v8functiontemplate.cc b/deps/chakrashim/src/v8functiontemplate.cc index 4d32b0cda57..abf2b863fee 100755 --- a/deps/chakrashim/src/v8functiontemplate.cc +++ b/deps/chakrashim/src/v8functiontemplate.cc @@ -170,6 +170,7 @@ class FunctionTemplateData : public TemplateData { Persistent instanceTemplate; Persistent parent; Persistent functionInstance; + Persistent className; bool removePrototype; public: @@ -191,6 +192,7 @@ class FunctionTemplateData : public TemplateData { instanceTemplate.Reset(); parent.Reset(); functionInstance.Reset(); + className.Reset(); } static void CHAKRA_CALLBACK FinalizeCallback(void *data) { @@ -210,6 +212,10 @@ class FunctionTemplateData : public TemplateData { this->removePrototype = removePrototype; } + void SetClassName(Handle className) { + this->className = className; + } + void Inherit(Local parent) { this->parent = parent; } @@ -237,13 +243,10 @@ class FunctionTemplateData : public TemplateData { return nullptr; } - Local className = !instanceTemplate.IsEmpty() ? - instanceTemplate->GetClassName() : Local(); - JsValueRef function = nullptr; { - if (!className.IsEmpty()) { - error = JsCreateNamedFunction(*className, + if (!this->className.IsEmpty()) { + error = JsCreateNamedFunction(*this->className, FunctionCallbackData::FunctionInvoked, funcCallbackObjectRef, &function); } else { @@ -368,7 +371,10 @@ Local FunctionTemplate::InstanceTemplate() { return Local(); } - return functionTemplateData->EnsureInstanceTemplate(); + Local instanceTemplate = + functionTemplateData->EnsureInstanceTemplate(); + instanceTemplate->SetConstructor(this); + return instanceTemplate; } Local FunctionTemplate::PrototypeTemplate() { @@ -382,10 +388,13 @@ Local FunctionTemplate::PrototypeTemplate() { } void FunctionTemplate::SetClassName(Handle name) { - Local instanceTemplate = InstanceTemplate(); - if (!instanceTemplate.IsEmpty()) { - instanceTemplate->SetClassName(name); + FunctionTemplateData* functionTemplateData = nullptr; + if (!ExternalData::TryGet(this, &functionTemplateData)) { + CHAKRA_ASSERT(false); // This should never happen + return; } + + return functionTemplateData->SetClassName(name); } void FunctionTemplate::SetHiddenPrototype(bool value) { diff --git a/deps/chakrashim/src/v8objecttemplate.cc b/deps/chakrashim/src/v8objecttemplate.cc index f11afc60613..96e795d6976 100755 --- a/deps/chakrashim/src/v8objecttemplate.cc +++ b/deps/chakrashim/src/v8objecttemplate.cc @@ -27,23 +27,23 @@ class ObjectTemplateData : public TemplateData { static const ExternalDataTypes ExternalDataType = ExternalDataTypes::ObjectTemplateData; - Persistent className; - SetterGetterInterceptor * setterGetterInterceptor; FunctionCallback functionCallDelegate; Persistent functionCallDelegateInterceptorData; + Persistent constructor; int internalFieldCount; - ObjectTemplateData() + explicit ObjectTemplateData(Handle constructor) : TemplateData(ExternalDataType), setterGetterInterceptor(nullptr), functionCallDelegate(nullptr), functionCallDelegateInterceptorData(nullptr), + constructor(*constructor), internalFieldCount(0) { } ~ObjectTemplateData() { - className.Reset(); + constructor.Reset(); if (setterGetterInterceptor != nullptr) { setterGetterInterceptor->namedPropertyInterceptorData.Reset(); @@ -822,9 +822,10 @@ JsValueRef CHAKRA_CALLBACK Utils::DefinePropertyCallback( return jsrt::GetTrue(); } -Local ObjectTemplate::New(Isolate* isolate) { +Local ObjectTemplate::New(Isolate* isolate, + Local constructor) { JsValueRef objectTemplateRef; - ObjectTemplateData* templateData = new ObjectTemplateData(); + ObjectTemplateData* templateData = new ObjectTemplateData(constructor); if (JsCreateExternalObject(templateData, ObjectTemplateData::FinalizeCallback, @@ -868,6 +869,27 @@ Local ObjectTemplate::NewInstance(Handle prototype) { return Local(); } + // If there was no prototype specifically provided, then try to get one from + // the constructor, if it exists. It's possible that the constructor + // function doesn't have a prototype to provide. + if (prototype.IsEmpty()) { + if (!objectTemplateData->constructor.IsEmpty()) { + Local function = objectTemplateData->constructor->GetFunction(); + + if (!function.IsEmpty()) { + jsrt::IsolateShim* iso = jsrt::IsolateShim::GetCurrent(); + JsValueRef prototypeValue = nullptr; + + if (JsGetProperty(*function, + iso->GetCachedPropertyIdRef( + jsrt::CachedPropertyIdRef::prototype), + &prototypeValue) == JsNoError) { + prototype = Local::New(prototypeValue); + } + } + } + } + if (!prototype.IsEmpty()) { if (JsSetPrototype(newInstanceRef, reinterpret_cast(*prototype)) != JsNoError) { @@ -1096,24 +1118,14 @@ void ObjectTemplate::SetInternalFieldCount(int value) { objectTemplateData->internalFieldCount = value; } -void ObjectTemplate::SetClassName(Handle className) { +void ObjectTemplate::SetConstructor(Handle constructor) { ObjectTemplateData* objectTemplateData = nullptr; if (!ExternalData::TryGet(this, &objectTemplateData)) { CHAKRA_ASSERT(false); // This should never happen return; } - objectTemplateData->className = className; -} - -Handle ObjectTemplate::GetClassName() { - ObjectTemplateData* objectTemplateData = nullptr; - if (!ExternalData::TryGet(this, &objectTemplateData)) { - CHAKRA_ASSERT(false); // This should never happen - return Handle(); - } - - return objectTemplateData->className; + objectTemplateData->constructor = constructor; } } // namespace v8