Skip to content

Commit

Permalink
Fixed issue [#6 instanceof regression]
Browse files Browse the repository at this point in the history
- Java side handled prototypes incorrectly, test was also incorrect -- fixed
- Added a default constructor for NodeInstance() in an attempt to find a fix for issue [#3], doesn't fix the problem yet
- Removed commented out code
- Moved prototype functions back from JSFunction to JSObject where they belong
  • Loading branch information
ericwlange committed Dec 7, 2016
1 parent bfc9a34 commit cb91b46
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 120 deletions.
4 changes: 0 additions & 4 deletions NodeDroid-library/jni/JSC/JSC_JSContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,6 @@ JS_EXPORT JSObjectRef JSContextGetGlobalObject(JSContextRef ctx)

V8_ISOLATE_CTX(ctx->Context(),isolate,Ctx)
JSValue<Object> *object = context_->Global();
/*
MaybeLocal<Object> proto = object->Value()->GetPrototype()->ToObject(Ctx);
Local<Object> global = proto.IsEmpty() ? object->Value() : proto.ToLocalChecked();
*/
Local<Object> global = object->Value();
v = JSValue<Value>::New(context_, global);
object->release();
Expand Down
6 changes: 0 additions & 6 deletions NodeDroid-library/jni/JSC/JSC_JSObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1058,14 +1058,8 @@ JS_EXPORT JSObjectRef JSObjectMake(JSContextRef ctx, JSClassRef jsClass, void* d
instance->SetPrivate(context, privateKey,
Number::New(isolate,(double)reinterpret_cast<long>(data)));
value = VALUE(jsClass->InitInstance(ctx, instance, payload));
/*
JSObjectRef proto = VALUE(jsClass->InitInstance(ctx, instance, payload));
value = JSValue<Value>::New(context_, Object::New(isolate));
VALUE(value)->Value()->ToObject(context).ToLocalChecked()->SetPrototype(context, VALUE(proto)->Value());
*/
} else {
value = JSValue<Value>::New(context_, Object::New(isolate));
//JSObjectSetPrivate(value, data);
}
V8_UNLOCK()

Expand Down
15 changes: 0 additions & 15 deletions NodeDroid-library/jni/androidTest/CustomGlobalObjectClassTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

#include "CustomGlobalObjectClassTest.h"

//#include <JavaScriptCore/JSObjectRefPrivate.h>
#include <JavaScriptCore/JavaScript.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -131,18 +130,4 @@ void globalObjectPrivatePropertyTest()
JSClassRef global = JSClassCreate(&definition);
JSGlobalContextRef context = JSGlobalContextCreate(global);
JSObjectRef globalObject = JSContextGetGlobalObject(context);

/*
JSStringRef privateName = JSStringCreateWithUTF8CString("private");
JSValueRef privateValue = JSValueMakeString(context, privateName);
assertTrue(JSObjectSetPrivateProperty(context, globalObject, privateName, privateValue), "JSObjectSetPrivateProperty succeeded");
JSValueRef result = JSObjectGetPrivateProperty(context, globalObject, privateName);
assertTrue(JSValueIsStrictEqual(context, privateValue, result), "privateValue === \"private\"");
assertTrue(JSObjectDeletePrivateProperty(context, globalObject, privateName), "JSObjectDeletePrivateProperty succeeded");
result = JSObjectGetPrivateProperty(context, globalObject, privateName);
assertTrue(JSValueIsNull(context, result), "Deleted private property is indeed no longer present");
JSStringRelease(privateName);
*/
}
8 changes: 0 additions & 8 deletions NodeDroid-library/jni/androidTest/testapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include <android/log.h>
#include <jni.h>
#include <exception>
//#define ASSERT(x) if(!(x)) throw std::runtime_error( "Assertion failure: " #x);
#define ASSERT(x) assertTrue(x, #x);
#undef printf
#define printf(...) __android_log_print(ANDROID_LOG_INFO, "testapi", __VA_ARGS__)
Expand Down Expand Up @@ -1209,8 +1208,6 @@ extern "C" JNIEXPORT jint JNICALL Java_org_liquidplayer_test_JSC_main(JNIEnv* en
const char *scriptPath = "testapi.js";
const char *scriptUTF8 = env->GetStringUTFChars(testapi_js, NULL);

try {

// Test garbage collection with a fresh context
context = JSGlobalContextCreateInGroup(NULL, NULL);
TestInitializeFinalize = true;
Expand Down Expand Up @@ -1914,11 +1911,6 @@ extern "C" JNIEXPORT jint JNICALL Java_org_liquidplayer_test_JSC_main(JNIEnv* en
globalObjectSetPrototypeTest();
globalObjectPrivatePropertyTest();

} catch (std::exception& e) {
printf( "FAIL: %s\n", e.what() );
failed = true;
}

env->ReleaseStringUTFChars(testapi_js, scriptUTF8);

if (failed) {
Expand Down
7 changes: 7 additions & 0 deletions NodeDroid-library/jni/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//

#include "common.h"
#include "node/NodeInstance.h"
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>
Expand Down Expand Up @@ -115,10 +116,12 @@ void ContextGroup::init_v8() {
// see: https://github.com/nodejs/node/issues/7918
const char *flags = "--harmony-instanceof";
V8::SetFlagsFromString(flags, strlen(flags));

s_platform = platform::CreateDefaultPlatform(4);
V8::InitializePlatform(s_platform);
V8::Initialize();
}

s_mutex.unlock();
}

Expand All @@ -139,6 +142,10 @@ void ContextGroup::dispose_v8() {
GenericAllocator ContextGroup::s_allocator;

ContextGroup::ContextGroup() {
if (!s_init_count) {
NodeInstance init;
}

init_v8();
m_create_params.array_buffer_allocator = &s_allocator;
m_isolate = Isolate::New(m_create_params);
Expand Down
71 changes: 43 additions & 28 deletions NodeDroid-library/jni/node/NodeInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ NodeInstance::NodeInstance(JNIEnv* env, jobject thiz) {
node_main_thread = new std::thread(node_main_task,reinterpret_cast<void*>(this));
}

NodeInstance::NodeInstance() {
m_jvm = nullptr;
m_JavaThis = nullptr;
node_main_thread = new std::thread(node_main_task,reinterpret_cast<void*>(this));
}

NodeInstance::~NodeInstance() {
node_main_thread->join();
delete node_main_thread;
Expand All @@ -39,7 +45,12 @@ NodeInstance::~NodeInstance() {
void NodeInstance::spawnedThread()
{
enum { kMaxArgs = 64 };
char cmd[] = "node -e global.__nodedroid_onLoad();";
char cmd[60];
if (m_jvm) {
strcpy(cmd, "node -e global.__nodedroid_onLoad();");
} else {
strcpy(cmd, "node");
}

int argc = 0;
char *argv[kMaxArgs];
Expand All @@ -54,37 +65,39 @@ void NodeInstance::spawnedThread()

int ret = Start(argc, argv);

JNIEnv *env;
int getEnvStat = m_jvm->GetEnv((void**)&env, JNI_VERSION_1_6);
if (getEnvStat == JNI_EDETACHED) {
m_jvm->AttachCurrentThread(&env, NULL);
}
if (m_jvm) {
JNIEnv *env;
int getEnvStat = m_jvm->GetEnv((void**)&env, JNI_VERSION_1_6);
if (getEnvStat == JNI_EDETACHED) {
m_jvm->AttachCurrentThread(&env, NULL);
}

jclass cls = env->GetObjectClass(m_JavaThis);
jmethodID mid;
do {
mid = env->GetMethodID(cls,"onNodeExit","(J)V");
if (!env->ExceptionCheck()) break;
env->ExceptionClear();
jclass super = env->GetSuperclass(cls);
env->DeleteLocalRef(cls);
if (super == NULL || env->ExceptionCheck()) {
if (super != NULL) env->DeleteLocalRef(super);
if (getEnvStat == JNI_EDETACHED) {
m_jvm->DetachCurrentThread();
jclass cls = env->GetObjectClass(m_JavaThis);
jmethodID mid;
do {
mid = env->GetMethodID(cls,"onNodeExit","(J)V");
if (!env->ExceptionCheck()) break;
env->ExceptionClear();
jclass super = env->GetSuperclass(cls);
env->DeleteLocalRef(cls);
if (super == NULL || env->ExceptionCheck()) {
if (super != NULL) env->DeleteLocalRef(super);
if (getEnvStat == JNI_EDETACHED) {
m_jvm->DetachCurrentThread();
}
return;
}
return;
}
cls = super;
} while (true);
env->DeleteLocalRef(cls);
cls = super;
} while (true);
env->DeleteLocalRef(cls);

env->CallVoidMethod(m_JavaThis, mid, (jlong)ret);
env->CallVoidMethod(m_JavaThis, mid, (jlong)ret);

env->DeleteGlobalRef(m_JavaThis);
env->DeleteGlobalRef(m_JavaThis);

if (getEnvStat == JNI_EDETACHED) {
m_jvm->DetachCurrentThread();
if (getEnvStat == JNI_EDETACHED) {
m_jvm->DetachCurrentThread();
}
}

// Commit suicide
Expand Down Expand Up @@ -366,7 +379,9 @@ int NodeInstance::StartNodeInstance(void* arg) {
}

java_node_context->retain();
notify_start(java_node_context);
if (m_jvm) {
notify_start(java_node_context);
}

bool more;
do {
Expand Down
1 change: 1 addition & 0 deletions NodeDroid-library/jni/node/NodeInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ using namespace node;
class NodeInstance {
public:
NodeInstance(JNIEnv* env, jobject thiz);
NodeInstance();
virtual ~NodeInstance();

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public Integer[] arrayFunc() {
public class ConstructorFunction extends JSFunction {
public ConstructorFunction(JSContext ctx) {
super(ctx,"constructor");
prototype(new PrototypeObject(ctx));
property("prototype", new PrototypeObject(ctx));
}
@SuppressWarnings("unused")
public void constructor(int param) {
Expand Down Expand Up @@ -271,29 +271,29 @@ public void testJSFunctionConstructors(JSContext context) throws Exception {

final String script2 =
"var empty = {}; \n" +
"var constructorObject = function(val) {\n" +
" this.value = val; \n" +
"};" +
"constructorObject.prototype = { \n" +
" voidFunc: function() {}, \n" +
" jsvalueFunc: function() { var undef; return undef; }, \n" +
" jsobjectFunc:function() { return {}; }, \n" +
" intFunc: function() { return 5; }, \n" +
" intFunc2: function() { return 9; }, \n" +
" longFunc: function() { return 6; }, \n" +
" longFunc2: function() { return 10; }, \n" +
" floatFunc: function() { return 7.6; }, \n" +
" floatFunc2: function() { return 17.6; }, \n" +
" doubleFunc: function() { return 8.8; }, \n" +
" doubleFunc2: function() { return 18.8; }, \n" +
" stringFunc: function() { return 'string'; }, \n" +
" arrayFunc: function() { return [5,6,7,8]; }, \n" +
" booleanFunc: function() { return true; }, \n" +
" myValue: function() { return this.value; } \n" +
"};";
"var constructorObject = function(val) {\n" +
" this.value = val; \n" +
"};" +
"constructorObject.prototype = { \n" +
" voidFunc: function() {}, \n" +
" jsvalueFunc: function() { var undef; return undef; }, \n" +
" jsobjectFunc:function() { return {}; }, \n" +
" intFunc: function() { return 5; }, \n" +
" intFunc2: function() { return 9; }, \n" +
" longFunc: function() { return 6; }, \n" +
" longFunc2: function() { return 10; }, \n" +
" floatFunc: function() { return 7.6; }, \n" +
" floatFunc2: function() { return 17.6; }, \n" +
" doubleFunc: function() { return 8.8; }, \n" +
" doubleFunc2: function() { return 18.8; }, \n" +
" stringFunc: function() { return 'string'; }, \n" +
" arrayFunc: function() { return [5,6,7,8]; }, \n" +
" booleanFunc: function() { return true; }, \n" +
" myValue: function() { return this.value; } \n" +
"};";

ConstructorFunction constructorFunction = new ConstructorFunction(context);
assertEquals(PrototypeObject.class,constructorFunction.prototype().toObject().getClass());
assertEquals(PrototypeObject.class,constructorFunction.property("prototype").toObject().getClass());
context.property("constructorObjectJava", constructorFunction);
context.evaluateScript(script2);
JSObject js1 = context.evaluateScript("new constructorObject(5)").toObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,37 +493,6 @@ public void run() {
return context.getObjectFromRef(testException(runnable.jni));
}

/**
* Gets the prototype object, if it exists
* @return A JSValue referencing the prototype object, or null if none
* @since 3.0
*/
public JSValue prototype() {
JNIReturnClass runnable = new JNIReturnClass() {
@Override
public void run() {
jni = new JNIReturnObject();
jni.reference = getPrototype(context.ctxRef(), valueRef);
}
};
context.sync(runnable);
//if (runnable.jni.reference==0) return null;
return new JSValue(runnable.jni.reference,context);
}
/**
* Sets the prototype object
* @param proto The object defining the function prototypes
* @since 3.0
*/
public void prototype(final JSValue proto) {
context.sync(new Runnable() {
@Override
public void run() {
setPrototype(context.ctxRef(), valueRef, proto.valueRef());
}
});
}

@SuppressWarnings("unused") // This is called directly from native code
private long functionCallback(long thisObjectRef,
long argumentsValueRef[], long exceptionRefRef) {
Expand Down Expand Up @@ -596,15 +565,9 @@ public void run() {
JSObject thiz = (JSObject) defaultConstructor.newInstance();
thiz.context = context;
thiz.valueRef = thisObj;
thiz.property("constructor",JSFunction.this,JSObject.JSPropertyAttributeDontEnum);
function(thiz,args);
context.persistObject(thiz);
context.zombies.add(thiz);
if (proto != null && proto.isObject()) {
for (String prop : proto.toObject().propertyNames()) {
thiz.property(prop, proto.toObject().property(prop));
}
}
} catch (NoSuchMethodException e) {
String error = e.toString() + "If " + subclass.getName() + " is an embedded " +
"class, did you specify it as 'static'?";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,37 @@ public int hashCode() {
return valueRef().intValue();
}

/**
* Gets the prototype object, if it exists
* @return A JSValue referencing the prototype object, or null if none
* @since 3.0
*/
public JSValue prototype() {
JNIReturnClass runnable = new JNIReturnClass() {
@Override
public void run() {
jni = new JNIReturnObject();
jni.reference = getPrototype(context.ctxRef(), valueRef);
}
};
context.sync(runnable);
return new JSValue(runnable.jni.reference,context);
}

/**
* Sets the prototype object
* @param proto The object defining the function prototypes
* @since 3.0
*/
public void prototype(final JSValue proto) {
context.sync(new Runnable() {
@Override
public void run() {
setPrototype(context.ctxRef(), valueRef, proto.valueRef());
}
});
}

protected final List<JSObject> zombies = new ArrayList<>();

@Override
Expand Down

0 comments on commit cb91b46

Please sign in to comment.