From 86225cbc6843e129b1fde78007f8b883d7ba558c Mon Sep 17 00:00:00 2001 From: freetlab Date: Sun, 12 Jul 2026 00:04:52 +0200 Subject: [PATCH] fix(alarm): scope native stop to the ringing id and intercept system back Two exit-path holes found by adversarial review before the next build: PluriWaveAlarmService.stopAlarm() never compared the requested id to activeAlarmId, so any stop request for a DIFFERENT alarm tore down whichever ring was active: with two alarms firing close together, the second one's routine hide-notification call (via dismissAlarmNotification -> ACTION_STOP) killed the first alarm mid-ring and prematurely restored the device volume override. A mismatched id now only cancels that id's notification and returns; null keeps full-teardown semantics for internal/onDestroy callers. The ringing screen never intercepted the system back gesture: a plain route pop ran only dispose(), leaving the shared radio player ringing with no alarm UI left anywhere to stop it. Back now routes through PopScope into the same _detener() flow as the Stop button, guarded by a single-exit flag so a back-press racing a button tap cannot run the teardown twice and pop the route underneath. Also resets the shared handler gain to 1.0 on ring exit: the fade-in mutates the radio player's persistent volume, and exiting mid-ramp used to leave every later radio play at the partial ramp level. --- .../pluriwave/PluriWaveAlarmService.kt | 18 +++++++ lib/pantallas/pantalla_alarma_sonando.dart | 39 +++++++++++++++ .../pantalla_alarma_sonando_test.dart | 48 +++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/android/app/src/main/kotlin/es/freetimelab/pluriwave/PluriWaveAlarmService.kt b/android/app/src/main/kotlin/es/freetimelab/pluriwave/PluriWaveAlarmService.kt index 309ea20..d90c1ac 100644 --- a/android/app/src/main/kotlin/es/freetimelab/pluriwave/PluriWaveAlarmService.kt +++ b/android/app/src/main/kotlin/es/freetimelab/pluriwave/PluriWaveAlarmService.kt @@ -360,6 +360,24 @@ class PluriWaveAlarmService : Service() { private fun stopAlarm(alarmId: String?) { Log.d(TAG, "alarm.service stop id=$alarmId active=$activeAlarmId") + // Scope the teardown to the alarm that is actually ringing: a stop + // request for a DIFFERENT id (e.g. a second alarm firing while this + // one rings — Dart hides the newcomer's notification, which routes + // through ACTION_STOP with the newcomer's id) must not kill the + // active ring, release its wake lock, or prematurely restore the + // device volume. Only the id-specific notification cancel below is + // honored for the mismatched id. A null alarmId (internal callers, + // onDestroy) keeps full-teardown semantics. + if (alarmId != null && activeAlarmId != null && alarmId != activeAlarmId) { + Log.d( + TAG, + "alarm.service stop ignored for id=$alarmId (active=$activeAlarmId)" + ) + NotificationManagerCompat.from(this).cancel( + PluriWaveAlarmReceiver.fireNotificationIdForAlarm(alarmId) + ) + return + } cancelStationFallback() cancelFadeIn() try { diff --git a/lib/pantallas/pantalla_alarma_sonando.dart b/lib/pantallas/pantalla_alarma_sonando.dart index 1ae8f8e..a492110 100644 --- a/lib/pantallas/pantalla_alarma_sonando.dart +++ b/lib/pantallas/pantalla_alarma_sonando.dart @@ -13,6 +13,7 @@ import '../modelos/alarma_musical.dart'; import '../servicios/servicio_audio.dart'; import '../tema/pluri_animate.dart'; import '../tema/pluriwave_theme.dart'; +import '../tema/pluriwave_tokens.dart'; import '../widgets/pluri_glass_surface.dart'; import '../widgets/pluri_wave_scaffold.dart'; @@ -205,7 +206,15 @@ class _PantallaAlarmaSonandoState extends State { await _fallbackPlayer.stop(); } + /// Single-exit guard: Stop, snooze and the system back gesture all funnel + /// into the same teardown; whichever lands first wins and the rest no-op, + /// so a back-press racing a button tap can never run the exit flow twice + /// (a second _dismissScreen would pop the route UNDER the alarm screen). + bool _salidaEnCurso = false; + Future _detener() async { + if (_salidaEnCurso) return; + _salidaEnCurso = true; final radio = context.read(); final alarmas = context.read(); await _silenciarAudio(radio); @@ -225,6 +234,8 @@ class _PantallaAlarmaSonandoState extends State { /// through the canonical EstadoAlarmas.posponerAlarma, which hides the /// native notification (same stop path as dismiss) and re-programs Android. Future _posponer(int minutos) async { + if (_salidaEnCurso) return; + _salidaEnCurso = true; final radio = context.read(); final alarmas = context.read(); // Captured BEFORE dismiss (Design D4): the ringing screen dismisses by @@ -264,6 +275,15 @@ class _PantallaAlarmaSonandoState extends State { } catch (e) { debugPrint('[PluriWave][alarmas] pausar radio fallo: $e'); } + try { + // The fade-in mutates the SHARED radio handler gain; exiting mid-ramp + // would otherwise leave every later radio play at the partial ramp + // level until the next full ramp or app restart. 1.0 is the handler's + // default gain; the player is already paused, so this is inaudible. + await radio.audio.setVolumen(1.0); + } catch (e) { + debugPrint('[PluriWave][alarmas] restaurar ganancia radio fallo: $e'); + } await _restaurarVolumenMediaUnaVez(); } @@ -308,6 +328,25 @@ class _PantallaAlarmaSonandoState extends State { // the first frame after a screen-off FSI wake can stutter. The blur sigma // is capped here, and reduced-motion users skip the entry animation // entirely via pluriFadeIn. + return PopScope( + // System back / predictive back must behave exactly like Stop: a plain + // route pop would run only dispose(), leaving the shared radio player + // ringing with no alarm UI left anywhere to stop it. + canPop: false, + onPopInvokedWithResult: (didPop, _) { + if (didPop) return; + unawaited(_detener()); + }, + child: _cuerpo(context, alarma, l10n, tokens), + ); + } + + Widget _cuerpo( + BuildContext context, + AlarmaMusical alarma, + AppLocalizations l10n, + PluriWaveTokens tokens, + ) { return PluriWaveScaffold( body: SafeArea( child: Padding( diff --git a/test/pantallas/pantalla_alarma_sonando_test.dart b/test/pantallas/pantalla_alarma_sonando_test.dart index fc510fe..12b32f2 100644 --- a/test/pantallas/pantalla_alarma_sonando_test.dart +++ b/test/pantallas/pantalla_alarma_sonando_test.dart @@ -31,6 +31,7 @@ class _Entorno { Future<_Entorno> _montarPantalla( WidgetTester tester, { int snoozeMinutos = 5, + int fadeInSegundos = 0, // Slice 3 (fade-in dedup): existing callers rely on the radio already // being "reproduciendo" by mount time, which cancels the fallback timer // synchronously and leaves nothing to observe mid-handoff. Fade-in-gate @@ -75,6 +76,7 @@ Future<_Entorno> _montarPantalla( tipoProgramacion: TipoProgramacionAlarma.diaria, diasSemana: const [], snoozeMinutos: snoozeMinutos, + fadeInSegundos: fadeInSegundos, emisora: const Emisora( uuid: 'e1', nombre: 'Radio Uno', @@ -255,6 +257,52 @@ void main() { }); }); + group('salidas del ring fuera de los botones', () { + testWidgets('el boton atras del sistema se comporta como Detener: para la ' + 'radio, finaliza y cierra', (tester) async { + final entorno = await _montarPantalla(tester); + + // Simulate the SYSTEM back (routes through PopScope via the binding), + // not a direct Navigator.pop: a plain pop only runs dispose(), which + // leaves the shared radio player ringing with no alarm UI left to + // stop it. + await tester.binding.handlePopRoute(); + await tester.pumpAndSettle(); + + expect(find.byType(PantallaAlarmaSonando), findsNothing); + expect( + entorno.audio.pausas, + greaterThanOrEqualTo(1), + reason: 'atras debe pausar la radio como lo hace Detener', + ); + expect( + entorno.android.ocultadas, + contains('ring1'), + reason: 'atras debe finalizar la ejecucion (mismo camino que Detener)', + ); + }); + + testWidgets('salir a mitad de rampa restaura la ganancia del reproductor ' + 'a 1.0 para la radio normal', (tester) async { + final entorno = await _montarPantalla(tester, fadeInSegundos: 30); + + // Let the ramp advance a few steps (250ms per step) so the shared + // handler gain sits at a partial value well below 1.0. + await tester.pump(const Duration(seconds: 2)); + expect(entorno.audio.volumenesAplicados.last, lessThan(0.2)); + + await tester.tap(find.text(l10n.stopAlarmAction)); + await tester.pumpAndSettle(); + + expect( + entorno.audio.volumenesAplicados.last, + 1.0, + reason: 'la salida del ring no debe dejar la radio normal al nivel ' + 'parcial de la rampa', + ); + }); + }); + group('restore de volumen de medios con dispose como unico llamador', () { testWidgets('desmontar la pantalla sin detener ni posponer restaura el ' 'volumen exactamente una vez', (tester) async {