From 6070d9110a9cec61f6073c7669f2f8185a635767 Mon Sep 17 00:00:00 2001
From: tonihei <tonihei@google.com>
Date: Thu, 13 Oct 2022 12:29:51 +0000
Subject: [PATCH] Ensure sessions without MediaPeriodId are ended after seek to
 new item

We already have logic to end all session except the current one if the
current one doesn't have a MediaPeriodId yet. This is assuming that this
only happens after a seek on the app side where the player doesn't have
detailled knowledge about the MediaPeriodIds yet.

Currently this logic isn't triggered if the window we are coming from
doesn't have its MediaPeriodId either as we run into another check that
keeps sessions around until we have a valid windowSequenceNumber.

Swapping both conditions fixes this case without breaking any of the
other known transition scenarios.

Issue: androidx/media#180
PiperOrigin-RevId: 480866465
---
 .../DefaultPlaybackSessionManager.java        |  8 +++---
 .../DefaultPlaybackSessionManagerTest.java    | 27 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
index b3393b67fd5..0e3dd578d6e 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
@@ -380,15 +380,15 @@ public void maybeSetWindowSequenceNumber(
     }
 
     public boolean isFinishedAtEventTime(EventTime eventTime) {
-      if (windowSequenceNumber == C.INDEX_UNSET) {
-        // Sessions with unspecified window sequence number are kept until we know more.
-        return false;
-      }
       if (eventTime.mediaPeriodId == null) {
         // For event times without media period id (e.g. after seek to new window), we only keep
         // sessions of this window.
         return windowIndex != eventTime.windowIndex;
       }
+      if (windowSequenceNumber == C.INDEX_UNSET) {
+        // Sessions with unspecified window sequence number are kept until we know more.
+        return false;
+      }
       if (eventTime.mediaPeriodId.windowSequenceNumber > windowSequenceNumber) {
         // All past window sequence numbers are finished.
         return true;
diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
index 44aba86db3c..c9c3178255a 100644
--- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
+++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
@@ -925,7 +925,8 @@ public void positionDiscontinuity_toNewWindow_withPeriodTransitionReason_finishe
   }
 
   @Test
-  public void positionDiscontinuity_toNewWindow_withSeekTransitionReason_finishesSession() {
+  public void
+      positionDiscontinuity_toNewWindow_withMediaPeriodIds_withSeekTransitionReason_finishesSession() {
     Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
     EventTime eventTime1 =
         createEventTime(
@@ -957,6 +958,30 @@ public void positionDiscontinuity_toNewWindow_withSeekTransitionReason_finishesS
     verifyNoMoreInteractions(mockListener);
   }
 
+  @Test
+  public void
+      positionDiscontinuity_toNewWindow_withoutMediaPeriodIds_withSeekTransitionReason_finishesSession() {
+    Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
+    EventTime eventTime1 =
+        createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
+    EventTime eventTime2 =
+        createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null);
+    sessionManager.updateSessionsWithTimelineChange(eventTime1);
+
+    sessionManager.updateSessionsWithDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK);
+
+    ArgumentCaptor<String> sessionId1 = ArgumentCaptor.forClass(String.class);
+    ArgumentCaptor<String> sessionId2 = ArgumentCaptor.forClass(String.class);
+    verify(mockListener).onSessionCreated(eq(eventTime1), sessionId1.capture());
+    verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue());
+    verify(mockListener).onSessionCreated(eq(eventTime2), sessionId2.capture());
+    verify(mockListener)
+        .onSessionFinished(
+            eventTime2, sessionId1.getValue(), /* automaticTransitionToNextPlayback= */ false);
+    verify(mockListener).onSessionActive(eventTime2, sessionId2.getValue());
+    verifyNoMoreInteractions(mockListener);
+  }
+
   @Test
   public void positionDiscontinuity_toSameWindow_withoutMediaPeriodId_doesNotFinishSession() {
     Timeline timeline = new FakeTimeline();