Improve thread safety

Author: CrisBarreiro

app/src/main/java/com/duckduckgo/app/browser/RealWebViewCompatWrapper.kt

@@ -44,7 +44,7 @@ class RealWebViewCompatWrapper @Inject constructor(
     private val webViewCapabilityChecker: WebViewCapabilityChecker,
 ) : WebViewCompatWrapper {
     override suspend fun addDocumentStartJavaScript(
-        webView: android.webkit.WebView,
+        webView: WebView,
         script: String,
         allowedOriginRules: Set<String>,
     ): ScriptHandler? {
@@ -65,19 +65,23 @@ class RealWebViewCompatWrapper @Inject constructor(
         }
     }
 
-    override fun removeWebMessageListener(webView: WebView, jsObjectName: String) {
-        WebViewCompat.removeWebMessageListener(
-            webView,
-            jsObjectName,
-        )
+    override suspend fun removeWebMessageListener(webView: WebView, jsObjectName: String) {
+        withContext(dispatcherProvider.main()) {
+            WebViewCompat.removeWebMessageListener(
+                webView,
+                jsObjectName,
+            )
+        }
     }
 
-    override fun addWebMessageListener(
+    override suspend fun addWebMessageListener(
         webView: WebView,
         jsObjectName: String,
         allowedOriginRules: Set<String>,
         listener: WebViewCompat.WebMessageListener,
     ) {
-        return WebViewCompat.addWebMessageListener(webView, jsObjectName, allowedOriginRules, listener)
+        return withContext(dispatcherProvider.main()) {
+            WebViewCompat.addWebMessageListener(webView, jsObjectName, allowedOriginRules, listener)
+        }
     }
 }

browser-api/src/main/java/com/duckduckgo/browser/api/webviewcompat/WebViewCompatWrapper.kt

@@ -28,12 +28,12 @@ interface WebViewCompatWrapper {
         allowedOriginRules: Set<String>,
     ): ScriptHandler?
 
-    fun removeWebMessageListener(
+    suspend fun removeWebMessageListener(
         webView: WebView,
         jsObjectName: String,
     )
 
-    fun addWebMessageListener(
+    suspend fun addWebMessageListener(
         webView: WebView,
         jsObjectName: String,
         allowedOriginRules: Set<String>,

content-scope-scripts/content-scope-scripts-impl/src/main/java/com/duckduckgo/contentscopescripts/impl/messaging/WebViewCompatWebCompatMessagingPlugin.kt

@@ -18,7 +18,6 @@ package com.duckduckgo.contentscopescripts.impl.messaging
 
 import androidx.annotation.VisibleForTesting
 import androidx.webkit.WebViewCompat.WebMessageListener
-import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.common.utils.plugins.PluginPoint
 import com.duckduckgo.contentscopescripts.api.WebViewCompatContentScopeJsMessageHandlersPlugin
 import com.duckduckgo.contentscopescripts.impl.WebViewCompatContentScopeScripts
@@ -29,7 +28,6 @@ import com.duckduckgo.js.messaging.api.WebMessagingPlugin
 import com.squareup.anvil.annotations.ContributesMultibinding
 import com.squareup.moshi.Moshi
 import javax.inject.Inject
-import kotlinx.coroutines.withContext
 import logcat.LogPriority.ERROR
 import logcat.asLog
 import logcat.logcat
@@ -41,7 +39,6 @@ class WebViewCompatWebCompatMessagingPlugin @Inject constructor(
     private val handlers: PluginPoint<WebViewCompatContentScopeJsMessageHandlersPlugin>,
     private val globalHandlers: PluginPoint<GlobalContentScopeJsMessageHandlersPlugin>,
     private val webViewCompatContentScopeScripts: WebViewCompatContentScopeScripts,
-    private val dispatcherProvider: DispatcherProvider,
 ) : WebMessagingPlugin {
 
     private val moshi = Moshi.Builder().add(JSONObjectAdapter()).build()
@@ -83,7 +80,7 @@ class WebViewCompatWebCompatMessagingPlugin @Inject constructor(
         jsMessageCallback: JsMessageCallback?,
         registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
     ) {
-        if (withContext(dispatcherProvider.io()) { !webViewCompatContentScopeScripts.isEnabled() }) return
+        if (!webViewCompatContentScopeScripts.isEnabled()) return
         if (jsMessageCallback == null) throw Exception("Callback cannot be null")
 
         runCatching {
@@ -107,13 +104,11 @@ class WebViewCompatWebCompatMessagingPlugin @Inject constructor(
         unregisterer: suspend (objectName: String) -> Boolean,
     ) {
         if (!webViewCompatContentScopeScripts.isEnabled()) return
-        withContext(dispatcherProvider.main()) {
-            runCatching {
-                return@runCatching unregisterer(JS_OBJECT_NAME)
-            }.getOrElse { exception ->
-                logcat(ERROR) {
-                    "Error removing WebMessageListener for contentScopeAdsjs: ${exception.asLog()}"
-                }
+        runCatching {
+            return@runCatching unregisterer(JS_OBJECT_NAME)
+        }.getOrElse { exception ->
+            logcat(ERROR) {
+                "Error removing WebMessageListener for contentScopeAdsjs: ${exception.asLog()}"
             }
         }
     }

content-scope-scripts/content-scope-scripts-impl/src/test/java/com/duckduckgo/contentscopescripts/impl/messaging/WebViewCompatWebCompatMessagingPluginTest.kt

@@ -18,7 +18,6 @@ package com.duckduckgo.contentscopescripts.impl.messaging
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.webkit.WebViewCompat.WebMessageListener
-import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.common.utils.plugins.PluginPoint
 import com.duckduckgo.contentscopescripts.api.WebViewCompatContentScopeJsMessageHandlersPlugin
 import com.duckduckgo.contentscopescripts.impl.WebViewCompatContentScopeScripts
@@ -30,15 +29,13 @@ import junit.framework.TestCase.assertNull
 import kotlinx.coroutines.test.runTest
 import org.json.JSONObject
 import org.junit.Before
-import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
 
 @RunWith(AndroidJUnit4::class)
 class WebViewCompatWebCompatMessagingPluginTest {
-    @get:Rule var coroutineRule = CoroutineTestRule()
 
     private val webViewCompatContentScopeScripts: WebViewCompatContentScopeScripts = mock()
     private val handlers: PluginPoint<WebViewCompatContentScopeJsMessageHandlersPlugin> = FakePluginPoint()
@@ -97,7 +94,6 @@ class WebViewCompatWebCompatMessagingPluginTest {
             handlers = handlers,
             globalHandlers = globalHandlers,
             webViewCompatContentScopeScripts = webViewCompatContentScopeScripts,
-            coroutineRule.testDispatcherProvider,
         )
     }