From b24161a6f6af184840d9eae7a63276b73a4a4d80 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 10 Nov 2022 11:10:11 +0000 Subject: [PATCH] Avoid notifying connection twice from MediaControllerImplLegacy. The connection to a legacy MediaSession may receive additional onSessionReady callbacks that are treated as additional state updates. We currently also set the "notifyConnected" flag for these updates even though we are connected already, causing an IllegalStateException. Fix the exception by not setting this flag. We can also remove the wording about "locked" updates since this class operates everything on a single application thread. Issue: androidx/media#49 PiperOrigin-RevId: 487487286 --- RELEASENOTES.md | 3 +++ .../session/MediaControllerImplLegacy.java | 17 +++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index a4a54914ff3..ecb022a96fe 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -99,6 +99,9 @@ Release notes `DefaultNotificationProvider` on API 26 and API 27 (the badge is automatically hidden on API 28+) ([#131](https://github.com/androidx/media/issues/131)). + * Fix bug where a second binder connection from a legacy MediaSession to a + Media3 MediaController causes IllegalStateExceptions + ([#49](https://github.com/androidx/media/issues/49)). * RTSP: * Add H263 fragmented packet handling ([#119](https://github.com/androidx/media/pull/119)). diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java index f6c9ef440bd..aafc21450af 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java @@ -1211,8 +1211,7 @@ public MediaBrowserCompat getBrowserCompat() { return browserCompat; } - // Should be used without a lock to prevent potential deadlock. - void onConnectedNotLocked() { + void onConnected() { if (released || connected) { return; } @@ -1240,18 +1239,16 @@ private void connectToSession(MediaSessionCompat.Token sessionCompatToken) { controllerCompat.registerCallback( controllerCompatCallback, getInstance().applicationHandler); }); - // Post a runnable to prevent callbacks from being called by onConnectedNotLocked() + // Post a runnable to prevent callbacks from being called by onConnected() // before the constructor returns (b/196941334). getInstance() .applicationHandler .post( () -> { if (!controllerCompat.isSessionReady()) { - // If the session not ready here, then call onConnectedNotLocked() immediately. The - // session may be a framework MediaSession and we cannot know whether it can be - // ready - // later. - onConnectedNotLocked(); + // If the session not ready here, then call onConnected() immediately. The session + // may be a framework MediaSession and we cannot know whether it can be ready later. + onConnected(); } }); } @@ -1608,7 +1605,7 @@ public void release() { @Override public void onSessionReady() { if (!connected) { - onConnectedNotLocked(); + onConnected(); } else { // Handle the case when extra binder is available after connectToSession(). // Initial values are notified already, so only notify values that are available when @@ -1622,7 +1619,7 @@ public void onSessionReady() { onCaptioningEnabledChanged(isCaptioningEnabled); pendingChangesHandler.removeMessages(MSG_HANDLE_PENDING_UPDATES); - handleNewLegacyParameters(/* notifyConnected= */ true, pendingLegacyPlayerInfo); + handleNewLegacyParameters(/* notifyConnected= */ false, pendingLegacyPlayerInfo); } }