loliball/media
1
0
Fork 0
mirror of https://github.com/androidx/media.git synced 2025-04-30 06:46:50 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Mark iterationFinished when triggering release event.

Browse Source 
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

PiperOrigin-RevId: 488645089
(cherry picked from commit 1def68bf3c00035aad0821d3997187317ce099dc)
This commit is contained in:
tonihei 2022-11-15 14:48:02 +00:00 committed by microkatz
parent bf67d1cc11
commit dab5b3c152
3 changed files with 51 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
RELEASENOTES.md
View File

@ -30,6 +30,9 @@ Release notes
([#10684](https://github.com/google/ExoPlayer/issues/10684)).
* Add `Player.getVideoSurfaceSize` that returns the size of the surface on
which the video is rendered.
* Fix bug where removing listeners during the player release can cause an
`IllegalStateException`
([#10758](https://github.com/google/ExoPlayer/issues/10758)).
* Build:
* Avoid publishing block when included in another gradle build.
* Downloads:

1
libraries/common/src/main/java/androidx/media3/common/util/ListenerSet.java
View File

@ -270,6 +270,7 @@ public final class ListenerSet<T extends @NonNull Object> {
public void release(IterationFinishedEvent<T> event) {
released = true;
if (needsIterationFinishedEvent) {
needsIterationFinishedEvent = false;
event.invoke(listener, flagsBuilder.build());
}
}

58
libraries/common/src/test/java/androidx/media3/common/util/ListenerSetTest.java
View File

@ -49,7 +49,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
verifyNoMoreInteractions(listener);
}
@ -67,6 +67,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1();
@ -75,6 +76,8 @@ public class ListenerSetTest {
inOrder.verify(listener2).callback2();
inOrder.verify(listener1).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();
}
@ -99,6 +102,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1();
@ -107,6 +111,8 @@ public class ListenerSetTest {
inOrder.verify(listener2).callback2();
inOrder.verify(listener1).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();
}
@ -131,7 +137,7 @@ public class ListenerSetTest {
// Iteration with single flush.
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
// Iteration with multiple flushes.
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
@ -139,11 +145,11 @@ public class ListenerSetTest {
listenerSet.flushEvents();
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
// Iteration with recursive call.
listenerSet.sendEvent(EVENT_ID_3, TestListener::callback3);
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback2();
@ -192,7 +198,7 @@ public class ListenerSetTest {
listenerSet.add(listener3);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3);
inOrder.verify(listener1).callback2();
@ -216,7 +222,7 @@ public class ListenerSetTest {
listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1);
listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
// Asserts that negative event flag (INDEX_UNSET) can be used without throwing.
}
@ -242,7 +248,7 @@ public class ListenerSetTest {
// listener2 was added.
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1();
@ -267,7 +273,7 @@ public class ListenerSetTest {
listenerSet.add(listener2);
listenerSet.queueEvent(EVENT_ID_2, TestListener::callback2);
listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
InOrder inOrder = Mockito.inOrder(listener1, listener2);
inOrder.verify(listener1).callback1();
@ -299,7 +305,7 @@ public class ListenerSetTest {
listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.remove(listener1);
listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2);
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
verify(listener1).callback1();
verify(listener1).iterationFinished(createFlagSet(EVENT_ID_1));
@ -320,7 +326,7 @@ public class ListenerSetTest {
listenerSet.remove(listener1);
listenerSet.queueEvent(EVENT_ID_1, TestListener::callback1);
listenerSet.flushEvents();
ShadowLooper.runMainLooperToNextTask();
ShadowLooper.idleMainLooper();
verify(listener2, times(2)).callback1();
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
@ -347,13 +353,43 @@ public class ListenerSetTest {
// 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_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).iterationFinished(createFlagSet(EVENT_ID_1));
verify(listener2).callback1();
verify(listener2).iterationFinished(createFlagSet(EVENT_ID_1));
verifyNoMoreInteractions(listener1, listener2);
}
@Test
public void release_preventsRegisteringNewListeners() {
ListenerSet<TestListener> listenerSet =