Call onRelease callback in the same order as onForgotten
SubcomposeLayout depends on this order to dispose nested subcompositions when parent exits the composition. In particular, we want to make sure that child can unpin itself (and cleanup e.g. focus) before parent onForgotten is dispatched. Deactivations were already dispatched in this order to account for a similar issue.
Fixes: 322579799
Test: Added ordering test in SubcomposeLayoutTest and pinning test in LazyListPinnableContainerTest
(cherry picked from https://android-review.googlesource.com/q/commit:01c9d9953b00e5111ff8a2075208ff78601a572e)
(cherry picked from https://android-review.googlesource.com/q/commit:13a8f4633327392402d5282a254eabea94f8f526)
Merged-In: I0429d56ae6d02406fbc6f062d1c1ca040030ab09
Change-Id: I0429d56ae6d02406fbc6f062d1c1ca040030ab09
diff --git a/compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyListPinnableContainerTest.kt b/compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyListPinnableContainerTest.kt
index 82963da..30c436c 100644
--- a/compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyListPinnableContainerTest.kt
+++ b/compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyListPinnableContainerTest.kt
@@ -20,6 +20,7 @@
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyListState
+import androidx.compose.foundation.lazy.LazyRow
import androidx.compose.foundation.lazy.items
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
@@ -666,6 +667,42 @@
assertThat(parent2Pinned).isTrue()
}
}
+
+ @Test
+ fun pinnedItemIsRemovedAfterContainerExitsComposition() {
+ var active by mutableStateOf(true)
+ // Arrange.
+ rule.setContentParameterized {
+ if (active) {
+ LazyColumn(Modifier.size(itemSize * 2)) {
+ items(3) { colIndex ->
+ LazyRow {
+ items(3) { rowIndex ->
+ if (colIndex == 1 && rowIndex == 1) {
+ pinnableContainer = LocalPinnableContainer.current
+ }
+ Box(Modifier.size(itemSize).testTag("$colIndex:$rowIndex"))
+ }
+ }
+ }
+ }
+ }
+ }
+
+ rule.onNodeWithTag("1:1")
+ .assertIsPlaced()
+
+ rule.runOnIdle {
+ requireNotNull(pinnableContainer).pin()
+ }
+
+ rule.runOnIdle {
+ active = !active
+ }
+
+ rule.onNodeWithTag("1:1")
+ .assertIsNotPlaced()
+ }
}
/**
diff --git a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt b/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt
index 83eaa14..7810951 100644
--- a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt
+++ b/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt
@@ -17,6 +17,8 @@
@file:OptIn(InternalComposeApi::class)
package androidx.compose.runtime
+import androidx.collection.MutableScatterSet
+import androidx.collection.mutableScatterSetOf
import androidx.compose.runtime.changelist.ChangeList
import androidx.compose.runtime.collection.IdentityArrayMap
import androidx.compose.runtime.collection.IdentityArraySet
@@ -1236,7 +1238,7 @@
private val remembering = mutableListOf<RememberObserver>()
private val forgetting = mutableListOf<Any>()
private val sideEffects = mutableListOf<() -> Unit>()
- private var releasing: MutableList<ComposeNodeLifecycleCallback>? = null
+ private var releasing: MutableScatterSet<ComposeNodeLifecycleCallback>? = null
override fun remembering(instance: RememberObserver) {
remembering.add(instance)
@@ -1255,15 +1257,18 @@
}
override fun releasing(instance: ComposeNodeLifecycleCallback) {
- (releasing ?: mutableListOf<ComposeNodeLifecycleCallback>().also {
- releasing = it
- }) += instance
+ val releasing = releasing
+ ?: mutableScatterSetOf<ComposeNodeLifecycleCallback>().also { releasing = it }
+
+ releasing += instance
+ forgetting += instance
}
fun dispatchRememberObservers() {
// Send forgets and node deactivations
if (forgetting.isNotEmpty()) {
trace("Compose:onForgotten") {
+ val releasing = releasing
for (i in forgetting.size - 1 downTo 0) {
val instance = forgetting[i]
abandoning.remove(instance)
@@ -1271,7 +1276,12 @@
instance.onForgotten()
}
if (instance is ComposeNodeLifecycleCallback) {
- instance.onDeactivate()
+ // node callbacks are in the same queue as forgets to ensure ordering
+ if (releasing != null && instance in releasing) {
+ instance.onRelease()
+ } else {
+ instance.onDeactivate()
+ }
}
}
}
@@ -1286,17 +1296,6 @@
}
}
}
-
- // Send node releases
- val releasing = releasing
- if (!releasing.isNullOrEmpty()) {
- trace("Compose:releases") {
- for (i in releasing.size - 1 downTo 0) {
- val instance = releasing[i]
- instance.onRelease()
- }
- }
- }
}
fun dispatchSideEffects() {
diff --git a/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt b/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
index e3bd0f6..ef2de8e 100644
--- a/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
+++ b/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
@@ -52,6 +52,7 @@
import androidx.compose.ui.composed
import androidx.compose.ui.draw.assertColor
import androidx.compose.ui.draw.drawBehind
+import androidx.compose.ui.focus.isExactly
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.ImageBitmap
import androidx.compose.ui.graphics.asAndroidBitmap
@@ -2694,6 +2695,47 @@
}
}
+ @Test
+ fun nestedDisposeIsCalledInOrder() {
+ val disposeOrder = mutableListOf<String>()
+ var active by mutableStateOf(true)
+ rule.setContent {
+ if (active) {
+ BoxWithConstraints {
+ BoxWithConstraints {
+ DisposableEffect(Unit) {
+ onDispose {
+ disposeOrder += "inner 1"
+ }
+ }
+ }
+
+ DisposableEffect(Unit) {
+ onDispose {
+ disposeOrder += "outer"
+ }
+ }
+
+ BoxWithConstraints {
+ DisposableEffect(Unit) {
+ onDispose {
+ disposeOrder += "inner 2"
+ }
+ }
+ }
+ }
+ }
+ }
+
+ rule.runOnIdle {
+ active = false
+ }
+
+ rule.runOnIdle {
+ assertThat(disposeOrder).isExactly("inner 2", "outer", "inner 1")
+ }
+ }
+
private fun SubcomposeMeasureScope.measure(
slotId: Any,
constraints: Constraints,
