Release AnalyticsListeners after calling SimpleExoPlayer.release

Currently we don't remove the AnalyticsListeners registed to
SimpleExoPlayer after calling release. We didn't do this mainly
because there are messages triggered as part of the release that
still cause interesting events (e.g. decoderDisabled with the
final counters, final dropped counts etc).

However, we should fully release/remove the listeners once these
pending events are delivered to:
 1. Not leak listener implementations (e.g. if the listener is an
    Activity)
 2. Ensure we don't send future events that may cause listeners
    to unintentionally access released or nulled variables. This
    could happen for example if someone calls a player method
    after the player was released.
In addition, we can add a onPlayerReleased callback to
AnalyticsListener to allow implementations to clean themselves up
once all pending events are delivered.

PiperOrigin-RevId: 347434344

library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java

@@ -1785,6 +1785,7 @@ public void release() {
     wifiLockManager.setStayAwake(false);
     audioFocusManager.release();
     player.release();
+    analyticsCollector.release();
     removeSurfaceCallbacks();
     if (surface != null) {
       if (ownsSurface) {

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

@@ -144,6 +144,19 @@ public void setPlayer(Player player, Looper looper) {
             });
   }
 
+  /**
+   * Releases the collector. Must be called after the player for which data is collected has been
+   * released.
+   */
+  public void release() {
+    EventTime eventTime = generateCurrentPlayerMediaPeriodEventTime();
+    eventTimes.put(AnalyticsListener.EVENT_PLAYER_RELEASED, eventTime);
+    // Release listeners lazily so that all events that got triggered as part of player.release()
+    // are still delivered to all listeners.
+    listeners.lazyRelease(
+        AnalyticsListener.EVENT_PLAYER_RELEASED, listener -> listener.onPlayerReleased(eventTime));
+  }
+
   /**
    * Updates the playback queue information used for event association.
    *

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

@@ -196,7 +196,8 @@ public int get(int index) {
     EVENT_DRM_SESSION_MANAGER_ERROR,
     EVENT_DRM_KEYS_RESTORED,
     EVENT_DRM_KEYS_REMOVED,
-    EVENT_DRM_SESSION_RELEASED
+    EVENT_DRM_SESSION_RELEASED,
+    EVENT_PLAYER_RELEASED,
   })
   @interface EventFlags {}
   /** {@link Player#getCurrentTimeline()} changed. */
@@ -309,6 +310,8 @@ public int get(int index) {
   int EVENT_DRM_KEYS_REMOVED = 1034;
   /** A DRM session has been released. */
   int EVENT_DRM_SESSION_RELEASED = 1035;
+  /** The player was released. */
+  int EVENT_PLAYER_RELEASED = 1036;
 
   /** Time information of an event. */
   final class EventTime {
@@ -1013,6 +1016,13 @@ default void onDrmKeysRemoved(EventTime eventTime) {}
    */
   default void onDrmSessionReleased(EventTime eventTime) {}
 
+  /**
+   * Called when the {@link Player} is released.
+   *
+   * @param eventTime The event time.
+   */
+  default void onPlayerReleased(EventTime eventTime) {}
+
   /**
    * Called after one or more events occurred.
    *

library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java

@@ -70,8 +70,9 @@ public interface IterationFinishedEvent<T, E extends MutableFlags> {
   }
 
   private static final int MSG_ITERATION_FINISHED = 0;
+  private static final int MSG_LAZY_RELEASE = 1;
 
-  private final Handler iterationFinishedHandler;
+  private final Handler handler;
   private final Supplier<E> eventFlagsSupplier;
   private final IterationFinishedEvent<T, E> iterationFinishedEvent;
   private final CopyOnWriteArraySet<ListenerHolder<T, E>> listeners;
@@ -113,8 +114,8 @@ private ListenerSet(
     queuedEvents = new ArrayDeque<>();
     // It's safe to use "this" because we don't send a message before exiting the constructor.
     @SuppressWarnings("methodref.receiver.bound.invalid")
-    Handler handler = Util.createHandler(looper, this::handleIterationFinished);
-    iterationFinishedHandler = handler;
+    Handler handler = Util.createHandler(looper, this::handleMessage);
+    this.handler = handler;
   }
 
   /**
@@ -165,8 +166,8 @@ public void remove(T listener) {
   /**
    * Adds an event that is sent to the listeners when {@link #flushEvents} is called.
    *
-   * @param eventFlag An integer indicating the type of the event, or {@link C#INDEX_UNSET} to not
-   *     report this event with a flag.
+   * @param eventFlag An integer indicating the type of the event, or {@link C#INDEX_UNSET} to
+   *     report this event without flag.
    * @param event The event.
    */
   public void queueEvent(int eventFlag, Event<T> event) {
@@ -185,8 +186,8 @@ public void flushEvents() {
     if (queuedEvents.isEmpty()) {
       return;
     }
-    if (!iterationFinishedHandler.hasMessages(MSG_ITERATION_FINISHED)) {
-      iterationFinishedHandler.obtainMessage(MSG_ITERATION_FINISHED).sendToTarget();
+    if (!handler.hasMessages(MSG_ITERATION_FINISHED)) {
+      handler.obtainMessage(MSG_ITERATION_FINISHED).sendToTarget();
     }
     boolean recursiveFlushInProgress = !flushingEvents.isEmpty();
     flushingEvents.addAll(queuedEvents);
@@ -206,7 +207,7 @@ public void flushEvents() {
    * flushes} the event queue to notify all listeners.
    *
    * @param eventFlag An integer flag indicating the type of the event, or {@link C#INDEX_UNSET} to
-   *     not report this event with a flag.
+   *     report this event without flag.
    * @param event The event.
    */
   public void sendEvent(int eventFlag, Event<T> event) {
@@ -215,7 +216,7 @@ public void sendEvent(int eventFlag, Event<T> event) {
   }
 
   /**
-   * Releases the set of listeners.
+   * Releases the set of listeners immediately.
    *
    * <p>This will ensure no events are sent to any listener after this method has been called.
    */
@@ -227,15 +228,37 @@ public void release() {
     released = true;
   }
 
-  private boolean handleIterationFinished(Message message) {
-    for (ListenerHolder<T, E> holder : listeners) {
-      holder.iterationFinished(eventFlagsSupplier, iterationFinishedEvent);
-      if (iterationFinishedHandler.hasMessages(MSG_ITERATION_FINISHED)) {
-        // The invocation above triggered new events (and thus scheduled a new message). We need to
-        // stop here because this new message will take care of informing every listener about the
-        // new update (including the ones already called here).
-        break;
+  /**
+   * Releases the set of listeners after all already scheduled {@link Looper} messages were able to
+   * trigger final events.
+   *
+   * <p>After the specified released callback event, no other events are sent to a listener.
+   *
+   * @param releaseEventFlag An integer flag indicating the type of the release event, or {@link
+   *     C#INDEX_UNSET} to report this event without a flag.
+   * @param releaseEvent The release event.
+   */
+  public void lazyRelease(int releaseEventFlag, Event<T> releaseEvent) {
+    handler.obtainMessage(MSG_LAZY_RELEASE, releaseEventFlag, 0, releaseEvent).sendToTarget();
+  }
+
+  private boolean handleMessage(Message message) {
+    if (message.what == MSG_ITERATION_FINISHED) {
+      for (ListenerHolder<T, E> holder : listeners) {
+        holder.iterationFinished(eventFlagsSupplier, iterationFinishedEvent);
+        if (handler.hasMessages(MSG_ITERATION_FINISHED)) {
+          // The invocation above triggered new events (and thus scheduled a new message). We need
+          // to stop here because this new message will take care of informing every listener about
+          // the new update (including the ones already called here).
+          break;
+        }
       }
+    } else if (message.what == MSG_LAZY_RELEASE) {
+      int releaseEventFlag = message.arg1;
+      @SuppressWarnings("unchecked")
+      Event<T> releaseEvent = (Event<T>) message.obj;
+      sendEvent(releaseEventFlag, releaseEvent);
+      release();
     }
     return true;
   }

library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java

@@ -15,14 +15,25 @@
  */
 package com.google.android.exoplayer2;
 
+import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlaybackState;
 import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.analytics.AnalyticsListener;
+import com.google.android.exoplayer2.testutil.AutoAdvancingFakeClock;
+import com.google.android.exoplayer2.testutil.ExoPlayerTestRunner;
+import com.google.android.exoplayer2.testutil.FakeMediaSource;
+import com.google.android.exoplayer2.testutil.FakeTimeline;
+import com.google.android.exoplayer2.testutil.FakeVideoRenderer;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowLooper;
 
 /** Unit test for {@link SimpleExoPlayer}. */
 @RunWith(AndroidJUnit4.class)
@@ -43,4 +54,29 @@ public void builder_inBackgroundThread_doesNotThrow() throws Exception {
 
     assertThat(builderThrow.get()).isNull();
   }
+
+  @Test
+  public void release_triggersAllPendingEventsInAnalyticsListeners() throws Exception {
+    SimpleExoPlayer player =
+        new SimpleExoPlayer.Builder(
+                ApplicationProvider.getApplicationContext(),
+                (handler, videoListener, audioListener, textOutput, metadataOutput) ->
+                    new Renderer[] {new FakeVideoRenderer(handler, videoListener)})
+            .setClock(new AutoAdvancingFakeClock())
+            .build();
+    AnalyticsListener listener = mock(AnalyticsListener.class);
+    player.addAnalyticsListener(listener);
+    // Do something that requires clean-up callbacks like decoder disabling.
+    player.setMediaSource(
+        new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT));
+    player.prepare();
+    player.play();
+    runUntilPlaybackState(player, Player.STATE_READY);
+
+    player.release();
+    ShadowLooper.runMainLooperToNextTask();
+
+    verify(listener).onVideoDisabled(any(), any());
+    verify(listener).onPlayerReleased(any());
+  }
 }

library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java

@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
+import android.os.Handler;
 import android.os.Looper;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.android.exoplayer2.C;
@@ -366,6 +367,34 @@ public void release_preventsRegisteringNewListeners() {
     verify(listener, never()).callback1();
   }
 
+  @Test
+  public void lazyRelease_stopsForwardingEventsFromNewHandlerMessagesAndCallsReleaseCallback() {
+    ListenerSet<TestListener, Flags> listenerSet =
+        new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished);
+    TestListener listener = mock(TestListener.class);
+    listenerSet.add(listener);
+
+    // In-line event before release.
+    listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
+    // Message triggering event sent before release.
+    new Handler().post(() -> listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1));
+    // Lazy release with release callback.
+    listenerSet.lazyRelease(EVENT_ID_3, TestListener::callback3);
+    // In-line event after release.
+    listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
+    // Message triggering event sent after release.
+    new Handler().post(() -> listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2));
+    ShadowLooper.runMainLooperToNextTask();
+
+    // Verify all events are delivered except for the one triggered by the message sent after the
+    // lazy release.
+    verify(listener, times(3)).callback1();
+    verify(listener).callback3();
+    verify(listener).iterationFinished(Flags.create(EVENT_ID_1));
+    verify(listener).iterationFinished(Flags.create(EVENT_ID_1, EVENT_ID_3));
+    verifyNoMoreInteractions(listener);
+  }
+
   private interface TestListener {
     default void callback1() {}