diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5795c7f31d..8dfb57e1c4 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -54,6 +54,9 @@ ([#296](https://github.com/androidx/media/issues/296)). * Fix issue where `Player.COMMAND_GET_CURRENT_MEDIA_ITEM` needs to be available to access metadata via the legacy `MediaSessionCompat`. + * Fix issue where `MediaSession` instances on a background thread cause + crashes when used in `MediaSessionService` + ([#318](https://github.com/androidx/media/issues/318)). * Audio: * Fix bug where some playbacks fail when tunneling is enabled and `AudioProcessors` are active, e.g. for gapless trimming diff --git a/libraries/session/build.gradle b/libraries/session/build.gradle index b64f025b9c..860137f376 100644 --- a/libraries/session/build.gradle +++ b/libraries/session/build.gradle @@ -43,6 +43,7 @@ dependencies { androidTestImplementation 'androidx.multidex:multidex:' + androidxMultidexVersion androidTestImplementation 'androidx.test:runner:' + androidxTestRunnerVersion testImplementation project(modulePrefix + 'test-utils') + testImplementation project(modulePrefix + 'test-utils-robolectric') testImplementation project(modulePrefix + 'lib-exoplayer') testImplementation 'org.robolectric:robolectric:' + robolectricVersion } diff --git a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java index ef453b7cc7..02488affc2 100644 --- a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java +++ b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java @@ -33,8 +33,6 @@ import android.app.NotificationManager; import android.content.Context; import android.graphics.Bitmap; import android.os.Bundle; -import android.os.Handler; -import android.os.Looper; import androidx.annotation.DoNotInline; import androidx.annotation.DrawableRes; import androidx.annotation.Nullable; @@ -245,7 +243,6 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi private final String channelId; @StringRes private final int channelNameResourceId; private final NotificationManager notificationManager; - private final Handler mainHandler; private @MonotonicNonNull OnBitmapLoadedFutureCallback pendingOnBitmapLoadedFutureCallback; @DrawableRes private int smallIconResourceId; @@ -278,7 +275,6 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi notificationManager = checkStateNotNull( (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE)); - mainHandler = new Handler(Looper.getMainLooper()); smallIconResourceId = R.drawable.media3_notification_small_icon; } @@ -346,7 +342,7 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi pendingOnBitmapLoadedFutureCallback, // This callback must be executed on the next looper iteration, after this method has // returned a media notification. - mainHandler::post); + mediaSession.getImpl().getApplicationHandler()::post); } } } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaNotification.java b/libraries/session/src/main/java/androidx/media3/session/MediaNotification.java index fae9ae1fee..e9ebd7bd5c 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotification.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotification.java @@ -35,6 +35,10 @@ public final class MediaNotification { /** * Creates {@linkplain NotificationCompat.Action actions} and {@linkplain PendingIntent pending * intents} for notifications. + * + *
All methods will be called on the {@link Player#getApplicationLooper() application thread} + * of the {@link Player} associated with the {@link MediaSession} the notification is provided + * for. */ @UnstableApi public interface ActionFactory { @@ -109,10 +113,20 @@ public final class MediaNotification { * *
The provider is required to create a {@linkplain androidx.core.app.NotificationChannelCompat * notification channel}, which is required to show notification for {@code SDK_INT >= 26}. + * + *
All methods will be called on the {@link Player#getApplicationLooper() application thread} + * of the {@link Player} associated with the {@link MediaSession} the notification is provided + * for. */ @UnstableApi public interface Provider { - /** Receives updates for a notification. */ + /** + * Receives updates for a notification. + * + *
All methods will be called on the {@link Player#getApplicationLooper() application thread} + * of the {@link Player} associated with the {@link MediaSession} the notification is provided + * for. + */ interface Callback { /** * Called when a {@link MediaNotification} is changed. 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 30afcb411b..a65bbf2837 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java @@ -50,6 +50,8 @@ import java.util.concurrent.TimeoutException; /** * Manages media notifications for a {@link MediaSessionService} and sets the service as * foreground/background according to the player state. + * + *
All methods must be called on the main thread.
*/
/* package */ final class MediaNotificationManager {
@@ -96,11 +98,12 @@ import java.util.concurrent.TimeoutException;
.setListener(listener)
.setApplicationLooper(Looper.getMainLooper())
.buildAsync();
+ controllerMap.put(session, controllerFuture);
controllerFuture.addListener(
() -> {
try {
MediaController controller = controllerFuture.get(/* time= */ 0, TimeUnit.MILLISECONDS);
- listener.onConnected();
+ listener.onConnected(shouldShowNotification(session));
controller.addListener(listener);
} catch (CancellationException
| ExecutionException
@@ -111,7 +114,6 @@ import java.util.concurrent.TimeoutException;
}
},
mainExecutor);
- controllerMap.put(session, controllerFuture);
}
public void removeSession(MediaSession session) {
@@ -123,46 +125,19 @@ import java.util.concurrent.TimeoutException;
}
public void onCustomAction(MediaSession session, String action, Bundle extras) {
- @Nullable ListenableFuture This method must be called on the main thread.
+ */
/* package */ final void setListener(Listener listener) {
impl.setMediaSessionListener(listener);
}
- /** Clears the {@linkplain Listener listener}. */
+ /**
+ * Clears the {@linkplain Listener listener}.
+ *
+ * This method must be called on the main thread.
+ */
/* package */ final void clearListener() {
impl.clearMediaSessionListener();
}
@@ -1439,7 +1447,11 @@ public class MediaSession {
default void onRenderedFirstFrame(int seq) throws RemoteException {}
}
- /** Listener for media session events */
+ /**
+ * Listener for media session events.
+ *
+ * All methods must be called on the main thread.
+ */
/* package */ interface Listener {
/**
diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
index 3fac801364..b593447dfd 100644
--- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
+++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
@@ -69,9 +69,11 @@ import androidx.media3.session.SequencedFutureManager.SequencedFuture;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.SettableFuture;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.ExecutionException;
import org.checkerframework.checker.initialization.qual.Initialized;
/* package */ class MediaSessionImpl {
@@ -113,8 +115,10 @@ import org.checkerframework.checker.initialization.qual.Initialized;
private final Handler applicationHandler;
private final BitmapLoader bitmapLoader;
private final Runnable periodicSessionPositionInfoUpdateRunnable;
+ private final Handler mainHandler;
@Nullable private PlayerListener playerListener;
+
@Nullable private MediaSession.Listener mediaSessionListener;
private PlayerInfo playerInfo;
@@ -149,6 +153,7 @@ import org.checkerframework.checker.initialization.qual.Initialized;
sessionStub = new MediaSessionStub(thisRef);
this.sessionActivity = sessionActivity;
+ mainHandler = new Handler(Looper.getMainLooper());
applicationHandler = new Handler(player.getApplicationLooper());
this.callback = callback;
this.bitmapLoader = bitmapLoader;
@@ -546,12 +551,25 @@ import org.checkerframework.checker.initialization.qual.Initialized;
}
/* package */ void onNotificationRefreshRequired() {
- if (this.mediaSessionListener != null) {
- this.mediaSessionListener.onNotificationRefreshRequired(instance);
- }
+ postOrRun(
+ mainHandler,
+ () -> {
+ if (this.mediaSessionListener != null) {
+ this.mediaSessionListener.onNotificationRefreshRequired(instance);
+ }
+ });
}
/* package */ boolean onPlayRequested() {
+ if (Looper.myLooper() != Looper.getMainLooper()) {
+ SettableFuture Override this method if you need your own initialization.
+ *
+ * This method will be called on the main thread.
*/
@CallSuper
@Override
@@ -234,7 +234,7 @@ public abstract class MediaSessionService extends Service {
* For those special cases, the values returned by {@link ControllerInfo#getUid()} and {@link
* ControllerInfo#getConnectionHints()} have no meaning.
*
- * This method is always called on the main thread.
+ * This method will be called on the main thread.
*
* @param controllerInfo The information of the controller that is trying to connect.
* @return A {@link MediaSession} for the controller, or {@code null} to reject the connection.
@@ -251,6 +251,8 @@ public abstract class MediaSessionService extends Service {
* The added session will be removed automatically {@linkplain MediaSession#release() when the
* session is released}.
*
+ * This method can be called from any thread.
+ *
* @param session A session to be added.
* @see #removeSession(MediaSession)
* @see #getSessions()
@@ -268,8 +270,12 @@ public abstract class MediaSessionService extends Service {
// Session has returned for the first time. Register callbacks.
// TODO(b/191644474): Check whether the session is registered to multiple services.
MediaNotificationManager notificationManager = getMediaNotificationManager();
- postOrRun(mainHandler, () -> notificationManager.addSession(session));
- session.setListener(new MediaSessionListener());
+ postOrRun(
+ mainHandler,
+ () -> {
+ notificationManager.addSession(session);
+ session.setListener(new MediaSessionListener());
+ });
}
}
@@ -277,6 +283,8 @@ public abstract class MediaSessionService extends Service {
* Removes a {@link MediaSession} from this service. This is not necessary for most media apps.
* See Supporting Multiple Sessions for details.
*
+ * This method can be called from any thread.
+ *
* @param session A session to be removed.
* @see #addSession(MediaSession)
* @see #getSessions()
@@ -288,13 +296,19 @@ public abstract class MediaSessionService extends Service {
sessions.remove(session.getId());
}
MediaNotificationManager notificationManager = getMediaNotificationManager();
- postOrRun(mainHandler, () -> notificationManager.removeSession(session));
- session.clearListener();
+ postOrRun(
+ mainHandler,
+ () -> {
+ notificationManager.removeSession(session);
+ session.clearListener();
+ });
}
/**
* Returns the list of {@linkplain MediaSession sessions} that you've added to this service via
* {@link #addSession} or {@link #onGetSession(ControllerInfo)}.
+ *
+ * This method can be called from any thread.
*/
public final List This method can be called from any thread.
*/
public final boolean isSessionAdded(MediaSession session) {
synchronized (lock) {
@@ -312,7 +328,11 @@ public abstract class MediaSessionService extends Service {
}
}
- /** Sets the {@linkplain Listener listener}. */
+ /**
+ * Sets the {@linkplain Listener listener}.
+ *
+ * This method can be called from any thread.
+ */
@UnstableApi
public final void setListener(Listener listener) {
synchronized (lock) {
@@ -320,7 +340,11 @@ public abstract class MediaSessionService extends Service {
}
}
- /** Clears the {@linkplain Listener listener}. */
+ /**
+ * Clears the {@linkplain Listener listener}.
+ *
+ * This method can be called from any thread.
+ */
@UnstableApi
public final void clearListener() {
synchronized (lock) {
@@ -335,6 +359,8 @@ public abstract class MediaSessionService extends Service {
* controllers}. In this case, the intent will have the action {@link #SERVICE_INTERFACE}.
* Override this method if this service also needs to handle actions other than {@link
* #SERVICE_INTERFACE}.
+ *
+ * This method will be called on the main thread.
*/
@CallSuper
@Override
@@ -378,6 +404,8 @@ public abstract class MediaSessionService extends Service {
* The default implementation handles the incoming media button events. In this case, the
* intent will have the action {@link Intent#ACTION_MEDIA_BUTTON}. Override this method if this
* service also needs to handle actions other than {@link Intent#ACTION_MEDIA_BUTTON}.
+ *
+ * This method will be called on the main thread.
*/
@CallSuper
@Override
@@ -417,6 +445,8 @@ public abstract class MediaSessionService extends Service {
* Called when the service is no longer used and is being removed.
*
* Override this method if you need your own clean up.
+ *
+ * This method will be called on the main thread.
*/
@CallSuper
@Override
@@ -435,7 +465,7 @@ public abstract class MediaSessionService extends Service {
*/
@Deprecated
public void onUpdateNotification(MediaSession session) {
- setDefaultMethodCalled(true);
+ defaultMethodCalled = true;
}
/**
@@ -460,13 +490,15 @@ public abstract class MediaSessionService extends Service {
* Apps targeting {@code SDK_INT >= 28} must request the permission, {@link
* android.Manifest.permission#FOREGROUND_SERVICE}.
*
+ * This method will be called on the main thread.
+ *
* @param session A session that needs notification update.
* @param startInForegroundRequired Whether the service is required to start in the foreground.
*/
@SuppressWarnings("deprecation") // Calling deprecated method.
public void onUpdateNotification(MediaSession session, boolean startInForegroundRequired) {
onUpdateNotification(session);
- if (isDefaultMethodCalled()) {
+ if (defaultMethodCalled) {
getMediaNotificationManager().updateNotification(session, startInForegroundRequired);
}
}
@@ -475,6 +507,8 @@ 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
protected final void setMediaNotificationProvider(
@@ -491,11 +525,16 @@ public abstract class MediaSessionService extends Service {
}
}
+ /**
+ * Triggers notification update and handles {@code ForegroundServiceStartNotAllowedException}.
+ *
+ * This method will be called on the main thread.
+ */
/* package */ boolean onUpdateNotificationInternal(
MediaSession session, boolean startInForegroundWhenPaused) {
try {
boolean startInForegroundRequired =
- shouldRunInForeground(session, startInForegroundWhenPaused);
+ getMediaNotificationManager().shouldRunInForeground(session, startInForegroundWhenPaused);
onUpdateNotification(session, startInForegroundRequired);
} catch (/* ForegroundServiceStartNotAllowedException */ IllegalStateException e) {
if ((Util.SDK_INT >= 31) && Api31.instanceOfForegroundServiceStartNotAllowedException(e)) {
@@ -508,14 +547,6 @@ public abstract class MediaSessionService extends Service {
return true;
}
- /* package */ static boolean shouldRunInForeground(
- MediaSession session, boolean startInForegroundWhenPaused) {
- Player player = session.getPlayer();
- return (player.getPlayWhenReady() || startInForegroundWhenPaused)
- && (player.getPlaybackState() == Player.STATE_READY
- || player.getPlaybackState() == Player.STATE_BUFFERING);
- }
-
private MediaNotificationManager getMediaNotificationManager() {
synchronized (lock) {
if (mediaNotificationManager == null) {
@@ -547,18 +578,6 @@ public abstract class MediaSessionService extends Service {
}
}
- private boolean isDefaultMethodCalled() {
- synchronized (lock) {
- return this.defaultMethodCalled;
- }
- }
-
- private void setDefaultMethodCalled(boolean defaultMethodCalled) {
- synchronized (lock) {
- this.defaultMethodCalled = defaultMethodCalled;
- }
- }
-
@RequiresApi(31)
private void onForegroundServiceStartNotAllowedException() {
mainHandler.post(
diff --git a/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java
new file mode 100644
index 0000000000..fbac60257a
--- /dev/null
+++ b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package androidx.media3.session;
+
+import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil;
+import static com.google.common.truth.Truth8.assertThat;
+import static java.util.Arrays.stream;
+
+import android.app.NotificationManager;
+import android.content.Context;
+import android.os.Handler;
+import android.os.HandlerThread;
+import android.service.notification.StatusBarNotification;
+import androidx.annotation.Nullable;
+import androidx.media3.common.MediaItem;
+import androidx.media3.exoplayer.ExoPlayer;
+import androidx.media3.test.utils.TestExoPlayerBuilder;
+import androidx.test.core.app.ApplicationProvider;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.android.controller.ServiceController;
+import org.robolectric.shadows.ShadowLooper;
+
+@RunWith(AndroidJUnit4.class)
+public class MediaSessionServiceTest {
+
+ @Test
+ public void service_multipleSessionsOnMainThread_createsNotificationForEachSession() {
+ Context context = ApplicationProvider.getApplicationContext();
+ ExoPlayer player1 = new TestExoPlayerBuilder(context).build();
+ ExoPlayer player2 = new TestExoPlayerBuilder(context).build();
+ MediaSession session1 = new MediaSession.Builder(context, player1).setId("1").build();
+ MediaSession session2 = new MediaSession.Builder(context, player2).setId("2").build();
+ ServiceController