Ensure pending commands are still sent in MediaController.release()

We currently clear all pending messages, including the one that flushes
pending commands to the MediaSession. To ensure all commands that have
been called before controller.release() are still sent, we can manually
trigger the flush message from the release call.

Related to handling the final flush because disconnecting the controller,
MediaSessionStub didn't post the removal of the controller to the
session thread, creating a race condition between removing the controller
and actually handling the flush.

Issue: androidx/media#99
PiperOrigin-RevId: 462342860

RELEASENOTES.md

@@ -25,6 +25,8 @@
         channel name used by the provider. Also, add method
         channel name used by the provider. Also, add method
         `DefaultNotificationProvider.setSmallIcon(int)` to set the notifications
         `DefaultNotificationProvider.setSmallIcon(int)` to set the notifications
         small icon ([#104](https://github.com/androidx/media/issues/104)).
         small icon ([#104](https://github.com/androidx/media/issues/104)).
+    *   Ensure commands sent before `MediaController.release()` are not dropped
+        ([#99](https://github.com/androidx/media/issues/99)).
 
 
 ### 1.0.0-beta02 (2022-07-15)
 ### 1.0.0-beta02 (2022-07-15)
 
 

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

@@ -320,7 +320,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
       serviceConnection = null;
       serviceConnection = null;
     }
     }
     connectedToken = null;
     connectedToken = null;
-    flushCommandQueueHandler.removeCallbacksAndMessages(/* token= */ null);
+    flushCommandQueueHandler.release();
     this.iSession = null;
     this.iSession = null;
     controllerStub.destroy();
     controllerStub.destroy();
     if (iSession != null) {
     if (iSession != null) {
@@ -3070,17 +3070,39 @@ import org.checkerframework.checker.nullness.qual.NonNull;
     }
     }
   }
   }
 
 
-  private class FlushCommandQueueHandler extends Handler {
+  private class FlushCommandQueueHandler {
 
 
     private static final int MSG_FLUSH_COMMAND_QUEUE = 1;
     private static final int MSG_FLUSH_COMMAND_QUEUE = 1;
 
 
+    private final Handler handler;
+
     public FlushCommandQueueHandler(Looper looper) {
     public FlushCommandQueueHandler(Looper looper) {
-      super(looper);
+      handler = new Handler(looper, /* callback= */ this::handleMessage);
     }
     }
 
 
-    @Override
-    public void handleMessage(Message msg) {
+    public void sendFlushCommandQueueMessage() {
+      if (iSession != null && !handler.hasMessages(MSG_FLUSH_COMMAND_QUEUE)) {
+        // Send message to notify the end of the transaction. It will be handled when the current
+        // looper iteration is over.
+        handler.sendEmptyMessage(MSG_FLUSH_COMMAND_QUEUE);
+      }
+    }
+
+    public void release() {
+      if (handler.hasMessages(MSG_FLUSH_COMMAND_QUEUE)) {
+        flushCommandQueue();
+      }
+      handler.removeCallbacksAndMessages(/* token= */ null);
+    }
+
+    private boolean handleMessage(Message msg) {
       if (msg.what == MSG_FLUSH_COMMAND_QUEUE) {
       if (msg.what == MSG_FLUSH_COMMAND_QUEUE) {
+        flushCommandQueue();
+      }
+      return true;
+    }
+
+    private void flushCommandQueue() {
       try {
       try {
         iSession.flushCommandQueue(controllerStub);
         iSession.flushCommandQueue(controllerStub);
       } catch (RemoteException e) {
       } catch (RemoteException e) {
@@ -3089,15 +3111,6 @@ import org.checkerframework.checker.nullness.qual.NonNull;
     }
     }
   }
   }
 
 
-    public void sendFlushCommandQueueMessage() {
-      if (iSession != null && !hasMessages(MSG_FLUSH_COMMAND_QUEUE)) {
-        // Send message to notify the end of the transaction. It will be handled when the current
-        // looper iteration is over.
-        sendEmptyMessage(MSG_FLUSH_COMMAND_QUEUE);
-      }
-    }
-  }
-
   private static final class PeriodInfo {
   private static final class PeriodInfo {
     private final int index;
     private final int index;
     private final long periodPositionUs;
     private final long periodPositionUs;

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

@@ -552,7 +552,13 @@ import java.util.concurrent.ExecutionException;
     }
     }
     long token = Binder.clearCallingIdentity();
     long token = Binder.clearCallingIdentity();
     try {
     try {
-      connectedControllersManager.removeController(caller.asBinder());
+      @Nullable MediaSessionImpl sessionImpl = this.sessionImpl.get();
+      if (sessionImpl == null || sessionImpl.isReleased()) {
+        return;
+      }
+      postOrRun(
+          sessionImpl.getApplicationHandler(),
+          () -> connectedControllersManager.removeController(caller.asBinder()));
     } finally {
     } finally {
       Binder.restoreCallingIdentity(token);
       Binder.restoreCallingIdentity(token);
     }
     }

libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java

@@ -225,4 +225,27 @@ public class MediaSessionAndControllerTest {
     assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
     assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
     assertThat(playWhenReadyRef.get()).isEqualTo(testPlayWhenReady);
     assertThat(playWhenReadyRef.get()).isEqualTo(testPlayWhenReady);
   }
   }
+
+  @Test
+  public void commandBeforeControllerRelease_handledBySession() throws Exception {
+    MockPlayer player =
+        new MockPlayer.Builder().setApplicationLooper(Looper.getMainLooper()).build();
+    MediaSession session =
+        sessionTestRule.ensureReleaseAfterTest(
+            new MediaSession.Builder(context, player).setId(TAG).build());
+    MediaController controller = controllerTestRule.createController(session.getToken());
+
+    threadTestRule
+        .getHandler()
+        .postAndSync(
+            () -> {
+              controller.prepare();
+              controller.play();
+              controller.release();
+            });
+
+    // Assert these methods are called without timing out.
+    player.awaitMethodCalled(MockPlayer.METHOD_PREPARE, TIMEOUT_MS);
+    player.awaitMethodCalled(MockPlayer.METHOD_PLAY, TIMEOUT_MS);
+  }
 }
 }