From f18d81f8a88833d1410344bdb70c66ff3f16cb3c Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 9 Dec 2020 16:07:44 +0000 Subject: [PATCH] Ensure onEvents is called when listener is removed. When a listener is removed or released we may not have called onEvents for events that happened before this point. To ensure listeners don't miss events we need to trigger a final onEvents with all events we have happened so far (if any). PiperOrigin-RevId: 346553030 --- .../android/exoplayer2/util/ListenerSet.java | 16 +++++++++++----- .../android/exoplayer2/util/ListenerSetTest.java | 4 +++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java b/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java index f98ff33f6d..24bf624338 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java @@ -156,7 +156,7 @@ public final class ListenerSet { public void remove(T listener) { for (ListenerHolder listenerHolder : listeners) { if (listenerHolder.listener.equals(listener)) { - listenerHolder.release(); + listenerHolder.release(iterationFinishedEvent); listeners.remove(listenerHolder); } } @@ -221,7 +221,7 @@ public final class ListenerSet { */ public void release() { for (ListenerHolder listenerHolder : listeners) { - listenerHolder.release(); + listenerHolder.release(iterationFinishedEvent); } listeners.clear(); released = true; @@ -245,6 +245,7 @@ public final class ListenerSet { @Nonnull public final T listener; private E eventsFlags; + private boolean needsIterationFinishedEvent; private boolean released; public ListenerHolder(@Nonnull T listener, Supplier eventFlagSupplier) { @@ -252,26 +253,31 @@ public final class ListenerSet { this.eventsFlags = eventFlagSupplier.get(); } - public void release() { + public void release(IterationFinishedEvent event) { released = true; + if (needsIterationFinishedEvent) { + event.invoke(listener, eventsFlags); + } } public void invoke(int eventFlag, Event event) { if (!released) { - event.invoke(listener); if (eventFlag != C.INDEX_UNSET) { eventsFlags.add(eventFlag); } + needsIterationFinishedEvent = true; + event.invoke(listener); } } public void iterationFinished( Supplier eventFlagSupplier, IterationFinishedEvent event) { - if (!released) { + if (!released && needsIterationFinishedEvent) { // Reset flags before invoking the listener to ensure we keep all new flags that are set by // recursive events triggered from this callback. E flagToNotify = eventsFlags; eventsFlags = eventFlagSupplier.get(); + needsIterationFinishedEvent = false; event.invoke(listener, flagToNotify); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java b/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java index 604e0d5a0a..19ffd24899 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java @@ -297,10 +297,11 @@ public class ListenerSetTest { // Listener2 shouldn't even get this event as it's removed before the event can be invoked. listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); listenerSet.remove(listener1); - listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); + listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2); ShadowLooper.runMainLooperToNextTask(); verify(listener1).callback1(); + verify(listener1).iterationFinished(Flags.create(EVENT_ID_1)); verifyNoMoreInteractions(listener1, listener2); } @@ -348,6 +349,7 @@ public class ListenerSetTest { ShadowLooper.runMainLooperToNextTask(); verify(listener1).callback1(); + verify(listener1).iterationFinished(Flags.create(EVENT_ID_1)); verifyNoMoreInteractions(listener1, listener2); }