Harden media button event sequencing

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)
+        }
+    }
+}

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++
         }

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<MediaEvent>()
+        val handler = MediaButtonEventHandler(
+            scopeProvider = { this },
+            onImmediatePlay = { immediatePlayCount++ },
+            onMediaEvent = events::add,
+        )
+
+        assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY)))
+        assertEquals(1, immediatePlayCount)
+        assertEquals(emptyList<MediaEvent>(), events)
+
+        advanceUntilIdle()
+        assertEquals(emptyList<MediaEvent>(), 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<MediaEvent>()
+        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<Exception>()
+        val events = mutableListOf<MediaEvent>()
+        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<MediaEvent>()
+        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<MediaEvent>()
+        val handler = MediaButtonEventHandler(
+            scopeProvider = { this },
+            onImmediatePlay = {},
+            onMediaEvent = events::add,
+        )
+
+        assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_NEXT)))
+        assertEquals(emptyList<MediaEvent>(), 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<MediaEvent>()
+        val handler = MediaButtonEventHandler(
+            scopeProvider = { cancelledScope },
+            onImmediatePlay = { immediatePlayCount++ },
+            onMediaEvent = events::add,
+        )
+
+        assertTrue(handler.handle(keyEvent(KeyEvent.KEYCODE_MEDIA_PLAY)))
+        assertEquals(0, immediatePlayCount)
+        assertEquals(emptyList<MediaEvent>(), 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)
+}