diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef9ace396..c71e3eeea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Changed: Fixed: - Breaking `content: UIView` retain cycle in `UIViewLazyList`'s `LazyListContainerCell`. +- Update `ProtocolNode` widget IDs when recycling widgets. This was causing pooled nodes to be leaked. ## [0.13.0] - 2024-07-25 diff --git a/redwood-protocol-host/api/redwood-protocol-host.api b/redwood-protocol-host/api/redwood-protocol-host.api index dd61a2e8f5..85f67dc21f 100644 --- a/redwood-protocol-host/api/redwood-protocol-host.api +++ b/redwood-protocol-host/api/redwood-protocol-host.api @@ -41,6 +41,7 @@ public abstract class app/cash/redwood/protocol/host/ProtocolNode { public final fun getId-0HhLjSo ()I public abstract fun getWidget ()Lapp/cash/redwood/widget/Widget; public final fun getWidgetTag-BlhN7y0 ()I + public final fun setId-ou3jOuA (I)V public final fun updateModifier (Lapp/cash/redwood/Modifier;)V public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V } diff --git a/redwood-protocol-host/api/redwood-protocol-host.klib.api b/redwood-protocol-host/api/redwood-protocol-host.klib.api index 23ad9f283b..307284bb29 100644 --- a/redwood-protocol-host/api/redwood-protocol-host.klib.api +++ b/redwood-protocol-host/api/redwood-protocol-host.klib.api @@ -34,11 +34,13 @@ abstract class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolNode { // abstract val widget // app.cash.redwood.protocol.host/ProtocolNode.widget|{}widget[0] abstract fun (): app.cash.redwood.widget/Widget<#A> // app.cash.redwood.protocol.host/ProtocolNode.widget.|(){}[0] - final val id // app.cash.redwood.protocol.host/ProtocolNode.id|{}id[0] - final fun (): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.host/ProtocolNode.id.|(){}[0] final val widgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag|{}widgetTag[0] final fun (): app.cash.redwood.protocol/WidgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag.|(){}[0] + final var id // app.cash.redwood.protocol.host/ProtocolNode.id|{}id[0] + final fun (): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.host/ProtocolNode.id.|(){}[0] + final fun (app.cash.redwood.protocol/Id) // app.cash.redwood.protocol.host/ProtocolNode.id.|(app.cash.redwood.protocol.Id){}[0] + abstract fun apply(app.cash.redwood.protocol/PropertyChange, app.cash.redwood.protocol/EventSink) // app.cash.redwood.protocol.host/ProtocolNode.apply|apply(app.cash.redwood.protocol.PropertyChange;app.cash.redwood.protocol.EventSink){}[0] abstract fun children(app.cash.redwood.protocol/ChildrenTag): app.cash.redwood.protocol.host/ProtocolChildren<#A>? // app.cash.redwood.protocol.host/ProtocolNode.children|children(app.cash.redwood.protocol.ChildrenTag){}[0] abstract fun detach() // app.cash.redwood.protocol.host/ProtocolNode.detach|detach(){}[0] diff --git a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt index 50db1fc4b0..b8ce628d0d 100644 --- a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt +++ b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt @@ -353,6 +353,9 @@ public class HostProtocolAdapter( "Insert attempted to replace existing widget with ID $widgetId" } + val skippedCreate = changesAndNulls[changeIndexForCreate] as Create + pooled.id = skippedCreate.id + // Remove the corresponding changes that we avoided by node reuse. We don't clear the 'Add' // that adds the node to its new parent. changesAndNulls[changeIndexForCreate] = null diff --git a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt index 6451fc2c33..70124a4bc7 100644 --- a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt +++ b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt @@ -33,7 +33,8 @@ import kotlin.math.min */ @RedwoodCodegenApi public abstract class ProtocolNode( - public val id: Id, + /** Updated in place when a node is reused due to pooling. */ + public var id: Id, public val widgetTag: WidgetTag, ) { public abstract val widget: Widget diff --git a/redwood-testing/src/commonTest/kotlin/app/cash/redwood/testing/ViewRecyclingTest.kt b/redwood-testing/src/commonTest/kotlin/app/cash/redwood/testing/ViewRecyclingTest.kt index 74056e9fcc..0f47645fef 100644 --- a/redwood-testing/src/commonTest/kotlin/app/cash/redwood/testing/ViewRecyclingTest.kt +++ b/redwood-testing/src/commonTest/kotlin/app/cash/redwood/testing/ViewRecyclingTest.kt @@ -16,14 +16,19 @@ package app.cash.redwood.testing import app.cash.redwood.Modifier +import app.cash.redwood.RedwoodCodegenApi import app.cash.redwood.layout.compose.Box import app.cash.redwood.layout.compose.Column import app.cash.redwood.layout.testing.BoxValue import app.cash.redwood.layout.testing.ColumnValue import app.cash.redwood.layout.widget.Box +import app.cash.redwood.protocol.ChildrenTag +import app.cash.redwood.protocol.Id +import app.cash.redwood.protocol.host.HostProtocolAdapter import assertk.assertThat import assertk.assertions.containsExactly import assertk.assertions.hasSize +import assertk.assertions.isEmpty import assertk.assertions.isNotSameInstanceAs import assertk.assertions.isSameInstanceAs import com.example.redwood.testapp.compose.Button @@ -56,6 +61,7 @@ class ViewRecyclingTest { ) val snapshot1Box = widgets.single() as Box val snapshot1BoxText = snapshot1Box.children.widgets.single() + val snapshot1ProtocolNodeIds = hostAdapter.allProtocolNodeIds() // Update the content. The old widgets are pooled and new widgets are created. setContent { @@ -71,6 +77,7 @@ class ViewRecyclingTest { ) val snapshot2Box = widgets.single() as Box val snapshot2BoxText = snapshot2Box.children.widgets.single() + val snapshot2ProtocolNodeIds = hostAdapter.allProtocolNodeIds() assertThat(snapshot2Box).isNotSameInstanceAs(snapshot1Box) assertThat(snapshot2BoxText).isNotSameInstanceAs(snapshot1Box) @@ -88,8 +95,13 @@ class ViewRecyclingTest { ) val snapshot3Box = widgets.single() as Box val snapshot3BoxText = snapshot3Box.children.widgets.single() + val snapshot3ProtocolNodeIds = hostAdapter.allProtocolNodeIds() assertThat(snapshot3Box).isSameInstanceAs(snapshot1Box) assertThat(snapshot3BoxText).isSameInstanceAs(snapshot1BoxText) + + // Confirm that the protocol IDs are updated in place. + assertThat(snapshot1ProtocolNodeIds.intersect(snapshot2ProtocolNodeIds)).isEmpty() + assertThat(snapshot2ProtocolNodeIds.intersect(snapshot3ProtocolNodeIds)).isEmpty() } } @@ -698,4 +710,14 @@ class ViewRecyclingTest { // We should hit for every element in the pool (and 1 miss). assertThat(step1Texts.intersect(step3Texts.toSet())).hasSize(poolSize) } + + @OptIn(RedwoodCodegenApi::class) + @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE") // To test implementation details! + private fun HostProtocolAdapter.allProtocolNodeIds(): Set { + val result = mutableSetOf() + node(Id.Root).children(ChildrenTag.Root)!!.visitIds { + result += it + } + return result + } }