diff --git a/CHANGELOG.md b/CHANGELOG.md index 502af60a79f..15688686b08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ * Bug Fixes * Allow copying description and show notes links with a long press ([#5628](https://github.com/Automattic/pocket-casts-android/pull/5628)) + * Prevent a failed Cast session resume from interrupting local playback or removing playback controls + ([#5656](https://github.com/Automattic/pocket-casts-android/pull/5656)) 8.17 ----- diff --git a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManager.kt b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManager.kt index e4d0fb8fbfa..fb3f98bc33f 100644 --- a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManager.kt +++ b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManager.kt @@ -6,6 +6,11 @@ interface CastManager { val isConnectedFlow: StateFlow + enum class SessionFailureType { + START, + RESUME, + } + suspend fun isAvailable(): Boolean suspend fun isConnected(): Boolean suspend fun endSession() @@ -17,6 +22,6 @@ interface CastManager { fun sessionStarted() fun sessionEnded() fun sessionReconnected() - fun sessionFailed(errorCode: Int) {} + fun sessionFailed(errorCode: Int, failureType: SessionFailureType) {} } } diff --git a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManagerImpl.kt b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManagerImpl.kt index 0a751e72ed3..bf20bbdd7bf 100644 --- a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManagerImpl.kt +++ b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/chromecast/CastManagerImpl.kt @@ -124,7 +124,7 @@ class CastManagerImpl @Inject constructor( override fun onSessionStartFailed(session: Session, i: Int) { LogBuffer.e(LogBuffer.TAG_PLAYBACK, "Cast session start failed with error code $i") - sessionListener?.sessionFailed(i) + sessionListener?.sessionFailed(i, CastManager.SessionFailureType.START) } override fun onSessionEnding(session: Session) { @@ -150,7 +150,7 @@ class CastManagerImpl @Inject constructor( override fun onSessionResumeFailed(session: Session, i: Int) { LogBuffer.e(LogBuffer.TAG_PLAYBACK, "Cast session resume failed with error code $i") - sessionListener?.sessionFailed(i) + sessionListener?.sessionFailed(i, CastManager.SessionFailureType.RESUME) } override fun onSessionSuspended(session: Session, i: Int) { diff --git a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt index 717da8d82d9..1b1661ad31e 100644 --- a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt +++ b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt @@ -308,7 +308,16 @@ open class PlaybackManager @Inject constructor( castReconnected() } - override fun sessionFailed(errorCode: Int) { + override fun sessionFailed(errorCode: Int, failureType: CastManager.SessionFailureType) { + // The Cast SDK can resume a saved session without user interaction. A failed cold resume must not + // turn healthy local playback into an error or tear down its media notification. + if (!shouldSurfaceCastSessionFailure(failureType, isCastPlayerActive = player?.isRemote == true)) { + LogBuffer.i( + LogBuffer.TAG_PLAYBACK, + "Ignoring Cast session resume failure with error code $errorCode during local playback", + ) + return + } LogBuffer.e(LogBuffer.TAG_PLAYBACK, "Cast session failed with error code $errorCode") launch(Dispatchers.Main) { val message = application.getString(LR.string.error_cast_connection_failed) @@ -2928,6 +2937,16 @@ open class PlaybackManager @Inject constructor( } } +internal fun shouldSurfaceCastSessionFailure( + failureType: CastManager.SessionFailureType, + isCastPlayerActive: Boolean, +): Boolean { + return when (failureType) { + CastManager.SessionFailureType.START -> true + CastManager.SessionFailureType.RESUME -> isCastPlayerActive + } +} + internal data class PrefetchRequest( val episodeUuid: String, val downloadUrl: String, diff --git a/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/CastSessionFailureTest.kt b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/CastSessionFailureTest.kt new file mode 100644 index 00000000000..73ca5cb0a6a --- /dev/null +++ b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/CastSessionFailureTest.kt @@ -0,0 +1,39 @@ +package au.com.shiftyjelly.pocketcasts.repositories.playback + +import au.com.shiftyjelly.pocketcasts.repositories.chromecast.CastManager +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class CastSessionFailureTest { + + @Test + fun `start failure is surfaced during local playback`() { + val shouldSurface = shouldSurfaceCastSessionFailure( + failureType = CastManager.SessionFailureType.START, + isCastPlayerActive = false, + ) + + assertTrue(shouldSurface) + } + + @Test + fun `resume failure is ignored during local playback`() { + val shouldSurface = shouldSurfaceCastSessionFailure( + failureType = CastManager.SessionFailureType.RESUME, + isCastPlayerActive = false, + ) + + assertFalse(shouldSurface) + } + + @Test + fun `resume failure is surfaced when Cast player is active`() { + val shouldSurface = shouldSurfaceCastSessionFailure( + failureType = CastManager.SessionFailureType.RESUME, + isCastPlayerActive = true, + ) + + assertTrue(shouldSurface) + } +}