Store Page Context when received from JS#7997
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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: Wrong URL source used for storing page context
- The PAGE_CONTEXT handler now reads the current WebView URL on the main dispatcher via
getWebViewUrl()before storing page context, and the related test now asserts that exact URL is persisted.
- The PAGE_CONTEXT handler now reads the current WebView URL on the main dispatcher via
Or push these changes by commenting:
@cursor push 7cf6c97f91
Preview (7cf6c97f91)
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
@@ -4225,9 +4225,10 @@
val pageContext = pageContextJSHelper.processPageContext(featureName, method, data, tabId)
if (pageContext != null) {
if (androidBrowserConfig.storePageContext().isEnabled()) {
+ val webViewUrl = withContext(dispatchers.main()) { getWebViewUrl() }
tabPageContextRepository.storePageContext(
tabId = tabId,
- url = url.orEmpty(),
+ url = webViewUrl.orEmpty(),
serializedPageContext = pageContext,
)
}
diff --git a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
--- a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
+++ b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
@@ -5601,7 +5601,7 @@
verify(mockTabPageContextRepository).storePageContext(
eq("abc"),
- anyString(),
+ eq("someUrl"),
eq(serializedContext),
)
}
app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Outdated
Show resolved
Hide resolved
9572e9e to
e3a463f
Compare
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: Unhandled storage exception breaks existing page context flow
- Wrapped page-context persistence in runCatching with error logging so storage failures no longer interrupt enrichment and PageContextReceived dispatch.
Or push these changes by commenting:
@cursor push dedaca5dd7
Preview (dedaca5dd7)
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
@@ -4226,11 +4226,15 @@
if (pageContext != null) {
if (androidBrowserConfig.storePageContext().isEnabled()) {
val pageContextUrl = JSONObject(pageContext).optString("url", "")
- tabPageContextRepository.storePageContext(
- tabId = tabId,
- url = pageContextUrl,
- serializedPageContext = pageContext,
- )
+ runCatching {
+ tabPageContextRepository.storePageContext(
+ tabId = tabId,
+ url = pageContextUrl,
+ serializedPageContext = pageContext,
+ )
+ }.onFailure {
+ logcat(ERROR) { "Failed to store page context: ${it.asLog()}" }
+ }
}
val enrichedContext = enrichPageContextIfPossible(pageContext)e3a463f to
539ebce
Compare
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:
runCatchingswallowsCancellationExceptionfrom suspend call- I kept the existing
runCatchingbehavior but re-throwCancellationExceptioninonFailureso coroutine cancellation now propagates correctly.
- I kept the existing
Or push these changes by commenting:
@cursor push bef3344a41
Preview (bef3344a41)
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
@@ -4232,6 +4232,8 @@
url = pageContextUrl,
serializedPageContext = pageContext,
)
+ }.onFailure { throwable ->
+ if (throwable is kotlinx.coroutines.CancellationException) throw throwable
}
}539ebce to
1da0f1f
Compare
3801308 to
6b66f7c
Compare
malmstein
left a comment
There was a problem hiding this comment.
works as expected, nice work!
6b66f7c to
889ab10
Compare



Task/Issue URL: https://app.asana.com/1/137249556945/project/1212810093780571/task/1213484200004108?focus=true
Description
Adds the ability to store tab content which will be used later for attaching tabs to ai chat prompts. It will be gated by a new feature flag
storePageContextand currently only hooked to the existing PAGE_CONTEXT_FEATURE_NAME handler.So at this point it will only be triggered by opening the contextual ai chat. This will allow us to test the storage without impact on existing logic.
Steps to test this PR (See video below)
storePageContextfeature flag (under androidBrowserConfig)Notes:
Database Demo Video
Store.Page.Context.DB.demo.mov
Note
Medium Risk
Introduces a new Room table and migration (60→61) plus new write-path in
BrowserTabViewModel, so there’s some risk of migration issues and additional on-device storage of page content when the flag is enabled.Overview
Adds a new
androidBrowserConfig.storePageContexttoggle to optionally persist the JSPAGE_CONTEXT_FEATURE_NAMEpayload for a tab.Bumps Room DB to v61 and introduces
tab_page_context(FK totabs, cascade delete) with a newTabPageContextRepository/DAO/entity implementation, wired intoBrowserTabViewModelwith failure logging and updated DI + tests.Written by Cursor Bugbot for commit 889ab10. This will update automatically on new commits. Configure here.