android: jni 0.22 update, exception checks, support any 'Context'

This addresses some exception-handling hygiene issues in the Android
backend, and adds support for non-`Activity` `Context` references
(considering that `ndk-context` does not guarantee that it provides an
`Activity` reference).

With the previous implementation, if any of the JNI calls failed (such
as the `find_class` call), that would have thrown an exception which was
not automatically caught by the jni 0.21 crate. Since there was no code
to ensure exceptions were checked and cleared on failure then the
previous implementation would have had the side effect of blocking any
further JNI on the same thread until the exception was cleared - but it
would not have reported a recognisable error for indicating this side
effect.

By contrast, with jni 0.22 we can rely on the `attach_current_thread`
call to check and clear exceptions from the closure before returning.

This change also prepares this crate for a planned change in
`android-activity` to initialize `ndk-context` with an `Application`
reference instead of an `Activity` reference, ref:
rust-mobile/android-activity#228

The fact that `android-activity` currently initializes `ndk-context`
with an `Activity` reference is not itself guaranteed by the
`ndk-context` crate (it only promises an `android.content.Context`
reference) but the previous android backend was depending on receiving
an `Activity` reference and would throw an exception if it didn't.

Note: Associating `ndk-context` with an `Activity` does not align well
with Android application programming model because Android applications
may often manage many Activities and it doesn't align with the design of
`ndk-context` because the `ndk-context` state can't be re-initialized.
Even if ndk-context could be changed to support state updates without a
panic it could still be ambiguous to maintain a single global reference
to one Activity in an application with multiple or possibly no Activity.

The new implementation will continue as it did before in case it does
receive an Activity reference, but in case it receives some other kind
of `Context` then now adds a `ACTIVITY_NEW_TASK` flag to the `Intent` to
indicate that the new Activity is allowed to create a new `Task`. In
practice this expected to be irrelevant (and result in identical
behaviour) because the URL is most likely to be handled by a separate
browser application.

This needed to bump the rust-version to 1.85 since that's the MSRV for
jni 0.22

This also updates the Android test to be based on android-activity
instead of ndk-glue

Author: rib

Cargo.toml

@@ -10,7 +10,7 @@ readme = "README.md"
 keywords = ["webbrowser", "browser"]
 license = "MIT OR Apache-2.0"
 edition = "2021"
-rust-version = "1.60"
+rust-version = "1.85"
 
 [dependencies]
 log = { version = "0.4", default-features = false }
@@ -29,7 +29,7 @@ wasm-console = ["web-sys/console"]
 core-foundation = "0.10"
 
 [target.'cfg(target_os = "android")'.dependencies]
-jni = "0.21"
+jni = "0.22.2"
 ndk-context = "0.1"
 
 [target.'cfg(any(target_os = "ios", target_os = "tvos", target_os = "visionos"))'.dependencies]
@@ -52,4 +52,4 @@ tokio = { version = "1", features = ["full"] }
 urlencoding = "2.1"
 
 [target.'cfg(target_os = "android")'.dev-dependencies]
-ndk-glue = { version = ">= 0.3, <= 0.7" }
+android-activity = { version = "0.6", features = ["native-activity"] }

src/android.rs

@@ -1,5 +1,5 @@
 use crate::{Browser, BrowserOptions, Error, ErrorKind, Result, TargetType};
-use jni::objects::{JObject, JValue};
+use jni::{objects::JString, refs::Global};
 use std::process::{Command, Stdio};
 
 /// Deal with opening of browsers on Android. Only [Browser::Default] is supported, and
@@ -21,6 +21,77 @@ pub(super) fn open_browser_internal(
     }
 }
 
+jni::bind_java_type! {
+    AUri => "android.net.Uri",
+    methods {
+        static fn parse(uri: JString) -> AUri
+    }
+
+}
+
+jni::bind_java_type! {
+    AIntent => "android.content.Intent",
+    type_map {
+        AUri => "android.net.Uri"
+    },
+    constructors {
+        fn new_with_url(action: JString, uri: AUri)
+    },
+    fields {
+        #[allow(non_snake_case)]
+        static ACTION_VIEW: JString,
+        #[allow(non_snake_case)]
+        static FLAG_ACTIVITY_NEW_TASK: jint
+    },
+    methods {
+        fn add_flags(flags: jint) -> AIntent
+    }
+}
+
+jni::bind_java_type! {
+    AContext => "android.content.Context",
+    type_map {
+        AIntent => "android.content.Intent"
+    },
+    methods {
+        fn start_activity(intent: AIntent)
+    }
+}
+
+jni::bind_java_type! {
+    AActivity => "android.app.Activity",
+    type_map {
+        AContext => "android.content.Context"
+    },
+    is_instance_of {
+        activity: AContext
+    },
+}
+
+#[derive(Debug)]
+enum OpenUrlJniError {
+    Jni(jni::errors::Error),
+    Other(crate::Error),
+}
+impl From<jni::errors::Error> for OpenUrlJniError {
+    fn from(err: jni::errors::Error) -> Self {
+        OpenUrlJniError::Jni(err)
+    }
+}
+impl From<crate::Error> for OpenUrlJniError {
+    fn from(err: crate::Error) -> Self {
+        OpenUrlJniError::Other(err)
+    }
+}
+impl std::fmt::Display for OpenUrlJniError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            OpenUrlJniError::Jni(err) => write!(f, "JNI error: {}", err),
+            OpenUrlJniError::Other(err) => write!(f, "{}", err),
+        }
+    }
+}
+
 /// Open the default browser
 fn open_browser_default(url: &str, options: &BrowserOptions) -> Result<()> {
     // always return true for a dry run
@@ -34,64 +105,85 @@ fn open_browser_default(url: &str, options: &BrowserOptions) -> Result<()> {
         return Ok(());
     }
 
-    // Create a VM for executing Java calls
+    // Get a `JavaVM` reference for executing Java calls
     let ctx = ndk_context::android_context();
-    let vm = unsafe { jni::JavaVM::from_raw(ctx.vm() as _) }.map_err(|_| {
-        Error::new(
+    if ctx.vm().is_null() || ctx.context().is_null() {
+        return Err(Error::new(
             ErrorKind::NotFound,
-            "Expected to find JVM via ndk_context crate",
-        )
-    })?;
-
-    let activity = unsafe { jni::objects::JObject::from_raw(ctx.context() as _) };
-    let mut env = vm
-        .attach_current_thread()
-        .map_err(|_| Error::new(ErrorKind::Other, "Failed to attach current thread"))?;
-
-    // Create ACTION_VIEW object
-    let intent_class = env
-        .find_class("android/content/Intent")
-        .map_err(|_| Error::new(ErrorKind::NotFound, "Failed to find Intent class"))?;
-    let action_view = env
-        .get_static_field(&intent_class, "ACTION_VIEW", "Ljava/lang/String;")
-        .map_err(|_| Error::new(ErrorKind::NotFound, "Failed to get intent.ACTION_VIEW"))?;
-
-    // Create Uri object
-    let uri_class = env
-        .find_class("android/net/Uri")
-        .map_err(|_| Error::new(ErrorKind::NotFound, "Failed to find Uri class"))?;
-    let url = env
-        .new_string(url)
-        .map_err(|_| Error::new(ErrorKind::Other, "Failed to create JNI string"))?;
-    let uri = env
-        .call_static_method(
-            &uri_class,
-            "parse",
-            "(Ljava/lang/String;)Landroid/net/Uri;",
-            &[JValue::Object(&JObject::from(url))],
-        )
-        .map_err(|_| Error::new(ErrorKind::Other, "Failed to parse JNI Uri"))?;
-
-    // Create new ACTION_VIEW intent with the uri
-    let intent = env
-        .alloc_object(&intent_class)
-        .map_err(|_| Error::new(ErrorKind::Other, "Failed to allocate intent"))?;
-    env.call_method(
-        &intent,
-        "<init>",
-        "(Ljava/lang/String;Landroid/net/Uri;)V",
-        &[action_view.borrow(), uri.borrow()],
-    )
-    .map_err(|_| Error::new(ErrorKind::Other, "Failed to initialize intent"))?;
-
-    // Start the intent activity.
-    env.call_method(
-        &activity,
-        "startActivity",
-        "(Landroid/content/Intent;)V",
-        &[JValue::Object(&intent)],
-    )
-    .map_err(|_| Error::new(ErrorKind::Other, "Failed to start activity"))?;
+            "Expected to find JVM and context.context.Context reference via ndk_context crate",
+        ));
+    }
+    // Safety: we have checked we have a non-null pointer
+    let vm = unsafe { jni::JavaVM::from_raw(ctx.vm() as _) };
+
+    // Note: attach_current_thread will also make sure to catch/clear any Java exceptions that could be
+    // thrown while attempting to interact with the Android SDK
+    vm.attach_current_thread(|env| -> std::result::Result<(), OpenUrlJniError> {
+        // Safety:
+        //
+        // The docs for `ndk-context` promise that `.context()` will return a reference to an
+        // `android.content.Context`
+        //
+        // The reference associated with ndk-context must implicitly be a global JNI reference
+        //
+        // Note: although we already check for a `null` `Context` reference above, that's not
+        // strictly required for _safety_ here (`null` JNI references are safe).
+        //
+        // Wrapping the raw reference with a `Cast` here ensures that we can't accidentally delete
+        // a global reference that we don't own.
+        let context: jni::sys::jobject = ctx.context() as _;
+        let context = unsafe { env.as_cast_raw::<Global<AContext>>(&context) }
+            .map_err(|e| Error::other(format!("Failed to cast context: {}", e)))?;
+        let context: &AContext = &context;
+
+        let action_view = AIntent::ACTION_VIEW(env).map_err(|e| {
+            Error::other(format!(
+                "Failed to lookup ACTION_VIEW field constant: {}",
+                e
+            ))
+        })?;
+
+        let url = JString::new(env, url)
+            .map_err(|e| Error::other(format!("Failed to create JString for URL: {}", e)))?;
+        let uri = AUri::parse(env, url)
+            .map_err(|e| Error::other(format!("Failed to parse URL: {}", e)))?;
+
+        // Create new ACTION_VIEW intent with the uri
+        let intent = AIntent::new_with_url(env, &action_view, &uri)
+            .map_err(|e| Error::other(format!("Failed to create ACTION_VIEW intent: {}", e)))?;
+
+        // `ndk-context only` promises to offer an `android.content.Context` reference and so we
+        // can't assume that this is an `Activity` (it's also likely to be an `Application`
+        // reference)
+        //
+        // If we have an `Activity` then `startActivity` will automatically try and associate the
+        // new activity with the `Task` that holds the existing `Activity`.
+        //
+        // If we have an `Application` reference then `startActivity` will need the
+        // `FLAG_ACTIVITY_NEW_TASK` flag to indicate that we expect the new activity to be started
+        // in a new task (and if we don't the system will throw an exception).
+        //
+        // Note: In practice the end result is likely to be the same either way because a http/https
+        // URL will almost certainly open in a separate browser application - not in any `Task`
+        // running in the current application.
+        let maybe_activity = env.as_cast::<AActivity>(&context);
+        match maybe_activity {
+            Ok(_activity) => {
+                // We have an Activity and so the associated Task is implied
+                context.start_activity(env, &intent)?;
+            }
+            Err(_) => {
+                // If we don't have an `Activity` then we need to add the `ACTIVITY_NEW_TASK` flag
+                // to the `Intent`, to ensure it can be started from a non-activity context.
+                let flag_activity_new_task = AIntent::FLAG_ACTIVITY_NEW_TASK(env)?;
+                let intent = intent.add_flags(env, flag_activity_new_task)?;
+                context.start_activity(env, &intent)?;
+            }
+        }
+
+        Ok(())
+    })
+    .map_err(|e| Error::other(format!("Failed to open URL via Android Intent: {}", e)))?;
 
     Ok(())
 }
@@ -117,13 +209,10 @@ fn try_for_termux(url: &str, options: &BrowserOptions) -> Result<()> {
             if status.success() {
                 Ok(())
             } else {
-                Err(Error::new(
-                    ErrorKind::Other,
-                    "command present but exited unsuccessfully",
-                ))
+                Err(Error::other("command present but exited unsuccessfully"))
             }
         })
     } else {
-        Err(Error::new(ErrorKind::Other, "Not a termux environment"))
+        Err(Error::other("Not a termux environment"))
     }
 }

tests/test-android-app/Cargo.toml

@@ -1,15 +1,14 @@
 [package]
 name = "test-android-app"
 version = "0.1.0"
-edition = "2021"
+edition = "2024"
 
 [lib]
 name = "test_android_app"
-crate-type = ["lib", "cdylib"]
+crate-type = ["cdylib"]
 
 [dependencies]
-jni = "0.19"
-ndk-glue = "0.7"
+android-activity = { version = "0.6", features = ["native-activity"] }
 webbrowser = { path = "../.." }
 
 [package.metadata.android.sdk]

tests/test-android-app/src/lib.rs

@@ -1,10 +1,7 @@
-#[allow(non_snake_case)]
-extern crate ndk_glue;
-
 const SERVER_URL: &str = "http://127.0.0.1";
 
-#[cfg_attr(target_os = "android", ndk_glue::main(backtrace = "on"))]
-pub fn android_main() {
+#[unsafe(no_mangle)]
+pub fn android_main(_app: android_activity::AndroidApp) {
     println!("****** [WEBB DEBUG] {} ***** begin", SERVER_URL);
     webbrowser::open(SERVER_URL).unwrap();
     println!("****** [WEBB DEBUG] {} ***** end", SERVER_URL);