Commit bf5b52e2 by tonihei Committed by Ian Baker

Make sure finishAllSessions() can be called without removing listener

Currently, this method is only supposed to be called before removing
the listener from the player or when releasing the player.

If called at other times, it will throw an exception later when
a playback session is ended automatically.

issue:#7193
PiperOrigin-RevId: 308254993
parent e9511a56
......@@ -202,6 +202,20 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
}
}
@Override
public void finishAllSessions(EventTime eventTime) {
currentSessionId = null;
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
while (iterator.hasNext()) {
SessionDescriptor session = iterator.next();
iterator.remove();
if (session.isCreated && listener != null) {
listener.onSessionFinished(
eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false);
}
}
}
private SessionDescriptor getOrAddSession(
int windowIndex, @Nullable MediaPeriodId mediaPeriodId) {
// There should only be one matching session if mediaPeriodId is non-null. If mediaPeriodId is
......
......@@ -117,4 +117,12 @@ public interface PlaybackSessionManager {
* @param reason The {@link DiscontinuityReason}.
*/
void handlePositionDiscontinuity(EventTime eventTime, @DiscontinuityReason int reason);
/**
* Finishes all existing sessions and calls their respective {@link
* Listener#onSessionFinished(EventTime, String, boolean)} callback.
*
* @param eventTime The event time at which sessions are finished.
*/
void finishAllSessions(EventTime eventTime);
}
......@@ -150,7 +150,6 @@ public final class PlaybackStatsListener
// TODO: Add AnalyticsListener.onAttachedToPlayer and onDetachedFromPlayer to auto-release with
// an actual EventTime. Should also simplify other cases where the listener needs to be released
// separately from the player.
HashMap<String, PlaybackStatsTracker> trackerCopy = new HashMap<>(playbackStatsTrackers);
EventTime dummyEventTime =
new EventTime(
SystemClock.elapsedRealtime(),
......@@ -160,9 +159,7 @@ public final class PlaybackStatsListener
/* eventPlaybackPositionMs= */ 0,
/* currentPlaybackPositionMs= */ 0,
/* totalBufferedDurationMs= */ 0);
for (String session : trackerCopy.keySet()) {
onSessionFinished(dummyEventTime, session, /* automaticTransition= */ false);
}
sessionManager.finishAllSessions(dummyEventTime);
}
// PlaybackSessionManager.Listener implementation.
......
......@@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
......@@ -1058,6 +1059,31 @@ public final class DefaultPlaybackSessionManagerTest {
verify(mockListener, never()).onSessionActive(any(), eq(adSessionId2));
}
@Test
public void finishAllSessions_callsOnSessionFinishedForAllCreatedSessions() {
Timeline timeline = new FakeTimeline(/* windowCount= */ 4);
EventTime eventTimeWindow0 =
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTimeWindow2 =
createEventTime(timeline, /* windowIndex= */ 2, /* mediaPeriodId= */ null);
// Actually create sessions for window 0 and 2.
sessionManager.updateSessions(eventTimeWindow0);
sessionManager.updateSessions(eventTimeWindow2);
// Query information about session for window 1, but don't create it.
sessionManager.getSessionForMediaPeriodId(
timeline,
new MediaPeriodId(
timeline.getPeriod(/* periodIndex= */ 1, new Timeline.Period(), /* setIds= */ true).uid,
/* windowSequenceNumber= */ 123));
verify(mockListener, times(2)).onSessionCreated(any(), anyString());
EventTime finishEventTime =
createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
sessionManager.finishAllSessions(finishEventTime);
verify(mockListener, times(2)).onSessionFinished(eq(finishEventTime), anyString(), eq(false));
}
private static EventTime createEventTime(
Timeline timeline, int windowIndex, @Nullable MediaPeriodId mediaPeriodId) {
return new EventTime(
......
......@@ -16,7 +16,14 @@
package com.google.android.exoplayer2.analytics;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.os.SystemClock;
import androidx.annotation.Nullable;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.android.exoplayer2.Player;
......@@ -42,7 +49,7 @@ public final class PlaybackStatsListenerTest {
private static final Timeline TEST_TIMELINE = new FakeTimeline(/* windowCount= */ 1);
private static final AnalyticsListener.EventTime TEST_EVENT_TIME =
new AnalyticsListener.EventTime(
/* realtimeMs= */ 700,
/* realtimeMs= */ 500,
TEST_TIMELINE,
/* windowIndex= */ 0,
new MediaSource.MediaPeriodId(
......@@ -119,4 +126,68 @@ public final class PlaybackStatsListenerTest {
assertThat(playbackStats).isNotNull();
assertThat(playbackStats.endedCount).isEqualTo(1);
}
@Test
public void finishedSession_callsCallback() {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback);
// Create session with an event and finish it by simulating removal from playlist.
playbackStatsListener.onPlaybackStateChanged(TEST_EVENT_TIME, Player.STATE_BUFFERING);
verify(callback, never()).onPlaybackStatsReady(any(), any());
playbackStatsListener.onTimelineChanged(
EMPTY_TIMELINE_EVENT_TIME, Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
verify(callback).onPlaybackStatsReady(eq(TEST_EVENT_TIME), any());
}
@Test
public void finishAllSessions_callsAllPendingCallbacks() {
AnalyticsListener.EventTime eventTimeWindow0 =
new AnalyticsListener.EventTime(
/* realtimeMs= */ 0,
Timeline.EMPTY,
/* windowIndex= */ 0,
/* mediaPeriodId= */ null,
/* eventPlaybackPositionMs= */ 0,
/* currentPlaybackPositionMs= */ 0,
/* totalBufferedDurationMs= */ 0);
AnalyticsListener.EventTime eventTimeWindow1 =
new AnalyticsListener.EventTime(
/* realtimeMs= */ 0,
Timeline.EMPTY,
/* windowIndex= */ 1,
/* mediaPeriodId= */ null,
/* eventPlaybackPositionMs= */ 0,
/* currentPlaybackPositionMs= */ 0,
/* totalBufferedDurationMs= */ 0);
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback);
playbackStatsListener.onPlaybackStateChanged(eventTimeWindow0, Player.STATE_BUFFERING);
playbackStatsListener.onPlaybackStateChanged(eventTimeWindow1, Player.STATE_BUFFERING);
playbackStatsListener.finishAllSessions();
verify(callback, times(2)).onPlaybackStatsReady(any(), any());
verify(callback).onPlaybackStatsReady(eq(eventTimeWindow0), any());
verify(callback).onPlaybackStatsReady(eq(eventTimeWindow1), any());
}
@Test
public void finishAllSessions_doesNotCallCallbackAgainWhenSessionWouldBeAutomaticallyFinished() {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback);
playbackStatsListener.onPlaybackStateChanged(TEST_EVENT_TIME, Player.STATE_BUFFERING);
SystemClock.setCurrentTimeMillis(TEST_EVENT_TIME.realtimeMs + 100);
playbackStatsListener.finishAllSessions();
// Simulate removing the playback item to ensure the session would finish if it hadn't already.
playbackStatsListener.onTimelineChanged(
EMPTY_TIMELINE_EVENT_TIME, Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
verify(callback).onPlaybackStatsReady(any(), any());
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment