Skip to content

Commit 62552d8

Browse files
authored
Fix data clearing ANR (#7112)
Task/Issue URL: https://app.asana.com/1/137249556945/task/1211892024022522?focus=true ### Description Fixes Data clearing ANR blocking main thread with runBlocking ### Steps to test this PR Smoke test data clearing when acting around the `WebLocalStorage` to confirm works as expected. ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)|
1 parent 27b9b29 commit 62552d8

File tree

3 files changed

+49
-53
lines changed

3 files changed

+49
-53
lines changed

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,13 @@ import com.squareup.anvil.annotations.ContributesBinding
3636
import dagger.SingleInstanceIn
3737
import kotlinx.coroutines.CoroutineScope
3838
import kotlinx.coroutines.launch
39+
import kotlinx.coroutines.withContext
3940
import logcat.LogPriority.ERROR
4041
import logcat.LogPriority.WARN
4142
import logcat.asLog
4243
import logcat.logcat
4344
import java.io.File
4445
import javax.inject.Inject
45-
import kotlin.coroutines.resume
46-
import kotlin.coroutines.suspendCoroutine
4746

4847
interface WebDataManager {
4948
suspend fun clearData(
@@ -92,25 +91,27 @@ class WebViewDataManager @Inject constructor(
9291
webView.clearHistory()
9392
}
9493

95-
private suspend fun clearWebStorage(webStorage: WebStorage) {
96-
suspendCoroutine { continuation ->
97-
if (androidBrowserConfigFeature.webLocalStorage().isEnabled()) {
98-
kotlin.runCatching {
99-
webLocalStorageManager.clearWebLocalStorage()
100-
continuation.resume(Unit)
101-
}.onFailure { e ->
102-
logcat(ERROR) { "WebDataManager: Could not selectively clear web storage: ${e.asLog()}" }
103-
if (appBuildConfig.isInternalBuild()) {
104-
sendCrashPixel(e)
105-
}
106-
// fallback, if we crash we delete everything
107-
webStorage.deleteAllData()
108-
continuation.resume(Unit)
94+
private suspend fun clearWebStorage(webStorage: WebStorage) = withContext(dispatcherProvider.io()) {
95+
if (androidBrowserConfigFeature.webLocalStorage().isEnabled()) {
96+
kotlin.runCatching {
97+
webLocalStorageManager.clearWebLocalStorage()
98+
}.onFailure { e ->
99+
logcat(ERROR) { "WebDataManager: Could not selectively clear web storage: ${e.asLog()}" }
100+
if (appBuildConfig.isInternalBuild()) {
101+
sendCrashPixel(e)
109102
}
110-
} else {
111-
webStorage.deleteAllData()
112-
continuation.resume(Unit)
103+
// fallback, if we crash we delete everything
104+
deleteAllData(webStorage)
113105
}
106+
} else {
107+
deleteAllData(webStorage)
108+
}
109+
}
110+
111+
private suspend fun deleteAllData(webStorage: WebStorage) {
112+
// WebStorage API must be called on main thread
113+
withContext(dispatcherProvider.main()) {
114+
webStorage.deleteAllData()
114115
}
115116
}
116117

app/src/main/java/com/duckduckgo/app/browser/weblocalstorage/WebLocalStorageManager.kt

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import dagger.Lazy
2828
import dagger.Module
2929
import dagger.Provides
3030
import dagger.SingleInstanceIn
31-
import kotlinx.coroutines.runBlocking
3231
import kotlinx.coroutines.withContext
3332
import logcat.logcat
3433
import org.iq80.leveldb.DB
@@ -39,7 +38,7 @@ import java.nio.charset.StandardCharsets
3938
import javax.inject.Inject
4039

4140
interface WebLocalStorageManager {
42-
fun clearWebLocalStorage()
41+
suspend fun clearWebLocalStorage()
4342
}
4443

4544
@ContributesBinding(AppScope::class)
@@ -56,40 +55,36 @@ class DuckDuckGoWebLocalStorageManager @Inject constructor(
5655
private var keysToDelete = emptyList<String>()
5756
private var matchingRegex = emptyList<String>()
5857

59-
override fun clearWebLocalStorage() = runBlocking {
60-
withContext(dispatcherProvider.io()) {
61-
val settings = androidBrowserConfigFeature.webLocalStorage().getSettings()
62-
val webLocalStorageSettings = webLocalStorageSettingsJsonParser.parseJson(settings)
58+
override suspend fun clearWebLocalStorage() = withContext(dispatcherProvider.io()) {
59+
val settings = androidBrowserConfigFeature.webLocalStorage().getSettings()
60+
val webLocalStorageSettings = webLocalStorageSettingsJsonParser.parseJson(settings)
6361

64-
val fireproofedDomains = withContext(dispatcherProvider.io()) {
65-
fireproofWebsiteRepository.fireproofWebsitesSync().map { it.domain }
66-
}
62+
val fireproofedDomains = fireproofWebsiteRepository.fireproofWebsitesSync().map { it.domain }
6763

68-
domains = webLocalStorageSettings.domains.list + fireproofedDomains
69-
keysToDelete = webLocalStorageSettings.keysToDelete.list
70-
matchingRegex = webLocalStorageSettings.matchingRegex.list
64+
domains = webLocalStorageSettings.domains.list + fireproofedDomains
65+
keysToDelete = webLocalStorageSettings.keysToDelete.list
66+
matchingRegex = webLocalStorageSettings.matchingRegex.list
7167

72-
logcat { "WebLocalStorageManager: Allowed domains: $domains" }
73-
logcat { "WebLocalStorageManager: Keys to delete: $keysToDelete" }
74-
logcat { "WebLocalStorageManager: Matching regex: $matchingRegex" }
68+
logcat { "WebLocalStorageManager: Allowed domains: $domains" }
69+
logcat { "WebLocalStorageManager: Keys to delete: $keysToDelete" }
70+
logcat { "WebLocalStorageManager: Matching regex: $matchingRegex" }
7571

76-
val db = databaseProvider.get()
77-
db.iterator().use { iterator ->
78-
iterator.seekToFirst()
72+
val db = databaseProvider.get()
73+
db.iterator().use { iterator ->
74+
iterator.seekToFirst()
7975

80-
while (iterator.hasNext()) {
81-
val entry = iterator.next()
82-
val key = String(entry.key, StandardCharsets.UTF_8)
76+
while (iterator.hasNext()) {
77+
val entry = iterator.next()
78+
val key = String(entry.key, StandardCharsets.UTF_8)
8379

84-
val domainForMatchingAllowedKey = getDomainForMatchingAllowedKey(key)
85-
if (domainForMatchingAllowedKey == null) {
80+
val domainForMatchingAllowedKey = getDomainForMatchingAllowedKey(key)
81+
if (domainForMatchingAllowedKey == null) {
82+
db.delete(entry.key)
83+
logcat { "WebLocalStorageManager: Deleted key: $key" }
84+
} else if (settingsDataStore.clearDuckAiData && domainForMatchingAllowedKey == DUCKDUCKGO_DOMAIN) {
85+
if (keysToDelete.any { key.endsWith(it) }) {
8686
db.delete(entry.key)
8787
logcat { "WebLocalStorageManager: Deleted key: $key" }
88-
} else if (settingsDataStore.clearDuckAiData && domainForMatchingAllowedKey == DUCKDUCKGO_DOMAIN) {
89-
if (keysToDelete.any { key.endsWith(it) }) {
90-
db.delete(entry.key)
91-
logcat { "WebLocalStorageManager: Deleted key: $key" }
92-
}
9388
}
9489
}
9590
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
8282
}
8383

8484
@Test
85-
fun whenClearWebLocalStorageThenIteratorSeeksToFirst() {
85+
fun whenClearWebLocalStorageThenIteratorSeeksToFirst() = runTest {
8686
whenever(mockDB.iterator()).thenReturn(mockIterator)
8787

8888
testee.clearWebLocalStorage()
@@ -91,7 +91,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
9191
}
9292

9393
@Test
94-
fun whenClearWebLocalStorageThenDeletesNonAllowedKeys() {
94+
fun whenClearWebLocalStorageThenDeletesNonAllowedKeys() = runTest {
9595
val key1 = bytes("_https://example.com\u0000\u0001key1")
9696
val key2 = bytes("_https://duckduckgo.com\u0000\u0001key2")
9797
val entry1 = createMockDBEntry(key1)
@@ -108,7 +108,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
108108
}
109109

110110
@Test
111-
fun whenClearWebLocalStorageThenDoesNotDeleteKeysWithValidSubdomains() {
111+
fun whenClearWebLocalStorageThenDoesNotDeleteKeysWithValidSubdomains() = runTest {
112112
val key1 = bytes("_https://subdomain.duckduckgo.com\u0000\u0001key1")
113113
val key2 = bytes("_https://another.subdomain.duckduckgo.com\u0000\u0001key2")
114114

@@ -126,7 +126,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
126126
}
127127

128128
@Test
129-
fun whenClearWebLocalStorageThenDeletesKeysWithPartialDomainMatch() {
129+
fun whenClearWebLocalStorageThenDeletesKeysWithPartialDomainMatch() = runTest {
130130
val key1 = bytes("_https://notduckduckgo.com\u0000\u0001key1")
131131
val key2 = bytes("_https://duckduckgo.something.com\u0000\u0001key2")
132132

@@ -144,7 +144,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
144144
}
145145

146146
@Test
147-
fun whenClearWebLocalStorageThenDeletesKeysWithInvalidFormat() {
147+
fun whenClearWebLocalStorageThenDeletesKeysWithInvalidFormat() = runTest {
148148
val key1 = bytes("malformed-key")
149149
val key2 = bytes("_https://duckduckgo.com")
150150
val key3 = bytes("_https://duckduckgo.com\u0000\u0001")
@@ -165,7 +165,7 @@ class DuckDuckGoWebLocalStorageManagerTest {
165165
}
166166

167167
@Test
168-
fun whenClearWebLocalStorageThenIteratorIsClosed() {
168+
fun whenClearWebLocalStorageThenIteratorIsClosed() = runTest {
169169
whenever(mockDB.iterator()).thenReturn(mockIterator)
170170
whenever(mockIterator.hasNext()).thenReturn(false)
171171

0 commit comments

Comments
 (0)