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
(cherry picked from commit 0b4ba3e)

Author: tonihei

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

@@ -47,6 +47,7 @@
 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 @@
   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 @@
   @Nullable private IMediaSession iSession;
   private long lastReturnedCurrentPositionMs;
   private long lastSetPlayWhenReadyCalledTimeMs;
+  @Nullable private Timeline pendingPlayerInfoUpdateTimeline;
 
   public MediaControllerImplBase(
       Context context,
@@ -154,6 +157,7 @@ public MediaControllerImplBase(
     this.context = context;
     sequencedFutureManager = new SequencedFutureManager();
     controllerStub = new MediaControllerStub(this);
+    pendingMaskingSequencedFutureNumbers = new ArraySet<>();
     this.token = token;
     this.connectionHints = connectionHints;
     deathRecipient =
@@ -303,7 +307,7 @@ private ListenableFuture dispatchRemoteSessionTaskWithPlayerComma
     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 @@ private ListenableFuture dispatchRemoteSessionTaskWithSessionComm
         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 @@ void notifyPeriodicSessionPositionInfoChanged(SessionPositionInfo sessionPositio
   }
 
   <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 @@ void onPlayerInfoChanged(
     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 @@ public void onRenderedFirstFrame() {
   }
 
   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 void moveMediaItems_moveNonCurrentItem_fromBeforeCurrentItemToAfter() thr
         /* 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 void setTimeline(String sessionId, Bundle timelineBundle) throws RemoteEx
             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 void createAndSetFakeTimeline(String sessionId, int windowCount) throws R
               mediaItems.add(
                   MediaTestUtils.createMediaItem(TestUtils.getMediaIdInFakeTimeline(windowIndex)));
             }
+            player.mediaItems.clear();
+            player.mediaItems.addAll(mediaItems);
             player.timeline = new PlaylistTimeline(mediaItems);
           });
     }