Skip to content

Commit 313632b

Browse files
CrisBarreiroCrisBarreiro
committed
Simplify APIs
1 parent d5abd49 commit 313632b

File tree

8 files changed

+99
-98
lines changed

8 files changed

+99
-98
lines changed

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

Lines changed: 3 additions & 4 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.WebViewCompat.WebMessageListener
4342
import com.duckduckgo.adclick.api.AdClickManager
4443
import com.duckduckgo.anrs.api.CrashLogger
4544
import com.duckduckgo.anrs.api.CrashLogger.Crash
@@ -1339,13 +1338,13 @@ class BrowserWebViewClientTest {
13391338
var registered = false
13401339
private set
13411340

1342-
override suspend fun unregister(unregisterer: suspend (objectName: String) -> Boolean) {
1341+
override fun unregister(webView: WebView) {
13431342
registered = false
13441343
}
13451344

1346-
override suspend fun register(
1345+
override fun register(
13471346
jsMessageCallback: JsMessageCallback?,
1348-
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
1347+
webView: WebView,
13491348
) {
13501349
registered = true
13511350
}

‎app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3869,13 +3869,11 @@ class BrowserTabFragment :
38693869

38703870
private fun destroyWebView() {
38713871
if (::webViewContainer.isInitialized) webViewContainer.removeAllViews()
3872-
appCoroutineScope.launch(dispatchers.main()) {
3873-
webView?.let {
3874-
webViewClient.destroy(it)
3875-
it.destroy()
3876-
}
3877-
webView = null
3872+
webView?.let {
3873+
webViewClient.destroy(it)
3874+
it.destroy()
38783875
}
3876+
webView = null
38793877
}
38803878

38813879
private fun convertBlobToDataUri(blob: Command.ConvertBlobToDataUri) {

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -476,13 +476,7 @@ class BrowserWebViewClient @Inject constructor(
476476
}
477477

478478
webMessagingPlugins.getPlugins().forEach { plugin ->
479-
plugin.register(callback) { objectName, allowedOriginRules, webMessageListener ->
480-
webView.safeAddWebMessageListener(
481-
objectName,
482-
allowedOriginRules,
483-
webMessageListener,
484-
)
485-
}
479+
plugin.register(callback, webView)
486480
}
487481
}
488482
}
@@ -771,11 +765,9 @@ class BrowserWebViewClient @Inject constructor(
771765
requestInterceptor.addExemptedMaliciousSite(url, feed)
772766
}
773767

774-
suspend fun destroy(webView: DuckDuckGoWebView) {
768+
fun destroy(webView: DuckDuckGoWebView) {
775769
webMessagingPlugins.getPlugins().forEach { plugin ->
776-
plugin.unregister { objectName ->
777-
webView.safeRemoveWebMessageListener(objectName)
778-
}
770+
plugin.unregister(webView)
779771
}
780772
}
781773
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import androidx.core.view.NestedScrollingChild3
3434
import androidx.core.view.NestedScrollingChildHelper
3535
import androidx.core.view.ViewCompat
3636
import androidx.webkit.ScriptHandler
37+
import androidx.webkit.WebViewCompat
3738
import androidx.webkit.WebViewCompat.WebMessageListener
3839
import com.duckduckgo.anvil.annotations.InjectWith
3940
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
@@ -432,7 +433,7 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
432433
listener: WebMessageListener,
433434
): Boolean = runCatching {
434435
if (webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener) && !isDestroyed) {
435-
webViewCompatWrapper.addWebMessageListener(
436+
WebViewCompat.addWebMessageListener(
436437
this,
437438
jsObjectName,
438439
allowedOriginRules,
@@ -452,7 +453,7 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
452453
jsObjectName: String,
453454
): Boolean = runCatching {
454455
if (webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener) && !isDestroyed) {
455-
webViewCompatWrapper.removeWebMessageListener(
456+
WebViewCompat.removeWebMessageListener(
456457
this,
457458
jsObjectName,
458459
)

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,16 @@ class RealWebViewCompatWrapper @Inject constructor(
6666
}
6767

6868
override suspend fun removeWebMessageListener(webView: WebView, jsObjectName: String) {
69-
withContext(dispatcherProvider.main()) {
70-
WebViewCompat.removeWebMessageListener(
71-
webView,
72-
jsObjectName,
73-
)
69+
if (!webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener)) {
70+
return
71+
}
72+
73+
if (webView is DuckDuckGoWebView) {
74+
webView.safeRemoveWebMessageListener(jsObjectName)
75+
return
76+
}
77+
return withContext(dispatcherProvider.main()) {
78+
WebViewCompat.removeWebMessageListener(webView, jsObjectName)
7479
}
7580
}
7681

@@ -80,6 +85,14 @@ class RealWebViewCompatWrapper @Inject constructor(
8085
allowedOriginRules: Set<String>,
8186
listener: WebViewCompat.WebMessageListener,
8287
) {
88+
if (!webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener)) {
89+
return
90+
}
91+
92+
if (webView is DuckDuckGoWebView) {
93+
webView.safeAddWebMessageListener(jsObjectName, allowedOriginRules, listener)
94+
return
95+
}
8396
return withContext(dispatcherProvider.main()) {
8497
WebViewCompat.addWebMessageListener(webView, jsObjectName, allowedOriginRules, listener)
8598
}

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

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package com.duckduckgo.contentscopescripts.impl.messaging
1818

19+
import android.webkit.WebView
1920
import androidx.annotation.VisibleForTesting
20-
import androidx.webkit.WebViewCompat.WebMessageListener
21+
import com.duckduckgo.app.di.AppCoroutineScope
22+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
2123
import com.duckduckgo.common.utils.plugins.PluginPoint
2224
import com.duckduckgo.contentscopescripts.api.WebViewCompatContentScopeJsMessageHandlersPlugin
2325
import com.duckduckgo.contentscopescripts.impl.WebViewCompatContentScopeScripts
@@ -28,6 +30,8 @@ import com.duckduckgo.js.messaging.api.WebMessagingPlugin
2830
import com.squareup.anvil.annotations.ContributesMultibinding
2931
import com.squareup.moshi.Moshi
3032
import javax.inject.Inject
33+
import kotlinx.coroutines.CoroutineScope
34+
import kotlinx.coroutines.launch
3135
import logcat.LogPriority.ERROR
3236
import logcat.asLog
3337
import logcat.logcat
@@ -39,6 +43,8 @@ class WebViewCompatWebCompatMessagingPlugin @Inject constructor(
3943
private val handlers: PluginPoint<WebViewCompatContentScopeJsMessageHandlersPlugin>,
4044
private val globalHandlers: PluginPoint<GlobalContentScopeJsMessageHandlersPlugin>,
4145
private val webViewCompatContentScopeScripts: WebViewCompatContentScopeScripts,
46+
private val webViewCompatWrapper: WebViewCompatWrapper,
47+
@AppCoroutineScope private val coroutineScope: CoroutineScope,
4248
) : WebMessagingPlugin {
4349

4450
private val moshi = Moshi.Builder().add(JSONObjectAdapter()).build()
@@ -76,39 +82,42 @@ class WebViewCompatWebCompatMessagingPlugin @Inject constructor(
7682
}
7783
}
7884

79-
override suspend fun register(
85+
override fun register(
8086
jsMessageCallback: JsMessageCallback?,
81-
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
87+
webView: WebView,
8288
) {
83-
if (!webViewCompatContentScopeScripts.isEnabled()) return
84-
if (jsMessageCallback == null) throw Exception("Callback cannot be null")
89+
coroutineScope.launch {
90+
if (!webViewCompatContentScopeScripts.isEnabled()) return@launch
91+
if (jsMessageCallback == null) throw Exception("Callback cannot be null")
8592

86-
runCatching {
87-
return@runCatching registerer(
88-
JS_OBJECT_NAME,
89-
allowedDomains,
90-
WebMessageListener { _, message, _, _, _ ->
93+
runCatching {
94+
return@runCatching webViewCompatWrapper.addWebMessageListener(
95+
webView,
96+
JS_OBJECT_NAME,
97+
allowedDomains,
98+
) { _, message, _, _, _ ->
9199
process(
92100
message.data ?: "",
93101
jsMessageCallback,
94102
)
95-
},
96-
)
97-
}.getOrElse { exception ->
98-
logcat(ERROR) { "Error adding WebMessageListener for contentScopeAdsjs: ${exception.asLog()}" }
99-
false
103+
}
104+
}.getOrElse { exception ->
105+
logcat(ERROR) { "Error adding WebMessageListener for contentScopeAdsjs: ${exception.asLog()}" }
106+
}
100107
}
101108
}
102109

103-
override suspend fun unregister(
104-
unregisterer: suspend (objectName: String) -> Boolean,
110+
override fun unregister(
111+
webView: WebView,
105112
) {
106-
if (!webViewCompatContentScopeScripts.isEnabled()) return
107-
runCatching {
108-
return@runCatching unregisterer(JS_OBJECT_NAME)
109-
}.getOrElse { exception ->
110-
logcat(ERROR) {
111-
"Error removing WebMessageListener for contentScopeAdsjs: ${exception.asLog()}"
113+
coroutineScope.launch {
114+
if (!webViewCompatContentScopeScripts.isEnabled()) return@launch
115+
runCatching {
116+
return@runCatching webViewCompatWrapper.removeWebMessageListener(webView, JS_OBJECT_NAME)
117+
}.getOrElse { exception ->
118+
logcat(ERROR) {
119+
"Error removing WebMessageListener for contentScopeAdsjs: ${exception.asLog()}"
120+
}
112121
}
113122
}
114123
}

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

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,41 @@
1616

1717
package com.duckduckgo.contentscopescripts.impl.messaging
1818

19+
import android.webkit.WebView
1920
import androidx.test.ext.junit.runners.AndroidJUnit4
20-
import androidx.webkit.WebViewCompat.WebMessageListener
21+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
22+
import com.duckduckgo.common.test.CoroutineTestRule
2123
import com.duckduckgo.common.utils.plugins.PluginPoint
2224
import com.duckduckgo.contentscopescripts.api.WebViewCompatContentScopeJsMessageHandlersPlugin
2325
import com.duckduckgo.contentscopescripts.impl.WebViewCompatContentScopeScripts
2426
import com.duckduckgo.js.messaging.api.JsMessage
2527
import com.duckduckgo.js.messaging.api.JsMessageCallback
2628
import com.duckduckgo.js.messaging.api.WebViewCompatMessageHandler
2729
import junit.framework.TestCase.assertEquals
28-
import junit.framework.TestCase.assertNull
2930
import kotlinx.coroutines.test.runTest
3031
import org.json.JSONObject
3132
import org.junit.Before
33+
import org.junit.Rule
3234
import org.junit.Test
3335
import org.junit.runner.RunWith
36+
import org.mockito.kotlin.any
37+
import org.mockito.kotlin.eq
3438
import org.mockito.kotlin.mock
39+
import org.mockito.kotlin.never
40+
import org.mockito.kotlin.verify
3541
import org.mockito.kotlin.whenever
3642

3743
@RunWith(AndroidJUnit4::class)
3844
class WebViewCompatWebCompatMessagingPluginTest {
3945

46+
@get:Rule
47+
val coroutineRule = CoroutineTestRule()
48+
4049
private val webViewCompatContentScopeScripts: WebViewCompatContentScopeScripts = mock()
4150
private val handlers: PluginPoint<WebViewCompatContentScopeJsMessageHandlersPlugin> = FakePluginPoint()
4251
private val globalHandlers: PluginPoint<GlobalContentScopeJsMessageHandlersPlugin> = FakeGlobalHandlersPluginPoint()
52+
private val mockWebViewCompatWrapper: WebViewCompatWrapper = mock()
53+
private val mockWebView: WebView = mock()
4354
private lateinit var testee: WebViewCompatWebCompatMessagingPlugin
4455

4556
private class FakePluginPoint : PluginPoint<WebViewCompatContentScopeJsMessageHandlersPlugin> {
@@ -94,6 +105,8 @@ class WebViewCompatWebCompatMessagingPluginTest {
94105
handlers = handlers,
95106
globalHandlers = globalHandlers,
96107
webViewCompatContentScopeScripts = webViewCompatContentScopeScripts,
108+
webViewCompatWrapper = mockWebViewCompatWrapper,
109+
coroutineScope = coroutineRule.testScope,
97110
)
98111
}
99112

@@ -162,67 +175,43 @@ class WebViewCompatWebCompatMessagingPluginTest {
162175
fun `when registering and adsjs is disabled then do not register`() = runTest {
163176
whenever(webViewCompatContentScopeScripts.isEnabled()).thenReturn(false)
164177

165-
var capturedObjectName: String? = null
166-
var capturedAllowedOriginRules: Set<String>? = null
167-
val registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean =
168-
{ objectName, allowedOriginRules, webMessageListener ->
169-
capturedObjectName = objectName
170-
capturedAllowedOriginRules = allowedOriginRules
171-
true
172-
}
173-
174-
testee.register(callback, registerer)
178+
testee.register(callback, mockWebView)
175179

176-
assertNull(capturedObjectName)
177-
assertNull(capturedAllowedOriginRules)
180+
verify(mockWebViewCompatWrapper, never())
181+
.addWebMessageListener(any(), any(), any(), any())
178182
}
179183

180184
@Test
181185
fun `when registering and adsjs is enabled then register`() = runTest {
182186
whenever(webViewCompatContentScopeScripts.isEnabled()).thenReturn(true)
183187

184-
var capturedObjectName: String? = null
185-
var capturedAllowedOriginRules: Set<String>? = null
186-
val registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean =
187-
{ objectName, allowedOriginRules, webMessageListener ->
188-
capturedObjectName = objectName
189-
capturedAllowedOriginRules = allowedOriginRules
190-
true
191-
}
192-
193-
testee.register(callback, registerer)
188+
testee.register(callback, mockWebView)
194189

195-
assertEquals("contentScopeAdsjs", capturedObjectName)
196-
assertEquals(setOf("*"), capturedAllowedOriginRules)
190+
verify(mockWebViewCompatWrapper).addWebMessageListener(
191+
eq(mockWebView),
192+
eq("contentScopeAdsjs"),
193+
eq(setOf("*")),
194+
any(),
195+
)
197196
}
198197

199198
@Test
200199
fun `when unregistering and adsjs is disabled then do not unregister`() = runTest {
201200
whenever(webViewCompatContentScopeScripts.isEnabled()).thenReturn(false)
202-
var capturedObjectName: String? = null
203-
val unregisterer: suspend (objectName: String) -> Boolean =
204-
{ objectName ->
205-
capturedObjectName = objectName
206-
true
207-
}
208201

209-
testee.unregister(unregisterer)
202+
testee.unregister(mockWebView)
210203

211-
assertNull(capturedObjectName)
204+
verify(mockWebViewCompatWrapper, never())
205+
.removeWebMessageListener(any(), any())
212206
}
213207

214208
@Test
215209
fun `when unregistering and adsjs is enabled then unregister`() = runTest {
216210
whenever(webViewCompatContentScopeScripts.isEnabled()).thenReturn(true)
217-
var capturedObjectName: String? = null
218-
val unregisterer: suspend (objectName: String) -> Boolean = { objectName ->
219-
capturedObjectName = objectName
220-
true
221-
}
222211

223-
testee.unregister(unregisterer)
212+
testee.unregister(mockWebView)
224213

225-
assertEquals("contentScopeAdsjs", capturedObjectName)
214+
verify(mockWebViewCompatWrapper).removeWebMessageListener(mockWebView, "contentScopeAdsjs")
226215
}
227216

228217
private val callback = object : JsMessageCallback() {
@@ -233,6 +222,6 @@ class WebViewCompatWebCompatMessagingPluginTest {
233222
}
234223

235224
private fun givenInterfaceIsRegistered() = runTest {
236-
testee.register(callback) { _, _, _ -> true }
225+
testee.register(callback, mockWebView)
237226
}
238227
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616

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

19-
import androidx.webkit.WebViewCompat.WebMessageListener
19+
import android.webkit.WebView
2020

2121
interface WebMessagingPlugin {
22-
suspend fun register(
22+
fun register(
2323
jsMessageCallback: JsMessageCallback?,
24-
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
24+
webView: WebView,
2525
)
2626

27-
suspend fun unregister(
28-
unregisterer: suspend (objectName: String) -> Boolean,
27+
fun unregister(
28+
webView: WebView,
2929
)
3030
}

0 commit comments

Comments
 (0)