Skip to content

Commit f727a70

Browse files
CrisBarreiroCrisBarreiro
committed
Fix issues
1 parent 8ac524e commit f727a70

File tree

6 files changed

+69
-47
lines changed

6 files changed

+69
-47
lines changed

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3022,18 +3022,21 @@ class BrowserTabFragment :
30223022
it.isSafeWebViewEnabled = safeWebViewFeature.self().isEnabled()
30233023
it.webViewClient = webViewClient
30243024
lifecycleScope.launch(dispatchers.main()) {
3025-
webViewClient.configureWebView(it, object : JsMessageCallback() {
3026-
override fun process(
3027-
featureName: String,
3028-
method: String,
3029-
id: String?,
3030-
data: JSONObject?,
3031-
) {
3032-
viewModel.processJsCallbackMessage(featureName, method, id, data, isActiveCustomTab()) {
3033-
it.url
3025+
webViewClient.configureWebView(
3026+
it,
3027+
object : JsMessageCallback() {
3028+
override fun process(
3029+
featureName: String,
3030+
method: String,
3031+
id: String?,
3032+
data: JSONObject?,
3033+
) {
3034+
viewModel.processJsCallbackMessage(featureName, method, id, data, isActiveCustomTab()) {
3035+
it.url
3036+
}
30343037
}
3035-
}
3036-
})
3038+
},
3039+
)
30373040
}
30383041

30393042
it.webChromeClient = webChromeClient

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ import logcat.LogPriority.INFO
9393
import logcat.LogPriority.VERBOSE
9494
import logcat.LogPriority.WARN
9595
import logcat.logcat
96-
import org.json.JSONObject
9796

9897
private const val ABOUT_BLANK = "about:blank"
9998

@@ -478,7 +477,6 @@ class BrowserWebViewClient @Inject constructor(
478477
}
479478
}
480479

481-
482480
webMessagingPlugins.getPlugins().forEach { plugin ->
483481
plugin.register(webView, callback) { objectName, allowedOriginRules, webMessageListener ->
484482
webView.safeAddWebMessageListener(
@@ -777,7 +775,6 @@ class BrowserWebViewClient @Inject constructor(
777775
}
778776

779777
suspend fun destroy(webView: DuckDuckGoWebView) {
780-
781778
webMessagingPlugins.getPlugins().forEach { plugin ->
782779
plugin.unregister { objectName ->
783780
webView.safeRemoveWebMessageListener(objectName)

‎app/src/main/java/com/duckduckgo/app/plugins/WebMessagingPluginPoint.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.duckduckgo.app.plugins
1818

1919
import com.duckduckgo.anvil.annotations.ContributesPluginPoint
20-
import com.duckduckgo.contentscopescripts.api.AddDocumentStartJavaScriptPlugin
2120
import com.duckduckgo.contentscopescripts.api.WebMessagingPlugin
2221
import com.duckduckgo.di.scopes.AppScope
2322

‎content-scope-scripts/content-scope-scripts-api/src/main/java/com/duckduckgo/contentscopescripts/api/WebMessagingPlugin.kt

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

1717
package com.duckduckgo.contentscopescripts.api
1818

19-
import android.webkit.WebView
20-
import androidx.webkit.ScriptHandler
2119
import androidx.webkit.WebViewCompat.WebMessageListener
22-
import com.duckduckgo.feature.toggles.api.Toggle
2320
import com.duckduckgo.js.messaging.api.JsMessageCallback
24-
import kotlinx.coroutines.withContext
2521

2622
interface WebMessagingPlugin {
2723
suspend fun register(
28-
webView: WebView,
2924
jsMessageCallback: JsMessageCallback?,
30-
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean)
25+
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
26+
)
3127

3228
suspend fun unregister(
33-
unregisterer: suspend (objectName: String) -> Boolean)
29+
unregisterer: suspend (objectName: String) -> Boolean,
30+
)
3431
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

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

19-
import android.webkit.WebView
2019
import androidx.annotation.VisibleForTesting
2120
import androidx.webkit.WebViewCompat.WebMessageListener
2221
import com.duckduckgo.common.utils.DispatcherProvider
@@ -84,7 +83,6 @@ class WebCompatMessagingPlugin @Inject constructor(
8483
}
8584

8685
override suspend fun register(
87-
webView: WebView,
8886
jsMessageCallback: JsMessageCallback?,
8987
registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean,
9088
) {
@@ -109,7 +107,7 @@ class WebCompatMessagingPlugin @Inject constructor(
109107
if (!adsJsContentScopeScripts.isEnabled()) return
110108
withContext(dispatcherProvider.main()) {
111109
runCatching {
112-
return@runCatching unregister(unregisterer)
110+
return@runCatching unregisterer(JS_OBJECT_NAME)
113111
}.getOrElse { exception ->
114112
logcat(ERROR) {
115113
"Error removing WebMessageListener for contentScopeAdsjs: ${exception.asLog()}"

‎content-scope-scripts/content-scope-scripts-impl/src/test/java/com/duckduckgo/contentscopescripts/impl/messaging/AdsjsContentScopeScriptsJsMessagingTest.kt renamed to ‎content-scope-scripts/content-scope-scripts-impl/src/test/java/com/duckduckgo/contentscopescripts/impl/messaging/WebCompatMessagingPluginTest.kt

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
package com.duckduckgo.contentscopescripts.impl.messaging
1818

1919
import androidx.test.ext.junit.runners.AndroidJUnit4
20-
import com.duckduckgo.app.browser.api.DuckDuckGoWebView
20+
import androidx.webkit.WebViewCompat.WebMessageListener
2121
import com.duckduckgo.common.test.CoroutineTestRule
2222
import com.duckduckgo.common.utils.plugins.PluginPoint
2323
import com.duckduckgo.contentscopescripts.api.GlobalContentScopeJsMessageHandlersPlugin
@@ -28,27 +28,24 @@ import com.duckduckgo.js.messaging.api.JsMessage
2828
import com.duckduckgo.js.messaging.api.JsMessageCallback
2929
import com.duckduckgo.js.messaging.api.WebCompatMessageHandler
3030
import junit.framework.TestCase.assertEquals
31+
import junit.framework.TestCase.assertNull
3132
import kotlinx.coroutines.test.runTest
3233
import org.json.JSONObject
3334
import org.junit.Before
3435
import org.junit.Rule
3536
import org.junit.Test
3637
import org.junit.runner.RunWith
37-
import org.mockito.kotlin.any
3838
import org.mockito.kotlin.mock
39-
import org.mockito.kotlin.never
40-
import org.mockito.kotlin.verify
4139
import org.mockito.kotlin.whenever
4240

4341
@RunWith(AndroidJUnit4::class)
44-
class AdsjsContentScopeScriptsJsMessagingTest {
42+
class WebCompatMessagingPluginTest {
4543
@get:Rule var coroutineRule = CoroutineTestRule()
4644

47-
private val mockWebView: DuckDuckGoWebView = mock()
4845
private val adsJsContentScopeScripts: AdsJsContentScopeScripts = mock()
4946
private val handlers: PluginPoint<WebCompatContentScopeJsMessageHandlersPlugin> = FakePluginPoint()
5047
private val globalHandlers: PluginPoint<GlobalContentScopeJsMessageHandlersPlugin> = FakeGlobalHandlersPluginPoint()
51-
private lateinit var contentScopeScriptsJsMessaging: AdsjsContentScopeMessaging
48+
private lateinit var testee: WebCompatMessagingPlugin
5249

5350
private class FakePluginPoint : PluginPoint<WebCompatContentScopeJsMessageHandlersPlugin> {
5451
override fun getPlugins(): Collection<WebCompatContentScopeJsMessageHandlersPlugin> {
@@ -98,7 +95,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
9895
@Before
9996
fun setUp() = runTest {
10097
whenever(adsJsContentScopeScripts.isEnabled()).thenReturn(true)
101-
contentScopeScriptsJsMessaging = AdsjsContentScopeMessaging(
98+
testee = WebCompatMessagingPlugin(
10299
handlers = handlers,
103100
globalHandlers = globalHandlers,
104101
adsJsContentScopeScripts = adsJsContentScopeScripts,
@@ -114,7 +111,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
114111
{"context":"contentScopeScripts","featureName":"webCompat","id":"myId","method":"webShare","params":{}}
115112
""".trimIndent()
116113

117-
contentScopeScriptsJsMessaging.process(message, callback)
114+
testee.process(message, callback)
118115

119116
assertEquals(1, callback.counter)
120117
}
@@ -123,7 +120,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
123120
fun `when processing unknown message do nothing`() = runTest {
124121
givenInterfaceIsRegistered()
125122

126-
contentScopeScriptsJsMessaging.process("", callback)
123+
testee.process("", callback)
127124

128125
assertEquals(0, callback.counter)
129126
}
@@ -136,7 +133,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
136133
{"context":"contentScopeScripts","featureName":"test","id":"myId","method":"webShare","params":{}}
137134
""".trimIndent()
138135

139-
contentScopeScriptsJsMessaging.process(message, callback)
136+
testee.process(message, callback)
140137

141138
assertEquals(0, callback.counter)
142139
}
@@ -149,7 +146,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
149146
{"context":"contentScopeScripts","webCompat":"test","method":"webShare","params":{}}
150147
""".trimIndent()
151148

152-
contentScopeScriptsJsMessaging.process(message, callback)
149+
testee.process(message, callback)
153150

154151
assertEquals(0, callback.counter)
155152
}
@@ -162,7 +159,7 @@ class AdsjsContentScopeScriptsJsMessagingTest {
162159
{"context":"contentScopeScripts","featureName":"debugFeature","id":"debugId","method":"addDebugFlag","params":{}}
163160
""".trimIndent()
164161

165-
contentScopeScriptsJsMessaging.process(message, callback)
162+
testee.process(message, callback)
166163

167164
assertEquals(1, callback.counter)
168165
}
@@ -171,36 +168,67 @@ class AdsjsContentScopeScriptsJsMessagingTest {
171168
fun `when registering and adsjs is disabled then do not register`() = runTest {
172169
whenever(adsJsContentScopeScripts.isEnabled()).thenReturn(false)
173170

174-
contentScopeScriptsJsMessaging.register(mockWebView, callback)
171+
var capturedObjectName: String? = null
172+
var capturedAllowedOriginRules: Set<String>? = null
173+
val registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean =
174+
{ objectName, allowedOriginRules, webMessageListener ->
175+
capturedObjectName = objectName
176+
capturedAllowedOriginRules = allowedOriginRules
177+
true
178+
}
179+
180+
testee.register(callback, registerer)
175181

176-
verify(mockWebView, never()).safeAddWebMessageListener(any(), any(), any())
182+
assertNull(capturedObjectName)
183+
assertNull(capturedAllowedOriginRules)
177184
}
178185

179186
@Test
180187
fun `when registering and adsjs is enabled then register`() = runTest {
181188
whenever(adsJsContentScopeScripts.isEnabled()).thenReturn(true)
182189

183-
contentScopeScriptsJsMessaging.register(mockWebView, callback)
190+
var capturedObjectName: String? = null
191+
var capturedAllowedOriginRules: Set<String>? = null
192+
val registerer: suspend (objectName: String, allowedOriginRules: Set<String>, webMessageListener: WebMessageListener) -> Boolean =
193+
{ objectName, allowedOriginRules, webMessageListener ->
194+
capturedObjectName = objectName
195+
capturedAllowedOriginRules = allowedOriginRules
196+
true
197+
}
198+
199+
testee.register(callback, registerer)
184200

185-
verify(mockWebView).safeAddWebMessageListener(any(), any(), any())
201+
assertEquals("contentScopeAdsjs", capturedObjectName)
202+
assertEquals(setOf("*"), capturedAllowedOriginRules)
186203
}
187204

188205
@Test
189206
fun `when unregistering and adsjs is disabled then do not unregister`() = runTest {
190207
whenever(adsJsContentScopeScripts.isEnabled()).thenReturn(false)
208+
var capturedObjectName: String? = null
209+
val unregisterer: suspend (objectName: String) -> Boolean =
210+
{ objectName ->
211+
capturedObjectName = objectName
212+
true
213+
}
191214

192-
contentScopeScriptsJsMessaging.unregister(mockWebView)
215+
testee.unregister(unregisterer)
193216

194-
verify(mockWebView, never()).safeRemoveWebMessageListener(any())
217+
assertNull(capturedObjectName)
195218
}
196219

197220
@Test
198221
fun `when unregistering and adsjs is enabled then unregister`() = runTest {
199222
whenever(adsJsContentScopeScripts.isEnabled()).thenReturn(true)
223+
var capturedObjectName: String? = null
224+
val unregisterer: suspend (objectName: String) -> Boolean = { objectName ->
225+
capturedObjectName = objectName
226+
true
227+
}
200228

201-
contentScopeScriptsJsMessaging.unregister(mockWebView)
229+
testee.unregister(unregisterer)
202230

203-
verify(mockWebView).safeRemoveWebMessageListener(any())
231+
assertEquals("contentScopeAdsjs", capturedObjectName)
204232
}
205233

206234
private val callback = object : JsMessageCallback() {
@@ -211,6 +239,6 @@ class AdsjsContentScopeScriptsJsMessagingTest {
211239
}
212240

213241
private fun givenInterfaceIsRegistered() = runTest {
214-
contentScopeScriptsJsMessaging.register(mockWebView, callback)
242+
testee.register(callback) { _, _, _ -> true }
215243
}
216244
}

0 commit comments

Comments
 (0)