Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
chakrashim: fixing ObjectTemplate implementation
Browse files Browse the repository at this point in the history
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: #439
Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
  • Loading branch information
kfarnung committed Nov 30, 2017
1 parent 74d08c6 commit ca720a3
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 31 deletions.
9 changes: 5 additions & 4 deletions deps/chakrashim/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -2366,13 +2366,13 @@ struct IndexedPropertyHandlerConfiguration {

class V8_EXPORT ObjectTemplate : public Template {
public:
static Local<ObjectTemplate> New(Isolate* isolate);
static Local<ObjectTemplate> New(
Isolate* isolate,
Local<FunctionTemplate> constructor = Local<FunctionTemplate>());

V8_DEPRECATE_SOON("Use maybe version", Local<Object> NewInstance());
V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(Local<Context> context);

void SetClassName(Handle<String> name);

void SetAccessor(Handle<String> name,
AccessorGetterCallback getter,
AccessorSetterCallback setter = 0,
Expand Down Expand Up @@ -2423,12 +2423,13 @@ class V8_EXPORT ObjectTemplate : public Template {
Handle<Value> data = Handle<Value>());

private:
friend class FunctionTemplate;
friend class FunctionCallbackData;
friend class FunctionTemplateData;
friend class Utils;

Local<Object> NewInstance(Handle<Object> prototype);
Handle<String> GetClassName();
void SetConstructor(Handle<FunctionTemplate> constructor);
};

class V8_EXPORT External : public Value {
Expand Down
27 changes: 18 additions & 9 deletions deps/chakrashim/src/v8functiontemplate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ class FunctionTemplateData : public TemplateData {
Persistent<ObjectTemplate> instanceTemplate;
Persistent<FunctionTemplate> parent;
Persistent<Function> functionInstance;
Persistent<String> className;
bool removePrototype;

public:
Expand All @@ -191,6 +192,7 @@ class FunctionTemplateData : public TemplateData {
instanceTemplate.Reset();
parent.Reset();
functionInstance.Reset();
className.Reset();
}

static void CHAKRA_CALLBACK FinalizeCallback(void *data) {
Expand All @@ -210,6 +212,10 @@ class FunctionTemplateData : public TemplateData {
this->removePrototype = removePrototype;
}

void SetClassName(Handle<String> className) {
this->className = className;
}

void Inherit(Local<FunctionTemplate> parent) {
this->parent = parent;
}
Expand Down Expand Up @@ -237,13 +243,10 @@ class FunctionTemplateData : public TemplateData {
return nullptr;
}

Local<String> className = !instanceTemplate.IsEmpty() ?
instanceTemplate->GetClassName() : Local<String>();

JsValueRef function = nullptr;
{
if (!className.IsEmpty()) {
error = JsCreateNamedFunction(*className,
if (!this->className.IsEmpty()) {
error = JsCreateNamedFunction(*this->className,
FunctionCallbackData::FunctionInvoked,
funcCallbackObjectRef, &function);
} else {
Expand Down Expand Up @@ -368,7 +371,10 @@ Local<ObjectTemplate> FunctionTemplate::InstanceTemplate() {
return Local<ObjectTemplate>();
}

return functionTemplateData->EnsureInstanceTemplate();
Local<ObjectTemplate> instanceTemplate =
functionTemplateData->EnsureInstanceTemplate();
instanceTemplate->SetConstructor(this);
return instanceTemplate;
}

Local<ObjectTemplate> FunctionTemplate::PrototypeTemplate() {
Expand All @@ -382,10 +388,13 @@ Local<ObjectTemplate> FunctionTemplate::PrototypeTemplate() {
}

void FunctionTemplate::SetClassName(Handle<String> name) {
Local<ObjectTemplate> 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) {
Expand Down
48 changes: 30 additions & 18 deletions deps/chakrashim/src/v8objecttemplate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,23 @@ class ObjectTemplateData : public TemplateData {
static const ExternalDataTypes ExternalDataType =
ExternalDataTypes::ObjectTemplateData;

Persistent<String> className;

SetterGetterInterceptor * setterGetterInterceptor;
FunctionCallback functionCallDelegate;
Persistent<Value> functionCallDelegateInterceptorData;
Persistent<FunctionTemplate> constructor;
int internalFieldCount;

ObjectTemplateData()
explicit ObjectTemplateData(Handle<FunctionTemplate> constructor)
: TemplateData(ExternalDataType),
setterGetterInterceptor(nullptr),
functionCallDelegate(nullptr),
functionCallDelegateInterceptorData(nullptr),
constructor(*constructor),
internalFieldCount(0) {
}

~ObjectTemplateData() {
className.Reset();
constructor.Reset();

if (setterGetterInterceptor != nullptr) {
setterGetterInterceptor->namedPropertyInterceptorData.Reset();
Expand Down Expand Up @@ -822,9 +822,10 @@ JsValueRef CHAKRA_CALLBACK Utils::DefinePropertyCallback(
return jsrt::GetTrue();
}

Local<ObjectTemplate> ObjectTemplate::New(Isolate* isolate) {
Local<ObjectTemplate> ObjectTemplate::New(Isolate* isolate,
Local<FunctionTemplate> constructor) {
JsValueRef objectTemplateRef;
ObjectTemplateData* templateData = new ObjectTemplateData();
ObjectTemplateData* templateData = new ObjectTemplateData(constructor);

if (JsCreateExternalObject(templateData,
ObjectTemplateData::FinalizeCallback,
Expand Down Expand Up @@ -868,6 +869,27 @@ Local<Object> ObjectTemplate::NewInstance(Handle<Object> prototype) {
return Local<Object>();
}

// 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> 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<Object>::New(prototypeValue);
}
}
}
}

if (!prototype.IsEmpty()) {
if (JsSetPrototype(newInstanceRef,
reinterpret_cast<JsValueRef>(*prototype)) != JsNoError) {
Expand Down Expand Up @@ -1096,24 +1118,14 @@ void ObjectTemplate::SetInternalFieldCount(int value) {
objectTemplateData->internalFieldCount = value;
}

void ObjectTemplate::SetClassName(Handle<String> className) {
void ObjectTemplate::SetConstructor(Handle<FunctionTemplate> constructor) {
ObjectTemplateData* objectTemplateData = nullptr;
if (!ExternalData::TryGet(this, &objectTemplateData)) {
CHAKRA_ASSERT(false); // This should never happen
return;
}

objectTemplateData->className = className;
}

Handle<String> ObjectTemplate::GetClassName() {
ObjectTemplateData* objectTemplateData = nullptr;
if (!ExternalData::TryGet(this, &objectTemplateData)) {
CHAKRA_ASSERT(false); // This should never happen
return Handle<String>();
}

return objectTemplateData->className;
objectTemplateData->constructor = constructor;
}

} // namespace v8

0 comments on commit ca720a3

Please sign in to comment.