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

chakrashim: fixing ObjectTemplate implementation #439

Merged
merged 1 commit into from
Nov 30, 2017
Merged

chakrashim: fixing ObjectTemplate implementation #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions deps/chakrashim/include/v8.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you chose not to make this an else?

I'd also like to see a comment here explaining the scenario this is supporting; it's something that I could easily see being broken if it is forgotten.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that the previous conditional is just an alternate way to get the prototype. It could have already been provided directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I'd missed that it was setting prototype

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