diff --git a/AGENTS.md b/AGENTS.md index 0efb9af..8953852 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -175,6 +175,14 @@ These are the things we've burned ourselves on. Following them isn't optional. relevant repo, every time. Both are clean across the codebase today; don't regress them. + **And the native formatters, if you touched native source.** CI runs + `xcrun clang-format --dry-run -Werror ios/mob_nif.m android/jni/mob_beam.h` + and swiftlint, and `mix format` says nothing about either. Adding one line + to an aligned C initialiser is enough to fail it, because clang-format + re-flows the whole block around the new entry — which is how this note came + to be written. Run `xcrun clang-format -i ` on any `.m`/`.h` you + edited before committing. + 6. **Multi-repo changes batch together.** A user-visible fix in mob often needs matching changes in mob_dev (build) and mob_new (template). Bumping versions without coordination produces ghost regressions. Check all three before diff --git a/CHANGELOG.md b/CHANGELOG.md index a6a1834..876ee9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,31 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). `clear_text/1` to the IOHID injection path they do not use. ### Added +- **Native frame timing on Android.** `Mob.RenderStats.native_enable/1`, + `native_frames/1` and `native_summary/1` returned `{:error, :unsupported}` on + Android, which looks identical to "you forgot to enable it". They now work, + emitting the same JSON shape iOS does so nothing needs per-platform parsing. + + First baseline, physical moto g power, 1600-node screen: a one-field + re-render costs 266ms p50, a `push` 841ms and a `pop` 916ms — navigation is + **3.2x** a re-render of the same tree. See + `decisions/2026-09-05-measure-the-native-half-on-android.md`, which also + records why the obvious closing brackets (`MessageQueue.IdleHandler`, a + plain `post`) measure the wrong thing without failing. + + Android's `apply_us` includes a thread handoff and up to one vsync of queue + latency that iOS's does not; it is a before-and-after tool for one platform, + not a cross-platform comparison. **Requires an app generated by `mob_new` + 0.4.32 or newer** — the buffer lives in the app's own `MobBridge.kt`. + +### Fixed +- `Mob.RenderStats.native_*` now report `{:error, :unsupported}` when the + running app's bridge lacks the method, instead of leaking + `{:error, :not_loaded}` past a `@spec` promising two shapes. Android signals + a missing bridge method by returning rather than raising, so it took the one + path that was not converted — which every app generated before this release + hits. + - **`Mob.Test.capabilities/1`** — ask a build which test-harness probes it can actually serve, before choosing how to drive it. Which probes work is a runtime fact: on Android each harness NIF bails when the app's generated diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index f12fb64..3d97f1b 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -287,6 +287,8 @@ pub const BridgeMethods = extern struct { type_text: jni.JMethodID = null, delete_backward: jni.JMethodID = null, clear_text: jni.JMethodID = null, + render_stats: jni.JMethodID = null, + render_stats_enable: jni.JMethodID = null, long_press_xy: jni.JMethodID = null, swipe_xy: jni.JMethodID = null, screenshot: jni.JMethodID = null, @@ -675,6 +677,7 @@ export fn nif_capabilities( erts.atom(env, "ax_action"), erts.atom(env, "element_frames"), erts.atom(env, "scroll_info"), erts.atom(env, "scroll_to"), erts.atom(env, "sample_region"), erts.atom(env, "screenshot"), + erts.atom(env, "native_stats"), }; const vals = [_]erts.ERL_NIF_TERM{ boolAtom(env, Bridge.ui_view_tree != null), @@ -702,6 +705,10 @@ export fn nif_capabilities( // See decisions/2026-08-10-sample-region-crops-natively-and-stays-debug-only.md boolAtom(env, false), boolAtom(env, Bridge.screenshot != null), + // Both halves come from the same generated bridge, so one flag answers + // for the pair; renderStatsEnable without renderStats would be an app + // that half-applied a template update. + boolAtom(env, Bridge.render_stats != null), }; return erts.makeMap(env, &keys, &vals) orelse erts.atom(env, "error"); } @@ -4241,11 +4248,84 @@ fn nifLoad(env: ?*erts.ErlNifEnv, priv: *?*anyopaque, info: erts.ERL_NIF_TERM) c cacheOptional(jenv, "clearText", "()Z", &Bridge.clear_text); cacheOptional(jenv, "longPressXy", "(FFJ)Z", &Bridge.long_press_xy); cacheOptional(jenv, "swipeXy", "(FFFF)Z", &Bridge.swipe_xy); + cacheOptional(jenv, "renderStats", "()Ljava/lang/String;", &Bridge.render_stats); + cacheOptional(jenv, "renderStatsEnable", "(Z)Z", &Bridge.render_stats_enable); logi_nif("Mob NIF loaded (Compose backend)", .{}); return 0; } +// nif_native_stats/0 — JSON of the recorded native frame samples, newest +// first, matching the shape iOS emits so `Mob.RenderStats` needs no +// per-platform parsing: +// +// {"enabled":bool,"recorded":N,"dropped":M, +// "samples":[{"apply_us":f,"transition":s,"seq":n},...]} +// +// The ring buffer lives in Kotlin rather than here, unlike iOS where it sits in +// C beside the NIF. On Android the measurement can only be taken on the main +// thread — it brackets a Compose frame — so keeping the buffer next to the +// thing that writes it avoids a JNI hop per sample on the hot path. This +// mirrors elementFrames, which is built in Kotlin for the same reason. +export fn nif_native_stats( + env: ?*erts.ErlNifEnv, + argc: c_int, + argv: [*]const erts.ERL_NIF_TERM, +) callconv(.c) erts.ERL_NIF_TERM { + _ = argc; + _ = argv; + if (Bridge.render_stats == null) return notLoaded(env); + var attached: c_int = 0; + const jenv = get_jenv(&attached) orelse return erts.atom(env, "error"); + const jresult = jenv.*.CallStaticObjectMethod.?(jenv, Bridge.cls, Bridge.render_stats); + const result = jstringToBin(env, jenv, jresult); + detachIfAttached(attached); + return result; +} + +// nif_native_stats_enable/1 — turn native frame timing on or off. +// +// Off by default: an enabled measurement arms an idle handler per set_root and +// takes two timestamps, which is not free on a screen that re-renders steadily. +// Enabling also clears the buffer, so a caller measures the run it just started +// rather than whatever was left over from the last one. +export fn nif_native_stats_enable( + env: ?*erts.ErlNifEnv, + argc: c_int, + argv: [*]const erts.ERL_NIF_TERM, +) callconv(.c) erts.ERL_NIF_TERM { + _ = argc; + if (Bridge.render_stats_enable == null) return notLoaded(env); + + var on_buf: [8]u8 = @splat(0); + if (erts.enif_get_atom(env, argv[0], &on_buf, on_buf.len, erts.ERL_NIF_LATIN1) == 0) + return erts.badarg(env); + + // Exact compare against the NUL-terminated atom, not a prefix: `on_buf` is + // 8 bytes, so a prefix test on the first four accepts `:truthy` and + // `:true_x` as "on" — the very typo-enables-it case this refuses. Anything + // that is neither `true` nor `false` is a caller error rather than a + // silent no-op, matching what iOS does. + const on_atom = std.mem.sliceTo(&on_buf, 0); + const on: bool = if (std.mem.eql(u8, on_atom, "true")) + true + else if (std.mem.eql(u8, on_atom, "false")) + false + else + return erts.badarg(env); + + var attached: c_int = 0; + const jenv = get_jenv(&attached) orelse return erts.atom(env, "error"); + _ = jenv.*.CallStaticBooleanMethod.?( + jenv, + Bridge.cls, + Bridge.render_stats_enable, + @as(jni.JBoolean, if (on) 1 else 0), + ); + detachIfAttached(attached); + return erts.ok(env); +} + // ── NIF table + ERL_NIF_INIT entry point ───────────────────────────────── // Replaces the static `ErlNifFunc nif_funcs[]` + `ERL_NIF_INIT` macro // that used to live at the bottom of mob_nif.c. The entry point is the @@ -4272,6 +4352,8 @@ const nif_funcs = [_]erts.ErlNifFunc{ .{ .name = "clear_text", .arity = 0, .fptr = nif_clear_text, .flags = erts.ERL_NIF_DIRTY_JOB_IO_BOUND }, .{ .name = "long_press_xy", .arity = 3, .fptr = nif_long_press_xy, .flags = erts.ERL_NIF_DIRTY_JOB_IO_BOUND }, .{ .name = "swipe_xy", .arity = 4, .fptr = nif_swipe_xy, .flags = erts.ERL_NIF_DIRTY_JOB_IO_BOUND }, + .{ .name = "native_stats", .arity = 0, .fptr = nif_native_stats, .flags = erts.ERL_NIF_DIRTY_JOB_CPU_BOUND }, + .{ .name = "native_stats_enable", .arity = 1, .fptr = nif_native_stats_enable, .flags = 0 }, .{ .name = "screenshot", .arity = 3, .fptr = nif_screenshot, .flags = erts.ERL_NIF_DIRTY_JOB_CPU_BOUND }, .{ .name = "scroll_info", .arity = 1, .fptr = nif_scroll_info, .flags = 0 }, .{ .name = "scroll_to", .arity = 3, .fptr = nif_scroll_to, .flags = 0 }, diff --git a/decisions/2026-09-05-measure-the-native-half-on-android.md b/decisions/2026-09-05-measure-the-native-half-on-android.md new file mode 100644 index 0000000..4bfaecb --- /dev/null +++ b/decisions/2026-09-05-measure-the-native-half-on-android.md @@ -0,0 +1,92 @@ +# Measuring the native half of a frame on Android + +Date: 2026-09-05 +Status: accepted +Ticket: MOB-146 + +## Context + +`decisions/2026-09-03-measure-the-native-half-of-a-frame.md` established the +iOS side: `Mob.RenderStats` can time the BEAM half of a render but not the +native half, and on a dense screen the native half is most of the cost. +Android had none of it — `native_summary/1` returned `{:error, :unsupported}` +there, which is indistinguishable from "you forgot to enable it". + +That made MOB-146 unarguable in the direction it needed to be argued. The +claim is that Android navigation disposes and recreates the composition, the +same defect MOB-129 fixed on iOS. Without a number, a fix for it lands with no +before and no after. + +## Decision + +### The ring buffer lives in Kotlin, not in the NIF + +iOS keeps it in C beside the NIF. On Android the measurement can only be taken +on the main thread, so keeping the buffer next to the writer avoids a JNI hop +per sample on a hot path. `nif_native_stats` fetches the serialised JSON the +way `nif_element_frames` fetches frames, and both Kotlin methods are looked up +with `cacheOptional` so an app generated before they existed still loads. + +### The closing bracket rides the frame + +This is the part that fails silently with a plausible number, so the rejected +options are worth recording. + +`MessageQueue.IdleHandler` is the literal analogue of the +`CFRunLoopObserver(.beforeWaiting)` iOS uses, and it is wrong here. Compose +requests its frame through `Choreographer`, and a vsync callback arrives +asynchronously rather than sitting in the queue. Between the request and the +vsync the queue is genuinely empty, so the handler fires there — before any of +the work being measured — and reports the cost of a field write. + +Posting a plain `Runnable` and registering the frame callback from inside it +fails differently and less visibly. `ViewRootImpl.scheduleTraversals` installs +a sync barrier that blocks non-async messages until `doTraversal` runs. With a +traversal already pending, which is the steady state on exactly the busy +screens being measured, that post is held while Compose recomposes at frame V, +so it registers for V+1 and the sample absorbs a whole extra frame. Upward +bias, bimodal, worst under load. Choreographer's own vsync messages are +asynchronous and sail past the barrier, which is why registering directly from +the calling thread does not have the problem. + +So the frame callback is registered straight from the NIF thread against a +`Choreographer` captured on the main thread at init. It fires at the start of +the next frame; a message posted from inside it cannot run until the traversal +has measured, laid out and drawn, because that traversal is synchronous. + +### Android's `apply_us` is not iOS's `apply_us` + +Recorded plainly because a differential test (MOB-157) would otherwise compare +them and conclude something false. + +iOS starts its clock on the main thread, after the dispatch hop, with the node +already parsed. Android starts on the NIF thread, after `MobJson.parseNode` +but before the state write, and the interval therefore includes a thread +handoff and up to one vsync of queue latency that iOS's does not. + +The parse is deliberately excluded on both. On Android `setRootJson` runs +synchronously from the NIF, so the BEAM-side `set_root_us` already spans the +parse; measuring it here too would double-count it against anyone adding the +two windows. + +What this measurement is for is a before-and-after on one platform, taken the +same way on both sides. It is not an absolute native frame cost, and it is not +comparable across platforms. + +## Consequences + +- First Android navigation baseline, physical moto g power, 1600-node screen: + `none` p50 266ms, `push` p50 841ms, `pop` p50 916ms. Navigation costs 3.2x a + re-render of the same tree, which is the gap MOB-146 exists to close. +- The instrument was corroborated before its numbers were believed: in the + same window the platform logged `Davey! duration=1414ms` and + `Choreographer: Skipped 67 frames` (~1120ms). The measured figures sit just + below the platform's, which is the right direction, since the bracket closes + after the traversal but before GPU swap. +- A burst of `setRootJson` calls arms several brackets that all close on the + same frame, so each attributes the cost of rendering the last tree to trees + that were superseded. Over-counting rather than losing samples, and the same + behaviour iOS has. +- `native_disable/0` keeps the window readable, matching iOS. Clearing on + disable would empty the buffer the caller is about to read, and the result + would look exactly like the feature being off. diff --git a/guides/agentic_coding.md b/guides/agentic_coding.md index 1a6db37..f696a89 100644 --- a/guides/agentic_coding.md +++ b/guides/agentic_coding.md @@ -192,15 +192,23 @@ Mob.Test.capabilities(node) #=> swipe_xy: false, type_text: false, delete_backward: false, #=> clear_text: false, ax_action: false, element_frames: true, #=> scroll_info: true, scroll_to: true, sample_region: false, -#=> screenshot: true +#=> screenshot: true, native_stats: false #=> } ``` -That is a **freshly generated Android app** — abridged only in layout, not in -content; every one of the seventeen keys is shown, because guessing at the rest -is exactly what goes wrong. The template defines `screenInfo`, `elementFrames`, -`screenshot`, `scrollInfo` and `scrollTo`, and nothing else in the harness set -(MOB-160). +That is an Android app generated **before `mob_new` 0.4.32** — abridged only in +layout, not in content; every one of the eighteen keys is shown, because +guessing at the rest is exactly what goes wrong. That template defined +`screenInfo`, `elementFrames`, `screenshot`, `scrollInfo` and `scrollTo`, and +nothing else in the harness set. + +Regenerating against 0.4.32 or newer flips `tap_xy`, `long_press_xy`, +`swipe_xy`, `type_text` and `delete_backward` to `true` (MOB-160) and +`native_stats` to `true` (MOB-146). `clear_text` stays `false` there on +purpose — two implementations of it reported success while clearing nothing, +so the bridge ships without one rather than lie. Which is the point of asking +the build instead of reading a table: this paragraph is already a snapshot of +two releases, and `capabilities/1` is not. Two of those `false`s bite harder than they look: diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 9934f95..17cb188 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -7096,12 +7096,25 @@ static ERL_NIF_TERM nif_capabilities(ErlNifEnv *env, int argc, const ERL_NIF_TER const char *name; int on; } caps[] = { - {"view_tree", harness}, {"ui_tree", harness}, {"screen_info", harness}, - {"tap_xy", harness}, {"tap_by_label", harness}, {"long_press_xy", harness}, - {"swipe_xy", harness}, {"type_text", harness}, {"delete_backward", harness}, - {"clear_text", harness}, {"ax_action", harness}, {"element_frames", harness}, - {"scroll_info", harness}, {"scroll_to", harness}, {"sample_region", harness}, + {"view_tree", harness}, + {"ui_tree", harness}, + {"screen_info", harness}, + {"tap_xy", harness}, + {"tap_by_label", harness}, + {"long_press_xy", harness}, + {"swipe_xy", harness}, + {"type_text", harness}, + {"delete_backward", harness}, + {"clear_text", harness}, + {"ax_action", harness}, + {"element_frames", harness}, + {"scroll_info", harness}, + {"scroll_to", harness}, + {"sample_region", harness}, {"screenshot", screenshot}, + // Frame timing is part of the harness, so it lives or dies with it — + // compiled out of release builds along with everything else here. + {"native_stats", harness}, }; for (size_t i = 0; i < sizeof(caps) / sizeof(caps[0]); i++) { diff --git a/lib/mob/render_stats.ex b/lib/mob/render_stats.ex index e733d08..715cc3b 100644 --- a/lib/mob/render_stats.ex +++ b/lib/mob/render_stats.ex @@ -564,11 +564,13 @@ defmodule Mob.RenderStats do `set_root`; the timestamps and the run loop observer are downstream of that check. - Returns `{:error, :unsupported}` in three cases, all of which look identical + Returns `{:error, :unsupported}` in four cases, all of which look identical to a caller: on the host, where there is no native side at all; on a platform - whose native half has not implemented it, which today means Android; and in - an **iOS release build**, because the reading NIFs sit inside the same - `MOB_RELEASE` guard as the rest of the test harness. Profile a debug build. + whose native half has not implemented it; in an **iOS release build**, + because the reading NIFs sit inside the same `MOB_RELEASE` guard as the rest + of the test harness (profile a debug build); and on an Android app whose + generated `MobBridge.kt` predates the frame-timing methods — `MobBridge.kt` + is generated once and never re-rendered, so regenerate the app. """ @spec native_enable(module()) :: :ok | {:error, :unsupported} def native_enable(nif \\ :mob_nif), do: native_call(nif, :native_stats_enable, [true]) @@ -655,7 +657,17 @@ defmodule Mob.RenderStats do # whose native half lacks this function). Both mean the same thing to a # caller, and neither should take down whatever is reading stats. defp native_call(nif, fun, args) do - apply(nif, fun, args) + case apply(nif, fun, args) do + # Android reports a missing bridge method by RETURNING this rather than + # raising: the NIF is present in the loaded library, so nothing raises, + # but the Kotlin half is absent because `MobBridge.kt` is generated once + # and never re-rendered. Every app generated before the frame-timing + # methods existed reaches here. Same meaning as the raised cases below — + # this build cannot serve the call — so it gets the same answer, instead + # of leaking a fourth shape past a @spec that promises two. + {:error, :not_loaded} -> {:error, :unsupported} + other -> other + end rescue e in UndefinedFunctionError -> # Only this module's own absence. A different UndefinedFunctionError diff --git a/lib/mob/test.ex b/lib/mob/test.ex index 13c3987..c0d8b35 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -538,7 +538,8 @@ defmodule Mob.Test do :scroll_info, :scroll_to, :sample_region, - :screenshot + :screenshot, + :native_stats ] @doc false diff --git a/test/mob/android_native_stats_test.exs b/test/mob/android_native_stats_test.exs new file mode 100644 index 0000000..30ca001 --- /dev/null +++ b/test/mob/android_native_stats_test.exs @@ -0,0 +1,80 @@ +# Source-contract test: the Android NIF table and its capability map are native +# structures Elixir cannot execute. Guards MOB-146. +# credo:disable-for-this-file Jump.CredoChecks.VacuousTest +defmodule Mob.AndroidNativeStatsTest do + use ExUnit.Case, async: true + + @root Path.expand("../..", __DIR__) + + setup_all do + {:ok, zig: File.read!(Path.join(@root, "android/jni/mob_nif.zig"))} + end + + test "native frame timing is registered as a NIF", %{zig: zig} do + # Without these Mob.RenderStats.native_summary/1 returns {:error, :unsupported} + # on Android, which is what it did before this ticket — and it looks + # identical to "the feature is off", which is why it went unnoticed. + assert zig =~ ~s(.name = "native_stats", .arity = 0) + assert zig =~ ~s(.name = "native_stats_enable", .arity = 1) + end + + test "the Kotlin methods behind them are looked up optionally", %{zig: zig} do + # cacheOptional, not cacheRequired: an app generated before this ticket has + # no renderStats method, and must keep loading rather than failing at boot + # with every other NIF. + assert zig =~ ~s|cacheOptional(jenv, "renderStats", "()Ljava/lang/String;"| + assert zig =~ ~s|cacheOptional(jenv, "renderStatsEnable", "(Z)Z"| + end + + test "capability keys and values stay positionally aligned", %{zig: zig} do + # These are two parallel arrays zipped into a map by position. Getting them + # out of step does not fail to compile — the map then reports one NIF's + # availability under another's name. + # + # Counting is not enough to catch that: inserting a key mid-array while + # appending its value at the end keeps the counts equal and produces + # exactly the misordering. So this pairs them up and checks the pairing. + body = capabilities_body(zig) + keys = Regex.scan(~r/erts\.atom\(env, "(\w+)"\)/, section(body, "keys")) + vals = Regex.scan(~r/boolAtom\(env, ([^)]*)\)/, section(body, "vals")) + + assert length(keys) == length(vals), + "capabilities has #{length(keys)} keys and #{length(vals)} values" + + pairs = + Enum.zip( + Enum.map(keys, fn [_, name] -> name end), + Enum.map(vals, fn [_, expr] -> String.trim(expr) end) + ) + + # Every key whose value names a Bridge handle must name the handle for + # THAT key. `native_stats` is served by `render_stats`, and the two + # deliberate `false` literals (ax_action, sample_region) are exempt because + # no bridge method backs them at all. + aliases = %{"native_stats" => "render_stats", "view_tree" => "ui_view_tree"} + + for {key, expr} <- pairs, String.starts_with?(expr, "Bridge.") do + expected = Map.get(aliases, key, key) + + assert expr == "Bridge.#{expected} != null", + "capability #{inspect(key)} reports #{expr}, which belongs to a " <> + "different NIF — the arrays are out of step from here on" + end + + assert {"native_stats", "Bridge.render_stats != null"} in pairs, + "native_stats is registered but not reported by capabilities/1, so " <> + "an agent cannot discover it without calling it and catching the error" + end + + defp capabilities_body(zig) do + [_, body] = String.split(zig, "export fn nif_capabilities", parts: 2) + [body | _] = String.split(body, "\nexport fn ", parts: 2) + body + end + + defp section(body, name) do + [_, rest] = String.split(body, "const #{name} = [_]erts.ERL_NIF_TERM{", parts: 2) + [section | _] = String.split(rest, " };", parts: 2) + section + end +end diff --git a/test/mob/native_frame_stats_test.exs b/test/mob/native_frame_stats_test.exs index e9ab440..23e6c04 100644 --- a/test/mob/native_frame_stats_test.exs +++ b/test/mob/native_frame_stats_test.exs @@ -48,8 +48,8 @@ defmodule Mob.NativeFrameStatsTest do describe "graceful degradation" do test "a native half that has not implemented it reports :unsupported" do - # Android has no native_stats yet, so :mob_nif keeps the Erlang stub and - # it raises. Reading stats must not take down whatever is reading them. + # A platform whose native half lacks the function keeps the Erlang stub, + # and it raises. Reading stats must not take down whatever is reading them. assert {:error, :unsupported} = RenderStats.native_enable(NotLoadedNif) assert {:error, :unsupported} = RenderStats.native_disable(NotLoadedNif) assert {:error, :unsupported} = RenderStats.native_frames(NotLoadedNif) @@ -342,4 +342,104 @@ defmodule Mob.NativeFrameStatsTest do :nomatch -> flunk("expected to find #{inspect(needle)}") end end + + describe "a bridge method the app does not have" do + # Android reports a missing bridge method by RETURNING {:error, :not_loaded} + # rather than raising: the NIF is present in the loaded library, so nothing + # raises, but the Kotlin half is absent because MobBridge.kt is generated + # once and never re-rendered. Every app generated before the frame-timing + # methods existed takes this path — which is to say, the common case. + # + # It went untested when it was written, and the omission was invisible: + # deleting the conversion in native_call/3 left the entire suite green, + # because every other stub here either raises or returns valid JSON. + defmodule ReturnsNotLoadedNif do + @moduledoc false + def native_stats, do: {:error, :not_loaded} + def native_stats_enable(_on), do: {:error, :not_loaded} + end + + test "reports :unsupported, not the raw :not_loaded" do + # The @spec promises :ok | {:error, :unsupported}. Leaking a third shape + # breaks every caller matching on :unsupported, and it looks like a + # working NIF returning an error rather than a build that cannot serve + # the call at all. + assert {:error, :unsupported} = RenderStats.native_enable(ReturnsNotLoadedNif) + assert {:error, :unsupported} = RenderStats.native_disable(ReturnsNotLoadedNif) + assert {:error, :unsupported} = RenderStats.native_frames(ReturnsNotLoadedNif) + assert {:error, :unsupported} = RenderStats.native_summary(ReturnsNotLoadedNif) + end + end + + describe "Android-shaped payloads" do + # Android builds this JSON by hand in Kotlin (a StringBuilder in + # MobBridge.kt) rather than through a serialiser the way iOS does with + # NSJSONSerialization. Hand-built JSON is exactly the kind that is + # well-formed for the values you tried and malformed for the one you did + # not, so the shape it actually emits is pinned here. + + defmodule AndroidNif do + @moduledoc false + # A wrapped ring: 250 frames recorded, 240 retained, 10 scrolled off. + def native_stats do + samples = + for seq <- 249..10//-1 do + ~s({"apply_us":#{seq}.5,"transition":"push","seq":#{seq}}) + end + + ~s({"enabled":true,"recorded":250,"dropped":10,"samples":[) <> + Enum.join(samples, ",") <> "]}" + end + + def native_stats_enable(_on), do: :ok + end + + defmodule QuotedTransitionNif do + @moduledoc false + # `transition` reaches the buffer from nif_set_transition, which accepts + # any atom up to 15 characters verbatim. An unescaped quote here costs + # the reader the whole window, not the one sample. + def native_stats do + ~s({"enabled":true,"recorded":1,"dropped":0,) <> + ~s("samples":[{"apply_us":1.0,"transition":"a\\"b","seq":0}]}) + end + + def native_stats_enable(_on), do: :ok + end + + test "a wrapped ring reports retained, recorded and dropped consistently" do + summary = RenderStats.native_summary(AndroidNif) + + assert summary.samples == 240 + assert summary.recorded == 250 + assert summary.dropped == 10 + + # The identity that makes `dropped` meaningful: what you can still read, + # plus what scrolled away, is everything that happened. + assert summary.samples + summary.dropped == summary.recorded + end + + test "an escaped quote in a transition still parses" do + summary = RenderStats.native_summary(QuotedTransitionNif) + + assert summary.samples == 1 + assert Map.has_key?(summary.apply_us, ~s(a"b)) + end + + test "an UNescaped quote loses the whole window, not one sample" do + # Pinning the consequence, so the escaping in MobBridge.kt is understood + # as load-bearing rather than defensive tidiness. + defmodule BrokenNif do + @moduledoc false + def native_stats do + ~s({"enabled":true,"recorded":1,"dropped":0,) <> + ~s("samples":[{"apply_us":1.0,"transition":"a"b","seq":0}]}) + end + + def native_stats_enable(_on), do: :ok + end + + assert {:error, _} = RenderStats.native_frames(BrokenNif) + end + end end