Skip to content

Commit

Permalink
Scope the FrameClock to a CodeSession
Browse files Browse the repository at this point in the history
  • Loading branch information
squarejesse committed Oct 30, 2023
1 parent 2d69dce commit 621c4fe
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import kotlinx.coroutines.runBlocking

class AndroidChoreographerFrameClockTest : AbstractFrameClockTest() {
// Tests run on a background thread but Choreographer can only be grabbed from the main thread.
override val frameClock = runBlocking(Dispatchers.Main) {
AndroidChoreographerFrameClock()
override val frameClockFactory = runBlocking(Dispatchers.Main) {
AndroidChoreographerFrameClock.Factory()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,11 @@ import kotlinx.coroutines.launch
/**
* A [FrameClock] that sends frames using [Choreographer].
*/
internal class AndroidChoreographerFrameClock : FrameClock {
private val choreographer = Choreographer.getInstance()
private lateinit var scope: CoroutineScope
private lateinit var dispatchers: TreehouseDispatchers

override fun start(
scope: CoroutineScope,
dispatchers: TreehouseDispatchers,
) {
this.scope = scope
this.dispatchers = dispatchers
}
internal class AndroidChoreographerFrameClock private constructor(
private val choreographer: Choreographer,
private val scope: CoroutineScope,
private val dispatchers: TreehouseDispatchers,
) : FrameClock {

override fun requestFrame(appLifecycle: AppLifecycle) {
choreographer.postFrameCallback { frameTimeNanos ->
Expand All @@ -42,4 +35,12 @@ internal class AndroidChoreographerFrameClock : FrameClock {
}
}
}

class Factory : FrameClock.Factory {
private val choreographer = Choreographer.getInstance()
override fun create(
scope: CoroutineScope,
dispatchers: TreehouseDispatchers,
) = AndroidChoreographerFrameClock(choreographer, scope, dispatchers)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public fun TreehouseAppFactory(
dispatchers = AndroidTreehouseDispatchers(),
eventListener = eventListener,
httpClient = httpClient.asZiplineHttpClient(),
frameClock = AndroidChoreographerFrameClock(),
frameClockFactory = AndroidChoreographerFrameClock.Factory(),
manifestVerifier = manifestVerifier,
embeddedDir = embeddedDir,
embeddedFileSystem = embeddedFileSystem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal interface CodeSession<A : AppService> {

val json: Json

fun start(sessionScope: CoroutineScope)
fun start(sessionScope: CoroutineScope, frameClock: FrameClock)

fun addListener(listener: Listener<A>)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,16 @@ package app.cash.redwood.treehouse
import kotlinx.coroutines.CoroutineScope

internal interface FrameClock {
/** Run this clock until [scope] is canceled. */
fun start(
scope: CoroutineScope,
dispatchers: TreehouseDispatchers,
)

/**
* Request a call to [AppLifecycle.sendFrame]. It is an error to call [requestFrame] again before
* that call is made.
*
* It is an error to call this before [start].
*/
fun requestFrame(appLifecycle: AppLifecycle)

interface Factory {
/** Creates a new FrameClock that sends frames on [dispatchers] in [scope]. */
fun create(scope: CoroutineScope, dispatchers: TreehouseDispatchers): FrameClock
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ public class TreehouseApp<A : AppService> private constructor(
dispatchers = dispatchers,
eventPublisher = eventPublisher,
appScope = appScope,
frameClock = factory.frameClock,
appService = appService,
zipline = zipline,
)
Expand All @@ -228,7 +227,10 @@ public class TreehouseApp<A : AppService> private constructor(

session = next
next.addListener(this@ZiplineCodeHost)
next.start(sessionScope)
next.start(
sessionScope = sessionScope,
frameClock = factory.frameClockFactory.create(sessionScope, dispatchers),
)

for (listener in listeners) {
listener.codeSessionChanged(next)
Expand All @@ -249,7 +251,7 @@ public class TreehouseApp<A : AppService> private constructor(
public val dispatchers: TreehouseDispatchers,
internal val eventListener: EventListener,
internal val httpClient: ZiplineHttpClient,
internal val frameClock: FrameClock,
internal val frameClockFactory: FrameClock.Factory,
internal val manifestVerifier: ManifestVerifier,
internal val embeddedDir: Path?,
internal val embeddedFileSystem: FileSystem?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ internal class ZiplineCodeSession<A : AppService>(
private val dispatchers: TreehouseDispatchers,
private val eventPublisher: EventPublisher,
private val appScope: CoroutineScope,
private val frameClock: FrameClock,
override val appService: A,
val zipline: Zipline,
) : CodeSession<A>, AppLifecycle.Host {
Expand All @@ -41,20 +40,19 @@ internal class ZiplineCodeSession<A : AppService>(
override val json: Json
get() = zipline.json

/** Only accessed on [TreehouseDispatchers.zipline]. */
// These vars only accessed on TreehouseDispatchers.zipline.
private lateinit var sessionScope: CoroutineScope

/** Only accessed on [TreehouseDispatchers.zipline]. */
private lateinit var frameClock: FrameClock
private lateinit var appLifecycle: AppLifecycle

private var canceled = false

override fun start(sessionScope: CoroutineScope) {
override fun start(sessionScope: CoroutineScope, frameClock: FrameClock) {
dispatchers.checkUi()

sessionScope.launch(dispatchers.zipline) {
this@ZiplineCodeSession.sessionScope = sessionScope

frameClock.start(sessionScope, dispatchers)
this@ZiplineCodeSession.frameClock = frameClock

val service = appService.withScope(ziplineScope).appLifecycle
appLifecycle = service
Expand All @@ -73,11 +71,11 @@ internal class ZiplineCodeSession<A : AppService>(
}

override fun cancel() {
dispatchers.checkUi()

if (canceled) return
canceled = true

dispatchers.checkUi()

val listenersArray = listeners.toTypedArray() // onCancel mutates.
for (listener in listenersArray) {
listener.onCancel(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import kotlinx.coroutines.test.runTest

@OptIn(ExperimentalCoroutinesApi::class)
abstract class AbstractFrameClockTest {
internal abstract val frameClock: FrameClock
internal abstract val frameClockFactory: FrameClock.Factory

@Test fun ticksWithTime() = runTest {
val dispatchers = object : TreehouseDispatchers {
Expand All @@ -38,7 +38,7 @@ abstract class AbstractFrameClockTest {
override fun checkZipline() {}
override fun close() {}
}
frameClock.start(this, dispatchers)
val frameClock = frameClockFactory.create(this, dispatchers)

val frameTimes = Channel<Long>(Channel.UNLIMITED)
val appLifecycle = object : AppLifecycle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ internal class FakeCodeHost(
previous?.cancel()

if (value != null) {
value.start(CoroutineScope(EmptyCoroutineContext))
value.start(CoroutineScope(EmptyCoroutineContext), FakeFrameClock())
for (listener in listeners) {
listener.codeSessionChanged(value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class FakeCodeSession(

private var canceled = false

override fun start(sessionScope: CoroutineScope) {
override fun start(sessionScope: CoroutineScope, frameClock: FrameClock) {
eventLog += "$name.start()"
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright (C) 2023 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

class FakeFrameClock : FrameClock {
override fun requestFrame(appLifecycle: AppLifecycle) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,19 @@ import kotlinx.coroutines.launch
import platform.posix.CLOCK_MONOTONIC_RAW
import platform.posix.clock_gettime_nsec_np

internal class IosDisplayLinkClock : FrameClock {
private lateinit var scope: CoroutineScope
private lateinit var dispatchers: TreehouseDispatchers
private lateinit var displayLinkTarget: DisplayLinkTarget

internal class IosDisplayLinkClock private constructor(
private val scope: CoroutineScope,
private val dispatchers: TreehouseDispatchers,
) : FrameClock {
/** Non-null if we're expecting a call to [AppLifecycle.sendFrame]. */
private var appLifecycle: AppLifecycle? = null

override fun start(scope: CoroutineScope, dispatchers: TreehouseDispatchers) {
this.scope = scope
this.dispatchers = dispatchers
this.displayLinkTarget = DisplayLinkTarget {
unsubscribe()
scope.launch(dispatchers.zipline) {
val nanos = clock_gettime_nsec_np(CLOCK_MONOTONIC_RAW.convert()).convert<Long>()
appLifecycle?.sendFrame(nanos)
appLifecycle = null
}
private val displayLinkTarget = DisplayLinkTarget {
unsubscribe()
scope.launch(dispatchers.zipline) {
val nanos = clock_gettime_nsec_np(CLOCK_MONOTONIC_RAW.convert()).convert<Long>()
appLifecycle?.sendFrame(nanos)
appLifecycle = null
}
}

Expand All @@ -48,4 +43,11 @@ internal class IosDisplayLinkClock : FrameClock {
displayLinkTarget.subscribe()
}
}

companion object : FrameClock.Factory {
override fun create(
scope: CoroutineScope,
dispatchers: TreehouseDispatchers,
) = IosDisplayLinkClock(scope, dispatchers)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public fun TreehouseAppFactory(
dispatchers = IosTreehouseDispatchers(),
eventListener = eventListener,
httpClient = httpClient,
frameClock = IosDisplayLinkClock(),
frameClockFactory = IosDisplayLinkClock,
manifestVerifier = manifestVerifier,
embeddedDir = embeddedDir,
embeddedFileSystem = embeddedFileSystem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ import kotlin.test.Ignore

@Ignore // TODO Does not work without linking XCTest and showing a XCUIApplication.
class IosDisplayLinkClockTest : AbstractFrameClockTest() {
override val frameClock = IosDisplayLinkClock()
override val frameClockFactory: FrameClock.Factory
get() = IosDisplayLinkClock
}

0 comments on commit 621c4fe

Please sign in to comment.