ReviewOS

also looking at this

craft-native/craft

feat(android): the first listener shim, and network monitoring on it — 61 → 59

#183
Merged glennmichael123 wants to merge feat/android-network-watch into main
5 files +246 -13
packages/android/templates/CraftBridge.kt.templatemodified+8-0
Changes to packages/android/templates/CraftBridge.kt.template
@@ -1962,6 +1962,12 @@ class CraftBridge(
19621962
19631963 @JavascriptInterface
19641964 fun startNetworkMonitoring() {
1965 // The callback object is CraftNative's, because JNI cannot subclass
1966 // an abstract Java class — what Zig owns is what happens when it
1967 // fires. Returning here keeps this file's networkCallback null, which
1968 // is why stopNetworkMonitoring has to reach the same way.
1969 if (CraftNative.startNetworkMonitoring(activity)) return
1970
19651971 val connectivityManager = activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
19661972
19671973 networkCallback = object : ConnectivityManager.NetworkCallback() {
@@ -1987,6 +1993,8 @@ class CraftBridge(
19871993
19881994 @JavascriptInterface
19891995 fun stopNetworkMonitoring() {
1996 if (CraftNative.stopNetworkMonitoring(activity)) return
1997
19901998 val connectivityManager = activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
19911999 networkCallback?.let {
19922000 connectivityManager.unregisterNetworkCallback(it)
packages/android/templates/CraftNative.kt.templatemodified+101-0
Changes to packages/android/templates/CraftNative.kt.template
@@ -1,6 +1,11 @@
11package com.craft.runtime
22
33import android.app.Activity
4import android.net.NetworkRequest
5import android.net.NetworkCapabilities
6import android.net.Network
7import android.net.ConnectivityManager
8import android.content.Context
49import androidx.core.content.ContextCompat
510import android.content.Intent
611import android.os.Handler
@@ -139,6 +144,9 @@ object CraftNative {
139144 private external fun nativeStopLocationRecording(activity: Activity): String?
140145 private external fun nativePauseLocationRecording(activity: Activity): String?
141146 private external fun nativeResumeLocationRecording(activity: Activity): String?
147 private external fun nativeNetworkChanged(activity: Activity)
148 private external fun nativeStartNetworkMonitoring(activity: Activity): Boolean
149 private external fun nativeStopNetworkMonitoring(activity: Activity): Boolean
142150
143151 /**
144152 * `getDeviceInfo`, or null to use the Kotlin implementation.
@@ -747,4 +755,97 @@ object CraftNative {
747755 .setAction(LocationRecordingService.ACTION_STOP)
748756 )
749757 }
758
759 // ==================== Network monitoring ====================
760 //
761 // The first listener shim. `ConnectivityManager.NetworkCallback` is an
762 // abstract Java class, and JNI cannot subclass one — `RegisterNatives`
763 // binds methods onto a class that already exists in the APK, so a native
764 // can be *called* from Java but cannot be a Java object of a type the
765 // framework demands.
766 //
767 // The split is the one `runOnMain` uses: the object lives here, the work
768 // lives in Zig. This class holds the callback and the registration; every
769 // override forwards to `nativeNetworkChanged`, which reads the status and
770 // sends the event.
771 //
772 // Unlike `runOnMain` there is no token, because there is only ever one
773 // watch — which is also true of the CraftBridge field this replaces, and
774 // is the reason both of them leak a second one (see the note on start).
775
776 private var networkWatch: ConnectivityManager.NetworkCallback? = null
777
778 /**
779 * Register a network callback, exactly as `CraftBridge` would.
780 *
781 * Including the part that is wrong: a second call registers a second
782 * callback and overwrites the reference to the first, which then runs
783 * until the process dies and can never be unregistered. That is the
784 * shim's behaviour and is reproduced rather than guarded, because guarding
785 * would mean a page calling start twice gets one event per change here and
786 * two on the Kotlin path.
787 */
788 @JvmStatic
789 fun startNetworkWatch(activity: Activity) {
790 val manager =
791 activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
792
793 val watch = object : ConnectivityManager.NetworkCallback() {
794 override fun onAvailable(network: Network) = changed(activity)
795 override fun onLost(network: Network) = changed(activity)
796 override fun onCapabilitiesChanged(
797 network: Network,
798 networkCapabilities: NetworkCapabilities
799 ) = changed(activity)
800 }
801
802 val request = NetworkRequest.Builder()
803 .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
804 .build()
805
806 manager.registerNetworkCallback(request, watch)
807 networkWatch = watch
808 }
809
810 @JvmStatic
811 fun stopNetworkWatch(activity: Activity) {
812 val manager =
813 activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
814 networkWatch?.let { manager.unregisterNetworkCallback(it) }
815 networkWatch = null
816 }
817
818 /**
819 * Every override lands here.
820 *
821 * This runs on a Binder thread, not the main one — which is fine, because
822 * the reply channel attaches the thread it is called on and hops to the
823 * main looper itself.
824 */
825 private fun changed(activity: Activity) {
826 try {
827 nativeNetworkChanged(activity)
828 } catch (e: UnsatisfiedLinkError) {
829 // The library went away. There is no promise waiting on this and
830 // nothing to retry.
831 }
832 }
833
834 fun startNetworkMonitoring(activity: Activity): Boolean {
835 if (!isAvailable) return false
836 return try {
837 nativeStartNetworkMonitoring(activity)
838 } catch (e: UnsatisfiedLinkError) {
839 false
840 }
841 }
842
843 fun stopNetworkMonitoring(activity: Activity): Boolean {
844 if (!isAvailable) return false
845 return try {
846 nativeStopNetworkMonitoring(activity)
847 } catch (e: UnsatisfiedLinkError) {
848 false
849 }
850 }
750851}
packages/zig/src/android_dispatch.zigmodified+88-11
Changes to packages/zig/src/android_dispatch.zig
@@ -391,6 +391,22 @@ const natives = [_]jni.JNINativeMethod{
391391 .signature = "(Landroid/app/Activity;)Ljava/lang/String;",
392392 .fnPtr = @ptrCast(&nativeResumeLocationRecording),
393393 },
394 // Not an action: the other end of the network callback the holder owns.
395 .{
396 .name = "nativeNetworkChanged",
397 .signature = "(Landroid/app/Activity;)V",
398 .fnPtr = @ptrCast(&nativeNetworkChanged),
399 },
400 .{
401 .name = "nativeStartNetworkMonitoring",
402 .signature = "(Landroid/app/Activity;)Z",
403 .fnPtr = @ptrCast(&nativeStartNetworkMonitoring),
404 },
405 .{
406 .name = "nativeStopNetworkMonitoring",
407 .signature = "(Landroid/app/Activity;)Z",
408 .fnPtr = @ptrCast(&nativeStopNetworkMonitoring),
409 },
394410};
395411
396412/// How binding went. A value rather than a log line, so every path is
@@ -1676,25 +1692,86 @@ fn nativeResumeLocationRecording(
16761692}
16771693
16781694fn startRecordingService(j: Jni, activity: jni.jobject) !void {
1679 try j.pushLocalFrame(8);
1680 defer _ = j.popLocalFrame(null);
1681
1682 const holder = try j.findClass(holder_class);
1683 try j.callStaticVoidMethodA(
1684 holder,
1685 try j.staticMethodId(holder, "startRecordingService", "(Landroid/app/Activity;)V"),
1686 &.{.{ .l = activity }},
1687 );
1695 return callHolderVoid(j, "startRecordingService", activity);
16881696}
16891697
16901698fn stopRecordingService(j: Jni, activity: jni.jobject) !void {
1699 return callHolderVoid(j, "stopRecordingService", activity);
1700}
1701
1702/// `nativeNetworkChanged(activity)` — a network callback, arrived at.
1703///
1704/// Runs on a Binder thread. That is fine and is the reason the reply channel
1705/// exists in the shape it does: `events.settle` attaches whatever thread it is
1706/// called on and the Kotlin deliverer hops to the main looper.
1707///
1708/// Void and silent. There is no promise waiting on this — the page registered
1709/// a callback with `onNetworkChange` and every change calls it — so a failure
1710/// has nowhere to be reported and shows up in `droppedCount` instead.
1711fn nativeNetworkChanged(
1712 env: jni.JNIEnv,
1713 _: jni.jobject,
1714 activity: jni.jobject,
1715) callconv(.c) void {
1716 const j = Jni.init(env);
1717 var arena = std.heap.ArenaAllocator.init(backing);
1718 defer arena.deinit();
1719 const allocator = arena.allocator();
1720
1721 const status = network.read(j, activity) catch return;
1722 const json = network.render(allocator, status) catch return;
1723 events.settle(allocator, network.change_global, json) catch {};
1724}
1725
1726/// `nativeStartNetworkMonitoring(activity)`.
1727///
1728/// The callback object is the holder's, because
1729/// `ConnectivityManager.NetworkCallback` is an abstract Java class and JNI
1730/// cannot subclass one. What Zig owns is what happens when it fires.
1731fn nativeStartNetworkMonitoring(
1732 env: jni.JNIEnv,
1733 _: jni.jobject,
1734 activity: jni.jobject,
1735) callconv(.c) jni.jboolean {
1736 const j = Jni.init(env);
1737
1738 callHolderVoid(j, "startNetworkWatch", activity) catch |err| {
1739 std.log.warn("craft: startNetworkMonitoring fell through to the shim ({s})", .{@errorName(err)});
1740 return jni.JNI_FALSE;
1741 };
1742 return jni.JNI_TRUE;
1743}
1744
1745/// `nativeStopNetworkMonitoring(activity)`.
1746///
1747/// A false here after a successful start would leave the callback registered:
1748/// the shim's own `networkCallback` field is null, so its stop finds nothing
1749/// to unregister. The holder's stop is the only thing that can undo the
1750/// holder's start, which is why this reports the JNI failure rather than
1751/// pretending the shim can finish the job.
1752fn nativeStopNetworkMonitoring(
1753 env: jni.JNIEnv,
1754 _: jni.jobject,
1755 activity: jni.jobject,
1756) callconv(.c) jni.jboolean {
1757 const j = Jni.init(env);
1758
1759 callHolderVoid(j, "stopNetworkWatch", activity) catch |err| {
1760 std.log.warn("craft: stopNetworkMonitoring failed ({s})", .{@errorName(err)});
1761 return jni.JNI_FALSE;
1762 };
1763 return jni.JNI_TRUE;
1764}
1765
1766/// Call a `(Activity) -> void` static on the holder.
1767fn callHolderVoid(j: Jni, comptime name: [:0]const u8, activity: jni.jobject) !void {
16911768 try j.pushLocalFrame(8);
16921769 defer _ = j.popLocalFrame(null);
16931770
16941771 const holder = try j.findClass(holder_class);
16951772 try j.callStaticVoidMethodA(
16961773 holder,
1697 try j.staticMethodId(holder, "stopRecordingService", "(Landroid/app/Activity;)V"),
1774 try j.staticMethodId(holder, name, "(Landroid/app/Activity;)V"),
16981775 &.{.{ .l = activity }},
16991776 );
17001777}
@@ -1810,7 +1887,7 @@ test "the registered natives name methods the Kotlin actually declares" {
18101887 // A descriptor is checked by the JVM at registration, so a wrong one fails
18111888 // at load rather than at call — but only if the *name* matches something.
18121889 // These two strings are the contract with CraftBridge.kt.
1813 try testing.expectEqual(@as(usize, 43), natives.len);
1890 try testing.expectEqual(@as(usize, 46), natives.len);
18141891 try testing.expectEqualStrings("nativeGetDeviceInfo", std.mem.span(natives[0].name));
18151892
18161893 // And the class they bind to is the fixed one, not the templated bridge.
packages/zig/src/bridge_android_network.zigmodified+45-0
Changes to packages/zig/src/bridge_android_network.zig
@@ -38,8 +38,18 @@ const jobject = jni.jobject;
3838
3939pub const A = struct {
4040 pub const get_network_status = "getNetworkStatus";
41 pub const start_network_monitoring = "startNetworkMonitoring";
42 pub const stop_network_monitoring = "stopNetworkMonitoring";
4143};
4244
45/// The global the injected JS assigns for network changes.
46///
47/// Not a promise: `onNetworkChange(cb)` stores the callback once and every
48/// change calls it, so this global outlives any single reply — which is why
49/// the guard in front of it matters more here than elsewhere. A change
50/// arriving before the page has registered is dropped rather than throwing.
51pub const change_global = "_craftNetworkChangeCallback";
52
4353/// The transports the Kotlin's `when` distinguishes, in its order.
4454///
4555/// Order is contract: `when` takes the first match, so a connection that is
@@ -152,6 +162,14 @@ pub fn read(j: Jni, activity: jobject) !NetworkStatus {
152162 return .{ .connected = true, .transport = .none };
153163}
154164
165/// `NetworkCapabilities.NET_CAPABILITY_INTERNET`, the one capability the
166/// shim's `NetworkRequest` asks for.
167///
168/// Written down rather than read, because it is the Kotlin holder that builds
169/// the request — this constant is here only so the test can say what the
170/// holder is expected to have asked for.
171pub const net_capability_internet: i32 = 12;
172
155173// =============================================================================
156174// Tests
157175// =============================================================================
@@ -351,3 +369,30 @@ test "null capabilities is not connected, and an unknown transport still is" {
351369 try testing.expectEqual(true, odd.connected);
352370 try testing.expectEqual(Transport.none, odd.transport);
353371}
372
373test "the monitoring pair names itself as the shim does" {
374 try testing.expectEqualStrings("startNetworkMonitoring", A.start_network_monitoring);
375 try testing.expectEqualStrings("stopNetworkMonitoring", A.stop_network_monitoring);
376}
377
378test "a change is delivered to the callback the page registered, not a promise" {
379 // `onNetworkChange(cb)` assigns this once and every change calls it. A
380 // promise global is assigned per call and consumed; this one is not, so a
381 // page that never called `onNetworkChange` leaves it undefined and the
382 // guard in `settle` is what stops a ReferenceError inside
383 // evaluateJavascript, where nothing would see it.
384 try testing.expectEqualStrings("_craftNetworkChangeCallback", change_global);
385 try testing.expect(!std.mem.eql(u8, change_global, "_craftNetworkResolve"));
386}
387
388test "a change carries the same object getNetworkStatus returns" {
389 // `sendNetworkChange` calls `getNetworkStatus()` and sends the result
390 // verbatim, so the event payload and the polled answer cannot drift —
391 // and here they are literally the same function.
392 const json = try render(testing.allocator, .{ .connected = true, .transport = .wifi });
393 defer testing.allocator.free(json);
394
395 try testing.expectEqualStrings(
396 \\{"isConnected":true,"type":"wifi","isWifi":true,"isCellular":false}
397 , json);
398}
packages/zig/test/android_conformance_test.zigmodified+4-2
Changes to packages/zig/test/android_conformance_test.zig
@@ -87,8 +87,10 @@ const zig_sources = [_][]const u8{
8787/// 67 with downloadFile and saveFile, neither of which needed the main thread
8888/// at all; 65 with the recording store's two reads, whose writers stay with
8989/// the service that owns them; 61 with the four controls, once Kotlin agreed
90/// to name its own service class.
91const max_not_yet_migrated: usize = 61;
90/// to name its own service class; 59 with the network monitoring pair, the
91/// first listener shim — the callback object is the holder's, the work is
92/// Zig's.
93const max_not_yet_migrated: usize = 59;
9294
9395/// Every `@JavascriptInterface fun <name>(` in the Kotlin bridge.
9496///