ReviewOS

also looking at this

craft-native/craft

feat(android): the widget pair in Zig — ratchet 77 → 75

#167
Merged chris wants to merge feat/android-widgets into main
6 files +456 -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+7-0
Changes to packages/android/templates/CraftBridge.kt.template
@@ -3740,6 +3740,11 @@ class CraftBridge(
37403740
37413741 @JavascriptInterface
37423742 fun updateWidget(dataJson: String) {
3743 // Zig answers through the reply channel, so returning here is what
3744 // stops the promise being settled twice. The action is passed across
3745 // rather than rebuilt there — see CraftNative.updateWidget.
3746 if (CraftNative.updateWidget(activity, ".WIDGET_UPDATE", dataJson)) return
3747
37433748 try {
37443749 val data = JSONObject(dataJson)
37453750 val editor = widgetPrefs.edit()
@@ -3782,6 +3787,8 @@ class CraftBridge(
37823787
37833788 @JavascriptInterface
37843789 fun reloadWidgets() {
3790 if (CraftNative.reloadWidgets(activity, ".WIDGET_UPDATE")) return
3791
37853792 val intent = Intent(".WIDGET_UPDATE")
37863793 intent.setPackage(activity.packageName)
37873794 activity.sendBroadcast(intent)
packages/android/templates/CraftNative.kt.templatemodified+28-0
Changes to packages/android/templates/CraftNative.kt.template
@@ -118,6 +118,8 @@ object CraftNative {
118118 private external fun nativeRemoveSharedItem(activity: Activity, key: String, group: String): Boolean
119119 private external fun nativeGetContacts(activity: Activity): Boolean
120120 private external fun nativeAddContact(activity: Activity, contactJson: String): Boolean
121 private external fun nativeUpdateWidget(activity: Activity, action: String, dataJson: String): Boolean
122 private external fun nativeReloadWidgets(activity: Activity, action: String): Boolean
121123
122124 /**
123125 * `getDeviceInfo`, or null to use the Kotlin implementation.
@@ -483,4 +485,30 @@ object CraftNative {
483485 false
484486 }
485487 }
488
489 /**
490 * The widget pair. `action` is the caller's own broadcast constant.
491 *
492 * Zig could build it from `activity.packageName`, and today that is the
493 * same string — but an `applicationIdSuffix` on a build type would move
494 * the package name and leave the constant where it was, and the widget
495 * would silently stop updating. Only one side gets to decide.
496 */
497 fun updateWidget(activity: Activity, action: String, dataJson: String): Boolean {
498 if (!isAvailable) return false
499 return try {
500 nativeUpdateWidget(activity, action, dataJson)
501 } catch (e: UnsatisfiedLinkError) {
502 false
503 }
504 }
505
506 fun reloadWidgets(activity: Activity, action: String): Boolean {
507 if (!isAvailable) return false
508 return try {
509 nativeReloadWidgets(activity, action)
510 } catch (e: UnsatisfiedLinkError) {
511 false
512 }
513 }
486514}
packages/zig/build.zigmodified+3-0
Changes to packages/zig/build.zig
@@ -467,6 +467,9 @@ pub fn build(b: *std.Build) void {
467467 android_conformance_tests.root_module.addAnonymousImport("src/bridge_android_contacts.zig", .{
468468 .root_source_file = b.path("src/bridge_android_contacts.zig"),
469469 });
470 android_conformance_tests.root_module.addAnonymousImport("src/bridge_android_widgets.zig", .{
471 .root_source_file = b.path("src/bridge_android_widgets.zig"),
472 });
470473 const run_android_conformance_tests = b.addRunArtifact(android_conformance_tests);
471474
472475 // The page surface, across both bridges. Not folded into the iOS
packages/zig/src/android_dispatch.zigmodified+81-1
Changes to packages/zig/src/android_dispatch.zig
@@ -50,6 +50,7 @@ const calendar = ;
5050const db = ;
5151const shareditem = ;
5252const contacts = ;
53const widgets = ;
5354
5455const Jni = jni.Jni;
5556
@@ -292,6 +293,20 @@ const natives = [_]jni.JNINativeMethod{
292293 .signature = "(Landroid/app/Activity;Ljava/lang/String;)Z",
293294 .fnPtr = @ptrCast(&nativeAddContact),
294295 },
296 // The broadcast action is passed across rather than built here: it is a
297 // compile-time constant in the generated app, and deriving it from the
298 // runtime package name would silently stop matching under an
299 // `applicationIdSuffix`.
300 .{
301 .name = "nativeUpdateWidget",
302 .signature = "(Landroid/app/Activity;Ljava/lang/String;Ljava/lang/String;)Z",
303 .fnPtr = @ptrCast(&nativeUpdateWidget),
304 },
305 .{
306 .name = "nativeReloadWidgets",
307 .signature = "(Landroid/app/Activity;Ljava/lang/String;)Z",
308 .fnPtr = @ptrCast(&nativeReloadWidgets),
309 },
295310};
296311
297312/// How binding went. A value rather than a log line, so every path is
@@ -1087,6 +1102,71 @@ fn nativeAddContact(
10871102 return jni.JNI_TRUE;
10881103}
10891104
1105/// `nativeUpdateWidget(activity, action, dataJson)`.
1106fn nativeUpdateWidget(
1107 env: jni.JNIEnv,
1108 _: jni.jobject,
1109 activity: jni.jobject,
1110 action: jni.jstring,
1111 data_json: jni.jstring,
1112) callconv(.c) jni.jboolean {
1113 const j = Jni.init(env);
1114 var arena = std.heap.ArenaAllocator.init(backing);
1115 defer arena.deinit();
1116 const allocator = arena.allocator();
1117
1118 const text = j.stringToUtf8(allocator, data_json) catch return jni.JNI_FALSE;
1119 var parsed = std.json.parseFromSlice(std.json.Value, allocator, text, .{}) catch
1120 return jni.JNI_FALSE;
1121 defer parsed.deinit();
1122
1123 const update = widgets.parseUpdate(parsed.value) orelse return jni.JNI_FALSE;
1124 const action_text = j.stringToUtf8(allocator, action) catch return jni.JNI_FALSE;
1125
1126 widgets.writeUpdate(j, allocator, activity, update) catch |err| {
1127 calendar.rejectOn(allocator, widgets.reject_global, @errorName(err)) catch {};
1128 return jni.JNI_TRUE;
1129 };
1130 widgets.broadcast(j, allocator, activity, action_text) catch |err| {
1131 calendar.rejectOn(allocator, widgets.reject_global, @errorName(err)) catch {};
1132 return jni.JNI_TRUE;
1133 };
1134
1135 events.settle(allocator, widgets.resolve_global, widgets.updated_result) catch
1136 return jni.JNI_FALSE;
1137 return jni.JNI_TRUE;
1138}
1139
1140/// `nativeReloadWidgets(activity, action)`.
1141///
1142/// The shim wraps none of `reloadWidgets` in a try/catch, so a `sendBroadcast`
1143/// that throws escapes the `@JavascriptInterface` method and the promise
1144/// hangs. Rejecting here diverges in the page's favour, and unlike
1145/// `getCalendarEvents` there is nothing to fall through *to* — the shim would
1146/// hang. So this one rejects, and says so rather than reproducing a hang.
1147fn nativeReloadWidgets(
1148 env: jni.JNIEnv,
1149 _: jni.jobject,
1150 activity: jni.jobject,
1151 action: jni.jstring,
1152) callconv(.c) jni.jboolean {
1153 const j = Jni.init(env);
1154 var arena = std.heap.ArenaAllocator.init(backing);
1155 defer arena.deinit();
1156 const allocator = arena.allocator();
1157
1158 const action_text = j.stringToUtf8(allocator, action) catch return jni.JNI_FALSE;
1159
1160 widgets.broadcast(j, allocator, activity, action_text) catch |err| {
1161 calendar.rejectOn(allocator, widgets.reject_global, @errorName(err)) catch {};
1162 return jni.JNI_TRUE;
1163 };
1164
1165 events.settle(allocator, widgets.resolve_global, widgets.reloaded_result) catch
1166 return jni.JNI_FALSE;
1167 return jni.JNI_TRUE;
1168}
1169
10901170// =============================================================================
10911171// Tests
10921172// =============================================================================
@@ -1198,7 +1278,7 @@ test "the registered natives name methods the Kotlin actually declares" {
11981278 // A descriptor is checked by the JVM at registration, so a wrong one fails
11991279 // at load rather than at call — but only if the *name* matches something.
12001280 // These two strings are the contract with CraftBridge.kt.
1201 try testing.expectEqual(@as(usize, 26), natives.len);
1281 try testing.expectEqual(@as(usize, 28), natives.len);
12021282 try testing.expectEqualStrings("nativeGetDeviceInfo", std.mem.span(natives[0].name));
12031283
12041284 // And the class they bind to is the fixed one, not the templated bridge.
packages/zig/src/bridge_android_widgets.zigadded+332-0
Changes to packages/zig/src/bridge_android_widgets.zig
@@ -0,0 +1,332 @@
1//! `updateWidget` and `reloadWidgets` on Android.
2//!
3//! A widget cannot be drawn from here — that is `CraftWidgetProvider`'s job,
4//! in the app's own process, when the system asks. These two actions are the
5//! page's side of that: write what the widget should say into a preferences
6//! file the provider reads, then broadcast so it reads it now rather than at
7//! the next scheduled update.
8//!
9//! ## The broadcast action is passed in rather than built
10//!
11//! The shim broadcasts `".WIDGET_UPDATE"`, and
12//! `CraftWidgetProvider` filters on the same templated constant — both are
13//! compile-time strings in the generated app.
14//!
15//! Zig could build it from `activity.getPackageName()`, and today that is the
16//! same string: the generated `build.gradle.kts` sets
17//! `applicationId = ""` with no suffix. But an app author who
18//! adds `applicationIdSuffix = ".debug"` to a build type moves the runtime
19//! package name and leaves the constant where it was — and then Zig would
20//! broadcast an action no filter matches. Nothing throws, nothing logs, and
21//! the widget silently stops updating in debug builds only.
22//!
23//! So the Kotlin passes its own constant across, the way it passes the
24//! database connection. The two cannot disagree if only one of them decides.
25//!
26//! ## `has` and `getString` are not the same question
27//!
28//! `updateWidget` writes a key only when `data.has(name)`, so a field the page
29//! left out keeps whatever the widget last showed rather than being cleared.
30//! `has` is true for an explicit null, and `getString` then returns the four
31//! characters "null" — so `{"title": null}` sets the widget's title to the
32//! word null rather than clearing it. That is the shim's behaviour and it is
33//! reproduced.
34
35const std = ;
36const jni = ;
37
38const Jni = jni.Jni;
39const jobject = jni.jobject;
40
41pub const A = struct {
42 pub const update_widget = "updateWidget";
43 pub const reload_widgets = "reloadWidgets";
44};
45
46pub const resolve_global = "_craftWidgetResolve";
47pub const reject_global = "_craftWidgetReject";
48
49/// What each action resolves with, exactly as the shim writes it.
50pub const updated_result = "{\"updated\":true}";
51pub const reloaded_result = "{\"reloaded\":true}";
52
53/// `activity.getSharedPreferences("craft_widget_prefs", MODE_PRIVATE)`.
54const prefs_name = "craft_widget_prefs";
55const mode_private: i32 = 0;
56
57/// The four fields, and the preference key each is written to.
58///
59/// The names differ on the two sides — `title` in, `widget_title` out — which
60/// is the sort of mapping that goes wrong silently, so it lives in one table
61/// rather than in four `if` blocks.
62pub const fields = [_]Field{
63 .{ .name = "title", .key = "widget_title" },
64 .{ .name = "subtitle", .key = "widget_subtitle" },
65 .{ .name = "value", .key = "widget_value" },
66 .{ .name = "icon", .key = "widget_icon" },
67};
68
69pub const Field = struct {
70 name: []const u8,
71 key: [:0]const u8,
72};
73
74/// What to write, one slot per `fields` entry; null means the page did not
75/// mention that field and the widget keeps what it had.
76pub const Update = [fields.len]?[]const u8;
77
78/// Read the declared shape, or null where only `org.json` would.
79///
80/// `getString` coerces a number or a nested object into its string form, and
81/// `Double.toString` and `JSONObject.toString` are not things to reproduce
82/// from memory — those payloads go back to the shim. An explicit null is the
83/// one coercion carried over: `has` says the key is there, and `getString`
84/// returns the four characters "null".
85pub fn parseUpdate(value: std.json.Value) ?Update {
86 const object = switch (value) {
87 .object => |o| o,
88 else => return null,
89 };
90
91 var update: Update = @splat(null);
92 inline for (fields, 0..) |field, i| {
93 if (object.get(field.name)) |found| {
94 update[i] = switch (found) {
95 .string => |text| text,
96 .null => "null",
97 else => return null,
98 };
99 }
100 }
101 return update;
102}
103
104/// Write the mentioned fields and apply.
105pub fn writeUpdate(j: Jni, allocator: std.mem.Allocator, activity: jobject, update: Update) !void {
106 try j.pushLocalFrame(16);
107 defer _ = j.popLocalFrame(null);
108
109 const prefs = try j.callObjectMethodA(
110 activity,
111 try j.methodId(
112 try j.objectClass(activity),
113 "getSharedPreferences",
114 "(Ljava/lang/String;I)Landroid/content/SharedPreferences;",
115 ),
116 &.{ .{ .l = try j.newStringUtf(prefs_name) }, .{ .i = mode_private } },
117 );
118
119 const editor = try j.callObjectMethod(
120 prefs,
121 try j.methodId(
122 try j.objectClass(prefs),
123 "edit",
124 "()Landroid/content/SharedPreferences$Editor;",
125 ),
126 );
127 const editor_cls = try j.objectClass(editor);
128 const put_string = try j.methodId(
129 editor_cls,
130 "putString",
131 "(Ljava/lang/String;Ljava/lang/String;)Landroid/content/SharedPreferences$Editor;",
132 );
133
134 inline for (fields, 0..) |field, i| {
135 if (update[i]) |text| {
136 try j.pushLocalFrame(4);
137 defer _ = j.popLocalFrame(null);
138 _ = try j.callObjectMethodA(editor, put_string, &.{
139 .{ .l = try j.newStringUtf(field.key) },
140 .{ .l = try javaString(j, allocator, text) },
141 });
142 }
143 }
144
145 // `apply`, as the shim does — the write lands on a background thread and
146 // the resolve is a promise about having asked.
147 try j.callVoidMethodA(editor, try j.methodId(editor_cls, "apply", "()V"), &.{});
148}
149
150/// `Intent(action).setPackage(activity.packageName)`, then `sendBroadcast`.
151pub fn broadcast(j: Jni, allocator: std.mem.Allocator, activity: jobject, action: []const u8) !void {
152 try j.pushLocalFrame(16);
153 defer _ = j.popLocalFrame(null);
154
155 const intent_cls = try j.findClass("android/content/Intent");
156 const intent = try j.newObjectA(
157 intent_cls,
158 try j.methodId(intent_cls, "<init>", "(Ljava/lang/String;)V"),
159 &.{.{ .l = try javaString(j, allocator, action) }},
160 );
161
162 const activity_cls = try j.objectClass(activity);
163 const package_name = try j.callObjectMethod(
164 activity,
165 try j.methodId(activity_cls, "getPackageName", "()Ljava/lang/String;"),
166 );
167
168 // `setPackage` keeps the broadcast inside this app. It is a delivery
169 // restriction and not the action, which is why the runtime package name is
170 // safe here and would not be safe as the action itself.
171 _ = try j.callObjectMethodA(
172 intent,
173 try j.methodId(intent_cls, "setPackage", "(Ljava/lang/String;)Landroid/content/Intent;"),
174 &.{.{ .l = package_name }},
175 );
176
177 try j.callVoidMethodA(
178 activity,
179 try j.methodId(activity_cls, "sendBroadcast", "(Landroid/content/Intent;)V"),
180 &.{.{ .l = intent }},
181 );
182}
183
184fn javaString(j: Jni, allocator: std.mem.Allocator, text: []const u8) !jni.jstring {
185 const terminated = try allocator.allocSentinel(u8, text.len, 0);
186 defer allocator.free(terminated);
187 @memcpy(terminated, text);
188 return j.newStringUtf(terminated.ptr);
189}
190
191// =============================================================================
192// Tests
193// =============================================================================
194
195const testing = std.testing;
196
197fn parsed(json: []const u8) !?Update {
198 var arena = std.heap.ArenaAllocator.init(testing.allocator);
199 defer arena.deinit();
200
201 var doc = try std.json.parseFromSlice(std.json.Value, arena.allocator(), json, .{});
202 defer doc.deinit();
203
204 const update = parseUpdate(doc.value) orelse return null;
205
206 // Copied out, because the slices borrow from the arena.
207 var copy: Update = @splat(null);
208 for (update, 0..) |slot, i| {
209 if (slot) |text| copy[i] = try testing.allocator.dupe(u8, text);
210 }
211 return copy;
212}
213
214fn freeUpdate(update: Update) void {
215 for (update) |slot| {
216 if (slot) |text| testing.allocator.free(text);
217 }
218}
219
220test "every field maps to its widget_ key" {
221 const update = (try parsed(
222 \\{"title":"Sales","subtitle":"today","value":"42","icon":"chart"}
223 )).?;
224 defer freeUpdate(update);
225
226 try testing.expectEqualStrings("Sales", update[0].?);
227 try testing.expectEqualStrings("today", update[1].?);
228 try testing.expectEqualStrings("42", update[2].?);
229 try testing.expectEqualStrings("chart", update[3].?);
230
231 // The mapping itself, which is what a rewrite would get subtly wrong.
232 try testing.expectEqualStrings("title", fields[0].name);
233 try testing.expectEqualStrings("widget_title", fields[0].key);
234 try testing.expectEqualStrings("subtitle", fields[1].name);
235 try testing.expectEqualStrings("widget_subtitle", fields[1].key);
236 try testing.expectEqualStrings("value", fields[2].name);
237 try testing.expectEqualStrings("widget_value", fields[2].key);
238 try testing.expectEqualStrings("icon", fields[3].name);
239 try testing.expectEqualStrings("widget_icon", fields[3].key);
240}
241
242test "an unmentioned field is left alone rather than cleared" {
243 // `if (data.has("title"))` — a page updating only the value keeps the
244 // title the widget is already showing.
245 const update = (try parsed(
246 \\{"value":"42"}
247 )).?;
248 defer freeUpdate(update);
249
250 try testing.expect(update[0] == null);
251 try testing.expect(update[1] == null);
252 try testing.expectEqualStrings("42", update[2].?);
253 try testing.expect(update[3] == null);
254}
255
256test "an empty string clears the field, where an absent one does not" {
257 // The distinction the null above exists to preserve: "" is a value the
258 // page chose, and it is written.
259 const update = (try parsed(
260 \\{"title":""}
261 )).?;
262 defer freeUpdate(update);
263
264 try testing.expect(update[0] != null);
265 try testing.expectEqualStrings("", update[0].?);
266}
267
268test "an explicit null sets the field to the word null" {
269 // `has` is true for a JSON null and `getString` returns "null", so this
270 // does not clear the title — it sets it to four characters. Reproduced
271 // because `JSON.stringify({title: null})` is an ordinary thing to send.
272 const update = (try parsed(
273 \\{"title":null}
274 )).?;
275 defer freeUpdate(update);
276
277 try testing.expectEqualStrings("null", update[0].?);
278}
279
280test "a shape only org.json would stringify is handed back" {
281 for ([_][]const u8{
282 \\{"title":1.5}
283 ,
284 \\{"value":42}
285 ,
286 \\{"icon":true}
287 ,
288 \\{"title":{"a":1}}
289 ,
290 \\{"title":["a"]}
291 ,
292 \\[]
293 ,
294 \\"Sales"
295 ,
296 }) |payload| {
297 if (try parsed(payload)) |update| {
298 freeUpdate(update);
299 std.debug.print("payload was served rather than handed back: {s}\n", .{payload});
300 return error.CoercedShapeAccepted;
301 }
302 }
303}
304
305test "an empty object writes nothing and still resolves" {
306 const update = (try parsed("{}")).?;
307 defer freeUpdate(update);
308 for (update) |slot| try testing.expect(slot == null);
309}
310
311test "the results are the shim's constants" {
312 try testing.expectEqualStrings("{\"updated\":true}", updated_result);
313 try testing.expectEqualStrings("{\"reloaded\":true}", reloaded_result);
314
315 inline for (.{ updated_result, reloaded_result }) |result| {
316 var doc = try std.json.parseFromSlice(std.json.Value, testing.allocator, result, .{});
317 defer doc.deinit();
318 try testing.expectEqual(@as(usize, 1), doc.value.object.count());
319 }
320}
321
322test "the preferences file is the one CraftWidgetProvider reads" {
323 try testing.expectEqualStrings("craft_widget_prefs", prefs_name);
324 try testing.expectEqual(@as(i32, 0), mode_private);
325}
326
327test "the actions and globals match the shim exactly" {
328 try testing.expectEqualStrings("updateWidget", A.update_widget);
329 try testing.expectEqualStrings("reloadWidgets", A.reload_widgets);
330 try testing.expectEqualStrings("_craftWidgetResolve", resolve_global);
331 try testing.expectEqualStrings("_craftWidgetReject", reject_global);
332}
packages/zig/test/android_conformance_test.zigmodified+5-2
Changes to packages/zig/test/android_conformance_test.zig
@@ -51,6 +51,7 @@ const zig_sources = [_][]const u8{
5151 @embedFile("src/bridge_android_db.zig"),
5252 @embedFile("src/bridge_android_shareditem.zig"),
5353 @embedFile("src/bridge_android_contacts.zig"),
54 @embedFile("src/bridge_android_widgets.zig"),
5455};
5556
5657/// How many spec actions Zig does not serve yet.
@@ -70,8 +71,10 @@ const zig_sources = [_][]const u8{
7071/// getCalendarEvents, the first to send the page a payload it built rather
7172/// than a constant; 84 with createCalendarEvent, the first to read one; 82
7273/// with the database pair, the first to borrow a connection the Kotlin owns;
73/// 79 with the shared-item trio; 77 with the contacts pair.
74const max_not_yet_migrated: usize = 77;
74/// 79 with the shared-item trio; 77 with the contacts pair; 75 with the
75/// widget pair, the first to take a constant across from the Kotlin because
76/// deriving it here would be silently wrong under an applicationIdSuffix.
77const max_not_yet_migrated: usize = 75;
7578
7679/// Every `@JavascriptInterface fun <name>(` in the Kotlin bridge.
7780///