Commit 7756d982 by jaewan Committed by Ian Baker

Fix NPE in MediaController's constructor

MediaController tries to release itself when the binder to the
session becomes unavailable. However, if such thing happens while
connecting in the constructor, it causes NPE when accessing
MediaControllerImpl which is only initialized after it's
connected.

This fixes random failures in existing tests.

PiperOrigin-RevId: 421423381
parent 9a4ad055
...@@ -374,6 +374,7 @@ public class MediaController implements Player { ...@@ -374,6 +374,7 @@ public class MediaController implements Player {
@Initialized @Initialized
MediaController thisRef = this; MediaController thisRef = this;
impl = thisRef.createImpl(context, thisRef, token, connectionHints); impl = thisRef.createImpl(context, thisRef, token, connectionHints);
impl.connect();
} }
/* package */ MediaControllerImpl createImpl( /* package */ MediaControllerImpl createImpl(
...@@ -1823,6 +1824,8 @@ public class MediaController implements Player { ...@@ -1823,6 +1824,8 @@ public class MediaController implements Player {
interface MediaControllerImpl { interface MediaControllerImpl {
void connect();
void addListener(Player.Listener listener); void addListener(Player.Listener listener);
void removeListener(Player.Listener listener); void removeListener(Player.Listener listener);
......
...@@ -166,6 +166,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; ...@@ -166,6 +166,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
private final Context context; private final Context context;
private final SessionToken token; private final SessionToken token;
private final Bundle connectionHints;
private final IBinder.DeathRecipient deathRecipient; private final IBinder.DeathRecipient deathRecipient;
private final SurfaceCallback surfaceCallback; private final SurfaceCallback surfaceCallback;
private final ListenerSet<Listener> listeners; private final ListenerSet<Listener> listeners;
...@@ -211,12 +212,24 @@ import org.checkerframework.checker.nullness.qual.NonNull; ...@@ -211,12 +212,24 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sequencedFutureManager = new SequencedFutureManager(); sequencedFutureManager = new SequencedFutureManager();
controllerStub = new MediaControllerStub(this); controllerStub = new MediaControllerStub(this);
this.token = token; this.token = token;
this.connectionHints = connectionHints;
deathRecipient = deathRecipient =
() -> () ->
MediaControllerImplBase.this.instance.runOnApplicationLooper( MediaControllerImplBase.this.instance.runOnApplicationLooper(
MediaControllerImplBase.this.instance::release); MediaControllerImplBase.this.instance::release);
surfaceCallback = new SurfaceCallback(); surfaceCallback = new SurfaceCallback();
serviceConnection =
(this.token.getType() == TYPE_SESSION)
? null
: new SessionServiceConnection(connectionHints);
flushCommandQueueHandler = new FlushCommandQueueHandler(instance.getApplicationLooper());
lastReturnedContentPositionMs = C.TIME_UNSET;
lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET;
}
@Override
public void connect() {
boolean connectionRequested; boolean connectionRequested;
if (this.token.getType() == TYPE_SESSION) { if (this.token.getType() == TYPE_SESSION) {
// Session // Session
...@@ -229,9 +242,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; ...@@ -229,9 +242,6 @@ import org.checkerframework.checker.nullness.qual.NonNull;
if (!connectionRequested) { if (!connectionRequested) {
this.instance.runOnApplicationLooper(MediaControllerImplBase.this.instance::release); this.instance.runOnApplicationLooper(MediaControllerImplBase.this.instance::release);
} }
flushCommandQueueHandler = new FlushCommandQueueHandler(instance.getApplicationLooper());
lastReturnedContentPositionMs = C.TIME_UNSET;
lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET;
} }
@Override @Override
......
...@@ -155,9 +155,11 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -155,9 +155,11 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
this.instance = instance; this.instance = instance;
controllerCompatCallback = new ControllerCompatCallback(); controllerCompatCallback = new ControllerCompatCallback();
this.token = token; this.token = token;
}
@Override
public void connect() {
if (this.token.getType() == SessionToken.TYPE_SESSION) { if (this.token.getType() == SessionToken.TYPE_SESSION) {
browserCompat = null;
connectToSession((MediaSessionCompat.Token) checkStateNotNull(this.token.getBinder())); connectToSession((MediaSessionCompat.Token) checkStateNotNull(this.token.getBinder()));
} else { } else {
connectToService(); connectToService();
......
...@@ -38,6 +38,7 @@ import java.util.concurrent.ExecutionException; ...@@ -38,6 +38,7 @@ import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.Before; import org.junit.Before;
import org.junit.ClassRule; import org.junit.ClassRule;
import org.junit.Rule; import org.junit.Rule;
...@@ -117,11 +118,15 @@ public class MediaSessionAndControllerTest { ...@@ -117,11 +118,15 @@ public class MediaSessionAndControllerTest {
Handler mainHandler = new Handler(Looper.getMainLooper()); Handler mainHandler = new Handler(Looper.getMainLooper());
Executor mainExecutor = (runnable) -> Util.postOrRun(mainHandler, runnable); Executor mainExecutor = (runnable) -> Util.postOrRun(mainHandler, runnable);
for (int i = 0; i < 100; i++) { for (int i = 0; i < 100; i++) {
int idx = i;
MediaSession session = MediaSession session =
sessionTestRule.ensureReleaseAfterTest( sessionTestRule.ensureReleaseAfterTest(
new MediaSession.Builder(context, player).setId(TAG).build()); new MediaSession.Builder(context, player).setId(TAG).build());
CountDownLatch latch = new CountDownLatch(1); CountDownLatch latch = new CountDownLatch(1);
// Keep the instance in AtomicReference to prevent GC.
// Otherwise ListenableFuture and corresponding controllers can be GCed and disconnected
// callback can be ignored.
AtomicReference<ListenableFuture<MediaController>> controllerFutureRef =
new AtomicReference<>();
mainHandler.post( mainHandler.post(
() -> { () -> {
ListenableFuture<MediaController> controllerFuture = ListenableFuture<MediaController> controllerFuture =
...@@ -134,6 +139,7 @@ public class MediaSessionAndControllerTest { ...@@ -134,6 +139,7 @@ public class MediaSessionAndControllerTest {
} }
}) })
.buildAsync(); .buildAsync();
controllerFutureRef.set(controllerFuture);
controllerFuture.addListener( controllerFuture.addListener(
() -> { () -> {
try { try {
......
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