Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ANDROID: additional error checking on JNI an more #386

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ANDROID: additional error checking on JNI an more #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6cfb90
ANDROID: additional error checking on JNI an more
0-8-15 Nov 2, 2020
c1b681d
ANDROID: actually use the new file information
0-8-15 Nov 2, 2020
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
15 changes: 15 additions & 0 deletions libraries/liblambdanative/system.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,25 @@ static void find_directories()
#endif
#if defined(ANDROID)
// we put files on the sdcard, that's the only sane place (?)
extern char* android_getFilesDir();
char path[1024];
#if 0
sprintf(path,"/sdcard/%s", SYS_APPNAME);
sys_appdir=strdup(path);
sys_dir=strdup(path);
#endif
#if 0
sprintf(path,"%s/system", android_getFilesDir());
sys_dir=strdup(path);
sprintf(path,"%s/data", android_getFilesDir());
sys_appdir=strdup(path);
#endif
#if 1
sprintf(path,"/sdcard/%s", SYS_APPNAME);
sys_dir=strdup(path);
sys_appdir=android_getFilesDir();
Copy link
Contributor

Choose a reason for hiding this comment

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

A potential concern to confirm during testing is what we do with apps that get upgraded and that have files in the previous location, which is no longer accessible if we do this? The approach in #380 might be an interesting (or complementary) change.

#endif

#endif
#if defined(BB10) || defined(PLAYBOOK)
char path[1024], cwd[1024];
Expand Down
45 changes: 38 additions & 7 deletions loaders/android/bootstrap.c.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,23 @@ void Java_@SYS_PACKAGE_UNDERSCORE@_@SYS_APPNAME@_nativeEvent(JNIEnv* e, jobject
// JNI Hooks and Global Objects
static jobject globalObj=NULL;
static JavaVM* s_vm = NULL;
static const char* app_directory_files = NULL;
static const char* app_code_path = NULL;

void Java_@SYS_PACKAGE_UNDERSCORE@_@SYS_APPNAME@_nativeInstanceInit(JNIEnv* env, jobject thiz){
globalObj = (*env)->NewGlobalRef(env,thiz);
void Java_@SYS_PACKAGE_UNDERSCORE@_@SYS_APPNAME@_nativeInstanceInit(JNIEnv* env, jobject thiz, jstring codePath, jstring directoryFiles){

globalObj = (*env)->NewGlobalRef(env,thiz);
app_directory_files = strdup((*env)->GetStringUTFChars(env, directoryFiles, 0));
(*env)->ReleaseStringUTFChars(env, directoryFiles, NULL);
app_code_path = strdup((*env)->GetStringUTFChars(env, codePath, 0));
(*env)->ReleaseStringUTFChars(env, codePath, NULL);
}

char* android_getFilesDir() { return (char*) app_directory_files; }
char* android_getPackageCodePath() { return (char*) app_code_path; }

char* android_getFilesDir_info_get() { return android_getFilesDir(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry why is this needed - it does the exact same thing as android_getFilesDir(), which is defined 2 lines above?


jint JNI_OnLoad(JavaVM* vm, void* reserved){
JNIEnv *env;
s_vm=vm;
Expand All @@ -77,19 +89,38 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved){
JNIEnv* GetJNIEnv(){
int error=0;
JNIEnv* env = NULL;
if (s_vm) error=(*s_vm)->AttachCurrentThread(s_vm, &env, NULL);
if (!error&&(*env)->ExceptionCheck(env)) return NULL;
/* static `env` does NOT work! Once in a while we should ponder if
it still does not work or why.

if(env) {
if((*env)->ExceptionCheck(env)) (*env)->ExceptionClear(env);
return env;
}
*/
if(s_vm) error=(*s_vm)->AttachCurrentThread(s_vm, &env, NULL);
//if(!error&&(*env)->ExceptionCheck(env)) return NULL;
if(!error) error = JNI_forward_exception_to_gambit(env);
return (error?NULL:env);
}

int JNI_forward_exception_to_gambit(JNIEnv*env) {
// TBD: actually forward, not only clear!
if((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
Copy link
Contributor

Choose a reason for hiding this comment

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

So a call into log_c with content as described in https://stackoverflow.com/questions/10408972/how-to-obtain-a-description-of-a-java-exception-in-c-when-using-jni ?

return 1;
}
return 0;
}

// url launcher ffi
void android_launch_url(char* urlstring){
JNIEnv *env = GetJNIEnv();
jstring jurlstring = (*env)->NewStringUTF(env,urlstring);
if (env&&globalObj) {
jstring jurlstring = (*env)->NewStringUTF(env, urlstring);
jclass cls = (*env)->FindClass(env, "@SYS_PACKAGE_SLASH@/@SYS_APPNAME@");
jmethodID method = (*env)->GetMethodID(env, cls, "openURL", "(Ljava/lang/String;)V");
(*env)->CallVoidMethod(env, globalObj, method, jurlstring);
jmethodID method = cls ? (*env)->GetMethodID(env, cls, "openURL", "(Ljava/lang/String;)V") : NULL;
if(method) (*env)->CallVoidMethod(env, globalObj, method, jurlstring);
JNI_forward_exception_to_gambit(env);
}
}

Expand Down
67 changes: 45 additions & 22 deletions loaders/android/bootstrap.java.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,42 +126,65 @@ public class @SYS_APPNAME@ extends Activity implements @ANDROID_JAVA_IMPLEMENTS@
}
}

private android.view.View current_ContentView = null;
@Override
public void setContentView(android.view.View view) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self - move to separate commit - unrelated to everything else

if(current_ContentView != view) {
// Note: this is a bit brain deas as it ONLY handles GLSurfaceView
if(current_ContentView instanceof android.opengl.GLSurfaceView) {
((android.opengl.GLSurfaceView)current_ContentView).onPause();
}
android.view.ViewParent parent0 = view.getParent();
if(parent0 instanceof android.view.ViewGroup) {
android.view.ViewGroup parent = (android.view.ViewGroup) parent0;
if(parent!=null) { parent.removeView(current_ContentView); }
}
current_ContentView = view;
super.setContentView(current_ContentView);
if(current_ContentView instanceof android.opengl.GLSurfaceView) {
((android.opengl.GLSurfaceView)current_ContentView).onResume();
}
}
}

@Override
protected void onCreate(Bundle savedInstanceState) {
current_ContentView = null;
super.onCreate(savedInstanceState);
Thread.setDefaultUncaughtExceptionHandler(
new Thread.UncaughtExceptionHandler() {
public void uncaughtException(Thread t, Throwable e) {
final String TAG = "@SYS_PACKAGE_DOT@";
Log.e(TAG, e.toString());
Log.e(TAG, e.toString());
try { Thread.sleep(1000); } catch (Exception ex) { }
System.exit(1);
}
});
setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_PORTRAIT);
this.requestWindowFeature(Window.FEATURE_NO_TITLE);
this.requestWindowFeature(Window.FEATURE_NO_TITLE);
// make sure volume controls control media
this.setVolumeControlStream(AudioManager.STREAM_MUSIC);
getWindow().setFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN,
WindowManager.LayoutParams.FLAG_FULLSCREEN);
// prevent sleep
getWindow().addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON);
mGLView = new xGLSurfaceView(this);
setContentView(mGLView);
// NOTE: we MAY better move the following lines BELOW nativeInstanceInit
mSensorManager = (SensorManager)getSystemService(Context.SENSOR_SERVICE);

checkOrRequestPermission(android.Manifest.permission.WRITE_EXTERNAL_STORAGE);

// Additions needed by modules, e.g. gps
@ANDROID_JAVA_ONCREATE@

// start EVENT_IDLE
nativeInstanceInit(getApplicationContext().getPackageCodePath().toString(), getFilesDir().toString());
// start EVENT_IDLE
setContentView(mGLView); // MUST NOT run before nativeInstanceInit completed
if(idle_tmScheduleRate > 0) idle_tm.scheduleAtFixedRate(idle_task, 0, idle_tmScheduleRate);

nativeInstanceInit();
}
@Override
@Override
protected void onDestroy() {
setContentView(mGLView);
@ANDROID_JAVA_ONDESTROY@
nativeEvent(14,0,0); // EVENT_CLOSE
nativeEvent(127,0,0); // EVENT_TERMINATE
Expand All @@ -173,19 +196,19 @@ public class @SYS_APPNAME@ extends Activity implements @ANDROID_JAVA_IMPLEMENTS@
}
@Override
protected void onPause() {
super.onPause();
// Additions needed by modules, e.g. gps
@ANDROID_JAVA_ONPAUSE@
if (!isFinishing()) {
if (!isFinishing() && current_ContentView==mGLView) {
mGLView.onPause();
}
super.onPause();
}
@Override
protected void onResume() {
super.onResume();
if(current_ContentView==mGLView) { mGLView.onResume(); }
// Additions needed by modules, e.g. gps
@ANDROID_JAVA_ONRESUME@
mGLView.onResume();
}
@Override
public void onAccuracyChanged(Sensor sensor, int accuracy) {
Expand Down Expand Up @@ -220,13 +243,13 @@ public class @SYS_APPNAME@ extends Activity implements @ANDROID_JAVA_IMPLEMENTS@
}
}

native void nativeInstanceInit();
native void nativeInstanceInit(String packageCodePath, String filesDir);
}

class xGLSurfaceView extends GLSurfaceView {
public xGLSurfaceView(Context context) {
super(context);
setFocusable(true);
setFocusable(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

All changes from here on out in this file are cosmetic (space at end of line). I guess the vimrc from #135 was never included anywhere?

setFocusableInTouchMode(true);
renderer = new myRenderer();
setRenderer(renderer);
Expand All @@ -241,23 +264,23 @@ class xGLSurfaceView extends GLSurfaceView {
case MotionEvent.ACTION_UP: t=4; break;
case MotionEvent.ACTION_POINTER_UP: t=4; break;
}
if (t>0) {
if (t>0) {
final int n=event.getPointerCount();
final int t0=t;
final int id0=event.getPointerId(0);
final int x0=(int)event.getX(0);
final int y0=(int)event.getY(0);
if (n>1) { // MultiTouch
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(18,id0,0); }});
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(18,id0,0); }});
}
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(t0,x0,y0); }});
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(t0,x0,y0); }});
if (n>1) { // MultiTouch
final int id1=event.getPointerId(1);
final int x1=(int)event.getX(1);
final int y1=(int)event.getY(1);
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(18,id1,0); }});
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(t0,x1,y1); }});
}
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(18,id1,0); }});
queueEvent(new Runnable(){ public void run() { renderer.pointerEvent(t0,x1,y1); }});
}
}
return true;
}
Expand Down Expand Up @@ -295,7 +318,7 @@ class xGLSurfaceView extends GLSurfaceView {
}
if (t>0) {
queueEvent(new Runnable(){ public void run() {
renderer.nativeEvent(t,x,y); }});
renderer.nativeEvent(t,x,y); }});
}
return true;
}
Expand All @@ -311,15 +334,15 @@ class xGLSurfaceView extends GLSurfaceView {
myRenderer renderer;
}
class myRenderer implements GLSurfaceView.Renderer {
public void onSurfaceCreated(GL10 gl, EGLConfig config) {
public void onSurfaceCreated(GL10 gl, EGLConfig config) {
}
public void onSurfaceChanged(GL10 gl, int w, int h) {
gl.glViewport(0, 0, w, h);
width=(float)w; height=(float)h;
nativeEvent(127,w,h); // EVENT_INIT
}
public void onDrawFrame(GL10 gl) {
nativeEvent(15,0,0); // EVENT_REDRAW
public void onDrawFrame(GL10 gl) {
nativeEvent(15,0,0); // EVENT_REDRAW
}
public void pointerEvent(int t, int x, int y) { nativeEvent(t,x,(int)height-y); }
public float width,height;
Expand Down
14 changes: 14 additions & 0 deletions modules/config/config.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,18 @@ end-of-c-declare
(gambit-c (if (string=? (system-platform) "android") (##heartbeat-interval-set! -1.)))
(else (if (string=? (system-platform) "android") (##set-heartbeat-interval! -1.))))

(cond-expand
(android
(c-declare #<<EOF
extern char* android_getFilesDir_info_get();
char* android_getFilesDir_info()
Copy link
Contributor

Choose a reason for hiding this comment

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

You are now making a third way of calling android_getFilesDir() - am I missing something that makes this one different? as otherwise only the extern char* android_getPackageCodePath() needs to stay here?

{
return android_getFilesDir_info_get();
}
extern char* android_getPackageCodePath();
EOF
)
(define (android-PackageCodePath) ((c-lambda () char-string "android_getPackageCodePath"))))
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be a strange naming conversion for a scheme function, more like Java with CamelCase?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also this function is not defined on non-android platforms, which may be a problem as how do we prevent users from including this in their code? Wouldn't it be safer to always declare it and simply not do anything on non-android platforms?

(else #!void))

;; eof