Skip to content

Commit 3cb4d14

Browse files
CrisBarreiroCrisBarreiro
committed
Simplify APIs
1 parent 2b9079d commit 3cb4d14

File tree

11 files changed

+94
-85
lines changed

11 files changed

+94
-85
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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ 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
2525
import kotlinx.coroutines.test.runTest
2626
import org.junit.Assert.assertFalse
2727
import org.junit.Before

‎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: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ 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
4443
import com.duckduckgo.di.scopes.ViewScope
4544
import dagger.android.support.AndroidSupportInjection
4645
import javax.inject.Inject
@@ -74,9 +73,6 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
7473
private var isDestroyed: Boolean = false
7574
var isSafeWebViewEnabled: Boolean = false
7675

77-
@Inject
78-
lateinit var webViewCompatWrapper: WebViewCompatWrapper
79-
8076
@Inject
8177
lateinit var webViewCapabilityChecker: WebViewCapabilityChecker
8278

@@ -471,13 +467,13 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
471467
}
472468

473469
@SuppressLint("RequiresFeature")
474-
suspend fun safeAddDocumentStartJavaScript(
470+
fun safeAddDocumentStartJavaScript(
475471
script: String,
476472
allowedOriginRules: Set<String>,
477473
): ScriptHandler? {
478474
return runCatching {
479-
if (webViewCapabilityChecker.isSupported(WebViewCapability.DocumentStartJavaScript) && !isDestroyed) {
480-
webViewCompatWrapper.addDocumentStartJavaScript(this, script, allowedOriginRules)
475+
if (!isDestroyed) {
476+
WebViewCompat.addDocumentStartJavaScript(this, script, allowedOriginRules)
481477
} else {
482478
null
483479
}

‎content-scope-scripts/content-scope-scripts-impl/src/main/java/com/duckduckgo/contentscopescripts/impl/RealWebViewCompatWrapper.kt renamed to ‎app/src/main/java/com/duckduckgo/app/browser/RealWebViewCompatWrapper.kt

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:_"

‎content-scope-scripts/content-scope-scripts-impl/src/main/java/com/duckduckgo/contentscopescripts/impl/WebViewCompatWrapper.kt renamed to ‎browser-api/src/main/java/com/duckduckgo/browser/api/webviewcompat/WebViewCompatWrapper.kt

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
}

‎content-scope-scripts/content-scope-scripts-impl/src/test/java/com/duckduckgo/contentscopescripts/impl/ContentScopeScriptsAddDocumentStartJavaScriptPluginTest.kt

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
package com.duckduckgo.contentscopescripts.impl
22

3-
import androidx.webkit.ScriptHandler
3+
import android.webkit.WebView
44
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
5+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
56
import com.duckduckgo.common.test.CoroutineTestRule
67
import com.duckduckgo.feature.toggles.api.Toggle
78
import kotlinx.coroutines.test.runTest
8-
import org.junit.Assert.assertEquals
99
import org.junit.Before
1010
import org.junit.Rule
1111
import org.junit.Test
1212
import org.mockito.kotlin.any
1313
import org.mockito.kotlin.mock
14+
import org.mockito.kotlin.never
15+
import org.mockito.kotlin.verify
1416
import org.mockito.kotlin.whenever
1517

1618
class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
@@ -21,6 +23,8 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
2123
private val mockWebViewCompatContentScopeScripts: WebViewCompatContentScopeScripts = mock()
2224
private val mockToggle = mock<Toggle>()
2325
private val mockWebViewCapabilityChecker: WebViewCapabilityChecker = mock()
26+
private val mockWebViewCompatWrapper: WebViewCompatWrapper = mock()
27+
private val mockWebView: WebView = mock()
2428

2529
private lateinit var testee: ContentScopeScriptsAddDocumentStartJavaScriptPlugin
2630

@@ -30,6 +34,8 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
3034
mockWebViewCompatContentScopeScripts,
3135
coroutineRule.testDispatcherProvider,
3236
mockWebViewCapabilityChecker,
37+
mockWebViewCompatWrapper,
38+
coroutineRule.testScope,
3339
)
3440
}
3541

@@ -38,18 +44,10 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
3844
whenever(mockWebViewCompatContentScopeScripts.isEnabled()).thenReturn(true)
3945
whenever(mockWebViewCapabilityChecker.isSupported(any())).thenReturn(true)
4046
whenever(mockWebViewCompatContentScopeScripts.getScript(any())).thenReturn("script")
41-
var capturedScriptString: String? = null
42-
var capturedAllowedOriginRules: Set<String>? = null
43-
val scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler? = { scriptString, allowedOriginRules ->
44-
capturedScriptString = scriptString
45-
capturedAllowedOriginRules = allowedOriginRules
46-
mock<ScriptHandler>()
47-
}
4847

49-
testee.configureAddDocumentStartJavaScript(listOf(mockToggle), scriptInjector)
48+
testee.addDocumentStartJavaScript(listOf(mockToggle), mockWebView)
5049

51-
assertEquals("script", capturedScriptString)
52-
assertEquals(setOf("*"), capturedAllowedOriginRules)
50+
verify(mockWebViewCompatWrapper).addDocumentStartJavaScript(mockWebView, "script", setOf("*"))
5351
}
5452

5553
@Test
@@ -58,18 +56,9 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
5856
whenever(mockWebViewCapabilityChecker.isSupported(any())).thenReturn(true)
5957
whenever(mockWebViewCompatContentScopeScripts.getScript(any())).thenReturn("script")
6058

61-
var capturedScriptString: String? = null
62-
var capturedAllowedOriginRules: Set<String>? = null
63-
val scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler? = { scriptString, allowedOriginRules ->
64-
capturedScriptString = scriptString
65-
capturedAllowedOriginRules = allowedOriginRules
66-
mock<ScriptHandler>()
67-
}
59+
testee.addDocumentStartJavaScript(listOf(mockToggle), mockWebView)
6860

69-
testee.configureAddDocumentStartJavaScript(listOf(mockToggle), scriptInjector)
70-
71-
assertEquals(null, capturedScriptString)
72-
assertEquals(null, capturedAllowedOriginRules)
61+
verify(mockWebViewCompatWrapper, never()).addDocumentStartJavaScript(any(), any(), any())
7362
}
7463

7564
@Test
@@ -78,17 +67,8 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPluginTest {
7867
whenever(mockWebViewCapabilityChecker.isSupported(any())).thenReturn(false)
7968
whenever(mockWebViewCompatContentScopeScripts.getScript(any())).thenReturn("script")
8069

81-
var capturedScriptString: String? = null
82-
var capturedAllowedOriginRules: Set<String>? = null
83-
val scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler? = { scriptString, allowedOriginRules ->
84-
capturedScriptString = scriptString
85-
capturedAllowedOriginRules = allowedOriginRules
86-
mock<ScriptHandler>()
87-
}
88-
89-
testee.configureAddDocumentStartJavaScript(listOf(mockToggle), scriptInjector)
70+
testee.addDocumentStartJavaScript(listOf(mockToggle), mockWebView)
9071

91-
assertEquals(null, capturedScriptString)
92-
assertEquals(null, capturedAllowedOriginRules)
72+
verify(mockWebViewCompatWrapper, never()).addDocumentStartJavaScript(any(), any(), any())
9373
}
9474
}

‎js-messaging/js-messaging-api/src/main/java/com/duckduckgo/js/messaging/api/AddDocumentStartJavaScriptPlugin.kt

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
package com.duckduckgo.js.messaging.api
1818

19-
import androidx.webkit.ScriptHandler
19+
import android.webkit.WebView
2020
import com.duckduckgo.feature.toggles.api.Toggle
2121

2222
/**
@@ -26,14 +26,8 @@ import com.duckduckgo.feature.toggles.api.Toggle
2626
*/
2727
interface AddDocumentStartJavaScriptPlugin {
2828

29-
/**
30-
* Configures JavaScript injection for addDocumentStartJavaScript and executes it through [scriptInjector].
31-
* @param scriptInjector A suspend function that injects the script into the WebView.
32-
* Returns a [ScriptHandler] that can be used to remove the script later,
33-
* or null if injection failed.
34-
*/
35-
suspend fun configureAddDocumentStartJavaScript(
29+
fun addDocumentStartJavaScript(
3630
activeExperiments: List<Toggle>,
37-
scriptInjector: suspend (scriptString: String, allowedOriginRules: Set<String>) -> ScriptHandler?,
31+
webView: WebView,
3832
)
3933
}

0 commit comments

Comments
 (0)