Skip to content

Commit 67669ee

Browse files
committed
Simplify APIs
1 parent f6d470c commit 67669ee

File tree

12 files changed

+218
-224
lines changed

12 files changed

+218
-224
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/androidTest/java/com/duckduckgo/app/browser/DuckDuckGoWebViewTest.kt

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,55 +19,26 @@ package com.duckduckgo.app.browser
1919
import androidx.test.annotation.UiThreadTest
2020
import androidx.test.ext.junit.runners.AndroidJUnit4
2121
import androidx.test.platform.app.InstrumentationRegistry
22-
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
23-
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability.DocumentStartJavaScript
24-
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
25-
import kotlinx.coroutines.test.runTest
2622
import org.junit.Assert.assertFalse
2723
import org.junit.Before
2824
import org.junit.Test
2925
import org.junit.runner.RunWith
30-
import org.mockito.Mockito.mock
31-
import org.mockito.Mockito.verify
32-
import org.mockito.kotlin.never
33-
import org.mockito.kotlin.whenever
3426

3527
@RunWith(AndroidJUnit4::class)
3628
class DuckDuckGoWebViewTest {
3729

3830
private lateinit var testee: DuckDuckGoWebView
39-
private val mockWebViewCapabilityChecker: WebViewCapabilityChecker = mock()
40-
private val mockWebViewCompatWrapper: WebViewCompatWrapper = mock()
4131

4232
@Before
4333
@UiThreadTest
4434
fun setUp() {
4535
val context = InstrumentationRegistry.getInstrumentation().targetContext
4636
testee = DuckDuckGoWebView(context)
47-
testee.webViewCapabilityChecker = mockWebViewCapabilityChecker
4837
}
4938

5039
@Test
5140
@UiThreadTest
5241
fun whenWebViewInitialisedThenSafeBrowsingDisabled() {
5342
assertFalse(testee.settings.safeBrowsingEnabled)
5443
}
55-
56-
@Test
57-
fun whenSafeAddDocumentStartJavaScriptWithFeatureEnabledThenAddScript() = runTest {
58-
whenever(mockWebViewCapabilityChecker.isSupported(DocumentStartJavaScript)).thenReturn(true)
59-
60-
testee.safeAddDocumentStartJavaScript("script", setOf("*"))
61-
62-
verify(mockWebViewCompatWrapper).addDocumentStartJavaScript(testee, "script", setOf("*"))
63-
}
64-
65-
@Test
66-
fun whenSafeAddDocumentStartJavaScriptWithFeatureDisabledThenDoNotAddScript() = runTest {
67-
whenever(mockWebViewCapabilityChecker.isSupported(DocumentStartJavaScript)).thenReturn(false)
68-
69-
testee.safeAddDocumentStartJavaScript("script", setOf("*"))
70-
71-
verify(mockWebViewCompatWrapper, never()).addDocumentStartJavaScript(testee, "script", setOf("*"))
72-
}
7344
}

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ import com.duckduckgo.browser.api.brokensite.BrokenSiteData
247247
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.RELOAD_THREE_TIMES_WITHIN_20_SECONDS
248248
import com.duckduckgo.browser.api.ui.BrowserScreens.PrivateSearchScreenNoParams
249249
import com.duckduckgo.browser.api.ui.BrowserScreens.WebViewActivityWithParams
250+
import com.duckduckgo.browser.api.webviewcompat.WebViewCompatWrapper
250251
import com.duckduckgo.common.ui.DuckDuckGoActivity
251252
import com.duckduckgo.common.ui.DuckDuckGoFragment
252253
import com.duckduckgo.common.ui.experiments.visual.store.ExperimentalThemingDataStore
@@ -587,6 +588,9 @@ class BrowserTabFragment :
587588
@Inject
588589
lateinit var passkeyInitializer: WebViewPasskeyInitializer
589590

591+
@Inject
592+
lateinit var webViewCompatWrapper: WebViewCompatWrapper
593+
590594
/**
591595
* We use this to monitor whether the user was seeing the in-context Email Protection signup prompt
592596
* This is needed because the activity stack will be cleared if an external link is opened in our browser
@@ -3222,7 +3226,8 @@ class BrowserTabFragment :
32223226
val script = blobDownloadScript()
32233227
WebViewCompat.addDocumentStartJavaScript(webView, script, setOf("*"))
32243228

3225-
webView.safeAddWebMessageListener(
3229+
webViewCompatWrapper.addWebMessageListener(
3230+
webView,
32263231
"ddgBlobDownloadObj",
32273232
setOf("*"),
32283233
object : WebViewCompat.WebMessageListener {
@@ -3869,13 +3874,11 @@ class BrowserTabFragment :
38693874

38703875
private fun destroyWebView() {
38713876
if (::webViewContainer.isInitialized) webViewContainer.removeAllViews()
3872-
appCoroutineScope.launch(dispatchers.main()) {
3873-
webView?.let {
3874-
webViewClient.destroy(it)
3875-
it.destroy()
3876-
}
3877-
webView = null
3877+
webView?.let {
3878+
webViewClient.destroy(it)
3879+
it.destroy()
38783880
}
3881+
webView = null
38793882
}
38803883

38813884
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: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,12 @@ 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
39-
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
40-
import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability
4140
import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
4241
import com.duckduckgo.di.scopes.ViewScope
4342
import dagger.android.support.AndroidSupportInjection
44-
import javax.inject.Inject
4543
import logcat.LogPriority.ERROR
4644
import logcat.asLog
4745
import logcat.logcat
@@ -72,9 +70,6 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
7270
private var isDestroyed: Boolean = false
7371
var isSafeWebViewEnabled: Boolean = false
7472

75-
@Inject
76-
lateinit var webViewCapabilityChecker: WebViewCapabilityChecker
77-
7873
constructor(context: Context) : this(context, null)
7974
constructor(
8075
context: Context,
@@ -426,13 +421,13 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
426421
}
427422

428423
@SuppressLint("RequiresFeature", "AddWebMessageListenerUsage")
429-
suspend fun safeAddWebMessageListener(
424+
fun safeAddWebMessageListener(
430425
jsObjectName: String,
431426
allowedOriginRules: Set<String>,
432427
listener: WebMessageListener,
433428
): Boolean = runCatching {
434-
if (webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener) && !isDestroyed) {
435-
webViewCompatWrapper.addWebMessageListener(
429+
if (!isDestroyed) {
430+
WebViewCompat.addWebMessageListener(
436431
this,
437432
jsObjectName,
438433
allowedOriginRules,
@@ -448,11 +443,11 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
448443
}
449444

450445
@SuppressLint("RequiresFeature", "RemoveWebMessageListenerUsage")
451-
suspend fun safeRemoveWebMessageListener(
446+
fun safeRemoveWebMessageListener(
452447
jsObjectName: String,
453448
): Boolean = runCatching {
454-
if (webViewCapabilityChecker.isSupported(WebViewCapability.WebMessageListener) && !isDestroyed) {
455-
webViewCompatWrapper.removeWebMessageListener(
449+
if (!isDestroyed) {
450+
WebViewCompat.removeWebMessageListener(
456451
this,
457452
jsObjectName,
458453
)

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
}

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

Lines changed: 0 additions & 86 deletions
This file was deleted.

0 commit comments

Comments
 (0)