Add a test for CodeHost

Also adopt CoroutineScope more aggressively in CodeSession for lifecycle management.
cashapp · Nov 3, 2023 · 79b46c1 · 79b46c1
1 parent 932489d
commit 79b46c1
Show file tree

Hide file tree

Showing 11 changed files with 400 additions and 96 deletions.
diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeHost.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeHost.kt
@@ -77,7 +77,7 @@ internal abstract class CodeHost<A : AppService>(
     get() = state.codeSession
 
   /** Returns a flow that emits a new [CodeSession] each time we should load fresh code. */
-  abstract fun codeUpdatesFlow() : Flow<CodeSession<A>>
+  abstract fun codeUpdatesFlow(): Flow<CodeSession<A>>
 
   fun start() {
     dispatchers.checkUi()
@@ -88,9 +88,10 @@ internal abstract class CodeHost<A : AppService>(
 
     // Force a restart if we're crashed.
     previous.codeUpdatesScope?.cancel()
-    val codeUpdatesScope = startReceivingCodeUpdates()
 
+    val codeUpdatesScope = codeUpdatesScope()
     state = State.Starting(codeUpdatesScope)
+    codeUpdatesScope.collectCodeUpdates()
   }
 
   /** This function may only be invoked on [TreehouseDispatchers.zipline]. */
@@ -114,9 +115,10 @@ internal abstract class CodeHost<A : AppService>(
     previous.codeUpdatesScope?.cancel()
     previous.codeSession?.removeListener(codeSessionListener)
     previous.codeSession?.cancel()
-    val codeUpdatesScope = startReceivingCodeUpdates()
 
+    val codeUpdatesScope = codeUpdatesScope()
     state = State.Starting(codeUpdatesScope)
+    codeUpdatesScope.collectCodeUpdates()
   }
 
   fun addListener(listener: Listener<A>) {
@@ -129,43 +131,37 @@ internal abstract class CodeHost<A : AppService>(
     listeners -= listener
   }
 
-  private fun startReceivingCodeUpdates(): CoroutineScope {
-    val codeUpdatesScope = CoroutineScope(SupervisorJob(appScope.coroutineContext.job))
-    codeUpdatesScope.launch(dispatchers.zipline) {
+  private fun codeUpdatesScope() =
+    CoroutineScope(SupervisorJob(appScope.coroutineContext.job))
+
+  private fun CoroutineScope.collectCodeUpdates() {
+    launch(dispatchers.zipline) {
       codeUpdatesFlow().collect {
         codeSessionLoaded(it)
       }
     }
-    return codeUpdatesScope
   }
 
   private fun codeSessionLoaded(next: CodeSession<A>) {
     dispatchers.checkZipline()
 
-    val codeSessionScope = CoroutineScope(
-      SupervisorJob(appScope.coroutineContext.job) + next.coroutineExceptionHandler,
-    )
-
-    codeSessionScope.launch(dispatchers.ui) {
+    next.scope.launch(dispatchers.ui) {
       // Clean up the previous session.
       val previous = state
       previous.codeSession?.removeListener(codeSessionListener)
       previous.codeSession?.cancel()
 
       // If the codeUpdatesScope is null, we're stopped. Discard the newly-loaded code.
-      val scope = state.codeUpdatesScope
-      if (scope == null) {
+      val codeUpdatesScope = state.codeUpdatesScope
+      if (codeUpdatesScope == null) {
         next.cancel()
         return@launch
       }
 
       // Boot up the new code.
-      state = State.Running(scope, next)
+      state = State.Running(codeUpdatesScope, next)
       next.addListener(codeSessionListener)
-      next.start(
-        sessionScope = codeSessionScope,
-        frameClock = frameClockFactory.create(codeSessionScope, dispatchers),
-      )
+      next.start()
 
       for (listener in listeners) {
         listener.codeSessionChanged(next)

diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeSession.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeSession.kt
@@ -22,13 +22,15 @@ import kotlinx.serialization.json.Json
 
 /** The host state for a single code load. We get a new session each time we get new code. */
 internal interface CodeSession<A : AppService> {
+  val scope: CoroutineScope
+
   val eventPublisher: EventPublisher
 
   val appService: A
 
   val json: Json
 
-  fun start(sessionScope: CoroutineScope, frameClock: FrameClock)
+  fun start()
 
   fun addListener(listener: Listener<A>)
 

diff --git a/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseApp.kt b/redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseApp.kt
@@ -92,7 +92,6 @@ public class TreehouseApp<A : AppService> private constructor(
     return TreehouseAppContent(
       codeHost = codeHost,
       dispatchers = dispatchers,
-      appScope = appScope,
       codeListener = codeListener,
       source = source,
     )
@@ -179,10 +178,11 @@ public class TreehouseApp<A : AppService> private constructor(
 
     return ZiplineCodeSession(
       dispatchers = dispatchers,
-      appScope = appScope,
       eventPublisher = eventPublisher,
+      frameClockFactory = factory.frameClockFactory,
       appService = appService,
       zipline = zipline,
+      appScope = appScope,
     )
   }
 

diff --git a/...od-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt b/...od-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt
@@ -59,7 +59,9 @@ private sealed interface ViewState {
 }
 
 private sealed interface CodeState<A : AppService> {
-  class Idle<A : AppService> : CodeState<A>
+  class Idle<A : AppService>(
+    val isInitialLaunch: Boolean,
+  ) : CodeState<A>
 
   class Running<A : AppService>(
     val viewContentCodeBinding: ViewContentCodeBinding<A>,
@@ -70,11 +72,12 @@ private sealed interface CodeState<A : AppService> {
 internal class TreehouseAppContent<A : AppService>(
   private val codeHost: CodeHost<A>,
   private val dispatchers: TreehouseDispatchers,
-  private val appScope: CoroutineScope,
   private val codeListener: CodeListener,
   private val source: TreehouseContentSource<A>,
 ) : Content, CodeHost.Listener<A>, CodeSession.Listener<A> {
-  private val stateFlow = MutableStateFlow<State<A>>(State(ViewState.None, CodeState.Idle()))
+  private val stateFlow = MutableStateFlow<State<A>>(
+    State(ViewState.None, CodeState.Idle(isInitialLaunch = true)),
+  )
 
   override fun preload(
     onBackPressedDispatcher: OnBackPressedDispatcher,
@@ -169,7 +172,7 @@ internal class TreehouseAppContent<A : AppService>(
     if (previousViewState is ViewState.None) return // Idempotent.
 
     val nextViewState = ViewState.None
-    val nextCodeState = CodeState.Idle<A>()
+    val nextCodeState = CodeState.Idle<A>(isInitialLaunch = true)
 
     // Cancel the code if necessary.
     codeHost.removeListener(this)
@@ -204,7 +207,7 @@ internal class TreehouseAppContent<A : AppService>(
     val nextCodeState = CodeState.Running(
       startViewCodeContentBinding(
         codeSession = next,
-        isInitialLaunch = previousCodeState is CodeState.Idle,
+        isInitialLaunch = (previousCodeState as? CodeState.Idle)?.isInitialLaunch == true,
         onBackPressedDispatcher = onBackPressedDispatcher,
         firstUiConfiguration = uiConfiguration,
       ),
@@ -225,11 +228,19 @@ internal class TreehouseAppContent<A : AppService>(
     stateFlow.value = State(viewState, nextCodeState)
   }
 
+  override fun onUncaughtException(codeSession: CodeSession<A>, exception: Throwable) {
+    codeSessionCanceled(exception = exception)
+  }
+
+  override fun onCancel(codeSession: CodeSession<A>) {
+    codeSessionCanceled(exception = null)
+  }
+
   /**
-   * If the code crashes, show an error on the UI and cancel the UI binding. This sets the code
-   * state back to idle.
+   * If the code crashes or is unloaded, show an error on the UI and cancel the UI binding. This
+   * sets the code state back to idle.
    */
-  override fun onUncaughtException(codeSession: CodeSession<A>, exception: Throwable) {
+  private fun codeSessionCanceled(exception: Throwable?) {
     dispatchers.checkUi()
 
     val previousState = stateFlow.value
@@ -239,24 +250,21 @@ internal class TreehouseAppContent<A : AppService>(
     // This listener should only fire if we're actively running code.
     require(previousCodeState is CodeState.Running)
 
-    // Cancel the UI binding to the crashed code.
+    // Cancel the UI binding to the canceled code.
     val binding = previousCodeState.viewContentCodeBinding
     binding.cancel()
     binding.codeSession.removeListener(this)
 
-    // If there's a UI, give it the error to display.
+    // If there's an error and a UI, show it.
     val view = (viewState as? ViewState.Bound)?.view
-    if (view != null) {
+    if (exception != null && view != null) {
       codeListener.onUncaughtException(view, exception)
     }
 
-    val nextCodeState = CodeState.Idle<A>()
+    val nextCodeState = CodeState.Idle<A>(isInitialLaunch = false)
     stateFlow.value = State(viewState, nextCodeState)
   }
 
-  override fun onCancel(codeSession: CodeSession<A>) {
-  }
-
   /** This function may only be invoked on [TreehouseDispatchers.ui]. */
   private fun startViewCodeContentBinding(
     codeSession: CodeSession<A>,
@@ -270,7 +278,6 @@ internal class TreehouseAppContent<A : AppService>(
     return ViewContentCodeBinding(
       stateStore = codeHost.stateStore,
       dispatchers = dispatchers,
-      appScope = appScope,
       eventPublisher = codeSession.eventPublisher,
       contentSource = source,
       codeListener = codeListener,
@@ -300,7 +307,6 @@ internal class TreehouseAppContent<A : AppService>(
 private class ViewContentCodeBinding<A : AppService>(
   val stateStore: StateStore,
   val dispatchers: TreehouseDispatchers,
-  val appScope: CoroutineScope,
   val eventPublisher: EventPublisher,
   val contentSource: TreehouseContentSource<A>,
   val codeListener: CodeListener,
@@ -313,7 +319,7 @@ private class ViewContentCodeBinding<A : AppService>(
   private val uiConfigurationFlow = SequentialStateFlow(firstUiConfiguration)
 
   private val bindingScope = CoroutineScope(
-    SupervisorJob(appScope.coroutineContext.job) + codeSession.coroutineExceptionHandler,
+    SupervisorJob(codeSession.scope.coroutineContext.job),
   )
 
   /** Only accessed on [TreehouseDispatchers.ui]. Null before [initView] and after [cancel]. */
@@ -486,10 +492,10 @@ private class ViewContentCodeBinding<A : AppService>(
     viewOrNull?.saveCallback = null
     viewOrNull = null
     bridgeOrNull = null
-    appScope.launch(dispatchers.zipline) {
+    bindingScope.launch(dispatchers.zipline) {
       treehouseUiOrNull = null
-      bindingScope.cancel()
       serviceScope.close()
+      bindingScope.cancel()
     }
   }
 }
diff --git a/...ood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/ZiplineCodeSession.kt b/...ood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/ZiplineCodeSession.kt
@@ -23,37 +23,39 @@ import app.cash.zipline.Zipline
 import app.cash.zipline.ZiplineScope
 import app.cash.zipline.withScope
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.job
 import kotlinx.coroutines.launch
 import kotlinx.serialization.json.Json
 
 internal class ZiplineCodeSession<A : AppService>(
   private val dispatchers: TreehouseDispatchers,
-  private val appScope: CoroutineScope,
   override val eventPublisher: EventPublisher,
+  frameClockFactory: FrameClock.Factory,
   override val appService: A,
   val zipline: Zipline,
+  val appScope: CoroutineScope,
 ) : CodeSession<A>, AppLifecycle.Host {
   private val listeners = mutableListOf<Listener<A>>()
   private val ziplineScope = ZiplineScope()
 
+  override val scope = CoroutineScope(
+    SupervisorJob(appScope.coroutineContext.job) + coroutineExceptionHandler,
+  )
+
   override val json: Json
     get() = zipline.json
 
-  // These vars only accessed on TreehouseDispatchers.zipline.
-  private lateinit var sessionScope: CoroutineScope
-  private lateinit var frameClock: FrameClock
+  private val frameClock = frameClockFactory.create(scope, dispatchers)
   private lateinit var appLifecycle: AppLifecycle
 
   private var canceled = false
 
-  override fun start(sessionScope: CoroutineScope, frameClock: FrameClock) {
+  override fun start() {
     dispatchers.checkUi()
 
-    sessionScope.launch(dispatchers.zipline) {
-      this@ZiplineCodeSession.sessionScope = sessionScope
-      this@ZiplineCodeSession.frameClock = frameClock
-
+    scope.launch(dispatchers.zipline) {
       val service = appService.withScope(ziplineScope).appLifecycle
       appLifecycle = service
       service.start(this@ZiplineCodeSession)
@@ -81,10 +83,10 @@ internal class ZiplineCodeSession<A : AppService>(
       listener.onCancel(this)
     }
 
-    appScope.launch(dispatchers.zipline) {
-      sessionScope.cancel()
+    scope.launch(dispatchers.zipline) {
       ziplineScope.close()
       zipline.close()
+      scope.cancel()
     }
   }
 
@@ -107,7 +109,7 @@ internal class ZiplineCodeSession<A : AppService>(
   }
 
   override fun handleUncaughtException(exception: Throwable) {
-    appScope.launch(dispatchers.ui) {
+    scope.launch(dispatchers.ui) {
       val listenersArray = listeners.toTypedArray() // onUncaughtException mutates.
       for (listener in listenersArray) {
         listener.onUncaughtException(this@ZiplineCodeSession, exception)