`with_activity` Android utility assumes `ndk-context` provides `Activity` ref (undocumented + changing)

[rib]

Hi, I've recently been reviewing crates that use the ndk-context API to ensure they are not making any assumptions that the Context reference comes from an Activity reference.

I've noticed that the manganis package appears to make this assumption via the with_activity utility that was added in #4842

As it is currently, the ndk-context API only promises to provide an application.content.Context reference and does not specifically guarantee that this context comes from an Activity.

As an implementation detail android-activity does currently initialise ndk-context with an Activity-based context reference but we need to change this in order to address other bugs that this leads to: rust-mobile/android-activity#228, rust-mobile/android-activity#58

Briefly speaking, Android's application programming model allows an application to have multiple (or zero) Activities per application and the current choice to store an Activity reference in the global, static ndk-context state is problematic because:

• ndk-context is designed to only be initialized once (it will panic if trying to update its state), so even if you only ever have one Activity at a time, we have no way to reset the ndk-context state with a new Activity reference
• Since ndk-context only exposes a single Context reference then in case you have an application that creates multiple Activities then ndk-context can only be associated with one of them.
• whatever reference is put into ndk-context will need to have a global JNI reference to ensure it remains valid for the lifetime of the program which isn't appropriate for an Activity whose lifetime is only associated with a single thread.

For reference, and as a heads up; the plan (for android-activity) is to start initializing ndk-context with an Application Context, which is better aligned with the ndk-context design which only supports tracking one-time-initialized global state. (ref: rust-mobile/android-activity#229)

Expected behavior

Perhaps manganis will actually already work with any android.content.Context reference and doesn't strictly need an Activity in practice and so it could rename with_activity to with_context.

Note: that getAssets is a Java API that is exposed by any android.content.Context and so it would be possible to get to an AssetManager via any Context.

Technically the AssetManager associated with a specific Activity could be able to find additional resources in some circumstances (compared to the Application AssetManager) but it's more likely that there's no difference.

Perhaps the manganis crate could have some Android-specific config or initialization API that can be used to explicitly provide it a JNI reference to an Activity or maybe an AssetManager that is acquired externally - e.g. via android-activity (AndroidApp::activity_as_ptr) or perhaps via some Java native method / JNI integration with a custom Activity subclass.

[rib]

Looking closer at dioxus mobile support, my current understanding is that dioxus relies on taurie/tao on Android which will handle ndk-context initialization and so you shouldn't be affected by any change in android-activity if we merge rust-mobile/android-activity#229

It could still be good to consider how to avoid assuming that ndk-context gets initialized with an Activity since this could also be considered an implementation detail for tao that is not guaranteed by the ndk-context API.

It could make sense for tao to also initialize ndk-context with an Application Context in the future in order to better support applications that may be built with multiple Activities (as it is currently it looks like the ndk_glue layer for tao would panic if attempting to create multiple WryActivities per process)

[nicoburns]

@rib I will look into this more, but I assume that this change would be released as a semver breaking version?

they are not making any assumptions that the Context reference comes from an Activity reference.

This is a bit confusing to me. What use is a Context reference if you can't assume what it is a reference to? (if you want to change what it is to in a new version, then that seems fine (in theory at leas他）, but a reference for which "what it is a reference to" is undefined doesn't sound like it would be very useful?

[rib]

To clarify a bit @nicoburns; "Context" here is an abbreviation of "android.content.Context" which is a specific Android SDK type that both Activity and Application subclass.

The ndk-context API only promises to provide an android.content.Context reference.

Specifically the documentation says:

A handle to an android.content.Context. In most cases this will be a ptr to an Activity, but this isn’t guaranteed.

so for android-activity to initialize ndk-context with an Application instead of an Activity is not a semver breaking change since Application is also an android.content.Context

Although it's not a semver breaking change (and so the plan is to release this change/fix as part of an android-activity 0.6.1 release) I do appreciate that there is some risk in affecting crates that might have inadvertently made an assumption that this was an Activity reference - which is why I've been trying to review some of the dependants on ndk-context to try and check for any impact.

For example I opened this PR to fix this assumption in the webbrowser crate recently to ensure they won't be affected by this: amodm/webbrowser-rs#111

[rib]

Out of curiosity, since I found that DioxusLabs/blitz has an apps/browser and examples that use android-activity / NativeActivity instead of WryActivity, I smoke tested building these with the android-activity main branch and was able to get the browser and counter examples to run.

I added this to the workspace:

[patch.crates-io]
android-activity = { git = "https://github.com/rust-mobile/android-activity", branch = "main" }

and ran both with:

dx serve --android  --no-default-features --features skia

So these at least seem to work with the change to initialize ndk-context with an Application (though I'm not familiar enough with dioxus to know whether these use manganis to lookup any assets)