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)
This commit is contained in:
tonihei 2025-03-09 09:13:31 -07:00
parent 6b003993a5
commit 67ec5b76ad
2 changed files with 12 additions and 17 deletions

View File

@ -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.

View File

@ -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 {