Skip to content

SDK / exoplayer

Go to a project
Commit 3e5103a3 by tonihei Committed by microkatz
Options

Mark iterationFinished when triggering release event. 

When we currently trigger the iteration finished event during the
release, we don't mark the event as triggered. This means that
someone can trigger another release from within the callback,
which then tries to resend the event.

Issue: google/ExoPlayer#10758

#minor-release

PiperOrigin-RevId: 488645089
parent 01e0db62
Inline Side-by-side
Showing with 48 additions and 11 deletions
library/common/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java
...@@ -268,6 +268,7 @@ public final class ListenerSet<T extends @NonNull Object> { ...@@ -268,6 +268,7 @@ public final class ListenerSet<T extends @NonNull Object> {
public void release(IterationFinishedEvent<T> event) { public void release(IterationFinishedEvent<T> event) {
released = true; released = true;
if (needsIterationFinishedEvent) { if (needsIterationFinishedEvent) {
needsIterationFinishedEvent = false;
event.invoke(listener, flagsBuilder.build()); event.invoke(listener, flagsBuilder.build());
} }
} }
......
library/common/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java
...@@ -48,7 +48,7 @@ public class ListenerSetTest { ...@@ -48,7 +48,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2); listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
verifyNoMoreInteractions(listener); verifyNoMoreInteractions(listener);
} }
...@@ -66,6 +66,7 @@ public class ListenerSetTest { ...@@ -66,6 +66,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2); listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2); InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1(); inOrder.verify(listener1).callback1();
...@@ -74,6 +75,8 @@ public class ListenerSetTest { ...@@ -74,6 +75,8 @@ public class ListenerSetTest {
inOrder.verify(listener2).callback2(); inOrder.verify(listener2).callback2();
inOrder.verify(listener1).callback1(); inOrder.verify(listener1).callback1();
inOrder.verify(listener2).callback1(); inOrder.verify(listener2).callback1();
inOrder.verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2));
inOrder.verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2));
inOrder.verifyNoMoreInteractions(); inOrder.verifyNoMoreInteractions();
} }
...@@ -98,6 +101,7 @@ public class ListenerSetTest { ...@@ -98,6 +101,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2); listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2); InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1(); inOrder.verify(listener1).callback1();
...@@ -106,6 +110,8 @@ public class ListenerSetTest { ...@@ -106,6 +110,8 @@ public class ListenerSetTest {
inOrder.verify(listener2).callback2(); inOrder.verify(listener2).callback2();
inOrder.verify(listener1).callback3(); inOrder.verify(listener1).callback3();
inOrder.verify(listener2).callback3(); inOrder.verify(listener2).callback3();
inOrder.verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2, EVENT_ID_3));
inOrder.verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2, EVENT_ID_3));
inOrder.verifyNoMoreInteractions(); inOrder.verifyNoMoreInteractions();
} }
...@@ -130,7 +136,7 @@ public class ListenerSetTest { ...@@ -130,7 +136,7 @@ public class ListenerSetTest {
// Iteration with single flush. // Iteration with single flush.
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2); listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
// Iteration with multiple flushes. // Iteration with multiple flushes.
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
...@@ -138,11 +144,11 @@ public class ListenerSetTest { ...@@ -138,11 +144,11 @@ public class ListenerSetTest {
listenerSet.flushEvents(); listenerSet.flushEvents();
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
// Iteration with recursive call. // Iteration with recursive call.
listenerSet.sendEvent(EVENT_ID_3, TestListener::callback3); listenerSet.sendEvent(EVENT_ID_3, TestListener::callback3);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2); InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback2(); inOrder.verify(listener1).callback2();
...@@ -191,7 +197,7 @@ public class ListenerSetTest { ...@@ -191,7 +197,7 @@ public class ListenerSetTest {
listenerSet.add(listener3); listenerSet.add(listener3);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2); listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3); InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3);
inOrder.verify(listener1).callback2(); inOrder.verify(listener1).callback2();
...@@ -215,7 +221,7 @@ public class ListenerSetTest { ...@@ -215,7 +221,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1); listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
// Asserts that negative event flag (INDEX_UNSET) can be used without throwing. // Asserts that negative event flag (INDEX_UNSET) can be used without throwing.
} }
...@@ -241,7 +247,7 @@ public class ListenerSetTest { ...@@ -241,7 +247,7 @@ public class ListenerSetTest {
// listener2 was added. // listener2 was added.
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2); listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2); InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1(); inOrder.verify(listener1).callback1();
...@@ -266,7 +272,7 @@ public class ListenerSetTest { ...@@ -266,7 +272,7 @@ public class ListenerSetTest {
listenerSet.add(listener2); listenerSet.add(listener2);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2); listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2); InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1(); inOrder.verify(listener1).callback1();
...@@ -298,7 +304,7 @@ public class ListenerSetTest { ...@@ -298,7 +304,7 @@ public class ListenerSetTest {
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.remove(listener1); listenerSet.remove(listener1);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2); listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
verify(listener1).callback1(); verify(listener1).callback1();
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1)); verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
...@@ -319,7 +325,7 @@ public class ListenerSetTest { ...@@ -319,7 +325,7 @@ public class ListenerSetTest {
listenerSet.remove(listener1); listenerSet.remove(listener1);
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1); listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents(); listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
verify(listener2, times(2)).callback1(); verify(listener2, times(2)).callback1();
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1)); verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
...@@ -346,10 +352,40 @@ public class ListenerSetTest { ...@@ -346,10 +352,40 @@ public class ListenerSetTest {
// Listener2 shouldn't even get this event as it's released before the event can be invoked. // Listener2 shouldn't even get this event as it's released before the event can be invoked.
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2); listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask(); ShadowLooper.idleMainLooper();
verify(listener1).callback1();
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
verifyNoMoreInteractions(listener1, listener2);
}
@Test
public void remove_withRecursionDuringRelease_callsAllPendingEventsAndIterationFinished() {
ListenerSet<TestListener> listenerSet =
new ListenerSet<>(Looper.myLooper(), Clock.DEFAULT, TestListener::iterationFinished);
TestListener listener2 = mock(TestListener.class);
// Listener1 removes Listener2 from within the callback triggered by release().
TestListener listener1 =
spy(
new TestListener() {
@Override
public void iterationFinished(FlagSet flags) {
listenerSet.remove(listener2);
}
});
listenerSet.add(listener1);
listenerSet.add(listener2);
// Listener2 should still get the event and iterationFinished callback because it was triggered
// before the release and the listener removal.
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.release();
ShadowLooper.idleMainLooper();
verify(listener1).callback1(); verify(listener1).callback1();
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1)); verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
verify(listener2).callback1();
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
verifyNoMoreInteractions(listener1, listener2); verifyNoMoreInteractions(listener1, listener2);
} }
......
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