Fix `endToMarker()` when ending node groups.
`endToMarker()` is called when a composable function calls
an inline function that has a non-local return. If the non-local
return would return passed the `endNode()` call of an inline
composable function `endToMarker()` was supposed to call `end()`
of the composer with `true` but it was unconditionally calling
it with `false` causing the runtime to get confused as to when
to generate a node.
Also if the `currentMarker()` was called when recomposing existing
content but the `endToMaker()` was called with new content the
groups in the new content would not correctly be closed if the
current marker is in the direct parent group of the new content.
Fixes: 264467571
Fixes: 274889428
Test: ./gradlew :compose:r:r:tDUT
Change-Id: Ibe7063cf22f4bff6eda5bde05c37c1e665c09167
diff --git a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt b/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
index 3123770..6878dd2 100644
--- a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
+++ b/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
@@ -1673,10 +1673,31 @@
override fun endToMarker(marker: Int) {
if (marker < 0) {
+ // If the marker is negative then the marker is for the writer
val writerLocation = -marker
- while (writer.parent > writerLocation) end(false)
+ val writer = writer
+ while (true) {
+ val parent = writer.parent
+ if (parent <= writerLocation) break
+ end(writer.isNode(parent))
+ }
} else {
- while (reader.parent > marker) end(false)
+ // If the marker is positive then the marker is for the reader. However, if we are
+ // inserting then we need to close the inserting groups first.
+ if (inserting) {
+ // We might be inserting, we need to close all the groups until we are no longer
+ // inserting.
+ val writer = writer
+ while (inserting) {
+ end(writer.isNode(writer.parent))
+ }
+ }
+ val reader = reader
+ while (true) {
+ val parent = reader.parent
+ if (parent <= marker) break
+ end(reader.isNode(parent))
+ }
}
}
diff --git a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/CompositionTests.kt b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/CompositionTests.kt
index 2916991..bcf5e0f 100644
--- a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/CompositionTests.kt
+++ b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/CompositionTests.kt
@@ -21,6 +21,7 @@
import androidx.compose.runtime.mock.ContactModel
import androidx.compose.runtime.mock.Edit
import androidx.compose.runtime.mock.EmptyApplier
+import androidx.compose.runtime.mock.InlineLinear
import androidx.compose.runtime.mock.Linear
import androidx.compose.runtime.mock.MockViewValidator
import androidx.compose.runtime.mock.Point
@@ -3390,6 +3391,199 @@
revalidate()
}
+ @Test // regression test for 264467571
+ fun test_returnConditionally_fromNodeLambda_local_initial_return() = compositionTest {
+ var condition by mutableStateOf(true)
+ compose {
+ Text("Before outer")
+ InlineLinear {
+ Text("Before inner")
+ InlineLinear inner@{
+ Text("Before return")
+ if (condition) return@inner
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ validate {
+ Text("Before outer")
+ InlineLinear {
+ Text("Before inner")
+ InlineLinear inner@{
+ Text("Before return")
+ if (condition) return@inner
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ repeat(4) {
+ condition = !condition
+ expectChanges()
+ revalidate()
+ }
+ }
+
+ @Test // regression test for 264467571
+ fun test_returnConditionally_fromNodeLambda_local_initial_no_return() = compositionTest {
+ var condition by mutableStateOf(true)
+ compose {
+ Text("Before outer")
+ InlineLinear {
+ Text("Before inner")
+ InlineLinear inner@{
+ Text("Before return")
+ if (condition) return@inner
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ validate {
+ Text("Before outer")
+ InlineLinear {
+ Text("Before inner")
+ InlineLinear inner@{
+ Text("Before return")
+ if (condition) return@inner
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ repeat(4) {
+ condition = !condition
+ expectChanges()
+ revalidate()
+ }
+ }
+
+ @Test // regression test for 264467571
+ fun test_returnConditionally_fromNodeLambda_nonLocal_initial_return() = compositionTest {
+ var condition by mutableStateOf(true)
+ compose {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ InlineLinear {
+ Text("Before return")
+ if (condition) return@outer
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ validate {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ InlineLinear {
+ Text("Before return")
+ if (condition) return@outer
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ repeat(4) {
+ condition = !condition
+ expectChanges()
+ revalidate()
+ }
+ }
+
+ @Test // regression test for 264467571
+ fun test_returnConditionally_fromNodeLambda_nonLocal_initial_no_return() = compositionTest {
+ var condition by mutableStateOf(true)
+ compose {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ InlineLinear {
+ Text("Before return")
+ if (condition) return@outer
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ validate {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ InlineLinear {
+ Text("Before return")
+ if (condition) return@outer
+ Text("After return")
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ repeat(4) {
+ condition = !condition
+ expectChanges()
+ revalidate()
+ }
+ }
+
+ @Test
+ fun test_returnConditionally_fromConditionalNodeLambda_nonLocal_initial_no_return() =
+ compositionTest {
+ var condition by mutableStateOf(true)
+ compose {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ if (condition) {
+ InlineLinear {
+ Text("Before return")
+ return@outer
+ }
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ validate {
+ Text("Before outer")
+ InlineLinear outer@{
+ Text("Before inner")
+ if (condition) {
+ InlineLinear {
+ Text("Before return")
+ return@outer
+ }
+ }
+ Text("After inner")
+ }
+ Text("Before outer")
+ }
+
+ repeat(4) {
+ condition = !condition
+ expectChanges()
+ revalidate()
+ }
+ }
+
@Test
fun test_returnConditionally_fromFunction_nonLocal() = compositionTest {
val text = mutableStateOf<String?>(null)
@@ -3411,6 +3605,45 @@
revalidate()
}
+ @Test // regression test for 274889428
+ fun test_returnConditionally_simulatedIf() = compositionTest {
+ val condition1 = mutableStateOf(true)
+ val condition2 = mutableStateOf(true)
+ val condition3 = mutableStateOf(true)
+
+ compose block@{
+ Text("A")
+ simulatedIf(condition1.value) { return@block }
+ Text("B")
+ simulatedIf(condition2.value) { return@block }
+ Text("C")
+ simulatedIf(condition3.value) { return@block }
+ Text("D")
+ }
+
+ validate block@{
+ Text("A")
+ this.simulatedIf(condition1.value) { return@block }
+ Text("B")
+ this.simulatedIf(condition2.value) { return@block }
+ Text("C")
+ this.simulatedIf(condition3.value) { return@block }
+ Text("D")
+ }
+
+ condition1.value = false
+ expectChanges()
+ revalidate()
+
+ condition2.value = false
+ expectChanges()
+ revalidate()
+
+ condition3.value = false
+ expectChanges()
+ revalidate()
+ }
+
@Test // regression test for 267586102
fun test_remember_in_a_loop() = compositionTest {
var i1 = 0
@@ -3694,4 +3927,13 @@
if (text == null) return
Text(text)
}
+}
+
+@Composable
+private inline fun simulatedIf(condition: Boolean, block: () -> Unit) {
+ if (condition) block()
+}
+
+private inline fun MockViewValidator.simulatedIf(condition: Boolean, block: () -> Unit) {
+ if (condition) block()
}
\ No newline at end of file
diff --git a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/MockViewValidator.kt b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/MockViewValidator.kt
index 20ae087..cfa0614 100644
--- a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/MockViewValidator.kt
+++ b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/MockViewValidator.kt
@@ -47,6 +47,12 @@
assertEquals(0, views.size, "Not expecting children but some found")
}
}
+
+ inline fun inlineValidate(block: MockViewListValidator.() -> Unit) {
+ this.block()
+ val hasNext = next()
+ assertEquals(false, hasNext, "Expected children but none found")
+ }
}
fun MockViewValidator.view(name: String, block: (MockViewValidator.() -> Unit)? = null) {
@@ -56,6 +62,13 @@
MockViewListValidator(view.children).validate(block)
}
+inline fun MockViewValidator.inlineView(name: String, block: MockViewValidator.() -> Unit) {
+ val hasNext = next()
+ assertTrue(hasNext, "Expected a $name, but none found")
+ assertEquals(name, view.name)
+ MockViewListValidator(view.children).inlineValidate(block)
+}
+
fun <T> MockViewValidator.Repeated(of: Iterable<T>, block: MockViewValidator.(value: T) -> Unit) {
for (value in of) {
block(value)
@@ -64,6 +77,8 @@
fun MockViewValidator.Linear() = view("linear", null)
fun MockViewValidator.Linear(block: MockViewValidator.() -> Unit) = view("linear", block)
+inline fun MockViewValidator.InlineLinear(block: MockViewValidator.() -> Unit) =
+ inlineView("linear", block)
fun MockViewValidator.box(block: MockViewValidator.() -> Unit) = view("box", block)
fun MockViewValidator.Text(value: String) {
view("text")
diff --git a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/Views.kt b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/Views.kt
index 1c332b0..a5617c9 100644
--- a/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/Views.kt
+++ b/compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/mock/Views.kt
@@ -48,6 +48,16 @@
}
@Composable
+inline fun InlineLinear(content: @Composable () -> Unit) {
+ ReusableComposeNode<View, ViewApplier>(
+ factory = { View().also { it.name = "linear" } },
+ update = { }
+ ) {
+ content()
+ }
+}
+
+@Composable
fun Linear(
onReuse: () -> Unit = {},
onDeactivate: () -> Unit = {},
