diff --git a/AGENTS.md b/AGENTS.md index dd86082b..65bfd5d1 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 - `: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. **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..49135e3d --- /dev/null +++ b/decisions/2026-08-28-sender-serialises-render.md @@ -0,0 +1,152 @@ +# 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. + +### `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 +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. +- **`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. + `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..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 @@ -246,6 +250,13 @@ 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) + Mob.Sender.set_active(Mob.Nav.active_ref(nav)) + socket = if render_mode == :render do # Check for a notification that launched the app from a killed state. @@ -256,19 +267,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 +290,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 +303,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 +332,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 +372,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 +479,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 +499,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 +511,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 +552,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 @@ -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} @@ -764,7 +771,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 +785,33 @@ 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. + # + # 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. + 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) + # 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 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..d02cd634 --- /dev/null +++ b/lib/mob/sender.ex @@ -0,0 +1,206 @@ +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`, 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 + + 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 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. + + 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. + + "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) + + # ── 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. 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)} + + 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/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 new file mode 100644 index 00000000..0dbce39a --- /dev/null +++ b/test/mob/sender_test.exs @@ -0,0 +1,268 @@ +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 "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) + + 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