Add consistency check to sending and receiving position updates

The periodic updates are only meant to happen while we are in the
same period or ad. This was already guaranteed except for two cases:
1. The Player in a session has updated its state without yet calling
   its listeners
2. The session scheduled a PlayerInfo update that hasn't been sent yet

... and in both cases, the following happened:
 - The change updated the mediaItemIndex to an index that didn't exist
   in a previous Timeline known to the Controller
 - One of the period position updates happened to be sent at exactly
   this time

This problem can be avoided by only scheduling the update if we are
still in the same period/ad and haven't scheduled a normal PlayerInfo
update already.

Since new MediaControllers may still connect to old sessons with this
bug, we need an equivalent change on the controller side to ignore such
buggy updates.

PiperOrigin-RevId: 532089328
(cherry picked from commit 96dd0ae583)

RELEASENOTES.md

@@ -25,6 +25,9 @@
         ([#355](https://github.com/androidx/media/issues/355)).
     *   Fix memory leak of `MediaSessionService` or `MediaLibraryService`
         ([#346](https://github.com/androidx/media/issues/346)).
+    *   Fix bug where a combined `Timeline` and position update in a
+        `MediaSession` may cause a `MediaController` to throw an
+        `IllegalStateException`.
 
 ### 1.0.1 (2023-04-18)
 

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

@@ -2500,6 +2500,13 @@ import org.checkerframework.checker.nullness.qual.NonNull;
   private void updateSessionPositionInfoIfNeeded(SessionPositionInfo sessionPositionInfo) {
     if (pendingMaskingSequencedFutureNumbers.isEmpty()
         && playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) {
+      if (!MediaUtils.areSessionPositionInfosInSamePeriodOrAd(
+          sessionPositionInfo, playerInfo.sessionPositionInfo)) {
+        // MediaSessionImpl before version 1.0.2 has a bug that may send position info updates for
+        // new periods too early. Ignore these updates to avoid an inconsistent state (see
+        // [internal b/277301159]).
+        return;
+      }
       playerInfo = playerInfo.copyWithSessionPositionInfo(sessionPositionInfo);
     }
   }

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

@@ -733,7 +733,16 @@ import org.checkerframework.checker.initialization.qual.Initialized;
       }
     }
     SessionPositionInfo sessionPositionInfo = playerWrapper.createSessionPositionInfoForBundling();
-    dispatchOnPeriodicSessionPositionInfoChanged(sessionPositionInfo);
+    if (!onPlayerInfoChangedHandler.hasPendingPlayerInfoChangedUpdate()
+        && MediaUtils.areSessionPositionInfosInSamePeriodOrAd(
+            sessionPositionInfo, playerInfo.sessionPositionInfo)) {
+      // Send a periodic position info only if a PlayerInfo update is not already already pending
+      // and the player state is still corresponding to the currently known PlayerInfo. Both
+      // conditions will soon trigger a new PlayerInfo update with the latest position info anyway
+      // and we also don't want to send a new position info early if the corresponding Timeline
+      // update hasn't been sent yet (see [internal b/277301159]).
+      dispatchOnPeriodicSessionPositionInfoChanged(sessionPositionInfo);
+    }
     schedulePeriodicSessionPositionInfoChanges();
   }
 
@@ -1288,11 +1297,15 @@ import org.checkerframework.checker.initialization.qual.Initialized;
       }
     }
 
+    public boolean hasPendingPlayerInfoChangedUpdate() {
+      return hasMessages(MSG_PLAYER_INFO_CHANGED);
+    }
+
     public void sendPlayerInfoChangedMessage(boolean excludeTimeline, boolean excludeTracks) {
       this.excludeTimeline = this.excludeTimeline && excludeTimeline;
       this.excludeTracks = this.excludeTracks && excludeTracks;
-      if (!onPlayerInfoChangedHandler.hasMessages(MSG_PLAYER_INFO_CHANGED)) {
-        onPlayerInfoChangedHandler.sendEmptyMessage(MSG_PLAYER_INFO_CHANGED);
+      if (!hasMessages(MSG_PLAYER_INFO_CHANGED)) {
+        sendEmptyMessage(MSG_PLAYER_INFO_CHANGED);
       }
     }
   }

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

@@ -1460,6 +1460,19 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
     }
   }
 
+  /**
+   * Returns whether the two provided {@link SessionPositionInfo} describe a position in the same
+   * period or ad.
+   */
+  public static boolean areSessionPositionInfosInSamePeriodOrAd(
+      SessionPositionInfo info1, SessionPositionInfo info2) {
+    // TODO: b/259220235 - Use UIDs instead of mediaItemIndex and periodIndex
+    return info1.positionInfo.mediaItemIndex == info2.positionInfo.mediaItemIndex
+        && info1.positionInfo.periodIndex == info2.positionInfo.periodIndex
+        && info1.positionInfo.adGroupIndex == info2.positionInfo.adGroupIndex
+        && info1.positionInfo.adIndexInAdGroup == info2.positionInfo.adIndexInAdGroup;
+  }
+
   private static byte[] convertToByteArray(Bitmap bitmap) throws IOException {
     try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) {
       bitmap.compress(Bitmap.CompressFormat.PNG, /* ignored */ 0, stream);

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

@@ -34,6 +34,7 @@ import android.app.PendingIntent;
 import android.content.Context;
 import android.os.Bundle;
 import android.os.RemoteException;
+import androidx.annotation.Nullable;
 import androidx.media3.common.AudioAttributes;
 import androidx.media3.common.C;
 import androidx.media3.common.Format;
@@ -1064,6 +1065,51 @@ public class MediaControllerTest {
     assertThat(bufferedPositionAfterDelay.get()).isNotEqualTo(testBufferedPosition);
   }
 
+  @Test
+  public void
+      getCurrentMediaItemIndex_withPeriodicUpdateOverlappingTimelineChanges_updatesIndexCorrectly()
+          throws Exception {
+    Bundle playerConfig =
+        new RemoteMediaSession.MockPlayerConfigBuilder()
+            .setPlayWhenReady(true)
+            .setPlaybackState(Player.STATE_READY)
+            .build();
+    remoteSession.setPlayer(playerConfig);
+    MediaController controller = controllerTestRule.createController(remoteSession.getToken());
+    ArrayList<Integer> transitionMediaItemIndices = new ArrayList<>();
+    controller.addListener(
+        new Player.Listener() {
+          @Override
+          public void onMediaItemTransition(@Nullable MediaItem mediaItem, int reason) {
+            transitionMediaItemIndices.add(controller.getCurrentMediaItemIndex());
+          }
+        });
+
+    // Intentionally trigger update often to ensure there is a likely overlap with Timeline updates.
+    remoteSession.setSessionPositionUpdateDelayMs(1L);
+    // Trigger many timeline and position updates that are incompatible with any previous updates.
+    for (int i = 1; i <= 100; i++) {
+      remoteSession.getMockPlayer().createAndSetFakeTimeline(/* windowCount= */ i);
+      remoteSession.getMockPlayer().setCurrentMediaItemIndex(i - 1);
+      remoteSession
+          .getMockPlayer()
+          .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
+      remoteSession
+          .getMockPlayer()
+          .notifyMediaItemTransition(
+              /* index= */ i - 1, Player.MEDIA_ITEM_TRANSITION_REASON_PLAYLIST_CHANGED);
+    }
+    PollingCheck.waitFor(TIMEOUT_MS, () -> transitionMediaItemIndices.size() == 100);
+
+    ImmutableList.Builder<Integer> expectedMediaItemIndices = ImmutableList.builder();
+    for (int i = 0; i < 100; i++) {
+      expectedMediaItemIndices.add(i);
+    }
+    assertThat(transitionMediaItemIndices)
+        .containsExactlyElementsIn(expectedMediaItemIndices.build())
+        .inOrder();
+  }
+
   @Test
   public void getContentBufferedPosition_byDefault_returnsZero() throws Exception {
     MediaController controller = controllerTestRule.createController(remoteSession.getToken());