ReviewOS
Reload

also looking at this

craft-native/craft

feat(android): the app-state trio in Zig — ratchet 59 → 56

#184
Merged glennmichael123 wants to merge feat/android-app-state into main
6 files +422 -3

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/android/templates/CraftBridge.kt.templatemodified+12-0
@@ -2128,11 +2128,21 @@ class CraftBridge(
21282128
21292129 @JavascriptInterface
21302130 fun getAppState(): String {
2131 // Null is "Zig declined". Otherwise this is the state Zig's observer
2132 // has been maintaining, and currentAppState below is the one this
2133 // file's observer maintains — whichever of the two is running.
2134 CraftNative.getAppState()?.let { return it }
2135
21312136 return currentAppState
21322137 }
21332138
21342139 @JavascriptInterface
21352140 fun startAppStateMonitoring() {
2141 // The observer object is CraftNative's, and so is the state it
2142 // maintains — which is why getAppState below has to reach the same
2143 // way. All three move together or none of them can.
2144 if (CraftNative.startAppStateMonitoring(activity)) return
2145
21362146 activity.runOnUiThread {
21372147 ProcessLifecycleOwner.get().lifecycle.addObserver(appStateObserver)
21382148 }
@@ -2140,6 +2150,8 @@ class CraftBridge(
21402150
21412151 @JavascriptInterface
21422152 fun stopAppStateMonitoring() {
2153 if (CraftNative.stopAppStateMonitoring(activity)) return
2154
21432155 activity.runOnUiThread {
21442156 ProcessLifecycleOwner.get().lifecycle.removeObserver(appStateObserver)
21452157 }
packages/android/templates/CraftNative.kt.templatemodified+69-0
@@ -1,6 +1,8 @@
11package com.craft.runtime
22
33import android.app.Activity
4import androidx.lifecycle.ProcessLifecycleOwner
5import androidx.lifecycle.LifecycleEventObserver
46import android.net.NetworkRequest
57import android.net.NetworkCapabilities
68import android.net.Network
@@ -147,6 +149,10 @@ object CraftNative {
147149 private external fun nativeNetworkChanged(activity: Activity)
148150 private external fun nativeStartNetworkMonitoring(activity: Activity): Boolean
149151 private external fun nativeStopNetworkMonitoring(activity: Activity): Boolean
152 private external fun nativeAppStateEvent(eventName: String)
153 private external fun nativeStartAppStateMonitoring(activity: Activity): Boolean
154 private external fun nativeStopAppStateMonitoring(activity: Activity): Boolean
155 private external fun nativeGetAppState(): String?
150156
151157 /**
152158 * `getDeviceInfo`, or null to use the Kotlin implementation.
@@ -848,4 +854,67 @@ object CraftNative {
848854 false
849855 }
850856 }
857
858 // ==================== App state ====================
859 //
860 // The second listener shim. `LifecycleEventObserver` is a Java interface,
861 // so the object is here; the mapping, the state and the two deliveries are
862 // in Zig.
863 //
864 // One observer instance, added and removed — `LifecycleRegistry` keys its
865 // observers by identity, so adding twice is a no-op. That is why this one
866 // has no leak to reproduce, unlike the network watch.
867 //
868 // The native takes no Activity: everything it does is state and the reply
869 // channel, and neither needs one.
870
871 private val appStateObserver = LifecycleEventObserver { _, event ->
872 try {
873 nativeAppStateEvent(event.name)
874 } catch (e: UnsatisfiedLinkError) {
875 // The library went away. Nothing is waiting on this.
876 }
877 }
878
879 @JvmStatic
880 fun startAppStateWatch(activity: Activity) {
881 activity.runOnUiThread {
882 ProcessLifecycleOwner.get().lifecycle.addObserver(appStateObserver)
883 }
884 }
885
886 @JvmStatic
887 fun stopAppStateWatch(activity: Activity) {
888 activity.runOnUiThread {
889 ProcessLifecycleOwner.get().lifecycle.removeObserver(appStateObserver)
890 }
891 }
892
893 fun startAppStateMonitoring(activity: Activity): Boolean {
894 if (!isAvailable) return false
895 return try {
896 nativeStartAppStateMonitoring(activity)
897 } catch (e: UnsatisfiedLinkError) {
898 false
899 }
900 }
901
902 fun stopAppStateMonitoring(activity: Activity): Boolean {
903 if (!isAvailable) return false
904 return try {
905 nativeStopAppStateMonitoring(activity)
906 } catch (e: UnsatisfiedLinkError) {
907 false
908 }
909 }
910
911 /** Null is "Zig declined"; the state itself is never null. */
912 fun getAppState(): String? {
913 if (!isAvailable) return null
914 return try {
915 nativeGetAppState()
916 } catch (e: UnsatisfiedLinkError) {
917 null
918 }
919 }
851920}
packages/zig/build.zigmodified+3-0
@@ -482,6 +482,9 @@ pub fn build(b: *std.Build) void {
482482 android_conformance_tests.root_module.addAnonymousImport("src/bridge_android_locationstore.zig", .{
483483 .root_source_file = b.path("src/bridge_android_locationstore.zig"),
484484 });
485 android_conformance_tests.root_module.addAnonymousImport("src/bridge_android_appstate.zig", .{
486 .root_source_file = b.path("src/bridge_android_appstate.zig"),
487 });
485488 const run_android_conformance_tests = b.addRunArtifact(android_conformance_tests);
486489
487490 // The page surface, across both bridges. Not folded into the iOS
packages/zig/src/android_dispatch.zigmodified+97-1
@@ -42,6 +42,7 @@ const system = ;
4242const clipboard = ;
4343const intents = ;
4444const network = ;
45const appstate = ;
4546const securestore = ;
4647const haptics = ;
4748const notifications = ;
@@ -407,6 +408,29 @@ const natives = [_]jni.JNINativeMethod{
407408 .signature = "(Landroid/app/Activity;)Z",
408409 .fnPtr = @ptrCast(&nativeStopNetworkMonitoring),
409410 },
411 // Not an action: the other end of the lifecycle observer the holder owns.
412 // It takes no Activity, because everything it does is state and the reply
413 // channel — neither of which needs one.
414 .{
415 .name = "nativeAppStateEvent",
416 .signature = "(Ljava/lang/String;)V",
417 .fnPtr = @ptrCast(&nativeAppStateEvent),
418 },
419 .{
420 .name = "nativeStartAppStateMonitoring",
421 .signature = "(Landroid/app/Activity;)Z",
422 .fnPtr = @ptrCast(&nativeStartAppStateMonitoring),
423 },
424 .{
425 .name = "nativeStopAppStateMonitoring",
426 .signature = "(Landroid/app/Activity;)Z",
427 .fnPtr = @ptrCast(&nativeStopAppStateMonitoring),
428 },
429 .{
430 .name = "nativeGetAppState",
431 .signature = "()Ljava/lang/String;",
432 .fnPtr = @ptrCast(&nativeGetAppState),
433 },
410434};
411435
412436/// How binding went. A value rather than a log line, so every path is
@@ -1776,6 +1800,78 @@ fn callHolderVoid(j: Jni, comptime name: [:0]const u8, activity: jni.jobject) !v
17761800 );
17771801}
17781802
1803/// `nativeAppStateEvent(eventName)` — a lifecycle event, arrived at.
1804///
1805/// Runs on the main looper, where `ProcessLifecycleOwner` dispatches. Silent:
1806/// there is no promise waiting, and a page that never registered a callback is
1807/// the ordinary case rather than an error.
1808fn nativeAppStateEvent(
1809 env: jni.JNIEnv,
1810 _: jni.jobject,
1811 event_name: jni.jstring,
1812) callconv(.c) void {
1813 const j = Jni.init(env);
1814 var arena = std.heap.ArenaAllocator.init(backing);
1815 defer arena.deinit();
1816 const allocator = arena.allocator();
1817
1818 const name = j.stringToUtf8(allocator, event_name) catch return;
1819
1820 // Null covers both "this event carries no state" and "it carries the one
1821 // already held", and the shim announces neither.
1822 const changed = appstate.apply(name) orelse return;
1823 appstate.announce(allocator, changed) catch {};
1824}
1825
1826/// `nativeStartAppStateMonitoring(activity)`.
1827///
1828/// The observer is the holder's — `LifecycleEventObserver` is a Java interface
1829/// — and adding it twice is a no-op, because `LifecycleRegistry` keys its
1830/// observers by identity. The shim has the same property for the same reason,
1831/// so unlike the network watch there is nothing to leak here.
1832fn nativeStartAppStateMonitoring(
1833 env: jni.JNIEnv,
1834 _: jni.jobject,
1835 activity: jni.jobject,
1836) callconv(.c) jni.jboolean {
1837 const j = Jni.init(env);
1838
1839 callHolderVoid(j, "startAppStateWatch", activity) catch |err| {
1840 std.log.warn("craft: startAppStateMonitoring fell through to the shim ({s})", .{@errorName(err)});
1841 return jni.JNI_FALSE;
1842 };
1843 return jni.JNI_TRUE;
1844}
1845
1846/// `nativeStopAppStateMonitoring(activity)`.
1847fn nativeStopAppStateMonitoring(
1848 env: jni.JNIEnv,
1849 _: jni.jobject,
1850 activity: jni.jobject,
1851) callconv(.c) jni.jboolean {
1852 const j = Jni.init(env);
1853
1854 callHolderVoid(j, "stopAppStateWatch", activity) catch |err| {
1855 std.log.warn("craft: stopAppStateMonitoring failed ({s})", .{@errorName(err)});
1856 return jni.JNI_FALSE;
1857 };
1858 return jni.JNI_TRUE;
1859}
1860
1861/// `nativeGetAppState()`.
1862///
1863/// Reads the state Zig now owns. This is why the trio moves together: the
1864/// Kotlin's `currentAppState` is only written by the observer, so serving the
1865/// observer without serving this would leave the field frozen at "active"
1866/// while the real state moved on.
1867fn nativeGetAppState(env: jni.JNIEnv, _: jni.jobject) callconv(.c) jni.jstring {
1868 const j = Jni.init(env);
1869 var arena = std.heap.ArenaAllocator.init(backing);
1870 defer arena.deinit();
1871
1872 return j.newStringUtf8(arena.allocator(), appstate.state().name()) catch null;
1873}
1874
17791875// =============================================================================
17801876// Tests
17811877// =============================================================================
@@ -1887,7 +1983,7 @@ test "the registered natives name methods the Kotlin actually declares" {
18871983 // A descriptor is checked by the JVM at registration, so a wrong one fails
18881984 // at load rather than at call — but only if the *name* matches something.
18891985 // These two strings are the contract with CraftBridge.kt.
1890 try testing.expectEqual(@as(usize, 46), natives.len);
1986 try testing.expectEqual(@as(usize, 50), natives.len);
18911987 try testing.expectEqualStrings("nativeGetDeviceInfo", std.mem.span(natives[0].name));
18921988
18931989 // And the class they bind to is the fixed one, not the templated bridge.
packages/zig/src/bridge_android_appstate.zigadded+237-0
@@ -0,0 +1,237 @@
1//! `startAppStateMonitoring`, `stopAppStateMonitoring` and `getAppState`.
2//!
3//! The second listener shim, and the first migration that moves *state* rather
4//! than working around it.
5//!
6//! ## Why all three, or none
7//!
8//! `getAppState` returns `currentAppState`, a field only the lifecycle
9//! observer writes. Serving the observer from Zig without serving the getter
10//! would leave the Kotlin field frozen at its initial "active" while the real
11//! state moved — the exact failure the flashlight deferral describes, where
12//! one half writes a field the other half reads.
13//!
14//! So the state moves here, and all three actions come with it. That is what
15//! the flashlight row means by "migrating the pair together would need the
16//! state to move too": this is what it looks like when it can.
17//!
18//! Process-global, because `ProcessLifecycleOwner` is: one answer per process,
19//! not per Activity. Atomic, because the observer runs on the main looper and
20//! `getAppState` is called from the WebView's JavaBridge thread.
21//!
22//! ## Only a change is announced
23//!
24//! The observer compares against the current state and does nothing when they
25//! match. `ON_START` immediately after `ON_RESUME` is an ordinary sequence and
26//! both map to "active", so without that check a page would see two
27//! `craftAppStateChange` events for one foregrounding.
28//!
29//! ## Two deliveries, not one
30//!
31//! A change calls `window._craftAppStateCallback(state)` *and* dispatches
32//! `craftAppStateChange`. Both, in that order, because the shim does both —
33//! `onAppStateChange` reads the first and `addEventListener` the second, and a
34//! page may well use either.
35
36const std = ;
37const events = ;
38const bridge_error = ;
39
40pub const A = struct {
41 pub const start_app_state_monitoring = "startAppStateMonitoring";
42 pub const stop_app_state_monitoring = "stopAppStateMonitoring";
43 pub const get_app_state = "getAppState";
44};
45
46/// The global `onAppStateChange` assigns, and the event name `sendEvent` uses.
47pub const callback_global = "_craftAppStateCallback";
48pub const event_name = "craftAppStateChange";
49
50pub const State = enum {
51 active,
52 inactive,
53 background,
54
55 pub fn name(self: State) []const u8 {
56 return switch (self) {
57 .active => "active",
58 .inactive => "inactive",
59 .background => "background",
60 };
61 }
62};
63
64/// The state a page sees before anything has happened.
65///
66/// `private var currentAppState = "active"` — so `getAppState` answers "active"
67/// even when monitoring was never started, and that is what a page gets today.
68const initial: State = .active;
69
70var current: std.atomic.Value(u8) = .init(@backingInt(initial));
71
72/// What the process currently believes.
73pub fn state() State {
74 return @fromBackingInt(@intCast(current.load(.acquire)));
75}
76
77/// Put the state back to its process start value.
78///
79/// For tests. Nothing in the bridge resets it, because nothing in a process
80/// un-launches the app.
81pub fn reset() void {
82 current.store(@backingInt(initial), .release);
83}
84
85/// `Lifecycle.Event` to a state, or null where the shim's `when` falls to
86/// `else` and keeps what it had.
87///
88/// The names are `Lifecycle.Event`'s own — the holder passes `event.name`
89/// rather than an ordinal, because an ordinal is a number that means nothing
90/// on its own and would move if the enum ever gained a member.
91pub fn stateFor(event: []const u8) ?State {
92 if (std.mem.eql(u8, event, "ON_START")) return .active;
93 if (std.mem.eql(u8, event, "ON_RESUME")) return .active;
94 if (std.mem.eql(u8, event, "ON_PAUSE")) return .inactive;
95 if (std.mem.eql(u8, event, "ON_STOP")) return .background;
96 return null;
97}
98
99/// Apply an event, and say whether it changed anything.
100///
101/// Null means "announce nothing" — either the event has no state (`ON_CREATE`,
102/// `ON_DESTROY`, `ON_ANY`) or it maps to the state already held.
103pub fn apply(event: []const u8) ?State {
104 const next = stateFor(event) orelse return null;
105 if (next == state()) return null;
106 current.store(@backingInt(next), .release);
107 return next;
108}
109
110/// `window._craftAppStateCallback("active")` — the payload, quoted.
111pub fn callbackPayload(allocator: std.mem.Allocator, value: State) ![]u8 {
112 var out: std.ArrayListUnmanaged(u8) = .empty;
113 errdefer out.deinit(allocator);
114
115 try out.append(allocator, '"');
116 try bridge_error.appendJsonEscaped(allocator, &out, value.name());
117 try out.append(allocator, '"');
118 return out.toOwnedSlice(allocator);
119}
120
121/// `{"state":"active"}` — the detail `sendEvent` builds.
122pub fn eventDetail(allocator: std.mem.Allocator, value: State) ![]u8 {
123 var out: std.ArrayListUnmanaged(u8) = .empty;
124 errdefer out.deinit(allocator);
125
126 try out.appendSlice(allocator, "{\"state\":\"");
127 try bridge_error.appendJsonEscaped(allocator, &out, value.name());
128 try out.appendSlice(allocator, "\"}");
129 return out.toOwnedSlice(allocator);
130}
131
132/// Announce a change, both ways the shim announces it.
133pub fn announce(allocator: std.mem.Allocator, value: State) !void {
134 const payload = try callbackPayload(allocator, value);
135 defer allocator.free(payload);
136 try events.settle(allocator, callback_global, payload);
137
138 const detail = try eventDetail(allocator, value);
139 defer allocator.free(detail);
140 try events.emitEvent(allocator, event_name, detail);
141}
142
143// =============================================================================
144// Tests
145// =============================================================================
146
147const testing = std.testing;
148
149test "every lifecycle event maps where the shim's when maps it" {
150 try testing.expectEqual(State.active, stateFor("ON_START").?);
151 try testing.expectEqual(State.active, stateFor("ON_RESUME").?);
152 try testing.expectEqual(State.inactive, stateFor("ON_PAUSE").?);
153 try testing.expectEqual(State.background, stateFor("ON_STOP").?);
154}
155
156test "an event with no state keeps the one already held" {
157 // The shim's `else -> currentAppState`. ON_CREATE and ON_DESTROY really do
158 // arrive, and ON_ANY is dispatched to observers registered for it.
159 try testing.expect(stateFor("ON_CREATE") == null);
160 try testing.expect(stateFor("ON_DESTROY") == null);
161 try testing.expect(stateFor("ON_ANY") == null);
162
163 // And anything unrecognised, which is what a future enum member would be.
164 try testing.expect(stateFor("") == null);
165 try testing.expect(stateFor("on_start") == null);
166}
167
168test "a process starts active, whether or not anything is monitoring" {
169 // `private var currentAppState = "active"`. A page calling getAppState
170 // before startAppStateMonitoring gets "active" rather than an error or an
171 // empty string, and that is the answer it gets today.
172 reset();
173 try testing.expectEqual(State.active, state());
174 try testing.expectEqualStrings("active", state().name());
175}
176
177test "only a change is announced" {
178 reset();
179
180 // Already active: ON_START and ON_RESUME are both no-ops, which is what
181 // stops one foregrounding producing two craftAppStateChange events.
182 try testing.expect(apply("ON_START") == null);
183 try testing.expect(apply("ON_RESUME") == null);
184
185 // A real change reports the new state and keeps it.
186 try testing.expectEqual(State.inactive, apply("ON_PAUSE").?);
187 try testing.expectEqual(State.inactive, state());
188
189 // The same event again changes nothing.
190 try testing.expect(apply("ON_PAUSE") == null);
191
192 // And on to background, then back.
193 try testing.expectEqual(State.background, apply("ON_STOP").?);
194 try testing.expectEqual(State.active, apply("ON_START").?);
195 reset();
196}
197
198test "an event with no state does not disturb the one held" {
199 reset();
200 _ = apply("ON_PAUSE");
201 try testing.expect(apply("ON_CREATE") == null);
202 try testing.expectEqual(State.inactive, state());
203 reset();
204}
205
206test "the two deliveries carry the same state in two shapes" {
207 // `_craftAppStateCallback("active")` and
208 // `dispatchEvent(new CustomEvent('craftAppStateChange', {detail: {"state":"active"}}))`
209 // — a bare string for the callback, an object for the event.
210 const payload = try callbackPayload(testing.allocator, .background);
211 defer testing.allocator.free(payload);
212 try testing.expectEqualStrings("\"background\"", payload);
213
214 const detail = try eventDetail(testing.allocator, .background);
215 defer testing.allocator.free(detail);
216 try testing.expectEqualStrings("{\"state\":\"background\"}", detail);
217
218 var parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, detail, .{});
219 defer parsed.deinit();
220 try testing.expectEqualStrings("background", parsed.value.object.get("state").?.string);
221}
222
223test "the state names are the strings the page compares against" {
224 // A page writes `state === 'background'`, so these are API rather than
225 // internal spelling.
226 try testing.expectEqualStrings("active", State.active.name());
227 try testing.expectEqualStrings("inactive", State.inactive.name());
228 try testing.expectEqualStrings("background", State.background.name());
229}
230
231test "the actions and globals match the shim exactly" {
232 try testing.expectEqualStrings("startAppStateMonitoring", A.start_app_state_monitoring);
233 try testing.expectEqualStrings("stopAppStateMonitoring", A.stop_app_state_monitoring);
234 try testing.expectEqualStrings("getAppState", A.get_app_state);
235 try testing.expectEqualStrings("_craftAppStateCallback", callback_global);
236 try testing.expectEqualStrings("craftAppStateChange", event_name);
237}
packages/zig/test/android_conformance_test.zigmodified+4-2
@@ -56,6 +56,7 @@ const zig_sources = [_][]const u8{
5656 @embedFile("src/bridge_android_screen.zig"),
5757 @embedFile("src/bridge_android_files.zig"),
5858 @embedFile("src/bridge_android_locationstore.zig"),
59 @embedFile("src/bridge_android_appstate.zig"),
5960};
6061
6162/// How many spec actions Zig does not serve yet.
@@ -89,8 +90,9 @@ const zig_sources = [_][]const u8{
8990/// the service that owns them; 61 with the four controls, once Kotlin agreed
9091/// to name its own service class; 59 with the network monitoring pair, the
9192/// first listener shim — the callback object is the holder's, the work is
92/// Zig's.
93const max_not_yet_migrated: usize = 59;
93/// Zig's; 56 with the app-state trio, the first migration where the state
94/// itself moved rather than being worked around.
95const max_not_yet_migrated: usize = 56;
9496
9597/// Every `@JavascriptInterface fun <name>(` in the Kotlin bridge.
9698///