I wonder if this can be fixed in a simpler manner. According to this answer: http://stackoverflow.com/a/12946228/991573 it seems that it may be possible to create the clipboard service in Java on the main thread (probably before starting the native thread), and then pass that to the native side, which would require a minor change in SETUP_CLIPBOARD and in SDLActivity.

How does this sound? Is there anything I'm missing?

runOnUiThread() already checks if this is the UI thread and executes the runnable immediately if this is the case. [1] Checking this manually saves the creation of one Runnable object but produces two nearly identical sequences of statements (three times because of three functions).

I do not see what SDL_AndroidGetPackageCodePath() may be useful for. [2] There is already a possibility in SDL to access the assets and the returned path should be read-only on most devices anyway.

Also for me this did not crash yet (which does not matter if it crashes on other devices :). Maybe this is the reason problem was not found and fixed earlier.

[1] http://developer.android.com/reference/android/app/Activity.html#runOnUiThread%28java.lang.Runnable%29
[2] http://developer.android.com/reference/android/content/ContextWrapper.html#getPackageCodePath%28%29

Let's try to avoid adding more stuff on the Java side if possible! If getting the clipboard service once in the main thread and then using it from C works, let's do that.

Created attachment 1467 [details]
Patch for creating a single ClipboardManager on correct thread.

I added a patch for creating a single ClipboardManager on UI thread. It is always created and reused (was not the case before). Therefore this might be a new overhead for applications not using the clipboard.

A different solution would be to use the idea from the original patch (blocking the calling thread until something happened on UI thread) and just write a "wrapper" for getSystemService() which may be called from another thread. This solution could also be used if more system services are needed in the future (e.g. vibration for SDL's haptic subsystem).

Created attachment 1468 [details]
Patch for creating a single ClipboardManager on correct thread by blocking.

I added a new patch and marked the previous as obsolete because this one is better. It just wraps getSystemService() to be executed on UI thread using the "blocking" idea from the original patch. This way there is no overhead if clipboard is not used and few new Java source and the actual problem should be fixed.

It was now possible for me to get the "crash" in the Android emulator but not on a real device (maybe it depends on Android version). The "crash" did not happen with this and the last patch active.

Created attachment 1469 [details]
Android clipboard crash fix etc. (Second try)

The reason for SDL_AndroidGetPackageCodePath is my port of Tcl/Tk on Android. The Tcl part wants to "mount" the APK to get the "/assets..." payload without going through SDL's filesystem layer and needs therefore knowledge of the path name of the APK.

Regarding the Clipboard I think it's more complicated. On my Android 4.x devices it seems to be sufficient to obtain the clipboard service during SDLActivity.onCreate as described in the stackoverlow posting. But then there's API level < 11.

Another point is that it is desirable to get SDL_CLIPBOARDUPDATE events at least for API level >= 11.

I've tried to address these points in the attached patch.

@Philip, your patch looks ok to me, let's get it committed.
@chw, I'm not opposed to having notifications for clipboard updates on API level >=11, let's try to get that reworked after Philipp lands his patch and remember to try to keep the Java side to the bare minimum. Thanks!

@Gabriel, Philipp: please consider to take in SDL_AndroidGetPackageCodePath(), I desperately need it for Tcl/Tk on Android. And please check my X11 patch, too. It handles SelectionClear X11 events by sending SDL_CLIPBOARDUPDATE.

Fixed (reported bug, not additional features):
https://hg.libsdl.org/SDL/rev/cf81e6709b3d

chw, please report separate bugs for the individual improvements you're proposing.

Thanks!

Thanks!

@chw, I guess I don't really see the overall benefit of the SDL_AndroidGetPackageCodePath function for users in general, but I'm open to hearing the argument, either from reason or from popularity :)

BTW, you should submit your enhancements separately, that way it'll be easier to keep track of them (don't cram two Android and a X11 fixes in a single patch/bug report). Anyway, sorry for the red tape, and thanks for your time and effort!

(In reply to chw from comment #9)
> @Gabriel, Philipp: please consider to take in
> SDL_AndroidGetPackageCodePath(), I desperately need it for Tcl/Tk on
> Android. And please check my X11 patch, too. It handles SelectionClear X11
> events by sending SDL_CLIPBOARDUPDATE.

SDL_AndroidGetPackageCodePath() should work outside SDL_android.c if Android_JNI_GetEnv() would be replaced with SDL_AndroidGetJNIEnv() and the "get context" section would be replaced with SDL_AndroidGetActivity(). Both functions are public API of SDL_system.h. Also jni.h would be needed to make the JNI functions available.