ReviewOS

also looking at this

craft-native/craft

feat(android): the immediate half of scheduleNotification — ratchet 73 → 72

#172
Merged chris wants to merge feat/android-schedule-notification into main
7 files +690 -227
9 days ago

The cancel pair has been served since #151. This is the other end of the same feature, and it arrives with a boundary rather than whole.

Only delay <= 0 is served

A delay above zero posts through Handler(Looper.getMainLooper()).postDelayed { ... }, and Zig cannot make a Runnable — the same wall the reply channel exists to get around, except this one needs a Java object rather than a Java call.

So a delayed notification is handed back to the shim. Where it is handed back matters: the shim creates the channel and builds the notification on both paths, so declining after any of that would do the work twice. This declines before touching the JVM at all.

if (delay > 0) is the shim's branch, so a negative delay is not an error and not a rejection — the notification posts immediately. The test covers minInt as well as -1, because <= 0 and == 0 are the two readings and only one is the shim's.

NotificationCompat, not Notification

The shim builds with androidx's NotificationCompat.Builder, and the two do not produce identical objects — compat writes its own extras. A notification built one way and cancelled the other is a difference nobody would think to look for, so Zig uses the same class.

FindClass reaches androidx because a native method's class loader is the one that declared it, and CraftNative is an app class. If androidx were ever absent, FindClass fails, Zig returns false, and the shim — which could not compile without it — is the one that breaks.

Read rather than written down: android.R.drawable.ic_dialog_info, because a resource id is a number the platform assigns, and PRIORITY_DEFAULT, because it is androidx's own constant and the one number here a version bump could move.

Written down rather than read: the channel's id, name and description. The id has to match CraftBridge.notificationChannelId exactly — a notification posted to a channel that does not exist is dropped silently on API 26+.

The id is hashed by Java, as it is for cancel

notify takes an int and the page sends a string, so the hash comes from the Java object for the reason this file already documents: Java hashes UTF-16 code units, so an id carrying an emoji hashes differently from its UTF-8 bytes.

The page gets the id back, not the hash — that is what it needs to cancel with later.

Also

bridge_android_notifcancel.zig is now bridge_android_notifications.zig. A file named for cancelling that also schedules is the kind of name someone trusts and should not.

Testing

zig build test:android passes with the ratchet at 72. Both mutations fired before this was committed: narrowing delay <= 0 to == 0, and accepting a coerced integer id, each fail a named test. Both libcraft.so targets still build with no NDK.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.