From 67ec5b76ad7c1dc7199df73399d8aae64a8d5eec Mon Sep 17 00:00:00 2001 From: tonihei Date: Sun, 9 Mar 2025 09:13:31 -0700 Subject: [PATCH] Ensure notification is removed when shouldShowNotification==false We currently combine stopping the FGS and optionally removing the notification in one method, which unnecessarily gates its logic on checking the desired foreground state again. This causes a bug where the notification should be removed (because shouldShowNotification returns false), but stays visible because the service is allowed to stay in the foreground and the notification removal code is not triggered. Issue: androidx/media#2211 PiperOrigin-RevId: 735126704 (cherry picked from commit 91ecc16198bbb48d114a6d581669a9e670c161da) --- RELEASENOTES.md | 5 +++- .../session/MediaNotificationManager.java | 24 +++++++------------ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 01d43786fc..be97f45e6f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -3,7 +3,10 @@ ### Unreleased changes * Audio: - * Add support for float PCM to `ChannelMappingAudioProcessor` + * Add support for float PCM to `ChannelMappingAudioProcessor`. +* Session: + * Fix bug where a stale notification stays visible when the playlist is + cleared ([#2211](https://github.com/androidx/media/issues/2211)). * UI: * Add `PlaybackSpeedState` class and the corresponding `rememberPlaybackSpeedState` Composable to `media3-ui-compose` module. diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java index d135c66634..149badad18 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java @@ -152,7 +152,7 @@ import java.util.concurrent.TimeoutException; */ public void updateNotification(MediaSession session, boolean startInForegroundRequired) { if (!mediaSessionService.isSessionAdded(session) || !shouldShowNotification(session)) { - maybeStopForegroundService(/* removeNotifications= */ true); + removeNotification(); return; } @@ -275,22 +275,16 @@ import java.util.concurrent.TimeoutException; // (https://github.com/androidx/media/issues/192). notificationManagerCompat.notify( mediaNotification.notificationId, mediaNotification.notification); - maybeStopForegroundService(/* removeNotifications= */ false); + stopForeground(/* removeNotifications= */ false); } } - /** - * Stops the service from the foreground, if no player is actively playing content. - * - * @param removeNotifications Whether to remove notifications, if the service is stopped from the - * foreground. - */ - private void maybeStopForegroundService(boolean removeNotifications) { - if (shouldRunInForeground(/* startInForegroundWhenPaused= */ false)) { - return; - } - stopForeground(removeNotifications); - if (removeNotifications && mediaNotification != null) { + /** Removes the notification and stops the foreground service if running. */ + private void removeNotification() { + // To hide the notification on all API levels, we need to call both Service.stopForeground(true) + // and notificationManagerCompat.cancel(notificationId). + stopForeground(/* removeNotifications= */ true); + if (mediaNotification != null) { notificationManagerCompat.cancel(mediaNotification.notificationId); // Update the notification count so that if a pending notification callback arrives (e.g., a // bitmap is loaded), we don't show the notification. @@ -419,8 +413,6 @@ import java.util.concurrent.TimeoutException; } private void stopForeground(boolean removeNotifications) { - // To hide the notification on all API levels, we need to call both Service.stopForeground(true) - // and notificationManagerCompat.cancel(notificationId). if (Util.SDK_INT >= 24) { Api24.stopForeground(mediaSessionService, removeNotifications); } else {