Skip to content

Commit 49d929b

Browse files
oceanjulescopybara-github
authored andcommitted
[ui-compose] Eliminate race condition in PresentationState
PresentationState creation and listener registration are not an atomic operation. This happens because the LaunchedEffect which starts the listen-to-Player-Events coroutine can be pre-empted with other side effects, including the ones that change something in the Player. rememberPresentationState function creates a PresentationState object and initialises it with the correct fresh values pulled out of the Player. The subscription to Player events with a registration of a Listener (via PresentationState.observe()) is not immediate. It *returns* immediately, but is instead scheduled to happen later, although within the same Handler message. Other LaunchedEffects could have been scheduled earlier and could take place between the button state creation and listener subscription. This is not a problem if no changes to the player happen, but if we miss the relevant player events, we might end up with a UI that is out of sync with reality. The way to fix this is to pull the latest values out of the Player on demand upon starting to listen to Player events. PiperOrigin-RevId: 753129489
1 parent deb466c commit 49d929b

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

libraries/ui_compose/src/main/java/androidx/media3/ui/compose/state/PresentationState.kt

+4-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ class PresentationState(private val player: Player) {
8383
* * [Player.EVENT_RENDERED_FIRST_FRAME] and [Player.EVENT_TRACKS_CHANGED]to determine whether the
8484
* surface is ready to be shown
8585
*/
86-
suspend fun observe(): Nothing =
86+
suspend fun observe() {
87+
videoSizeDp = getVideoSizeDp(player)
88+
coverSurface = true
8789
player.listen { events ->
8890
if (events.contains(Player.EVENT_VIDEO_SIZE_CHANGED)) {
8991
if (videoSize != VideoSize.UNKNOWN && playbackState != Player.STATE_IDLE) {
@@ -98,6 +100,7 @@ class PresentationState(private val player: Player) {
98100
maybeHideSurface(player)
99101
}
100102
}
103+
}
101104

102105
@Nullable
103106
private fun getVideoSizeDp(player: Player): Size? {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2025 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package androidx.media3.ui.compose.state
18+
19+
import androidx.compose.runtime.LaunchedEffect
20+
import androidx.compose.ui.geometry.Size
21+
import androidx.compose.ui.test.junit4.createComposeRule
22+
import androidx.media3.common.Player
23+
import androidx.media3.common.VideoSize
24+
import androidx.media3.ui.compose.utils.TestPlayer
25+
import androidx.test.ext.junit.runners.AndroidJUnit4
26+
import com.google.common.truth.Truth.assertThat
27+
import org.junit.Rule
28+
import org.junit.Test
29+
import org.junit.runner.RunWith
30+
31+
/** Unit test for [PresentationState]. */
32+
@RunWith(AndroidJUnit4::class)
33+
class PresentationStateTest {
34+
35+
@get:Rule val composeTestRule = createComposeRule()
36+
37+
@Test
38+
fun playerInitialized_presentationStateInitialized() {
39+
val player = TestPlayer()
40+
player.playbackState = Player.STATE_IDLE
41+
42+
lateinit var state: PresentationState
43+
composeTestRule.setContent { state = rememberPresentationState(player) }
44+
45+
assertThat(state.coverSurface).isTrue()
46+
assertThat(state.keepContentOnReset).isFalse()
47+
assertThat(state.videoSizeDp).isEqualTo(null)
48+
}
49+
50+
@Test
51+
fun playerChangesVideoSizeBeforeEventListenerRegisters_observeGetsTheLatestValues_uiInSync() {
52+
val player = TestPlayer()
53+
player.playbackState = Player.STATE_IDLE
54+
55+
lateinit var state: PresentationState
56+
composeTestRule.setContent {
57+
// Schedule LaunchedEffect to update player state before PresentationState is created.
58+
// This update could end up being executed *before* PresentationState schedules the start
59+
// of event listening and we don't want to lose it.
60+
LaunchedEffect(player) { player.videoSize = VideoSize(480, 360) }
61+
state = rememberPresentationState(player)
62+
}
63+
64+
assertThat(state.videoSizeDp).isEqualTo(Size(480f, 360f))
65+
assertThat(state.coverSurface).isTrue()
66+
assertThat(state.keepContentOnReset).isFalse()
67+
}
68+
}

libraries/ui_compose/src/test/java/androidx/media3/ui/compose/utils/TestPlayer.kt

+6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import android.os.Looper
2020
import androidx.media3.common.PlaybackParameters
2121
import androidx.media3.common.Player
2222
import androidx.media3.common.SimpleBasePlayer
23+
import androidx.media3.common.VideoSize
2324
import com.google.common.collect.ImmutableList
2425
import com.google.common.util.concurrent.Futures
2526
import com.google.common.util.concurrent.ListenableFuture
@@ -124,6 +125,11 @@ internal class TestPlayer : SimpleBasePlayer(Looper.myLooper()!!) {
124125
invalidateState()
125126
}
126127

128+
fun setVideoSize(videoSize: VideoSize) {
129+
state = state.buildUpon().setVideoSize(videoSize).build()
130+
invalidateState()
131+
}
132+
127133
fun removeCommands(vararg commands: @Player.Command Int) {
128134
// It doesn't seem possible to propagate the @IntDef annotation through Kotlin's spread operator
129135
// in a way that lint understands.

0 commit comments

Comments
 (0)