From 30d8b91ef3d98448d1ed500190d7228b4b41fc47 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 13:51:53 -0600 Subject: [PATCH 1/2] =?UTF-8?q?MOB-110:=20sender=20process=20=E2=80=94=20s?= =?UTF-8?q?erialise=20render=20and=20the=20tap-table=20commit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering happened inline in whichever process handled the message. That is safe only because there is exactly one screen process; MOB-112 makes screens processes and two of them can then render at once. The native contract does not tolerate that. clear_taps prepares the inactive tap table and resets tap_build_count, register_tap appends at that cursor, and set_root swaps atomically. The double buffering exists so a concurrent *reader* — a drag arriving mid-render — resolves against the last committed table; it does nothing for concurrent *writers*, because there is one global build cursor. Two renders in flight interleave into the same building table and one screen's tree is never committed at all. Mob.Sender is now the only caller of the render NIFs. Screens still build their own tree — Composite/List/Component expansion all take self() — and hand the finished tree over. Mob.Renderer uses self() nowhere outside doc examples, so the sender can own the whole render/4 call. Queueing rather than executing in the caller buys coalescing: for one screen only the newest tree is committed, and a tree for a screen that is not active is dropped. That second part is what will let an inactive tab hold state without rendering. The first cut of sync/1 was wrong and the tests caught it. It relied on the render's self-sent :flush being queued ahead of a later sync call, but a self-send appends to the BACK of the mailbox — behind a sync the caller had already queued — so sync returned before the frame was committed. sync/1 now performs the flush itself, which also strengthens coalescing: a burst of renders followed by one sync produces a single commit. The barrier is on the call paths only. Mob.Test documents tap/2 and back/1 as fire-and-forget and its navigation helpers as synchronous, so Mob.Screen syncs in the handle_call paths and leaves handle_info asynchronous — which is what leaves anything to coalesce. commit/1 rescues: every screen renders through this process, so dying on a malformed tree would freeze the whole UI rather than one screen. No .m or .zig change — this is entirely about who calls the existing NIFs. Rationale in decisions/2026-08-28-sender-serialises-render.md; the never-call-render-NIFs-directly invariant added to AGENTS.md's pre-empt-failure rules, since it is easy to break by accident. Tests: 10 new. Suite 1171 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 32 ++- .../2026-08-28-sender-serialises-render.md | 93 +++++++++ lib/mob/app.ex | 8 + lib/mob/nav.ex | 11 + lib/mob/screen.ex | 57 ++++-- lib/mob/sender.ex | 155 ++++++++++++++ test/mob/sender_test.exs | 189 ++++++++++++++++++ 7 files changed, 516 insertions(+), 29 deletions(-) create mode 100644 decisions/2026-08-28-sender-serialises-render.md create mode 100644 lib/mob/sender.ex create mode 100644 test/mob/sender_test.exs diff --git a/AGENTS.md b/AGENTS.md index dd86082b..5e35c06f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,44 +133,54 @@ These are the things we've burned ourselves on. Following them isn't optional. the app sits on the "Starting BEAM…" splash forever. The on_start callback should `{:ok, _} = Mob.Screen.start_root(...)` so failures crash loudly. -3. **TDD discipline in mob_dev.** Every new public function gets a test. +3. **Never call the render NIFs outside `Mob.Sender`.** `clear_taps`, + `register_tap`, `set_transition`, and `set_root` are one build-then-commit + sequence sharing a single global build cursor in the native tap tables + (`ios/mob_nif.m`, `android/jni/mob_nif.zig`). The double buffering there + protects concurrent *readers* — a drag event mid-render — and does nothing + for concurrent *writers*: two renders in flight interleave their handles into + the same building table and one screen's tree is never committed. Screens + build a tree and hand it to `Mob.Sender.render/5`; the sender is the only + caller. See `decisions/2026-08-28-sender-serialises-render.md`. + +4. **TDD discipline in mob_dev.** Every new public function gets a test. `mob_dev/CLAUDE.md` makes this explicit. Don't bypass — the tests are how we catch the multi-step regressions like the iOS-device deploy chain. -4. **Format + credo before commit.** `mix format && mix credo --strict` from the +5. **Format + credo before commit.** `mix format && mix credo --strict` from the relevant repo, every time. Both are clean across the codebase today; don't regress them. -5. **Multi-repo changes batch together.** A user-visible fix in mob often needs +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 declaring done. -6. **iOS device sandbox blocks `fork()`.** The BEAM's `forker_start` and EPMD's +7. **iOS device sandbox blocks `fork()`.** The BEAM's `forker_start` and EPMD's `run_daemon` both call fork; both are patched in our OTP cross-compile. Patches at `mob_dev/scripts/release/patches/`. Don't undo them. -7. **iOS sim and iOS device are different build paths.** Sim → `ios/build.sh` +8. **iOS sim and iOS device are different build paths.** Sim → `ios/build.sh` (`build_ios/1` in NativeBuild). Device → `ios/build_device.sh` (`build_ios_physical/2`). When `--device ` is passed, mob_dev resolves it via `IOS.list_devices/0` to know which path to take. Don't shortcut. -8. **LV port 4200 is global per device.** Two installed Mob LV apps + one +9. **LV port 4200 is global per device.** Two installed Mob LV apps + one running = the second can't bind. Workaround for now: force-stop the squatter. Real fix tracked in `issues.md` #4 (hash bundle id into port). -9. **Compile-time `~r//` literals are unsafe on OTP 28.** They bake a +10. **Compile-time `~r//` literals are unsafe on OTP 28.** They bake a `:re_exported_pattern` and call `:re.import/1` at runtime; OTP 28.0 removed that function. Use `Regex.compile!("...", "flags")` to compile at runtime. 71 literals across mob_dev were swept in 0.3.17. -10. **`:mob_nif.log/1` for early startup logging, `Logger` after Mob.App.start.** +11. **`:mob_nif.log/1` for early startup logging, `Logger` after Mob.App.start.** `Mob.NativeLogger.install()` runs as part of `Mob.App.start` and reroutes `Logger` to NSLog/logcat. Before that point (steps 1–4 in the Erlang bootstrap), `Logger` output goes to stderr and is invisible. Use `:mob_nif.log("message")` for diagnostics during early init. -11. **NIFs on Android must be statically linked, not `dlopen`'d.** Android's +12. **NIFs on Android must be statically linked, not `dlopen`'d.** Android's `System.loadLibrary` loads native libs `RTLD_LOCAL` by default — the parent's `enif_*` symbols are invisible to subsequently-`dlopen`'d children. The OTP-internal NIFs (`crypto`, `asn1rt_nif`) are built as @@ -183,14 +193,14 @@ These are the things we've burned ourselves on. Following them isn't optional. tried `-Wl,--export-dynamic` and runtime `RTLD_GLOBAL` self-dlopen; neither works on Android). -12. **`:crypto` on-device is real OpenSSL** (3.x, statically linked). +13. **`:crypto` on-device is real OpenSSL** (3.x, statically linked). No more shim — old code that special-cased "no crypto on mobile" can be deleted. The deployer's `generate_crypto_shim/0` only fires when a cached OTP runtime *lacks* `lib/crypto-*/ebin/crypto.beam`; current tarballs have it. See `mob/crypto_plan.md` for the rebuild process when bumping OpenSSL. -13. **Igniter-based tasks live in mob_dev, never in the mob_new archive.** +14. **Igniter-based tasks live in mob_dev, never in the mob_new archive.** mob_new ships as a self-contained Mix archive; `ArchiveSelfContainedTest` pins that no hex-dep modules are reachable from archive code (an archive bundles only its own beams, so a call into a hex dep crashes every diff --git a/decisions/2026-08-28-sender-serialises-render.md b/decisions/2026-08-28-sender-serialises-render.md new file mode 100644 index 00000000..9f051d89 --- /dev/null +++ b/decisions/2026-08-28-sender-serialises-render.md @@ -0,0 +1,93 @@ +# The sender: one process owns every render NIF call + +- Date: 2026-08-28 +- Status: accepted +- Implements: MOB-110, second step of MOB-108 +- Builds on: `2026-08-27-screen-process-architecture.md` + +## Context + +`Mob.Screen.do_render/4` called `Mob.Renderer.render/4` inline, so rendering +happened in whichever process was handling the message. That is safe today only +because there is exactly one screen process. MOB-112 makes screens processes, +and at that point two of them can render at once. + +The native contract does not tolerate that. From `ios/mob_nif.m`: + +```c +static TapHandle tap_tables[2][MAX_TAP_HANDLES]; +static int tap_active = 0; +static int tap_build_count = 0; // cursor into the BUILDING table +``` + +`clear_taps` prepares the inactive table and resets the cursor, `register_tap` +appends at `tap_build_count++`, and `set_root` swaps the tables atomically. The +double buffering is explicitly there so a *concurrent reader* — a drag or scroll +event arriving mid-render — resolves against the last committed table. It does +nothing for concurrent *writers*: one global build cursor means two renders in +flight interleave their handles into the same building table, and whichever +reaches `set_root` first commits a table holding both screens' handles while the +other screen's tree is never committed at all. + +## Decision + +`Mob.Sender` is a named GenServer and the only caller of the render NIFs. +Screens build their tree — which must stay screen-side, since `Mob.Composite`, +`Mob.List`, and `Mob.Component` expansion all take `self()` and register +component pids — and hand the finished tree to `Mob.Sender.render/5`. + +`Mob.Renderer` itself uses `self()` nowhere outside doc examples, so the sender +can own the whole `render/4` call. The `{pid, tag}` in each tap comes from the +tree data the screen already baked in, not from the calling process. + +### Coalescing, and why it needs the render to be asynchronous + +Queuing rather than executing in the caller lets the sender look at what is +waiting: for one screen only the newest tree is committed, and a tree for a +screen that is not active is dropped outright. The second is what lets an +inactive tab hold state without rendering. + +### `sync/1` flushes; it does not rely on mailbox order + +The first design had `render/5` self-send a `:flush` and `sync/1` merely reply, +on the reasoning that the self-send would already be queued ahead of a later +call. That is wrong, and the tests caught it: `send(self(), :flush)` appends to +the *back* of the mailbox, which is behind a `sync` the caller has already +queued, so `sync/1` returned before the frame was committed. + +`sync/1` now performs the flush itself. This also strengthens coalescing — a +burst of renders followed by one `sync` produces a single commit. + +### The barrier goes on the call paths only + +`Mob.Test` documents `tap/2` and `back/1` as fire-and-forget and its navigation +helpers as synchronous. So `Mob.Screen` calls `Mob.Sender.sync/1` only in the +`handle_call` paths (`dispatch/3` and `{:navigate, _}`), which is exactly where +the documented guarantee lives. The `handle_info` paths stay asynchronous, which +is what leaves anything to coalesce. + +### A failed render must not kill the sender + +`commit/1` rescues. Every screen renders through this one process, so letting it +die on a malformed tree would freeze the entire UI rather than one screen. The +error is logged with a stacktrace. + +## Consequences + +- `do_render/4` now takes the nav state, because a render has to say which + screen it is for. The ref is `Mob.Nav.active_ref/1` — the active stack's name, + or `:__mob_single__` when the app declared no layout. MOB-112 replaces it with + a per-screen reference. +- `Mob.Screen` is still authoritative about which screen is active and calls + `set_active/1` on every render. MOB-113's router takes that over; the sender + already accepts it from anywhere. +- `Mob.Socket.put_root_view/2` now stores `:json_tree` directly. That was always + the only value `Mob.Renderer.render/4` returned, but the commit is now + asynchronous so there is no token to wait for. +- Coalescing is not observable in production yet: with one screen process, the + synchronous call paths flush every render, and the asynchronous ones rarely + queue two frames. It becomes load-bearing at MOB-112. +- If the sender is not running, renders are silently dropped — `GenServer.cast` + to an unregistered name is a no-op. `Mob.App.start/0` starts it before + `on_start/0`, so the only way to hit this is to bypass that entry point. + `running?/0` exists for a `mix mob.doctor` check. diff --git a/lib/mob/app.ex b/lib/mob/app.ex index ccba0c8e..2cd5f4e1 100644 --- a/lib/mob/app.ex +++ b/lib/mob/app.ex @@ -118,6 +118,14 @@ defmodule Mob.App do {:error, {:already_started, _}} -> :ok end + # The only process allowed to call the render NIFs. Must be up before + # on_start/0, which is where the app starts its root screen and the + # first render happens. + case Mob.Sender.start_link() do + {:ok, _} -> :ok + {:error, {:already_started, _}} -> :ok + end + # Mob.Device dispatcher + platform fan-out modules. Order matters: # the IOS / Android modules must exist before Mob.Device starts, # because Mob.Device forwards platform-tagged messages to them. diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index 6216d4f3..3b6ec9ee 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -130,6 +130,17 @@ defmodule Mob.Nav do @spec active(t()) :: stack_name() | nil def active(%__MODULE__{active: active}), do: active + @doc """ + A stable identifier for the active stack, for addressing renders. + + Falls back to `:__mob_single__` when the app declared no layout, so the sender + always has a concrete screen to compare against rather than a `nil` that would + match nothing. + """ + @spec active_ref(t()) :: stack_name() + def active_ref(%__MODULE__{active: nil}), do: :__mob_single__ + def active_ref(%__MODULE__{active: active}), do: active + @doc "Declared stack names, in declaration order." @spec stacks(t()) :: [stack_name()] def stacks(%__MODULE__{order: order}), do: order diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index 2c05b770..ec2343ca 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -246,6 +246,12 @@ defmodule Mob.Screen do # inset from storage never wins over the live value. loaded_socket = maybe_load_state(screen_module, mounted_socket) + # Seed the stacks this app declared. The screen we just mounted becomes + # the active stack's current screen; every other declared stack stays + # unmounted until first visited. With no declaration (or no registry, as + # in tests) this is an empty single-stack state — the old behaviour. + nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) + socket = if render_mode == :render do # Check for a notification that launched the app from a killed state. @@ -256,19 +262,13 @@ defmodule Mob.Screen do json -> send(self(), {:mob_launch_notification, json}) end - do_render(screen_module, loaded_socket) + do_render(screen_module, loaded_socket, nav) else loaded_socket end if screen_module.__mob_persist__(), do: schedule_state_sync() - # Seed the stacks this app declared. The screen we just mounted becomes - # the active stack's current screen; every other declared stack stays - # unmounted until first visited. With no declaration (or no registry, as - # in tests) this is an empty single-stack state — the old behaviour. - nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) - {:ok, {screen_module, socket, nav, render_mode}} {:error, reason} -> @@ -285,7 +285,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(module, new_socket, transition) + do_render_sync(module, new_socket, nav, transition) else new_socket end @@ -298,7 +298,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(module, new_socket, transition) + do_render_sync(module, new_socket, nav, transition) else new_socket end @@ -327,7 +327,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(new_module, new_socket, transition) + do_render_sync(new_module, new_socket, new_nav, transition) else new_socket end @@ -367,7 +367,7 @@ defmodule Mob.Screen do def handle_cast(:__mob_hot_reload__, {module, socket, nav, render_mode}) do new_socket = if render_mode == :render do - do_render(module, socket) + do_render(module, socket, nav) else socket end @@ -474,7 +474,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(module, new_socket, transition) + do_render(module, new_socket, new_nav, transition) else new_socket end @@ -494,7 +494,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(module, new_socket, transition) + do_render(module, new_socket, nav, transition) else new_socket end @@ -506,7 +506,7 @@ defmodule Mob.Screen do def handle_info({:component_changed, _id, _module}, {module, socket, nav, render_mode}) do new_socket = if render_mode == :render do - do_render(module, socket) + do_render(module, socket, nav) else socket end @@ -547,7 +547,7 @@ defmodule Mob.Screen do new_socket = if render_mode == :render do - do_render(module, new_socket, transition) + do_render(module, new_socket, nav, transition) else new_socket end @@ -764,7 +764,7 @@ defmodule Mob.Screen do # ── Render pipeline ─────────────────────────────────────────────────────── - defp do_render(module, socket, transition \\ :none) do + defp do_render(module, socket, nav, transition \\ :none) do platform = socket.__mob__.platform list_renderers = Map.get(socket.__mob__, :list_renderers, %{}) socket = ensure_safe_area(socket, platform) @@ -778,8 +778,29 @@ defmodule Mob.Screen do |> Mob.Component.expand(self(), platform) Mob.ComponentRegistry.reconcile(self(), active_component_keys) - {:ok, token} = Mob.Renderer.render(tree, platform, :mob_nif, transition) - Mob.Socket.put_root_view(socket, token) + + # Every render NIF call goes through the sender — the native tap tables + # share one build cursor, so the clear/register/set_root sequence has to be + # serialised through a single process. See Mob.Sender. + # + # This process is still authoritative about which stack is active; MOB-113's + # router takes that over. + ref = Mob.Nav.active_ref(nav) + Mob.Sender.set_active(ref) + Mob.Sender.render(ref, tree, platform, :mob_nif, transition) + + # The commit is asynchronous, so there is no token to wait for. Mob.Renderer + # has only ever returned this one constant. + Mob.Socket.put_root_view(socket, :json_tree) + end + + # The synchronous call paths must not reply until the frame is committed — + # Mob.Test's navigation helpers document that guarantee. The handle_info paths + # stay fire-and-forget, which is what leaves the sender free to coalesce them. + defp do_render_sync(module, socket, nav, transition) do + rendered = do_render(module, socket, nav, transition) + Mob.Sender.sync() + rendered end defp ensure_safe_area(socket, platform) do diff --git a/lib/mob/sender.ex b/lib/mob/sender.ex new file mode 100644 index 00000000..94c72352 --- /dev/null +++ b/lib/mob/sender.ex @@ -0,0 +1,155 @@ +defmodule Mob.Sender do + @moduledoc """ + The only process permitted to call the render NIFs. + + ## Why this is forced + + Not a style choice — the native tap registry requires it. From + `ios/mob_nif.m` (the Android side in `android/jni/mob_nif.zig` is the same + shape): + + static TapHandle tap_tables[2][MAX_TAP_HANDLES]; + static int tap_active = 0; + static int tap_build_count = 0; // cursor into the BUILDING table + + `clear_taps` prepares the inactive table and resets the cursor, `register_tap` + appends at `tap_build_count++`, and `set_root` swaps the tables atomically. + The double buffering makes a *concurrent reader* safe — a drag or scroll event + arriving mid-render still resolves against the last committed table. It does + nothing for concurrent *writers*: there is one global build cursor, so two + renders in flight interleave their handles into the same building table, and + whichever reaches `set_root` first commits a table holding both screens' + handles while the other screen's tree is never committed at all. + + So `clear_taps -> register_tap* -> set_root` is one indivisible sequence, and + serialising it through a single process is the only thing that keeps it that + way once more than one screen is live (MOB-112). + + ## Coalescing falls out of it + + Because renders are queued rather than executed by the caller, the sender can + look at what is waiting and commit only what matters: + + * for a given screen, only the newest tree is committed — a screen that + re-renders three times before the sender gets to it produces one commit, not + three + * a tree for a screen that is not active is dropped, never committed + + That second point is what lets an inactive tab keep its state without + rendering. It is also why switching stacks re-renders: the incoming screen's + tree is produced fresh at switch time rather than replayed from a queue. + + ## Ordering + + `render/5` is asynchronous, so a caller that needs the commit to have landed + calls `sync/1`. That works by mailbox ordering rather than by tracking work: + the flush is self-sent during the render cast, so it is already queued ahead + of any later `sync/1` call. `Mob.Screen` uses this to keep the guarantee + `Mob.Test` documents — that `tap/2` and `navigate/2` return only once the + re-render is complete. + """ + + use GenServer + + require Logger + + @typedoc """ + Identifies which screen a tree belongs to. Today this is the active + navigation stack's name (see `Mob.Nav.active_ref/1`); MOB-112 replaces it with + a per-screen reference. + """ + @type screen_ref :: atom() | reference() + + defstruct active: nil, pending: %{} + + @doc "Start the sender. Named, so there is exactly one." + @spec start_link(keyword()) :: GenServer.on_start() + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, opts, name: __MODULE__) + end + + @doc "Whether the sender is running. Renders are dropped when it is not." + @spec running?() :: boolean() + def running?, do: is_pid(Process.whereis(__MODULE__)) + + @doc """ + Declare which screen's trees may be committed. + + A render for any other screen is dropped. `Mob.Screen` sets this today; + MOB-113's router takes it over. + """ + @spec set_active(screen_ref()) :: :ok + def set_active(ref), do: GenServer.cast(__MODULE__, {:set_active, ref}) + + @doc """ + Queue `tree` for commit on behalf of screen `ref`. + + Returns immediately. The tree is committed only if `ref` is active when the + sender gets to it, and only if no newer tree for `ref` has arrived by then. + """ + @spec render(screen_ref(), map(), atom(), module() | atom(), atom()) :: :ok + def render(ref, tree, platform, nif, transition) do + GenServer.cast(__MODULE__, {:render, ref, tree, platform, nif, transition}) + end + + @doc """ + Block until every render queued before this call has been committed or + dropped. + """ + @spec sync(timeout()) :: :ok + def sync(timeout \\ 5000), do: GenServer.call(__MODULE__, :sync, timeout) + + # ── GenServer ───────────────────────────────────────────────────────────── + + @impl GenServer + def init(opts) do + {:ok, %__MODULE__{active: Keyword.get(opts, :active)}} + end + + @impl GenServer + def handle_cast({:set_active, ref}, state), do: {:noreply, %{state | active: ref}} + + def handle_cast({:render, ref, tree, platform, nif, transition}, state) do + # Overwrite rather than append: a newer tree for the same screen supersedes + # the one waiting, which is the whole point of queueing here. + pending = Map.put(state.pending, ref, {tree, platform, nif, transition}) + send(self(), :flush) + {:noreply, %{state | pending: pending}} + end + + @impl GenServer + def handle_info(:flush, state), do: {:noreply, flush(state)} + + def handle_info(_message, state), do: {:noreply, state} + + @impl GenServer + def handle_call(:sync, _from, state) do + # Flush here rather than just replying. The `:flush` this render self-sent + # lands at the BACK of the mailbox, which is behind a `sync` the caller has + # already queued — replying without flushing would return before the frame + # was committed, which is the one thing this function promises not to do. + {:reply, :ok, flush(state)} + end + + defp flush(state) do + case Map.fetch(state.pending, state.active) do + {:ok, payload} -> commit(payload) + :error -> :ok + end + + # Everything else waiting belongs to a screen that is not active. Dropping + # it is deliberate: by the time such a screen becomes active it will have + # re-rendered, so committing a queued tree would only show a stale frame. + %{state | pending: %{}} + end + + defp commit({tree, platform, nif, transition}) do + Mob.Renderer.render(tree, platform, nif, transition) + rescue + error -> + # A render that raises must not take the sender down with it: every other + # screen renders through this process, so losing it freezes the whole UI. + Logger.error("[mob] render failed: " <> Exception.format(:error, error, __STACKTRACE__)) + :error + end +end diff --git a/test/mob/sender_test.exs b/test/mob/sender_test.exs new file mode 100644 index 00000000..475a7687 --- /dev/null +++ b/test/mob/sender_test.exs @@ -0,0 +1,189 @@ +defmodule Mob.SenderTest do + use ExUnit.Case, async: false + + import ExUnit.CaptureLog + + alias Mob.Sender + + # Records what the render path would have called on the native side. Mirrors + # the stub in renderer_test.exs; kept local so the two can drift apart. + defmodule RecordingNif do + def start, do: Agent.start(fn -> [] end, name: __MODULE__) + def calls, do: __MODULE__ |> Agent.get(& &1) |> Enum.reverse() + def reset, do: Agent.update(__MODULE__, fn _ -> [] end) + + def clear_taps, do: record({:clear_taps, []}) + def set_transition(t), do: record({:set_transition, t}) + def set_root(json), do: record({:set_root, json}) + def register_tap(_term), do: 0 + + defp record(call) do + Agent.update(__MODULE__, &[call | &1]) + :ok + end + end + + defmodule RaisingNif do + def clear_taps, do: :ok + def set_transition(_), do: :ok + def register_tap(_), do: 0 + def set_root(_json), do: raise("native exploded") + end + + defp tree(text), do: %{type: :text, props: %{text: text}, children: []} + + defp committed_texts do + for {:set_root, json} <- RecordingNif.calls(), do: json + end + + setup do + # Order matters: a sender left running from an earlier test can still have a + # flush queued, and it would record into the fresh recorder. + case Process.whereis(Sender) do + nil -> :ok + pid -> GenServer.stop(pid) + end + + case Process.whereis(RecordingNif) do + nil -> :ok + pid -> Agent.stop(pid) + end + + RecordingNif.start() + :ok + end + + defp start_sender(active) do + {:ok, pid} = Sender.start_link(active: active) + on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + pid + end + + describe "committing" do + test "commits a tree for the active screen" do + start_sender(:home) + Sender.render(:home, tree("hello"), :ios, RecordingNif, :none) + Sender.sync() + + assert [json] = committed_texts() + assert json =~ "hello" + end + + test "issues the full clear/transition/set_root sequence" do + start_sender(:home) + Sender.render(:home, tree("x"), :ios, RecordingNif, :push) + Sender.sync() + + assert [{:clear_taps, []}, {:set_transition, :push}, {:set_root, _}] = RecordingNif.calls() + end + + test "drops a tree for a screen that is not active" do + start_sender(:home) + Sender.render(:settings, tree("inactive"), :ios, RecordingNif, :none) + Sender.sync() + + assert committed_texts() == [] + end + + test "set_active/1 changes which screen may commit" do + start_sender(:home) + Sender.set_active(:settings) + Sender.render(:home, tree("stale"), :ios, RecordingNif, :none) + Sender.render(:settings, tree("fresh"), :ios, RecordingNif, :none) + Sender.sync() + + assert [json] = committed_texts() + assert json =~ "fresh" + end + end + + describe "coalescing" do + # Driven through the callbacks rather than the running process: whether two + # casts land before a flush is a scheduling race, and the point being pinned + # is what the sender does when they do. + test "a newer tree for the same screen supersedes the one waiting" do + state = %Sender{active: :home} + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("first"), :ios, RecordingNif, :none}, state) + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("second"), :ios, RecordingNif, :none}, state) + + assert map_size(state.pending) == 1 + + {:noreply, state} = Sender.handle_info(:flush, state) + + assert [json] = committed_texts() + assert json =~ "second" + refute json =~ "first" + assert state.pending == %{} + end + + test "a flush drops every queued tree, not just the one it commits" do + state = %Sender{active: :home} + + {:noreply, state} = + Sender.handle_cast({:render, :settings, tree("other"), :ios, RecordingNif, :none}, state) + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("mine"), :ios, RecordingNif, :none}, state) + + {:noreply, state} = Sender.handle_info(:flush, state) + + assert state.pending == %{} + assert [json] = committed_texts() + assert json =~ "mine" + end + + test "a flush with nothing pending for the active screen commits nothing" do + {:noreply, state} = Sender.handle_info(:flush, %Sender{active: :home}) + assert committed_texts() == [] + assert state.pending == %{} + end + end + + describe "resilience" do + test "a render that raises does not take the sender down" do + pid = start_sender(:home) + + capture_log(fn -> + Sender.render(:home, tree("boom"), :ios, RaisingNif, :none) + Sender.sync() + end) + + assert Process.alive?(pid) + + # And it still serves the next screen — losing this process would freeze + # the whole UI, since every screen renders through it. + Sender.render(:home, tree("after"), :ios, RecordingNif, :none) + Sender.sync() + assert [json] = committed_texts() + assert json =~ "after" + end + end + + describe "sync/1" do + test "returns only after queued renders have been committed" do + start_sender(:home) + + for i <- 1..20 do + Sender.render(:home, tree("frame #{i}"), :ios, RecordingNif, :none) + end + + Sender.sync() + + # Coalescing means the count is not 20, but the LAST frame must be on + # screen by the time sync returns — that is the guarantee callers rely on. + assert List.last(committed_texts()) =~ "frame 20" + end + end + + describe "running?/0" do + test "false when not started, true once it is" do + refute Sender.running?() + start_sender(:home) + assert Sender.running?() + end + end +end From cdfbe868513d0e52300e19847e5f848adb5a647b Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 14:10:18 -0600 Subject: [PATCH 2/2] MOB-110: fix six defects found by adversarial review; device-verify both platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. set_active/1 was announced on every render. Reads harmless, is not: every screen process runs the same do_render/4, so at MOB-112 a background screen whose timer fires would promote itself and commit over the foreground screen — disarming the exact mechanism this step exists to build. It also guaranteed a fight with MOB-113's router, both writing `active` with no ownership rule. Now called only where navigation establishes the active stack: init/1 and apply_switch_tab/4, which are the two call sites the router takes over unchanged. 2. A boot path documented in this repo silently rendered nothing. liveview_notes.md prescribes skipping Mob.App entirely, and renders are casts — a cast to an unregistered name is :ok — so a missing sender gave a blank screen with no log until the first synchronous render exited :noproc. Mob.Screen.init/1 now calls Mob.Sender.ensure_started/0, started unlinked so a screen crash cannot take down the process every other screen renders through. 3. Coalescing silently swallowed navigation transitions. A :push rendered from handle_info (forward_to_screen, back gesture, list select) superseded by an ordinary :none re-render from a timer tick lost its animation: correct content, no transition, intermittently. A pending non-:none transition now survives being superseded; a newer explicit one still wins. 4. sync/1's default 5s timeout would newly kill the screen on a slow frame — rendering was unbounded when it ran inline. Mob.Screen passes :infinity. 5. Mob.Test's documented sync point was broken. :sys.get_state(:mob_screen) no longer implies the frame is on screen, which matters for the functions reading the native side (view_tree, screenshot, tap_id, element_frames). Added Mob.Test.settle/2, which drains both the screen and the sender, and corrected the docs. 6. The sender's moduledoc still taught the mailbox-ordering design the ADR and the previous commit message both identify as the bug — the worst kind of stale doc, since AGENTS.md points readers there as the authority. Also adds the screen-to-sender wiring test the review correctly identified as missing: nothing asserted who declares the active screen or when. One assertion fails if set_active/1 ever moves back into do_render/4. Two findings are carried into MOB-112 rather than fixed, recorded in the ADR: ComponentRegistry.reconcile/2 still runs screen-side and destructively before the commit, so a dropped tree leaves the displayed frame holding handles to dead pids (narrow now, routine once background renders are supposed to be dropped); and the sender has no supervisor, consistent with how Mob.App starts every other service but uniquely bad when it dies. Device-verified, since this changes when the first frame is committed. sheetprobe cold-started on the iOS simulator (BEAM log to `step 5 => ok`) and the Android emulator, both rendering the first frame through the sender, and a tap presented a sheet on each — exercising the sync path and a real transition. The first iOS attempt died with eaddrinuse, which was the documented adb/iOS-sim port collision, not this change. Tests: 10 more (16 sender, 4 wiring). Suite 1181 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 6 +- .../2026-08-28-sender-serialises-render.md | 59 +++++++++ lib/mob/screen.ex | 25 ++-- lib/mob/sender.ex | 65 ++++++++-- lib/mob/test.ex | 43 ++++++- liveview_notes.md | 2 +- test/mob/screen_sender_wiring_test.exs | 116 ++++++++++++++++++ test/mob/sender_test.exs | 79 ++++++++++++ 8 files changed, 371 insertions(+), 24 deletions(-) create mode 100644 test/mob/screen_sender_wiring_test.exs diff --git a/AGENTS.md b/AGENTS.md index 5e35c06f..65bfd5d1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -170,9 +170,9 @@ These are the things we've burned ourselves on. Following them isn't optional. Real fix tracked in `issues.md` #4 (hash bundle id into port). 10. **Compile-time `~r//` literals are unsafe on OTP 28.** They bake a - `:re_exported_pattern` and call `:re.import/1` at runtime; OTP 28.0 removed - that function. Use `Regex.compile!("...", "flags")` to compile at runtime. - 71 literals across mob_dev were swept in 0.3.17. + `:re_exported_pattern` and call `:re.import/1` at runtime; OTP 28.0 removed + that function. Use `Regex.compile!("...", "flags")` to compile at runtime. + 71 literals across mob_dev were swept in 0.3.17. 11. **`:mob_nif.log/1` for early startup logging, `Logger` after Mob.App.start.** `Mob.NativeLogger.install()` runs as part of `Mob.App.start` and reroutes diff --git a/decisions/2026-08-28-sender-serialises-render.md b/decisions/2026-08-28-sender-serialises-render.md index 9f051d89..49135e3d 100644 --- a/decisions/2026-08-28-sender-serialises-render.md +++ b/decisions/2026-08-28-sender-serialises-render.md @@ -66,6 +66,47 @@ helpers as synchronous. So `Mob.Screen` calls `Mob.Sender.sync/1` only in the the documented guarantee lives. The `handle_info` paths stay asynchronous, which is what leaves anything to coalesce. +### `set_active/1` is a navigation fact, not a render fact + +The first cut announced the active screen on every render, which reads as +harmless and is not. Every screen process runs the same `do_render/4`, so at +MOB-112 a *background* screen whose timer fires would promote itself and commit +over the foreground screen — disarming the exact mechanism this step exists to +build. It also guaranteed a fight with MOB-113's router, both writing `active` +with no ownership rule. + +`Mob.Sender.set_active/1` is now called only where navigation establishes the +active stack: `Mob.Screen.init/1` and `apply_switch_tab/4`. The router takes +those two call sites over unchanged. + +### No render path may run without a sender + +Renders are casts, and a cast to an unregistered name is `:ok`. A missing sender +therefore fails in the worst available way: a blank screen, no log, no crash — +until the first synchronous render exits `:noproc`. `Mob.App.start/0` is not the +only boot path (`liveview_notes.md` documents skipping `Mob.App` entirely), so +`Mob.Screen.init/1` calls `Mob.Sender.ensure_started/0` in `:render` mode. + +That start is deliberately unlinked. The caller is a screen, and a screen crash +must not take down the process every other screen renders through. + +### Coalescing carries the transition forward + +Superseding a tree must not silently swallow a navigation animation. A `:push` +rendered from `handle_info` — `forward_to_screen/2`, the back gesture, list +select — could be superseded by an ordinary `:none` re-render from a timer tick +or a component update, and the push would never reach native: correct content, +no animation, intermittently. The transition describes the frame's animation +rather than its content, so a pending non-`:none` transition survives being +superseded; a newer explicit transition still wins. + +### `sync/1` gets no deadline from `Mob.Screen` + +Rendering was unbounded when it ran inline. `sync/1` is a `GenServer.call`, so +the default 5s timeout would newly turn a slow frame — a large `Mob.List` tree, +a dirty-CPU `set_root` on a loaded device — into a dead screen process. +`Mob.Screen` passes `:infinity`, restoring the previous behaviour exactly. + ### A failed render must not kill the sender `commit/1` rescues. Every screen renders through this one process, so letting it @@ -87,6 +128,24 @@ error is logged with a stacktrace. - Coalescing is not observable in production yet: with one screen process, the synchronous call paths flush every render, and the asynchronous ones rarely queue two frames. It becomes load-bearing at MOB-112. +- **`Mob.Test` gained `settle/2`.** `:sys.get_state(:mob_screen)` was a + documented sync point and is no longer sufficient on its own: the screen hands + its tree to the sender and returns, so a drained screen mailbox does not mean + the frame is on screen. This only affects the functions that read the *native* + side (`view_tree/1`, `screenshot/2`, `tap_id/2`, `element_frames/2`); + `tree/1` and `assigns/1` re-render in-process. `settle/2` drains both. +- **Known gaps, carried into MOB-112 rather than fixed here.** + `Mob.ComponentRegistry.reconcile/2` still runs screen-side before the cast and + is destructive — it kills components absent from the new tree — so a tree that + is then dropped leaves the displayed frame holding handles to dead pids. + Narrow today; routine at MOB-112, where background renders are *supposed* to + be dropped, so reconcile has to move to the commit or become non-destructive. + The sender is also started with a bare `start_link` and no supervisor, matching + how `Mob.App.start/0` starts every other service, but it is the one process + whose death silently freezes the whole UI. +- `Mob.Socket.put_root_view/2` is now written even when the commit was dropped + or raised. The field is read nowhere in `lib/` and predates this change; it + should be removed rather than made to lie, which is its own change. - If the sender is not running, renders are silently dropped — `GenServer.cast` to an unregistered name is a no-op. `Mob.App.start/0` starts it before `on_start/0`, so the only way to hit this is to bypass that entry point. diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index ec2343ca..5d73efcd 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -229,7 +229,11 @@ defmodule Mob.Screen do # Register under :mob_screen so C-layer mob_handle_back() can find us. # Only in :render mode (production); tests use :no_render and run without a NIF. - if render_mode == :render, do: Process.register(self(), :mob_screen) + if render_mode == :render do + Process.register(self(), :mob_screen) + # Renders are casts, so a missing sender would blank the screen silently. + Mob.Sender.ensure_started() + end socket = if render_mode == :render do @@ -251,6 +255,7 @@ defmodule Mob.Screen do # unmounted until first visited. With no declaration (or no registry, as # in tests) this is an empty single-stack state — the old behaviour. nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) + Mob.Sender.set_active(Mob.Nav.active_ref(nav)) socket = if render_mode == :render do @@ -634,9 +639,11 @@ defmodule Mob.Screen do case Mob.Nav.switch(nav, tab, current) do {:switched, new_nav, {target_module, target_socket}} -> + Mob.Sender.set_active(Mob.Nav.active_ref(new_nav)) {target_module, target_socket, new_nav, :none} {:mount_root, new_nav, root_module} -> + Mob.Sender.set_active(Mob.Nav.active_ref(new_nav)) {mounted_module, mounted} = mount_destination(root_module, %{}, socket) {mounted_module, mounted, new_nav, :none} @@ -783,11 +790,12 @@ defmodule Mob.Screen do # share one build cursor, so the clear/register/set_root sequence has to be # serialised through a single process. See Mob.Sender. # - # This process is still authoritative about which stack is active; MOB-113's - # router takes that over. - ref = Mob.Nav.active_ref(nav) - Mob.Sender.set_active(ref) - Mob.Sender.render(ref, tree, platform, :mob_nif, transition) + # Which screen is active is declared by the navigation code (init and + # apply_switch_tab/4), not here. Announcing it on every render would let any + # screen promote itself simply by re-rendering — at MOB-112 a background + # screen's timer would then commit over the foreground one, disarming the + # drop-inactive mechanism this whole step exists to build. + Mob.Sender.render(Mob.Nav.active_ref(nav), tree, platform, :mob_nif, transition) # The commit is asynchronous, so there is no token to wait for. Mob.Renderer # has only ever returned this one constant. @@ -799,7 +807,10 @@ defmodule Mob.Screen do # stay fire-and-forget, which is what leaves the sender free to coalesce them. defp do_render_sync(module, socket, nav, transition) do rendered = do_render(module, socket, nav, transition) - Mob.Sender.sync() + # No deadline: rendering was unbounded when it ran inline, and sync/1 is a + # call, so a default 5s timeout would turn a slow frame on a loaded device + # into a dead screen process. + Mob.Sender.sync(:infinity) rendered end diff --git a/lib/mob/sender.ex b/lib/mob/sender.ex index 94c72352..d02cd634 100644 --- a/lib/mob/sender.ex +++ b/lib/mob/sender.ex @@ -42,11 +42,18 @@ defmodule Mob.Sender do ## Ordering `render/5` is asynchronous, so a caller that needs the commit to have landed - calls `sync/1`. That works by mailbox ordering rather than by tracking work: - the flush is self-sent during the render cast, so it is already queued ahead - of any later `sync/1` call. `Mob.Screen` uses this to keep the guarantee - `Mob.Test` documents — that `tap/2` and `navigate/2` return only once the - re-render is complete. + calls `sync/1`, which performs the flush itself rather than waiting for the + self-sent one. + + It has to. `send(self(), :flush)` during the render cast appends to the *back* + of the mailbox — behind a `sync/1` the caller has already queued — so a + `sync/1` that merely replied would return before the frame was committed. + Mailbox order is the wrong tool here, and it looks like the right one. + + `Mob.Screen` uses `sync/1` on its `handle_call` paths to keep the guarantee + `Mob.Test` documents for the synchronous navigation helpers. Note the ordering + guarantee only covers renders cast by the *calling* process; the BEAM promises + nothing about the relative order of sends from different processes. """ use GenServer @@ -68,10 +75,35 @@ defmodule Mob.Sender do GenServer.start_link(__MODULE__, opts, name: __MODULE__) end - @doc "Whether the sender is running. Renders are dropped when it is not." + @doc "Whether the sender is running. Renders are silently dropped when it is not." @spec running?() :: boolean() def running?, do: is_pid(Process.whereis(__MODULE__)) + @doc """ + Start the sender if it is not already running. + + `Mob.App.start/0` starts it on the normal boot path, but a screen can be + started without going through `Mob.App` — `liveview_notes.md` documents + exactly that — and a missing sender fails in the worst possible way: renders + are casts, so they vanish silently and the app shows a blank screen with no + log, until the first synchronous render exits `:noproc`. `Mob.Screen` calls + this so no render path can reach that state. + + Deliberately unlinked. The caller is usually a screen, and a screen crash must + not take down the process every other screen renders through. + """ + @spec ensure_started() :: :ok + def ensure_started do + if running?() do + :ok + else + case GenServer.start(__MODULE__, [], name: __MODULE__) do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + end + end + end + @doc """ Declare which screen's trees may be committed. @@ -95,6 +127,12 @@ defmodule Mob.Sender do @doc """ Block until every render queued before this call has been committed or dropped. + + "Queued before" means cast by the *calling* process — the BEAM orders sends + between a given pair of processes and says nothing about sends from different + ones. Committed *or dropped*: a return of `:ok` does not promise the caller's + own tree reached the screen, only that the sender has caught up. A tree for a + screen that is not active is dropped, and `sync/1` returns `:ok` all the same. """ @spec sync(timeout()) :: :ok def sync(timeout \\ 5000), do: GenServer.call(__MODULE__, :sync, timeout) @@ -111,12 +149,25 @@ defmodule Mob.Sender do def handle_cast({:render, ref, tree, platform, nif, transition}, state) do # Overwrite rather than append: a newer tree for the same screen supersedes - # the one waiting, which is the whole point of queueing here. + # the one waiting, which is the whole point of queueing here. The transition + # is the exception — it describes the navigation animation for this frame, + # not the frame's content, so a push superseded by an ordinary re-render + # still has to animate as a push or the transition is silently swallowed. + transition = carry_transition(state.pending, ref, transition) pending = Map.put(state.pending, ref, {tree, platform, nif, transition}) send(self(), :flush) {:noreply, %{state | pending: pending}} end + defp carry_transition(pending, ref, :none) do + case Map.fetch(pending, ref) do + {:ok, {_tree, _platform, _nif, superseded}} -> superseded + :error -> :none + end + end + + defp carry_transition(_pending, _ref, transition), do: transition + @impl GenServer def handle_info(:flush, state), do: {:noreply, flush(state)} diff --git a/lib/mob/test.ex b/lib/mob/test.ex index 241e46d5..bde72934 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -58,14 +58,21 @@ defmodule Mob.Test do they block until the navigation and re-render complete. This makes them safe to follow immediately with `screen/1` or `assigns/1` to verify the result. - `back/1` and `send_message/2` are fire-and-forget (they send a message to the - screen process and return immediately). Use `:sys.get_state/1` as a sync point + `back/1`, `tap/2` and `send_message/2` are fire-and-forget (they send a message + to the screen process and return immediately). Use `settle/2` as a sync point if you need to wait before reading state: Mob.Test.send_message(node, {:permission, :camera, :granted}) - :rpc.call(node, :sys, :get_state, [:mob_screen]) # flush mailbox + Mob.Test.settle(node) Mob.Test.assigns(node) + `:sys.get_state/1` on `:mob_screen` is no longer sufficient on its own: since + MOB-110 the screen hands its tree to `Mob.Sender` and returns, so a drained + screen mailbox does not mean the frame is on screen. That only matters for the + functions that read the *native* side — `view_tree/1`, `screenshot/2`, + `tap_id/2`, `element_frames/2`. `tree/1` and `assigns/1` re-render in-process + and are unaffected. + ## Two layers of inspection: render tree vs native UI `Mob.Test` exposes two complementary views of what the app is showing: @@ -205,7 +212,8 @@ defmodule Mob.Test do The tag comes from `on_tap: {self(), :tag_atom}` in the screen's `render/1`. Check the screen's render function to find available tags. - Fire-and-forget — does not wait for the screen to finish processing. + Fire-and-forget — does not wait for the screen to finish processing. Follow + with `settle/2` before reading the native side. Mob.Test.tap(node, :save) Mob.Test.tap(node, :open_detail) @@ -216,13 +224,36 @@ defmodule Mob.Test do :ok end + @doc """ + Block until the app has finished processing and the current frame is on + screen. + + Drains the screen process's mailbox, then waits for `Mob.Sender` to commit. + Both halves are needed: the screen builds the tree and the sender commits it, + so a drained screen mailbox alone does not mean the frame has been rendered. + + Use after any fire-and-forget call (`tap/2`, `back/1`, `send_message/2`) + before reading the native side with `view_tree/1`, `screenshot/2`, `tap_id/2` + or `element_frames/2`. + + Mob.Test.tap(node, :save) + Mob.Test.settle(node) + Mob.Test.view_tree(node) + """ + @spec settle(node(), timeout()) :: :ok + def settle(node, timeout \\ 5000) do + :rpc.call(node, :sys, :get_state, [:mob_screen]) + :rpc.call(node, Mob.Sender, :sync, [timeout]) + :ok + end + # ── System gestures ─────────────────────────────────────────────────────────── @doc """ Simulate the system back gesture (Android hardware back / iOS edge-pan). - Fire-and-forget. The framework pops the navigation stack; if already at the - root, it exits the app. Prefer `pop/1` when you need to know that navigation + Fire-and-forget — follow with `settle/2` before reading the native side. The + framework pops the navigation stack; if already at the root, it exits the app. Prefer `pop/1` when you need to know that navigation has finished before reading state. """ @spec back(node()) :: :ok diff --git a/liveview_notes.md b/liveview_notes.md index 304aaf10..fab062c2 100644 --- a/liveview_notes.md +++ b/liveview_notes.md @@ -91,7 +91,7 @@ Start it explicitly after `ensure_all_started/1`: ```elixir {:ok, _} = Application.ensure_all_started(:lv_test) {:ok, _} = Mob.ComponentRegistry.start_link() -Mob.Screen.start_root(LvTest.MobScreen) +Mob.Screen.start_root(LvTest.MobScreen) # starts Mob.Sender itself if needed ``` --- diff --git a/test/mob/screen_sender_wiring_test.exs b/test/mob/screen_sender_wiring_test.exs new file mode 100644 index 00000000..c8f3ffcf --- /dev/null +++ b/test/mob/screen_sender_wiring_test.exs @@ -0,0 +1,116 @@ +defmodule Mob.ScreenSenderWiringTest do + @moduledoc """ + The seam between `Mob.Screen` and `Mob.Sender`. + + Screens run `:no_render` here, so no tree is ever committed — but + `Mob.Sender.set_active/1` is a cast, so with a real sender running these + assertions pin *who* declares the active screen and *when*. That matters: + announcing it on every render instead would let any screen promote itself, + which is what disarms the drop-inactive mechanism at MOB-112. + """ + use ExUnit.Case, async: false + + alias Mob.Sender + + defmodule HomeScreen do + use Mob.Screen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :count, 0)} + def render(assigns), do: %{type: :text, props: %{text: "#{assigns.count}"}, children: []} + + def handle_event("bump", _, socket), + do: {:noreply, Mob.Socket.assign(socket, :count, socket.assigns.count + 1)} + + def handle_event("to_settings", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :settings)} + + def handle_event("to_home", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :home)} + + def handle_event("to_nowhere", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :not_a_stack)} + end + + defmodule SettingsScreen do + use Mob.Screen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :count, 0)} + + def render(assigns), + do: %{type: :text, props: %{text: "settings #{assigns.count}"}, children: []} + + def handle_event("bump", _, socket), + do: {:noreply, Mob.Socket.assign(socket, :count, socket.assigns.count + 1)} + + def handle_event("to_home", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :home)} + end + + defmodule TabApp do + @behaviour Mob.App + import Mob.App + + @home Mob.ScreenSenderWiringTest.HomeScreen + @settings Mob.ScreenSenderWiringTest.SettingsScreen + + def navigation(_) do + tab_bar([stack(:home, root: @home), stack(:settings, root: @settings)]) + end + end + + defp active, do: :sys.get_state(Process.whereis(Sender)).active + + setup do + for name <- [Sender, Mob.Nav.Registry] do + case Process.whereis(name) do + nil -> :ok + pid -> GenServer.stop(pid) + end + end + + {:ok, sender} = Sender.start_link([]) + {:ok, registry} = Mob.Nav.Registry.start_link(TabApp) + + on_exit(fn -> + for pid <- [sender, registry], Process.alive?(pid), do: GenServer.stop(pid) + end) + + {:ok, screen} = Mob.Screen.start_link(HomeScreen, %{}) + on_exit(fn -> if Process.alive?(screen), do: GenServer.stop(screen) end) + + %{screen: screen} + end + + test "mounting a screen declares its stack active", %{screen: _} do + Sender.sync() + assert active() == :home + end + + test "switching stacks moves the active screen", %{screen: screen} do + Mob.Screen.dispatch(screen, "to_settings", %{}) + Sender.sync() + assert active() == :settings + + Mob.Screen.dispatch(screen, "to_home", %{}) + Sender.sync() + assert active() == :home + end + + test "an ordinary re-render does not change the active screen", %{screen: screen} do + Mob.Screen.dispatch(screen, "to_settings", %{}) + Sender.sync() + assert active() == :settings + + # A screen re-rendering must not promote itself — this is the assertion that + # fails if set_active/1 moves back into do_render/4. + Mob.Screen.dispatch(screen, "bump", %{}) + Sender.sync() + assert active() == :settings + end + + test "a switch to an undeclared stack leaves the active screen alone", %{screen: screen} do + Mob.Screen.dispatch(screen, "to_nowhere", %{}) + Sender.sync() + assert active() == :home + end +end diff --git a/test/mob/sender_test.exs b/test/mob/sender_test.exs index 475a7687..0dbce39a 100644 --- a/test/mob/sender_test.exs +++ b/test/mob/sender_test.exs @@ -143,6 +143,85 @@ defmodule Mob.SenderTest do end end + describe "coalescing preserves the transition" do + test "a push superseded by an ordinary re-render still animates as a push" do + state = %Sender{active: :home} + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("a"), :ios, RecordingNif, :push}, state) + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("b"), :ios, RecordingNif, :none}, state) + + {:noreply, _state} = Sender.handle_info(:flush, state) + + assert {:set_transition, :push} in RecordingNif.calls() + assert [json] = committed_texts() + assert json =~ "b" + end + + test "a newer transition wins over an older one" do + state = %Sender{active: :home} + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("a"), :ios, RecordingNif, :push}, state) + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("b"), :ios, RecordingNif, :pop}, state) + + {:noreply, _state} = Sender.handle_info(:flush, state) + + assert {:set_transition, :pop} in RecordingNif.calls() + refute {:set_transition, :push} in RecordingNif.calls() + end + + test "a plain re-render with nothing pending stays :none" do + state = %Sender{active: :home} + + {:noreply, state} = + Sender.handle_cast({:render, :home, tree("a"), :ios, RecordingNif, :none}, state) + + {:noreply, _state} = Sender.handle_info(:flush, state) + + assert {:set_transition, :none} in RecordingNif.calls() + end + end + + describe "ensure_started/0" do + test "starts the sender when it is missing" do + refute Sender.running?() + assert :ok = Sender.ensure_started() + assert Sender.running?() + on_exit(fn -> if Sender.running?(), do: GenServer.stop(Sender) end) + end + + test "is a no-op when one is already running" do + pid = start_sender(:home) + assert :ok = Sender.ensure_started() + assert Process.whereis(Sender) == pid + end + + test "does not link to the caller — a screen crash must not take it down" do + test_pid = self() + + caller = + spawn(fn -> + Sender.ensure_started() + send(test_pid, :started) + receive do: (:die -> exit(:boom)) + end) + + assert_receive :started + sender = Process.whereis(Sender) + ref = Process.monitor(caller) + send(caller, :die) + assert_receive {:DOWN, ^ref, :process, ^caller, _} + + assert Process.alive?(sender) + on_exit(fn -> if Sender.running?(), do: GenServer.stop(Sender) end) + end + end + describe "resilience" do test "a render that raises does not take the sender down" do pid = start_sender(:home)