Merge pull request #1166 from google/ben/webview_state_fix

[Web] Fix url in remember not causing recomposition
google · May 17, 2022 · 8bcaf06 · 8bcaf06
2 parents c816dff + 1884715
commit 8bcaf06
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 13 deletions.
diff --git a/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt b/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt
@@ -39,8 +39,10 @@ import androidx.compose.material.TopAppBar
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.ArrowBack
 import androidx.compose.material.icons.filled.Error
+import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
@@ -49,7 +51,6 @@ import androidx.compose.ui.unit.dp
 import com.google.accompanist.sample.AccompanistSampleTheme
 import com.google.accompanist.web.AccompanistWebViewClient
 import com.google.accompanist.web.LoadingState
-import com.google.accompanist.web.WebContent
 import com.google.accompanist.web.WebView
 import com.google.accompanist.web.rememberWebViewNavigator
 import com.google.accompanist.web.rememberWebViewState
@@ -60,9 +61,10 @@ class BasicWebViewSample : ComponentActivity() {
         super.onCreate(savedInstanceState)
         setContent {
             AccompanistSampleTheme {
-                val state = rememberWebViewState(url = "https://google.com")
+                var url by remember { mutableStateOf("https://google.com") }
+                val state = rememberWebViewState(url = url)
                 val navigator = rememberWebViewNavigator()
-                val (textFieldValue, setTextFieldValue) = remember(state.content) {
+                var textFieldValue by remember(state.content.getCurrentUrl()) {
                     mutableStateOf(state.content.getCurrentUrl() ?: "")
                 }
 
@@ -96,14 +98,14 @@ class BasicWebViewSample : ComponentActivity() {
 
                             OutlinedTextField(
                                 value = textFieldValue,
-                                onValueChange = setTextFieldValue,
+                                onValueChange = { textFieldValue = it },
                                 modifier = Modifier.fillMaxWidth()
                             )
                         }
 
                         Button(
                             onClick = {
-                                state.content = WebContent.Url(textFieldValue)
+                                url = textFieldValue
                             },
                             modifier = Modifier.align(Alignment.CenterVertically)
                         ) {

diff --git a/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt b/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt
@@ -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.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.snapshotFlow
 import androidx.compose.ui.Modifier
@@ -48,6 +49,7 @@ import org.hamcrest.CoreMatchers.containsString
 import org.hamcrest.CoreMatchers.equalTo
 import org.junit.After
 import org.junit.Before
+import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -178,6 +180,55 @@ class WebTest {
             .check(webMatches(getCurrentUrl(), containsString(LINK_URL)))
     }
 
+    @Test
+    fun testUrlUpdatedWithRemember() {
+        val url = mutableStateOf("about:blank")
+        rule.setContent {
+            @Composable
+            fun MyWebView(url: String) {
+                val webViewState = rememberWebViewState(url = url)
+                WebTestContent(webViewState = webViewState, idlingResource = idleResource)
+            }
+
+            MyWebView(url = url.value)
+        }
+
+        // Ensure the data is loaded first
+        onWebView()
+            .check(webMatches(getCurrentUrl(), containsString("about:blank")))
+
+        url.value = LINK_URL
+
+        rule.waitForIdle()
+
+        onWebView()
+            .check(webMatches(getCurrentUrl(), containsString(LINK_URL)))
+    }
+
+    @Test
+    fun testDataUpdatedWithRemember() {
+        val data = mutableStateOf(TEST_DATA)
+        rule.setContent {
+            @Composable
+            fun MyWebView(data: String) {
+                val webViewState = rememberWebViewStateWithHTMLData(data = data)
+                WebTestContent(webViewState = webViewState, idlingResource = idleResource)
+            }
+
+            MyWebView(data = data.value)
+        }
+
+        // Ensure the data is loaded first
+        onWebView()
+            .withElement(findElement(Locator.TAG_NAME, "a"))
+            .check(webMatches(getText(), containsString(LINK_TEXT)))
+
+        data.value = TEST_TITLE_DATA
+
+        onWebView()
+            .check(webMatches(getTitle(), containsString(TITLE_TEXT)))
+    }
+
     @Test
     fun testCustomClientIsAssigned() {
         lateinit var state: WebViewState
@@ -510,6 +561,7 @@ class WebTest {
     }
 
     @FlakyTest
+    @Ignore("Slow and flaky: https://github.com/google/accompanist/issues/1085")
     @Test
     fun testAdditionalHttpHeaders() {
         val mockServer = MockWebServer()

diff --git a/web/src/main/java/com/google/accompanist/web/WebView.kt b/web/src/main/java/com/google/accompanist/web/WebView.kt
@@ -80,6 +80,13 @@ fun WebView(
         with(navigator) { webView?.handleNavigationEvents() }
     }
 
+    // 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
+    client.state = state
+    client.navigator = navigator
+    chromeClient.state = state
+
     AndroidView(
         factory = { context ->
             WebView(context).apply {
@@ -90,13 +97,6 @@ fun WebView(
                     ViewGroup.LayoutParams.MATCH_PARENT
                 )
 
-                // 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
-                client.state = state
-                client.navigator = navigator
-                chromeClient.state = state
-
                 webChromeClient = chromeClient
                 webViewClient = client
             }.also { webView = it }
@@ -410,6 +410,8 @@ data class WebViewError(
  */
 @Composable
 fun rememberWebViewState(url: String, additionalHttpHeaders: Map<String, String> = emptyMap()) =
+    // Rather than using .apply {} here we will recreate the state, this prevents
+    // a recomposition loop when the webview updates the url itself.
     remember(url, additionalHttpHeaders) {
         WebViewState(
             WebContent.Url(
@@ -426,4 +428,6 @@ fun rememberWebViewState(url: String, additionalHttpHeaders: Map<String, String>
  */
 @Composable
 fun rememberWebViewStateWithHTMLData(data: String, baseUrl: String? = null) =
-    remember(data, baseUrl) { WebViewState(WebContent.Data(data, baseUrl)) }
+    remember(data, baseUrl) {
+        WebViewState(WebContent.Data(data, baseUrl))
+    }