diff --git a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandler.kt b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandler.kt new file mode 100644 index 00000000000..8c117d84616 --- /dev/null +++ b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandler.kt @@ -0,0 +1,87 @@ +package au.com.shiftyjelly.pocketcasts.repositories.playback + +import android.view.KeyEvent +import au.com.shiftyjelly.pocketcasts.utils.log.LogBuffer +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.yield + +/** + * Serializes media-button key events before dispatching their resolved actions. + * + * Event registration starts synchronously to preserve framework callback order. [onImmediatePlay] may therefore run + * on the caller's stack and must stay fast. [onMediaEvent] runs only after a suspension boundary, outside that + * synchronous registration section. + */ +internal class MediaButtonEventHandler( + private val scopeProvider: () -> CoroutineScope, + private val onImmediatePlay: () -> Unit, + private val onMediaEvent: (MediaEvent) -> Unit, + private val onError: (Exception) -> Unit = { + LogBuffer.e(LogBuffer.TAG_PLAYBACK, it, "Media button event handling failed") + }, +) { + private val mediaEventQueue = MediaEventQueue(scopeProvider) + + private val scope: CoroutineScope get() = scopeProvider() + + fun handle(keyEvent: KeyEvent): Boolean { + if (keyEvent.action != KeyEvent.ACTION_DOWN) { + return false + } + + val inputEvent = when (keyEvent.keyCode) { + KeyEvent.KEYCODE_MEDIA_PLAY, + KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE, + KeyEvent.KEYCODE_HEADSETHOOK, + -> MediaEvent.SingleTap + + KeyEvent.KEYCODE_MEDIA_NEXT -> MediaEvent.DoubleTap + + KeyEvent.KEYCODE_MEDIA_PREVIOUS -> MediaEvent.TripleTap + + else -> null + } ?: return false + + val immediateSingleTapHandler = if (keyEvent.keyCode == KeyEvent.KEYCODE_MEDIA_PLAY) { + ::handleImmediatePlay + } else { + null + } + + // Register the event before returning to the framework callback. This preserves + // delivery order while the queue's timeout still resumes on the provided scope. + scope.launch(start = CoroutineStart.UNDISPATCHED) { + try { + coroutineContext.ensureActive() + val outputEvent = mediaEventQueue.consumeEvent( + event = inputEvent, + onImmediateSingleTap = immediateSingleTapHandler, + ) + if (outputEvent != null) { + // Output actions historically ran asynchronously on the callback scope. + yield() + onMediaEvent(outputEvent) + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + onError(e) + } + } + return true + } + + private fun handleImmediatePlay() { + try { + onImmediatePlay() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + onError(e) + } + } +} diff --git a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueue.kt b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueue.kt index 4393f441e84..fb048c85a3c 100644 --- a/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueue.kt +++ b/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueue.kt @@ -4,49 +4,70 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock internal class MediaEventQueue( private val scopeProvider: () -> CoroutineScope, ) { private var singleTapJob: SingleTapJob? = null private var multiTapJob: Job? = null + private val stateMutex = Mutex() private val scope: CoroutineScope get() = scopeProvider() - suspend fun consumeEvent(event: MediaEvent) = when (event) { - MediaEvent.SingleTap -> handleSingleTapEvent() + suspend fun consumeEvent( + event: MediaEvent, + onImmediateSingleTap: (() -> Unit)? = null, + ) = when (event) { + MediaEvent.SingleTap -> handleSingleTapEvent(onImmediateSingleTap) MediaEvent.DoubleTap, MediaEvent.TripleTap -> handleMultiTapEvent(event) } - private suspend fun handleSingleTapEvent(): MediaEvent? { - val currentSingleTapJob = singleTapJob - return when { - // Pixel Buds (and possibly other headphones) trigger KEYCODE_MEDIA_PLAY - // after KEYCODE_MEDIA_NEXT or KEYCODE_MEDIA_PREVIOUS. - // We need to ignore it so the single tap action isn't triggered in such cases. - multiTapJob?.isActive == true -> { - null - } + private suspend fun handleSingleTapEvent(onImmediateSingleTap: (() -> Unit)?): MediaEvent? { + val newSingleTapJob = stateMutex.withLock { + val currentSingleTapJob = singleTapJob + when { + // Pixel Buds (and possibly other headphones) trigger KEYCODE_MEDIA_PLAY + // after KEYCODE_MEDIA_NEXT or KEYCODE_MEDIA_PREVIOUS. + // We need to ignore it so the single tap action isn't triggered in such cases. + multiTapJob?.isActive == true -> null + + currentSingleTapJob?.isActive == true -> { + currentSingleTapJob.incrementTaps() + null + } - currentSingleTapJob?.isActive == true -> { - currentSingleTapJob.incrementTaps() - null + else -> SingleTapJob(scope).also { singleTapJob = it } } + } ?: return null - else -> { - val newSingleTapJob = SingleTapJob(scope) - singleTapJob = newSingleTapJob - newSingleTapJob.await() - newSingleTapJob.event() + try { + onImmediateSingleTap?.invoke() + } catch (e: Exception) { + stateMutex.withLock { + if (singleTapJob === newSingleTapJob) { + singleTapJob = null + newSingleTapJob.cancel() + } + } + throw e + } + newSingleTapJob.await() + return stateMutex.withLock { + // The immediate callback owns a resolved SingleTap. Follow-up taps still + // return their DoubleTap or TripleTap action after the window closes. + newSingleTapJob.event().takeUnless { + it == MediaEvent.SingleTap && onImmediateSingleTap != null } } } - private fun handleMultiTapEvent(event: MediaEvent): MediaEvent { + private suspend fun handleMultiTapEvent(event: MediaEvent): MediaEvent = stateMutex.withLock { val currentJob = multiTapJob multiTapJob = scope.launch { delay(250) } currentJob?.cancel() - return event + event } private class SingleTapJob( @@ -60,6 +81,8 @@ internal class MediaEventQueue( suspend fun await() = job.join() + fun cancel() = job.cancel() + fun incrementTaps() { counter++ } diff --git a/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandlerTest.kt b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandlerTest.kt new file mode 100644 index 00000000000..60f871666bf --- /dev/null +++ b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaButtonEventHandlerTest.kt @@ -0,0 +1,145 @@ +package au.com.shiftyjelly.pocketcasts.repositories.playback + +import android.view.KeyEvent +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +@OptIn(ExperimentalCoroutinesApi::class) +class MediaButtonEventHandlerTest { + @Test + fun `KEYCODE_MEDIA_PLAY runs the immediate action without a delayed single tap`() = runTest { + var immediatePlayCount = 0 + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = { immediatePlayCount++ }, + onMediaEvent = events::add, + ) + + assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY))) + assertEquals(1, immediatePlayCount) + assertEquals(emptyList(), events) + + advanceUntilIdle() + assertEquals(emptyList(), events) + } + + @Test + fun `rapid KEYCODE_MEDIA_PLAY events run the immediate action once and emit a double tap`() = runTest { + var immediatePlayCount = 0 + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = { immediatePlayCount++ }, + onMediaEvent = events::add, + ) + + handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY)) + handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY)) + + assertEquals(1, immediatePlayCount) + + advanceUntilIdle() + assertEquals(listOf(MediaEvent.DoubleTap), events) + } + + @Test + fun `immediate play failure is reported without losing the resolved double tap`() = runTest { + val failure = IllegalStateException("Immediate action failed") + val errors = mutableListOf() + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = { throw failure }, + onMediaEvent = events::add, + onError = errors::add, + ) + + assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY))) + assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY))) + + advanceUntilIdle() + assertEquals(listOf(failure), errors) + assertEquals(listOf(MediaEvent.DoubleTap), events) + } + + @Test + fun `KEYCODE_MEDIA_NEXT suppresses a following KEYCODE_MEDIA_PLAY`() = runTest { + var immediatePlayCount = 0 + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = { immediatePlayCount++ }, + onMediaEvent = events::add, + ) + + handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_NEXT)) + handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY)) + + assertEquals(0, immediatePlayCount) + + advanceUntilIdle() + assertEquals(listOf(MediaEvent.DoubleTap), events) + } + + @Test + fun `resolved multi tap actions are deferred beyond event registration`() = runTest { + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = {}, + onMediaEvent = events::add, + ) + + assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_NEXT))) + assertEquals(emptyList(), events) + + runCurrent() + assertEquals(listOf(MediaEvent.DoubleTap), events) + } + + @Test + fun `cancelled scope does not handle events`() = runTest { + val cancelledJob = Job().apply { cancel() } + val cancelledScope = CoroutineScope(coroutineContext + cancelledJob) + var immediatePlayCount = 0 + val events = mutableListOf() + val handler = MediaButtonEventHandler( + scopeProvider = { cancelledScope }, + onImmediatePlay = { immediatePlayCount++ }, + onMediaEvent = events::add, + ) + + assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY))) + assertEquals(0, immediatePlayCount) + assertEquals(emptyList(), events) + } + + @Test + fun `unhandled key events return false`() = runTest { + val handler = MediaButtonEventHandler( + scopeProvider = { this }, + onImmediatePlay = {}, + onMediaEvent = {}, + ) + + assertFalse(handler.handle(keyEvent(KeyEvent.KEYCODE_VOLUME_UP))) + assertFalse(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY, KeyEvent.ACTION_UP))) + } + + private fun keyEvent( + keyCode: Int, + action: Int = KeyEvent.ACTION_DOWN, + ) = KeyEvent(action, keyCode) +} diff --git a/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueueTest.kt b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueueTest.kt index 3e354c6bcac..931a0d150cf 100644 --- a/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueueTest.kt +++ b/modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaEventQueueTest.kt @@ -1,11 +1,19 @@ package au.com.shiftyjelly.pocketcasts.repositories.playback +import java.util.concurrent.CyclicBarrier +import java.util.concurrent.Executors +import java.util.concurrent.atomic.AtomicInteger +import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import kotlinx.coroutines.yield import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test class MediaEventQueueTest { @@ -75,6 +83,94 @@ class MediaEventQueueTest { assertEquals(MediaEvent.TripleTap, firstEvent.await()) } + @Test + fun `handle an immediate single tap before the multi tap window expires`() = runTest { + val handler = MediaEventQueue(scopeProvider = { this }) + var isHandled = false + + val event = async { + handler.consumeEvent(MediaEvent.SingleTap) { + isHandled = true + } + } + + yield() + assertTrue(isHandled) + assertNull(event.await()) + } + + @Test + fun `map immediate single taps to multi tap events`() = runTest { + val handler = MediaEventQueue(scopeProvider = { this }) + var immediateTapCount = 0 + + val firstEvent = async { + handler.consumeEvent(MediaEvent.SingleTap) { + immediateTapCount++ + } + } + + yield() + assertNull( + handler.consumeEvent(MediaEvent.SingleTap) { + immediateTapCount++ + }, + ) + + assertEquals(1, immediateTapCount) + assertEquals(MediaEvent.DoubleTap, firstEvent.await()) + } + + @Test + fun `immediate single tap failure does not orphan the tap window`() = runTest { + val handler = MediaEventQueue(scopeProvider = { this }) + val failure = IllegalStateException("Immediate action failed") + + val thrown = runCatching { + handler.consumeEvent(MediaEvent.SingleTap) { throw failure } + }.exceptionOrNull() + + assertEquals(failure, thrown) + assertEquals(MediaEvent.SingleTap, handler.consumeEvent(MediaEvent.SingleTap)) + } + + @Test + fun `handle concurrent immediate single taps exactly once`() = runBlocking { + val handler = MediaEventQueue(scopeProvider = { this }) + val immediateTapCount = AtomicInteger() + val eventCount = 8 + val startBarrier = CyclicBarrier(eventCount) + val dispatcher = Executors.newFixedThreadPool(eventCount).asCoroutineDispatcher() + + dispatcher.use { + List(eventCount) { + async(dispatcher) { + startBarrier.await() + handler.consumeEvent(MediaEvent.SingleTap) { + immediateTapCount.incrementAndGet() + } + } + }.awaitAll() + } + + assertEquals(1, immediateTapCount.get()) + } + + @Test + fun `do not handle immediate single tap while multi tap window is active`() = runTest { + val handler = MediaEventQueue(scopeProvider = { this }) + var isHandled = false + + handler.consumeEvent(MediaEvent.DoubleTap) + + assertNull( + handler.consumeEvent(MediaEvent.SingleTap) { + isHandled = true + }, + ) + assertFalse(isHandled) + } + @Test fun `map single tap events to multi tap event in time window`() = runTest { val handler = MediaEventQueue(scopeProvider = { this })