Fix spurious sessions created for events after the playlist is cleared

Some events may arrive after the playlist is cleared (e.g. load
cancellation). In this case, the DefaultPlaybackSessionManager may
create a new session for the already removed item.

We already have checks in place that ignore events with old
windowSequenceNumbers, but these checks only work if the current
session is set (i.e. the playlist is non-empty). The fix is to add
the same check for empty playlists by keeping note of the last
removed window sequence number.

PiperOrigin-RevId: 541870812
(cherry picked from commit 656e0723e51e1bc38d558e77fa2cb0cc17e37054)

library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java

@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.analytics;
 
+import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
 import static java.lang.Math.max;
 
 import android.util.Base64;
@@ -25,7 +26,6 @@ import com.google.android.exoplayer2.Player.DiscontinuityReason;
 import com.google.android.exoplayer2.Timeline;
 import com.google.android.exoplayer2.analytics.AnalyticsListener.EventTime;
 import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId;
-import com.google.android.exoplayer2.util.Assertions;
 import com.google.android.exoplayer2.util.Util;
 import com.google.common.base.Supplier;
 import java.util.HashMap;
@@ -63,6 +63,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
   private @MonotonicNonNull Listener listener;
   private Timeline currentTimeline;
   @Nullable private String currentSessionId;
+  private long lastRemovedCurrentWindowSequenceNumber;
 
   /**
    * Creates session manager with a {@link #DEFAULT_SESSION_ID_GENERATOR} to generate session ids.
@@ -83,6 +84,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
     period = new Timeline.Period();
     sessions = new HashMap<>();
     currentTimeline = Timeline.EMPTY;
+    lastRemovedCurrentWindowSequenceNumber = -1;
   }
 
   @Override
@@ -109,22 +111,22 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
 
   @Override
   public synchronized void updateSessions(EventTime eventTime) {
-    Assertions.checkNotNull(listener);
+    checkNotNull(listener);
     if (eventTime.timeline.isEmpty()) {
       // Don't try to create new sessions for empty timelines.
       return;
     }
+    if (eventTime.mediaPeriodId != null) {
+      if (eventTime.mediaPeriodId.windowSequenceNumber < getMinWindowSequenceNumber()) {
+        // Ignore event because it is part of a past window that has already been finished.
+        return;
+      }
       @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
-    if (eventTime.mediaPeriodId != null && currentSession != null) {
-      // If we receive an event associated with a media period, then it needs to be either part of
-      // the current window if it's the first created media period, or a window that will be played
-      // in the future. Otherwise, we know that it belongs to a session that was already finished
-      // and we can ignore the event.
-      boolean isAlreadyFinished =
-          currentSession.windowSequenceNumber == C.INDEX_UNSET
-              ? currentSession.windowIndex != eventTime.windowIndex
-              : eventTime.mediaPeriodId.windowSequenceNumber < currentSession.windowSequenceNumber;
-      if (isAlreadyFinished) {
+      if (currentSession != null
+          && currentSession.windowSequenceNumber == C.INDEX_UNSET
+          && currentSession.windowIndex != eventTime.windowIndex) {
+        // Ignore events for anything other than the current window before the first media period
+        // has been created.
         return;
       }
     }
@@ -177,7 +179,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
 
   @Override
   public synchronized void updateSessionsWithTimelineChange(EventTime eventTime) {
-    Assertions.checkNotNull(listener);
+    checkNotNull(listener);
     Timeline previousTimeline = currentTimeline;
     currentTimeline = eventTime.timeline;
     Iterator<SessionDescriptor> iterator = sessions.values().iterator();
@@ -188,7 +190,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
         iterator.remove();
         if (session.isCreated) {
           if (session.sessionId.equals(currentSessionId)) {
-            currentSessionId = null;
+            clearCurrentSession(session);
           }
           listener.onSessionFinished(
               eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false);
@@ -201,7 +203,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
   @Override
   public synchronized void updateSessionsWithDiscontinuity(
       EventTime eventTime, @DiscontinuityReason int reason) {
-    Assertions.checkNotNull(listener);
+    checkNotNull(listener);
     boolean hasAutomaticTransition = reason == Player.DISCONTINUITY_REASON_AUTO_TRANSITION;
     Iterator<SessionDescriptor> iterator = sessions.values().iterator();
     while (iterator.hasNext()) {
@@ -213,7 +215,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
           boolean isAutomaticTransition =
               hasAutomaticTransition && isRemovingCurrentSession && session.isActive;
           if (isRemovingCurrentSession) {
-            currentSessionId = null;
+            clearCurrentSession(session);
           }
           listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition);
         }
@@ -230,7 +232,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
 
   @Override
   public synchronized void finishAllSessions(EventTime eventTime) {
-    currentSessionId = null;
+    if (currentSessionId != null) {
+      clearCurrentSession(checkNotNull(sessions.get(currentSessionId)));
+    }
     Iterator<SessionDescriptor> iterator = sessions.values().iterator();
     while (iterator.hasNext()) {
       SessionDescriptor session = iterator.next();
@@ -246,7 +250,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
   private void updateCurrentSession(EventTime eventTime) {
     if (eventTime.timeline.isEmpty()) {
       // Clear current session if the Timeline is empty.
-      currentSessionId = null;
+      if (currentSessionId != null) {
+        clearCurrentSession(checkNotNull(sessions.get(currentSessionId)));
+      }
       return;
     }
     @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId);
@@ -275,6 +281,20 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
     }
   }
 
+  private void clearCurrentSession(SessionDescriptor currentSession) {
+    if (currentSession.windowSequenceNumber != C.INDEX_UNSET) {
+      lastRemovedCurrentWindowSequenceNumber = currentSession.windowSequenceNumber;
+    }
+    currentSessionId = null;
+  }
+
+  private long getMinWindowSequenceNumber() {
+    @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
+    return currentSession != null && currentSession.windowSequenceNumber != C.INDEX_UNSET
+        ? currentSession.windowSequenceNumber
+        : lastRemovedCurrentWindowSequenceNumber + 1;
+  }
+
   private SessionDescriptor getOrAddSession(
       int windowIndex, @Nullable MediaPeriodId mediaPeriodId) {
     // There should only be one matching session if mediaPeriodId is non-null. If mediaPeriodId is
@@ -379,7 +399,8 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
         int eventWindowIndex, @Nullable MediaPeriodId eventMediaPeriodId) {
       if (windowSequenceNumber == C.INDEX_UNSET
           && eventWindowIndex == windowIndex
-          && eventMediaPeriodId != null) {
+          && eventMediaPeriodId != null
+          && eventMediaPeriodId.windowSequenceNumber >= getMinWindowSequenceNumber()) {
         // Set window sequence number for this session as soon as we have one.
         windowSequenceNumber = eventMediaPeriodId.windowSequenceNumber;
       }

library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java

@@ -483,6 +483,110 @@ public final class DefaultPlaybackSessionManagerTest {
   }
 
   @Test
+  public void
+      updateSession_withNewMediaPeriodIdOfSameWindowAfterTimelineUpdateToEmpty_createsNewSession() {
+    Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
+    EventTime eventTimeBeforeClear =
+        createEventTime(
+            timeline,
+            /* windowIndex= */ 0,
+            new MediaPeriodId(
+                timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0));
+    sessionManager.updateSessions(eventTimeBeforeClear);
+    EventTime eventTimeWithEmptyTimeline =
+        createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
+    sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline);
+    EventTime newEventTime =
+        createEventTime(
+            timeline,
+            /* windowIndex= */ 0,
+            new MediaPeriodId(
+                timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 1));
+
+    sessionManager.updateSessions(newEventTime);
+
+    ArgumentCaptor<String> sessionId1 = ArgumentCaptor.forClass(String.class);
+    ArgumentCaptor<String> sessionId2 = ArgumentCaptor.forClass(String.class);
+    verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId1.capture());
+    verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId1.getValue());
+    verify(mockListener)
+        .onSessionFinished(
+            eventTimeWithEmptyTimeline,
+            sessionId1.getValue(),
+            /* automaticTransitionToNextPlayback= */ false);
+    verify(mockListener).onSessionCreated(eq(newEventTime), sessionId2.capture());
+    verify(mockListener).onSessionActive(newEventTime, sessionId2.getValue());
+    verifyNoMoreInteractions(mockListener);
+  }
+
+  @Test
+  public void
+      updateSession_withOldMediaPeriodIdOfSameWindowAfterTimelineUpdateToEmpty_doesNotCreateNewSession() {
+    Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
+    EventTime eventTimeBeforeClear =
+        createEventTime(
+            timeline,
+            /* windowIndex= */ 0,
+            new MediaPeriodId(
+                timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0));
+    sessionManager.updateSessions(eventTimeBeforeClear);
+    EventTime eventTimeWithEmptyTimeline =
+        createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
+    sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline);
+    EventTime newEventTime =
+        createEventTime(
+            timeline,
+            /* windowIndex= */ 0,
+            new MediaPeriodId(
+                timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0));
+
+    sessionManager.updateSessions(newEventTime);
+
+    ArgumentCaptor<String> sessionId = ArgumentCaptor.forClass(String.class);
+    verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId.capture());
+    verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId.getValue());
+    verify(mockListener)
+        .onSessionFinished(
+            eventTimeWithEmptyTimeline,
+            sessionId.getValue(),
+            /* automaticTransitionToNextPlayback= */ false);
+    verifyNoMoreInteractions(mockListener);
+  }
+
+  @Test
+  public void
+      updateSession_ofSameWindowWithoutMediaPeriodIdAfterTimelineUpdateToEmpty_createsNewSession() {
+    Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
+    EventTime eventTimeBeforeClear =
+        createEventTime(
+            timeline,
+            /* windowIndex= */ 0,
+            new MediaPeriodId(
+                timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0));
+    sessionManager.updateSessions(eventTimeBeforeClear);
+    EventTime eventTimeWithEmptyTimeline =
+        createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
+    sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline);
+    EventTime newEventTime =
+        createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
+
+    sessionManager.updateSessions(newEventTime);
+
+    ArgumentCaptor<String> sessionId1 = ArgumentCaptor.forClass(String.class);
+    ArgumentCaptor<String> sessionId2 = ArgumentCaptor.forClass(String.class);
+    verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId1.capture());
+    verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId1.getValue());
+    verify(mockListener)
+        .onSessionFinished(
+            eventTimeWithEmptyTimeline,
+            sessionId1.getValue(),
+            /* automaticTransitionToNextPlayback= */ false);
+    verify(mockListener).onSessionCreated(eq(newEventTime), sessionId2.capture());
+    verify(mockListener).onSessionActive(newEventTime, sessionId2.getValue());
+    verifyNoMoreInteractions(mockListener);
+  }
+
+  @Test
   public void getSessionForMediaPeriodId_returnsValue_butDoesNotCreateSession() {
     Timeline timeline = new FakeTimeline();
     MediaPeriodId mediaPeriodId =