Skip to content

Commit 2f4b48b

Browse files
committed
Simplify APIs
1 parent f9f0ce9 commit 2f4b48b

File tree

11 files changed

+112
-107
lines changed

11 files changed

+112
-107
lines changed

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import androidx.core.net.toUri
3939
import androidx.test.annotation.UiThreadTest
4040
import androidx.test.filters.SdkSuppress
4141
import androidx.test.platform.app.InstrumentationRegistry
42-
import androidx.webkit.ScriptHandler
4342
import com.duckduckgo.adclick.api.AdClickManager
4443
import com.duckduckgo.anrs.api.CrashLogger
4544
import com.duckduckgo.anrs.api.CrashLogger.Crash
@@ -1295,9 +1294,9 @@ class BrowserWebViewClientTest {
12951294
var countInitted = 0
12961295
private set
12971296

1298-
override suspend fun configureAddDocumentStartJavaScript(
1297+
override fun addDocumentStartJavaScript(
12991298
activeExperiments: List<Toggle>,
1300-
scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler?,
1299+
webView: WebView,
13011300
) {
13021301
countInitted++
13031302
}

app/src/androidTest/java/com/duckduckgo/app/browser/DuckDuckGoWebViewTest.kt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
2121
import androidx.test.platform.app.InstrumentationRegistry
2222
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
2323
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability.DocumentStartJavaScript
24-
import com.duckduckgo.contentscopescripts.impl.WebViewCompatWrapper
24+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
25+
import com.duckduckgo.common.test.CoroutineTestRule
2526
import kotlinx.coroutines.test.runTest
2627
import org.junit.Assert.assertFalse
2728
import org.junit.Before
29+
import org.junit.Rule
2830
import org.junit.Test
2931
import org.junit.runner.RunWith
3032
import org.mockito.Mockito.mock
@@ -35,6 +37,9 @@ import org.mockito.kotlin.whenever
3537
@RunWith(AndroidJUnit4::class)
3638
class DuckDuckGoWebViewTest {
3739

40+
@get:Rule
41+
val coroutineRule = CoroutineTestRule()
42+
3843
private lateinit var testee: DuckDuckGoWebView
3944
private val mockWebViewCapabilityChecker: WebViewCapabilityChecker = mock()
4045
private val mockWebViewCompatWrapper: WebViewCompatWrapper = mock()
@@ -44,8 +49,7 @@ class DuckDuckGoWebViewTest {
4449
fun setUp() {
4550
val context = InstrumentationRegistry.getInstrumentation().targetContext
4651
testee = DuckDuckGoWebView(context)
47-
testee.webViewCompatWrapper = mockWebViewCompatWrapper
48-
testee.webViewCapabilityChecker = mockWebViewCapabilityChecker
52+
testee.dispatcherProvider = coroutineRule.testDispatcherProvider
4953
}
5054

5155
@Test

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,7 @@ class BrowserWebViewClient @Inject constructor(
469469
appCoroutineScope.launch {
470470
val activeExperiments = contentScopeExperiments.getActiveExperiments()
471471
addDocumentStartJavascriptPlugins.getPlugins().forEach { plugin ->
472-
plugin.configureAddDocumentStartJavaScript(activeExperiments) { scriptString, allowedOrigins ->
473-
webView.safeAddDocumentStartJavaScript(scriptString, allowedOrigins)
474-
}
472+
plugin.addDocumentStartJavaScript(activeExperiments, webView)
475473
}
476474
}
477475
}
@@ -493,11 +491,10 @@ class BrowserWebViewClient @Inject constructor(
493491
(webView as? DuckDuckGoWebView)?.let { duckDuckGoWebView ->
494492
val activeExperiments = webViewClientListener?.getSite()?.activeContentScopeExperiments ?: listOf()
495493
addDocumentStartJavascriptPlugins.getPlugins().forEach {
496-
it.configureAddDocumentStartJavaScript(
494+
it.addDocumentStartJavaScript(
497495
activeExperiments,
498-
) { scriptString, allowedOrigins ->
499-
(webView as? DuckDuckGoWebView)?.safeAddDocumentStartJavaScript(scriptString, allowedOrigins)
500-
}
496+
webView,
497+
)
501498
}
502499
}
503500
}

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

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,11 @@ import com.duckduckgo.anvil.annotations.InjectWith
4040
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
4141
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability
4242
import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
43-
import com.duckduckgo.contentscopescripts.impl.WebViewCompatWrapper
43+
import com.duckduckgo.common.utils.DispatcherProvider
4444
import com.duckduckgo.di.scopes.ViewScope
4545
import dagger.android.support.AndroidSupportInjection
4646
import javax.inject.Inject
47+
import kotlinx.coroutines.withContext
4748
import logcat.LogPriority.ERROR
4849
import logcat.asLog
4950
import logcat.logcat
@@ -75,10 +76,7 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
7576
var isSafeWebViewEnabled: Boolean = false
7677

7778
@Inject
78-
lateinit var webViewCompatWrapper: WebViewCompatWrapper
79-
80-
@Inject
81-
lateinit var webViewCapabilityChecker: WebViewCapabilityChecker
79+
lateinit var dispatcherProvider: DispatcherProvider
8280

8381
constructor(context: Context) : this(context, null)
8482
constructor(
@@ -432,6 +430,7 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
432430

433431
@SuppressLint("RequiresFeature", "AddWebMessageListenerUsage")
434432
suspend fun safeAddWebMessageListener(
433+
webViewCapabilityChecker: WebViewCapabilityChecker,
435434
jsObjectName: String,
436435
allowedOriginRules: Set<String>,
437436
listener: WebMessageListener,
@@ -452,35 +451,24 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
452451
false
453452
}
454453

455-
@SuppressLint("RequiresFeature", "RemoveWebMessageListenerUsage")
456-
suspend fun safeRemoveWebMessageListener(
457-
jsObjectName: String,
458-
): Boolean = runCatching {
459-
if (webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener) && !isDestroyed) {
460-
WebViewCompat.removeWebMessageListener(
461-
this,
462-
jsObjectName,
463-
)
464-
true
465-
} else {
466-
false
467-
}
468-
}.getOrElse { exception ->
469-
logcat(ERROR) { "Error removing WebMessageListener: $jsObjectName: ${exception.asLog()}" }
470-
false
471-
}
472-
473454
@SuppressLint("RequiresFeature")
474455
suspend fun safeAddDocumentStartJavaScript(
475456
script: String,
476457
allowedOriginRules: Set<String>,
477458
): ScriptHandler? {
478459
return runCatching {
479-
if (webViewCapabilityChecker.isSupported(WebViewCapability.DocumentStartJavaScript) && !isDestroyed) {
480-
webViewCompatWrapper.addDocumentStartJavaScript(this, script, allowedOriginRules)
481-
} else {
482-
null
460+
if (!isDestroyed) {
461+
if (::dispatcherProvider.isInitialized) {
462+
return withContext(dispatcherProvider.main()) {
463+
return@withContext WebViewCompat.addDocumentStartJavaScript(
464+
this@DuckDuckGoWebView,
465+
script,
466+
allowedOriginRules,
467+
)
468+
}
469+
}
483470
}
471+
null
484472
}.getOrElse { e ->
485473
logcat(ERROR) { "Error calling addDocumentStartJavaScript: ${e.asLog()}" }
486474
null
Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,22 @@
1414
* limitations under the License.
1515
*/
1616

17-
package com.duckduckgo.contentscopescripts.impl
17+
package com.duckduckgo.app.browser
1818

1919
import android.annotation.SuppressLint
2020
import androidx.webkit.ScriptHandler
2121
import androidx.webkit.WebViewCompat
22+
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
23+
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability
24+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
2225
import com.duckduckgo.common.utils.DispatcherProvider
2326
import com.duckduckgo.di.scopes.AppScope
2427
import com.squareup.anvil.annotations.ContributesBinding
2528
import javax.inject.Inject
2629
import kotlinx.coroutines.withContext
30+
import logcat.LogPriority.ERROR
31+
import logcat.asLog
32+
import logcat.logcat
2733

2834
@SuppressLint(
2935
"RequiresFeature",
@@ -34,14 +40,27 @@ import kotlinx.coroutines.withContext
3440
@ContributesBinding(AppScope::class)
3541
class RealWebViewCompatWrapper @Inject constructor(
3642
private val dispatcherProvider: DispatcherProvider,
43+
private val webViewCapabilityChecker: WebViewCapabilityChecker,
3744
) : WebViewCompatWrapper {
3845
override suspend fun addDocumentStartJavaScript(
3946
webView: android.webkit.WebView,
4047
script: String,
4148
allowedOriginRules: Set<String>,
42-
): ScriptHandler {
43-
return withContext(dispatcherProvider.main()) {
44-
WebViewCompat.addDocumentStartJavaScript(webView, script, allowedOriginRules)
49+
): ScriptHandler? {
50+
return runCatching {
51+
if (!webViewCapabilityChecker.isSupported(WebViewCapability.DocumentStartJavaScript)) {
52+
return null
53+
}
54+
55+
if (webView is DuckDuckGoWebView) {
56+
return webView.safeAddDocumentStartJavaScript(script, allowedOriginRules)
57+
}
58+
return withContext(dispatcherProvider.main()) {
59+
WebViewCompat.addDocumentStartJavaScript(webView, script, allowedOriginRules)
60+
}
61+
}.getOrElse { e ->
62+
logcat(ERROR) { "Error calling addDocumentStartJavaScript: ${e.asLog()}" }
63+
null
4564
}
4665
}
4766
}

browser-api/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ dependencies {
2727
implementation project(path: ':common-utils')
2828
implementation project(':feature-toggles-api')
2929
implementation AndroidX.core.ktx
30+
implementation AndroidX.webkit
3031
implementation KotlinX.coroutines.core
3132

3233
implementation "com.squareup.logcat:logcat:_"
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024 DuckDuckGo
2+
* Copyright (c) 2025 DuckDuckGo
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
package com.duckduckgo.contentscopescripts.impl
17+
package com.duckduckgo.browser.api.webviewcompat
1818

1919
import android.webkit.WebView
2020
import androidx.webkit.ScriptHandler
@@ -25,5 +25,5 @@ interface WebViewCompatWrapper {
2525
webView: WebView,
2626
script: String,
2727
allowedOriginRules: Set<String>,
28-
): ScriptHandler
28+
): ScriptHandler?
2929
}

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

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,61 @@
1717
package com.duckduckgo.contentscopescripts.impl
1818

1919
import android.annotation.SuppressLint
20+
import android.webkit.WebView
2021
import androidx.webkit.ScriptHandler
2122
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
23+
import com.duckduckgo.app.di.AppCoroutineScope
24+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
2225
import com.duckduckgo.common.utils.DispatcherProvider
2326
import com.duckduckgo.di.scopes.AppScope
2427
import com.duckduckgo.feature.toggles.api.Toggle
2528
import com.duckduckgo.js.messaging.api.AddDocumentStartJavaScriptPlugin
2629
import com.squareup.anvil.annotations.ContributesMultibinding
2730
import javax.inject.Inject
31+
import kotlinx.coroutines.CoroutineScope
32+
import kotlinx.coroutines.launch
2833
import kotlinx.coroutines.withContext
2934

3035
@ContributesMultibinding(AppScope::class)
3136
class ContentScopeScriptsAddDocumentStartJavaScriptPlugin @Inject constructor(
3237
private val webViewCompatContentScopeScripts: WebViewCompatContentScopeScripts,
3338
private val dispatcherProvider: DispatcherProvider,
3439
private val webViewCapabilityChecker: WebViewCapabilityChecker,
40+
private val webViewCompatWrapper: WebViewCompatWrapper,
41+
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
3542
) : AddDocumentStartJavaScriptPlugin {
3643
private var script: ScriptHandler? = null
3744
private var currentScriptString: String? = null
3845

3946
@SuppressLint("RequiresFeature")
40-
override suspend fun configureAddDocumentStartJavaScript(
47+
override fun addDocumentStartJavaScript(
4148
activeExperiments: List<Toggle>,
42-
scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler?,
49+
webView: WebView,
4350
) {
44-
if (!webViewCompatContentScopeScripts.isEnabled() || !webViewCapabilityChecker.isSupported(
45-
WebViewCapabilityChecker.WebViewCapability.DocumentStartJavaScript,
46-
)
47-
) {
48-
return
49-
}
51+
appCoroutineScope.launch {
52+
if (!webViewCompatContentScopeScripts.isEnabled() || !webViewCapabilityChecker.isSupported(
53+
WebViewCapabilityChecker.WebViewCapability.DocumentStartJavaScript,
54+
)
55+
) {
56+
return@launch
57+
}
5058

51-
val scriptString = webViewCompatContentScopeScripts.getScript(activeExperiments)
52-
if (scriptString == currentScriptString) {
53-
return
54-
}
55-
withContext(dispatcherProvider.main()) {
59+
val scriptString = webViewCompatContentScopeScripts.getScript(activeExperiments)
60+
if (scriptString == currentScriptString) {
61+
return@launch
62+
}
5663
script?.let {
57-
it.remove()
64+
withContext(dispatcherProvider.main()) {
65+
it.remove()
66+
}
5867
script = null
5968
}
6069

61-
scriptInjector(scriptString, setOf("*"))?.let {
70+
webViewCompatWrapper.addDocumentStartJavaScript(
71+
webView,
72+
scriptString,
73+
setOf("*"),
74+
)?.let {
6275
script = it
6376
currentScriptString = scriptString
6477
}

0 commit comments

Comments
 (0)