From 9ca8540f85fba2ff5c8ff151706f36e7b30b46e6 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 11 Apr 2025 05:54:48 -0700 Subject: [PATCH] Ensure media notification provider can be updated Some interactions create a default notification provider if no custom one is set yet (e.g. setForegroundServiceTimeoutMs). This means a later call to setMediaNotificationProvider will silently fail to apply the new provider. This can be fixed by making the media notification provider updatable. Issue: androidx/media#2305 PiperOrigin-RevId: 746428193 --- RELEASENOTES.md | 5 ++++ .../session/MediaNotificationManager.java | 12 +++++++- .../media3/session/MediaSessionService.java | 21 +++++++------ .../session/MediaSessionServiceTest.java | 30 +++++++++++++++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5eb1e83125..e919d3082d 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -73,6 +73,11 @@ player doesn't have `COMMAND_GET_TIMELINE` available while `COMMAND_GET_CURRENT_MEDIA_ITEM` is available and the wrapped player is empty ([#2320](https://github.com/androidx/media/issues/2320)). + * Fix a bug where calling + `MediaSessionService.setMediaNotificationProvider` is silently ignored + after other interactions with the service like + `setForegroundServiceTimeoutMs` + ([#2305](https://github.com/androidx/media/issues/2305)). * UI: * Enable `PlayerSurface` to work with `ExoPlayer.setVideoEffects` and `CompositionPlayer`. 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 f624700f68..8e45a67159 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java @@ -60,7 +60,7 @@ import java.util.concurrent.TimeoutException; private static final int MSG_USER_ENGAGED_TIMEOUT = 1; private final MediaSessionService mediaSessionService; - private final MediaNotification.Provider mediaNotificationProvider; + private final MediaNotification.ActionFactory actionFactory; private final NotificationManagerCompat notificationManagerCompat; private final Handler mainHandler; @@ -68,6 +68,7 @@ import java.util.concurrent.TimeoutException; private final Intent startSelfIntent; private final Map controllerMap; + private MediaNotification.Provider mediaNotificationProvider; private int totalNotificationCount; @Nullable private MediaNotification mediaNotification; private boolean startedInForeground; @@ -146,6 +147,15 @@ import java.util.concurrent.TimeoutException; }); } + /** + * Updates the media notification provider. + * + * @param mediaNotificationProvider The {@link MediaNotification.Provider}. + */ + public void setMediaNotificationProvider(MediaNotification.Provider mediaNotificationProvider) { + this.mediaNotificationProvider = mediaNotificationProvider; + } + /** * Updates the notification. * diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java index 541db78088..35107f19bc 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java @@ -180,9 +180,6 @@ public abstract class MediaSessionService extends Service { @GuardedBy("lock") private @MonotonicNonNull MediaNotificationManager mediaNotificationManager; - @GuardedBy("lock") - private MediaNotification.@MonotonicNonNull Provider mediaNotificationProvider; - @GuardedBy("lock") private @MonotonicNonNull DefaultActionFactory actionFactory; @@ -637,8 +634,6 @@ public abstract class MediaSessionService extends Service { /** * Sets the {@link MediaNotification.Provider} to customize notifications. * - *

This should be called before {@link #onCreate()} returns. - * *

This method can be called from any thread. */ @UnstableApi @@ -646,7 +641,8 @@ public abstract class MediaSessionService extends Service { MediaNotification.Provider mediaNotificationProvider) { checkNotNull(mediaNotificationProvider); synchronized (lock) { - this.mediaNotificationProvider = mediaNotificationProvider; + getMediaNotificationManager(/* initialMediaNotificationProvider= */ mediaNotificationProvider) + .setMediaNotificationProvider(mediaNotificationProvider); } } @@ -679,16 +675,23 @@ public abstract class MediaSessionService extends Service { } private MediaNotificationManager getMediaNotificationManager() { + return getMediaNotificationManager(/* initialMediaNotificationProvider= */ null); + } + + private MediaNotificationManager getMediaNotificationManager( + @Nullable MediaNotification.Provider initialMediaNotificationProvider) { synchronized (lock) { if (mediaNotificationManager == null) { - if (mediaNotificationProvider == null) { + if (initialMediaNotificationProvider == null) { checkStateNotNull(getBaseContext(), "Accessing service context before onCreate()"); - mediaNotificationProvider = + initialMediaNotificationProvider = new DefaultMediaNotificationProvider.Builder(getApplicationContext()).build(); } mediaNotificationManager = new MediaNotificationManager( - /* mediaSessionService= */ this, mediaNotificationProvider, getActionFactory()); + /* mediaSessionService= */ this, + initialMediaNotificationProvider, + getActionFactory()); } return mediaNotificationManager; } diff --git a/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java index 97daf26a99..751833933c 100644 --- a/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java +++ b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java @@ -504,6 +504,36 @@ public class MediaSessionServiceTest { serviceController.destroy(); } + @Test + public void setMediaNotificationProvider_afterSetForegroundServiceTimeoutMs_usesCustomProvider() + throws TimeoutException { + Context context = ApplicationProvider.getApplicationContext(); + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + MediaSession session = new MediaSession.Builder(context, player).build(); + ServiceController serviceController = Robolectric.buildService(TestService.class); + TestService service = serviceController.create().get(); + + service.setForegroundServiceTimeoutMs(100); + service.setMediaNotificationProvider( + new DefaultMediaNotificationProvider( + service, + /* notificationIdProvider= */ mediaSession -> 2000, + DefaultMediaNotificationProvider.DEFAULT_CHANNEL_ID, + DefaultMediaNotificationProvider.DEFAULT_CHANNEL_NAME_RESOURCE_ID)); + service.addSession(session); + // Start a player to trigger notification creation. + player.setMediaItem(MediaItem.fromUri("asset:///media/mp4/sample.mp4")); + player.prepare(); + player.play(); + runMainLooperUntil(() -> notificationManager.getActiveNotifications().length == 1); + + assertThat(getStatusBarNotification(/* notificationId= */ 2000)).isNotNull(); + + session.release(); + player.release(); + serviceController.destroy(); + } + @Test public void onStartCommand_mediaButtonEvent_pausedByMediaNotificationController() throws InterruptedException {