diff --git a/App/serval_app/lib/data/dashboard_socket.dart b/App/serval_app/lib/data/dashboard_socket.dart index f5eb2e7..44c8f18 100644 --- a/App/serval_app/lib/data/dashboard_socket.dart +++ b/App/serval_app/lib/data/dashboard_socket.dart @@ -127,6 +127,31 @@ class DashboardSocket { _connected.add(false); } + /// Lets the socket go while nobody is looking at what comes down it, leaving this ready for a + /// later [connect] — which is what `reconnectNow` on the way back amounts to. + /// + /// Every camera's JPEG arrives here about once a second for as long as this is open, and a phone + /// in somebody's pocket is paying for all of it in radio and in battery. Nothing on the other side + /// stops: those frames are encoded at `Ingest:SnapshotFps` regardless, to feed the vision pipeline + /// and `/snapshot.jpg`, so what this saves is egress and this client's own decoding. + /// + /// The other half of what it saves is frames. Each arrival writes a notifier, which rebuilds a + /// tile, which asks the browser for an animation frame that a hidden page is never given — so an + /// open socket guarantees there is a frame outstanding at the moment the page goes away, which is + /// the state `frame_watchdog.dart` describes as unrecoverable from the inside. + /// + /// Deliberately silent, where [disconnect] announces itself: `connected: false` is a claim that + /// the Server could not be reached, and this is the App choosing to stop listening. Announcing it + /// would also rebuild the whole wall on the way past, which is precisely the frame this is here + /// to not schedule. + void pause() { + if (_closed) return; + _teardown(); + // Somebody coming back should not spend a wait earned before this, and the reconnect that + // follows a pause is always a person asking. + _backoff = _minBackoff; + } + /// Reconnects at once rather than waiting out the backoff. /// /// For the App coming back from the background, where the backoff is exactly wrong: a phone away diff --git a/App/serval_app/lib/data/live_repository.dart b/App/serval_app/lib/data/live_repository.dart index e43808f..238542e 100644 --- a/App/serval_app/lib/data/live_repository.dart +++ b/App/serval_app/lib/data/live_repository.dart @@ -462,6 +462,25 @@ class LiveServalRepository implements ServalRepository { /// Not on [ServalRepository] either, matching [start] and [stop]: `SampleServalRepository` has no /// sockets and no clock, and a no-op on the interface would be a member the widget tests have to /// answer for. `_RepositoryStarter` already narrows before calling any of the three. + /// Nobody is looking. Let go of the traffic whose only purpose is to be looked at. + /// + /// The wall socket alone, and the asymmetry with [resumeLive] is deliberate. `WS /api/events` is + /// the alerting path — an alert that arrived while a phone was in a pocket is the single most + /// important thing this App carries — and it is nearly free, a message per thing that happens + /// rather than a frame per camera per second. Dropping it to save nothing would be trading the + /// feature for the bill. + /// + /// Nothing here is torn down beyond the socket: the registry, the feed and the arrangement are + /// all still true, and the wall must be able to paint the moment somebody comes back. That is + /// what separates this from [stop], which is about a session ending rather than a person looking + /// away. + /// + /// Not on [ServalRepository], matching [start], [stop] and [resumeLive], for the same reason + /// given there. + void pauseLive() { + _dashboard.pause(); + } + void resumeLive() { _startListening(); _dashboard.reconnectNow(); diff --git a/App/serval_app/lib/main.dart b/App/serval_app/lib/main.dart index 340e026..41ea3bd 100644 --- a/App/serval_app/lib/main.dart +++ b/App/serval_app/lib/main.dart @@ -114,20 +114,31 @@ class _ServalMaterialAppState extends State<_ServalMaterialApp> { void initState() { super.initState(); + // The other half of a tap landing where it was sent. Routing it is only half the job: a + // browser that has come back from the background unable to paint holds the screen it was on + // however correct the route underneath it is. See [watchFrames], including why the route it + // recovers to has to come from the router rather than from the address bar. + // + // Before the push wiring below, which reaches into it: a tap can arrive at any moment after + // that line, and it must not find a watchdog that does not know where it would reload to. + watchFrames(() => _router.routeInformationProvider.value.uri.toString()); + // A tapped notification on a tab that is already open arrives as a message from the service // worker rather than as a navigation — see `sw.js`, which prefers focusing this tab to opening // a second copy of a live-video app. Routing it has to happen here because this is where the // router lives; without it the tab would come to the front unchanged, which reads as the tap // having done nothing at all. // + // The route is set and *then* the frame pipeline is asked about, in that order. The message + // arrives whether or not the App can paint, so this is the one moment the App knows somebody is + // waiting on a screen it may be unable to draw — and by then `go` has already told the router + // where the reload should land. + // // A no-op off the web and on a browser with no push, so there is no platform branch here. - PushClient.onNavigate(_router.go); - - // The other half of a tap landing where it was sent. Routing it is only half the job: a - // browser that has come back from the background unable to paint holds the screen it was on - // however correct the route underneath it is. See [watchFrames], including why the route it - // recovers to has to come from the router rather than from the address bar. - watchFrames(() => _router.routeInformationProvider.value.uri.toString()); + PushClient.onNavigate((route) { + _router.go(route); + probeFrames(); + }); } @override @@ -186,6 +197,16 @@ class _RepositoryStarterState extends ConsumerState<_RepositoryStarter> { /// — constructs nothing new and observes nothing. AppLifecycleListener? _lifecycle; + /// The grace running between the App going away and the wall socket being let go. + Timer? _pause; + + /// How long the App must be away before it stops listening to the wall. + /// + /// The same figure `WebRtcView` uses to decide whether a resume was long enough to be worth + /// rebuilding a session for, and for the same reason: it is how long a glance at something else + /// lasts. Both are about the difference between looking away and going away. + static const _pauseAfterHidden = Duration(seconds: 10); + @override void initState() { super.initState(); @@ -194,23 +215,29 @@ class _RepositoryStarterState extends ConsumerState<_RepositoryStarter> { _onAuthChanged(); if (ref.read(repositoryProvider) is LiveServalRepository) { - // `onShow`, and deliberately not `onResume`. Both fire on the way back, but `onResume` also - // fires when the window merely regains input focus — and a second monitor showing the wall - // while you work elsewhere is the case this must leave alone. `onShow` is the hidden→visible - // edge alone, which is what `document.visibilityState` means, and it buys that rule with no - // bookkeeping of our own. - _lifecycle = AppLifecycleListener(onShow: _onShown); + // `onShow`/`onHide`, and deliberately not `onResume`/`onPause`. All four fire on the way back + // and away, but the resume pair also fires when the window merely regains or loses input + // focus — and a second monitor showing the wall while you work elsewhere is the case this + // must leave alone. The show pair is the hidden→visible edge alone, which is what + // `document.visibilityState` means, and it buys that rule with no bookkeeping of our own. + _lifecycle = AppLifecycleListener(onShow: _onShown, onHide: _onHidden); } } @override void dispose() { + _pause?.cancel(); _lifecycle?.dispose(); _auth?.removeListener(_onAuthChanged); super.dispose(); } void _onShown() { + // Before the `_started` gate: a pause armed while signed in must not survive a sign-out into + // the next session, and cancelling one that is not there costs nothing. + _pause?.cancel(); + _pause = null; + // Only for a session that is actually running. Coming back to a tab sitting on `/login` must // not raise sockets, and `_started` is the same flag both edges below turn on. if (!_started) return; @@ -221,6 +248,26 @@ class _RepositoryStarterState extends ConsumerState<_RepositoryStarter> { repository.resumeLive(); } + /// The App has gone away. Start the clock on letting the wall socket go. + /// + /// Through a timer rather than at once because most of what this edge reports is somebody glancing + /// at something else. Dropping the socket for a five-second look and rebuilding it on the way back + /// would churn a connection and repaint a wall to save five seconds of frames — and the reconnect + /// is the expensive half of that trade, not the frames. + void _onHidden() { + if (!_started) return; + + _pause?.cancel(); + _pause = Timer(_pauseAfterHidden, () { + _pause = null; + + final repository = ref.read(repositoryProvider); + if (repository is! LiveServalRepository) return; + + repository.pauseLive(); + }); + } + void _onAuthChanged() { final auth = _auth; if (auth == null) return; diff --git a/App/serval_app/lib/platform/frame_watchdog.dart b/App/serval_app/lib/platform/frame_watchdog.dart index 178ad8d..0cfa3c2 100644 --- a/App/serval_app/lib/platform/frame_watchdog.dart +++ b/App/serval_app/lib/platform/frame_watchdog.dart @@ -34,6 +34,14 @@ import 'frame_watchdog_stub.dart' /// here. It reloads, which is the same thing the only available workaround does — closing the App /// and opening it again — minus the person having to know that. /// +/// Two properties are what make it fire at all, and both are answers to the same trap: the thing +/// being measured cannot be the thing doing the measuring. **Nothing it waits on is a +/// `requestAnimationFrame`** — a latched pipeline is one whose animation frame never arrives, so a +/// deadline counted in those cannot end in the very state it names; the deadline is a timer, which +/// a wedged page still runs. And **it does not rely on catching the moment**: it asks on every edge +/// that could carry a wedge and on a slow heartbeat besides, because an App that is wedged stays +/// wedged whether or not anything saw it happen. +/// /// [route] is where to reload *to*, and it must be the router's own answer rather than the address /// bar's. `Router` reports a navigation to the browser from a post-frame callback, so an App that /// cannot paint never writes the new address — a tapped notification routed while wedged leaves @@ -43,3 +51,13 @@ import 'frame_watchdog_stub.dart' /// /// A no-op off the web, so there is no platform branch at the call site. void watchFrames(String Function() route) => platform.watchFrames(route); + +/// Asks the question now rather than waiting for an edge or a heartbeat. +/// +/// For the caller who has just done something on somebody's behalf and knows they are watching for +/// the result. A tapped notification is the case that matters: it reaches the App through the +/// service worker's message, which is delivered whether or not the App can paint, so a tap landing +/// on a wedged App is indistinguishable from a tap that did nothing at all. +/// +/// A no-op off the web, and a no-op before [watchFrames] has run. +void probeFrames() => platform.probeFrames(); diff --git a/App/serval_app/lib/platform/frame_watchdog_decision.dart b/App/serval_app/lib/platform/frame_watchdog_decision.dart new file mode 100644 index 0000000..1f34f15 --- /dev/null +++ b/App/serval_app/lib/platform/frame_watchdog_decision.dart @@ -0,0 +1,59 @@ +/// What one liveness probe concluded about the frame pipeline. +enum FrameVerdict { + /// A frame was produced, or the App is not in a state this can judge. Nothing to do. + healthy, + + /// The main thread was too busy for the answer to mean anything. Ask again. + retry, + + /// The pipeline is latched shut, and only a reload clears it. + reload, +} + +/// How much later than its deadline a wait may end before the thread counts as jammed rather than +/// merely idle. +/// +/// Generous on purpose. The cost of calling a jammed thread wedged is reloading an App that was +/// about to paint; the cost of calling a wedged thread jammed is one more probe, a heartbeat later. +/// Those are not the same price. +const _jammed = 2; + +/// Reads one probe's result. +/// +/// Lives apart from the browser plumbing so it can be tested: the conditional import in +/// `frame_watchdog.dart` hands `flutter test` the stub, so nothing reachable from a widget test may +/// touch `dart:js_interop`. +/// +/// [waited] is the wall clock that actually passed while waiting [deadline] for a frame, and the gap +/// between the two is why this is not a single comparison. The timer ending the wait runs on the +/// same thread as the frame it waits for, so it can only fire when the event loop is free — which +/// means a wait that ended *on time* carries a second fact beyond "nothing painted": the thread was +/// free to paint and did not. A wait that ended late says only that the thread was busy, which is +/// what a resume decoding every camera's snapshot at once looks like, and a busy App is not a +/// broken one. +/// +/// Deliberately not a count of animation frames. A latched pipeline is one whose +/// `requestAnimationFrame` callback never arrives, so a deadline counted in those is a deadline +/// that cannot end in the very state it exists to name. +FrameVerdict judgeFrame({ + required bool painted, + required Duration waited, + required Duration deadline, + required bool visible, + required int attemptsLeft, +}) { + if (painted) return FrameVerdict.healthy; + + // A page that went back to the background mid-probe never owed anybody a frame. That is not the + // fault this recovers from, and reloading somebody's App on the strength of it would be worse + // than the fault. The next time the page is shown it is asked again. + if (!visible) return FrameVerdict.healthy; + + if (waited > deadline * _jammed) { + // Out of probes on a thread that has been busy throughout. Still no evidence of a latch, so + // this leaves it alone rather than guessing; the heartbeat asks again. + return attemptsLeft > 0 ? FrameVerdict.retry : FrameVerdict.healthy; + } + + return FrameVerdict.reload; +} diff --git a/App/serval_app/lib/platform/frame_watchdog_stub.dart b/App/serval_app/lib/platform/frame_watchdog_stub.dart index c4e6f1d..0bbfdfd 100644 --- a/App/serval_app/lib/platform/frame_watchdog_stub.dart +++ b/App/serval_app/lib/platform/frame_watchdog_stub.dart @@ -5,3 +5,5 @@ /// specifically — a `requestAnimationFrame` that a hidden page never receives — and a native /// embedder drives its frames from a vsync signal that a backgrounded app is simply not sent. void watchFrames(String Function() route) {} + +void probeFrames() {} diff --git a/App/serval_app/lib/platform/frame_watchdog_web.dart b/App/serval_app/lib/platform/frame_watchdog_web.dart index 4326888..6753eaa 100644 --- a/App/serval_app/lib/platform/frame_watchdog_web.dart +++ b/App/serval_app/lib/platform/frame_watchdog_web.dart @@ -1,83 +1,190 @@ import 'dart:async'; import 'dart:js_interop'; +import 'package:flutter/foundation.dart'; import 'package:flutter/scheduler.dart'; import 'package:web/web.dart' as web; -/// Browser animation frames the probe will sit through before deciding nothing is being painted. +import 'frame_watchdog_decision.dart'; + +/// How long one probe waits for a frame before judging the wait. /// -/// Deliberately a count of frames rather than a stopwatch, because the two failures it has to tell -/// apart share a thread. A main thread merely busy — a burst of snapshots decoded on resume, a -/// WebRTC session renegotiating — delays *these* callbacks exactly as much as it delays Flutter's, -/// so waiting a fixed wall-clock second would report a slow phone as a broken one. A page that is -/// genuinely animating and has painted none of thirty frames is not busy. -const _frames = 30; - -/// How long one animation frame is waited for before the probe gives up on the browser. +/// Long enough that a phone still finishing a resume has painted something — the first frame back +/// carries every camera's snapshot and the whole activity column — and short enough that nobody is +/// left holding a dead App while it runs. +const _deadline = Duration(seconds: 3); + +/// Probes spent on one suspicion before it is left until the next heartbeat. +const _attempts = 3; + +/// How often a visible App is asked whether it is still painting. /// -/// A page can stop animating for reasons that are nobody's fault — it went back to the background -/// mid-probe, or it never really came forward. That is not the fault this recovers from, and -/// reloading somebody's App on the strength of it would be worse than the fault. Abandoning also -/// releases [_probing], so the next time the page is shown it is asked again. -const _stall = Duration(seconds: 5); +/// The events below are where a wedge is *likely*, not where it is possible, and an App that is +/// wedged stays wedged whether or not anything caught the moment it happened. This is what makes +/// the recovery independent of catching the right edge: a cold load, a wedge that appears a moment +/// after a resume, and one that appears mid-session are all found within a period. It costs one +/// otherwise-idle frame per period, on an App that is already animating whenever somebody is +/// looking at it. +const _heartbeat = Duration(seconds: 20); + +/// Where a reload leaves word for the next launch. See [_recordRecovery]. +const _breadcrumbKey = 'serval.watchdog.recovery'; /// Whether a probe is already running. /// -/// `visibilitychange` can fire repeatedly while one is in flight — a phone answering a call, a -/// notification shade pulled down and let go — and each of those would otherwise start a second +/// The triggers below overlap freely — a phone answering a call, a notification shade pulled down +/// and let go, a heartbeat landing on a resume — and each of those would otherwise start a second /// probe racing the first to reload the page. bool _probing = false; +/// Where to reload to, held for [probeFrames] to reach. Null until [watchFrames] has run. +String Function()? _route; + void watchFrames(String Function() route) { - // The DOM event rather than `AppLifecycleListener`, and that is the point: this asks whether + _route = route; + _reportRecovery(); + + // DOM events rather than `AppLifecycleListener`, and that is the point: this asks whether // Flutter's own machinery is still running, so it cannot be scheduled by that machinery. The - // lifecycle listener is delivered through the engine and the framework binding, which is the - // half under suspicion. + // lifecycle listener is delivered through the engine and the framework binding, which is the half + // under suspicion. web.document.addEventListener( 'visibilitychange', (web.Event _) { - if (web.document.visibilityState == 'visible') { - unawaited(_probe(route)); - } + if (web.document.visibilityState == 'visible') unawaited(_probe()); }.toJS, ); + + // A page restored from the back/forward cache had its whole frame loop suspended and resumed, and + // it arrives without a `visibilitychange` to say so. + web.window.addEventListener( + 'pageshow', + ((web.Event _) => unawaited(_probe())).toJS, + ); + + // The Page Lifecycle API's own thaw — what Chrome fires when it releases a PWA it had frozen, + // which is the state this file exists for. + web.document.addEventListener( + 'resume', + ((web.Event _) => unawaited(_probe())).toJS, + ); + + Timer.periodic(_heartbeat, (_) { + if (web.document.visibilityState == 'visible') unawaited(_probe()); + }); +} + +/// Asks the question now, on behalf of somebody who just did something and expects an answer. +/// +/// A tapped notification is the case: it is routed through the service worker's message, which +/// arrives whether or not the App can paint, so a tap that lands on a wedged App looks exactly like +/// a tap that did nothing. Waiting out a heartbeat to find that out is most of a minute spent +/// staring at a screen that has already failed. +void probeFrames() { + if (_route == null) return; + unawaited(_probe()); } /// Asks the framework for a frame and watches whether one arrives. -Future _probe(String Function() route) async { - if (_probing) return; +Future _probe() async { + final route = _route; + if (route == null || _probing) return; _probing = true; try { - var painted = false; - - // A post-frame callback rather than anything that inspects the scheduler's own flags: it is - // set at the end of `handleDrawFrame`, so it is evidence a frame was actually produced rather - // than evidence one was asked for — and asking is precisely what is believed to have already - // happened. `ensureVisualUpdate` is what makes an idle App produce one; a frame with nothing - // dirty still runs the callback. - SchedulerBinding.instance - ..addPostFrameCallback((_) => painted = true) - ..ensureVisualUpdate(); - - for (var frame = 0; frame < _frames; frame++) { - if (!await _animationFrame()) return; - if (painted) return; - } + for (var attempt = _attempts; attempt > 0; attempt--) { + var painted = false; + + // A post-frame callback rather than anything that inspects the scheduler's own flags: it is + // set at the end of `handleDrawFrame`, so it is evidence a frame was actually produced rather + // than evidence one was asked for — and asking is precisely what is believed to have already + // happened. `ensureVisualUpdate` is what makes an idle App produce one; a frame with nothing + // dirty still runs the callback. + final started = DateTime.now(); + SchedulerBinding.instance + ..addPostFrameCallback((_) => painted = true) + ..ensureVisualUpdate(); + + // A timer, and nothing here waits on an animation frame. Timers are delivered to a page whose + // frame pipeline is latched — that is the whole shape of this fault, everything but the + // painting carrying on — so this is the one clock that still runs in the state being measured. + await Future.delayed(_deadline); + + final waited = DateTime.now().difference(started); + final verdict = judgeFrame( + painted: painted, + waited: waited, + deadline: _deadline, + visible: web.document.visibilityState == 'visible', + attemptsLeft: attempt - 1, + ); + + // Only what is not routine. A healthy verdict is every probe on a working App — one a + // heartbeat, forever — and a log line per heartbeat would bury the two that mean something. + if (verdict != FrameVerdict.healthy) { + debugPrint( + 'FrameWatchdog: $verdict — painted=$painted ' + 'waited=${waited.inMilliseconds}ms attemptsLeft=${attempt - 1}', + ); + } - // Thirty frames delivered to this file and none to Flutter. The pipeline is latched shut and - // there is no way to open it from here. - _reloadTo(route()); + switch (verdict) { + case FrameVerdict.healthy: + return; + case FrameVerdict.retry: + continue; + case FrameVerdict.reload: + final target = route(); + _recordRecovery(target, waited); + _reloadTo(target); + return; + } + } } finally { _probing = false; } } +/// Leaves word that this App reloaded itself. +/// +/// A recovery is invisible by design: somebody who was not holding the phone at the time sees an App +/// that works. Without a record, an App that recovered twice overnight and an App that never wedged +/// are the same observation from the outside, and telling those apart is the whole question once a +/// recovery exists at all. Read back by [_reportRecovery] on the launch the reload produces. +void _recordRecovery(String route, Duration waited) { + try { + web.window.localStorage.setItem( + _breadcrumbKey, + '${DateTime.now().toUtc().toIso8601String()} route=$route ' + 'waited=${waited.inMilliseconds}ms', + ); + } catch (_) { + // Storage refused. A recovery nobody can read about is still a recovery, and failing the launch + // that came out of one would be a poor trade for a diagnostic. + } +} + +void _reportRecovery() { + try { + final note = web.window.localStorage.getItem(_breadcrumbKey); + if (note == null) return; + + // Cleared before it is reported, so one recovery is announced once however this launch goes on + // to end. + web.window.localStorage.removeItem(_breadcrumbKey); + debugPrint( + 'FrameWatchdog: recovered from a latched frame pipeline — $note', + ); + } catch (_) { + // As above. + } +} + /// Loads the App again at [target]. /// -/// `replace` rather than `assign` where the address has to change: the entry being left is a -/// broken copy of this same App, and leaving it on the history stack would put the back button -/// one press away from returning to it. +/// `replace` rather than `assign` where the address has to change: the entry being left is a broken +/// copy of this same App, and leaving it on the history stack would put the back button one press +/// away from returning to it. void _reloadTo(String target) { final here = '${web.window.location.pathname}${web.window.location.search}'; if (target == here) { @@ -87,25 +194,3 @@ void _reloadTo(String target) { web.window.location.replace(target); } - -/// One animation frame. False if the browser stopped delivering them instead. -/// -/// Ordering matters and is in our favour: rAF callbacks run in the order they were registered, and -/// anything Flutter scheduled — including the frame [_probe] just asked for — was registered before -/// this one. So on a healthy page the frame is fully painted, post-frame callback and all, before -/// this future completes. -Future _animationFrame() { - final frame = Completer(); - - web.window.requestAnimationFrame( - (JSNumber _) { - if (!frame.isCompleted) frame.complete(true); - }.toJS, - ); - - Timer(_stall, () { - if (!frame.isCompleted) frame.complete(false); - }); - - return frame.future; -} diff --git a/App/serval_app/lib/push/push_client_web.dart b/App/serval_app/lib/push/push_client_web.dart index 52d673d..ab56ebe 100644 --- a/App/serval_app/lib/push/push_client_web.dart +++ b/App/serval_app/lib/push/push_client_web.dart @@ -97,10 +97,13 @@ void onNavigate(void Function(String route) handler) { ); } -/// Reads the destination `sw.js` recorded, clearing it on the way past. +/// Reads the destination `sw.js` recorded, clearing it once it has been acted on. /// -/// The delete happens before the handler runs rather than after, so a navigation that throws — or a -/// page closed halfway through one — cannot leave an entry behind for the next launch to open. +/// The delete happens after [_accept] rather than before, and the order is load-bearing: between the +/// two sits an `await`, and a page that reloads inside it — the frame watchdog recovering a wedged +/// pipeline is the way that happens — would come back to a destination already erased and land on +/// whatever screen it was on. Deleting afterwards cannot leak a second opening, because [_accept] +/// has by then recorded the id and every later delivery of the same tap is refused on it. Future _takePending(void Function(String route) handler) async { try { final cache = await web.window.caches.open(_pendingCache).toDart; @@ -109,17 +112,18 @@ Future _takePending(void Function(String route) handler) async { return; } - await cache.delete(_pendingKey.toJS).toDart; - final body = (await recorded.text().toDart).toDart; if (jsonDecode(body) case {'url': final String url, 'id': final num id}) { final tappedAt = DateTime.fromMillisecondsSinceEpoch(id.toInt()); if (DateTime.now().difference(tappedAt) > _pendingMaxAge) { + await cache.delete(_pendingKey.toJS).toDart; return; } _accept(id.toInt(), url, handler); } + + await cache.delete(_pendingKey.toJS).toDart; } catch (_) { // A browser that refuses the Cache API leaves the message as the only route, which is where // this started. Nothing here is worth failing a launch over. diff --git a/App/serval_app/lib/widgets/webrtc_view.dart b/App/serval_app/lib/widgets/webrtc_view.dart index 03048f7..8f35190 100644 --- a/App/serval_app/lib/widgets/webrtc_view.dart +++ b/App/serval_app/lib/widgets/webrtc_view.dart @@ -167,8 +167,12 @@ class _WebRtcViewState extends State { if (_session.isHealthy && away < _staleAfterHidden) return; // A resume is a person asking for this, not a failure — so it does not spend the budget and it - // does not wait out a backoff. + // does not wait out a backoff. The backoff already running belongs to the session about to be + // replaced, and letting it survive would have it tear down the one built here a few seconds + // later, on a screen somebody is by then watching. _restarts = 0; + _pendingRestart?.cancel(); + _pendingRestart = null; _restart(); } diff --git a/App/serval_app/test/frame_watchdog_decision_test.dart b/App/serval_app/test/frame_watchdog_decision_test.dart new file mode 100644 index 0000000..5bddcb6 --- /dev/null +++ b/App/serval_app/test/frame_watchdog_decision_test.dart @@ -0,0 +1,62 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:serval_app/platform/frame_watchdog_decision.dart'; + +/// The judgement the frame watchdog makes, tested without a browser. +/// +/// The watchdog itself cannot be reached from here — the conditional import in +/// `frame_watchdog.dart` hands `flutter test` the stub — and this is the half where being wrong +/// costs something: a verdict of [FrameVerdict.reload] throws away whatever somebody was doing. +void main() { + const deadline = Duration(seconds: 3); + + FrameVerdict judge({ + bool painted = false, + Duration waited = deadline, + bool visible = true, + int attemptsLeft = 2, + }) => judgeFrame( + painted: painted, + waited: waited, + deadline: deadline, + visible: visible, + attemptsLeft: attemptsLeft, + ); + + test('a frame arriving is the whole answer', () { + expect(judge(painted: true), FrameVerdict.healthy); + + // Even from a thread that took its time about it, and even from a page that has since gone + // away: a frame was produced, so the pipeline is not latched. + expect( + judge(painted: true, waited: const Duration(minutes: 1), visible: false), + FrameVerdict.healthy, + ); + }); + + test('a wait that ended on time with nothing painted is the wedge', () { + expect(judge(), FrameVerdict.reload); + }); + + test('a page that went away mid-probe is left alone', () { + // It never owed anybody a frame. Reloading somebody's App on this would be worse than the + // fault the reload exists to recover from. + expect(judge(visible: false), FrameVerdict.healthy); + }); + + test('a jammed thread is asked again rather than reloaded', () { + expect(judge(waited: const Duration(seconds: 30)), FrameVerdict.retry); + }); + + test('a thread jammed to the last probe is left alone, not reloaded', () { + expect( + judge(waited: const Duration(seconds: 30), attemptsLeft: 0), + FrameVerdict.healthy, + ); + }); + + test('overrunning the deadline a little is still the wedge', () { + // The margin exists to tell a busy thread from an idle one, not to excuse every late timer. + // Just over the deadline is a thread that was free within a frame or two of when it was asked. + expect(judge(waited: const Duration(seconds: 4)), FrameVerdict.reload); + }); +} diff --git a/Docs/browser-testing.md b/Docs/browser-testing.md index 0a2bee5..06de932 100644 --- a/Docs/browser-testing.md +++ b/Docs/browser-testing.md @@ -236,6 +236,38 @@ adb shell input tap 480 828 # the notification, shade already open That is what the pending-navigation record in `web/sw.js` and `_takePending` in `lib/push/push_client_web.dart` exist for; see the comments there. +### Wedging the frame pipeline on purpose + +The fault `lib/platform/frame_watchdog.dart` recovers from — the App coming back from the background +showing its last frame and ignoring every tap — has a one-line reproduction, and it is exact rather +than approximate: + +```bash +node cdp.js eval "window.requestAnimationFrame = function(){ return 0; }" +``` + +Both latches that stop frames are cleared only from inside a `requestAnimationFrame` callback, so a +callback that never arrives is the fault itself rather than an imitation of it. Background the app and +bring it back and it is wedged: the canvas holds, taps do nothing, and `adb exec-out screencap` +twice a second apart returns identical bytes. The recovery reloads, which discards the stub, so a +working watchdog un-wedges itself and a broken one sits there. + +Two things this is good for beyond the watchdog. It tells a frozen pipeline from a hung request — +`let n=0; requestAnimationFrame(()=>n++); setTimeout(()=>console.log(n),1000)` prints `0` on a wedged +page and `1` on one that is merely waiting on the network — and it proves timers still run in that +state, which is why the watchdog's deadline is a `Timer` and not a count of animation frames. + +**What does not reproduce it**, all tried: backgrounding with `keyevent 3` and returning with +`monkey`, CDP `Page.setWebLifecycleState` `frozen` → `active`, and `am send-trim-memory +com.android.chrome COMPLETE`. The emulator runs software GL on a machine with memory to spare, and +its renderer lifecycle is not a phone's. A device is the only place the natural trigger has been +seen; the stub is how the recovery is tested anywhere. + +**The CDP forward does not survive backgrounding.** `adb forward tcp:9222` dies with the devtools +socket once Chrome has been in the background a while, and every later `fetch` to `/json/list` fails +or hangs — which looks exactly like an app that never recovered. Re-run the `adb forward` after any +wait longer than a few seconds, and drive the phone with `adb` across the gap rather than over CDP. + ## Test the insecure origin too, and know what it costs Serving Serval over plain HTTP is supported — trying it out should not mean standing up diff --git a/Docs/live-view.md b/Docs/live-view.md index 727d6d3..eff8eea 100644 --- a/Docs/live-view.md +++ b/Docs/live-view.md @@ -140,33 +140,57 @@ through the same path for the same reason. None of that section is reachable from `flutter test`: `SampleServalRepository.canStreamLive` is false precisely so tests never construct a peer connection. It is verified on a device. -## Still not done: pausing live video when the App is hidden - -The App notices being hidden; it does not yet *stop* anything. A backgrounded tab holds its WebRTC -connection open and keeps draining every camera's JPEG off `WS /api/dashboard` for as long as it is -open. The wall socket is opened once in `LiveServalRepository.start()` and closed only when the -repository is disposed, which never happens. - -Two things would pause, and they are worth very different amounts: - -- **The focused WebRTC view is the real saving**, because go2rtc pulls RTSP lazily (above): closing - the peer connection stops the upstream pull as well as the browser's decode. `WebRtcView` now has - `_restart()`, which is most of the work — a pause is that without the reopen — and the resume path - it needs already exists. -- **The wall socket costs bandwidth, not Server CPU.** Those JPEGs are encoded by ffmpeg - unconditionally at `Ingest:SnapshotFps` to feed `CameraVisionPipeline` and `/snapshot.jpg`, so - closing the socket saves egress and the client's decode and nothing else. Gating the encode itself - on whether anyone is watching means reaching back into `StreamIngestManager` and `RecordArguments`, - and contending with those two non-UI consumers — a separate and much larger job. - `DashboardSocket` would need a `pause()`/`resume()` pair; `close()` cannot be reused, as it closes - the broadcast controllers one-way and would kill the repository's `frames` subscription with them. - -**The events socket is never paused.** `WS /api/events` is the alerting path and is nearly free. - -About ten seconds of grace before pausing keeps an alt-tab round trip from churning the socket and -renegotiating WebRTC for a glance at something else — the same figure `WebRtcView` already uses to -decide whether a resume was long enough to be worth rebuilding for. There is nothing visible to opt -out of, so this wants to be unconditional rather than a preference. +## Pausing when the App is hidden + +The wall socket is let go ten seconds after the App goes away and picked up again on the way back. +The WebRTC view is not, and that split is deliberate rather than unfinished — see the end of this +section for what the other half costs. + +`AppLifecycleListener(onHide:)` in `main.dart` arms the ten seconds and +`LiveServalRepository.pauseLive()` spends it, on `DashboardSocket.pause()` alone. The reopen needs +nothing new: `resumeLive()` already calls `reconnectNow()` on both sockets, which is a teardown and a +fresh connect whatever state the socket was left in. + +Ten seconds because that is how long a glance at something else lasts — the same figure +`WebRtcView` already uses to decide whether a resume was long enough to be worth rebuilding a session +for. Dropping the socket for a five-second look and rebuilding it on the way back would churn a +connection and repaint a wall to save five seconds of frames, and the reconnect is the expensive half +of that trade. There is nothing visible to opt out of, so it is unconditional rather than a +preference. + +`pause()` rather than `disconnect()`, and rather than `close()`. `close()` ends the broadcast +controllers one-way and would take the repository's `frames` subscription with them. `disconnect()` +is right but announces `connected: false`, which is a claim that the Server could not be reached — +this is the App choosing to stop listening — and which would rebuild the whole wall on the way past. + +**What it saves is bandwidth and frames, not Server CPU.** Those JPEGs are encoded by ffmpeg +unconditionally at `Ingest:SnapshotFps` to feed `CameraVisionPipeline` and `/snapshot.jpg`, so this +saves egress and the client's own decode and nothing else. Gating the encode on whether anyone is +watching means reaching back into `StreamIngestManager` and `RecordArguments` and contending with +those two non-UI consumers — a separate and much larger job. + +The frames are the other half, and are why this is in `frame_watchdog.dart`'s orbit as much as this +one's. Every arrival writes a notifier, which rebuilds a tile, which asks for an animation frame that +a hidden page is never given — so an open wall socket guarantees there is a frame outstanding at the +moment the page goes away, which is the state that file describes as unrecoverable from the inside. +It is a reduction rather than a cure: `WS /api/events` still writes the feed, so a busy site can +still schedule a frame while hidden. + +**The events socket is never paused.** `WS /api/events` is the alerting path — an alert that arrived +while a phone was in a pocket is the most important thing this App carries — and it is nearly free, a +message per thing that happens rather than a frame per camera per second. + +### Still not done: the focused WebRTC view + +**This is the larger saving of the two**, because go2rtc pulls RTSP lazily (above): closing the peer +connection stops the upstream pull as well as the browser's decode, and it is the heaviest renderer +state the App ever holds while going into the background. + +What it costs is a refactor rather than a call. `WebRtcView._restart()` is a pause plus a reopen, but +a pause on its own has nowhere to leave the view: `_session` is non-nullable and `build` binds to its +`stage` notifier, so pausing means either a session that exists but was never started or a nullable +one threaded through `build`, `dispose`, `didUpdateWidget` and both publishers. Worth doing, and worth +doing deliberately, on the file that owns the live picture. Independent of all of the above, and with the same trigger: `DashboardEndpoint.SendAsync` has no send timeout and the endpoint has no drain loop, so a client that stops reading without closing — a frozen