Merge pull request #59 from h6ah4i:fix/cleanup-callbacks-on-media-controller-release

PiperOrigin-RevId: 441253378

RELEASENOTES.md

@@ -65,6 +65,9 @@
         ([#47](https://github.com/androidx/media/pull/47)).
     *   Add RTP reader for WAV
         ([#56](https://github.com/androidx/media/pull/56)).
+*   Session:
+    *   Fix NPE in MediaControllerImplLegacy
+        ([#59](https://github.com/androidx/media/pull/59))
 *   Data sources:
     *   Rename `DummyDataSource` to `PlaceHolderDataSource`.
 *   Remove deprecated symbols:

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

@@ -415,6 +415,7 @@ public class MediaController implements Player {
       return;
     }
     released = true;
+    applicationHandler.removeCallbacksAndMessages(null);
     try {
       impl.release();
     } catch (Exception e) {

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

@@ -192,7 +192,7 @@ public class MediaControllerTest {
   }
 
   @Test
-  public void isConnected_afterDisconnection_returnsFalse() throws Exception {
+  public void isConnected_afterDisconnectionBySessionRelease_returnsFalse() throws Exception {
     CountDownLatch disconnectedLatch = new CountDownLatch(1);
     MediaController controller =
         controllerTestRule.createController(
@@ -210,6 +210,43 @@ public class MediaControllerTest {
     assertThat(controller.isConnected()).isFalse();
   }
 
+  @Test
+  public void isConnected_afterDisconnectionByControllerRelease_returnsFalse() throws Exception {
+    CountDownLatch latch = new CountDownLatch(1);
+    MediaController controller =
+        controllerTestRule.createController(
+            remoteSession.getToken(),
+            /* connectionHints= */ null,
+            new MediaController.Listener() {
+              @Override
+              public void onDisconnected(MediaController controller) {
+                latch.countDown();
+              }
+            });
+    threadTestRule.getHandler().postAndSync(controller::release);
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(controller.isConnected()).isFalse();
+  }
+
+  @Test
+  public void isConnected_afterDisconnectionByControllerReleaseRightAfterCreated_returnsFalse()
+      throws Exception {
+    CountDownLatch latch = new CountDownLatch(1);
+    MediaController controller =
+        controllerTestRule.createController(
+            remoteSession.getToken(),
+            /* connectionHints= */ null,
+            new MediaController.Listener() {
+              @Override
+              public void onDisconnected(MediaController controller) {
+                latch.countDown();
+              }
+            },
+            /* controllerCreationListener= */ MediaController::release);
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(controller.isConnected()).isFalse();
+  }
+
   @Test
   public void close_twice() throws Exception {
     MediaController controller = controllerTestRule.createController(remoteSession.getToken());

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

@@ -22,13 +22,16 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import android.content.Context;
 import android.os.Bundle;
 import android.support.v4.media.session.MediaSessionCompat;
+import androidx.annotation.MainThread;
 import androidx.annotation.Nullable;
 import androidx.collection.ArrayMap;
 import androidx.media3.common.util.Log;
 import androidx.media3.test.session.common.HandlerThreadTestRule;
 import androidx.test.core.app.ApplicationProvider;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.MoreExecutors;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 import org.junit.rules.ExternalResource;
 
 /**
@@ -44,6 +47,13 @@ public final class MediaControllerTestRule extends ExternalResource {
   private volatile Class<? extends MediaController> controllerType;
   private volatile long timeoutMs;
 
+  /** Listener to get notified when a controller has been created. */
+  public interface MediaControllerCreationListener {
+    /** Called immediately after the given controller has been created. */
+    @MainThread
+    void onCreated(MediaController controller);
+  }
+
   public MediaControllerTestRule(HandlerThreadTestRule handlerThreadTestRule) {
     this.handlerThreadTestRule = handlerThreadTestRule;
     controllers = new ArrayMap<>();
@@ -96,17 +106,31 @@ public final class MediaControllerTestRule extends ExternalResource {
   public MediaController createController(
       MediaSessionCompat.Token token, @Nullable MediaController.Listener listener)
       throws Exception {
+    return createController(token, listener, /* controllerCreateListener= */ null);
+  }
+
+  /** Creates {@link MediaController} from {@link MediaSessionCompat.Token}. */
+  public MediaController createController(
+      MediaSessionCompat.Token token,
+      @Nullable MediaController.Listener listener,
+      @Nullable MediaControllerCreationListener controllerCreationListener)
+      throws Exception {
     TestMediaBrowserListener testListener = new TestMediaBrowserListener(listener);
-    MediaController controller = createControllerOnHandler(token, testListener);
+    MediaController controller =
+        createControllerOnHandler(token, testListener, controllerCreationListener);
     controllers.put(controller, testListener);
     return controller;
   }
 
   private MediaController createControllerOnHandler(
-      MediaSessionCompat.Token token, TestMediaBrowserListener listener) throws Exception {
+      MediaSessionCompat.Token token,
+      TestMediaBrowserListener listener,
+      @Nullable MediaControllerCreationListener controllerCreationListener)
+      throws Exception {
     SessionToken sessionToken =
         SessionToken.createSessionToken(context, token).get(TIMEOUT_MS, MILLISECONDS);
-    return createControllerOnHandler(sessionToken, /* connectionHints= */ null, listener);
+    return createControllerOnHandler(
+        sessionToken, /* connectionHints= */ null, listener, controllerCreationListener);
   }
 
   /** Creates {@link MediaController} from {@link SessionToken} with default options. */
@@ -120,14 +144,29 @@ public final class MediaControllerTestRule extends ExternalResource {
       @Nullable Bundle connectionHints,
       @Nullable MediaController.Listener listener)
       throws Exception {
+    return createController(
+        token, connectionHints, listener, /* controllerCreationListener= */ null);
+  }
+
+  /** Creates {@link MediaController} from {@link SessionToken}. */
+  public MediaController createController(
+      SessionToken token,
+      @Nullable Bundle connectionHints,
+      @Nullable MediaController.Listener listener,
+      @Nullable MediaControllerCreationListener controllerCreationListener)
+      throws Exception {
     TestMediaBrowserListener testListener = new TestMediaBrowserListener(listener);
-    MediaController controller = createControllerOnHandler(token, connectionHints, testListener);
+    MediaController controller =
+        createControllerOnHandler(token, connectionHints, testListener, controllerCreationListener);
     controllers.put(controller, testListener);
     return controller;
   }
 
   private MediaController createControllerOnHandler(
-      SessionToken token, @Nullable Bundle connectionHints, TestMediaBrowserListener listener)
+      SessionToken token,
+      @Nullable Bundle connectionHints,
+      TestMediaBrowserListener listener,
+      @Nullable MediaControllerCreationListener controllerCreationListener)
       throws Exception {
     // Create controller on the test handler, for changing MediaBrowserCompat's Handler
     // Looper. Otherwise, MediaBrowserCompat will post all the commands to the handler
@@ -153,6 +192,22 @@ public final class MediaControllerTestRule extends ExternalResource {
                     return builder.buildAsync();
                   }
                 });
+
+    if (controllerCreationListener != null) {
+      future.addListener(
+          () -> {
+            @Nullable MediaController mediaController = null;
+            try {
+              mediaController = future.get();
+            } catch (ExecutionException | InterruptedException e) {
+              Log.e(TAG, "failed getting a controller", e);
+            }
+            if (mediaController != null) {
+              controllerCreationListener.onCreated(mediaController);
+            }
+          },
+          MoreExecutors.directExecutor());
+    }
     return future.get(timeoutMs, MILLISECONDS);
   }
 

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

@@ -170,6 +170,23 @@ public class MediaControllerWithMediaSessionCompatTest {
     assertThat(controller.isConnected()).isFalse();
   }
 
+  @Test
+  public void disconnected_byControllerReleaseRightAfterCreated() throws Exception {
+    CountDownLatch latch = new CountDownLatch(1);
+    MediaController controller =
+        controllerTestRule.createController(
+            session.getSessionToken(),
+            new MediaController.Listener() {
+              @Override
+              public void onDisconnected(MediaController controller) {
+                latch.countDown();
+              }
+            },
+            /* controllerCreationListener= */ MediaController::release);
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(controller.isConnected()).isFalse();
+  }
+
   @Test
   public void close_twice_doesNotCrash() throws Exception {
     MediaController controller = controllerTestRule.createController(session.getSessionToken());