Enforce that STLState is mutated on the right thread
SceneTransitionLayoutState currently does not do any synchronization
to support mutation from different threads. Usually this is not an issue
because most state changes naturally happen on the main thread, but I
noticed that it was causing some flakiness issues in tests given that
those run in a thread different than the main thread.
This CL enforces that all mutations of a STLState are done in the same
thread in which the state was created. If that STLState is used by a
SceneTransitionLayout, we also check that the thread is the same (which
is usually the main thread).
Bug: 290930950
Test: atest PlatformComposeSceneTransitionLayoutTests
Flag: com.android.systemui.scene_container
Change-Id: Ia928cc43305f4c226358dabd74fafee9d340fcc5
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
index d383cec..7856498 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
@@ -126,6 +126,10 @@
orientation = Orientation.Vertical,
coroutineScope = coroutineScope,
)
+
+ // Make sure that the state is created on the same thread (most probably the main thread)
+ // than this STLImpl.
+ state.checkThread()
}
internal fun draggableHandler(orientation: Orientation): DraggableHandlerImpl =
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
index cb4d582..e0ec33c 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
@@ -376,6 +376,8 @@
// TODO(b/290930950): Remove this flag.
internal var enableInterruptions: Boolean,
) : SceneTransitionLayoutState {
+ private val creationThread: Thread = Thread.currentThread()
+
/**
* The current [TransitionState]. This list will either be:
* 1. A list with a single [TransitionState.Idle] element, when we are idle.
@@ -420,6 +422,20 @@
*/
internal abstract fun CoroutineScope.onChangeScene(scene: SceneKey)
+ internal fun checkThread() {
+ val current = Thread.currentThread()
+ if (current !== creationThread) {
+ error(
+ """
+ Only the original thread that created a SceneTransitionLayoutState can mutate it
+ Expected: ${creationThread.name}
+ Current: ${current.name}
+ """
+ .trimIndent()
+ )
+ }
+ }
+
override fun isTransitioning(from: SceneKey?, to: SceneKey?): Boolean {
val transition = currentTransition ?: return false
return transition.isTransitioning(from, to)
@@ -444,6 +460,8 @@
transitionKey: TransitionKey?,
chain: Boolean = true,
) {
+ checkThread()
+
// Compute the [TransformationSpec] when the transition starts.
val fromScene = transition.fromScene
val toScene = transition.toScene
@@ -564,6 +582,8 @@
* nothing if [transition] was interrupted since it was started.
*/
internal fun finishTransition(transition: TransitionState.Transition, idleScene: SceneKey) {
+ checkThread()
+
val existingIdleScene = finishedTransitions[transition]
if (existingIdleScene != null) {
// This transition was already finished.
@@ -617,6 +637,8 @@
}
fun snapToScene(scene: SceneKey) {
+ checkThread()
+
// Force finish all transitions.
while (currentTransitions.isNotEmpty()) {
val transition = transitionStates[0] as TransitionState.Transition
@@ -755,6 +777,8 @@
coroutineScope: CoroutineScope,
transitionKey: TransitionKey?,
): TransitionState.Transition? {
+ checkThread()
+
return coroutineScope.animateToScene(
layoutState = this@MutableSceneTransitionLayoutStateImpl,
target = targetScene,
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
index 2c4fa5d..0fe50fa 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
@@ -610,14 +610,16 @@
// TODO(b/341072461): Remove this test.
fun layoutGetsCurrentTransitionStateFromComposition() {
val state =
- MutableSceneTransitionLayoutStateImpl(
- SceneA,
- transitions {
- from(SceneA, to = SceneB) {
- scaleSize(TestElements.Foo, width = 2f, height = 2f)
+ rule.runOnUiThread {
+ MutableSceneTransitionLayoutStateImpl(
+ SceneA,
+ transitions {
+ from(SceneA, to = SceneB) {
+ scaleSize(TestElements.Foo, width = 2f, height = 2f)
+ }
}
- }
- )
+ )
+ }
rule.setContent {
SceneTransitionLayout(state) {
@@ -630,10 +632,12 @@
rule.mainClock.autoAdvance = false
// Change the current transition.
- state.startTransition(
- transition(from = SceneA, to = SceneB, progress = { 0.5f }),
- transitionKey = null,
- )
+ rule.runOnUiThread {
+ state.startTransition(
+ transition(from = SceneA, to = SceneB, progress = { 0.5f }),
+ transitionKey = null,
+ )
+ }
// The size of Foo should still be 20dp given that the new state was not composed yet.
rule.onNode(isElement(TestElements.Foo)).assertSizeIsEqualTo(20.dp, 20.dp)
@@ -652,11 +656,13 @@
var touchSlop = 0f
val state =
- MutableSceneTransitionLayoutState(
- initialScene = SceneA,
- transitions = transitions(sceneTransitions),
- )
- as MutableSceneTransitionLayoutStateImpl
+ rule.runOnUiThread {
+ MutableSceneTransitionLayoutState(
+ initialScene = SceneA,
+ transitions = transitions(sceneTransitions),
+ )
+ as MutableSceneTransitionLayoutStateImpl
+ }
rule.setContent {
touchSlop = LocalViewConfiguration.current.touchSlop
@@ -762,16 +768,18 @@
val layoutHeight = 400.dp
val state =
- MutableSceneTransitionLayoutState(
- initialScene = SceneB,
- transitions =
- transitions {
- overscroll(SceneB, Orientation.Vertical) {
- translate(TestElements.Foo, y = overscrollTranslateY)
+ rule.runOnUiThread {
+ MutableSceneTransitionLayoutState(
+ initialScene = SceneB,
+ transitions =
+ transitions {
+ overscroll(SceneB, Orientation.Vertical) {
+ translate(TestElements.Foo, y = overscrollTranslateY)
+ }
}
- }
- )
- as MutableSceneTransitionLayoutStateImpl
+ )
+ as MutableSceneTransitionLayoutStateImpl
+ }
rule.setContent {
touchSlop = LocalViewConfiguration.current.touchSlop
@@ -938,32 +946,36 @@
val duration = 4 * 16
val state =
- MutableSceneTransitionLayoutState(
- SceneA,
- transitions {
- // Foo is at the top left corner of scene A. We make it disappear during A => B
- // to the right edge so it translates to the right.
- from(SceneA, to = SceneB) {
- spec = tween(duration, easing = LinearEasing)
- translate(
- TestElements.Foo,
- edge = Edge.Right,
- startsOutsideLayoutBounds = false,
- )
- }
+ rule.runOnUiThread {
+ MutableSceneTransitionLayoutState(
+ SceneA,
+ transitions {
+ // Foo is at the top left corner of scene A. We make it disappear during A
+ // => B
+ // to the right edge so it translates to the right.
+ from(SceneA, to = SceneB) {
+ spec = tween(duration, easing = LinearEasing)
+ translate(
+ TestElements.Foo,
+ edge = Edge.Right,
+ startsOutsideLayoutBounds = false,
+ )
+ }
- // Bar is at the top right corner of scene C. We make it appear during B => C
- // from the left edge so it translates to the right at same time as Foo.
- from(SceneB, to = SceneC) {
- spec = tween(duration, easing = LinearEasing)
- translate(
- TestElements.Bar,
- edge = Edge.Left,
- startsOutsideLayoutBounds = false,
- )
+ // Bar is at the top right corner of scene C. We make it appear during B =>
+ // C
+ // from the left edge so it translates to the right at same time as Foo.
+ from(SceneB, to = SceneC) {
+ spec = tween(duration, easing = LinearEasing)
+ translate(
+ TestElements.Bar,
+ edge = Edge.Left,
+ startsOutsideLayoutBounds = false,
+ )
+ }
}
- }
- )
+ )
+ }
val layoutSize = 150.dp
val elemSize = 50.dp
@@ -1059,14 +1071,16 @@
val duration = 4 * 16
val state =
- MutableSceneTransitionLayoutStateImpl(
- SceneA,
- transitions {
- from(SceneA, to = SceneB) { spec = tween(duration, easing = LinearEasing) }
- from(SceneB, to = SceneC) { spec = tween(duration, easing = LinearEasing) }
- },
- enableInterruptions = false,
- )
+ rule.runOnUiThread {
+ MutableSceneTransitionLayoutStateImpl(
+ SceneA,
+ transitions {
+ from(SceneA, to = SceneB) { spec = tween(duration, easing = LinearEasing) }
+ from(SceneB, to = SceneC) { spec = tween(duration, easing = LinearEasing) }
+ },
+ enableInterruptions = false,
+ )
+ }
val layoutSize = DpSize(200.dp, 100.dp)
val fooSize = DpSize(20.dp, 10.dp)
@@ -1177,8 +1191,10 @@
.assertPositionInRootIsEqualTo(offsetInC.x, offsetInC.y)
// Manually finish the transition.
- state.finishTransition(aToB, SceneB)
- state.finishTransition(bToC, SceneC)
+ rule.runOnUiThread {
+ state.finishTransition(aToB, SceneB)
+ state.finishTransition(bToC, SceneC)
+ }
rule.waitForIdle()
assertThat(state.transitionState).isIdle()
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
index 1dd9322..3a806a4 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
@@ -70,7 +70,9 @@
private fun layoutState(
initialScene: SceneKey = SceneA,
transitions: SceneTransitions = EmptyTestTransitions,
- ) = MutableSceneTransitionLayoutState(initialScene, transitions)
+ ): MutableSceneTransitionLayoutState {
+ return rule.runOnUiThread { MutableSceneTransitionLayoutState(initialScene, transitions) }
+ }
/** The content under test. */
@Composable
@@ -455,7 +457,7 @@
@Test
fun swipeEnabledLater() {
- val layoutState = MutableSceneTransitionLayoutState(SceneA)
+ val layoutState = layoutState()
var swipesEnabled by mutableStateOf(false)
var touchSlop = 0f
rule.setContent {
@@ -489,7 +491,7 @@
fun transitionKey() {
val transitionkey = TransitionKey(debugName = "foo")
val state =
- MutableSceneTransitionLayoutStateImpl(
+ layoutState(
SceneA,
transitions {
from(SceneA, to = SceneB) { fade(TestElements.Foo) }
@@ -553,7 +555,7 @@
}
val state =
- MutableSceneTransitionLayoutState(
+ layoutState(
SceneA,
transitions { from(SceneA, to = SceneB) { distance = swipeDistance } }
)