Skip to content

Properly dispose of web ComposeWindow on removing canvas from DOM #2126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: jb-main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ import org.w3c.dom.HTMLElement
import org.w3c.dom.HTMLStyleElement
import org.w3c.dom.HTMLTitleElement
import org.w3c.dom.MediaQueryListEvent
import org.w3c.dom.MutationObserver
import org.w3c.dom.MutationObserverInit
import org.w3c.dom.OPEN
import org.w3c.dom.ShadowRoot
import org.w3c.dom.ShadowRootInit
import org.w3c.dom.ShadowRootMode
import org.w3c.dom.TouchEvent
Expand Down Expand Up @@ -203,6 +206,14 @@ internal class ComposeWindow(

private val canvasEvents = EventTargetListener(canvas)

private val detachListener = MutationObserver { _, _ ->
val root = canvas.getRootNode()
val queryElement = if (root is ShadowRoot) root.host else canvas
if (!document.body!!.contains(queryElement)) {
dispose()
}
}

private var keyboardModeState: KeyboardModeState = KeyboardModeState.Hardware

private val platformContext: PlatformContext = object : PlatformContext by PlatformContext.Empty {
Expand Down Expand Up @@ -407,6 +418,8 @@ internal class ComposeWindow(
else Lifecycle.Event.ON_STOP
)
}

detachListener.observe(document.body!!, MutationObserverInit(childList = true, subtree = true))
}

init {
Expand Down Expand Up @@ -472,9 +485,11 @@ internal class ComposeWindow(
skiaLayer.needRedraw()
}

// TODO: need to call .dispose() on window close.
fun dispose() {
check(!isDisposed)

detachListener.disconnect()

lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
viewModelStore.clear()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,53 +21,46 @@ import androidx.compose.ui.sendFromScope
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleEventObserver
import androidx.lifecycle.LifecycleOwner
import kotlin.test.Ignore
import androidx.lifecycle.compose.LocalLifecycleOwner
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import kotlinx.browser.document
import kotlinx.browser.window
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.test.runTest
import org.w3c.dom.HTMLDivElement
import org.w3c.dom.events.Event

class ComposeWindowLifecycleTest : OnCanvasTests {
@Test
@Ignore // ignored while investigating CI issues: this test opens a new browser window which can be the cause
fun allEvents() = runTest {
val canvas = getCanvas()
canvas.focus()

val lifecycleOwner = ComposeWindow(
canvas = canvas,
interopContainerElement = document.createElement("div") as HTMLDivElement,
a11yContainerElement = null,
content = {},
configuration = ComposeViewportConfiguration(),
state = DefaultWindowState(document.documentElement!!)
)

val eventsChannel = Channel<Lifecycle.Event>(10)

lifecycleOwner.lifecycle.addObserver(object : LifecycleEventObserver {
override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) {
eventsChannel.sendFromScope(event)
}
})
createComposeWindow {
val lifecycle = LocalLifecycleOwner.current.lifecycle
lifecycle.addObserver(object : LifecycleEventObserver {
override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) {
eventsChannel.sendFromScope(event)
}
})
}

assertEquals(Lifecycle.State.CREATED, eventsChannel.receive().targetState)
assertEquals(Lifecycle.State.STARTED, eventsChannel.receive().targetState)
assertEquals(Lifecycle.State.RESUMED, eventsChannel.receive().targetState)

// Browsers don't allow to blur the window from code:
// https://developer.mozilla.org/en-US/docs/Web/API/Window/blur
// So we simulate a new tab being open:
val anotherWindow = window.open("about:config")
assertTrue(anotherWindow != null)
// Dispatch artificial events that would be sent when the window gains and loses focus.
// Starting with a focus event before checking for the initial RESUMED makes this test
// robust in the face of both an already-focused window and a non-focused window. Then,
// a blur plus focus cycle simulates losing focus and regaining it.
window.dispatchEvent(Event("focus"))
assertEquals(Lifecycle.State.RESUMED, eventsChannel.receive().targetState)
window.dispatchEvent(Event("blur"))
assertEquals(Lifecycle.State.STARTED, eventsChannel.receive().targetState)

// Now go back to the original window
anotherWindow.close()
window.dispatchEvent(Event("focus"))
assertEquals(Lifecycle.State.RESUMED, eventsChannel.receive().targetState)

// Destroy the ComposeWindow by removing its host container from the DOM.
val host = getShadowRoot().host
host.parentNode?.removeChild(host)
assertEquals(Lifecycle.State.STARTED, eventsChannel.receive().targetState)
assertEquals(Lifecycle.State.CREATED, eventsChannel.receive().targetState)
assertEquals(Lifecycle.State.DESTROYED, eventsChannel.receive().targetState)
}
}