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.

#cherrypick

Issue: androidx/media#2211
PiperOrigin-RevId: 735126704
This commit is contained in:
tonihei 2025-03-09 09:13:31 -07:00 committed by Copybara-Service
parent 8702554591
commit 91ecc16198
2 changed files with 10 additions and 16 deletions

View File

@ -28,6 +28,8 @@
instead of `MediaCodec.BufferInfo`. instead of `MediaCodec.BufferInfo`.
* IMA extension: * IMA extension:
* Session: * Session:
* Fix bug where a stale notification stays visible when the playlist is
cleared ([#2211](https://github.com/androidx/media/issues/2211)).
* UI: * UI:
* Add `PlaybackSpeedState` class and the corresponding * Add `PlaybackSpeedState` class and the corresponding
`rememberPlaybackSpeedState` Composable to `media3-ui-compose` module. `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) { public void updateNotification(MediaSession session, boolean startInForegroundRequired) {
if (!mediaSessionService.isSessionAdded(session) || !shouldShowNotification(session)) { if (!mediaSessionService.isSessionAdded(session) || !shouldShowNotification(session)) {
maybeStopForegroundService(/* removeNotifications= */ true); removeNotification();
return; return;
} }
@ -275,22 +275,16 @@ import java.util.concurrent.TimeoutException;
// (https://github.com/androidx/media/issues/192). // (https://github.com/androidx/media/issues/192).
notificationManagerCompat.notify( notificationManagerCompat.notify(
mediaNotification.notificationId, mediaNotification.notification); mediaNotification.notificationId, mediaNotification.notification);
maybeStopForegroundService(/* removeNotifications= */ false); stopForeground(/* removeNotifications= */ false);
} }
} }
/** /** Removes the notification and stops the foreground service if running. */
* Stops the service from the foreground, if no player is actively playing content. private void removeNotification() {
* // To hide the notification on all API levels, we need to call both Service.stopForeground(true)
* @param removeNotifications Whether to remove notifications, if the service is stopped from the // and notificationManagerCompat.cancel(notificationId).
* foreground. stopForeground(/* removeNotifications= */ true);
*/ if (mediaNotification != null) {
private void maybeStopForegroundService(boolean removeNotifications) {
if (shouldRunInForeground(/* startInForegroundWhenPaused= */ false)) {
return;
}
stopForeground(removeNotifications);
if (removeNotifications && mediaNotification != null) {
notificationManagerCompat.cancel(mediaNotification.notificationId); notificationManagerCompat.cancel(mediaNotification.notificationId);
// Update the notification count so that if a pending notification callback arrives (e.g., a // Update the notification count so that if a pending notification callback arrives (e.g., a
// bitmap is loaded), we don't show the notification. // bitmap is loaded), we don't show the notification.
@ -419,8 +413,6 @@ import java.util.concurrent.TimeoutException;
} }
private void stopForeground(boolean removeNotifications) { 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) { if (Util.SDK_INT >= 24) {
Api24.stopForeground(mediaSessionService, removeNotifications); Api24.stopForeground(mediaSessionService, removeNotifications);
} else { } else {