Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Concurrent deleteChat calls corrupt shared singleton state
- Added a Mutex to wrap the entire deleteChat method, ensuring only one concurrent call can access the shared mutable state at a time.
- ✅ Fixed: Duplicate JSONObjectAdapter in same Gradle module
- Extracted the duplicate JSONObjectAdapter class to a shared location at duckchat-impl/common/JSONObjectAdapter.kt and updated both files to import from there.
- ✅ Fixed: Chat not deleted when tab navigated away
- Added TabChatIdsRepository to track chatIds when Duck.ai URLs are visited, and DataClearing now uses this repository to delete all chats associated with a tab regardless of current URL.
Or push these changes by commenting:
@cursor push 20b1229255
Preview (20b1229255)
diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
--- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
+++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
@@ -242,6 +242,7 @@
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository
import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ALWAYS
import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ASK_EVERY_TIME
+import com.duckduckgo.app.fire.store.TabChatIdsRepository
import com.duckduckgo.app.fire.store.TabVisitedSitesRepository
import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler
import com.duckduckgo.app.global.events.db.UserEventKey
@@ -508,6 +509,7 @@
private val serpEasterEggLogosToggles: SerpEasterEggLogosToggles,
private val serpLogos: SerpLogos,
private val tabVisitedSitesRepository: TabVisitedSitesRepository,
+ private val tabChatIdsRepository: TabChatIdsRepository,
private val pageLoadWideEvent: PageLoadWideEvent,
private val queryUrlPredictor: QueryUrlPredictor,
private val browserUiLockFeature: BrowserUiLockFeature,
@@ -993,6 +995,9 @@
if (domain != null) {
tabVisitedSitesRepository.recordVisitedSite(tabId, domain)
}
+ duckChat.extractChatId(url)?.let { chatId ->
+ tabChatIdsRepository.recordChatId(tabId, chatId)
+ }
}
}
}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt
@@ -17,6 +17,7 @@
package com.duckduckgo.app.fire
import com.duckduckgo.app.fire.store.FireDataStore
+import com.duckduckgo.app.fire.store.TabChatIdsRepository
import com.duckduckgo.app.fire.store.TabVisitedSitesRepository
import com.duckduckgo.app.fire.wideevents.DataClearingWideEvent
import com.duckduckgo.app.global.view.ClearDataAction
@@ -28,7 +29,6 @@
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.duckchat.api.DuckAiChatClearer
import com.duckduckgo.duckchat.api.DuckAiFeatureState
-import com.duckduckgo.duckchat.api.DuckChat
import com.duckduckgo.history.api.NavigationHistory
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
@@ -57,9 +57,9 @@
private val duckAiFeatureState: DuckAiFeatureState,
private val dataClearingWideEvent: DataClearingWideEvent,
private val tabVisitedSitesRepository: TabVisitedSitesRepository,
+ private val tabChatIdsRepository: TabChatIdsRepository,
private val navigationHistory: NavigationHistory,
private val tabRepository: TabRepository,
- private val duckChat: DuckChat,
private val duckAiChatClearer: DuckAiChatClearer,
) : ManualDataClearing, AutomaticDataClearing {
@@ -69,8 +69,7 @@
val visitedSites = tabVisitedSitesRepository.getVisitedSites(tabId)
val clearDataResult = clearDataAction.clearDataForSpecificDomains(visitedSites)
- val tabUrl = tabRepository.getTab(tabId)?.url
- clearDuckAiChatIfNeeded(tabUrl)
+ clearDuckAiChatsForTab(tabId)
navigationHistory.removeHistoryForTab(tabId)
tabRepository.deleteTabAndSelectSource(tabId)
@@ -79,10 +78,12 @@
return clearDataResult
}
- private suspend fun clearDuckAiChatIfNeeded(tabUrl: String?) {
- if (tabUrl == null) return
- val chatId = duckChat.extractChatId(tabUrl) ?: return
- duckAiChatClearer.deleteChat(chatId)
+ private suspend fun clearDuckAiChatsForTab(tabId: String) {
+ val chatIds = tabChatIdsRepository.getChatIds(tabId)
+ for (chatId in chatIds) {
+ duckAiChatClearer.deleteChat(chatId)
+ }
+ tabChatIdsRepository.clearTab(tabId)
}
override suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean, wasAppUsedSinceLastClear: Boolean) {
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdEntity.kt
@@ -1,0 +1,28 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import androidx.room.Entity
+
+@Entity(
+ tableName = "tab_chat_ids",
+ primaryKeys = ["tabId", "chatId"],
+)
+data class TabChatIdEntity(
+ val tabId: String,
+ val chatId: String,
+)
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsDao.kt
@@ -1,0 +1,37 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import androidx.room.Dao
+import androidx.room.Insert
+import androidx.room.OnConflictStrategy
+import androidx.room.Query
+
+@Dao
+interface TabChatIdsDao {
+ @Insert(onConflict = OnConflictStrategy.IGNORE)
+ suspend fun insert(entity: TabChatIdEntity)
+
+ @Query("SELECT chatId FROM tab_chat_ids WHERE tabId = :tabId")
+ suspend fun getChatIds(tabId: String): List<String>
+
+ @Query("DELETE FROM tab_chat_ids WHERE tabId = :tabId")
+ suspend fun clearTab(tabId: String)
+
+ @Query("DELETE FROM tab_chat_ids")
+ suspend fun clearAll()
+}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt
new file mode 100644
--- /dev/null
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabChatIdsRepository.kt
@@ -1,0 +1,66 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.fire.store
+
+import com.duckduckgo.di.scopes.AppScope
+import com.squareup.anvil.annotations.ContributesBinding
+import dagger.SingleInstanceIn
+import javax.inject.Inject
+
+/**
+ * Tracks Duck.ai chat IDs associated with browser tabs.
+ *
+ * Used during single-tab burning to ensure chat data is cleared
+ * even if the user navigated away from the Duck.ai chat URL.
+ */
+interface TabChatIdsRepository {
+
+ /** Records that a Duck.ai chat with [chatId] was opened in the given [tabId]. */
+ suspend fun recordChatId(tabId: String, chatId: String)
+
+ /** Returns the set of chat IDs that were opened in the given [tabId]. */
+ suspend fun getChatIds(tabId: String): Set<String>
+
+ /** Removes all chat ID records for the given [tabId]. */
+ suspend fun clearTab(tabId: String)
+
+ /** Removes all chat ID records across all tabs. */
+ suspend fun clearAll()
+}
+
+@ContributesBinding(AppScope::class)
+@SingleInstanceIn(AppScope::class)
+class RealTabChatIdsRepository @Inject constructor(
+ private val dao: TabChatIdsDao,
+) : TabChatIdsRepository {
+
+ override suspend fun recordChatId(tabId: String, chatId: String) {
+ dao.insert(TabChatIdEntity(tabId = tabId, chatId = chatId))
+ }
+
+ override suspend fun getChatIds(tabId: String): Set<String> {
+ return dao.getChatIds(tabId).toSet()
+ }
+
+ override suspend fun clearTab(tabId: String) {
+ dao.clearTab(tabId)
+ }
+
+ override suspend fun clearAll() {
+ dao.clearAll()
+ }
+}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesDatabase.kt
@@ -21,9 +21,10 @@
@Database(
exportSchema = true,
- version = 1,
- entities = [TabVisitedSiteEntity::class],
+ version = 2,
+ entities = [TabVisitedSiteEntity::class, TabChatIdEntity::class],
)
abstract class TabVisitedSitesDatabase : RoomDatabase() {
abstract fun dao(): TabVisitedSitesDao
+ abstract fun chatIdsDao(): TabChatIdsDao
}
diff --git a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
--- a/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
+++ b/app/src/main/java/com/duckduckgo/app/fire/store/TabVisitedSitesModule.kt
@@ -39,4 +39,8 @@
@SingleInstanceIn(AppScope::class)
@Provides
fun provideDao(db: TabVisitedSitesDatabase): TabVisitedSitesDao = db.dao()
+
+ @SingleInstanceIn(AppScope::class)
+ @Provides
+ fun provideChatIdsDao(db: TabVisitedSitesDatabase): TabChatIdsDao = db.chatIdsDao()
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
@@ -18,6 +18,7 @@
import android.webkit.JavascriptInterface
import android.webkit.WebView
+import com.duckduckgo.duckchat.impl.common.JSONObjectAdapter
import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.JsMessage
import com.duckduckgo.js.messaging.api.JsMessageCallback
@@ -27,14 +28,8 @@
import com.duckduckgo.js.messaging.api.JsRequestResponse
import com.duckduckgo.js.messaging.api.SubscriptionEvent
import com.duckduckgo.js.messaging.api.SubscriptionEventData
-import com.squareup.moshi.FromJson
-import com.squareup.moshi.JsonReader
-import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
-import com.squareup.moshi.ToJson
import logcat.logcat
-import okio.Buffer
-import org.json.JSONException
import org.json.JSONObject
import javax.inject.Inject
@@ -125,21 +120,3 @@
const val METHOD_CLEAR_DATA_FAILED = "duckAiClearDataFailed"
}
}
-
-internal class JSONObjectAdapter {
- @FromJson
- fun fromJson(reader: JsonReader): JSONObject? {
- return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
- try {
- JSONObject(data)
- } catch (_: JSONException) {
- null
- }
- }
- }
-
- @ToJson
- fun toJson(writer: JsonWriter, value: JSONObject?) {
- value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
- }
-}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckAiChatClearer.kt
@@ -37,6 +37,8 @@
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import logcat.logcat
@@ -55,6 +57,7 @@
private val duckAiDataClearingFeature: DuckAiDataClearingFeature,
) : DuckAiChatClearer {
+ private val mutex = Mutex()
private var webView: WebView? = null
private var pageLoadDeferred: CompletableDeferred<Unit>? = null
private var readyDeferred: CompletableDeferred<Unit>? = null
@@ -64,25 +67,27 @@
private var cachedScript: String? = null
override suspend fun deleteChat(chatId: String): Boolean {
- return withContext(dispatchers.main()) {
- try {
- val script = getScript()
- val wv = getOrCreateWebView(script)
+ return mutex.withLock {
+ withContext(dispatchers.main()) {
+ try {
+ val script = getScript()
+ val wv = getOrCreateWebView(script)
- var allSucceeded = true
- for (domain in DOMAINS) {
- val success = clearFromDomain(wv, domain, chatId)
- if (!success) {
- logcat { "DuckAiChatClearer: clearing failed for domain $domain, chatId $chatId" }
- allSucceeded = false
+ var allSucceeded = true
+ for (domain in DOMAINS) {
+ val success = clearFromDomain(wv, domain, chatId)
+ if (!success) {
+ logcat { "DuckAiChatClearer: clearing failed for domain $domain, chatId $chatId" }
+ allSucceeded = false
+ }
}
+ allSucceeded
+ } catch (e: Exception) {
+ logcat { "DuckAiChatClearer: deleteChat failed with ${e.message}" }
+ false
+ } finally {
+ tearDown()
}
- allSucceeded
- } catch (e: Exception) {
- logcat { "DuckAiChatClearer: deleteChat failed with ${e.message}" }
- false
- } finally {
- tearDown()
}
}
}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt
new file mode 100644
--- /dev/null
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/common/JSONObjectAdapter.kt
@@ -1,0 +1,43 @@
+/*
+ * Copyright (c) 2026 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.duckchat.impl.common
+
+import com.squareup.moshi.FromJson
+import com.squareup.moshi.JsonReader
+import com.squareup.moshi.JsonWriter
+import com.squareup.moshi.ToJson
+import okio.Buffer
+import org.json.JSONException
+import org.json.JSONObject
+
+internal class JSONObjectAdapter {
+ @FromJson
+ fun fromJson(reader: JsonReader): JSONObject? {
+ return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
+ try {
+ JSONObject(data)
+ } catch (_: JSONException) {
+ null
+ }
+ }
+ }
+
+ @ToJson
+ fun toJson(writer: JsonWriter, value: JSONObject?) {
+ value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
+ }
+}
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/suggestions/reader/ChatSuggestionsJsMessaging.kt
@@ -18,6 +18,7 @@
import android.webkit.JavascriptInterface
import android.webkit.WebView
+import com.duckduckgo.duckchat.impl.common.JSONObjectAdapter
import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.JsMessage
import com.duckduckgo.js.messaging.api.JsMessageCallback
@@ -27,14 +28,8 @@
import com.duckduckgo.js.messaging.api.JsRequestResponse
import com.duckduckgo.js.messaging.api.SubscriptionEvent
import com.duckduckgo.js.messaging.api.SubscriptionEventData
-import com.squareup.moshi.FromJson
-import com.squareup.moshi.JsonReader
-import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
-import com.squareup.moshi.ToJson
import logcat.logcat
-import okio.Buffer
-import org.json.JSONException
import org.json.JSONObject
import javax.inject.Inject
@@ -117,21 +112,3 @@
const val JS_INTERFACE_NAME = "chatSuggestionsInterface"
}
}
-
-internal class JSONObjectAdapter {
- @FromJson
- fun fromJson(reader: JsonReader): JSONObject? {
- return (reader.readJsonValue() as? Map<*, *>)?.let { data ->
- try {
- JSONObject(data)
- } catch (_: JSONException) {
- null
- }
- }
- }
-
- @ToJson
- fun toJson(writer: JsonWriter, value: JSONObject?) {
- value?.let { writer.run { value(Buffer().writeUtf8(value.toString())) } }
- }
-}
...hat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/RealDuckChatDeleter.kt
Show resolved
Hide resolved
...hat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckAiChatClearerJsMessaging.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Feature flag gates chat ID extraction, blocking deletion
- Extracted URL-matching logic into matchesDuckChatUrlPattern so extractChatId no longer depends on the feature flag, allowing chat deletion regardless of feature flag state.
Or push these changes by commenting:
@cursor push d57be15e83
Preview (d57be15e83)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
@@ -642,7 +642,16 @@
override fun isDuckChatUrl(uri: Uri): Boolean {
if (!isDuckChatFeatureEnabled) return false
+ return matchesDuckChatUrlPattern(uri)
+ }
+ override fun extractChatId(url: String): String? {
+ val uri = Uri.parse(url) ?: return null
+ if (!matchesDuckChatUrlPattern(uri)) return null
+ return uri.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() }
+ }
+
+ private fun matchesDuckChatUrlPattern(uri: Uri): Boolean {
if (isDuckChatBang(uri)) return true
if (uri.host == DUCK_AI_HOST || uri.toString() == DUCK_AI_HOST) return true
@@ -654,12 +663,6 @@
}.getOrDefault(false)
}
- override fun extractChatId(url: String): String? {
- val uri = Uri.parse(url) ?: return null
- if (!isDuckChatUrl(uri)) return null
- return uri.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() }
- }
-
private fun isDuckChatBang(uri: Uri): Boolean = bangRegex?.containsMatchIn(uri.toString()) == true
override suspend fun wasOpenedBefore(): Boolean = duckChatFeatureRepository.wasOpenedBefore()
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Feature flag dependency changes
hasChatIdUI behavior- Changed hasChatId to perform a direct URL parse using toUri().getQueryParameter() instead of delegating to duckChat.extractChatId() which has feature flag dependency.
Or push these changes by commenting:
@cursor push 7444da476b
Preview (7444da476b)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt
@@ -16,6 +16,7 @@
package com.duckduckgo.duckchat.impl.contextual
+import androidx.core.net.toUri
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.duckduckgo.anvil.annotations.ContributesViewModel
@@ -30,6 +31,7 @@
import com.duckduckgo.duckchat.impl.store.DuckChatContextualDataStore
import com.duckduckgo.js.messaging.api.SubscriptionEventData
import com.google.android.material.bottomsheet.BottomSheetBehavior
+import javax.inject.Inject
import kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.MutableStateFlow
@@ -41,7 +43,6 @@
import kotlinx.coroutines.withContext
import logcat.logcat
import org.json.JSONObject
-import javax.inject.Inject
@ContributesViewModel(FragmentScope::class)
class DuckChatContextualViewModel @Inject constructor(
@@ -529,9 +530,13 @@
}
private fun hasChatId(url: String?): Boolean {
- return url != null && duckChat.extractChatId(url) != null
+ return url?.toUri()?.getQueryParameter(CHAT_ID_PARAM)?.isNotBlank() == true
}
+ companion object {
+ private const val CHAT_ID_PARAM = "chatID"
+ }
+
private suspend fun shouldReuseStoredChatUrl(tabId: String): Boolean {
val lastClosedTimestamp = contextualDataStore.getTabClosedTimestamp(tabId) ?: return true
val timeoutMs = sessionTimeoutProvider.sessionTimeoutMillis()| return url?.toUri()?.getQueryParameter("chatID") | ||
| .orEmpty() | ||
| .isNotBlank() | ||
| return url != null && duckChat.extractChatId(url) != null |
There was a problem hiding this comment.
Feature flag dependency changes hasChatId UI behavior
Medium Severity
The refactored hasChatId now delegates to duckChat.extractChatId(url), which internally calls isDuckChatUrl(uri). That method returns false when isDuckChatFeatureEnabled is false. The old implementation was a pure URL parse (url?.toUri()?.getQueryParameter("chatID")) with no feature-flag dependency. This means hasChatId now returns false for valid Duck.ai URLs with a chatID whenever the feature toggle is off, affecting showFullscreen state and chat context storage decisions in the ViewModel — a behavioral change unrelated to the chat-deletion goal of this PR.
Please tell me if this was useful or not with a 👍 or 👎.
Additional Locations (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1213087856649984?focus=true
Description
This PR adds Duck.ai chat deletion when a Duck.ai tab is burned.
Steps to test this PR
For testing this feature, you need to use the following privacy config URL:
https://raw.githubusercontent.com/duckduckgo/privacy-configuration/90d195f34fb14f4eb47f519294376ad068e48df7/pr-4676/v3/android-config.jsonimprovedDataClearingOptionsandsingleTabFireDialogFFs enabledNote
Medium Risk
Touches data-clearing flows and introduces a headless WebView+JS messaging path to delete per-chat storage, which can be error-prone/flaky and risks deleting the wrong chat if URL parsing or messaging fails.
Overview
Single-tab burn now removes the specific Duck.ai conversation associated with the burned tab.
DataClearing.clearSingleTabDataextracts achatIDfrom the tab URL (via newDuckChat.extractChatId) and calls a newDuckChatDeleterto delete that chat’s local storage acrossduck.aiandduckduckgo.com.Granular site-data clearing was simplified:
ClearDataAction.clearDataForSpecificDomainsdrops theshouldClearDuckAiDataparameter and always excludes Duck.ai-related domains, since Duck.ai data is now cleared through the dedicated deleter.This adds the new deleter implementation (
RealDuckChatDeleter) using a headlessWebViewwith injectedduckAiDataClearing.jsand a new remote feature toggleduckAiDataClearing, plus refactors JSON Moshi support into a sharedJSONObjectAdapterand updates/extends tests accordingly.Written by Cursor Bugbot for commit 78df013. This will update automatically on new commits. Configure here.