From 19fa538efd48fcfc9fe2d4206549301a0a23456a Mon Sep 17 00:00:00 2001
From: Marijn Suijten <marijns95@gmail.com>
Date: Mon, 17 Jun 2024 23:19:32 +0200
Subject: [PATCH] Upgrade to `ndk 0.9` and delete unused `ndk-sys/context`
 dependencies

The `ndk` crate received some marginal API upgrades, besides fixing
soundness issues.  Specifcally, `ForeignLooper::add_fd_with_callback()`
now signifies that the incoming file descriptor is a `BorrowedFd` and
the callback is executed on a different thread (where the looper is
polled on the `ThreadLooper`) and must hence be `Send.`.

It appears the `ndk-sys` and `ndk-context` crates are not used directly,
hence they are removed from `Cargo.toml` here.
---
 .changes/ndk-0.9.md      |  7 +++++++
 Cargo.toml               |  4 +---
 src/android/binding.rs   |  2 +-
 src/android/main_pipe.rs | 12 +++++++++---
 src/android/mod.rs       | 12 ++++++++----
 5 files changed, 26 insertions(+), 11 deletions(-)
 create mode 100644 .changes/ndk-0.9.md

diff --git a/.changes/ndk-0.9.md b/.changes/ndk-0.9.md
new file mode 100644
index 0000000000..2a97399213
--- /dev/null
+++ b/.changes/ndk-0.9.md
@@ -0,0 +1,7 @@
+---
+"wry": major
+---
+
+**Breaking change**: Upgrade `ndk` crate to `0.9` and delete unused `ndk-sys` and `ndk-context` dependencies.  Types from the `ndk` crates are used in public API surface.
+
+**Breaking change**: The public `android_setup()` function now takes `&ThreadLooper` instead of `&ForeignLooper`, signifying that the setup function must be called on the thread where the looper is attached (and the `JNIEnv` argument is already thread-local as well).
diff --git a/Cargo.toml b/Cargo.toml
index 514f5fad30..1a74c1ce5a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -104,9 +104,7 @@ kuchiki = { package = "kuchikiki", version = "0.8" }
 sha2 = "0.10"
 base64 = "0.22"
 jni = "0.21"
-ndk = "0.7"
-ndk-sys = "0.4"
-ndk-context = "0.1"
+ndk = "0.9"
 tao-macros = "0.1"
 libc = "0.2"
 
diff --git a/src/android/binding.rs b/src/android/binding.rs
index 8855f00b40..63a91400b1 100644
--- a/src/android/binding.rs
+++ b/src/android/binding.rs
@@ -27,7 +27,7 @@ macro_rules! android_binding {
   ($domain:ident, $package:ident) => {
     ::wry::android_binding!($domain, $package, ::wry)
   };
-  // use import `android_setup` just to force the import path to use `wry::{}`
+  // use imported `android_setup` just to force the import path to use `wry::{}`
   // as the macro breaks without braces
   ($domain:ident, $package:ident, $wry:path) => {{
     use $wry::{android_setup as _, prelude::*};
diff --git a/src/android/main_pipe.rs b/src/android/main_pipe.rs
index 042bcb2db0..ae7ebc4604 100644
--- a/src/android/main_pipe.rs
+++ b/src/android/main_pipe.rs
@@ -15,10 +15,10 @@ use std::{os::unix::prelude::*, sync::atomic::Ordering};
 use super::{find_class, EvalCallback, EVAL_CALLBACKS, EVAL_ID_GENERATOR, PACKAGE};
 
 static CHANNEL: Lazy<(Sender<WebViewMessage>, Receiver<WebViewMessage>)> = Lazy::new(|| bounded(8));
-pub static MAIN_PIPE: Lazy<[RawFd; 2]> = Lazy::new(|| {
+pub static MAIN_PIPE: Lazy<[OwnedFd; 2]> = Lazy::new(|| {
   let mut pipe: [RawFd; 2] = Default::default();
   unsafe { libc::pipe(pipe.as_mut_ptr()) };
-  pipe
+  unsafe { pipe.map(|fd| OwnedFd::from_raw_fd(fd)) }
 });
 
 pub struct MainPipe<'a> {
@@ -32,7 +32,13 @@ impl<'a> MainPipe<'a> {
   pub(crate) fn send(message: WebViewMessage) {
     let size = std::mem::size_of::<bool>();
     if let Ok(()) = CHANNEL.0.send(message) {
-      unsafe { libc::write(MAIN_PIPE[1], &true as *const _ as *const _, size) };
+      unsafe {
+        libc::write(
+          MAIN_PIPE[1].as_raw_fd(),
+          &true as *const _ as *const _,
+          size,
+        )
+      };
     }
   }
 
diff --git a/src/android/mod.rs b/src/android/mod.rs
index 37d186ce0b..f8d69e2ad3 100644
--- a/src/android/mod.rs
+++ b/src/android/mod.rs
@@ -17,13 +17,14 @@ use jni::{
   JNIEnv,
 };
 use kuchiki::NodeRef;
-use ndk::looper::{FdEvent, ForeignLooper};
+use ndk::looper::{FdEvent, ThreadLooper};
 use once_cell::sync::OnceCell;
 use raw_window_handle::HasWindowHandle;
 use sha2::{Digest, Sha256};
 use std::{
   borrow::Cow,
   collections::HashMap,
+  os::fd::{AsFd as _, AsRawFd as _},
   sync::{atomic::AtomicI32, mpsc::channel, Mutex},
 };
 
@@ -75,10 +76,13 @@ pub static EVAL_CALLBACKS: once_cell::sync::OnceCell<Mutex<HashMap<i32, EvalCall
   once_cell::sync::OnceCell::new();
 
 /// Sets up the necessary logic for wry to be able to create the webviews later.
+///
+/// This function must be run on the thread where the [`JNIEnv`] is registered and the looper is local,
+/// hence the requirement for a [`ThreadLooper`].
 pub unsafe fn android_setup(
   package: &str,
   mut env: JNIEnv,
-  looper: &ForeignLooper,
+  looper: &ThreadLooper,
   activity: GlobalRef,
 ) {
   PACKAGE.get_or_init(move || package.to_string());
@@ -108,10 +112,10 @@ pub unsafe fn android_setup(
   };
 
   looper
-    .add_fd_with_callback(MAIN_PIPE[0], FdEvent::INPUT, move |_| {
+    .add_fd_with_callback(MAIN_PIPE[0].as_fd(), FdEvent::INPUT, move |fd, _event| {
       let size = std::mem::size_of::<bool>();
       let mut wake = false;
-      if libc::read(MAIN_PIPE[0], &mut wake as *mut _ as *mut _, size) == size as libc::ssize_t {
+      if libc::read(fd.as_raw_fd(), &mut wake as *mut _ as *mut _, size) == size as libc::ssize_t {
         main_pipe.recv().is_ok()
       } else {
         false