session: Avoid ForegroundServiceDidNotStartInTimeException...

...and throw more useful exceptions instead.

This will however not throw exceptions where foreground service
start is not absolutely required, and keep logging warnings in
these cases instead.

Issue: #2591
Issue: #2622

Author: nift4

libraries/session/src/main/java/androidx/media3/session/MediaSession.java

@@ -1896,6 +1896,14 @@ default ListenableFuture<MediaItemsWithStartPosition> onPlaybackResumption(
      * automatically as required. Any additional initial setup like setting playback speed, repeat
      * mode or shuffle mode can be done from within this callback.
      *
+     * <p>If the returned list is empty or an exception is returned, and the request to resume
+     * playback came from {@link MediaButtonReceiver}, an {@link IllegalStateException} will be
+     * thrown. This is because the {@link MediaButtonReceiver} has already requested a foreground
+     * service start, and it's not possible to avoid a {@code
+     * ForegroundServiceDidNotStartInTimeException} anymore. To avoid a crash as result, override
+     * {@link MediaButtonReceiver#shouldStartForegroundService(Context, Intent)} to return {@code
+     * false} if there is nothing to resume. This will avoid requesting a foreground service start.
+     *
      * <p>The method will only be called if the {@link Player} has {@link
      * Player#COMMAND_GET_CURRENT_MEDIA_ITEM} and either {@link Player#COMMAND_SET_MEDIA_ITEM} or
      * {@link Player#COMMAND_CHANGE_MEDIA_ITEMS} available.

libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java

@@ -1127,7 +1127,9 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
    * @param controller The controller requesting to play.
    */
   /* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
-      ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
+      ControllerInfo controller,
+      boolean callOnPlayerInteractionFinished,
+      boolean mustStartForegroundService) {
     SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
     ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
     playRequestedFuture.addListener(
@@ -1181,6 +1183,26 @@ public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
                     callWithControllerForCurrentRequestSet(
                             controllerForRequest,
                             () -> {
+                              if (mediaItemsWithStartPosition.mediaItems.isEmpty()) {
+                                if (mustStartForegroundService) {
+                                  applicationHandler.postAtFrontOfQueue(
+                                      () -> {
+                                        throw new IllegalArgumentException(
+                                            "Callback.onPlaybackResumption must return non-empty"
+                                                + " MediaItemsWithStartPosition if started from a"
+                                                + " media button receiver. If there is nothing to"
+                                                + " resume playback with, override"
+                                                + " MediaButtonReceiver.shouldStartForegroundService()"
+                                                + " and return false.");
+                                      });
+                                  return;
+                                }
+                                Log.w(
+                                    TAG,
+                                    "onPlaybackResumption() is trying to resume with empty"
+                                        + " playlist, this will make the resumption notification"
+                                        + " appear broken.");
+                              }
                               MediaUtils.setMediaItemsWithStartIndexAndPosition(
                                   playerWrapper, mediaItemsWithStartPosition);
                               Util.handlePlayButtonAction(playerWrapper);
@@ -1195,21 +1217,33 @@ public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
 
                   @Override
                   public void onFailure(Throwable t) {
+                    RuntimeException e;
                     if (t instanceof UnsupportedOperationException) {
-                      Log.w(
-                          TAG,
-                          "UnsupportedOperationException: Make sure to implement"
-                              + " MediaSession.Callback.onPlaybackResumption() if you add a media"
-                              + " button receiver to your manifest or if you implement the recent"
-                              + " media item contract with your MediaLibraryService.",
-                          t);
+                      e =
+                          new UnsupportedOperationException(
+                              "Make sure to implement MediaSession.Callback.onPlaybackResumption()"
+                                  + " if you add a media button receiver to your manifest or if you"
+                                  + " implement the recent media item contract with your"
+                                  + " MediaLibraryService.",
+                              t);
                     } else {
-                      Log.e(
-                          TAG,
-                          "Failure calling MediaSession.Callback.onPlaybackResumption(): "
-                              + t.getMessage(),
-                          t);
+                      e =
+                          new IllegalStateException(
+                              "Failure calling MediaSession.Callback.onPlaybackResumption(): "
+                                  + t.getMessage(),
+                              t);
+                    }
+                    if (mustStartForegroundService) {
+                      // MediaButtonReceiver already called startForegroundService(). If we do not
+                      // crash ourselves, ForegroundServiceDidNotStartInTimeException will do it
+                      // for us. Let's at least get a useful stack trace out there.
+                      applicationHandler.postAtFrontOfQueue(
+                          () -> {
+                            throw e;
+                          });
+                      return;
                     }
+                    Log.e(TAG, Objects.requireNonNull(Log.getThrowableString(e)));
                     // Play as requested even if playback resumption fails.
                     Util.handlePlayButtonAction(playerWrapper);
                     sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
@@ -1467,13 +1501,13 @@ private void handleAvailablePlayerCommandsChanged(Player.Commands availableComma
     // Double tap detection.
     int keyCode = keyEvent.getKeyCode();
     boolean isTvApp = context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_LEANBACK);
+    boolean isEventSourceMediaButtonReceiver =
+        callerInfo.getControllerVersion() != ControllerInfo.LEGACY_CONTROLLER_VERSION;
     boolean doubleTapCompleted = false;
     switch (keyCode) {
       case KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE:
       case KeyEvent.KEYCODE_HEADSETHOOK:
-        if (isTvApp
-            || callerInfo.getControllerVersion() != ControllerInfo.LEGACY_CONTROLLER_VERSION
-            || keyEvent.getRepeatCount() != 0) {
+        if (isTvApp || isEventSourceMediaButtonReceiver || keyEvent.getRepeatCount() != 0) {
           // Double tap detection is only for mobile apps that receive a media button event from
           // external sources (for instance Bluetooth) and excluding long press (repeatCount > 0).
           mediaPlayPauseKeyHandler.flush();
@@ -1513,11 +1547,18 @@ private void handleAvailablePlayerCommandsChanged(Player.Commands availableComma
         intent.getBooleanExtra(
             MediaNotification.NOTIFICATION_DISMISSED_EVENT_KEY, /* defaultValue= */ false);
     return keyEvent.getRepeatCount() > 0
-        || applyMediaButtonKeyEvent(keyEvent, doubleTapCompleted, isDismissNotificationEvent);
+        || applyMediaButtonKeyEvent(
+            keyEvent,
+            doubleTapCompleted,
+            isDismissNotificationEvent,
+            isEventSourceMediaButtonReceiver);
   }
 
   private boolean applyMediaButtonKeyEvent(
-      KeyEvent keyEvent, boolean doubleTapCompleted, boolean isDismissNotificationEvent) {
+      KeyEvent keyEvent,
+      boolean doubleTapCompleted,
+      boolean isDismissNotificationEvent,
+      boolean mustStartForegroundService) {
     ControllerInfo controllerInfo = checkNotNull(instance.getMediaNotificationControllerInfo());
     Runnable command;
     int keyCode = keyEvent.getKeyCode();
@@ -1530,10 +1571,15 @@ private boolean applyMediaButtonKeyEvent(
         command =
             getPlayerWrapper().getPlayWhenReady()
                 ? () -> sessionStub.pauseForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER)
-                : () -> sessionStub.playForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER);
+                : () ->
+                    sessionStub.playForControllerInfo(
+                        controllerInfo, UNKNOWN_SEQUENCE_NUMBER, mustStartForegroundService);
         break;
       case KEYCODE_MEDIA_PLAY:
-        command = () -> sessionStub.playForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER);
+        command =
+            () ->
+                sessionStub.playForControllerInfo(
+                    controllerInfo, UNKNOWN_SEQUENCE_NUMBER, mustStartForegroundService);
         break;
       case KEYCODE_MEDIA_PAUSE:
         command = () -> sessionStub.pauseForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER);
@@ -2115,7 +2161,8 @@ public void setPendingPlayPauseTask(ControllerInfo controllerInfo, KeyEvent keyE
               applyMediaButtonKeyEvent(
                   keyEvent,
                   /* doubleTapCompleted= */ false,
-                  /* isDismissNotificationEvent= */ false);
+                  /* isDismissNotificationEvent= */ false,
+                  /* mustStartForegroundService= */ false);
             } else {
               sessionLegacyStub.handleMediaPlayPauseOnHandler(
                   checkNotNull(controllerInfo.getRemoteUserInfo()));

libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java

@@ -610,7 +610,9 @@ public void onPlay() {
         controller -> {
           ListenableFuture<SessionResult> resultFuture =
               sessionImpl.handleMediaControllerPlayRequest(
-                  controller, /* callOnPlayerInteractionFinished= */ true);
+                  controller,
+                  /* callOnPlayerInteractionFinished= */ true,
+                  /* mustStartForegroundService= */ false);
           Futures.addCallback(
               resultFuture,
               new FutureCallback<SessionResult>() {

libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java

@@ -752,19 +752,22 @@ public void play(@Nullable IMediaController caller, int sequenceNumber) {
     @Nullable
     ControllerInfo controller = connectedControllersManager.getController(caller.asBinder());
     if (controller != null) {
-      playForControllerInfo(controller, sequenceNumber);
+      playForControllerInfo(controller, sequenceNumber, /* mustStartForegroundService= */ false);
     }
   }
 
-  public void playForControllerInfo(ControllerInfo controller, int sequenceNumber) {
+  public void playForControllerInfo(
+      ControllerInfo controller, int sequenceNumber, boolean mustStartForegroundService) {
     queueSessionTaskWithPlayerCommandForControllerInfo(
         controller,
         sequenceNumber,
         COMMAND_PLAY_PAUSE,
         sendSessionResultWhenReady(
             (session, theController, sequenceId) ->
                 session.handleMediaControllerPlayRequest(
-                    theController, /* callOnPlayerInteractionFinished= */ false)));
+                    theController,
+                    /* callOnPlayerInteractionFinished= */ false,
+                    /* mustStartForegroundService= */ mustStartForegroundService)));
   }
 
   @Override