From a39d079e1076fd060ce2c7b9582e3ae79d037600 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Wed, 29 May 2024 15:28:39 -0400 Subject: [PATCH] Tests that we don't leak ContentSources (#2054) * Tests that we don't leak ContentSources This is another one that wanted some small internal changes to our observability APIs. * apiDump * Track API changes in CodeListener --- .../api/android/redwood-treehouse-host.api | 2 +- .../api/jvm/redwood-treehouse-host.api | 2 +- .../api/redwood-treehouse-host.klib.api | 2 +- .../app/cash/redwood/treehouse/LeaksTest.kt | 46 +++++++++++++++++++ .../RetainEverythingEventListenerFactory.kt | 5 +- .../RetainEverythingTreehouseContentSource.kt | 28 +++++++++++ .../redwood/treehouse/CodeEventPublisher.kt | 4 +- .../cash/redwood/treehouse/CodeListener.kt | 19 ++++---- .../treehouse/RealCodeEventPublisher.kt | 4 +- .../redwood/treehouse/RealEventPublisher.kt | 3 +- .../redwood/treehouse/TreehouseAppContent.kt | 26 ++++++----- .../cash/redwood/treehouse/CodeHostTest.kt | 15 ++++-- .../treehouse/FakeCodeEventPublisher.kt | 6 +-- .../redwood/treehouse/FakeCodeListener.kt | 6 +-- .../treehouse/TreehouseAppContentTest.kt | 20 ++++++-- .../android/views/EmojiSearchActivity.kt | 16 ++++--- .../EmojiSearchViewController.swift | 26 ++++++----- 17 files changed, 170 insertions(+), 60 deletions(-) create mode 100644 redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingTreehouseContentSource.kt diff --git a/redwood-treehouse-host/api/android/redwood-treehouse-host.api b/redwood-treehouse-host/api/android/redwood-treehouse-host.api index 81f6c00033..7cf544e98c 100644 --- a/redwood-treehouse-host/api/android/redwood-treehouse-host.api +++ b/redwood-treehouse-host/api/android/redwood-treehouse-host.api @@ -5,9 +5,9 @@ public final class app/cash/redwood/treehouse/ChangeListRenderer { public class app/cash/redwood/treehouse/CodeListener { public fun ()V + public fun onCodeDetached (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Ljava/lang/Throwable;)V public fun onCodeLoaded (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Z)V public fun onInitialCodeLoading (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;)V - public fun onUncaughtException (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Ljava/lang/Throwable;)V } public abstract interface class app/cash/redwood/treehouse/Content { diff --git a/redwood-treehouse-host/api/jvm/redwood-treehouse-host.api b/redwood-treehouse-host/api/jvm/redwood-treehouse-host.api index b06476021a..3cec56f152 100644 --- a/redwood-treehouse-host/api/jvm/redwood-treehouse-host.api +++ b/redwood-treehouse-host/api/jvm/redwood-treehouse-host.api @@ -5,9 +5,9 @@ public final class app/cash/redwood/treehouse/ChangeListRenderer { public class app/cash/redwood/treehouse/CodeListener { public fun ()V + public fun onCodeDetached (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Ljava/lang/Throwable;)V public fun onCodeLoaded (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Z)V public fun onInitialCodeLoading (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;)V - public fun onUncaughtException (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseView;Ljava/lang/Throwable;)V } public abstract interface class app/cash/redwood/treehouse/Content { diff --git a/redwood-treehouse-host/api/redwood-treehouse-host.klib.api b/redwood-treehouse-host/api/redwood-treehouse-host.klib.api index fa8b421816..7e900992f0 100644 --- a/redwood-treehouse-host/api/redwood-treehouse-host.klib.api +++ b/redwood-treehouse-host/api/redwood-treehouse-host.klib.api @@ -99,9 +99,9 @@ final fun <#A: app.cash.redwood.treehouse/AppService, #B: kotlin/Any> (app.cash. final fun <#A: kotlin/Any> (app.cash.redwood.treehouse/Content).app.cash.redwood.treehouse/bindWhenReady(app.cash.redwood.treehouse/TreehouseView<#A>): okio/Closeable // app.cash.redwood.treehouse/bindWhenReady|bindWhenReady@app.cash.redwood.treehouse.Content(app.cash.redwood.treehouse.TreehouseView<0:0>){0ยง}[0] open class app.cash.redwood.treehouse/CodeListener { // app.cash.redwood.treehouse/CodeListener|null[0] constructor () // app.cash.redwood.treehouse/CodeListener.|(){}[0] + open fun onCodeDetached(app.cash.redwood.treehouse/TreehouseApp<*>, app.cash.redwood.treehouse/TreehouseView<*>, kotlin/Throwable?) // app.cash.redwood.treehouse/CodeListener.onCodeDetached|onCodeDetached(app.cash.redwood.treehouse.TreehouseApp<*>;app.cash.redwood.treehouse.TreehouseView<*>;kotlin.Throwable?){}[0] open fun onCodeLoaded(app.cash.redwood.treehouse/TreehouseApp<*>, app.cash.redwood.treehouse/TreehouseView<*>, kotlin/Boolean) // app.cash.redwood.treehouse/CodeListener.onCodeLoaded|onCodeLoaded(app.cash.redwood.treehouse.TreehouseApp<*>;app.cash.redwood.treehouse.TreehouseView<*>;kotlin.Boolean){}[0] open fun onInitialCodeLoading(app.cash.redwood.treehouse/TreehouseApp<*>, app.cash.redwood.treehouse/TreehouseView<*>) // app.cash.redwood.treehouse/CodeListener.onInitialCodeLoading|onInitialCodeLoading(app.cash.redwood.treehouse.TreehouseApp<*>;app.cash.redwood.treehouse.TreehouseView<*>){}[0] - open fun onUncaughtException(app.cash.redwood.treehouse/TreehouseApp<*>, app.cash.redwood.treehouse/TreehouseView<*>, kotlin/Throwable) // app.cash.redwood.treehouse/CodeListener.onUncaughtException|onUncaughtException(app.cash.redwood.treehouse.TreehouseApp<*>;app.cash.redwood.treehouse.TreehouseView<*>;kotlin.Throwable){}[0] } open class app.cash.redwood.treehouse/EventListener { // app.cash.redwood.treehouse/EventListener|null[0] abstract fun interface Factory { // app.cash.redwood.treehouse/EventListener.Factory|null[0] diff --git a/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt index 72ff0c68ae..aa5dba93d1 100644 --- a/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt +++ b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt @@ -22,7 +22,9 @@ import assertk.assertions.isEqualTo import assertk.assertions.isNotNull import com.example.redwood.testapp.testing.TextInputValue import kotlin.test.Test +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.first +import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest class LeaksTest { @@ -121,4 +123,48 @@ class LeaksTest { treehouseApp.close() eventListenerLeakWatcher.assertNotLeaked() } + + @Test + fun contentSourceNotLeaked() = runTest { + val tester = TreehouseTester(this) + val treehouseApp = tester.loadApp() + val view = tester.view() + + // Use an inline run() to try to prevent contentSource from being retained on the stack. + val (content, contentSourceLeakWatcher) = run { + val contentSource = RetainEverythingTreehouseContentSource() + + val content = treehouseApp.createContent( + source = contentSource, + codeListener = FakeCodeListener(tester.eventLog), + ) + + return@run content to LeakWatcher { contentSource } + } + + // After we bind the content, it'll be in a retain cycle. + content.bind(view) + tester.eventLog.takeEvent("onCodeLoaded(test_app, view, initial = true)", skipOthers = true) + contentSourceLeakWatcher.assertObjectInReferenceCycle() + + // Unbind it, and it's no longer retained. + content.unbind() + tester.eventLog.takeEvent("onCodeDetached(test_app, view, null)", skipOthers = true) + treehouseApp.dispatchers.awaitLaunchedTasks() + contentSourceLeakWatcher.assertNotLeaked() + + treehouseApp.stop() + } + + /** + * This is unfortunate. Some cleanup functions launch jobs on another dispatcher and we don't have + * a natural way to wait for those jobs to complete. So we launch empty jobs on each dispatcher, + * and trust that they won't start until existing queued jobs finish. + */ + private suspend fun TreehouseDispatchers.awaitLaunchedTasks() { + coroutineScope { + launch(zipline) {}.join() + launch(ui) {}.join() + } + } } diff --git a/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingEventListenerFactory.kt b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingEventListenerFactory.kt index a7be65186b..2dd3fb4260 100644 --- a/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingEventListenerFactory.kt +++ b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingEventListenerFactory.kt @@ -18,7 +18,10 @@ package app.cash.redwood.treehouse import app.cash.zipline.Zipline import app.cash.zipline.ZiplineManifest -/** An event listener that keeps a reference to everything it sees, for defensive leak testing. */ +/** + * An event listener (and factory) that keeps a reference to everything it sees, for defensive leak + * testing. + */ class RetainEverythingEventListenerFactory( private val eventLog: EventLog, ) : EventListener(), EventListener.Factory { diff --git a/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingTreehouseContentSource.kt b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingTreehouseContentSource.kt new file mode 100644 index 0000000000..2ddc2496e1 --- /dev/null +++ b/redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingTreehouseContentSource.kt @@ -0,0 +1,28 @@ +/* + * 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 com.example.redwood.testapp.treehouse.TestAppPresenter + +/** A content source that keeps a reference to everything it sees, for defensive leak testing. */ +class RetainEverythingTreehouseContentSource : TreehouseContentSource { + private var app: TestAppPresenter? = null + + override fun get(app: TestAppPresenter): ZiplineTreehouseUi { + this.app = app + return app.launchForTester() + } +} diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeEventPublisher.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeEventPublisher.kt index 56c1f83553..42ccbb1fa3 100644 --- a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeEventPublisher.kt +++ b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeEventPublisher.kt @@ -25,8 +25,8 @@ internal interface CodeEventPublisher { initial: Boolean, ) - fun onUncaughtException( + fun onCodeDetached( view: TreehouseView<*>, - exception: Throwable, + exception: Throwable?, ) } diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeListener.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeListener.kt index ab4d6055c2..542641dc76 100644 --- a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeListener.kt +++ b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeListener.kt @@ -43,22 +43,25 @@ public open class CodeListener { } /** - * Invoked when the application powering [view] fails with an uncaught exception. This function - * should display an error UI. + * Invoked when the application powering [view] stops sending updates. This is triggered by: * - * Typical implementations call [TreehouseView.reset] and display an error placeholder. - * Development builds may show more diagnostic information than production builds. + * * the UI no longer needing code to drive it, perhaps because it's detached from the screen + * * the code being hot-reloaded + * * the code failing with an exception + * + * If it is failing due to a failure, [exception] will be non-null and this function should + * display an error UI. Typical implementations call [TreehouseView.reset] and display an error + * placeholder. Development builds may show more diagnostic information than production builds. * * When a Treehouse app fails, its current Zipline instance is canceled so no further code will * execute. A new Zipline will start when new code available. * - * This condition is not permanent! If new code is loaded after an error, [onCodeLoaded] will be - * called. + * This condition is not permanent! If so, [onCodeLoaded] will be called. */ - public open fun onUncaughtException( + public open fun onCodeDetached( app: TreehouseApp<*>, view: TreehouseView<*>, - exception: Throwable, + exception: Throwable?, ) { } } diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealCodeEventPublisher.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealCodeEventPublisher.kt index 331cd11aa6..5d733d296c 100644 --- a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealCodeEventPublisher.kt +++ b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealCodeEventPublisher.kt @@ -27,7 +27,7 @@ internal class RealCodeEventPublisher( return codeListener.onCodeLoaded(app, view, initial) } - override fun onUncaughtException(view: TreehouseView<*>, exception: Throwable) { - return codeListener.onUncaughtException(app, view, exception) + override fun onCodeDetached(view: TreehouseView<*>, exception: Throwable?) { + return codeListener.onCodeDetached(app, view, exception) } } diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealEventPublisher.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealEventPublisher.kt index fe25ac851b..97f15b3bcc 100644 --- a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealEventPublisher.kt +++ b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealEventPublisher.kt @@ -173,7 +173,8 @@ internal class RealEventPublisher( } override fun serviceLeaked(zipline: Zipline, name: String) { - listener!!.serviceLeaked(name) + // Drop serviceLeaked() calls made after close(). This can happen in practice. + listener?.serviceLeaked(name) } override fun ziplineClosed(zipline: Zipline) { diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt index c3f4d63226..dc1d606971 100644 --- a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt +++ b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt @@ -172,7 +172,7 @@ internal class TreehouseAppContent( codeHost.removeListener(this) if (previousState.codeState is CodeState.Running) { val binding = previousState.codeState.viewContentCodeBinding - binding.cancel() + binding.cancel(null) binding.codeSession.removeListener(this) } @@ -215,7 +215,7 @@ internal class TreehouseAppContent( // If we replaced an old binding, cancel that old binding. if (previousCodeState is CodeState.Running) { val binding = previousCodeState.viewContentCodeBinding - binding.cancel() + binding.cancel(null) binding.codeSession.removeListener(this) } @@ -246,15 +246,9 @@ internal class TreehouseAppContent( // Cancel the UI binding to the canceled code. val binding = previousCodeState.viewContentCodeBinding - binding.cancel() + binding.cancel(exception) binding.codeSession.removeListener(this) - // If there's an error and a UI, show it. - val view = (viewState as? ViewState.Bound)?.view - if (exception != null && view != null) { - codeEventPublisher.onUncaughtException(view, exception) - } - val nextCodeState = CodeState.Idle(isInitialLaunch = false) stateFlow.value = State(viewState, nextCodeState) } @@ -302,7 +296,7 @@ private class ViewContentCodeBinding( val stateStore: StateStore, val dispatchers: TreehouseDispatchers, val eventPublisher: EventPublisher, - val contentSource: TreehouseContentSource, + contentSource: TreehouseContentSource, val codeEventPublisher: CodeEventPublisher, val stateFlow: MutableStateFlow>, val codeSession: CodeSession, @@ -328,6 +322,9 @@ private class ViewContentCodeBinding( /** Only accessed on [TreehouseDispatchers.zipline]. */ private val serviceScope = codeSession.newServiceScope() + /** Only accessed on [TreehouseDispatchers.zipline]. Null after [cancel]. */ + private var contentSource: TreehouseContentSource? = contentSource + /** Only accessed on [TreehouseDispatchers.zipline]. Null after [cancel]. */ private var treehouseUiOrNull: ZiplineTreehouseUi? = null @@ -435,7 +432,7 @@ private class ViewContentCodeBinding( fun start() { bindingScope.launch(dispatchers.zipline) { val scopedAppService = serviceScope.apply(codeSession.appService) - val treehouseUi = contentSource.get(scopedAppService) + val treehouseUi = contentSource!!.get(scopedAppService) treehouseUiOrNull = treehouseUi eventBridge.delegate = treehouseUi stateSnapshot = viewOrNull?.stateSnapshotId?.let { @@ -497,9 +494,13 @@ private class ViewContentCodeBinding( } @OptIn(ExperimentalCoroutinesApi::class) - fun cancel() { + fun cancel(exception: Throwable?) { dispatchers.checkUi() + + if (canceled) return canceled = true + + viewOrNull?.let { codeEventPublisher.onCodeDetached(it, exception) } viewOrNull?.saveCallback = null viewOrNull = null bridgeOrNull?.close() @@ -507,6 +508,7 @@ private class ViewContentCodeBinding( eventBridge.bindingScope = null eventBridge.ziplineDispatcher = null bindingScope.launch(dispatchers.zipline, start = CoroutineStart.ATOMIC) { + contentSource = null treehouseUiOrNull = null eventBridge.delegate = null serviceScope.close() diff --git a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/CodeHostTest.kt b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/CodeHostTest.kt index ad465fd400..b6381be9ae 100644 --- a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/CodeHostTest.kt +++ b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/CodeHostTest.kt @@ -65,6 +65,7 @@ class CodeHostTest { codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -87,6 +88,7 @@ class CodeHostTest { codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -108,6 +110,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionA.app.uis[0].start()") codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -118,6 +121,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionB.app.uis[0].start()") codeHost.stop() eventLog.takeEvent("codeHostUpdates2.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") eventLog.takeEvent("codeSessionB.stop()") @@ -138,7 +142,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionA.start()") eventLog.takeEvent("codeSessionA.app.uis[0].start()") codeSessionA.handleUncaughtException(Exception("boom!")) - eventLog.takeEvent("codeListener.onUncaughtException(view1, kotlin.Exception: boom!)") + eventLog.takeEvent("codeListener.onCodeDetached(view1, kotlin.Exception: boom!)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -150,6 +154,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionB.app.uis[0].start()") codeHost.stop() eventLog.takeEvent("codeHostUpdates2.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") eventLog.takeEvent("codeSessionB.stop()") @@ -170,7 +175,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionA.start()") eventLog.takeEvent("codeSessionA.app.uis[0].start()") codeSessionA.handleUncaughtException(Exception("boom!")) - eventLog.takeEvent("codeListener.onUncaughtException(view1, kotlin.Exception: boom!)") + eventLog.takeEvent("codeListener.onCodeDetached(view1, kotlin.Exception: boom!)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -179,6 +184,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionB.app.uis[0].start()") codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") eventLog.takeEvent("codeSessionB.stop()") @@ -199,7 +205,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionA.start()") eventLog.takeEvent("codeSessionA.app.uis[0].start()") codeSessionA.handleUncaughtException(Exception("boom!")) - eventLog.takeEvent("codeListener.onUncaughtException(view1, kotlin.Exception: boom!)") + eventLog.takeEvent("codeListener.onCodeDetached(view1, kotlin.Exception: boom!)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -229,6 +235,7 @@ class CodeHostTest { eventLog.takeEvent("codeSessionA.app.uis[0].start()") codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -254,6 +261,7 @@ class CodeHostTest { codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") @@ -276,6 +284,7 @@ class CodeHostTest { codeHost.stop() eventLog.takeEvent("codeHostUpdates1.close()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") diff --git a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeEventPublisher.kt b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeEventPublisher.kt index 689723d8fe..9c7f0cb973 100644 --- a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeEventPublisher.kt +++ b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeEventPublisher.kt @@ -26,9 +26,9 @@ class FakeCodeEventPublisher( eventLog += "codeListener.onCodeLoaded($view, initial = $initial)" } - override fun onUncaughtException(view: TreehouseView<*>, exception: Throwable) { + override fun onCodeDetached(view: TreehouseView<*>, exception: Throwable?) { // Canonicalize "java.lang.Exception(boom!)" to "kotlin.Exception(boom!)". - val exceptionString = exception.toString().replace("java.lang.", "kotlin.") - eventLog += "codeListener.onUncaughtException($view, $exceptionString)" + val exceptionString = exception?.toString()?.replace("java.lang.", "kotlin.") + eventLog += "codeListener.onCodeDetached($view, $exceptionString)" } } diff --git a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeListener.kt b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeListener.kt index 7b828ad210..52c38b0354 100644 --- a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeListener.kt +++ b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeCodeListener.kt @@ -23,10 +23,10 @@ class FakeCodeListener( } override fun onCodeLoaded(app: TreehouseApp<*>, view: TreehouseView<*>, initial: Boolean) { - eventLog += "onCodeLoaded(${app.spec.name}, $view, $initial)" + eventLog += "onCodeLoaded(${app.spec.name}, $view, initial = $initial)" } - override fun onUncaughtException(app: TreehouseApp<*>, view: TreehouseView<*>, exception: Throwable) { - eventLog += "onUncaughtException(${app.spec.name}, $view, $exception)" + override fun onCodeDetached(app: TreehouseApp<*>, view: TreehouseView<*>, exception: Throwable?) { + eventLog += "onCodeDetached(${app.spec.name}, $view, $exception)" } } diff --git a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/TreehouseAppContentTest.kt b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/TreehouseAppContentTest.kt index e7fa6d33fb..95a5d306f5 100644 --- a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/TreehouseAppContentTest.kt +++ b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/TreehouseAppContentTest.kt @@ -91,6 +91,7 @@ class TreehouseAppContentTest { eventLog.takeEvent("codeSessionA.app.uis[0].sendEvent()") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -114,6 +115,7 @@ class TreehouseAppContentTest { eventLog.takeEvent("codeListener.onCodeLoaded(view1, initial = true)") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -137,6 +139,7 @@ class TreehouseAppContentTest { assertThat(buttonValue.text).isEqualTo("hello") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -157,6 +160,7 @@ class TreehouseAppContentTest { assertThat(buttonValue.text).isEqualTo("hello") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -177,6 +181,7 @@ class TreehouseAppContentTest { val codeSessionB = codeHost.startCodeSession("codeSessionB") eventLog.takeEventsInAnyOrder( + "codeListener.onCodeDetached(view1, null)", "codeSessionA.app.uis[0].close()", "codeSessionA.stop()", "codeSessionB.start()", @@ -193,6 +198,7 @@ class TreehouseAppContentTest { assertThat(buttonB.text).isEqualTo("helloB") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") } @@ -248,6 +254,7 @@ class TreehouseAppContentTest { assertThat(buttonA.text).isEqualTo("helloA") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") content.bind(view1) @@ -259,6 +266,7 @@ class TreehouseAppContentTest { assertThat(buttonB.text).isEqualTo("helloB") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[1].close()") } @@ -285,6 +293,7 @@ class TreehouseAppContentTest { eventLog.assertNoEvents() content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -305,6 +314,7 @@ class TreehouseAppContentTest { content.unbind() eventLog.takeEvent("onBackPressedDispatcher.callbacks[0].cancel()") + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") } @@ -324,6 +334,7 @@ class TreehouseAppContentTest { // When we close codeSessionA, its back handlers are released with it. eventLog.takeEventsInAnyOrder( + "codeListener.onCodeDetached(view1, null)", "codeSessionA.app.uis[0].close()", "onBackPressedDispatcher.callbacks[0].cancel()", "codeSessionA.stop()", @@ -333,6 +344,7 @@ class TreehouseAppContentTest { assertThat(onBackPressedDispatcher.callbacks).isEmpty() content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") } @@ -350,7 +362,7 @@ class TreehouseAppContentTest { codeSessionA.handleUncaughtException(Exception("boom!")) eventLog.takeEventsInAnyOrder( "codeSessionA.app.uis[0].close()", - "codeListener.onUncaughtException(view1, kotlin.Exception: boom!)", + "codeListener.onCodeDetached(view1, kotlin.Exception: boom!)", "codeSessionA.stop()", ) @@ -376,6 +388,7 @@ class TreehouseAppContentTest { eventLog.takeEvent("codeSessionB.app.uis[0].start()") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") } @@ -393,7 +406,7 @@ class TreehouseAppContentTest { codeSessionA.handleUncaughtException(Exception("boom!")) eventLog.takeEventsInAnyOrder( "codeSessionA.app.uis[0].close()", - "codeListener.onUncaughtException(view1, kotlin.Exception: boom!)", + "codeListener.onCodeDetached(view1, kotlin.Exception: boom!)", "codeSessionA.stop()", ) @@ -402,6 +415,7 @@ class TreehouseAppContentTest { eventLog.takeEvent("codeSessionB.app.uis[0].start()") content.unbind() + eventLog.takeEvent("codeListener.onCodeDetached(view1, null)") eventLog.takeEvent("codeSessionB.app.uis[0].close()") } @@ -449,7 +463,7 @@ class TreehouseAppContentTest { val button = view1.views.single() as ButtonValue button.onClick!!.invoke() eventLog.takeEvent("codeSessionA.app.uis[0].sendEvent()") - eventLog.takeEvent("codeListener.onUncaughtException(view1, kotlin.Exception: boom!)") + eventLog.takeEvent("codeListener.onCodeDetached(view1, kotlin.Exception: boom!)") eventLog.takeEvent("codeSessionA.app.uis[0].close()") eventLog.takeEvent("codeSessionA.stop()") diff --git a/samples/emoji-search/android-views/src/main/kotlin/com/example/redwood/emojisearch/android/views/EmojiSearchActivity.kt b/samples/emoji-search/android-views/src/main/kotlin/com/example/redwood/emojisearch/android/views/EmojiSearchActivity.kt index 1c8222c468..37a5ece8ca 100644 --- a/samples/emoji-search/android-views/src/main/kotlin/com/example/redwood/emojisearch/android/views/EmojiSearchActivity.kt +++ b/samples/emoji-search/android-views/src/main/kotlin/com/example/redwood/emojisearch/android/views/EmojiSearchActivity.kt @@ -87,16 +87,18 @@ class EmojiSearchActivity : ComponentActivity() { } private val codeListener: CodeListener = object : CodeListener() { - override fun onUncaughtException( + override fun onCodeDetached( app: TreehouseApp<*>, view: TreehouseView<*>, - exception: Throwable, + exception: Throwable?, ) { - treehouseLayout.reset() - treehouseLayout.addView( - ExceptionView(treehouseLayout, exception), - LinearLayout.LayoutParams(MATCH_PARENT, MATCH_PARENT), - ) + if (exception != null) { + treehouseLayout.reset() + treehouseLayout.addView( + ExceptionView(treehouseLayout, exception), + LinearLayout.LayoutParams(MATCH_PARENT, MATCH_PARENT), + ) + } } } diff --git a/samples/emoji-search/ios-uikit/EmojiSearchApp/EmojiSearchViewController.swift b/samples/emoji-search/ios-uikit/EmojiSearchApp/EmojiSearchViewController.swift index 89b47bc927..92c854cbec 100644 --- a/samples/emoji-search/ios-uikit/EmojiSearchApp/EmojiSearchViewController.swift +++ b/samples/emoji-search/ios-uikit/EmojiSearchApp/EmojiSearchViewController.swift @@ -78,18 +78,20 @@ class EmojiSearchCodeListener : CodeListener { self.treehouseView = treehouseView } - override func onUncaughtException(app: TreehouseApp, view: TreehouseView, exception: KotlinThrowable) { - treehouseView.reset() - - let exceptionView = ExceptionView(exception) - exceptionView.translatesAutoresizingMaskIntoConstraints = false - treehouseView.view.addSubview(exceptionView) - NSLayoutConstraint.activate([ - exceptionView.topAnchor.constraint(equalTo: treehouseView.view.topAnchor), - exceptionView.leftAnchor.constraint(equalTo: treehouseView.view.leftAnchor), - exceptionView.rightAnchor.constraint(equalTo: treehouseView.view.rightAnchor), - exceptionView.bottomAnchor.constraint(equalTo: treehouseView.view.bottomAnchor), - ]) + override func onCodeDetached(app: TreehouseApp, view: TreehouseView, exception: KotlinThrowable?) { + if (exception != nil) { + treehouseView.reset() + + let exceptionView = ExceptionView(exception!) + exceptionView.translatesAutoresizingMaskIntoConstraints = false + treehouseView.view.addSubview(exceptionView) + NSLayoutConstraint.activate([ + exceptionView.topAnchor.constraint(equalTo: treehouseView.view.topAnchor), + exceptionView.leftAnchor.constraint(equalTo: treehouseView.view.leftAnchor), + exceptionView.rightAnchor.constraint(equalTo: treehouseView.view.rightAnchor), + exceptionView.bottomAnchor.constraint(equalTo: treehouseView.view.bottomAnchor), + ]) + } } }