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: #99 PiperOrigin-RevId: 462342860
androidx · Jul 21, 2022 · ee20969 · ee20969
1 parent 287c757
commit ee20969
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 16 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -25,6 +25,8 @@
         channel name used by the provider. Also, add method
         `DefaultNotificationProvider.setSmallIcon(int)` to set the notifications
         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)
 

diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java
@@ -320,7 +320,7 @@ public void release() {
       serviceConnection = null;
     }
     connectedToken = null;
-    flushCommandQueueHandler.removeCallbacksAndMessages(/* token= */ null);
+    flushCommandQueueHandler.release();
     this.iSession = null;
     controllerStub.destroy();
     if (iSession != null) {
@@ -3070,30 +3070,43 @@ public void onSurfaceTextureUpdated(SurfaceTexture surface) {
     }
   }
 
-  private class FlushCommandQueueHandler extends Handler {
+  private class FlushCommandQueueHandler {
 
     private static final int MSG_FLUSH_COMMAND_QUEUE = 1;
 
+    private final Handler handler;
+
     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) {
-        try {
-          iSession.flushCommandQueue(controllerStub);
-        } catch (RemoteException e) {
-          Log.w(TAG, "Error in sending flushCommandQueue");
-        }
+        flushCommandQueue();
       }
+      return true;
     }
 
-    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 void flushCommandQueue() {
+      try {
+        iSession.flushCommandQueue(controllerStub);
+      } catch (RemoteException e) {
+        Log.w(TAG, "Error in sending flushCommandQueue");
       }
     }
   }

diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java
@@ -552,7 +552,13 @@ public void release(@Nullable IMediaController caller, int sequenceNumber)
     }
     long token = Binder.clearCallingIdentity();
     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 {
       Binder.restoreCallingIdentity(token);
     }

diff --git a/...n_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java b/...n_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java
@@ -225,4 +225,27 @@ public void onPlayWhenReadyChanged(boolean playWhenReady, int reason) {
     assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
     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);
+  }
 }