Address the #25 review: per-host TalkBack Activity, reply draft

.claude/skills/gpui-mobile/SKILL.md

@@ -108,7 +108,7 @@ These come from bugs actually hit in this stack.
    them from `cx.background_spawn(...)`; from GPUI's thread they return an
    error on purpose.
 2. **Host wiring order matters.** In the host's `onCreate`, call
-   `set_host_activity` first, then register `host::on_open_window`, then
+   `host::set_activity(env, host, &activity)` first, then register `host::on_open_window`, then
    `host::start_with_assets(gpui_kit::assets::AllAssets, launch)`. Without
    `start_with_assets` Kit's icons are blank; without `on_open_window` before
    the first surface, the screen stays blank.

.claude/skills/gpui-mobile/references/gotchas.md

@@ -9,7 +9,7 @@ anything newer.
 |---|---|
 | Blank screen on start | `host::on_open_window` registered after the first `surface_created`, or not at all. |
 | Icons and SVGs missing | `host::start` instead of `host::start_with_assets(gpui_kit::assets::AllAssets, …)`. |
-| Emoji show as boxes | Android 13+ ships a COLRv1 emoji font that swash can't draw. Bundle `assets/fonts/NotoColorEmoji.ttf` (CBDT) and call `set_host_activity` before `host::start`. Flags and ❤️ with VS16 still don't render. |
+| Emoji show as boxes | Android 13+ ships a COLRv1 emoji font that swash can't draw. Bundle `assets/fonts/NotoColorEmoji.ttf` (CBDT) and call `host::set_activity` (or `set_host_activity`) before `host::start`. Flags and ❤️ with VS16 still don't render. |
 | Dark mode doesn't follow the system | The Activity lists `uiMode` in `configChanges` but doesn't call `host::configuration_changed()` from `onConfigurationChanged`. |
 | Text white on white in a dark chat bubble; shimmer invisible | Kit bugs kit#3326, kit#3327; not Android-specific. |
 | Bold/italic look regular | Roboto is a variable font; weights aren't applied. |

.claude/skills/gpui-mobile/references/new-app.md

@@ -94,7 +94,7 @@ API:
 
 | Call | When |
 |---|---|
-| `jni::set_host_activity(env, &activity)` | first thing in every `onCreate`, recreation included |
+| `host::set_activity(env, host, &activity)` (or `jni::set_host_activity`, which does not tell hosts apart: TalkBack then lands on the wrong Activity after Back) | first thing in every `onCreate`, recreation included |
 | `host::on_open_window(\|_, cx\| open_window(cx))` | before the first surface |
 | `host::start_with_assets(gpui_kit::assets::AllAssets, launch)` | every `onCreate`; only the first call starts the render thread |
 | `host::surface_created(host, NativeWindow::from_surface(env, surface), scale)` | `surfaceCreated` and `surfaceChanged` (a repeat is a resize) |

Cargo.toml

@@ -20,7 +20,7 @@ crate-type = ["cdylib"]
 # (see MOBILE_PATCHES.md for what each carries on top of upstream).
 gpui = { package = "gpui-pre", version = "=0.3.8", default-features = false }
 gpui-kit = { git = "https://github.com/Bombatomica64/gpui-kit", rev = "e8b28164ac7fa7e1f7d393d441beb738c286bbc8", default-features = false, features = ["component", "assets"] }
-gpui-mobile = { package = "gpui-pre-mobile", git = "https://github.com/Bombatomica64/gpui-mobile", rev = "c71a31aaa12660552ca7a772f6fdd890d9c8cf7a", default-features = false, features = [
+gpui-mobile = { package = "gpui-pre-mobile", git = "https://github.com/Bombatomica64/gpui-mobile", rev = "4acd1761b4b2a734f49ff2bce930c61b01cfe5be", default-features = false, features = [
     "clipboard", "share",
     # Exercised by the Platform APIs screen; build.sh copies their Java helpers from this checkout.
     "file_selector", "image_picker", "local_auth", "permission_handler", "calendar", "contacts",

MOBILE_PATCHES.md

@@ -35,6 +35,19 @@ follows whichever window is active.
 [Bounds](docs/demos/android-a11y-bounds.png),
 [recording](docs/demos/android-a11y-talkback.gif).
 
+Review changes requested; rebased onto upstream `main` and fixed in separate
+commits. The reply draft is in
+[docs/upstream/replies](docs/upstream/replies/gpui-mobile-25.md). The
+adapter now goes on each host's own Activity: hosts call the new
+`host::set_activity(env, host, activity)` (the lab does) in place of
+`set_host_activity`. It is attached when the window becomes active rather than
+in `onCreate`, is retried after a failure, and is dropped with its window. A
+view that already has another delegate is skipped. The exception-clearing
+branch below is folded in. One gap stays open: a screen reader turned off
+between the GPUI-side `isEnabled()` check and `accesskit_android`'s UI-thread
+runnable still throws, because only the adapter can check there
+(`accesskit_android` 0.9 does, but needs `accesskit` 0.25).
+
 ### Keep focus when the IME's Done action hides the keyboard — branch `android-ime-dismiss`
 
 Upstream (#26) stopped back from injecting an `escape` keystroke (IME event
@@ -97,7 +110,7 @@ change. `safe_area_insets()` and `keyboard_height()` return them too, as on
 iOS. LabActivity calls it from its `OnApplyWindowInsetsListener`.
 Draft upstream PR: [docs/upstream/safe-area-insets.md](docs/upstream/safe-area-insets.md).
 
-### Clear the Java exceptions the accessibility bridge leaves pending — branch `android-a11y-clear-exceptions`
+### Clear the Java exceptions the accessibility bridge leaves pending — branch `android-a11y-clear-exceptions`, now part of `android-accessibility`
 
 `accesskit_android`'s jni 0.21 API leaves a thrown Java exception pending,
 and the bridge never cleared it, so the next JNI call on the render thread

android/app/src/main/java/dev/gpui/mobile/lab/LabActivity.java

@@ -106,7 +106,7 @@ protected void onCreate(Bundle state) {
             return insets;
         });
 
-        nativeOnCreate(this, Build.VERSION.SDK_INT);
+        nativeOnCreate(this, hostId, Build.VERSION.SDK_INT);
         forwardScreenExtra(getIntent());
     }
 
@@ -453,7 +453,7 @@ private void endEdit() {
 
     // ── Native (src/host.rs) ───────────────────────────────────────────────
 
-    private static native void nativeOnCreate(Activity activity, int apiLevel);
+    private static native void nativeOnCreate(Activity activity, long host, int apiLevel);
     private static native void nativeOpenScreen(String screen);
     private static native void nativeDeepLink(String url);
     private static native void nativeConfigurationChanged();

docs/upstream/README.md

@@ -33,7 +33,7 @@ captures in [../demos](../demos) and say where a capture is missing.
 | 11 | mobile | [`upstream/android-ime-dismiss`](https://github.com/Bombatomica64/gpui-mobile/tree/upstream/android-ime-dismiss) | ✅ | — | after only | [draft](gpui-mobile/android-ime-dismiss.md) |
 | 12 | kit | [`upstream/input-tap-requests-keyboard`](https://github.com/Bombatomica64/gpui-kit/tree/upstream/input-tap-requests-keyboard) | ⏳ | #11 (otherwise it's a no-op on Android) | no | [draft](gpui-kit/input-tap-requests-keyboard.md) |
 | 13 | mobile | [`upstream/android-text-input-configuration`](https://github.com/Bombatomica64/gpui-mobile/tree/upstream/android-text-input-configuration) | ⏳ | #11 merged, #4 released in Kit | yes | [draft](gpui-mobile/android-text-input-configuration.md) |
-| 14 | mobile | [`upstream/android-a11y-clear-exceptions`](https://github.com/Bombatomica64/gpui-mobile/tree/upstream/android-a11y-clear-exceptions) | ⏳ | open #25 (push it there, or send after #25 merges) | not possible (code audit) | [draft](gpui-mobile/android-a11y-clear-exceptions.md) |
+| 14 | mobile | [`upstream/android-a11y-clear-exceptions`](https://github.com/Bombatomica64/gpui-mobile/tree/upstream/android-a11y-clear-exceptions) | ✅ | pushed to #25 (7006f50) with the review fixes | not possible (code audit) | [draft](gpui-mobile/android-a11y-clear-exceptions.md) |
 | 15 | kit | [`upstream/input-reveal-on-focus`](https://github.com/Bombatomica64/gpui-kit/tree/upstream/input-reveal-on-focus) | ⏳ | Zed autoscroll PR in a gpui-pre release (its own test fails until then) | no | [draft](gpui-kit/input-reveal-on-focus.md) |
 | 16 | mobile | [`upstream/android-text-input-purpose`](https://github.com/Bombatomica64/gpui-mobile/tree/upstream/android-text-input-purpose) | ⛔ | #13, plus Zed `TextInputConfiguration::purpose` in a gpui-pre release; doesn't build until then | yes | [draft](gpui-mobile/android-text-input-purpose.md) |
 | 17 | kit | `input-purpose` (fork branch only) | ⛔ | #4, plus the same GPUI change; conflicts with `main` (to rebase then) | yes | — |

docs/upstream/replies/gpui-mobile-25.md

@@ -0,0 +1,61 @@
+# Reply to huacnlee's review of longbridge/gpui-mobile#25
+
+Draft, to be posted by hand. The branch was rebased onto `main` (it conflicted
+with #29 in `jni.rs`) and force-pushed; the fixes are separate commits on top,
+head [31d79d8](https://github.com/longbridge/gpui-mobile/pull/25/commits/31d79d8f51ac71eb58bad9fa433e17758002cf96).
+
+---
+
+Thanks for the careful review! I rebased onto `main` (it conflicted with #29)
+and added one commit per point:
+
+1. **Wrong Activity after Back** (2b761c1). New `host::set_activity(env, host,
+   activity)` does what `set_host_activity` does and also remembers the Activity
+   per `HostId`. Each window knows its host, so the adapter goes on that host's
+   own Activity. Hosts that only call `set_host_activity` keep the old behaviour,
+   and the "What the host has to do" docs now point to `set_activity`.
+2. **`isEnabled()` race** (not fully fixed, see below). I don't think
+   gpui-mobile can close it on its own. The runnable that raises the events is
+   posted by `InjectingAdapter::update_if_active` inside `accesskit_android`,
+   and nothing runs between it and `requestSendAccessibilityEvent`. A
+   `Looper`-side check could still lose to a queued `MSG_SET_STATE`.
+   `accesskit_android` 0.9 fixes this the way you describe: `QueuedEvents::raise`
+   checks `isEnabled()` on the UI thread. But 0.9 needs `accesskit` 0.25, and
+   gpui-pre 0.3.8 is on 0.24. What changed here:
+   - 7006f50 clears the Java exceptions that `accesskit_android`'s jni 0.21
+     leaves pending.
+   - 31d79d8 makes the GPUI-thread check cheaper and documents the remaining
+     window in the module docs.
+
+   The real fix is either a backported `accesskit_android` 0.7.x, which I can
+   ask AccessKit for, or moving to 0.9 once gpui-pre is on `accesskit` 0.25.
+   Would you merge with this documented, or wait for one of those?
+3. **`BRIDGE` not cleared** (d0bea7c). `HostDestroyed` and `Drop for
+   AndroidWindow` now drop the bridge, and with it the adapter, when it belongs
+   to that window. The closed window's callbacks are not called.
+4. **Existing AccessibilityDelegate** (34f8c96). Before creating the adapter,
+   `attach` reads `getAccessibilityDelegate()`. If the delegate is not an earlier
+   AccessKit one (whose removal is already queued ahead), it logs and skips the
+   view.
+5. **No retry** (ea97c6d). `init` now attaches again when there is no adapter.
+   Since 9418278 it also does so when the window's Activity has changed.
+6. **Attach before `setContentView`** (9418278). `set_host_activity` no longer
+   touches the adapter. It is attached whenever the window becomes active: on
+   resume, and again (as a cheap no-op if nothing changed) when its surface
+   arrives.
+
+Non-blocking:
+
+- **`isEnabled()` under the lock** (31d79d8): the `AccessibilityManager` and
+  the `JavaVM` are cached once, and the check runs before `BRIDGE` is taken.
+  Nothing takes `BRIDGE` on the UI thread any more. An
+  `AccessibilityStateChangeListener` would need a Java class, which this crate
+  doesn't ship, so I kept the per-frame call. It is a synchronized field read.
+- **Two jni versions**: the second one stays as long as we use
+  `accesskit_android`, because `InjectingAdapter::new` takes its jni 0.21
+  `JNIEnv`. The VM is no longer rebuilt per attach (31d79d8).
+- **Hover before `hit_test`** (6d443bd): moved.
+
+Checked: `cargo check` and `cargo clippy --target aarch64-linux-android` (no
+warnings in the touched files), `cargo fmt --check`, and `cargo test --lib`
+(46 passed). On a device: pending — I will add TalkBack results (toggling it off mid-draw, Back from a second screen) here before posting.

src/host.rs

@@ -45,12 +45,14 @@ pub extern "system" fn Java_dev_gpui_mobile_lab_LabActivity_nativeOnCreate<'loca
     mut env: EnvUnowned<'local>,
     _this: JObject<'local>,
     activity: JObject<'local>,
+    host: i64,
     api_level: i32,
 ) {
     init_logging();
     env.with_env(|env| -> jni::errors::Result<()> {
-        if let Err(err) = mobile_jni::set_host_activity(env, &activity) {
-            log::error!("set_host_activity failed: {err}");
+        // Per host, so the screen reader goes on this Activity's views after Back.
+        if let Err(err) = host::set_activity(env, host as host::HostId, &activity) {
+            log::error!("set_activity failed: {err}");
         }
         Ok(())
     })