From fc616c818fc17d336869fa2e7fcff953a3c3cc95 Mon Sep 17 00:00:00 2001 From: Jake Wharton Date: Tue, 10 Dec 2024 23:51:27 -0500 Subject: [PATCH] Implement host-side protocol widget reuse (#2501) This does not yet support sending the detach flag from the guest. --- .../protocol/host/HostProtocolAdapter.kt | 11 +-- .../protocol/host/HostProtocolAdapterTest.kt | 71 ++++++++++++++++++- redwood-protocol/api/redwood-protocol.api | 6 +- .../api/redwood-protocol.klib.api | 4 +- .../app/cash/redwood/protocol/protocol.kt | 9 ++- .../protocol/SnapshotChangeListTest.kt | 2 +- 6 files changed, 92 insertions(+), 11 deletions(-) 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 0a0af0f43d..f14b7a05f8 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 @@ -36,7 +36,6 @@ import app.cash.redwood.protocol.RedwoodVersion import app.cash.redwood.protocol.WidgetTag import app.cash.redwood.widget.ChangeListener import app.cash.redwood.widget.Widget -import kotlin.collections.mutableMapOf import kotlin.native.ObjCName /** @@ -107,10 +106,12 @@ public class HostProtocolAdapter( } is Remove -> { - for (childIndex in change.index until change.index + change.count) { - val child = children.nodes[childIndex] - child.visitIds(removeNodeById) - poolOrDetach(child) + if (!change.detach) { + for (childIndex in change.index until change.index + change.count) { + val child = children.nodes[childIndex] + child.visitIds(removeNodeById) + poolOrDetach(child) + } } children.remove(change.index, change.count) } diff --git a/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt b/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt index ab0d7e6f29..4148476f39 100644 --- a/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt +++ b/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt @@ -29,6 +29,7 @@ import app.cash.redwood.protocol.PropertyChange import app.cash.redwood.protocol.PropertyTag import app.cash.redwood.protocol.WidgetTag import app.cash.redwood.protocol.guest.guestRedwoodVersion +import app.cash.redwood.testing.WidgetValue import app.cash.redwood.widget.MutableListChildren import assertk.assertFailure import assertk.assertThat @@ -37,7 +38,9 @@ import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf import assertk.assertions.message import com.example.redwood.testapp.protocol.host.TestSchemaProtocolFactory +import com.example.redwood.testapp.testing.TestRowValue import com.example.redwood.testapp.testing.TestSchemaTestingWidgetFactory +import com.example.redwood.testapp.testing.TextValue import com.example.redwood.testapp.widget.TestSchemaWidgetSystem import kotlin.test.Test import kotlin.test.assertFailsWith @@ -204,9 +207,10 @@ class HostProtocolAdapterTest { } @Test fun entireSubtreeRemoved() { + val container = MutableListChildren() val host = HostProtocolAdapter( guestVersion = guestRedwoodVersion, - container = MutableListChildren(), + container = container, factory = TestSchemaProtocolFactory( widgetSystem = TestSchemaWidgetSystem( TestSchema = TestSchemaTestingWidgetFactory(), @@ -263,4 +267,69 @@ class HostProtocolAdapterTest { .message() .isEqualTo("Unknown widget ID 3") } + + @Test fun detachAndAdd() { + val container = MutableListChildren() + val host = HostProtocolAdapter( + guestVersion = guestRedwoodVersion, + container = container, + factory = TestSchemaProtocolFactory( + widgetSystem = TestSchemaWidgetSystem( + TestSchema = TestSchemaTestingWidgetFactory(), + RedwoodLayout = RedwoodLayoutTestingWidgetFactory(), + RedwoodLazyLayout = RedwoodLazyLayoutTestingWidgetFactory(), + ), + ), + eventSink = ::error, + leakDetector = LeakDetector.none(), + ) + + // TestRow { + // TestRow { + // Text("hello") + host.sendChanges( + listOf( + // TestRow + Create(Id(1), WidgetTag(1)), + ModifierChange(Id(1)), + // TestRow + Create(Id(2), WidgetTag(1)), + ModifierChange(Id(2)), + // Text + Create(Id(3), WidgetTag(3)), + PropertyChange(Id(3), WidgetTag(3), PropertyTag(1), JsonPrimitive("hello")), + ModifierChange(Id(3)), + Add(Id(2), ChildrenTag(1), Id(3), 0), + Add(Id(1), ChildrenTag(1), Id(2), 0), + Add(Id.Root, ChildrenTag.Root, Id(1), 0), + ), + ) + + host.sendChanges( + listOf( + // Detach inner TestRow. + Remove(Id(1), ChildrenTag(1), 0, 1, detach = true), + // Remove outer TestRow. + Remove(Id.Root, ChildrenTag.Root, 0, 1), + // New outer TestRow. + Create(Id(4), WidgetTag(1)), + ModifierChange(Id(4)), + Add(Id.Root, ChildrenTag.Root, Id(4), 0), + // Re-attach inner TestRow. + Add(Id(4), ChildrenTag(1), Id(2), 0), + ), + ) + + assertThat(container.single().value).isEqualTo( + TestRowValue( + children = listOf( + TestRowValue( + children = listOf( + TextValue(text = "hello"), + ), + ), + ), + ), + ) + } } diff --git a/redwood-protocol/api/redwood-protocol.api b/redwood-protocol/api/redwood-protocol.api index 55ce9368f6..24281a9bd2 100644 --- a/redwood-protocol/api/redwood-protocol.api +++ b/redwood-protocol/api/redwood-protocol.api @@ -79,9 +79,10 @@ public final class app/cash/redwood/protocol/ChildrenChange$Move$Companion { public final class app/cash/redwood/protocol/ChildrenChange$Remove : app/cash/redwood/protocol/ChildrenChange { public static final field Companion Lapp/cash/redwood/protocol/ChildrenChange$Remove$Companion; - public synthetic fun (IIIILkotlin/jvm/internal/DefaultConstructorMarker;)V + public synthetic fun (IIIIZLkotlin/jvm/internal/DefaultConstructorMarker;)V public fun equals (Ljava/lang/Object;)Z public final fun getCount ()I + public final fun getDetach ()Z public fun getId-0HhLjSo ()I public final fun getIndex ()I public fun getTag-b0W0yNk ()I @@ -101,7 +102,8 @@ public synthetic class app/cash/redwood/protocol/ChildrenChange$Remove$$serializ } public final class app/cash/redwood/protocol/ChildrenChange$Remove$Companion { - public final fun invoke-ARs5Qwk (IIII)Lapp/cash/redwood/protocol/ChildrenChange$Remove; + public final fun invoke-HpxY78w (IIIIZ)Lapp/cash/redwood/protocol/ChildrenChange$Remove; + public static synthetic fun invoke-HpxY78w$default (Lapp/cash/redwood/protocol/ChildrenChange$Remove$Companion;IIIIZILjava/lang/Object;)Lapp/cash/redwood/protocol/ChildrenChange$Remove; public final fun serializer ()Lkotlinx/serialization/KSerializer; } diff --git a/redwood-protocol/api/redwood-protocol.klib.api b/redwood-protocol/api/redwood-protocol.klib.api index 699bc7f0c7..561a5c36a6 100644 --- a/redwood-protocol/api/redwood-protocol.klib.api +++ b/redwood-protocol/api/redwood-protocol.klib.api @@ -91,6 +91,8 @@ sealed interface app.cash.redwood.protocol/ChildrenChange : app.cash.redwood.pro final class Remove : app.cash.redwood.protocol/ChildrenChange { // app.cash.redwood.protocol/ChildrenChange.Remove|null[0] final val count // app.cash.redwood.protocol/ChildrenChange.Remove.count|{}count[0] final fun (): kotlin/Int // app.cash.redwood.protocol/ChildrenChange.Remove.count.|(){}[0] + final val detach // app.cash.redwood.protocol/ChildrenChange.Remove.detach|{}detach[0] + final fun (): kotlin/Boolean // app.cash.redwood.protocol/ChildrenChange.Remove.detach.|(){}[0] final val id // app.cash.redwood.protocol/ChildrenChange.Remove.id|{}id[0] final fun (): app.cash.redwood.protocol/Id // app.cash.redwood.protocol/ChildrenChange.Remove.id.|(){}[0] final val index // app.cash.redwood.protocol/ChildrenChange.Remove.index|{}index[0] @@ -112,7 +114,7 @@ sealed interface app.cash.redwood.protocol/ChildrenChange : app.cash.redwood.pro } final object Companion { // app.cash.redwood.protocol/ChildrenChange.Remove.Companion|null[0] - final fun invoke(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, kotlin/Int): app.cash.redwood.protocol/ChildrenChange.Remove // app.cash.redwood.protocol/ChildrenChange.Remove.Companion.invoke|invoke(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;kotlin.Int){}[0] + final fun invoke(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, kotlin/Int, kotlin/Boolean = ...): app.cash.redwood.protocol/ChildrenChange.Remove // app.cash.redwood.protocol/ChildrenChange.Remove.Companion.invoke|invoke(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;kotlin.Int;kotlin.Boolean){}[0] final fun serializer(): kotlinx.serialization/KSerializer // app.cash.redwood.protocol/ChildrenChange.Remove.Companion.serializer|serializer(){}[0] } } diff --git a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt index 4a64fc113a..5e646d5c8c 100644 --- a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt +++ b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt @@ -248,6 +248,12 @@ public sealed interface ChildrenChange : Change { private val _tag: Int, public val index: Int, public val count: Int, + /** + * When true, the associated nodes should only be detached from the tree with the expectation + * that a future [Add] will re-attach them. Otherwise, nodes should be detached and fully + * removed for garbage collection. + */ + public val detach: Boolean = false, ) : ChildrenChange { override val id: Id get() = Id(_id) override val tag: ChildrenTag get() = ChildrenTag(_tag) @@ -258,7 +264,8 @@ public sealed interface ChildrenChange : Change { tag: ChildrenTag, index: Int, count: Int, - ): Remove = Remove(id.value, tag.value, index, count) + detach: Boolean = false, + ): Remove = Remove(id.value, tag.value, index, count, detach) } } } diff --git a/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/SnapshotChangeListTest.kt b/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/SnapshotChangeListTest.kt index 02761c3c2b..57f3dd44e8 100644 --- a/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/SnapshotChangeListTest.kt +++ b/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/SnapshotChangeListTest.kt @@ -71,7 +71,7 @@ class SnapshotChangeListTest { | |Found: | - Move(_id=0, _tag=1, fromIndex=1, toIndex=2, count=3) - | - Remove(_id=0, _tag=1, index=1, count=2) + | - Remove(_id=0, _tag=1, index=1, count=2, detach=false) """.trimMargin(), ) }