Import Intent from android-intent crate

[matthunz]

This adds support for android intents from my android-intent crate. I saw there was some interest to merge the two in rust-mobile/cargo-apk#18

I think Intents need better integration with this crate's API but I'm not sure how to get there. My current ideas are

android_app.intent(Action::Send))
  .with_type("text/plain")
  .with_extra(Extra::Text, "Hello World!")
  .into_chooser()
  .start_activity()?

or

Intent::new(env, Action::Send)
  .with_type("text/plain")
  .with_extra(Extra::Text, "Hello World!")
  .into_chooser()
  .start_activity(&android_app)?

I prefer the first example because it specifies how the JNIEnv comes from an AndroidApp right away. The env has to be included in the Intent struct so each JNI method can be called without storing all of the arguments in rust first (e.g. using a Vec of methods to call and then doing so one by one after we have an env. This uses the builder pattern where errors are handle on the build method start_activity (similar to how the http crate keeps an error and handles it on build).

[rib]

Ah, we might have been a little unclear in that discussion - I think @MarijnS95 and I were initially thinking about seeing if you'd like to move your repo to the rust-mobile organisation.

Tighter integration with android-activity might be good to consider too, but my initial guess is that android-intent-rs would be usable for more people / use cases if it wasn't tied to android-activity.

E.g. another architecture for utilizing Rust code on Android is to have a dylib / shared library that can be loaded from a Java application and the Rust code is driven via native method calls from Java. In this architecture an application wouldn't use android-activity but they might still want to have an API to deal with Intents from Rust.

If this crate were merged into android-activity that would mean it's only usable for standalone native Rust applications on Android, not for Java apps that embed Rust code via a shared library.

[rib]

Integration like this does also open this can of worms with exposing the jni API that I was mentioning briefly here: #58 (comment)

E.g. this currently uses jni 0.20 and it's notable that there are quite a few breaking API changes in jni 0.21.

If the jni API gets exposed publicly in this crate then we end up with a tight coupling such that we need to start bumping the android-activity version in sync with jni releases, and to some extent downstream users are going to also be effectively forced to use whatever version of jni is exposed by android-activity.

[matthunz]

Oh ok I misunderstood - the repo is now live at https://github.com/rust-mobile/android-intent-rs

But huh that dylib stuff is interesting I hadn't heard of that before - I'll look into tighter integration with maybe an optional android-intent feature flag to support that case as well

[matthunz]

Also that's an interesting side effect of re-exporting jni
I'd say pinning android-activity and android-intent to the latest version of JNI should make the versioning ok

[rib]

Oh ok I misunderstood - the repo is now live at https://github.com/rust-mobile/android-intent-rs

But huh that dylib stuff is interesting I hadn't heard of that before - I'll look into tighter integration with maybe an optional android-intent feature flag to support that case as well

There's maybe some other better illustration of this, but here's one blog post that shows someone following that kind of architecture: https://blog.devgenius.io/integrating-rust-with-android-development-ef341c2f9cca

The key bits being that they have something like this in their Cargo.toml:

[lib]
name = "rust_lib"
crate-type = ["cdylib"]

And then they load that library explicitly within their Activity subclass:

class MainActivity : ComponentActivity() {

    companion object {
        init {
            System.loadLibrary("rust_lib")
            RustLog.initialiseLogging()
        }
    }
    // ...
}

So they treat the Java/Kotlin onCreate code as the primary entry point instead of having a native android_main() entrypoint that runs in a separately spawned thread.

That particular blog post uses some bindgen stuff I'm not familiar with, instead of directly working with the jni crate.

[rib]

Also that's an interesting side effect of re-exporting jni I'd say pinning android-activity and android-intent to the latest version of JNI should make the versioning ok

It's not so much the effect of re-exporting jni that causes the coupling, it's just that if a type like JNIEnv or JObject is in the public API for android-activity you have to consider that JObject from jni 0.20 is a different type to JObject from jni 0.19 or 0.21 as far as Rust is concerned.

So if android-activity depends on jni 0.21 and provides an API like AndroidApp::with_env() then the JNIEnv that's provided to that closure is specifically from jni 0.21 and so then downstream from that you need to continue with jni 0.21, unless at some point you bridge between abstractions via some raw pointers.

As it is currently, individual crates can be more orthogonal with what version of the jni crate they use since they acquire the JavaVM and Activity pointers as raw pointers that don't force them to use any particular version of the jni crate.

This means it's more practical to let crates migrate to new versions of the jni API at their convenience, since it's technically possible to have different crates in one application use different versions of the jni crate.

If we remove those raw pointers in favor of stronger jni types we also end up forcing them to all use the same version of the jni crate.

[MarijnS95]

Suggested change

	/// Builds a new [`Action::Chooser`](Action) Intent that wraps the given target intent.
	/// Builds a new [`Action::Chooser`] Intent that wraps the given target intent.

Let's link directly to the desired enum variant instead of to the type?

[MarijnS95]

Yes the main point was to keep utility crates under one roof, and merge them if the brunt of it is duplicate/shared (or have a "base" utility crate that provides this, which is subsequently inherited by other utility crates); without being coupled to android-activity as mentioned above. Looks like that was already achieved overnight with the migration 🎉

---

I'm partly in favour of using jni in the public signature, in no particular order:

1. Shows that we consider jni as the de-facto standard Rust crate for JNI;
2. Makes it clear what the type is (instead of a *mut c_void with massive doc comments);
3. Uses Rust types to communicate what is expected of the user (ownership/lifetime-wise);
4. No need for (unsafe) conversions in user code (including guesswork re. point 2/3);
5. With most crates under the @rust-mobile umbrella, and @rib being a maintainer of the JNI crate, version bumping and releases should be more straightforward.

But yes, I've been working on Rust long enough to know that breaking version releases of crates are a pain to propagate through the ecosystem. Ranging from simply bumping internal utilities to stop cargo deny from complaining about duplicates in our crate graph, to propagating updates in the right order when crates (like jni here) are used in public signatures that would otherwise fail to compile...

That said, perhaps JNI should push through with as many breaking improvements now and then postpone breaking changes for some time; I've done the same with ash successfully (though the way I deal with merging the PRs should have been different as 98% of our contributions are non-breaking)... Or https://github.com/dtolnay/semver-trick.

[MarijnS95]

In other words, should this PR be closed as we intend for this intent helper code to be accessible without utilizing the android-activity crate?

[rib]

That said, perhaps JNI should push through with as many breaking improvements now and then postpone breaking changes for some time; I've done the same with ash successfully (though the way I deal with merging the PRs should have been different as 98% of our contributions are non-breaking)... Or https://github.com/dtolnay/semver-trick.

Yeah, we kind of did this with the 0.21 release. We already had a few notable changes that required breaking API changes and took the opportunity to make some other breaking changes at the same time. So we effectively bunched the breaking changes together since we knew it was going to take some work to migrate to 0.21 anyway. I can't be entirely sure yet if we'll manage to minimize breaking changes for 0.22 but it would be my preference that 0.22 is a bit more conservative. I can think of at least one breaking change (that we ideally need to rework GlobalRef so it doesn't erase the type of reference it wraps) that we intentionally didn't tackle in the last release, which is likely to happen for 0.22.

That said though, I have found that working on the jni crate has sometimes felt a bit like pulling on a lose thread, the more you look at it the more issues you realize there are. With 0.21 we sometimes consciously decided to be incremental with changes because otherwise we'd get stuck boiling the ocean. (For example the get_array_elements API is marked as unsafe now and ideally it'd be good to iterate the design to remove some of the safety concerns but there were numerous factors that made that too awkward to tackle in one release.)

Yeah we can use the semver trick where that helps but at least for 0.21 that kind of issue wasn't really the source of breakages. The majority of API breakage comes from making reference types (like JObject) !Copy to help avoid pointer use-after-free issues in case local references are explicitly deleted. This generally means code needs to borrow object references now, which is generally a simple change to make but will affect quite a bit of code using jni.

[rib]

In other words, should this PR be closed as we intend for this intent helper code to be accessible without utilizing the android-activity crate?

yep, I think it makes sense to close this issue for now.

@matthunz was added as a member of the rust-mobile org yesterday and was able to migrate the android-intent repo too, which seems like a good starting point to me.

It's possible that we can consider tighter integration with the android-activity crate if there's a specific benefit to doing that but we'll have to think through the trade offs from exposing jni types in the API, and I think instead of embedding the whole crate, we'd add it as a dependency instead I guess.

[MarijnS95]

Not too concerned with breaking changes as long as they're behind a breaking semver bump... Which are typically the problem themselves, especially when only breaking minimal API surface while still requiring everyone to bump the version of their JNI dependency and itself provide a breaking release if they publicly expose jni types.

(That was the main complaint with ash: we broke the function signature of a single extension function, causing the whole crate to require a breaking semver bump, and in turn requiring the entire recursive dependency stack using ash to bump and release in a set order)

That said though, I have found that working on the jni crate has sometimes felt a bit like pulling on a lose thread

That's true for any software project, IMO, for better or for worse. Let's just keep going for now, use JNI in crate's public API, and see where we end up?

The semver trick is indeed only relevant once you start adjusting function signatures but leave the underlying types mostly the same.

---

It's possible that we can consider tighter integration with the android-activity crate if there's a specific benefit to doing that but we'll have to think through the trade offs from exposing jni types in the API, and I think instead of embedding the whole crate, we'd add it as a dependency instead I guess.

As mentioned in rust-mobile/android-intent#1 (comment) the webbrowser crate for example might make good use out of android-intent without pulling in all of android-activity. android-activity could perhaps depend on android-intent to provide Intent-creation helpers out of the box? That'll also rid it of a direct (internal-only) JNI dependency :)

[rib]

(That was the main complaint with ash: we broke the function signature of a single extension function, causing the whole crate to require a breaking semver bump, and in turn requiring the entire recursive dependency stack using ash to bump and release in a set order)

Yeah I'm a bit concerned the jni crate could be the kind of crate that could become really painful to deal if we connect everything up via strong types.

The jni crate is very likely to be used by almost any crate supporting Android in some way.

For standalone apps based on android-activity then we'd be the root of a potentially large graph of dependencies that use jni.

Whenever a semver incompatible jni release is made (that's unavoidable for the next release of jni) that will require a corresponding release of android-activity and that potentially kicks of the need for all those downstream crates to have to synchronize their migration to the next release of jni too.

Maybe the semver trick will be able to avoid the worst case scenario in some instances though.

[MarijnS95]

Sounds like we should just try it out and see how it fares?