Skip to content

Commit 2b9079d

Browse files
committed
Improve thread safety
1 parent fc1e54f commit 2b9079d

File tree

8 files changed

+126
-78
lines changed

8 files changed

+126
-78
lines changed

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

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,49 +21,26 @@ import com.squareup.anvil.annotations.ContributesBinding
2121
import dagger.SingleInstanceIn
2222
import java.io.BufferedReader
2323
import javax.inject.Inject
24-
import javax.inject.Named
2524

2625
interface ContentScopeJSReader {
2726
fun getContentScopeJS(): String
2827
}
2928

30-
abstract class GenericContentScopeJSReader {
31-
abstract val fileName: String
32-
29+
@SingleInstanceIn(AppScope::class)
30+
@ContributesBinding(AppScope::class)
31+
class RealContentScopeJSReader @Inject constructor() : ContentScopeJSReader {
3332
private lateinit var contentScopeJS: String
3433

35-
protected fun getContentScopeJSFile(): String {
34+
override fun getContentScopeJS(): String {
3635
if (!this::contentScopeJS.isInitialized) {
37-
contentScopeJS = readResource(fileName).use { it?.readText() }.orEmpty()
36+
contentScopeJS = loadJs("contentScope.js")
3837
}
3938
return contentScopeJS
4039
}
4140

41+
fun loadJs(resourceName: String): String = readResource(resourceName).use { it?.readText() }.orEmpty()
42+
4243
private fun readResource(resourceName: String): BufferedReader? {
4344
return javaClass.classLoader?.getResource(resourceName)?.openStream()?.bufferedReader()
4445
}
4546
}
46-
47-
@SingleInstanceIn(AppScope::class)
48-
@ContributesBinding(AppScope::class, boundType = ContentScopeJSReader::class)
49-
@Named("contentScope")
50-
class RealContentScopeJSReader @Inject constructor() : GenericContentScopeJSReader(), ContentScopeJSReader {
51-
override val fileName: String
52-
get() = "contentScope.js"
53-
54-
override fun getContentScopeJS(): String {
55-
return getContentScopeJSFile()
56-
}
57-
}
58-
59-
@SingleInstanceIn(AppScope::class)
60-
@ContributesBinding(AppScope::class, boundType = ContentScopeJSReader::class)
61-
@Named("adsJS")
62-
class AdsContentScopeJSReader @Inject constructor() : GenericContentScopeJSReader(), ContentScopeJSReader {
63-
override val fileName: String
64-
get() = "adsjsContentScope.js"
65-
66-
override fun getContentScopeJS(): String {
67-
return getContentScopeJSFile()
68-
}
69-
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ class ContentScopeScriptsAddDocumentStartJavaScriptPlugin @Inject constructor(
4848
return
4949
}
5050

51+
val scriptString = webViewCompatContentScopeScripts.getScript(activeExperiments)
52+
if (scriptString == currentScriptString) {
53+
return
54+
}
5155
withContext(dispatcherProvider.main()) {
52-
val scriptString = webViewCompatContentScopeScripts.getScript(activeExperiments)
53-
if (scriptString == currentScriptString) {
54-
return@withContext
55-
}
5656
script?.let {
5757
it.remove()
5858
script = null

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ import com.squareup.moshi.JsonAdapter
3030
import com.squareup.moshi.Moshi.Builder
3131
import com.squareup.moshi.Types
3232
import dagger.SingleInstanceIn
33-
import java.util.*
33+
import java.util.Locale
34+
import java.util.UUID
3435
import java.util.concurrent.CopyOnWriteArrayList
3536
import javax.inject.Inject
36-
import javax.inject.Named
3737
import kotlinx.coroutines.runBlocking
3838

3939
interface CoreContentScopeScripts {
@@ -54,7 +54,7 @@ interface CoreContentScopeScripts {
5454
class RealContentScopeScripts @Inject constructor(
5555
private val pluginPoint: PluginPoint<ContentScopeConfigPlugin>,
5656
private val userAllowListRepository: UserAllowListRepository,
57-
@Named("contentScope") private val contentScopeJSReader: ContentScopeJSReader,
57+
private val contentScopeJSReader: ContentScopeJSReader,
5858
private val appBuildConfig: AppBuildConfig,
5959
private val unprotectedTemporary: UnprotectedTemporary,
6060
private val fingerprintProtectionManager: FingerprintProtectionManager,

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

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package com.duckduckgo.contentscopescripts.impl
1818

1919
import com.duckduckgo.app.privacy.db.UserAllowListRepository
2020
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
21+
import com.duckduckgo.common.utils.DispatcherProvider
2122
import com.duckduckgo.common.utils.plugins.PluginPoint
2223
import com.duckduckgo.contentscopescripts.api.ContentScopeConfigPlugin
2324
import com.duckduckgo.di.scopes.AppScope
@@ -33,11 +34,11 @@ import dagger.SingleInstanceIn
3334
import java.util.*
3435
import java.util.concurrent.CopyOnWriteArrayList
3536
import javax.inject.Inject
36-
import javax.inject.Named
3737
import kotlinx.coroutines.runBlocking
38+
import kotlinx.coroutines.withContext
3839

3940
interface WebViewCompatContentScopeScripts {
40-
fun getScript(
41+
suspend fun getScript(
4142
activeExperiments: List<Toggle>,
4243
): String
4344

@@ -53,11 +54,12 @@ interface WebViewCompatContentScopeScripts {
5354
class RealWebViewCompatContentScopeScripts @Inject constructor(
5455
private val pluginPoint: PluginPoint<ContentScopeConfigPlugin>,
5556
private val userAllowListRepository: UserAllowListRepository,
56-
@Named("adsJS") private val adsContentScopeJSReader: ContentScopeJSReader,
57+
private val webViewCompatContentScopeJSReader: WebViewCompatContentScopeJSReader,
5758
private val appBuildConfig: AppBuildConfig,
5859
private val unprotectedTemporary: UnprotectedTemporary,
5960
private val fingerprintProtectionManager: FingerprintProtectionManager,
6061
private val contentScopeScriptsFeature: ContentScopeScriptsFeature,
62+
private val dispatcherProvider: DispatcherProvider,
6163
) : WebViewCompatContentScopeScripts {
6264

6365
private var cachedContentScopeJson: String = getContentScopeJson("", emptyList())
@@ -76,43 +78,47 @@ class RealWebViewCompatContentScopeScripts @Inject constructor(
7678
override val javascriptInterface: String = getSecret()
7779
override val callbackName: String = getSecret()
7880

79-
override fun getScript(
81+
override suspend fun getScript(
8082
activeExperiments: List<Toggle>,
8183
): String {
82-
var updateJS = false
84+
return withContext(dispatcherProvider.io()) {
85+
var updateJS = false
8386

84-
val pluginParameters = getPluginParameters()
87+
val pluginParameters = getPluginParameters()
8588

86-
if (cachedUnprotectTemporaryExceptions != unprotectedTemporary.unprotectedTemporaryExceptions) {
87-
cacheUserUnprotectedTemporaryExceptions(unprotectedTemporary.unprotectedTemporaryExceptions)
88-
updateJS = true
89-
}
89+
if (cachedUnprotectTemporaryExceptions != unprotectedTemporary.unprotectedTemporaryExceptions) {
90+
cacheUserUnprotectedTemporaryExceptions(unprotectedTemporary.unprotectedTemporaryExceptions)
91+
updateJS = true
92+
}
9093

91-
val contentScopeJson = getContentScopeJson(pluginParameters.config, cachedUnprotectTemporaryExceptions)
92-
if (cachedContentScopeJson != contentScopeJson) {
93-
cachedContentScopeJson = contentScopeJson
94-
updateJS = true
95-
}
94+
val contentScopeJson = getContentScopeJson(pluginParameters.config, cachedUnprotectTemporaryExceptions)
95+
if (cachedContentScopeJson != contentScopeJson) {
96+
cachedContentScopeJson = contentScopeJson
97+
updateJS = true
98+
}
9699

97-
if (cachedUserUnprotectedDomains != userAllowListRepository.domainsInUserAllowList()) {
98-
cacheUserUnprotectedDomains(userAllowListRepository.domainsInUserAllowList())
99-
updateJS = true
100-
}
100+
if (cachedUserUnprotectedDomains != userAllowListRepository.domainsInUserAllowList()) {
101+
cacheUserUnprotectedDomains(userAllowListRepository.domainsInUserAllowList())
102+
updateJS = true
103+
}
101104

102-
val userPreferencesJson = getUserPreferencesJson(pluginParameters.preferences, activeExperiments = activeExperiments)
103-
if (cachedUserPreferencesJson != userPreferencesJson) {
104-
cachedUserPreferencesJson = userPreferencesJson
105-
updateJS = true
106-
}
105+
val userPreferencesJson = getUserPreferencesJson(pluginParameters.preferences, activeExperiments = activeExperiments)
106+
if (cachedUserPreferencesJson != userPreferencesJson) {
107+
cachedUserPreferencesJson = userPreferencesJson
108+
updateJS = true
109+
}
107110

108-
if (!this::cachedAdsJS.isInitialized || updateJS) {
109-
cacheJs()
111+
if (!this@RealWebViewCompatContentScopeScripts::cachedAdsJS.isInitialized || updateJS) {
112+
cacheJs()
113+
}
114+
return@withContext cachedAdsJS
110115
}
111-
return cachedAdsJS
112116
}
113117

114118
override suspend fun isEnabled(): Boolean {
115-
return contentScopeScriptsFeature.self().isEnabled() && contentScopeScriptsFeature.useNewWebCompatApis().isEnabled()
119+
return withContext(dispatcherProvider.io()) {
120+
contentScopeScriptsFeature.self().isEnabled() && contentScopeScriptsFeature.useNewWebCompatApis().isEnabled()
121+
}
116122
}
117123

118124
private fun getSecretKeyValuePair() = "\"messageSecret\":\"$secret\""
@@ -159,8 +165,8 @@ class RealWebViewCompatContentScopeScripts @Inject constructor(
159165
}
160166
}
161167

162-
private fun cacheJs() {
163-
val adsContentScopeJs = adsContentScopeJSReader.getContentScopeJS()
168+
private suspend fun cacheJs() {
169+
val adsContentScopeJs = webViewCompatContentScopeJSReader.getContentScopeJS()
164170

165171
cachedAdsJS = adsContentScopeJs
166172
.replace(contentScope, cachedContentScopeJson)

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ package com.duckduckgo.contentscopescripts.impl
1919
import android.annotation.SuppressLint
2020
import androidx.webkit.ScriptHandler
2121
import androidx.webkit.WebViewCompat
22+
import com.duckduckgo.common.utils.DispatcherProvider
2223
import com.duckduckgo.di.scopes.AppScope
2324
import com.squareup.anvil.annotations.ContributesBinding
2425
import javax.inject.Inject
26+
import kotlinx.coroutines.withContext
2527

2628
@SuppressLint(
2729
"RequiresFeature",
@@ -30,12 +32,16 @@ import javax.inject.Inject
3032
"RemoveWebMessageListenerUsage",
3133
)
3234
@ContributesBinding(AppScope::class)
33-
class RealWebViewCompatWrapper @Inject constructor() : WebViewCompatWrapper {
34-
override fun addDocumentStartJavaScript(
35+
class RealWebViewCompatWrapper @Inject constructor(
36+
private val dispatcherProvider: DispatcherProvider,
37+
) : WebViewCompatWrapper {
38+
override suspend fun addDocumentStartJavaScript(
3539
webView: android.webkit.WebView,
3640
script: String,
3741
allowedOriginRules: Set<String>,
3842
): ScriptHandler {
39-
return WebViewCompat.addDocumentStartJavaScript(webView, script, allowedOriginRules)
43+
return withContext(dispatcherProvider.main()) {
44+
WebViewCompat.addDocumentStartJavaScript(webView, script, allowedOriginRules)
45+
}
4046
}
4147
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright (c) 2022 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.contentscopescripts.impl
18+
19+
import com.duckduckgo.common.utils.DispatcherProvider
20+
import com.duckduckgo.di.scopes.AppScope
21+
import com.squareup.anvil.annotations.ContributesBinding
22+
import dagger.SingleInstanceIn
23+
import java.io.BufferedReader
24+
import javax.inject.Inject
25+
import kotlinx.coroutines.withContext
26+
27+
interface WebViewCompatContentScopeJSReader {
28+
suspend fun getContentScopeJS(): String
29+
}
30+
31+
@SingleInstanceIn(AppScope::class)
32+
@ContributesBinding(AppScope::class)
33+
class RealWebViewCompatContentScopeJSReader @Inject constructor(
34+
private val dispatcherProvider: DispatcherProvider,
35+
) : WebViewCompatContentScopeJSReader {
36+
private lateinit var contentScopeJS: String
37+
38+
override suspend fun getContentScopeJS(): String {
39+
if (!this@RealWebViewCompatContentScopeJSReader::contentScopeJS.isInitialized) {
40+
contentScopeJS = loadJs("contentScope.js")
41+
}
42+
return contentScopeJS
43+
}
44+
45+
private suspend fun loadJs(resourceName: String): String = readResource(resourceName).use { it?.readText() }.orEmpty()
46+
47+
private suspend fun readResource(resourceName: String): BufferedReader? {
48+
return withContext(dispatcherProvider.io()) {
49+
javaClass.classLoader?.getResource(resourceName)?.openStream()?.bufferedReader()
50+
}
51+
}
52+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import androidx.webkit.ScriptHandler
2121

2222
interface WebViewCompatWrapper {
2323

24-
fun addDocumentStartJavaScript(
24+
suspend fun addDocumentStartJavaScript(
2525
webView: WebView,
2626
script: String,
2727
allowedOriginRules: Set<String>,

0 commit comments

Comments
 (0)