playback: take the start lock before reading the tap preference

This commit is contained in:
austinried
2026-11-12 07:39:14 +00:00
parent e94b365495
commit 2bbff7e477
4 changed files with 97 additions and 8 deletions
@@ -990,19 +990,28 @@ class PlaybackController(
connect()
return
}
scope.launch { start(entry, startPosition, behavior ?: preferences.songTapBehavior().first()) }
scope.launch {
startLock.withLock {
startLocked(entry, startPosition, behavior ?: preferences.songTapBehavior().first())
}
}
}
private suspend fun startPending(pending: PendingPlay) {
val behavior = pending.behavior ?: preferences.songTapBehavior().first()
start(pending.entry, pending.position, behavior)
start(pending.entry, pending.position, pending.behavior ?: preferences.songTapBehavior().first())
}
private suspend fun start(
entry: QueueEntry,
startPosition: Long,
behavior: SongTapBehavior,
) = startLock.withLock {
) = startLock.withLock { startLocked(entry, startPosition, behavior) }
private suspend fun startLocked(
entry: QueueEntry,
startPosition: Long,
behavior: SongTapBehavior,
) {
val modes = queueRepository.modes()
val shuffle =
when (behavior) {
@@ -1017,7 +1026,7 @@ class PlaybackController(
queueSourceId = entry.sourceId
if (snapshot.size == 0L) {
stopLocked()
return@withLock
return
}
val start = startPosition.coerceIn(0, snapshot.contextSize - 1)
val position =
@@ -849,8 +849,6 @@ private fun SortNameModeDialog(
)
}
private val songTapBehaviorOptions = SongTapBehavior.entries
private fun songTapBehaviorLabel(
resources: Resources,
behavior: SongTapBehavior,
@@ -875,7 +873,7 @@ private fun SongTapBehaviorDialog(
text = {
ChoiceGroup(
header = stringResource(R.string.settings_music_options_song_tap_prompt),
options = songTapBehaviorOptions.map { it to songTapBehaviorLabel(resources, it) },
options = SongTapBehavior.entries.map { it to songTapBehaviorLabel(resources, it) },
selected = draft,
onSelect = { draft = it },
)
@@ -67,6 +67,26 @@ class UserPreferencesTest {
file.delete()
}
@Test
fun songTapBehaviorDefaultsToInOrderAndRoundTrips() =
runTest {
val file = File.createTempFile("user-prefs", ".preferences_pb").apply { delete() }
val store = PreferenceDataStoreFactory.create(scope = backgroundScope) { file }
val prefs = UserPreferences(store)
assertEquals(SongTapBehavior.InOrder, prefs.songTapBehavior().first())
prefs.setSongTapBehavior(SongTapBehavior.Shuffle)
assertEquals(SongTapBehavior.Shuffle, prefs.songTapBehavior().first())
store.edit { it[stringPreferencesKey("song_tap_behavior")] = "Bogus" }
assertEquals(SongTapBehavior.InOrder, prefs.songTapBehavior().first())
file.delete()
}
@Test
fun theSelectedLibraryTabRoundTrips() =
runTest {
@@ -2,6 +2,9 @@ package com.subtracks.playback
import android.content.Context
import android.content.res.Resources
import androidx.datastore.core.DataStore
import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.emptyPreferences
import androidx.media3.common.PlaybackException
import androidx.room3.useWriterConnection
import androidx.test.core.app.ApplicationProvider
@@ -33,14 +36,17 @@ import com.subtracks.data.prefs.fakeUserPreferences
import com.subtracks.data.repo.DownloadRepository
import com.subtracks.data.repo.QueueRepository
import com.subtracks.data.repo.SourceRepository
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
@@ -2565,6 +2571,8 @@ class PlaybackControllerTest {
fun playAllStartsInOrderRegardlessOfTheSongTapBehavior() {
seedAlbum(3, sourceId = 1)
runBlocking { prefs.setSongTapBehavior(SongTapBehavior.Shuffle) }
controller.shuffleAlbum(1, "al1")
await { controller.state.value.shuffle }
controller.playAlbumInOrder(1, "al1")
await {
@@ -2575,6 +2583,36 @@ class PlaybackControllerTest {
assertEquals(0L, controller.state.value.position)
}
@Test
fun twoTapsKeepTheirOrderWhileThePreferenceReadIsSlow() {
seedAlbum(3, sourceId = 1)
val (slowPrefs, entered, gate) = gatedSongTapPreferences()
val slowController =
PlaybackController(
sources,
queues,
FakePlayerConnection(handle),
downloads,
slowPrefs,
scope = CoroutineScope(SupervisorJob() + dispatcher).also { extraScopes += it },
)
slowController.connect()
await { slowController.ready.value }
slowController.playAlbum(1, "al1", 0)
runBlocking { entered.await() }
slowController.playAlbum(1, "al1", 1)
gate.complete(Unit)
runBlocking { delay(1_000) }
assertEquals(
"s2",
slowController.state.value.item
?.id,
)
assertEquals(1L, slowController.state.value.position)
}
@Test
fun jumpingInTheQueueWhileShuffledKeepsTheOrder() {
seedAlbum(5, sourceId = 1)
@@ -3694,6 +3732,30 @@ class PlaybackControllerTest {
(0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id }
}
private fun gatedSongTapPreferences(): Triple<UserPreferences, CompletableDeferred<Unit>, CompletableDeferred<Unit>> {
val entered = CompletableDeferred<Unit>()
val gate = CompletableDeferred<Unit>()
var reads = 0
val prefs =
UserPreferences(
object : DataStore<Preferences> {
private val state = MutableStateFlow(emptyPreferences())
override val data: Flow<Preferences> =
state.onStart {
if (reads++ == 0) {
entered.complete(Unit)
gate.await()
}
}
override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences =
transform(state.value).also { state.value = it }
},
)
return Triple(prefs, entered, gate)
}
private fun controllerOn(
dispatcher: CoroutineDispatcher,
handle: FakePlayerHandle,