Cleanup after PR review.

docs/oidc.md

@@ -1,3 +1,5 @@
+This file contains some rough notes about Oidc implementation, with some examples of actual data.
+
 [ios implementation](https://github.com/vector-im/element-x-ios/compare/develop...doug/oidc-temp)
 
 Rust sdk branch: https://github.com/matrix-org/matrix-rust-sdk/tree/oidc-ffi

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/oidc/webview/OidcView.kt

@@ -21,7 +21,10 @@ import androidx.activity.compose.BackHandler
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.statusBarsPadding
 import androidx.compose.runtime.Composable
+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.tooling.preview.Preview
@@ -42,7 +45,7 @@ fun OidcView(
     modifier: Modifier = Modifier,
 ) {
     val oidcUrlParser = remember { OidcUrlParser() }
-    var webView: WebView? = null
+    var webView by remember { mutableStateOf<WebView?>(null) }
     fun shouldOverrideUrl(url: String): Boolean {
         val action = oidcUrlParser.parse(url)
         if (action != null) {
@@ -53,11 +56,7 @@ fun OidcView(
     }
 
     val oidcWebViewClient = remember {
-        OidcWebViewClient(eventListener = object : WebViewEventListener {
-            override fun shouldOverrideUrlLoading(url: String): Boolean {
-                return shouldOverrideUrl(url)
-            }
-        })
+        OidcWebViewClient(::shouldOverrideUrl)
     }
 
     BackHandler {
@@ -71,8 +70,6 @@ fun OidcView(
 
     Box(modifier = modifier.statusBarsPadding()) {
         AndroidView(
-            modifier = Modifier
-                .statusBarsPadding(),
             factory = { context ->
                 WebView(context).apply {
                     webViewClient = oidcWebViewClient

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/oidc/webview/OidcWebViewClient.kt

@@ -22,9 +22,10 @@ import android.os.Build
 import android.webkit.WebResourceRequest
 import android.webkit.WebView
 import android.webkit.WebViewClient
-import timber.log.Timber
 
-class OidcWebViewClient(private val eventListener: WebViewEventListener) : WebViewClient() {
+class OidcWebViewClient(
+    private val eventListener: WebViewEventListener,
+) : WebViewClient() {
     // We will revert to API 23, in the mean time ignore the warning here.
     @SuppressLint("ObsoleteSdkInt")
     @TargetApi(Build.VERSION_CODES.N)
@@ -38,7 +39,7 @@ class OidcWebViewClient(private val eventListener: WebViewEventListener) : WebVi
     }
 
     private fun shouldOverrideUrl(url: String): Boolean {
-        Timber.d("shouldOverrideUrl: $url")
+        // Timber.d("shouldOverrideUrl: $url")
         return eventListener.shouldOverrideUrlLoading(url)
     }
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/oidc/webview/WebViewEventListener.kt

@@ -16,14 +16,12 @@
 
 package io.element.android.features.login.impl.oidc.webview
 
-interface WebViewEventListener {
+fun interface WebViewEventListener {
     /**
      * Triggered when a Webview loads an url.
      *
      * @param url The url about to be rendered.
      * @return true if the method needs to manage some custom handling
      */
-    fun shouldOverrideUrlLoading(url: String): Boolean {
-        return false
-    }
+    fun shouldOverrideUrlLoading(url: String): Boolean
 }

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/FakeAuthenticationService.kt

@@ -22,6 +22,7 @@ import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
 import io.element.android.libraries.matrix.api.auth.OidcDetails
 import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.test.A_USER_ID
+import io.element.android.libraries.matrix.test.FAKE_DELAY_IN_MS
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -58,12 +59,12 @@ class FakeAuthenticationService : MatrixAuthenticationService {
     }
 
     override suspend fun setHomeserver(homeserver: String): Result<Unit> {
-        delay(100)
+        delay(FAKE_DELAY_IN_MS)
         return changeServerError?.let { Result.failure(it) } ?: Result.success(Unit)
     }
 
     override suspend fun login(username: String, password: String): Result<SessionId> {
-        delay(100)
+        delay(FAKE_DELAY_IN_MS)
         return loginError?.let { Result.failure(it) } ?: Result.success(A_USER_ID)
     }
 
@@ -76,7 +77,7 @@ class FakeAuthenticationService : MatrixAuthenticationService {
     }
 
     override suspend fun loginWithOidc(callbackUrl: String): Result<SessionId> {
-        delay(100)
+        delay(FAKE_DELAY_IN_MS)
         return loginError?.let { Result.failure(it) } ?: Result.success(A_USER_ID)
     }