Dispose of the composition when we're done with it. (#2494)

* Dispose of the composition when we're done with it.

This adds a test for DisposableEffect callbacks.

#2248

* Fixup LeaksTest

* Spotless

* Track HostApi change

* Track HostApi interface change

---------

Co-authored-by: Jesse Wilson <[email protected]>

redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/PlatformSet.kt

@@ -24,3 +24,5 @@ internal expect inline operator fun <T> PlatformSet<T>.plusAssign(element: T)
 internal expect inline fun <T> PlatformSet<T>.forEach(crossinline block: (T) -> Unit)
 
 internal expect inline fun <T> PlatformSet<T>.clear()
+
+internal expect val <T> PlatformSet<T>.size: Int

redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/RedwoodComposition.kt

@@ -173,6 +173,7 @@ private class WidgetRedwoodComposition<W : Any>(
   }
 
   override fun cancel() {
+    composition.dispose()
     snapshotHandle.dispose()
     snapshotJob?.cancel()
     recomposeJob.cancel()

redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/WidgetApplier.kt

@@ -74,7 +74,8 @@ internal class NodeApplier<W : Any>(
   }
 
   override fun onEndChanges() {
-    check(!closed)
+    // When the composition is disposed, onEndChanges() is called after onClear().
+    check(!closed || changedWidgets.size == 0)
 
     changedWidgets.let { changedWidgets ->
       changedWidgets.forEach { changedWidget ->

redwood-compose/src/jsMain/kotlin/app/cash/redwood/compose/PlatformSet.kt

@@ -19,6 +19,7 @@ package app.cash.redwood.compose
 
 @JsName("Set")
 internal external class JsSet<T> {
+  val size: Int
   fun add(element: T)
   fun clear()
   fun forEach(block: (T) -> Unit)
@@ -41,3 +42,6 @@ internal actual inline fun <T> PlatformSet<T>.forEach(crossinline block: (T) ->
 internal actual inline fun <T> PlatformSet<T>.clear() {
   clear()
 }
+
+internal actual val <T> PlatformSet<T>.size: Int
+  get() = size

redwood-compose/src/nonJsMain/kotlin/app/cash/redwood/compose/PlatformSet.kt

@@ -37,3 +37,6 @@ internal actual inline fun <T> PlatformSet<T>.forEach(block: (T) -> Unit) {
 internal actual inline fun <T> PlatformSet<T>.clear() {
   clear()
 }
+
+internal actual val <T> PlatformSet<T>.size: Int
+  get() = size

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/FakeHostApi.kt

@@ -16,9 +16,20 @@
 package app.cash.redwood.treehouse
 
 import com.example.redwood.testapp.treehouse.HostApi
+import kotlinx.coroutines.channels.Channel
 
 class FakeHostApi : HostApi {
+  private val messagesChannel = Channel<String>(capacity = Int.MAX_VALUE)
+
   override suspend fun httpCall(url: String, headers: Map<String, String>): String {
     error("unexpected call")
   }
+
+  override fun log(message: String) {
+    messagesChannel.trySend(message)
+  }
+
+  suspend fun takeMessage(): String {
+    return messagesChannel.receive()
+  }
 }

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/GuestLifecycleTest.kt

@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2024 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood.treehouse
+
+import assertk.assertThat
+import assertk.assertions.isEmpty
+import assertk.assertions.isEqualTo
+import com.example.redwood.testapp.testing.ButtonValue
+import com.example.redwood.testapp.testing.TextInputValue
+import kotlin.test.Test
+import kotlinx.coroutines.test.runTest
+
+class GuestLifecycleTest {
+  @Test
+  fun disposableEffectDisposedWhenRemovedFromComposition() = runTest {
+    val tester = TreehouseTester(this)
+    val treehouseApp = tester.loadApp()
+    val content = tester.content(treehouseApp)
+    val view = tester.view()
+
+    content.bind(view)
+
+    content.awaitContent(1)
+    val textInputValue = view.views.single() as TextInputValue
+    assertThat(textInputValue.text).isEqualTo("what would you like to see?")
+    textInputValue.onChange!!.invoke("GuestLifecycleTestShowDisposable")
+
+    tester.sendFrame()
+    assertThat(tester.hostApi.takeMessage()).isEqualTo("DisposableEffect.effect()")
+    content.awaitContent(2)
+    val nextButton = view.views.single() as ButtonValue
+    assertThat(nextButton.text).isEqualTo("Next")
+    nextButton.onClick!!.invoke()
+
+    tester.sendFrame()
+    assertThat(tester.hostApi.takeMessage()).isEqualTo("DisposableEffect.dispose()")
+    content.awaitContent(3)
+    assertThat(view.views).isEmpty()
+
+    treehouseApp.stop()
+    treehouseApp.close()
+  }
+
+  @Test
+  fun disposableEffectDisposedWhenContentUnbound() = runTest {
+    val tester = TreehouseTester(this)
+    val treehouseApp = tester.loadApp()
+    val content = tester.content(treehouseApp)
+    val view = tester.view()
+
+    content.bind(view)
+
+    content.awaitContent(1)
+    val textInputValue = view.views.single() as TextInputValue
+    assertThat(textInputValue.text).isEqualTo("what would you like to see?")
+    textInputValue.onChange!!.invoke("GuestLifecycleTestShowDisposable")
+
+    tester.sendFrame()
+    assertThat(tester.hostApi.takeMessage()).isEqualTo("DisposableEffect.effect()")
+
+    content.unbind()
+    assertThat(tester.hostApi.takeMessage()).isEqualTo("DisposableEffect.dispose()")
+
+    treehouseApp.stop()
+    treehouseApp.close()
+  }
+}

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/TreehouseTester.kt

@@ -44,7 +44,7 @@ internal class TreehouseTester(
 ) {
   val eventLog = EventLog()
 
-  var hostApi: HostApi = FakeHostApi()
+  var hostApi = FakeHostApi()
 
   var eventListenerFactory: EventListener.Factory = FakeEventListener.Factory(eventLog)
 
@@ -98,7 +98,7 @@ internal class TreehouseTester(
     override fun create(scope: CoroutineScope, dispatchers: TreehouseDispatchers) = frameClock
   }
 
-  val treehouseAppFactory = RealTreehouseApp.Factory(
+  private val treehouseAppFactory = RealTreehouseApp.Factory(
     platform = platform,
     httpClient = httpClient,
     frameClockFactory = frameClockFactory,
@@ -129,7 +129,7 @@ internal class TreehouseTester(
       treehouseApp: TreehouseApp<TestAppPresenter>,
       zipline: Zipline,
     ) {
-      zipline.bind("HostApi", hostApi)
+      zipline.bind<HostApi>("HostApi", hostApi)
     }
 
     override fun create(zipline: Zipline): TestAppPresenter {

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/leaks/JvmHeap.kt

@@ -18,6 +18,7 @@ package app.cash.redwood.treehouse.leaks
 import androidx.collection.IntObjectMap
 import androidx.collection.ScatterSet
 import app.cash.redwood.treehouse.EventLog
+import com.example.redwood.testapp.treehouse.HostApi
 import java.lang.ref.WeakReference
 import java.lang.reflect.Field
 import kotlinx.coroutines.CoroutineDispatcher
@@ -83,6 +84,7 @@ internal object JvmHeap : Heap {
       instance is CoroutineDispatcher -> listOf()
       instance is Enum<*> -> listOf()
       instance is EventLog -> listOf()
+      instance is HostApi -> listOf()
       instance is Int -> listOf()
       instance is Job -> listOf()
       instance is Json -> listOf()

samples/emoji-search/ios-uikit/EmojiSearchApp/IosHostApi.swift

@@ -48,6 +48,9 @@ class IosHostApi : HostApi {
         }
     }
 
+    func log(message: String) {
+    }
+
     func close() {
     }
 }

test-app/android-views/src/main/kotlin/com/example/redwood/testapp/android/views/RealHostApi.kt

@@ -59,4 +59,7 @@ class RealHostApi(
       continuation.invokeOnCancellation { call.cancel() }
     }
   }
+
+  override fun log(message: String) {
+  }
 }

test-app/ios-uikit/TestApp/IosHostApi.swift

@@ -42,6 +42,9 @@ class IosHostApi : HostApi {
         }
     }
 
+    func log(message: String) {
+    }
+
     func close() {
     }
 }

test-app/presenter-treehouse/src/commonMain/kotlin/com/example/redwood/testapp/treehouse/HostApi.kt

@@ -22,4 +22,7 @@ import kotlin.native.ObjCName
 interface HostApi : ZiplineService {
   /** Decodes the response as a string and returns it. */
   suspend fun httpCall(url: String, headers: Map<String, String>): String
+
+  /** Records a message for any potential consumer. */
+  fun log(message: String)
 }

test-app/presenter-treehouse/src/jsMain/kotlin/com/example/redwood/testapp/treehouse/RealTestAppPresenter.kt

@@ -37,6 +37,7 @@ class RealTestAppPresenter(
   }
 
   override fun launchForTester(): ZiplineTreehouseUi {
-    return TesterTreehouseUi().asZiplineTreehouseUi(appLifecycle)
+    val treehouseUi = TesterTreehouseUi(hostApi)
+    return treehouseUi.asZiplineTreehouseUi(appLifecycle)
   }
 }

test-app/presenter-treehouse/src/jsMain/kotlin/com/example/redwood/testapp/treehouse/TesterTreehouseUi.kt

@@ -16,6 +16,7 @@
 package com.example.redwood.testapp.treehouse
 
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -24,19 +25,25 @@ import app.cash.redwood.treehouse.TreehouseUi
 import com.example.redwood.testapp.compose.Button
 import com.example.redwood.testapp.compose.TextInput
 
-class TesterTreehouseUi : TreehouseUi {
+class TesterTreehouseUi(
+  private val hostApi: HostApi,
+) : TreehouseUi {
   @Composable
   override fun Show() {
     var content by remember { mutableStateOf(Content.InitialValue) }
-    content.Show { newContent ->
-      content = newContent
-    }
+    content.Show(
+      changeContent = { newContent -> content = newContent },
+      log = { message -> hostApi.log(message) },
+    )
   }
 
   enum class Content {
     InitialValue {
       @Composable
-      override fun Show(changeContent: (Content) -> Unit) {
+      override fun Show(
+        changeContent: (Content) -> Unit,
+        log: (String) -> Unit,
+      ) {
         TextInput(
           text = "what would you like to see?",
           customType = null,
@@ -47,9 +54,33 @@ class TesterTreehouseUi : TreehouseUi {
       }
     },
 
+    GuestLifecycleTestShowDisposable {
+      @Composable
+      override fun Show(
+        changeContent: (Content) -> Unit,
+        log: (String) -> Unit,
+      ) {
+        DisposableEffect(log) {
+          log("DisposableEffect.effect()")
+          onDispose {
+            log("DisposableEffect.dispose()")
+          }
+        }
+        Button(
+          text = "Next",
+          onClick = {
+            changeContent(Empty)
+          },
+        )
+      }
+    },
+
     TreehouseTesterTestHappyPathStep2 {
       @Composable
-      override fun Show(changeContent: (Content) -> Unit) {
+      override fun Show(
+        changeContent: (Content) -> Unit,
+        log: (String) -> Unit,
+      ) {
         Button(
           text = "This is TreehouseTesterTestHappyPathStep2",
           onClick = {
@@ -60,13 +91,19 @@ class TesterTreehouseUi : TreehouseUi {
 
     Empty {
       @Composable
-      override fun Show(changeContent: (Content) -> Unit) {
+      override fun Show(
+        changeContent: (Content) -> Unit,
+        log: (String) -> Unit,
+      ) {
       }
     },
 
     ;
 
     @Composable
-    abstract fun Show(changeContent: (Content) -> Unit)
+    abstract fun Show(
+      changeContent: (Content) -> Unit,
+      log: (String) -> Unit,
+    )
   }
 }