Change how AnalyticsCollector releases listeners

The AnalyticsCollector releases listeners lazily so that listener
callbacks triggered on the application looper after
SimpleExoPlayer.release() are still handled. The change in ListenerSet
to post the onEvents callback on the front of the application looper
changed (correctly) how onEvents are propagated, however this made
the AnalyticsCollector deliver onEvents with out-of-order EventTimes.

This change fixes AnalyticsCollector to trigger onPlayerReleased() and
the matching onEvents() event in the correct order.

#minor-release

PiperOrigin-RevId: 388668739
This commit is contained in:
christosts 2021-08-04 12:55:00 +01:00 committed by Christos Tsilopoulos
parent 6157c615b2
commit 07c49cdad8
3 changed files with 16 additions and 61 deletions

View File

@ -67,7 +67,6 @@ public final class ListenerSet<T> {
} }
private static final int MSG_ITERATION_FINISHED = 0; private static final int MSG_ITERATION_FINISHED = 0;
private static final int MSG_LAZY_RELEASE = 1;
private final Clock clock; private final Clock clock;
private final HandlerWrapper handler; private final HandlerWrapper handler;
@ -220,22 +219,7 @@ public final class ListenerSet<T> {
released = true; released = true;
} }
/**
* 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) { private boolean handleMessage(Message message) {
if (message.what == MSG_ITERATION_FINISHED) {
for (ListenerHolder<T> holder : listeners) { for (ListenerHolder<T> holder : listeners) {
holder.iterationFinished(iterationFinishedEvent); holder.iterationFinished(iterationFinishedEvent);
if (handler.hasMessages(MSG_ITERATION_FINISHED)) { if (handler.hasMessages(MSG_ITERATION_FINISHED)) {
@ -245,13 +229,6 @@ public final class ListenerSet<T> {
break; 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; return true;
} }

View File

@ -22,7 +22,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions;
import android.os.Handler;
import android.os.Looper; import android.os.Looper;
import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
@ -367,34 +366,6 @@ public class ListenerSetTest {
verify(listener, never()).callback1(); verify(listener, never()).callback1();
} }
@Test
public void lazyRelease_stopsForwardingEventsFromNewHandlerMessagesAndCallsReleaseCallback() {
ListenerSet<TestListener> listenerSet =
new ListenerSet<>(Looper.myLooper(), Clock.DEFAULT, 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, times(2)).iterationFinished(createFlagSet(EVENT_ID_1));
verify(listener).iterationFinished(createFlagSet(EVENT_ID_3));
verifyNoMoreInteractions(listener);
}
private interface TestListener { private interface TestListener {
default void callback1() {} default void callback1() {}

View File

@ -16,6 +16,7 @@
package com.google.android.exoplayer2.analytics; package com.google.android.exoplayer2.analytics;
import static com.google.android.exoplayer2.util.Assertions.checkNotNull; import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
import static com.google.android.exoplayer2.util.Assertions.checkStateNotNull;
import android.os.Looper; import android.os.Looper;
import android.util.SparseArray; import android.util.SparseArray;
@ -50,6 +51,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
import com.google.android.exoplayer2.upstream.BandwidthMeter; import com.google.android.exoplayer2.upstream.BandwidthMeter;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.Clock;
import com.google.android.exoplayer2.util.HandlerWrapper;
import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.ListenerSet;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import com.google.android.exoplayer2.video.VideoRendererEventListener; import com.google.android.exoplayer2.video.VideoRendererEventListener;
@ -82,6 +84,7 @@ public class AnalyticsCollector
private ListenerSet<AnalyticsListener> listeners; private ListenerSet<AnalyticsListener> listeners;
private @MonotonicNonNull Player player; private @MonotonicNonNull Player player;
private @MonotonicNonNull HandlerWrapper handler;
private boolean isSeeking; private boolean isSeeking;
/** /**
@ -131,6 +134,7 @@ public class AnalyticsCollector
Assertions.checkState( Assertions.checkState(
this.player == null || mediaPeriodQueueTracker.mediaPeriodQueue.isEmpty()); this.player == null || mediaPeriodQueueTracker.mediaPeriodQueue.isEmpty());
this.player = checkNotNull(player); this.player = checkNotNull(player);
handler = clock.createHandler(looper, null);
listeners = listeners =
listeners.copy( listeners.copy(
looper, looper,
@ -146,10 +150,13 @@ public class AnalyticsCollector
public void release() { public void release() {
EventTime eventTime = generateCurrentPlayerMediaPeriodEventTime(); EventTime eventTime = generateCurrentPlayerMediaPeriodEventTime();
eventTimes.put(AnalyticsListener.EVENT_PLAYER_RELEASED, eventTime); eventTimes.put(AnalyticsListener.EVENT_PLAYER_RELEASED, eventTime);
sendEvent(
eventTime,
AnalyticsListener.EVENT_PLAYER_RELEASED,
listener -> listener.onPlayerReleased(eventTime));
// Release listeners lazily so that all events that got triggered as part of player.release() // Release listeners lazily so that all events that got triggered as part of player.release()
// are still delivered to all listeners. // are still delivered to all listeners.
listeners.lazyRelease( checkStateNotNull(handler).post(() -> listeners.release());
AnalyticsListener.EVENT_PLAYER_RELEASED, listener -> listener.onPlayerReleased(eventTime));
} }
/** /**