From 900df9d3d1bba10a968232a5b79f1dd3abb01088 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 29 Aug 2026 00:11:05 -0600 Subject: [PATCH 1/2] MOB-113: extract Mob.Router and pin the hot-path property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MOB-112 gave Mob.Screen a third job. It was already the behaviour screens implement and the macro generating their boilerplate; it became the process owning navigation and every screen process too — nearly 1000 lines, with a moduledoc that had to describe all three, which is part of why that moduledoc has been wrong twice. Mob.Router now holds navigation, the screen processes, and the :mob_screen registered name. Mob.Screen keeps the behaviour and macro and delegates its public API, so Mob.Screen.dispatch/3 and friends are unchanged. A move, not a redesign: the process model landed in MOB-112 and is untouched. The property MOB-113 actually exists to guarantee — the router must not be in the per-message path — already held. MOB-111's listener delivers native events straight to the owning screen's pid, so a tap goes native -> listener -> screen -> sender with the router uninvolved. What is new is that it is asserted rather than reasoned about. router_hot_path_test.exs traces :receive on the router across a tap, a value-carrying event, and a burst of fifty messages, and asserts the trace is empty. Verified as a negative control: making the screen notify the router on each message fails exactly those three tests. That is worth a test rather than a comment. An earlier costing of this architecture assumed a router in the loop and concluded per-screen processes could not escape a hop per message; splitting the router from the sender is what dissolved that, and a property that load-bearing should fail loudly when someone breaks it. Also: decode_file_result/3 stopped being a @doc false public function and became private to Mob.Screen.Server, its only caller. And two guides were making claims that were aspirational before MOB-112 and are now nearly true — screen_lifecycle.md called each screen "a separate, supervised process", which is half right: separate yes, supervised no, because the router restarts screens itself since only it knows where a crashed one sat. Rationale in decisions/2026-08-29-router-off-the-hot-path.md. Tests: 5 new. Suite 1238 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../2026-08-29-router-off-the-hot-path.md | 73 ++ guides/architecture.md | 6 +- guides/screen_lifecycle.md | 2 +- lib/mob/nav.ex | 2 +- lib/mob/router.ex | 791 +++++++++++++++++ lib/mob/screen.ex | 821 +----------------- lib/mob/screen/server.ex | 56 +- test/mob/nav/screen_nav_test.exs | 13 +- test/mob/router_hot_path_test.exs | 157 ++++ 9 files changed, 1121 insertions(+), 800 deletions(-) create mode 100644 decisions/2026-08-29-router-off-the-hot-path.md create mode 100644 lib/mob/router.ex create mode 100644 test/mob/router_hot_path_test.exs diff --git a/decisions/2026-08-29-router-off-the-hot-path.md b/decisions/2026-08-29-router-off-the-hot-path.md new file mode 100644 index 00000000..c889e4c5 --- /dev/null +++ b/decisions/2026-08-29-router-off-the-hot-path.md @@ -0,0 +1,73 @@ +# The router: navigation extracted, and kept out of the per-message path + +- Date: 2026-08-29 +- Status: accepted +- Implements: MOB-113, fifth step of MOB-108 +- Builds on: `2026-08-28-screen-processes-and-supervision.md` + +## Context + +MOB-112 gave `Mob.Screen` a third job. It was already the behaviour screens +implement and the macro that generates their boilerplate; it became the process +owning navigation and every screen process as well. Nearly 1000 lines, and a +moduledoc that had to describe all three — which is part of why that moduledoc +had been wrong twice. + +The epic's remaining constraint on that process is sharper than "tidy it up": +the router must **not** be in the per-message path. + +## Decision + +`Mob.Router` holds navigation, the screen processes, and the `:mob_screen` +registered name. `Mob.Screen` keeps the behaviour and the macro, and delegates +its public API, so `Mob.Screen.dispatch/3` and friends still work. + +This is a move, not a redesign — the process model landed in MOB-112 and is +unchanged here. + +### The hot-path property already held; this pins it + +Tracing the router's mailbox while a screen handles ordinary messages shows it +receives nothing. That is not new — MOB-111's listener already delivers native +events straight to the owning screen's pid, so a tap goes native → listener → +screen → sender with the router uninvolved. + +What is new is that it is now **asserted rather than reasoned about**. +`test/mob/router_hot_path_test.exs` traces `:receive` on the router across a +tap, a value-carrying event, and a burst of fifty messages, and asserts the +trace is empty. A negative control confirms it bites: making the screen notify +the router on each message fails exactly those three tests. + +This matters more than a tidy-up. An earlier costing of this architecture +assumed a router in the loop and concluded per-screen processes could not escape +a hop per message. Splitting the router from the sender is what dissolved that, +and a property that load-bearing should fail loudly when someone breaks it — +not be rediscovered by reading the code. + +### What still goes through the router, deliberately + +* navigation actions a screen produces (`{:nav_action, …}`) +* the back gesture, alert actions, and launch notifications, which native + addresses to `:mob_screen` +* device events and plugin messages sent to `:mob_screen`, forwarded to the + active screen +* `Mob.Screen.dispatch/3`, the programmatic entry point used by tests and + tooling + +None is a per-message path for a running screen. The last two are the ones to +watch: they make the router a shared serialisation point, which is MOB-121. + +## Consequences + +- `Mob.Screen` drops from ~995 lines to ~230, and its moduledoc describes one + thing. +- `decode_file_result/3` stopped being a `@doc false` public function on the + owner and became private to `Mob.Screen.Server`, its only caller. +- Two guides were making claims that were aspirational before MOB-112 and are + now nearly true; they now say what actually happens. `screen_lifecycle.md` + claimed each screen was "a separate, supervised process" — separate is now + right, supervised is still not: the router restarts screens itself because + only it knows where a crashed one sat. +- The `:mob_screen` name still belongs to the router, so no native change and no + generator-template change. The epic said the router would take that name over; + it already had it under a different module name. diff --git a/guides/architecture.md b/guides/architecture.md index 9a9ca2c9..a133a534 100644 --- a/guides/architecture.md +++ b/guides/architecture.md @@ -7,7 +7,7 @@ Mob takes an unusual position in the mobile framework landscape. To understand w ```mermaid flowchart TD A["Your Elixir App
(GenServers, Phoenix, Ecto, whatever you normally use)"] - B["Mob.Screen
(your UI module) — GenServer"] + B["Mob.Screen.Server
(your UI module) — one GenServer per live screen"] C["Mob.Renderer
serialise + token resolution"] D1["Compose (JVM)
Android"] D2["SwiftUI (Swift)
iOS"] @@ -20,7 +20,7 @@ flowchart TD BEAM and OTP run **on the device** — embedded inside the APK and the iOS app bundle. There is no server. Your screen logic, navigation state, and business logic all execute locally in the same BEAM node that the user has installed. -The rendering layer is thin: `render/1` returns a plain Elixir map (the component tree), `Mob.Renderer` serialises it to JSON and passes it to the native side via a NIF call. Compose or SwiftUI diff and display it. UI events travel back as NIF callbacks that send messages to the screen GenServer. The BEAM owns state; the native UI is a thin view. +The rendering layer is thin: `render/1` returns a plain Elixir map (the component tree), `Mob.Renderer` serialises it to JSON and passes it to the native side via a NIF call. Compose or SwiftUI diff and display it. UI events travel back as NIF callbacks; `Mob.Listener` unwraps them and sends them straight to the owning screen's process, without going through the router. The BEAM owns state; the native UI is a thin view. ## Erlang distribution for development @@ -81,4 +81,4 @@ The right choice between them depends on your app's connectivity requirements an - **Native UI.** Components render as Compose and SwiftUI primitives. Animations, accessibility, platform gestures, and dark mode all work because the native layer handles them. - **No server required.** The app is self-contained. Online features are optional add-ons, not the foundation. - **Development speed.** `mix mob.connect` + `nl/1` gives you sub-second code push to a running device. The OTP debug toolchain — tracing, observer, remote IEx — is available without any extra infrastructure. -- **OTP reliability.** A crashed screen is a crashed GenServer. OTP can restart it, log it, and keep the rest of the app running. You get fault tolerance on mobile for free. +- **OTP reliability.** A crashed screen is a crashed GenServer. `Mob.Router` restarts it, logs it, and keeps navigation and every other screen running. You get fault tolerance on mobile for free. diff --git a/guides/screen_lifecycle.md b/guides/screen_lifecycle.md index cb0ee41e..c45d5b6d 100644 --- a/guides/screen_lifecycle.md +++ b/guides/screen_lifecycle.md @@ -1,6 +1,6 @@ # Screen Lifecycle -A Mob screen is a GenServer wrapped by `Mob.Screen`. Each screen in the navigation stack is a separate, supervised process. Understanding the lifecycle means understanding when each callback fires and what you can do in it. +A Mob screen is a GenServer — a `Mob.Screen.Server` process holding your module's socket. Each live screen in the navigation stack is a separate process, and `Mob.Router` owns them: it starts them, stops them, and restarts one that crashes. It is not an OTP `Supervisor`, because only the router knows where in the navigation a crashed screen sat; a restarted screen re-mounts and loses its assigns. Understanding the lifecycle means understanding when each callback fires and what you can do in it. ## Callbacks diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index 87d7a55d..a41659df 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -111,7 +111,7 @@ defmodule Mob.Nav do # `navigation/1` is app-supplied and unvalidated. `Nav.Registry` has always # tolerated a shape it doesn't recognise (`register_nav(_), do: :ok`), and this - # runs inside `Mob.Screen.init/1` — raising here would turn a declaration the + # runs inside `Mob.Router.init/1` — raising here would turn a declaration the # framework previously ignored into a failure to boot. def from_layout(_layout, _current_module), do: new() diff --git a/lib/mob/router.ex b/lib/mob/router.ex new file mode 100644 index 00000000..8c850aa2 --- /dev/null +++ b/lib/mob/router.ex @@ -0,0 +1,791 @@ +defmodule Mob.Router do + @moduledoc """ + Owns navigation, and the one process per live screen that serves it. + + Which stacks exist, which is active, and one `Mob.Screen.Server` per live + screen — this process starts them, stops them, and restarts one that crashes. + It keeps the `:mob_screen` registered name, so the native layer's + `enif_whereis_pid` lookups (back gesture, alert actions, launch + notifications) are unaffected. + + ## Not in the per-message path + + A screen handling an ordinary message never touches this process. Native + events reach a screen directly: `Mob.Listener` unwraps the envelope and sends + to the screen's own pid, the screen renders, and `Mob.Sender` commits. The + router hears only about navigation. + + That is the property MOB-113 exists to guarantee, and it is what makes one + process per screen affordable. An earlier costing of this design assumed a + router in the loop and concluded per-screen processes could not escape a hop + per message; splitting the router from the sender is what dissolved that. + + `Mob.Screen` delegates its public API here, so callers keep using + `Mob.Screen.dispatch/3` and friends. + + ## A navigation entry + + `%{module:, pid:, params:, ref:}`. + + `params` is carried because a restart has to reproduce the screen exactly — + one that mounts on `%{id: id}` cannot come back from `%{}`. + + `ref` identifies the screen to `Mob.Sender`, and is unique **per screen**, + not per stack. Every screen is a live process that repaints on any message it + receives, including the ones below the top of a stack; keyed by stack, a timer + tick in a screen the user cannot see would commit its tree — tap table + included — over the screen they can. The sender only commits the tree whose + ref is active. The ref survives a restart, because the replacement is the same + logical screen. + + See `decisions/2026-08-28-screen-processes-and-supervision.md`. + """ + + use GenServer + + require Logger + + # Stopping a screen must not block the owner forever on one that is wedged in + # a long callback. GenServer.stop/3 otherwise waits :infinity. + @stop_timeout_ms 5_000 + + # A screen that mounts fine but crashes on every render otherwise loops at + # full speed — measured at ~6500 restarts/sec, each writing a log line. An + # OTP supervisor would cap this with max_restart_intensity; the owner restarts + # screens itself (it is the only thing that knows where one sat), so it has to + # carry the ceiling too. + @max_restarts 5 + @restart_window_ms 10_000 + + @doc """ + Start a screen process linked to the calling process. + + `params` is passed as the first argument to `mount/3`. + """ + @spec start_link(module(), map(), keyword()) :: GenServer.on_start() + def start_link(screen_module, params, opts \\ []) do + GenServer.start_link(__MODULE__, {screen_module, params, :no_render, :android}, opts) + end + + @doc """ + Return the module of the currently active screen in the navigation stack. + Intended for testing and debugging. + """ + @spec get_current_module(pid()) :: module() + def get_current_module(pid), do: GenServer.call(pid, :get_current_module) + + @doc """ + Return the navigation history (list of `{module, socket}` pairs, head = most recent). + Intended for testing and debugging. + """ + @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t() | nil}] + def get_nav_history(pid), do: GenServer.call(pid, :get_nav_history) + + @doc """ + Start a screen as the root UI screen. Calls mount, renders the component tree + via `Mob.Renderer`, and calls `set_root` on the resulting view. + + This is the main entry point for production use. `start_link/2` is for tests + (no NIF calls). + """ + @spec start_root(module(), map(), keyword()) :: GenServer.on_start() + def start_root(screen_module, params \\ %{}, opts \\ []) do + platform = :mob_nif.platform() + GenServer.start_link(__MODULE__, {screen_module, params, :render, platform}, opts) + end + + @doc """ + Dispatch a UI event to the screen process. Returns `:ok` synchronously once + the event has been processed and the state updated. + """ + @spec dispatch(pid(), String.t(), map()) :: :ok + def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}, :infinity) + + @doc """ + Return the current socket state of a running screen, or `nil` while that + screen is being restarted. + + Intended for testing and debugging — not for production app logic. + """ + @spec get_socket(pid()) :: Mob.Socket.t() | nil + def get_socket(pid), do: GenServer.call(pid, :get_socket) + + @doc """ + Return the pid of the process owning the currently active screen. + + Each live screen is its own process since MOB-112; this is how tooling + reaches the one that is on screen. + """ + @spec get_screen_pid(GenServer.server()) :: pid() + def get_screen_pid(pid), do: GenServer.call(pid, :get_screen_pid) + + # ── GenServer callbacks ─────────────────────────────────────────────────── + + @impl GenServer + def init({screen_module, params, render_mode, platform}) do + # Linked *and* trapping. Linking alone makes the owner die with any screen + # it stops or that crashes; trapping alone orphans every screen when the + # owner dies — and an orphaned persisted screen keeps dumping to + # Mob.ScreenState under the same key as its live replacement. Together the + # owner sees each exit as a message and screens still come down with it. + Process.flag(:trap_exit, true) + + 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() + # Started before the first render: that render is what bakes the + # listener's pid into the native tap handles. + Mob.Listener.ensure_started() + end + + # Seed the stacks this app declared. The screen we are about to mount + # becomes the active stack's current screen; every other declared stack + # stays unmounted until first visited. + nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) + + state = %{ + current: nil, + nav: nav, + render_mode: render_mode, + platform: platform, + screens: %{}, + restarts: %{} + } + + case start_screen(screen_module, params, state) do + {:ok, entry, state} -> + state = make_current(state, entry) + + if render_mode == :render do + # A notification that launched the app from a killed state. Sent to + # self so it arrives via handle_info after init returns, consistent + # with foreground notification delivery. + case :mob_nif.take_launch_notification() do + :none -> :ok + json -> send(self(), {:mob_launch_notification, json}) + end + + paint(entry, :none, state) + end + + {:ok, state} + + {:error, reason} -> + {:stop, reason} + end + end + + @impl GenServer + def handle_call({:event, event, params}, _from, state) do + # The whole point of MOB-112: a crash in the user's handle_event must not + # come back up this call and take the owner — and with it navigation and + # every sibling screen — down. The exit that follows restarts the screen. + case safe_call(fn -> Mob.Screen.Server.dispatch(state.current.pid, event, params) end) do + {:ok, {:ok, nav_action}} -> {:reply, :ok, apply_nav_action(nav_action, state, :sync)} + {:exit, _reason} -> {:reply, :ok, state} + end + end + + def handle_call({:navigate, nav_action}, _from, state) do + {:reply, :ok, apply_nav_action(nav_action, state, :sync)} + end + + def handle_call(:get_socket, _from, state) do + {:reply, current_socket(state), state} + end + + def handle_call(:get_screen_pid, _from, state) do + {:reply, state.current.pid, state} + end + + def handle_call(:get_current_module, _from, state) do + {:reply, state.current.module, state} + end + + def handle_call(:get_nav_history, _from, state) do + {:reply, Enum.map(Mob.Nav.history(state.nav), &entry_with_socket/1), state} + end + + def handle_call(:inspect, _from, state) do + socket = current_socket(state) + + # The tree is built in the screen's own process. Calling render/1 here would + # run user code in the owner, so a raise in it — reached by Mob.Test.tree/1, + # i.e. the debugging path — would kill navigation and every screen. + tree = + case safe_call(fn -> Mob.Screen.Server.tree(state.current.pid) end) do + {:ok, tree} -> tree + {:exit, _reason} -> nil + end + + info = %{ + screen: state.current.module, + assigns: socket && socket.assigns, + nav_history: Enum.map(Mob.Nav.history(state.nav), & &1.module), + tree: tree + } + + {:reply, info, state} + end + + @impl GenServer + def handle_cast(:__mob_hot_reload__, state) do + # A broadcast, not one cast: every live screen has to repaint with the + # newly loaded code, not just the one on screen. + Enum.each(all_entries(state), &Mob.Screen.Server.hot_reload(&1.pid)) + {:noreply, state} + end + + @impl GenServer + # A screen's callback asked to navigate. Only the active screen may drive + # navigation — a background screen's timer must not yank the stack out from + # under whatever the user is looking at. + def handle_info({:nav_action, action, from}, state) do + if from == state.current.pid do + {:noreply, apply_nav_action(action, state, :async)} + else + {:noreply, state} + end + end + + # A screen exited. Trapping turns this into a message rather than the owner's + # death — the isolation MOB-112 exists to deliver. A pid we have already + # dropped from `screens` was stopped deliberately, so its exit is expected. + def handle_info({:EXIT, pid, reason}, state) do + case Map.pop(state.screens, pid) do + {nil, _screens} -> {:noreply, state} + {entry, screens} -> {:noreply, restart_screen(entry, reason, %{state | screens: screens})} + end + end + + # A notification that launched the app from a killed state. + def handle_info({:mob_launch_notification, json}, state) do + # Decoded here rather than in the screen because the payload comes from + # native, not from a screen. :json.decode/1 raises on malformed input, and + # this runs in the owner — so a bad payload would take down every screen. + notification = + try do + decode_notification_json(json) + rescue + error -> + Logger.error( + "[mob] launch notification could not be decoded: " <> Exception.message(error) + ) + + %{source: :local, data: %{}} + end + + handle_info({:notification, notification}, state) + end + + # System back gesture (Android hardware/swipe, iOS edge-pan). Handled here so + # every screen gets back navigation without implementing anything. If a + # WebView is present and has internal history, navigate within it first. + def handle_info({:mob, :back}, state) do + if state.render_mode == :render && :mob_nif.webview_can_go_back() do + :mob_nif.webview_go_back() + {:noreply, state} + else + case {Mob.Nav.history(state.nav), Mob.Nav.back_target(state.nav)} do + {[_ | _], _} -> + {:noreply, apply_nav_action({:pop}, state, :async)} + + # Nothing left to pop on a secondary stack: fall back to the first one + # rather than exiting and discarding every parked stack. + {[], {:switch, target}} -> + {:noreply, apply_nav_action({:switch_tab, target}, state, :async)} + + {[], :exit} -> + if state.render_mode == :render, do: :mob_nif.exit_app() + {:noreply, state} + end + end + end + + # Anything else addressed to :mob_screen — device events, notifications, + # plugin messages — belongs to the screen the user is looking at. + def handle_info(message, state) do + send(state.current.pid, message) + {:noreply, state} + end + + @impl GenServer + def terminate(_reason, _state) do + # Deliberately does NOT stop screens. They are linked and trap exits, so + # each shuts down gracefully on its own — gen_server turns the parent's EXIT + # into a terminate/2 call, which runs the user's terminate/2 and the final + # state dump. + # + # Calling GenServer.stop/3 here instead corrupts the owner's own exit: the + # screen's :shutdown travels back while the owner is mid-terminate and + # replaces its reason, so a clean stop(owner, :normal) exits :shutdown. + :ok + end + + # ── Screen lifecycle ────────────────────────────────────────────────────── + + # Calling into a screen that is mid-crash must not propagate into the owner. + # The exit signal that follows is what actually repairs the screen. + defp safe_call(fun) do + {:ok, fun.()} + catch + :exit, reason -> {:exit, reason} + end + + defp start_screen(module, params, state), do: start_screen(module, params, make_ref(), state) + + defp start_screen(module, params, ref, state) do + opts = [ + module: module, + params: params, + ref: ref, + owner: self(), + render_mode: state.render_mode, + platform: state.platform + ] + + case Mob.Screen.Server.start_link(opts) do + {:ok, pid} -> + entry = %{module: module, pid: pid, params: params, ref: ref} + {:ok, entry, %{state | screens: Map.put(state.screens, pid, entry)}} + + {:error, reason} -> + {:error, reason} + end + end + + # The single place `current` changes. The sender is told here and nowhere + # else, so only the screen the user is looking at can commit a frame. + defp make_current(state, entry) do + Mob.Sender.set_active(entry.ref) + %{state | current: entry} + end + + defp all_entries(state) do + parked = + state.nav + |> Map.get(:parked, %{}) + |> Enum.flat_map(fn {_name, %{current: current, history: history}} -> + [current | history] + end) + + ([state.current] ++ Mob.Nav.history(state.nav) ++ parked) + |> Enum.reject(&is_nil/1) + |> Enum.uniq_by(& &1.pid) + end + + defp current_socket(state) do + case safe_call(fn -> Mob.Screen.Server.socket(state.current.pid) end) do + {:ok, socket} -> socket + {:exit, _reason} -> nil + end + end + + defp entry_with_socket(entry) do + case safe_call(fn -> Mob.Screen.Server.socket(entry.pid) end) do + {:ok, socket} -> {entry.module, socket} + {:exit, _reason} -> {entry.module, nil} + end + end + + # A crashed screen is re-mounted in place, with the params and stack ref it + # was created with. It loses its assigns — a restart runs mount/3 again — + # which is the documented consequence of the isolation. + defp restart_screen(entry, reason, state) do + {allowed?, state} = record_restart(entry.ref, state) + + if allowed? do + do_restart_screen(entry, reason, state) + else + Logger.error( + "[mob] screen #{inspect(entry.module)} crashed #{@max_restarts + 1} times in " <> + "#{@restart_window_ms}ms and is being given up on rather than restarted in a loop. " <> + "Reason: #{inspect(reason)}" + ) + + recover_from_failed_restart(entry, state) + end + end + + # Sliding window per screen ref, so the ceiling follows a logical screen + # across its restarts rather than resetting with each new pid. + defp record_restart(ref, state) do + now = System.monotonic_time(:millisecond) + recent = Map.get(state.restarts, ref, []) |> Enum.filter(&(now - &1 < @restart_window_ms)) + state = %{state | restarts: Map.put(state.restarts, ref, [now | recent])} + {length(recent) < @max_restarts, state} + end + + defp do_restart_screen(%{pid: dead_pid} = entry, reason, state) do + log_restart(entry.module, reason) + + case start_screen(entry.module, entry.params, entry.ref, state) do + {:ok, new_entry, state} -> + state = substitute(state, dead_pid, new_entry) + if state.current.pid == new_entry.pid, do: paint(new_entry, :none, state) + state + + {:error, mount_reason} -> + # Re-mounting failed, so the screen cannot come back. Leaving the dead + # entry in place would freeze the app silently — every later event + # would call a corpse and return :ok. Pop to whatever is underneath if + # there is anything; otherwise say so loudly rather than pretend. + log_restart_failure(entry.module, mount_reason) + recover_from_failed_restart(entry, state) + end + end + + defp substitute(state, dead_pid, new_entry) do + replace = fn + %{pid: ^dead_pid} -> new_entry + other -> other + end + + nav = + state.nav + |> Mob.Nav.put_history(Enum.map(Mob.Nav.history(state.nav), replace)) + |> Mob.Nav.map_parked(replace) + + current = if state.current.pid == dead_pid, do: new_entry, else: state.current + %{state | nav: nav, current: current} + end + + defp recover_from_failed_restart(%{pid: dead_pid} = entry, state) do + cond do + state.current.pid != dead_pid -> + # A background screen. Drop it from the stack it sat in rather than + # leave a corpse for a later pop or tab switch to restore. + drop_entry(state, dead_pid) + + Mob.Nav.history(state.nav) != [] -> + state = drop_entry(state, dead_pid) + [previous | rest] = Mob.Nav.history(state.nav) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) + paint(previous, :pop, state) + state + + true -> + fall_back_to_parked_stack(entry, state) + end + end + + # "Nothing beneath it" is the *common* shape in a tab-bar app — every tab root + # has an empty history — and there is usually a live screen parked under + # another tab. Leaving a dead pid as `current` bricks the app: every later + # event is safe_call'd into a corpse and replies :ok, while a perfectly good + # screen sits parked one call away. + defp fall_back_to_parked_stack(entry, state) do + with [name | _] <- Mob.Nav.parked_stacks(state.nav), + {:switched, nav, live} <- Mob.Nav.switch(state.nav, name, entry) do + # switch/3 parks the dead entry under the outgoing stack; drop it straight + # after, so nothing can restore a corpse by switching back. + nav = Mob.Nav.drop_parked(nav, &(&1.pid == entry.pid)) + state = make_current(%{state | nav: nav}, live) + paint(live, :pop, state) + state + else + _ -> + Logger.error( + "[mob] #{inspect(entry.module)} could not be restarted and there is no other live " <> + "screen to fall back to. The app has no live screen." + ) + + state + end + end + + defp drop_entry(state, dead_pid) do + dead? = fn %{pid: pid} -> pid == dead_pid end + + nav = + state.nav + |> Mob.Nav.put_history(Enum.reject(Mob.Nav.history(state.nav), dead?)) + |> Mob.Nav.drop_parked(dead?) + + %{state | nav: nav} + end + + defp log_restart(module, reason) do + Logger.error( + "[mob] screen #{inspect(module)} crashed and is being restarted; its assigns are lost. " <> + "Reason: #{inspect(reason)}" + ) + end + + defp log_restart_failure(module, reason) do + Logger.error( + "[mob] screen #{inspect(module)} could not be restarted — mount/3 failed: " <> + "#{inspect(reason)}" + ) + end + + defp paint(entry, transition, state), do: do_paint(entry, transition, state, :async) + + defp do_paint(_entry, _transition, %{render_mode: :no_render}, _mode), do: :ok + + defp do_paint(entry, transition, _state, :sync) do + # Unprotected, this is the other way a screen crash killed the owner: the + # user's render/1 runs inside the screen, and a raise there exits this call. + case safe_call(fn -> Mob.Screen.Server.render_sync(entry.pid, transition) end) do + {:ok, _} -> :ok + {:exit, _reason} -> :ok + end + end + + defp do_paint(entry, transition, _state, :async), + do: Mob.Screen.Server.render(entry.pid, transition) + + # Drop the entry from tracking BEFORE stopping, so the exit we asked for is + # recognised as deliberate rather than restarted as a crash. + defp stop_screen(%{pid: pid} = entry, state) do + # Forget the restart history too — the map is keyed by screen ref and + # nothing else ever removes a key, so it grows for the owner's lifetime. + state = %{ + state + | screens: Map.delete(state.screens, pid), + restarts: Map.delete(state.restarts, entry.ref) + } + + stop_process(pid) + state + end + + defp stop_process(pid) do + if Process.alive?(pid) do + # Unlink first. We are discarding this screen deliberately, so its + # :shutdown exit must not travel back up the link — during the owner's own + # terminate/2 that signal overrides the owner's exit reason, which turns a + # clean GenServer.stop(owner, :normal) into an exit with :shutdown. + Process.unlink(pid) + + try do + GenServer.stop(pid, :shutdown, @stop_timeout_ms) + catch + # A wedged screen ignores the shutdown request. GenServer.stop/3 kills + # only its own proxy on timeout, so without this the screen survives — + # unlinked, untracked, and still dumping to Mob.ScreenState under the + # same key as its replacement. That is the orphan hazard linking exists + # to prevent. + :exit, _reason -> Process.exit(pid, :kill) + end + end + + :ok + end + + # ── Navigation ──────────────────────────────────────────────────────────── + + defp apply_nav_action(nil, state, _mode), do: state + + defp apply_nav_action({:push, dest, params}, state, mode) do + with {:ok, new_module, route_params} <- safe_resolve(dest, state) do + push_resolved(new_module, Map.merge(route_params, params), state, mode) + end + end + + defp apply_nav_action({:pop}, state, mode) do + case Mob.Nav.history(state.nav) do + [previous | rest] -> + # The screen being popped off leaves the stack for good, so its process + # goes with it. The ones still in `rest` stay resident — that is what + # makes pop restore prior state without re-mounting. + state = stop_screen(state.current, state) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) + do_paint(previous, :pop, state, mode) + state + + [] -> + repaint_current(state, mode) + end + end + + defp apply_nav_action({:pop_to_root}, state, mode) do + case Enum.reverse(Mob.Nav.history(state.nav)) do + [root | _] -> + discarded = [ + state.current | Enum.reject(Mob.Nav.history(state.nav), &(&1.pid == root.pid)) + ] + + state = Enum.reduce(discarded, state, &stop_screen/2) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, root) + do_paint(root, :pop, state, mode) + state + + [] -> + repaint_current(state, mode) + end + end + + defp apply_nav_action({:pop_to, dest}, state, mode) do + with {:ok, target, _params} <- safe_resolve(dest, state) do + pop_to_resolved(target, state, mode) + end + end + + defp apply_nav_action({:reset, dest, params}, state, mode) do + with {:ok, new_module, route_params} <- safe_resolve(dest, state) do + reset_resolved(new_module, Map.merge(route_params, params), state, mode) + end + end + + defp apply_nav_action({:switch_tab, tab}, state, mode) do + case Mob.Nav.switch(state.nav, tab, state.current) do + {:switched, nav, entry} -> + state = make_current(%{state | nav: nav}, entry) + do_paint(entry, :none, state, mode) + state + + {:mount_root, nav, root_module} -> + # Start first, mutate after. Switching nav before the mount could fail + # leaves navigation pointing at a stack whose screen never started. + case start_screen(root_module, %{}, state) do + {:ok, entry, state} -> + state = make_current(%{state | nav: nav}, entry) + do_paint(entry, :none, state, mode) + state + + {:error, _reason} -> + repaint_current(state, mode) + end + + :noop -> + repaint_current(state, mode) + end + end + + defp push_resolved(new_module, mount_params, state, mode) do + case start_screen(new_module, mount_params, state) do + {:ok, entry, state} -> + nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) + state = make_current(%{state | nav: nav}, entry) + do_paint(entry, :push, state, mode) + state + + {:error, _reason} -> + repaint_current(state, mode) + end + end + + defp reset_resolved(new_module, mount_params, state, mode) do + case start_screen(new_module, mount_params, state) do + {:ok, entry, state} -> + discarded = [state.current | Mob.Nav.history(state.nav)] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry) + do_paint(entry, :reset, state, mode) + state + + {:error, _reason} -> + repaint_current(state, mode) + end + end + + defp pop_to_resolved(target, state, mode) do + history = Mob.Nav.history(state.nav) + + case pop_to_module(history, target) do + {:found, previous, rest} -> + discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) + do_paint(previous, :pop, state, mode) + state + + :not_found -> + repaint_current(state, mode) + end + end + + # A nav action that changed nothing still has to paint. The screen deliberately + # does not paint when it produced an action, so without this the assigns it set + # in the same callback would never reach the screen — re-tapping the active tab + # being the everyday case. + defp repaint_current(state, mode) do + do_paint(state.current, :none, state, mode) + state + end + + # `push_screen/2` and friends take any atom, and an unregistered one raises. + # That raise runs in the OWNER, where it would kill navigation and every + # screen — falsifying the isolation this module's moduledoc promises, over a + # typo. A bad destination leaves navigation untouched and repaints instead. + # + # Returns the state unchanged (via repaint_current/2) rather than an error + # tuple, so the `with` in each caller falls straight through. + defp safe_resolve(dest, state) do + {module, route_params} = resolve_destination(dest) + {:ok, module, route_params} + rescue + error -> + Logger.error( + "[mob] navigation to #{inspect(dest)} failed and was ignored: " <> + Exception.message(error) + ) + + repaint_current(state, :async) + end + + # Resolves a navigation destination to {module, route_params}. A loaded + # module navigates directly (no route-bound params); a registered route atom + # may carry params (Mob.Nav.Registry.register/3 — the data-driven-plugin + # pattern), merged UNDER the caller's push params at the mount call site. + defp resolve_destination(dest) when is_atom(dest) do + case Code.ensure_loaded(dest) do + {:module, ^dest} -> + {dest, %{}} + + _ -> + case Mob.Nav.Registry.lookup_route(dest) do + {:ok, module, route_params} -> + {module, route_params} + + {:error, :not_found} -> + raise ArgumentError, + "Mob.Screen: unknown navigation destination #{inspect(dest)}. " <> + "Register it via Mob.Nav.Registry.register/2 or declare it in " <> + "your App.navigation/1." + end + end + end + + defp pop_to_module([], _target), do: :not_found + + defp pop_to_module([%{module: module} = entry | rest], target) do + if module == target do + {:found, entry, rest} + else + pop_to_module(rest, target) + end + end + + # ── Helpers ─────────────────────────────────────────────────────────────── + + defp decode_notification_json(json) when is_binary(json) do + case :json.decode(json) do + map when is_map(map) -> + source = + case Map.get(map, "source", "local") do + "push" -> :push + _ -> :local + end + + data = + case Map.get(map, "data") do + d when is_map(d) -> Map.new(d, fn {k, v} -> {String.to_atom(k), v} end) + _ -> %{} + end + + %{ + id: Map.get(map, "id"), + title: Map.get(map, "title"), + body: Map.get(map, "body"), + data: data, + source: source + } + + _ -> + %{source: :local, data: %{}} + end + end +end diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index a40e767f..6e9a9c08 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -3,17 +3,20 @@ defmodule Mob.Screen do Behaviour and GenServer wrapper for a Mob screen. Each live screen runs in its own `Mob.Screen.Server` process holding a - `Mob.Socket`. This module is their **owner**: it holds the navigation state, - starts and stops screens, and restarts one that crashes. + `Mob.Socket`. `Mob.Router` owns them: it holds the navigation state, starts + and stops screens, and restarts one that crashes. That gives you isolation — a buggy `handle_event` crashes its own screen and - the owner restarts it without taking down navigation, sibling screens, - background services, or the BEAM. The owner is not an OTP `Supervisor`; it + the router restarts it without taking down navigation, sibling screens, + background services, or the BEAM. The router is not an OTP `Supervisor`; it restarts screens itself because only it knows where in the navigation a crashed screen sat (see `decisions/2026-08-28-screen-processes-and-supervision.md`). A restarted screen re-mounts and loses its assigns. + The functions below delegate to `Mob.Router`; this module is the behaviour + your screens implement. + Lifecycle callbacks (`mount`, `render`, `handle_event`, `handle_info`, `terminate`) map directly to the GenServer lifecycle, so the BEAM's existing tools (selective receive, monitors, hot code push) work on screens without @@ -165,44 +168,10 @@ defmodule Mob.Screen do end end - # ── Owner process ───────────────────────────────────────────────────────── - # - # This process owns navigation: which stacks exist, which is active, and one - # `Mob.Screen.Server` process per live screen. It keeps the `:mob_screen` - # registered name, so the native layer's `enif_whereis_pid` lookups (back - # gesture, alert actions, launch notifications) are unaffected. - # - # A navigation entry is `%{module:, pid:, params:, ref:}`. - # - # `params` is carried because a restart has to reproduce the screen exactly — - # one that mounts on `%{id: id}` cannot come back from `%{}`. + # ── Public API ──────────────────────────────────────────────────────────── # - # `ref` identifies the screen to `Mob.Sender`, and is unique **per screen**, - # not per stack. That distinction is load-bearing: every screen is a live - # process now, including the ones below the top of the stack, and they repaint - # on any message they receive. Keyed by stack, a timer tick in a screen the - # user cannot see would commit its tree — tap table included — over the screen - # they can. The sender only ever commits the tree whose ref is active, so a - # background repaint is dropped wherever that screen sits. The ref survives a - # restart, because the replacement is the same logical screen. - # - # MOB-113 extracts this role into `Mob.Router`. - - use GenServer - - require Logger - - # Stopping a screen must not block the owner forever on one that is wedged in - # a long callback. GenServer.stop/3 otherwise waits :infinity. - @stop_timeout_ms 5_000 - - # A screen that mounts fine but crashes on every render otherwise loops at - # full speed — measured at ~6500 restarts/sec, each writing a log line. An - # OTP supervisor would cap this with max_restart_intensity; the owner restarts - # screens itself (it is the only thing that knows where one sat), so it has to - # carry the ceiling too. - @max_restarts 5 - @restart_window_ms 10_000 + # Navigation and the screen processes live in `Mob.Router`. These delegate, so + # callers keep the entry points they have always used. @doc """ Start a screen process linked to the calling process. @@ -210,23 +179,7 @@ defmodule Mob.Screen do `params` is passed as the first argument to `mount/3`. """ @spec start_link(module(), map(), keyword()) :: GenServer.on_start() - def start_link(screen_module, params, opts \\ []) do - GenServer.start_link(__MODULE__, {screen_module, params, :no_render, :android}, opts) - end - - @doc """ - Return the module of the currently active screen in the navigation stack. - Intended for testing and debugging. - """ - @spec get_current_module(pid()) :: module() - def get_current_module(pid), do: GenServer.call(pid, :get_current_module) - - @doc """ - Return the navigation history (list of `{module, socket}` pairs, head = most recent). - Intended for testing and debugging. - """ - @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t() | nil}] - def get_nav_history(pid), do: GenServer.call(pid, :get_nav_history) + defdelegate start_link(screen_module, params, opts \\ []), to: Mob.Router @doc """ Start a screen as the root UI screen. Calls mount, renders the component tree @@ -236,17 +189,14 @@ defmodule Mob.Screen do (no NIF calls). """ @spec start_root(module(), map(), keyword()) :: GenServer.on_start() - def start_root(screen_module, params \\ %{}, opts \\ []) do - platform = :mob_nif.platform() - GenServer.start_link(__MODULE__, {screen_module, params, :render, platform}, opts) - end + defdelegate start_root(screen_module, params \\ %{}, opts \\ []), to: Mob.Router @doc """ Dispatch a UI event to the screen process. Returns `:ok` synchronously once the event has been processed and the state updated. """ @spec dispatch(pid(), String.t(), map()) :: :ok - def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}, :infinity) + defdelegate dispatch(pid, event, params), to: Mob.Router @doc """ Return the current socket state of a running screen, or `nil` while that @@ -255,7 +205,21 @@ defmodule Mob.Screen do Intended for testing and debugging — not for production app logic. """ @spec get_socket(pid()) :: socket() | nil - def get_socket(pid), do: GenServer.call(pid, :get_socket) + defdelegate get_socket(pid), to: Mob.Router + + @doc """ + Return the module of the currently active screen in the navigation stack. + Intended for testing and debugging. + """ + @spec get_current_module(pid()) :: module() + defdelegate get_current_module(pid), to: Mob.Router + + @doc """ + Return the navigation history (list of `{module, socket}` pairs, head = most recent). + Intended for testing and debugging. + """ + @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t() | nil}] + defdelegate get_nav_history(pid), to: Mob.Router @doc """ Return the pid of the process owning the currently active screen. @@ -264,732 +228,5 @@ defmodule Mob.Screen do reaches the one that is on screen. """ @spec get_screen_pid(GenServer.server()) :: pid() - def get_screen_pid(pid), do: GenServer.call(pid, :get_screen_pid) - - # ── GenServer callbacks ─────────────────────────────────────────────────── - - @impl GenServer - def init({screen_module, params, render_mode, platform}) do - # Linked *and* trapping. Linking alone makes the owner die with any screen - # it stops or that crashes; trapping alone orphans every screen when the - # owner dies — and an orphaned persisted screen keeps dumping to - # Mob.ScreenState under the same key as its live replacement. Together the - # owner sees each exit as a message and screens still come down with it. - Process.flag(:trap_exit, true) - - 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() - # Started before the first render: that render is what bakes the - # listener's pid into the native tap handles. - Mob.Listener.ensure_started() - end - - # Seed the stacks this app declared. The screen we are about to mount - # becomes the active stack's current screen; every other declared stack - # stays unmounted until first visited. - nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) - - state = %{ - current: nil, - nav: nav, - render_mode: render_mode, - platform: platform, - screens: %{}, - restarts: %{} - } - - case start_screen(screen_module, params, state) do - {:ok, entry, state} -> - state = make_current(state, entry) - - if render_mode == :render do - # A notification that launched the app from a killed state. Sent to - # self so it arrives via handle_info after init returns, consistent - # with foreground notification delivery. - case :mob_nif.take_launch_notification() do - :none -> :ok - json -> send(self(), {:mob_launch_notification, json}) - end - - paint(entry, :none, state) - end - - {:ok, state} - - {:error, reason} -> - {:stop, reason} - end - end - - @impl GenServer - def handle_call({:event, event, params}, _from, state) do - # The whole point of MOB-112: a crash in the user's handle_event must not - # come back up this call and take the owner — and with it navigation and - # every sibling screen — down. The exit that follows restarts the screen. - case safe_call(fn -> Mob.Screen.Server.dispatch(state.current.pid, event, params) end) do - {:ok, {:ok, nav_action}} -> {:reply, :ok, apply_nav_action(nav_action, state, :sync)} - {:exit, _reason} -> {:reply, :ok, state} - end - end - - def handle_call({:navigate, nav_action}, _from, state) do - {:reply, :ok, apply_nav_action(nav_action, state, :sync)} - end - - def handle_call(:get_socket, _from, state) do - {:reply, current_socket(state), state} - end - - def handle_call(:get_screen_pid, _from, state) do - {:reply, state.current.pid, state} - end - - def handle_call(:get_current_module, _from, state) do - {:reply, state.current.module, state} - end - - def handle_call(:get_nav_history, _from, state) do - {:reply, Enum.map(Mob.Nav.history(state.nav), &entry_with_socket/1), state} - end - - def handle_call(:inspect, _from, state) do - socket = current_socket(state) - - # The tree is built in the screen's own process. Calling render/1 here would - # run user code in the owner, so a raise in it — reached by Mob.Test.tree/1, - # i.e. the debugging path — would kill navigation and every screen. - tree = - case safe_call(fn -> Mob.Screen.Server.tree(state.current.pid) end) do - {:ok, tree} -> tree - {:exit, _reason} -> nil - end - - info = %{ - screen: state.current.module, - assigns: socket && socket.assigns, - nav_history: Enum.map(Mob.Nav.history(state.nav), & &1.module), - tree: tree - } - - {:reply, info, state} - end - - @impl GenServer - def handle_cast(:__mob_hot_reload__, state) do - # A broadcast, not one cast: every live screen has to repaint with the - # newly loaded code, not just the one on screen. - Enum.each(all_entries(state), &Mob.Screen.Server.hot_reload(&1.pid)) - {:noreply, state} - end - - @impl GenServer - # A screen's callback asked to navigate. Only the active screen may drive - # navigation — a background screen's timer must not yank the stack out from - # under whatever the user is looking at. - def handle_info({:nav_action, action, from}, state) do - if from == state.current.pid do - {:noreply, apply_nav_action(action, state, :async)} - else - {:noreply, state} - end - end - - # A screen exited. Trapping turns this into a message rather than the owner's - # death — the isolation MOB-112 exists to deliver. A pid we have already - # dropped from `screens` was stopped deliberately, so its exit is expected. - def handle_info({:EXIT, pid, reason}, state) do - case Map.pop(state.screens, pid) do - {nil, _screens} -> {:noreply, state} - {entry, screens} -> {:noreply, restart_screen(entry, reason, %{state | screens: screens})} - end - end - - # A notification that launched the app from a killed state. - def handle_info({:mob_launch_notification, json}, state) do - # Decoded here rather than in the screen because the payload comes from - # native, not from a screen. :json.decode/1 raises on malformed input, and - # this runs in the owner — so a bad payload would take down every screen. - notification = - try do - decode_notification_json(json) - rescue - error -> - Logger.error( - "[mob] launch notification could not be decoded: " <> Exception.message(error) - ) - - %{source: :local, data: %{}} - end - - handle_info({:notification, notification}, state) - end - - # System back gesture (Android hardware/swipe, iOS edge-pan). Handled here so - # every screen gets back navigation without implementing anything. If a - # WebView is present and has internal history, navigate within it first. - def handle_info({:mob, :back}, state) do - if state.render_mode == :render && :mob_nif.webview_can_go_back() do - :mob_nif.webview_go_back() - {:noreply, state} - else - case {Mob.Nav.history(state.nav), Mob.Nav.back_target(state.nav)} do - {[_ | _], _} -> - {:noreply, apply_nav_action({:pop}, state, :async)} - - # Nothing left to pop on a secondary stack: fall back to the first one - # rather than exiting and discarding every parked stack. - {[], {:switch, target}} -> - {:noreply, apply_nav_action({:switch_tab, target}, state, :async)} - - {[], :exit} -> - if state.render_mode == :render, do: :mob_nif.exit_app() - {:noreply, state} - end - end - end - - # Anything else addressed to :mob_screen — device events, notifications, - # plugin messages — belongs to the screen the user is looking at. - def handle_info(message, state) do - send(state.current.pid, message) - {:noreply, state} - end - - @impl GenServer - def terminate(_reason, _state) do - # Deliberately does NOT stop screens. They are linked and trap exits, so - # each shuts down gracefully on its own — gen_server turns the parent's EXIT - # into a terminate/2 call, which runs the user's terminate/2 and the final - # state dump. - # - # Calling GenServer.stop/3 here instead corrupts the owner's own exit: the - # screen's :shutdown travels back while the owner is mid-terminate and - # replaces its reason, so a clean stop(owner, :normal) exits :shutdown. - :ok - end - - # ── Screen lifecycle ────────────────────────────────────────────────────── - - # Calling into a screen that is mid-crash must not propagate into the owner. - # The exit signal that follows is what actually repairs the screen. - defp safe_call(fun) do - {:ok, fun.()} - catch - :exit, reason -> {:exit, reason} - end - - defp start_screen(module, params, state), do: start_screen(module, params, make_ref(), state) - - defp start_screen(module, params, ref, state) do - opts = [ - module: module, - params: params, - ref: ref, - owner: self(), - render_mode: state.render_mode, - platform: state.platform - ] - - case Mob.Screen.Server.start_link(opts) do - {:ok, pid} -> - entry = %{module: module, pid: pid, params: params, ref: ref} - {:ok, entry, %{state | screens: Map.put(state.screens, pid, entry)}} - - {:error, reason} -> - {:error, reason} - end - end - - # The single place `current` changes. The sender is told here and nowhere - # else, so only the screen the user is looking at can commit a frame. - defp make_current(state, entry) do - Mob.Sender.set_active(entry.ref) - %{state | current: entry} - end - - defp all_entries(state) do - parked = - state.nav - |> Map.get(:parked, %{}) - |> Enum.flat_map(fn {_name, %{current: current, history: history}} -> - [current | history] - end) - - ([state.current] ++ Mob.Nav.history(state.nav) ++ parked) - |> Enum.reject(&is_nil/1) - |> Enum.uniq_by(& &1.pid) - end - - defp current_socket(state) do - case safe_call(fn -> Mob.Screen.Server.socket(state.current.pid) end) do - {:ok, socket} -> socket - {:exit, _reason} -> nil - end - end - - defp entry_with_socket(entry) do - case safe_call(fn -> Mob.Screen.Server.socket(entry.pid) end) do - {:ok, socket} -> {entry.module, socket} - {:exit, _reason} -> {entry.module, nil} - end - end - - # A crashed screen is re-mounted in place, with the params and stack ref it - # was created with. It loses its assigns — a restart runs mount/3 again — - # which is the documented consequence of the isolation. - defp restart_screen(entry, reason, state) do - {allowed?, state} = record_restart(entry.ref, state) - - if allowed? do - do_restart_screen(entry, reason, state) - else - Logger.error( - "[mob] screen #{inspect(entry.module)} crashed #{@max_restarts + 1} times in " <> - "#{@restart_window_ms}ms and is being given up on rather than restarted in a loop. " <> - "Reason: #{inspect(reason)}" - ) - - recover_from_failed_restart(entry, state) - end - end - - # Sliding window per screen ref, so the ceiling follows a logical screen - # across its restarts rather than resetting with each new pid. - defp record_restart(ref, state) do - now = System.monotonic_time(:millisecond) - recent = Map.get(state.restarts, ref, []) |> Enum.filter(&(now - &1 < @restart_window_ms)) - state = %{state | restarts: Map.put(state.restarts, ref, [now | recent])} - {length(recent) < @max_restarts, state} - end - - defp do_restart_screen(%{pid: dead_pid} = entry, reason, state) do - log_restart(entry.module, reason) - - case start_screen(entry.module, entry.params, entry.ref, state) do - {:ok, new_entry, state} -> - state = substitute(state, dead_pid, new_entry) - if state.current.pid == new_entry.pid, do: paint(new_entry, :none, state) - state - - {:error, mount_reason} -> - # Re-mounting failed, so the screen cannot come back. Leaving the dead - # entry in place would freeze the app silently — every later event - # would call a corpse and return :ok. Pop to whatever is underneath if - # there is anything; otherwise say so loudly rather than pretend. - log_restart_failure(entry.module, mount_reason) - recover_from_failed_restart(entry, state) - end - end - - defp substitute(state, dead_pid, new_entry) do - replace = fn - %{pid: ^dead_pid} -> new_entry - other -> other - end - - nav = - state.nav - |> Mob.Nav.put_history(Enum.map(Mob.Nav.history(state.nav), replace)) - |> Mob.Nav.map_parked(replace) - - current = if state.current.pid == dead_pid, do: new_entry, else: state.current - %{state | nav: nav, current: current} - end - - defp recover_from_failed_restart(%{pid: dead_pid} = entry, state) do - cond do - state.current.pid != dead_pid -> - # A background screen. Drop it from the stack it sat in rather than - # leave a corpse for a later pop or tab switch to restore. - drop_entry(state, dead_pid) - - Mob.Nav.history(state.nav) != [] -> - state = drop_entry(state, dead_pid) - [previous | rest] = Mob.Nav.history(state.nav) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) - paint(previous, :pop, state) - state - - true -> - fall_back_to_parked_stack(entry, state) - end - end - - # "Nothing beneath it" is the *common* shape in a tab-bar app — every tab root - # has an empty history — and there is usually a live screen parked under - # another tab. Leaving a dead pid as `current` bricks the app: every later - # event is safe_call'd into a corpse and replies :ok, while a perfectly good - # screen sits parked one call away. - defp fall_back_to_parked_stack(entry, state) do - with [name | _] <- Mob.Nav.parked_stacks(state.nav), - {:switched, nav, live} <- Mob.Nav.switch(state.nav, name, entry) do - # switch/3 parks the dead entry under the outgoing stack; drop it straight - # after, so nothing can restore a corpse by switching back. - nav = Mob.Nav.drop_parked(nav, &(&1.pid == entry.pid)) - state = make_current(%{state | nav: nav}, live) - paint(live, :pop, state) - state - else - _ -> - Logger.error( - "[mob] #{inspect(entry.module)} could not be restarted and there is no other live " <> - "screen to fall back to. The app has no live screen." - ) - - state - end - end - - defp drop_entry(state, dead_pid) do - dead? = fn %{pid: pid} -> pid == dead_pid end - - nav = - state.nav - |> Mob.Nav.put_history(Enum.reject(Mob.Nav.history(state.nav), dead?)) - |> Mob.Nav.drop_parked(dead?) - - %{state | nav: nav} - end - - defp log_restart(module, reason) do - Logger.error( - "[mob] screen #{inspect(module)} crashed and is being restarted; its assigns are lost. " <> - "Reason: #{inspect(reason)}" - ) - end - - defp log_restart_failure(module, reason) do - Logger.error( - "[mob] screen #{inspect(module)} could not be restarted — mount/3 failed: " <> - "#{inspect(reason)}" - ) - end - - defp paint(entry, transition, state), do: do_paint(entry, transition, state, :async) - - defp do_paint(_entry, _transition, %{render_mode: :no_render}, _mode), do: :ok - - defp do_paint(entry, transition, _state, :sync) do - # Unprotected, this is the other way a screen crash killed the owner: the - # user's render/1 runs inside the screen, and a raise there exits this call. - case safe_call(fn -> Mob.Screen.Server.render_sync(entry.pid, transition) end) do - {:ok, _} -> :ok - {:exit, _reason} -> :ok - end - end - - defp do_paint(entry, transition, _state, :async), - do: Mob.Screen.Server.render(entry.pid, transition) - - # Drop the entry from tracking BEFORE stopping, so the exit we asked for is - # recognised as deliberate rather than restarted as a crash. - defp stop_screen(%{pid: pid} = entry, state) do - # Forget the restart history too — the map is keyed by screen ref and - # nothing else ever removes a key, so it grows for the owner's lifetime. - state = %{ - state - | screens: Map.delete(state.screens, pid), - restarts: Map.delete(state.restarts, entry.ref) - } - - stop_process(pid) - state - end - - defp stop_process(pid) do - if Process.alive?(pid) do - # Unlink first. We are discarding this screen deliberately, so its - # :shutdown exit must not travel back up the link — during the owner's own - # terminate/2 that signal overrides the owner's exit reason, which turns a - # clean GenServer.stop(owner, :normal) into an exit with :shutdown. - Process.unlink(pid) - - try do - GenServer.stop(pid, :shutdown, @stop_timeout_ms) - catch - # A wedged screen ignores the shutdown request. GenServer.stop/3 kills - # only its own proxy on timeout, so without this the screen survives — - # unlinked, untracked, and still dumping to Mob.ScreenState under the - # same key as its replacement. That is the orphan hazard linking exists - # to prevent. - :exit, _reason -> Process.exit(pid, :kill) - end - end - - :ok - end - - # ── Navigation ──────────────────────────────────────────────────────────── - - defp apply_nav_action(nil, state, _mode), do: state - - defp apply_nav_action({:push, dest, params}, state, mode) do - with {:ok, new_module, route_params} <- safe_resolve(dest, state) do - push_resolved(new_module, Map.merge(route_params, params), state, mode) - end - end - - defp apply_nav_action({:pop}, state, mode) do - case Mob.Nav.history(state.nav) do - [previous | rest] -> - # The screen being popped off leaves the stack for good, so its process - # goes with it. The ones still in `rest` stay resident — that is what - # makes pop restore prior state without re-mounting. - state = stop_screen(state.current, state) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) - do_paint(previous, :pop, state, mode) - state - - [] -> - repaint_current(state, mode) - end - end - - defp apply_nav_action({:pop_to_root}, state, mode) do - case Enum.reverse(Mob.Nav.history(state.nav)) do - [root | _] -> - discarded = [ - state.current | Enum.reject(Mob.Nav.history(state.nav), &(&1.pid == root.pid)) - ] - - state = Enum.reduce(discarded, state, &stop_screen/2) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, root) - do_paint(root, :pop, state, mode) - state - - [] -> - repaint_current(state, mode) - end - end - - defp apply_nav_action({:pop_to, dest}, state, mode) do - with {:ok, target, _params} <- safe_resolve(dest, state) do - pop_to_resolved(target, state, mode) - end - end - - defp apply_nav_action({:reset, dest, params}, state, mode) do - with {:ok, new_module, route_params} <- safe_resolve(dest, state) do - reset_resolved(new_module, Map.merge(route_params, params), state, mode) - end - end - - defp apply_nav_action({:switch_tab, tab}, state, mode) do - case Mob.Nav.switch(state.nav, tab, state.current) do - {:switched, nav, entry} -> - state = make_current(%{state | nav: nav}, entry) - do_paint(entry, :none, state, mode) - state - - {:mount_root, nav, root_module} -> - # Start first, mutate after. Switching nav before the mount could fail - # leaves navigation pointing at a stack whose screen never started. - case start_screen(root_module, %{}, state) do - {:ok, entry, state} -> - state = make_current(%{state | nav: nav}, entry) - do_paint(entry, :none, state, mode) - state - - {:error, _reason} -> - repaint_current(state, mode) - end - - :noop -> - repaint_current(state, mode) - end - end - - defp push_resolved(new_module, mount_params, state, mode) do - case start_screen(new_module, mount_params, state) do - {:ok, entry, state} -> - nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) - state = make_current(%{state | nav: nav}, entry) - do_paint(entry, :push, state, mode) - state - - {:error, _reason} -> - repaint_current(state, mode) - end - end - - defp reset_resolved(new_module, mount_params, state, mode) do - case start_screen(new_module, mount_params, state) do - {:ok, entry, state} -> - discarded = [state.current | Mob.Nav.history(state.nav)] - state = Enum.reduce(discarded, state, &stop_screen/2) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry) - do_paint(entry, :reset, state, mode) - state - - {:error, _reason} -> - repaint_current(state, mode) - end - end - - defp pop_to_resolved(target, state, mode) do - history = Mob.Nav.history(state.nav) - - case pop_to_module(history, target) do - {:found, previous, rest} -> - discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] - state = Enum.reduce(discarded, state, &stop_screen/2) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) - do_paint(previous, :pop, state, mode) - state - - :not_found -> - repaint_current(state, mode) - end - end - - # A nav action that changed nothing still has to paint. The screen deliberately - # does not paint when it produced an action, so without this the assigns it set - # in the same callback would never reach the screen — re-tapping the active tab - # being the everyday case. - defp repaint_current(state, mode) do - do_paint(state.current, :none, state, mode) - state - end - - # `push_screen/2` and friends take any atom, and an unregistered one raises. - # That raise runs in the OWNER, where it would kill navigation and every - # screen — falsifying the isolation this module's moduledoc promises, over a - # typo. A bad destination leaves navigation untouched and repaints instead. - # - # Returns the state unchanged (via repaint_current/2) rather than an error - # tuple, so the `with` in each caller falls straight through. - defp safe_resolve(dest, state) do - {module, route_params} = resolve_destination(dest) - {:ok, module, route_params} - rescue - error -> - Logger.error( - "[mob] navigation to #{inspect(dest)} failed and was ignored: " <> - Exception.message(error) - ) - - repaint_current(state, :async) - end - - # Resolves a navigation destination to {module, route_params}. A loaded - # module navigates directly (no route-bound params); a registered route atom - # may carry params (Mob.Nav.Registry.register/3 — the data-driven-plugin - # pattern), merged UNDER the caller's push params at the mount call site. - defp resolve_destination(dest) when is_atom(dest) do - case Code.ensure_loaded(dest) do - {:module, ^dest} -> - {dest, %{}} - - _ -> - case Mob.Nav.Registry.lookup_route(dest) do - {:ok, module, route_params} -> - {module, route_params} - - {:error, :not_found} -> - raise ArgumentError, - "Mob.Screen: unknown navigation destination #{inspect(dest)}. " <> - "Register it via Mob.Nav.Registry.register/2 or declare it in " <> - "your App.navigation/1." - end - end - end - - defp pop_to_module([], _target), do: :not_found - - defp pop_to_module([%{module: module} = entry | rest], target) do - if module == target do - {:found, entry, rest} - else - pop_to_module(rest, target) - end - end - - # ── Helpers ─────────────────────────────────────────────────────────────── - - @doc false - # Public only so Mob.Screen.Server can reuse it — the decoding belongs with - # the rest of the native-event translation, not duplicated per process. - @spec decode_file_result(String.t(), String.t(), binary()) :: tuple() - def decode_file_result(event, sub, json_binary) do - event_atom = String.to_atom(event) - sub_atom = String.to_atom(sub) - - items = - case :json.decode(json_binary) do - list when is_list(list) -> - Enum.map(list, fn item when is_map(item) -> - Map.new(item, fn {k, v} -> {String.to_atom(k), v} end) - end) - - _ -> - [] - end - - case {event_atom, sub_atom} do - {:camera, :photo} -> - {:camera, :photo, List.first(items) || %{}} - - {:camera, :video} -> - {:camera, :video, List.first(items) || %{}} - - {:camera, :cancelled} -> - {:camera, :cancelled} - - {:photos, :picked} -> - {:photos, :picked, items} - - {:files, :picked} -> - {:files, :picked, items} - - {:audio, :recorded} -> - {:audio, :recorded, List.first(items) || %{}} - - {:storage, :saved_to_library} -> - {:storage, :saved_to_library, (List.first(items) || %{})[:path]} - - {:scan, :result} -> - scan_result(List.first(items) || %{}) - - _ -> - {event_atom, sub_atom, items} - end - end - - defp scan_result(item) do - {:scan, :result, %{type: to_atom_safe(item[:type]), value: item[:value]}} - end - - defp to_atom_safe(nil), do: :qr - defp to_atom_safe(s) when is_binary(s), do: String.to_atom(s) - defp to_atom_safe(a) when is_atom(a), do: a - - defp decode_notification_json(json) when is_binary(json) do - case :json.decode(json) do - map when is_map(map) -> - source = - case Map.get(map, "source", "local") do - "push" -> :push - _ -> :local - end - - data = - case Map.get(map, "data") do - d when is_map(d) -> Map.new(d, fn {k, v} -> {String.to_atom(k), v} end) - _ -> %{} - end - - %{ - id: Map.get(map, "id"), - title: Map.get(map, "title"), - body: Map.get(map, "body"), - data: data, - source: source - } - - _ -> - %{source: :local, data: %{}} - end - end + defdelegate get_screen_pid(pid), to: Mob.Router end diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex index 6f378425..7d02aa4a 100644 --- a/lib/mob/screen/server.ex +++ b/lib/mob/screen/server.ex @@ -205,7 +205,7 @@ defmodule Mob.Screen.Server do # Android file/camera/photo/scan results arrive JSON-encoded; decode and # re-dispatch as the user-facing event tuple. def handle_info({:mob_file_result, event, sub, json_binary}, state) do - handle_info(Mob.Screen.decode_file_result(event, sub, json_binary), state) + handle_info(decode_file_result(event, sub, json_binary), state) end # A few Peripheral.* events carry JSON-encoded device records; the @@ -345,6 +345,60 @@ defmodule Mob.Screen.Server do end end + # Android file/camera/photo/scan results arrive JSON-encoded from native. + defp decode_file_result(event, sub, json_binary) do + event_atom = String.to_atom(event) + sub_atom = String.to_atom(sub) + + items = + case :json.decode(json_binary) do + list when is_list(list) -> + Enum.map(list, fn item when is_map(item) -> + Map.new(item, fn {k, v} -> {String.to_atom(k), v} end) + end) + + _ -> + [] + end + + case {event_atom, sub_atom} do + {:camera, :photo} -> + {:camera, :photo, List.first(items) || %{}} + + {:camera, :video} -> + {:camera, :video, List.first(items) || %{}} + + {:camera, :cancelled} -> + {:camera, :cancelled} + + {:photos, :picked} -> + {:photos, :picked, items} + + {:files, :picked} -> + {:files, :picked, items} + + {:audio, :recorded} -> + {:audio, :recorded, List.first(items) || %{}} + + {:storage, :saved_to_library} -> + {:storage, :saved_to_library, (List.first(items) || %{})[:path]} + + {:scan, :result} -> + scan_result(List.first(items) || %{}) + + _ -> + {event_atom, sub_atom, items} + end + end + + defp scan_result(item) do + {:scan, :result, %{type: to_atom_safe(item[:type]), value: item[:value]}} + end + + defp to_atom_safe(nil), do: :qr + defp to_atom_safe(s) when is_binary(s), do: String.to_atom(s) + defp to_atom_safe(a) when is_atom(a), do: a + defp schedule_state_sync do Process.send_after(self(), :__mob_sync_state__, @state_sync_interval_ms) end diff --git a/test/mob/nav/screen_nav_test.exs b/test/mob/nav/screen_nav_test.exs index 4ce30577..76e1cb75 100644 --- a/test/mob/nav/screen_nav_test.exs +++ b/test/mob/nav/screen_nav_test.exs @@ -3,6 +3,15 @@ defmodule Mob.Nav.ScreenNavTest do import ExUnit.CaptureLog + # `if Process.alive?, do: GenServer.stop` races: the router dies with the test + # process, so it can exit between the check and the stop and fail the test + # from inside the on_exit runner. + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + # ── Screen fixtures ──────────────────────────────────────────────────────── # Bare module names inside nested defmodule blocks don't auto-alias to siblings. # Use module attributes with fully qualified names for cross-screen references. @@ -87,7 +96,7 @@ defmodule Mob.Nav.ScreenNavTest do end {:ok, pid} = Mob.Nav.Registry.start_link(DemoApp) - on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + on_exit(fn -> stop_safely(pid) end) :ok end @@ -291,7 +300,7 @@ defmodule Mob.Nav.ScreenNavTest do # would take down every screen — over a typo in push_screen/2. It is # caught: navigation is left untouched and the app carries on. {:ok, pid} = Mob.Screen.start_link(UnknownNavScreen, %{}) - on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + on_exit(fn -> stop_safely(pid) end) log = capture_log(fn -> assert :ok = Mob.Screen.dispatch(pid, "bad_nav", %{}) end) diff --git a/test/mob/router_hot_path_test.exs b/test/mob/router_hot_path_test.exs new file mode 100644 index 00000000..aa481cbe --- /dev/null +++ b/test/mob/router_hot_path_test.exs @@ -0,0 +1,157 @@ +defmodule Mob.RouterHotPathTest do + @moduledoc """ + The router must not be in the per-message path. + + This is the constraint MOB-113 exists to guarantee and the reason one process + per screen is affordable at all: an earlier costing of this design assumed a + router in the loop and concluded per-screen processes could not escape a hop + per message. + + Asserted by tracing the router's mailbox rather than by reasoning about the + code, so it keeps holding when someone adds a message. + """ + use ExUnit.Case, async: false + + defmodule HomeScreen do + use Mob.Screen + + @detail Mob.RouterHotPathTest.DetailScreen + + 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_info({:tap, :bump}, socket), + do: {:noreply, Mob.Socket.assign(socket, :count, socket.assigns.count + 1)} + + def handle_info({:change, :field, value}, socket), + do: {:noreply, Mob.Socket.assign(socket, :typed, value)} + + def handle_info({:tap, :go}, socket), + do: {:noreply, Mob.Socket.push_screen(socket, @detail)} + + def handle_info(_msg, socket), do: {:noreply, socket} + end + + defmodule DetailScreen do + use Mob.Screen + def mount(_params, _session, socket), do: {:ok, socket} + def render(_assigns), do: %{type: :text, props: %{text: "detail"}, children: []} + end + + defmodule DemoApp do + @behaviour Mob.App + import Mob.App + @home Mob.RouterHotPathTest.HomeScreen + def navigation(_), do: stack(:home, root: @home) + end + + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + + setup do + case Process.whereis(Mob.Nav.Registry) do + nil -> :ok + pid -> GenServer.stop(pid) + end + + {:ok, registry} = Mob.Nav.Registry.start_link(DemoApp) + on_exit(fn -> stop_safely(registry) end) + + {:ok, router} = Mob.Screen.start_link(HomeScreen, %{}) + on_exit(fn -> stop_safely(router) end) + + screen = Mob.Screen.get_screen_pid(router) + %{router: router, screen: screen} + end + + # Trace the router's receives while `fun` runs, and return what it got. + # Nothing may call the router during the window — that would be a message too. + defp router_messages(router, fun) do + tracer = self() + :erlang.trace(router, true, [:receive, {:tracer, tracer}]) + + try do + fun.() + after + :erlang.trace(router, false, [:receive]) + end + + collect_trace([]) + end + + defp collect_trace(acc) do + receive do + {:trace, _pid, :receive, msg} -> collect_trace([msg | acc]) + after + 50 -> Enum.reverse(acc) + end + end + + describe "the router is not in the per-message path" do + test "an ordinary tap on the active screen never reaches it", %{ + router: router, + screen: screen + } do + # What Mob.Listener does on a real tap: straight to the screen's own pid. + messages = + router_messages(router, fn -> + send(screen, {:tap, :bump}) + :sys.get_state(screen) + end) + + assert messages == [] + assert Mob.Screen.get_socket(router).assigns.count == 1 + end + + test "a value-carrying event never reaches it either", %{router: router, screen: screen} do + messages = + router_messages(router, fn -> + send(screen, {:change, :field, "hello"}) + :sys.get_state(screen) + end) + + assert messages == [] + end + + test "a burst of messages produces no router traffic at all", %{ + router: router, + screen: screen + } do + messages = + router_messages(router, fn -> + for _ <- 1..50, do: send(screen, {:tap, :bump}) + :sys.get_state(screen) + end) + + assert messages == [] + assert Mob.Screen.get_socket(router).assigns.count == 50 + end + end + + describe "navigation does reach the router" do + test "a nav action from a screen callback arrives", %{router: router, screen: screen} do + messages = + router_messages(router, fn -> + send(screen, {:tap, :go}) + :sys.get_state(screen) + # Give the router a moment to receive the forwarded action. + Process.sleep(20) + end) + + assert Enum.any?(messages, &match?({:nav_action, {:push, _, _}, ^screen}, &1)), + "expected a {:nav_action, …} from the screen, got: #{inspect(messages)}" + end + + test "and the navigation actually happens", %{router: router, screen: screen} do + send(screen, {:tap, :go}) + :sys.get_state(screen) + :sys.get_state(router) + + assert Mob.Screen.get_current_module(router) == DetailScreen + assert [{HomeScreen, _}] = Mob.Screen.get_nav_history(router) + end + end +end From a22ab513dd554cf14644a3eedf635417f0e7f398 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 29 Aug 2026 04:52:39 -0600 Subject: [PATCH 2/2] MOB-113: cover the render half of the hot path, and finish the rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the extraction faithful — three hunks, every clause and attribute intact, all seven entry points at every master arity — and one real problem, which was mine: the test pinning this step's headline property was blind to half the path. Mob.Screen.Server.paint/3 short-circuits under :no_render, which is the only mode host tests could reach. So tree expansion, ComponentRegistry.reconcile/2 and the hand-off to Mob.Sender never ran under trace, and a router hop added to paint/3 passed the entire suite. That is exactly where a future hop would appear — an "am I still active?" check in the render body is the obvious shape of one. The reviewer offered qualifying the claim or filing follow-up. Neither is good enough for a commit whose deliverable is the test, so the gap is closed instead: Mob.Screen.Server takes its NIF module as an option, defaulting to :mob_nif. That is not test-only scaffolding — Mob.Renderer and Mob.Sender already take it as a parameter and the screen was the outlier that hardcoded it. With a stub NIF the test drives real renders off-device. Both halves now bite, verified as negative controls: a hop in forward/2 fails the three callback tests, a hop in paint/3 fails the two render tests. The value-carrying test also gained the positive assertion it was missing — it could previously have passed with its handler deleted. Also from review, all documentation the rename left behind: screen/server.ex's moduledoc still named Mob.Screen as the navigation owner (in a file this commit edits, which would have been the third wrong moduledoc in this area), the same in nav.ex and sender.ex, the comment twin in nav_test.exs that the commit fixed in lib/ but not test/, and a user-facing ArgumentError still prefixed "Mob.Screen:" for code that now lives in Mob.Router. Nav's moduledoc also still described the pre-MOB-112 shape, claiming the current screen lives in "Mob.Screen's {module, socket}". Tests: 7 in the hot-path file, up from 5. Suite 1240 passed, 8/8 clean runs, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../2026-08-29-router-off-the-hot-path.md | 22 ++++-- lib/mob/nav.ex | 17 ++--- lib/mob/router.ex | 24 ++++--- lib/mob/screen/server.ex | 32 +++++---- lib/mob/sender.ex | 6 +- test/mob/nav_test.exs | 2 +- test/mob/router_hot_path_test.exs | 71 +++++++++++++++++++ 7 files changed, 132 insertions(+), 42 deletions(-) diff --git a/decisions/2026-08-29-router-off-the-hot-path.md b/decisions/2026-08-29-router-off-the-hot-path.md index c889e4c5..4bbe7a6c 100644 --- a/decisions/2026-08-29-router-off-the-hot-path.md +++ b/decisions/2026-08-29-router-off-the-hot-path.md @@ -33,10 +33,24 @@ events straight to the owning screen's pid, so a tap goes native → listener screen → sender with the router uninvolved. What is new is that it is now **asserted rather than reasoned about**. -`test/mob/router_hot_path_test.exs` traces `:receive` on the router across a -tap, a value-carrying event, and a burst of fifty messages, and asserts the -trace is empty. A negative control confirms it bites: making the screen notify -the router on each message fails exactly those three tests. +`test/mob/router_hot_path_test.exs` traces `:receive` on the router and asserts +the trace is empty. + +It covers both halves of the path, which took a change to make possible. The +callback half (`handle_info` into user code) runs under `:no_render`. The render +half — tree expansion, `Mob.ComponentRegistry.reconcile/2`, the hand-off to +`Mob.Sender` — is skipped entirely by `:no_render`, and `:render` needs a NIF. +The first version of this test therefore proved nothing about the half where a +hop is *most* likely to appear: an "am I still active?" check in the render body +is the obvious shape of one. A router hop added to `paint/3` passed the whole +suite. + +`Mob.Screen.Server` now takes its NIF module as an option, defaulting to +`:mob_nif`. That is not test-only scaffolding — `Mob.Renderer` and `Mob.Sender` +already take it as a parameter, and the screen was the outlier that hardcoded +it. With a stub NIF the test drives real renders off-device, and negative +controls confirm both halves bite: a hop in `forward/2` fails the callback +tests, a hop in `paint/3` fails the render tests. This matters more than a tidy-up. An earlier costing of this architecture assumed a router in the loop and concluded per-screen processes could not escape diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index a41659df..1284f5fe 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -15,16 +15,13 @@ defmodule Mob.Nav do ## Shape The *active* stack's current screen is deliberately **not** stored here. It - lives where it always did, in `Mob.Screen`'s `{module, socket}`, and this - struct holds only the active stack's `history` plus the fully parked state of - every inactive stack. Keeping the hot path untouched is the point: an ordinary - message to the active screen reads and writes the same two variables it did - before, and only `switch/3` moves state in or out of `parked`. + lives in `Mob.Router`'s `current`, and this struct holds only the active + stack's `history` plus the fully parked state of every inactive stack. Only + `switch/3` moves state in or out of `parked`. * `active` — name of the stack the current screen belongs to (`nil` when the app declares no stacks at all, i.e. a bare `start_root/1` with no layout) - * `history` — the active stack's history, head = most recent, exactly the list - `Mob.Screen` used to hold + * `history` — the active stack's history, head = most recent * `parked` — `%{name => %{current: entry, history: [entry]}}` for inactive stacks. Never contains `active`. * `order` — declared stack order, for tab-index mapping @@ -41,7 +38,7 @@ defmodule Mob.Nav do @typedoc """ Whatever the caller uses to identify a screen. Opaque here. - `Mob.Screen` puts `%{module:, pid:, params:, ref:}` in these slots since + `Mob.Router` puts `%{module:, pid:, params:, ref:}` in these slots since MOB-112 — this module never looks inside one. """ @type entry :: term() @@ -143,7 +140,7 @@ defmodule Mob.Nav do every entry in its history. Entries are opaque to this module, so the caller decides what an entry is and - what replacing one means. `Mob.Screen` uses it to substitute a restarted + what replacing one means. `Mob.Router` uses it to substitute a restarted screen process wherever it was referenced. The *active* stack's history is not covered: it lives in `history`, which the @@ -173,7 +170,7 @@ defmodule Mob.Nav do all is removed from `parked` entirely, so the next switch to it mounts its root fresh rather than restoring a screen that is gone. - `Mob.Screen` uses this when a screen crashes and cannot be re-mounted — + `Mob.Router` uses this when a screen crashes and cannot be re-mounted — leaving the dead entry in place would freeze that tab permanently, since switching to it would restore a corpse. """ diff --git a/lib/mob/router.ex b/lib/mob/router.ex index 8c850aa2..052dc8fc 100644 --- a/lib/mob/router.ex +++ b/lib/mob/router.ex @@ -64,7 +64,8 @@ defmodule Mob.Router do """ @spec start_link(module(), map(), keyword()) :: GenServer.on_start() def start_link(screen_module, params, opts \\ []) do - GenServer.start_link(__MODULE__, {screen_module, params, :no_render, :android}, opts) + {nif, opts} = Keyword.pop(opts, :nif, :mob_nif) + GenServer.start_link(__MODULE__, {screen_module, params, :no_render, :android, nif}, opts) end @doc """ @@ -90,8 +91,9 @@ defmodule Mob.Router do """ @spec start_root(module(), map(), keyword()) :: GenServer.on_start() def start_root(screen_module, params \\ %{}, opts \\ []) do - platform = :mob_nif.platform() - GenServer.start_link(__MODULE__, {screen_module, params, :render, platform}, opts) + {nif, opts} = Keyword.pop(opts, :nif, :mob_nif) + platform = nif.platform() + GenServer.start_link(__MODULE__, {screen_module, params, :render, platform, nif}, opts) end @doc """ @@ -122,7 +124,7 @@ defmodule Mob.Router do # ── GenServer callbacks ─────────────────────────────────────────────────── @impl GenServer - def init({screen_module, params, render_mode, platform}) do + def init({screen_module, params, render_mode, platform, nif}) do # Linked *and* trapping. Linking alone makes the owner die with any screen # it stops or that crashes; trapping alone orphans every screen when the # owner dies — and an orphaned persisted screen keeps dumping to @@ -149,6 +151,7 @@ defmodule Mob.Router do nav: nav, render_mode: render_mode, platform: platform, + nif: nif, screens: %{}, restarts: %{} } @@ -161,7 +164,7 @@ defmodule Mob.Router do # A notification that launched the app from a killed state. Sent to # self so it arrives via handle_info after init returns, consistent # with foreground notification delivery. - case :mob_nif.take_launch_notification() do + case nif.take_launch_notification() do :none -> :ok json -> send(self(), {:mob_launch_notification, json}) end @@ -283,8 +286,8 @@ defmodule Mob.Router do # every screen gets back navigation without implementing anything. If a # WebView is present and has internal history, navigate within it first. def handle_info({:mob, :back}, state) do - if state.render_mode == :render && :mob_nif.webview_can_go_back() do - :mob_nif.webview_go_back() + if state.render_mode == :render && state.nif.webview_can_go_back() do + state.nif.webview_go_back() {:noreply, state} else case {Mob.Nav.history(state.nav), Mob.Nav.back_target(state.nav)} do @@ -297,7 +300,7 @@ defmodule Mob.Router do {:noreply, apply_nav_action({:switch_tab, target}, state, :async)} {[], :exit} -> - if state.render_mode == :render, do: :mob_nif.exit_app() + if state.render_mode == :render, do: state.nif.exit_app() {:noreply, state} end end @@ -342,7 +345,8 @@ defmodule Mob.Router do ref: ref, owner: self(), render_mode: state.render_mode, - platform: state.platform + platform: state.platform, + nif: state.nif ] case Mob.Screen.Server.start_link(opts) do @@ -742,7 +746,7 @@ defmodule Mob.Router do {:error, :not_found} -> raise ArgumentError, - "Mob.Screen: unknown navigation destination #{inspect(dest)}. " <> + "Mob.Router: unknown navigation destination #{inspect(dest)}. " <> "Register it via Mob.Nav.Registry.register/2 or declare it in " <> "your App.navigation/1." end diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex index 7d02aa4a..e839dea2 100644 --- a/lib/mob/screen/server.ex +++ b/lib/mob/screen/server.ex @@ -8,9 +8,9 @@ defmodule Mob.Screen.Server do navigation and every other screen with it — the isolation `Mob.Screen`'s moduledoc claimed and mob#76 had to write around. - Now `Mob.Screen` owns navigation and starts one of these per live screen. A - crash here kills this screen only; the owner sees the `:DOWN`, restarts it, - and re-renders. + `Mob.Router` owns navigation and starts one of these per live screen. A crash + here kills this screen only; the router sees the exit, restarts it, and + re-renders. ## `self()` means what users already assume @@ -55,7 +55,7 @@ defmodule Mob.Screen.Server do """ @type render_ref :: reference() - defstruct [:module, :socket, :render_mode, :ref, :owner] + defstruct [:module, :socket, :render_mode, :ref, :owner, :nif] @doc """ Start a screen linked to the calling process. @@ -63,7 +63,7 @@ defmodule Mob.Screen.Server do `:owner` receives nav actions and the exit signal. `:ref` identifies this screen to `Mob.Sender` and is unique per screen — see `t:render_ref/0`. - `Mob.Screen` links *and* traps exits. Linking alone would make the owner die + `Mob.Router` links *and* traps exits. Linking alone would make the owner die with any screen it stopped or that crashed; trapping alone would leave every screen orphaned when the owner died — and an orphaned persisted screen keeps dumping to `Mob.ScreenState` under the same key as its live replacement. @@ -126,11 +126,14 @@ defmodule Mob.Screen.Server do module = Keyword.fetch!(opts, :module) render_mode = Keyword.get(opts, :render_mode, :no_render) platform = Keyword.get(opts, :platform, :android) + # Injectable for the same reason Mob.Renderer and Mob.Sender take it as a + # parameter: without it nothing can exercise the render path off-device. + nif = Keyword.get(opts, :nif, :mob_nif) socket = module |> Mob.Socket.new(platform: platform) - |> Mob.Socket.assign(:safe_area, initial_safe_area(render_mode)) + |> Mob.Socket.assign(:safe_area, initial_safe_area(render_mode, nif)) case module.mount(Keyword.get(opts, :params, %{}), %{}, socket) do {:ok, mounted} -> @@ -144,7 +147,8 @@ defmodule Mob.Screen.Server do socket: socket, render_mode: render_mode, ref: Keyword.get(opts, :ref, :__mob_single__), - owner: Keyword.fetch!(opts, :owner) + owner: Keyword.fetch!(opts, :owner), + nif: nif }} {:error, reason} -> @@ -285,7 +289,7 @@ defmodule Mob.Screen.Server do defp paint(%{render_mode: :no_render} = state, _transition, _mode), do: state.socket defp paint(state, transition, mode) do - socket = ensure_safe_area(state.socket, state.socket.__mob__.platform) + socket = ensure_safe_area(state.socket, state.socket.__mob__.platform, state.nif) platform = socket.__mob__.platform list_renderers = Map.get(socket.__mob__, :list_renderers, %{}) @@ -298,26 +302,26 @@ defmodule Mob.Screen.Server do |> Mob.Component.expand(self(), platform) Mob.ComponentRegistry.reconcile(self(), active_component_keys) - Mob.Sender.render(state.ref, tree, platform, :mob_nif, transition) + Mob.Sender.render(state.ref, tree, platform, state.nif, transition) if mode == :sync, do: Mob.Sender.sync(:infinity) Mob.Socket.put_root_view(socket, :json_tree) end - defp initial_safe_area(:render) do - {t, r, b, l} = :mob_nif.safe_area() + defp initial_safe_area(:render, nif) do + {t, r, b, l} = nif.safe_area() %{top: t, right: r, bottom: b, left: l} end - defp initial_safe_area(_mode), do: %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} + defp initial_safe_area(_mode, _nif), do: %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} - defp ensure_safe_area(socket, platform) do + defp ensure_safe_area(socket, platform, nif) do if Map.has_key?(socket.assigns, :safe_area) do socket else safe_area = if platform == :ios do - {t, r, b, l} = :mob_nif.safe_area() + {t, r, b, l} = nif.safe_area() %{top: t, right: r, bottom: b, left: l} else %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} diff --git a/lib/mob/sender.ex b/lib/mob/sender.ex index 35b7c74e..cce99070 100644 --- a/lib/mob/sender.ex +++ b/lib/mob/sender.ex @@ -50,7 +50,7 @@ defmodule Mob.Sender do `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.Router` 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. @@ -87,7 +87,7 @@ defmodule Mob.Sender do 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 + log, until the first synchronous render exits `:noproc`. `Mob.Router` calls this so no render path can reach that state. Deliberately unlinked. The caller is usually a screen, and a screen crash must @@ -108,7 +108,7 @@ defmodule Mob.Sender do @doc """ Declare which screen's trees may be committed. - A render for any other screen is dropped. `Mob.Screen` sets this today; + A render for any other screen is dropped. `Mob.Router` sets this; MOB-113's router takes it over. """ @spec set_active(screen_ref()) :: :ok diff --git a/test/mob/nav_test.exs b/test/mob/nav_test.exs index 7f987e09..528be045 100644 --- a/test/mob/nav_test.exs +++ b/test/mob/nav_test.exs @@ -72,7 +72,7 @@ defmodule Mob.NavTest do test "an unrecognised layout is ignored rather than raising" do # navigation/1 is app-supplied and unvalidated, and this runs inside - # Mob.Screen.init/1 — raising would turn a shape Nav.Registry has always + # Mob.Router.init/1 — raising would turn a shape Nav.Registry has always # tolerated into a failure to boot. assert Nav.from_layout([stack(:home, root: HomeScreen)], HomeScreen) == Nav.new() assert Nav.from_layout(:nonsense, HomeScreen) == Nav.new() diff --git a/test/mob/router_hot_path_test.exs b/test/mob/router_hot_path_test.exs index aa481cbe..af59a3a1 100644 --- a/test/mob/router_hot_path_test.exs +++ b/test/mob/router_hot_path_test.exs @@ -9,6 +9,14 @@ defmodule Mob.RouterHotPathTest do Asserted by tracing the router's mailbox rather than by reasoning about the code, so it keeps holding when someone adds a message. + + Both halves of the path are covered. The callback half (`handle_info` -> + user code) runs under `:no_render`. The render half (tree expansion, + `Mob.ComponentRegistry.reconcile/2`, the hand-off to `Mob.Sender`) only runs + in `:render`, which needs a NIF — so those tests inject a stub one. Covering + only the callback half would leave the property unpinned exactly where it is + most likely to break: an "am I still active?" check added to the render body + is the obvious shape of a future router hop. """ use ExUnit.Case, async: false @@ -45,6 +53,17 @@ defmodule Mob.RouterHotPathTest do def navigation(_), do: stack(:home, root: @home) end + # Enough of the NIF surface for a screen to mount and render off-device. + defmodule StubNif do + def platform, do: :android + def safe_area, do: {0.0, 0.0, 0.0, 0.0} + def take_launch_notification, do: :none + def clear_taps, do: :ok + def set_transition(_), do: :ok + def register_tap(_), do: 0 + def set_root(_json), do: :ok + end + defp stop_safely(pid) do GenServer.stop(pid) catch @@ -114,6 +133,8 @@ defmodule Mob.RouterHotPathTest do end) assert messages == [] + # Without this the test could pass because the handler never ran. + assert Mob.Screen.get_socket(router).assigns.typed == "hello" end test "a burst of messages produces no router traffic at all", %{ @@ -131,6 +152,56 @@ defmodule Mob.RouterHotPathTest do end end + describe "the render path does not reach it either" do + setup do + services = [Mob.Sender, Mob.Listener, Mob.ComponentRegistry] + for name <- services, pid = Process.whereis(name), do: stop_safely(pid) + + # The render path reconciles components, which needs the registry's table. + {:ok, _} = Mob.ComponentRegistry.start_link() + {:ok, router} = Mob.Router.start_root(HomeScreen, %{}, nif: StubNif) + + on_exit(fn -> + stop_safely(router) + for name <- services, pid = Process.whereis(name), do: stop_safely(pid) + end) + + %{rendering_router: router, rendering_screen: Mob.Screen.get_screen_pid(router)} + end + + test "a message that triggers a real render leaves the router untouched", %{ + rendering_router: router, + rendering_screen: screen + } do + # This is the half :no_render skips: tree expansion, component reconcile, + # and the hand-off to Mob.Sender all execute here. + messages = + router_messages(router, fn -> + send(screen, {:tap, :bump}) + :sys.get_state(screen) + Mob.Sender.sync() + end) + + assert messages == [] + assert Mob.Screen.get_socket(router).assigns.count == 1 + end + + test "a burst of real renders leaves it untouched", %{ + rendering_router: router, + rendering_screen: screen + } do + messages = + router_messages(router, fn -> + for _ <- 1..25, do: send(screen, {:tap, :bump}) + :sys.get_state(screen) + Mob.Sender.sync() + end) + + assert messages == [] + assert Mob.Screen.get_socket(router).assigns.count == 25 + end + end + describe "navigation does reach the router" do test "a nav action from a screen callback arrives", %{router: router, screen: screen} do messages =