Synthesize depth-first removes for old hosts

This works around a since-fixed memory leak.

redwood-protocol-guest/api/android/redwood-protocol-guest.api

@@ -25,7 +25,7 @@ public final class app/cash/redwood/protocol/guest/ProtocolRedwoodCompositionKt
 
 public final class app/cash/redwood/protocol/guest/ProtocolState {
 	public static final field $stable I
-	public fun <init> ()V
+	public synthetic fun <init> (Ljava/lang/String;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun addWidget (Lapp/cash/redwood/protocol/guest/ProtocolWidget;)V
 	public final fun append (Lapp/cash/redwood/protocol/Change;)V
 	public final fun getChangesOrNull ()Ljava/util/List;
@@ -35,23 +35,23 @@ public final class app/cash/redwood/protocol/guest/ProtocolState {
 }
 
 public abstract interface class app/cash/redwood/protocol/guest/ProtocolWidget : app/cash/redwood/widget/Widget {
+	public abstract fun depthFirstWalk (Lkotlin/jvm/functions/Function3;)V
 	public abstract fun getId-0HhLjSo ()I
 	public abstract fun getTag-BlhN7y0 ()I
 	public synthetic fun getValue ()Ljava/lang/Object;
 	public fun getValue ()Lkotlin/Unit;
 	public abstract fun sendEvent (Lapp/cash/redwood/protocol/Event;)V
-	public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }
 
 public final class app/cash/redwood/protocol/guest/ProtocolWidgetChildren : app/cash/redwood/widget/Widget$Children {
 	public static final field $stable I
 	public synthetic fun <init> (IILapp/cash/redwood/protocol/guest/ProtocolState;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public final fun depthFirstWalk (Lapp/cash/redwood/protocol/guest/ProtocolWidget;Lkotlin/jvm/functions/Function3;)V
 	public fun getWidgets ()Ljava/util/List;
 	public fun insert (ILapp/cash/redwood/widget/Widget;)V
 	public fun move (III)V
 	public fun onModifierUpdated (ILapp/cash/redwood/widget/Widget;)V
 	public fun remove (II)V
-	public final fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }
 
 public final class app/cash/redwood/protocol/guest/VersionKt {

redwood-protocol-guest/api/jvm/redwood-protocol-guest.api

@@ -25,7 +25,7 @@ public final class app/cash/redwood/protocol/guest/ProtocolRedwoodCompositionKt
 
 public final class app/cash/redwood/protocol/guest/ProtocolState {
 	public static final field $stable I
-	public fun <init> ()V
+	public synthetic fun <init> (Ljava/lang/String;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun addWidget (Lapp/cash/redwood/protocol/guest/ProtocolWidget;)V
 	public final fun append (Lapp/cash/redwood/protocol/Change;)V
 	public final fun getChangesOrNull ()Ljava/util/List;
@@ -35,23 +35,23 @@ public final class app/cash/redwood/protocol/guest/ProtocolState {
 }
 
 public abstract interface class app/cash/redwood/protocol/guest/ProtocolWidget : app/cash/redwood/widget/Widget {
+	public abstract fun depthFirstWalk (Lkotlin/jvm/functions/Function3;)V
 	public abstract fun getId-0HhLjSo ()I
 	public abstract fun getTag-BlhN7y0 ()I
 	public synthetic fun getValue ()Ljava/lang/Object;
 	public fun getValue ()Lkotlin/Unit;
 	public abstract fun sendEvent (Lapp/cash/redwood/protocol/Event;)V
-	public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }
 
 public final class app/cash/redwood/protocol/guest/ProtocolWidgetChildren : app/cash/redwood/widget/Widget$Children {
 	public static final field $stable I
 	public synthetic fun <init> (IILapp/cash/redwood/protocol/guest/ProtocolState;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public final fun depthFirstWalk (Lapp/cash/redwood/protocol/guest/ProtocolWidget;Lkotlin/jvm/functions/Function3;)V
 	public fun getWidgets ()Ljava/util/List;
 	public fun insert (ILapp/cash/redwood/widget/Widget;)V
 	public fun move (III)V
 	public fun onModifierUpdated (ILapp/cash/redwood/widget/Widget;)V
 	public fun remove (II)V
-	public final fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }
 
 public final class app/cash/redwood/protocol/guest/VersionKt {

redwood-protocol-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/ProtocolState.kt

@@ -18,14 +18,24 @@ package app.cash.redwood.protocol.guest
 import app.cash.redwood.RedwoodCodegenApi
 import app.cash.redwood.protocol.Change
 import app.cash.redwood.protocol.Id
+import app.cash.redwood.protocol.RedwoodVersion
 
 /** @suppress For generated code use only. */
 @RedwoodCodegenApi
-public class ProtocolState {
+public class ProtocolState(
+  hostVersion: RedwoodVersion,
+) {
   private var nextValue = Id.Root.value + 1
   private val widgets = PlatformMap<Int, ProtocolWidget>()
   private var changes = PlatformList<Change>()
 
+  /**
+   * Host versions prior to 0.10.0 contained a bug where they did not recursively remove widgets
+   * from the protocol map which leaked any child views of a removed node. We can work around this
+   * on the guest side by synthesizing removes for every node in the subtree.
+   */
+  internal val synthesizeSubtreeRemoval = hostVersion < RedwoodVersion("0.10.0-SNAPSHOT")
+
   public fun nextId(): Id {
     val value = nextValue
     nextValue = value + 1

redwood-protocol-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/ProtocolWidget.kt

@@ -16,6 +16,7 @@
 package app.cash.redwood.protocol.guest
 
 import app.cash.redwood.RedwoodCodegenApi
+import app.cash.redwood.protocol.ChildrenTag
 import app.cash.redwood.protocol.Event
 import app.cash.redwood.protocol.Id
 import app.cash.redwood.protocol.WidgetTag
@@ -36,6 +37,37 @@ public interface ProtocolWidget : Widget<Unit> {
 
   public fun sendEvent(event: Event)
 
-  /** Recursively visit IDs in this widget's tree, starting with this widget's [id]. */
-  public fun visitIds(block: (Id) -> Unit)
+  /**
+   * Perform a depth-first walk of this widget's children hierarchy.
+   *
+   * For example, given the hierarchy:
+   * ```kotlin
+   * Split(
+   *   left = {
+   *     Row {
+   *       Text(..)
+   *       Button(..)
+   *     }
+   *   },
+   *   right = {
+   *     Column {
+   *       Button(..)
+   *       Text(..)
+   *     }
+   *   }
+   * }
+   * ```
+   * You will see the following argument values passed to [block] if invoked on the `Split`:
+   * 1. parent: `Row`, childrenTag: 1, children: `[Text+Button]`
+   * 2. parent: `Split`, childrenTag: 1, children: `[Row]`
+   * 3. parent: `Column`, childrenTag: 1, children: `[Button+Text]`
+   * 4. parent: `Split`, childrenTag: 2, children: `[Column]`
+   */
+  public fun depthFirstWalk(
+    block: (
+      parent: ProtocolWidget,
+      childrenTag: ChildrenTag,
+      children: ProtocolWidgetChildren,
+    ) -> Unit,
+  )
 }

redwood-protocol-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/ProtocolWidgetChildren.kt

@@ -39,15 +39,36 @@ public class ProtocolWidgetChildren(
   }
 
   override fun remove(index: Int, count: Int) {
-    val removedIds = ArrayList<Id>(count)
-    for (i in index until index + count) {
-      val widget = _widgets[i]
-      widget.visitIds(state::removeWidget)
-      removedIds += widget.id
+    if (state.synthesizeSubtreeRemoval) {
+      val removedIds = ArrayList<Id>(count)
+      for (i in index until index + count) {
+        val widget = _widgets[i]
+        removedIds += widget.id
+        state.removeWidget(widget.id)
+
+        widget.depthFirstWalk { parent, childrenTag, children ->
+          val childIds = children.widgets.map(ProtocolWidget::id)
+          for (childId in childIds) {
+            state.removeWidget(childId)
+          }
+          state.append(ChildrenChange.Remove(parent.id, childrenTag, 0, childIds.size, childIds))
+        }
+      }
+      state.append(ChildrenChange.Remove(id, tag, index, count, removedIds))
+    } else {
+      for (i in index until index + count) {
+        val widget = _widgets[i]
+        state.removeWidget(widget.id)
+        widget.depthFirstWalk { _, _, children ->
+          for (childWidget in children.widgets) {
+            state.removeWidget(childWidget.id)
+          }
+        }
+      }
+      state.append(ChildrenChange.Remove(id, tag, index, count))
     }
 
     _widgets.remove(index, count)
-    state.append(ChildrenChange.Remove(id, tag, index, count, removedIds))
   }
 
   override fun move(fromIndex: Int, toIndex: Int, count: Int) {
@@ -58,9 +79,13 @@ public class ProtocolWidgetChildren(
   override fun onModifierUpdated(index: Int, widget: Widget<Unit>) {
   }
 
-  public fun visitIds(block: (Id) -> Unit) {
-    for (i in _widgets.indices) {
-      _widgets[i].visitIds(block)
+  public fun depthFirstWalk(
+    parent: ProtocolWidget,
+    block: (ProtocolWidget, ChildrenTag, ProtocolWidgetChildren) -> Unit,
+  ) {
+    for (widget in widgets) {
+      widget.depthFirstWalk(block)
     }
+    block(parent, tag, this)
   }
 }

redwood-protocol-guest/src/commonTest/kotlin/app/cash/redwood/protocol/guest/ProtocolTest.kt

@@ -32,6 +32,7 @@ import app.cash.redwood.protocol.Id
 import app.cash.redwood.protocol.ModifierChange
 import app.cash.redwood.protocol.PropertyChange
 import app.cash.redwood.protocol.PropertyTag
+import app.cash.redwood.protocol.RedwoodVersion
 import app.cash.redwood.protocol.WidgetTag
 import app.cash.redwood.testing.TestRedwoodComposition
 import app.cash.redwood.ui.Cancellable
@@ -40,6 +41,7 @@ import app.cash.redwood.ui.OnBackPressedDispatcher
 import app.cash.redwood.ui.UiConfiguration
 import assertk.assertFailure
 import assertk.assertThat
+import assertk.assertions.containsExactly
 import assertk.assertions.isEqualTo
 import assertk.assertions.isInstanceOf
 import assertk.assertions.message
@@ -58,15 +60,13 @@ import kotlinx.coroutines.test.runTest
 import kotlinx.serialization.json.JsonPrimitive
 
 class ProtocolTest {
-  private val bridge = TestSchemaProtocolBridge.create(
-    // Use latest guest version as the host version to avoid any compatibility behavior.
-    hostVersion = guestRedwoodVersion,
-  )
+  // Use latest guest version as the host version to avoid any compatibility behavior.
+  private val latestVersion = guestRedwoodVersion
 
   @Test fun widgetVersionPropagated() = runTest {
     val composition = ProtocolRedwoodComposition(
       scope = this + BroadcastFrameClock(),
-      bridge = bridge,
+      bridge = TestSchemaProtocolBridge.create(latestVersion),
       changesSink = ::error,
       widgetVersion = 22U,
       onBackPressedDispatcher = object : OnBackPressedDispatcher {
@@ -90,7 +90,7 @@ class ProtocolTest {
   }
 
   @Test fun protocolChangeOrder() = runTest {
-    val composition = testProtocolComposition()
+    val (composition) = testProtocolComposition()
 
     composition.setContent {
       TestRow {
@@ -128,7 +128,7 @@ class ProtocolTest {
   }
 
   @Test fun protocolAlwaysSendsInitialLambdaPresence() = runTest {
-    val composition = testProtocolComposition()
+    val (composition) = testProtocolComposition()
     composition.setContent {
       Button("hi", onClick = null)
       Button("hi", onClick = {})
@@ -166,7 +166,7 @@ class ProtocolTest {
   }
 
   @Test fun protocolSkipsNullableLambdaChangeOfSamePresence() = runTest {
-    val composition = testProtocolComposition()
+    val (composition, bridge) = testProtocolComposition()
 
     var state by mutableStateOf(0)
     composition.setContent {
@@ -241,7 +241,7 @@ class ProtocolTest {
   }
 
   @Test fun protocolSkipsNonNullLambdaChange() = runTest {
-    val composition = testProtocolComposition()
+    val (composition, bridge) = testProtocolComposition()
 
     var state by mutableStateOf(0)
     composition.setContent {
@@ -285,8 +285,24 @@ class ProtocolTest {
     )
   }
 
-  @Test fun entireSubtreeRemoved() = runTest {
-    val composition = testProtocolComposition()
+  @Test fun entireSubtreeRemovedLatest() = runTest {
+    assertThat(removeSubtree(latestVersion))
+      .containsExactly(
+        ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, 1),
+      )
+  }
+
+  @Test fun entireSubtreeRemovedOldHostSynthesizesDepthFirstRemoval() = runTest {
+    assertThat(removeSubtree(RedwoodVersion("0.9.0")))
+      .containsExactly(
+        ChildrenChange.Remove(Id(2), ChildrenTag(1), 0, 1, listOf(Id(3))),
+        ChildrenChange.Remove(Id(1), ChildrenTag(1), 0, 1, listOf(Id(2))),
+        ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, 1, listOf(Id(1))),
+      )
+  }
+
+  private suspend fun TestScope.removeSubtree(hostVersion: RedwoodVersion): List<Change> {
+    val (composition, bridge) = testProtocolComposition(hostVersion)
 
     var clicks = 0
     var remove by mutableStateOf(false)
@@ -307,17 +323,22 @@ class ProtocolTest {
     assertThat(clicks).isEqualTo(1)
 
     remove = true
-    composition.awaitSnapshot()
+    val removeChanges = composition.awaitSnapshot()
 
     // If the whole tree was removed, we cannot target the button anymore.
     assertFailure { bridge.sendEvent(Event(Id(3), EventTag(2))) }
       .isInstanceOf<IllegalArgumentException>()
       .message()
       .isEqualTo("Unknown node ID 3 for event with tag 2")
     assertThat(clicks).isEqualTo(1)
+
+    return removeChanges
   }
 
-  private fun TestScope.testProtocolComposition(): TestRedwoodComposition<List<Change>> {
+  private fun TestScope.testProtocolComposition(
+    hostVersion: RedwoodVersion = latestVersion,
+  ): Pair<TestRedwoodComposition<List<Change>>, ProtocolBridge> {
+    val bridge = TestSchemaProtocolBridge.create(hostVersion)
     val composition = TestRedwoodComposition(
       scope = backgroundScope,
       widgetSystem = bridge.widgetSystem,
@@ -328,6 +349,6 @@ class ProtocolTest {
     backgroundScope.coroutineContext.job.invokeOnCompletion {
       composition.cancel()
     }
-    return composition
+    return composition to bridge
   }
 }

redwood-protocol/api/redwood-protocol.api

@@ -77,6 +77,7 @@ 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 <init> (IIIILjava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public synthetic fun <init> (IIIILjava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getCount ()I

redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt

@@ -16,6 +16,7 @@
 package app.cash.redwood.protocol
 
 import dev.drewhamilton.poko.Poko
+import kotlin.DeprecationLevel.ERROR
 import kotlinx.serialization.KSerializer
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
@@ -151,12 +152,8 @@ public sealed interface ChildrenChange : Change {
     override val tag: ChildrenTag,
     public val index: Int,
     public val count: Int,
-    public val removedIds: List<Id>,
-  ) : ChildrenChange {
-    init {
-      require(count == removedIds.size) {
-        "Count $count != Removed ID list size ${removedIds.size}"
-      }
-    }
-  }
+    // TODO Remove this for Redwood 1.0.0.
+    @Deprecated("Only sent for compatibility with old hosts. Do not consume.", level = ERROR)
+    public val removedIds: List<Id> = emptyList(),
+  ) : ChildrenChange
 }

redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/ProtocolTest.kt

@@ -57,6 +57,8 @@ class ProtocolTest {
       Create(Id(1), WidgetTag(2)),
       ChildrenChange.Add(Id(1), ChildrenTag(2), Id(3), 4),
       ChildrenChange.Move(Id(1), ChildrenTag(2), 3, 4, 5),
+      ChildrenChange.Remove(Id(4), ChildrenTag(3), 2, 1),
+      // We send a list of removed IDs only for old hosts.
       ChildrenChange.Remove(Id(1), ChildrenTag(2), 3, 4, listOf(Id(5), Id(6), Id(7), Id(8))),
       ModifierChange(
         Id(1),
@@ -90,6 +92,7 @@ class ProtocolTest {
       """["create",{"id":1,"tag":2}],""" +
       """["add",{"id":1,"tag":2,"childId":3,"index":4}],""" +
       """["move",{"id":1,"tag":2,"fromIndex":3,"toIndex":4,"count":5}],""" +
+      """["remove",{"id":4,"tag":3,"index":2,"count":1}],""" +
       """["remove",{"id":1,"tag":2,"index":3,"count":4,"removedIds":[5,6,7,8]}],""" +
       """["modifier",{"id":1,"elements":[[1,{}],[2,3],[3,[]],[4],[5]]}],""" +
       """["property",{"id":1,"tag":2,"value":"hello"}],""" +
@@ -98,13 +101,6 @@ class ProtocolTest {
     assertJsonRoundtrip(ListSerializer(Change.serializer()), changes, json)
   }
 
-  @Test fun removeCountMustMatchListSize() {
-    val t = assertFailsWith<IllegalArgumentException> {
-      ChildrenChange.Remove(Id(1), ChildrenTag(2), 3, 4, listOf(Id(5), Id(6), Id(7)))
-    }
-    assertThat(t).hasMessage("Count 4 != Removed ID list size 3")
-  }
-
   @Test fun modifierElementSerialization() {
     assertJsonRoundtrip(
       ModifierElement.serializer(),