ReviewOS

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
9 days ago

The second listener shim, and the first migration where the state itself moved rather than being worked around.

Why all three, or none

getAppState returns currentAppState, a field only the lifecycle observer writes. Serving the observer from Zig without serving the getter would leave the Kotlin field frozen at "active" while the real state moved on — the exact failure the flashlight deferral describes, where one half writes a field the other half reads.

So the state moves, and all three actions come with it. That deferral row says migrating the flashlight pair "would need the state to move too"; this is what it looks like when it can.

Process-global, because ProcessLifecycleOwner is — one answer per process, not per Activity. Atomic, because the observer runs on the main looper and getAppState is called from the WebView's JavaBridge thread.

Only a change is announced

The observer compares before announcing, and that check is load-bearing: ON_START immediately after ON_RESUME is an ordinary sequence and both map to "active", so without it a page would see two craftAppStateChange events for one foregrounding. The mutation that removes the comparison fails two tests.

Two deliveries, not one

A change calls window._craftAppStateCallback(state) and dispatches craftAppStateChange. Both, in that order, because the shim does both — onAppStateChange reads the first and addEventListener the second, and a page may use either.

No leak to reproduce this time

LifecycleRegistry keys its observers by identity, so adding the same one twice is a no-op. The holder keeps one instance and adds it, exactly as the shim does.

That is worth contrasting with #183: there the shim's second startNetworkMonitoring orphans the first callback, and Zig had to reproduce the leak (#182) because guarding on one path only would have changed the event count. Here there is nothing to reproduce, and the reason is a property of LifecycleRegistry rather than of either implementation.

The event arrives by name

event.name rather than an ordinal — an ordinal is a number that means nothing on its own and would move if Lifecycle.Event ever gained a member. Everything the shim's when does not match falls to else -> currentAppState, which is a null here and announces nothing; ON_CREATE, ON_DESTROY and ON_ANY all take that path and have tests.

Testing

zig build test:android passes with the ratchet at 56. Two mutations fired: removing the changed-check, and pointing ON_PAUSE at background. Both libcraft.so targets still build with no NDK.

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

Sign in to comment on this pull request.