Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Web] Fix webview not destroyed #1221

Merged
merged 4 commits into from Jul 4, 2022
Merged

[Web] Fix webview not destroyed #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2985c8c
Add onDispose param
rlatapy-luna Jun 30, 2022
4d20e8a
Add minimal test testOnDisposeCalled
rlatapy-luna Jul 1, 2022
fc358b6
Update API
rlatapy-luna Jul 1, 2022
a3cc01f
Remove default destroy call
rlatapy-luna Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/api/current.api
View file
Open in desktop
Expand Up @@ -75,7 +75,7 @@ package com.google.accompanist.web {
}

public final class WebViewKt {
method @androidx.compose.runtime.Composable public static void WebView(com.google.accompanist.web.WebViewState state, optional androidx.compose.ui.Modifier modifier, optional boolean captureBackPresses, optional com.google.accompanist.web.WebViewNavigator navigator, optional kotlin.jvm.functions.Function1<? super android.webkit.WebView,kotlin.Unit> onCreated, optional com.google.accompanist.web.AccompanistWebViewClient client, optional com.google.accompanist.web.AccompanistWebChromeClient chromeClient);
method @androidx.compose.runtime.Composable public static void WebView(com.google.accompanist.web.WebViewState state, optional androidx.compose.ui.Modifier modifier, optional boolean captureBackPresses, optional com.google.accompanist.web.WebViewNavigator navigator, optional kotlin.jvm.functions.Function1<? super android.webkit.WebView,kotlin.Unit> onCreated, optional kotlin.jvm.functions.Function1<? super android.webkit.WebView,kotlin.Unit> onDispose, optional com.google.accompanist.web.AccompanistWebViewClient client, optional com.google.accompanist.web.AccompanistWebChromeClient chromeClient);
method @androidx.compose.runtime.Composable public static com.google.accompanist.web.WebViewNavigator rememberWebViewNavigator(optional kotlinx.coroutines.CoroutineScope coroutineScope);
method @androidx.compose.runtime.Composable public static com.google.accompanist.web.WebViewState rememberWebViewState(String url, optional java.util.Map<java.lang.String,java.lang.String> additionalHttpHeaders);
method @androidx.compose.runtime.Composable public static com.google.accompanist.web.WebViewState rememberWebViewStateWithHTMLData(String data, optional String? baseUrl);
Expand Down
26 changes: 26 additions & 0 deletions web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@ import android.webkit.WebView
import androidx.compose.material.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshotFlow
Expand Down Expand Up @@ -593,6 +594,29 @@ class WebTest {
mockServer.shutdown()
}

@Test
fun testOnDisposeCalled() {
lateinit var state: WebViewState
lateinit var toggle: MutableState<Boolean>
var isOnDisposeCalled = false

rule.setContent {
toggle = remember { mutableStateOf(true) }
if (toggle.value) {
state = rememberWebViewStateWithHTMLData(data = TEST_DATA)
WebTestContent(
state,
idleResource,
onDispose = { isOnDisposeCalled = true },
)
}
}

toggle.value = false
webNode.assertDoesNotExist()
assertThat(isOnDisposeCalled).isTrue()
}

private val webNode: SemanticsNodeInteraction
get() = rule.onNodeWithTag(WebViewTag)
}
Expand All @@ -614,6 +638,7 @@ private fun WebTestContent(
webViewState: WebViewState,
idlingResource: WebViewIdlingResource,
navigator: WebViewNavigator = rememberWebViewNavigator(),
onDispose: (WebView) -> Unit = {},
client: AccompanistWebViewClient = remember { AccompanistWebViewClient() },
chromeClient: AccompanistWebChromeClient = remember { AccompanistWebChromeClient() }
) {
Expand All @@ -625,6 +650,7 @@ private fun WebTestContent(
modifier = Modifier.testTag(WebViewTag),
navigator = navigator,
onCreated = { it.settings.javaScriptEnabled = true },
onDispose = onDispose,
client = client,
chromeClient = chromeClient
)
Expand Down
11 changes: 11 additions & 0 deletions web/src/main/java/com/google/accompanist/web/WebView.kt
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import android.webkit.WebView
import android.webkit.WebViewClient
import androidx.activity.compose.BackHandler
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.Immutable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.Stable
Expand All @@ -33,6 +34,7 @@ import androidx.compose.runtime.mutableStateListOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalInspectionMode
Expand Down Expand Up @@ -68,6 +70,7 @@ fun WebView(
captureBackPresses: Boolean = true,
navigator: WebViewNavigator = rememberWebViewNavigator(),
onCreated: (WebView) -> Unit = {},
onDispose: (WebView) -> Unit = {},
client: AccompanistWebViewClient = remember { AccompanistWebViewClient() },
chromeClient: AccompanistWebChromeClient = remember { AccompanistWebChromeClient() }
) {
Expand All @@ -81,6 +84,14 @@ fun WebView(
with(navigator) { webView?.handleNavigationEvents() }
}

val currentOnDispose by rememberUpdatedState(onDispose)

webView?.let { it ->
DisposableEffect(it) {
onDispose { currentOnDispose(it) }
}
}

// Set the state of the client and chrome client
// This is done internally to ensure they always are the same instance as the
// parent Web composable
Expand Down