Skip to content

Commit

Permalink
fix: Avoid setting the same property on an ObjectTemplate twice
Browse files Browse the repository at this point in the history 
Due to Kotlin extension methods with the same names as Java methods, we
may be setting the same property twice on a class's prototype template.
The V8 API does not allow this and will fail a dcheck.

Since there is no API to check if a property has already been added to
v8::ObjectTemplate, wrap the prototype template in a small class
PrototypeTemplateFiller which keeps track of what's already been added and
avoids adding it again.
  • Loading branch information
@ptomato @edusperoni
ptomato authored and edusperoni committed Jun 23, 2023
1 parent 9dfae65 commit 9e610c8
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 53 deletions.
23 changes: 23 additions & 0 deletions test-app/app/src/main/assets/app/tests/kotlin/extensions/testExtensionFunctionsSupport.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,27 @@ describe("Tests Kotlin extension functions support", function () {
expect(hasException).toBe(false);
});

describe("Kotlin extension functions that shadow Java functions", function () {
let handler;
beforeEach(function () {
handler = new android.os.Handler(android.os.Looper.getMainLooper());
});

it("should call the Java function", function (done) {
const run = jasmine.createSpy("java.lang.Runnable.run").and.callFake(function () {
done();
});
const javaRunnable = new java.lang.Runnable({run});
handler.postAtTime(javaRunnable, 1);
})

it("should call the Kotlin function", function (done) {
const invoke = jasmine.createSpy("kotlin.jvm.functions.Function0.invoke").and.callFake(function () {
done();
});
const cancelToken = new java.lang.Object();
const kotlinFunc = new kotlin.jvm.functions.Function0({invoke});
handler.postAtTime(1, cancelToken, kotlinFunc);
})
});
});
148 changes: 102 additions & 46 deletions test-app/runtime/src/main/cpp/MetadataNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,23 +483,86 @@ void MetadataNode::SuperAccessorGetterCallback(Local<Name> property, const Prope
}
}

std::vector<MetadataNode::MethodCallbackData*> MetadataNode::SetInstanceMembers(Isolate* isolate, Local<FunctionTemplate>& ctorFuncTemplate, Local<ObjectTemplate>& prototypeTemplate, vector<MethodCallbackData*>& instanceMethodsCallbackData, const vector<MethodCallbackData*>& baseInstanceMethodsCallbackData, MetadataTreeNode* treeNode) {
class MetadataNode::PrototypeTemplateFiller {
public:
explicit PrototypeTemplateFiller(Local<ObjectTemplate> protoTemplate)
: m_protoTemplate(protoTemplate) {}

void FillPrototypeMethod(Isolate *isolate, const std::string& name,
MethodCallbackData* methodInfo) {
if (m_addedNames.count(name) != 0) {
DEBUG_WRITE("Not defining method prototype.%s which has already been added", name.c_str());
return;
}

Local<External> funcData = External::New(isolate, methodInfo);
Local<FunctionTemplate> funcTemplate = FunctionTemplate::New(isolate, MetadataNode::MethodCallback, funcData);
Local<String> nameString = ArgConverter::ConvertToV8String(isolate, name);
m_protoTemplate->Set(nameString, funcTemplate);
m_addedNames.insert(name);
}

void FillPrototypeField(Isolate* isolate, const std::string& name, FieldCallbackData* fieldInfo,
AccessControl access = AccessControl::DEFAULT) {
if (m_addedNames.count(name) != 0) {
DEBUG_WRITE("Not defining field prototype.%s which already exists", name.c_str());
return;
}

Local<External> fieldData = External::New(isolate, fieldInfo);
Local<Name> nameString = ArgConverter::ConvertToV8String(isolate, name);
m_protoTemplate->SetAccessor(nameString, MetadataNode::FieldAccessorGetterCallback,
MetadataNode::FieldAccessorSetterCallback, fieldData, access,
PropertyAttribute::DontDelete);
m_addedNames.insert(name);
}

void FillPrototypeProperty(Isolate* isolate, const std::string& name,
PropertyCallbackData* propertyInfo) {
if (m_addedNames.count(name) != 0) {
DEBUG_WRITE("Not defining property prototype.%s which already exists", name.c_str());
return;
}

Local<External> propertyData = External::New(isolate, propertyInfo);
Local<Name> nameString = ArgConverter::ConvertToV8String(isolate, name);
m_protoTemplate->SetAccessor(nameString, MetadataNode::PropertyAccessorGetterCallback,
MetadataNode::PropertyAccessorSetterCallback, propertyData,
AccessControl::DEFAULT, PropertyAttribute::DontDelete);
m_addedNames.insert(name);
}

private:
Local<ObjectTemplate> m_protoTemplate;
std::unordered_set<std::string> m_addedNames;
};

std::vector<MetadataNode::MethodCallbackData*> MetadataNode::SetInstanceMembers(
Isolate* isolate, Local<FunctionTemplate>& ctorFuncTemplate,
PrototypeTemplateFiller& protoFiller,
vector<MethodCallbackData*>& instanceMethodsCallbackData,
const vector<MethodCallbackData*>& baseInstanceMethodsCallbackData,
MetadataTreeNode* treeNode) {
auto hasCustomMetadata = treeNode->metadata != nullptr;

if (hasCustomMetadata) {
return SetInstanceMembersFromRuntimeMetadata(isolate, ctorFuncTemplate, prototypeTemplate, instanceMethodsCallbackData, baseInstanceMethodsCallbackData, treeNode);
} else {
SetInstanceFieldsFromStaticMetadata(isolate, ctorFuncTemplate, prototypeTemplate, instanceMethodsCallbackData, baseInstanceMethodsCallbackData, treeNode);
return SetInstanceMethodsFromStaticMetadata(isolate, ctorFuncTemplate, prototypeTemplate, instanceMethodsCallbackData, baseInstanceMethodsCallbackData, treeNode);
return SetInstanceMembersFromRuntimeMetadata(
isolate, protoFiller, instanceMethodsCallbackData,
baseInstanceMethodsCallbackData, treeNode);
}

SetInstanceFieldsFromStaticMetadata(isolate, protoFiller, treeNode);
return SetInstanceMethodsFromStaticMetadata(
isolate, ctorFuncTemplate, protoFiller, instanceMethodsCallbackData,
baseInstanceMethodsCallbackData, treeNode);
}

vector<MetadataNode::MethodCallbackData *> MetadataNode::SetInstanceMethodsFromStaticMetadata(Isolate *isolate,
Local<FunctionTemplate> &ctorFuncTemplate,
Local<ObjectTemplate> &prototypeTemplate,
vector<MethodCallbackData *> &instanceMethodsCallbackData,
const vector<MethodCallbackData *> &baseInstanceMethodsCallbackData,
MetadataTreeNode *treeNode) {
vector<MetadataNode::MethodCallbackData *> MetadataNode::SetInstanceMethodsFromStaticMetadata(
Isolate *isolate, Local<FunctionTemplate> &ctorFuncTemplate,
PrototypeTemplateFiller& protoFiller,
vector<MethodCallbackData *> &instanceMethodsCallbackData,
const vector<MethodCallbackData *> &baseInstanceMethodsCallbackData,
MetadataTreeNode *treeNode) {
SET_PROFILER_FRAME();

std::vector<MethodCallbackData *> instanceMethodData;
Expand All @@ -520,7 +583,6 @@ vector<MetadataNode::MethodCallbackData *> MetadataNode::SetInstanceMethodsFromS
MethodCallbackData *callbackData = nullptr;

auto context = isolate->GetCurrentContext();
auto origin = Constants::APP_ROOT_FOLDER_PATH + GetOrCreateInternal(treeNode)->m_name;

std::unordered_map<std::string, MethodCallbackData *> collectedExtensionMethodDatas;

Expand All @@ -535,10 +597,7 @@ vector<MetadataNode::MethodCallbackData *> MetadataNode::SetInstanceMethodsFromS
entry.name);
if (callbackData == nullptr) {
callbackData = new MethodCallbackData(this);
auto funcData = External::New(isolate, callbackData);
auto funcTemplate = FunctionTemplate::New(isolate, MethodCallback, funcData);
auto funcName = ArgConverter::ConvertToV8String(isolate, entry.name);
prototypeTemplate->Set(funcName, funcTemplate);
protoFiller.FillPrototypeMethod(isolate, entry.name, callbackData);

lastMethodName = entry.name;
std::pair<std::string, MethodCallbackData *> p(entry.name, callbackData);
Expand Down Expand Up @@ -581,25 +640,24 @@ vector<MetadataNode::MethodCallbackData *> MetadataNode::SetInstanceMethodsFromS
callbackData->parent = *itFound;
}

auto funcData = External::New(isolate, callbackData);
auto funcTemplate = FunctionTemplate::New(isolate, MethodCallback, funcData);

auto funcName = ArgConverter::ConvertToV8String(isolate, entry.name);

if (s_profilerEnabled) {
Local<External> funcData = External::New(isolate, callbackData);
Local<FunctionTemplate> funcTemplate = FunctionTemplate::New(isolate, MethodCallback, funcData);
auto func = funcTemplate->GetFunction(context).ToLocalChecked();
std::string origin = Constants::APP_ROOT_FOLDER_PATH + GetOrCreateInternal(treeNode)->m_name;
Local<Function> wrapperFunc = Wrap(isolate, func, entry.name, origin,
false /* isCtorFunc */);
Local<Function> ctorFunc = ctorFuncTemplate->GetFunction(context).ToLocalChecked();
Local<Value> protoVal;
ctorFunc->Get(context,
ArgConverter::ConvertToV8String(isolate, "prototype")).ToLocal(
&protoVal);
Local<String> funcName = ArgConverter::ConvertToV8String(isolate, entry.name);
if (!protoVal.IsEmpty() && !protoVal->IsUndefined() && !protoVal->IsNull()) {
protoVal.As<Object>()->Set(context, funcName, wrapperFunc);
}
} else {
prototypeTemplate->Set(funcName, funcTemplate);
protoFiller.FillPrototypeMethod(isolate, entry.name, callbackData);
}

lastMethodName = entry.name;
Expand Down Expand Up @@ -628,7 +686,8 @@ MetadataNode::MethodCallbackData *MetadataNode::tryGetExtensionMethodCallbackDat
return nullptr;
}

void MetadataNode::SetInstanceFieldsFromStaticMetadata(Isolate* isolate, Local<FunctionTemplate>& ctorFuncTemplate, Local<ObjectTemplate>& prototypeTemplate, vector<MethodCallbackData*>& instanceMethodsCallbackData, const vector<MethodCallbackData*>& baseInstanceMethodsCallbackData, MetadataTreeNode* treeNode) {
void MetadataNode::SetInstanceFieldsFromStaticMetadata(
Isolate* isolate, PrototypeTemplateFiller& prototypeTemplate, MetadataTreeNode* treeNode) {
SET_PROFILER_FRAME();

Local<Function> ctorFunction;
Expand Down Expand Up @@ -665,12 +724,9 @@ void MetadataNode::SetInstanceFieldsFromStaticMetadata(Isolate* isolate, Local<F
curPtr += sizeof(uint16_t);
for (auto i = 0; i < instanceFieldCout; i++) {
auto entry = s_metadataReader.ReadInstanceFieldEntry(&curPtr);

auto fieldName = ArgConverter::ConvertToV8String(isolate, entry.name);
auto fieldInfo = new FieldCallbackData(entry);
fieldInfo->declaringType = curType;
auto fieldData = External::New(isolate, fieldInfo);
prototypeTemplate->SetAccessor(fieldName, FieldAccessorGetterCallback, FieldAccessorSetterCallback, fieldData, AccessControl::DEFAULT, PropertyAttribute::DontDelete);
prototypeTemplate.FillPrototypeField(isolate, entry.name, fieldInfo);
}

auto kotlinPropertiesCount = *reinterpret_cast<uint16_t*>(curPtr);
Expand Down Expand Up @@ -699,12 +755,15 @@ void MetadataNode::SetInstanceFieldsFromStaticMetadata(Isolate* isolate, Local<F
}

auto propertyInfo = new PropertyCallbackData(propertyName, getterMethodName, setterMethodName);
auto propertyData = External::New(isolate, propertyInfo);
prototypeTemplate->SetAccessor(ArgConverter::ConvertToV8String(isolate, propertyName), PropertyAccessorGetterCallback, PropertyAccessorSetterCallback, propertyData, AccessControl::DEFAULT, PropertyAttribute::DontDelete);
prototypeTemplate.FillPrototypeProperty(isolate, propertyName, propertyInfo);
}
}

vector<MetadataNode::MethodCallbackData*> MetadataNode::SetInstanceMembersFromRuntimeMetadata(Isolate* isolate, Local<FunctionTemplate>& ctorFuncTemplate, Local<ObjectTemplate>& prototypeTemplate, vector<MethodCallbackData*>& instanceMethodsCallbackData, const vector<MethodCallbackData*>& baseInstanceMethodsCallbackData, MetadataTreeNode* treeNode) {
vector<MetadataNode::MethodCallbackData*> MetadataNode::SetInstanceMembersFromRuntimeMetadata(
Isolate* isolate, PrototypeTemplateFiller& protoFiller,
vector<MethodCallbackData*>& instanceMethodsCallbackData,
const vector<MethodCallbackData*>& baseInstanceMethodsCallbackData,
MetadataTreeNode* treeNode) {
SET_PROFILER_FRAME();

assert(treeNode->metadata != nullptr);
Expand Down Expand Up @@ -757,18 +816,14 @@ vector<MetadataNode::MethodCallbackData*> MetadataNode::SetInstanceMembersFromRu
callbackData->parent = *itFound;
}

auto funcData = External::New(isolate, callbackData);
auto funcTemplate = FunctionTemplate::New(isolate, MethodCallback, funcData);
auto funcName = ArgConverter::ConvertToV8String(isolate, entry.name);
prototypeTemplate->Set(funcName, funcTemplate);
protoFiller.FillPrototypeMethod(isolate, entry.name, callbackData);
lastMethodName = entry.name;
}
callbackData->candidates.push_back(entry);
} else if (chKind == 'F') {
auto fieldName = ArgConverter::ConvertToV8String(isolate, entry.name);
auto fieldData = External::New(isolate, new FieldCallbackData(entry));
auto* fieldInfo = new FieldCallbackData(entry);
auto access = entry.isFinal ? AccessControl::ALL_CAN_READ : AccessControl::DEFAULT;
prototypeTemplate->SetAccessor(fieldName, FieldAccessorGetterCallback, FieldAccessorSetterCallback, fieldData, access, PropertyAttribute::DontDelete);
protoFiller.FillPrototypeField(isolate, entry.name, fieldInfo, access);
}
}
return instanceMethodData;
Expand Down Expand Up @@ -974,11 +1029,13 @@ Local<FunctionTemplate> MetadataNode::GetConstructorFunctionTemplate(Isolate* is
break;
}

auto prototypeTemplate = ctorFuncTemplate->PrototypeTemplate();
PrototypeTemplateFiller protoFiller{ctorFuncTemplate->PrototypeTemplate()};

auto instanceMethodData = node->SetInstanceMembers(isolate, ctorFuncTemplate, prototypeTemplate, instanceMethodsCallbackData, baseInstanceMethodsCallbackData, treeNode);
auto instanceMethodData = node->SetInstanceMembers(
isolate, ctorFuncTemplate, protoFiller, instanceMethodsCallbackData,
baseInstanceMethodsCallbackData, treeNode);
if (!skippedBaseTypes.empty()) {
node->SetMissingBaseMethods(isolate, skippedBaseTypes, instanceMethodData, prototypeTemplate);
node->SetMissingBaseMethods(isolate, skippedBaseTypes, instanceMethodData, protoFiller);
}

auto context = isolate->GetCurrentContext();
Expand Down Expand Up @@ -2075,7 +2132,10 @@ bool MetadataNode::CheckClassHierarchy(JEnv& env, jclass currentClass, MetadataT
* This method handles scenrios like Bundle/BaseBundle class hierarchy change in API level 21.
* See https://github.com/NativeScript/android-runtime/issues/628
*/
void MetadataNode::SetMissingBaseMethods(Isolate* isolate, const vector<MetadataTreeNode*>& skippedBaseTypes, const vector<MethodCallbackData*>& instanceMethodData, Local<ObjectTemplate>& prototypeTemplate) {
void MetadataNode::SetMissingBaseMethods(
Isolate* isolate, const vector<MetadataTreeNode*>& skippedBaseTypes,
const vector<MethodCallbackData*>& instanceMethodData,
PrototypeTemplateFiller& protoFiller) {
for (auto treeNode: skippedBaseTypes) {
uint8_t* curPtr = s_metadataReader.GetValueData() + treeNode->offsetValue + 1;

Expand Down Expand Up @@ -2111,11 +2171,7 @@ void MetadataNode::SetMissingBaseMethods(Isolate* isolate, const vector<Metadata

if (callbackData == nullptr) {
callbackData = new MethodCallbackData(this);

auto funcData = External::New(isolate, callbackData);
auto funcTemplate = FunctionTemplate::New(isolate, MethodCallback, funcData);
auto funcName = ArgConverter::ConvertToV8String(isolate, entry.name);
prototypeTemplate->Set(funcName, funcTemplate);
protoFiller.FillPrototypeMethod(isolate, entry.name, callbackData);
}

bool foundSameSig = false;
Expand Down

0 comments on commit 9e610c8

Please sign in to comment.