Wait with PlayerInfo updates until all pending operations are done

Accepting a PlayerInfo while the MediaController is masking its state
means we are reverting all masking changes we've made earlier. This
only makes sense if the update already contains the masked operation.
If multiple operations are in flight (or are sent from the session
while they are in flight), we need to wait until all of them are
handled before accepting new updates.

In cases where a new update from the session excludes the Timeline
and the masked state is incompatible with the new update, we also
risk an exception if we accept the update too early.

PiperOrigin-RevId: 487266899

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

@@ -47,6 +47,7 @@ import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 import android.view.TextureView;
 import androidx.annotation.Nullable;
+import androidx.collection.ArraySet;
 import androidx.media3.common.AdPlaybackState;
 import androidx.media3.common.AudioAttributes;
 import androidx.media3.common.BundleListRetriever;
@@ -110,6 +111,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
   private final SurfaceCallback surfaceCallback;
   private final ListenerSet<Listener> listeners;
   private final FlushCommandQueueHandler flushCommandQueueHandler;
+  private final ArraySet<Integer> pendingMaskingSequencedFutureNumbers;
 
   @Nullable private SessionToken connectedToken;
   @Nullable private SessionServiceConnection serviceConnection;
@@ -127,6 +129,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
   @Nullable private IMediaSession iSession;
   private long lastReturnedCurrentPositionMs;
   private long lastSetPlayWhenReadyCalledTimeMs;
+  @Nullable private Timeline pendingPlayerInfoUpdateTimeline;
 
   public MediaControllerImplBase(
       Context context,
@@ -154,6 +157,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
     this.context = context;
     sequencedFutureManager = new SequencedFutureManager();
     controllerStub = new MediaControllerStub(this);
+    pendingMaskingSequencedFutureNumbers = new ArraySet<>();
     this.token = token;
     this.connectionHints = connectionHints;
     deathRecipient =
@@ -303,7 +307,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
     if (command != Player.COMMAND_SET_VIDEO_SURFACE) {
       flushCommandQueueHandler.sendFlushCommandQueueMessage();
     }
-    return dispatchRemoteSessionTask(iSession, task);
+    return dispatchRemoteSessionTask(iSession, task, /* addToPendingMaskingOperations= */ true);
   }
 
   private ListenableFuture<SessionResult> dispatchRemoteSessionTaskWithSessionCommand(
@@ -326,17 +330,22 @@ import org.checkerframework.checker.nullness.qual.NonNull;
         sessionCommand != null
             ? getSessionInterfaceWithSessionCommandIfAble(sessionCommand)
             : getSessionInterfaceWithSessionCommandIfAble(commandCode),
-        task);
+        task,
+        /* addToPendingMaskingOperations= */ false);
   }
 
   private ListenableFuture<SessionResult> dispatchRemoteSessionTask(
-      IMediaSession iSession, RemoteSessionTask task) {
+      IMediaSession iSession, RemoteSessionTask task, boolean addToPendingMaskingOperations) {
     if (iSession != null) {
       SequencedFutureManager.SequencedFuture<SessionResult> result =
           sequencedFutureManager.createSequencedFuture(
               new SessionResult(SessionResult.RESULT_INFO_SKIPPED));
       try {
-        task.run(iSession, result.getSequenceNumber());
+        int sequenceNumber = result.getSequenceNumber();
+        task.run(iSession, sequenceNumber);
+        if (addToPendingMaskingOperations) {
+          pendingMaskingSequencedFutureNumbers.add(sequenceNumber);
+        }
       } catch (RemoteException e) {
         Log.w(TAG, "Cannot connect to the service or the session is gone", e);
         result.set(new SessionResult(SessionResult.RESULT_ERROR_SESSION_DISCONNECTED));
@@ -2223,7 +2232,12 @@ import org.checkerframework.checker.nullness.qual.NonNull;
   }
 
   <T extends @NonNull Object> void setFutureResult(int seq, T futureResult) {
+    // Don't set the future result on the application looper so that the result can be obtained by a
+    // blocking future.get() on the application looper. But post a message to remove the pending
+    // masking operation on the application looper to ensure it's executed in order with other
+    // updates sent to the application looper.
     sequencedFutureManager.setFutureResult(seq, futureResult);
+    getInstance().runOnApplicationLooper(() -> pendingMaskingSequencedFutureNumbers.remove(seq));
   }
 
   void onConnected(ConnectionState result) {
@@ -2313,11 +2327,23 @@ import org.checkerframework.checker.nullness.qual.NonNull;
     if (!isConnected()) {
       return;
     }
+    if (!pendingMaskingSequencedFutureNumbers.isEmpty()) {
+      // We are still waiting for all pending masking operations to be handled.
+      if (!isTimelineExcluded) {
+        pendingPlayerInfoUpdateTimeline = newPlayerInfo.timeline;
+      }
+      return;
+    }
     PlayerInfo oldPlayerInfo = playerInfo;
     playerInfo = newPlayerInfo;
     if (isTimelineExcluded) {
-      playerInfo = playerInfo.copyWithTimeline(oldPlayerInfo.timeline);
+      playerInfo =
+          playerInfo.copyWithTimeline(
+              pendingPlayerInfoUpdateTimeline != null
+                  ? pendingPlayerInfoUpdateTimeline
+                  : oldPlayerInfo.timeline);
     }
+    pendingPlayerInfoUpdateTimeline = null;
     PlaybackException oldPlayerError = oldPlayerInfo.playerError;
     PlaybackException playerError = playerInfo.playerError;
     boolean errorsMatch =
@@ -2568,7 +2594,8 @@ import org.checkerframework.checker.nullness.qual.NonNull;
   }
 
   private void updateSessionPositionInfoIfNeeded(SessionPositionInfo sessionPositionInfo) {
-    if (playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) {
+    if (pendingMaskingSequencedFutureNumbers.isEmpty()
+        && playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) {
       playerInfo = playerInfo.copyWithSessionPositionInfo(sessionPositionInfo);
     }
   }

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

@@ -2878,6 +2878,85 @@ public class MediaControllerStateMaskingTest {
         /* testPreviousMediaItemIndex= */ C.INDEX_UNSET);
   }
 
+  @Test
+  public void incompatibleUpdatesDuringMasking_areOnlyReportedOnceAllPendingUpdatesAreResolved()
+      throws Exception {
+    // Test setup:
+    //  1. Report a discontinuity from item 0 to item 1 in the session.
+    //  2. Before (1) can be handled by the controller, remove item 1.
+    // Expectation:
+    //  - Session: State is updated to ENDED as the current item is removed.
+    //  - Controller: Discontinuity is only reported after the state is fully resolved
+    //     = The discontinuity is only reported once we also report the state change to ENDED.
+    Timeline timeline = MediaTestUtils.createTimeline(/* windowCount= */ 2);
+    remoteSession.getMockPlayer().setTimeline(timeline);
+    remoteSession
+        .getMockPlayer()
+        .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
+    remoteSession.getMockPlayer().setCurrentMediaItemIndex(0);
+    MediaController controller = controllerTestRule.createController(remoteSession.getToken());
+    CountDownLatch positionDiscontinuityReported = new CountDownLatch(1);
+    AtomicBoolean reportedStateChangeToEndedAtSameTimeAsDiscontinuity = new AtomicBoolean();
+    Player.Listener listener =
+        new Player.Listener() {
+          @Override
+          public void onEvents(Player player, Player.Events events) {
+            if (events.contains(Player.EVENT_POSITION_DISCONTINUITY)) {
+              if (events.contains(Player.EVENT_PLAYBACK_STATE_CHANGED)
+                  && player.getPlaybackState() == Player.STATE_ENDED) {
+                reportedStateChangeToEndedAtSameTimeAsDiscontinuity.set(true);
+              }
+              positionDiscontinuityReported.countDown();
+            }
+          }
+        };
+    threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener));
+
+    // Step 1: Report a discontinuity from item 0 to item 1 in the session.
+    PositionInfo oldPositionInfo =
+        new PositionInfo(
+            /* windowUid= */ timeline.getWindow(/* windowIndex= */ 0, new Window()).uid,
+            /* mediaItemIndex= */ 0,
+            MediaItem.EMPTY,
+            /* periodUid= */ timeline.getPeriod(
+                    /* periodIndex= */ 0, new Period(), /* setIds= */ true)
+                .uid,
+            /* periodIndex= */ 0,
+            /* positionMs= */ 10_000,
+            /* contentPositionMs= */ 10_000,
+            /* adGroupIndex= */ C.INDEX_UNSET,
+            /* adIndexInAdGroup= */ C.INDEX_UNSET);
+    PositionInfo newPositionInfo =
+        new PositionInfo(
+            /* windowUid= */ timeline.getWindow(/* windowIndex= */ 1, new Window()).uid,
+            /* mediaItemIndex= */ 1,
+            MediaItem.EMPTY,
+            /* periodUid= */ timeline.getPeriod(
+                    /* periodIndex= */ 1, new Period(), /* setIds= */ true)
+                .uid,
+            /* periodIndex= */ 1,
+            /* positionMs= */ 0,
+            /* contentPositionMs= */ 0,
+            /* adGroupIndex= */ C.INDEX_UNSET,
+            /* adIndexInAdGroup= */ C.INDEX_UNSET);
+    remoteSession.getMockPlayer().setCurrentMediaItemIndex(1);
+    remoteSession
+        .getMockPlayer()
+        .notifyPositionDiscontinuity(
+            oldPositionInfo, newPositionInfo, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
+    // Step 2: Before step 1 can be handled by the controller, remove item 1.
+    threadTestRule.getHandler().postAndSync(() -> controller.removeMediaItem(/* index= */ 1));
+    remoteSession.getMockPlayer().setCurrentMediaItemIndex(0);
+    remoteSession.getMockPlayer().setTimeline(MediaTestUtils.createTimeline(/* windowCount= */ 1));
+    remoteSession.getMockPlayer().notifyPlaybackStateChanged(Player.STATE_ENDED);
+    remoteSession
+        .getMockPlayer()
+        .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
+
+    assertThat(positionDiscontinuityReported.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(reportedStateChangeToEndedAtSameTimeAsDiscontinuity.get()).isTrue();
+  }
+
   private void assertMoveMediaItems(
       int initialMediaItemCount,
       int initialMediaItemIndex,

libraries/test_session_current/src/main/java/androidx/media3/session/MediaSessionProviderService.java

@@ -785,6 +785,13 @@ public class MediaSessionProviderService extends Service {
             MediaSession session = sessionMap.get(sessionId);
             MockPlayer player = (MockPlayer) session.getPlayer();
             player.timeline = Timeline.CREATOR.fromBundle(timelineBundle);
+            List<MediaItem> mediaItems = new ArrayList<>();
+            for (int i = 0; i < player.timeline.getWindowCount(); i++) {
+              mediaItems.add(
+                  player.timeline.getWindow(/* windowIndex= */ i, new Timeline.Window()).mediaItem);
+            }
+            player.mediaItems.clear();
+            player.mediaItems.addAll(mediaItems);
           });
     }
 
@@ -800,6 +807,8 @@ public class MediaSessionProviderService extends Service {
               mediaItems.add(
                   MediaTestUtils.createMediaItem(TestUtils.getMediaIdInFakeTimeline(windowIndex)));
             }
+            player.mediaItems.clear();
+            player.mediaItems.addAll(mediaItems);
             player.timeline = new PlaylistTimeline(mediaItems);
           });
     }