Skip to content

Commit 88804dd

Browse files
toniheimicrokatz
tonihei
authored and
microkatz
committed
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: #10758 #minor-release PiperOrigin-RevId: 488645089 (cherry picked from commit 3e5103a)
1 parent 5def6e4 commit 88804dd

File tree

2 files changed

+48
-11
lines changed

2 files changed

+48
-11
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ public ListenerHolder(T listener) {
268268
public void release(IterationFinishedEvent<T> event) {
269269
released = true;
270270
if (needsIterationFinishedEvent) {
271+
needsIterationFinishedEvent = false;
271272
event.invoke(listener, flagsBuilder.build());
272273
}
273274
}

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

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void queueEvent_withoutFlush_sendsNoEvents() {
4848

4949
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
5050
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
51-
ShadowLooper.runMainLooperToNextTask();
51+
ShadowLooper.idleMainLooper();
5252

5353
verifyNoMoreInteractions(listener);
5454
}
@@ -66,6 +66,7 @@ public void flushEvents_sendsPreviouslyQueuedEventsToAllListeners() {
6666
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
6767
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
6868
listenerSet.flushEvents();
69+
ShadowLooper.idleMainLooper();
6970

7071
InOrder inOrder = Mockito.inOrder(listener1, listener2);
7172
inOrder.verify(listener1).callback1();
@@ -74,6 +75,8 @@ public void flushEvents_sendsPreviouslyQueuedEventsToAllListeners() {
7475
inOrder.verify(listener2).callback2();
7576
inOrder.verify(listener1).callback1();
7677
inOrder.verify(listener2).callback1();
78+
inOrder.verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2));
79+
inOrder.verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2));
7780
inOrder.verifyNoMoreInteractions();
7881
}
7982

@@ -98,6 +101,7 @@ public void callback1() {
98101
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
99102
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
100103
listenerSet.flushEvents();
104+
ShadowLooper.idleMainLooper();
101105

102106
InOrder inOrder = Mockito.inOrder(listener1, listener2);
103107
inOrder.verify(listener1).callback1();
@@ -106,6 +110,8 @@ public void callback1() {
106110
inOrder.verify(listener2).callback2();
107111
inOrder.verify(listener1).callback3();
108112
inOrder.verify(listener2).callback3();
113+
inOrder.verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2, EVENT_ID_3));
114+
inOrder.verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1, EVENT_ID_2, EVENT_ID_3));
109115
inOrder.verifyNoMoreInteractions();
110116
}
111117

@@ -130,19 +136,19 @@ public void callback3() {
130136
// Iteration with single flush.
131137
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
132138
listenerSet.flushEvents();
133-
ShadowLooper.runMainLooperToNextTask();
139+
ShadowLooper.idleMainLooper();
134140

135141
// Iteration with multiple flushes.
136142
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
137143
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
138144
listenerSet.flushEvents();
139145
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
140146
listenerSet.flushEvents();
141-
ShadowLooper.runMainLooperToNextTask();
147+
ShadowLooper.idleMainLooper();
142148

143149
// Iteration with recursive call.
144150
listenerSet.sendEvent(EVENT_ID_3, TestListener::callback3);
145-
ShadowLooper.runMainLooperToNextTask();
151+
ShadowLooper.idleMainLooper();
146152

147153
InOrder inOrder = Mockito.inOrder(listener1, listener2);
148154
inOrder.verify(listener1).callback2();
@@ -191,7 +197,7 @@ public void iterationFinished(FlagSet flags) {
191197
listenerSet.add(listener3);
192198

193199
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
194-
ShadowLooper.runMainLooperToNextTask();
200+
ShadowLooper.idleMainLooper();
195201

196202
InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3);
197203
inOrder.verify(listener1).callback2();
@@ -215,7 +221,7 @@ public void flushEvents_withUnsetEventFlag_doesNotThrow() {
215221

216222
listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1);
217223
listenerSet.flushEvents();
218-
ShadowLooper.runMainLooperToNextTask();
224+
ShadowLooper.idleMainLooper();
219225

220226
// Asserts that negative event flag (INDEX_UNSET) can be used without throwing.
221227
}
@@ -241,7 +247,7 @@ public void callback1() {
241247
// listener2 was added.
242248
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
243249
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
244-
ShadowLooper.runMainLooperToNextTask();
250+
ShadowLooper.idleMainLooper();
245251

246252
InOrder inOrder = Mockito.inOrder(listener1, listener2);
247253
inOrder.verify(listener1).callback1();
@@ -266,7 +272,7 @@ public void add_withQueueing_onlyReceivesUpdatesForFutureEvents() {
266272
listenerSet.add(listener2);
267273
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
268274
listenerSet.flushEvents();
269-
ShadowLooper.runMainLooperToNextTask();
275+
ShadowLooper.idleMainLooper();
270276

271277
InOrder inOrder = Mockito.inOrder(listener1, listener2);
272278
inOrder.verify(listener1).callback1();
@@ -298,7 +304,7 @@ public void callback1() {
298304
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
299305
listenerSet.remove(listener1);
300306
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
301-
ShadowLooper.runMainLooperToNextTask();
307+
ShadowLooper.idleMainLooper();
302308

303309
verify(listener1).callback1();
304310
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
@@ -319,7 +325,7 @@ public void remove_withQueueing_stopsReceivingEventsImmediately() {
319325
listenerSet.remove(listener1);
320326
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
321327
listenerSet.flushEvents();
322-
ShadowLooper.runMainLooperToNextTask();
328+
ShadowLooper.idleMainLooper();
323329

324330
verify(listener2, times(2)).callback1();
325331
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
@@ -346,10 +352,40 @@ public void callback1() {
346352
// Listener2 shouldn't even get this event as it's released before the event can be invoked.
347353
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
348354
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
349-
ShadowLooper.runMainLooperToNextTask();
355+
ShadowLooper.idleMainLooper();
356+
357+
verify(listener1).callback1();
358+
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
359+
verifyNoMoreInteractions(listener1, listener2);
360+
}
361+
362+
@Test
363+
public void remove_withRecursionDuringRelease_callsAllPendingEventsAndIterationFinished() {
364+
ListenerSet<TestListener> listenerSet =
365+
new ListenerSet<>(Looper.myLooper(), Clock.DEFAULT, TestListener::iterationFinished);
366+
TestListener listener2 = mock(TestListener.class);
367+
// Listener1 removes Listener2 from within the callback triggered by release().
368+
TestListener listener1 =
369+
spy(
370+
new TestListener() {
371+
@Override
372+
public void iterationFinished(FlagSet flags) {
373+
listenerSet.remove(listener2);
374+
}
375+
});
376+
listenerSet.add(listener1);
377+
listenerSet.add(listener2);
378+
379+
// Listener2 should still get the event and iterationFinished callback because it was triggered
380+
// before the release and the listener removal.
381+
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
382+
listenerSet.release();
383+
ShadowLooper.idleMainLooper();
350384

351385
verify(listener1).callback1();
352386
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
387+
verify(listener2).callback1();
388+
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
353389
verifyNoMoreInteractions(listener1, listener2);
354390
}
355391

0 commit comments

Comments
 (0)