feat: multi-browser web watcher via Accessibility Service#151
feat: multi-browser web watcher via Accessibility Service#151FractalMachinist wants to merge 8 commits into
Conversation
Picks up PR #547 (merged Dec 2025) which fixes a panic in jstring_to_string when JNI passes modified UTF-8 strings (e.g. app names containing emoji). The old CStr::to_str().unwrap() path would abort on surrogate pairs; the fix uses jstr.into() which handles the JNI encoding correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Firefox ~v130+ uses a Jetpack Compose toolbar where the URL lives in the content-desc of a node with viewIdResourceName "ADDRESSBAR_URL_BOX". Two issues prevented the original approach from working: 1. findAccessibilityNodeInfosByViewId silently returns empty for IDs that don't contain ":" (requires "package:id/name" format); bare Compose testTag names like ADDRESSBAR_URL_BOX are rejected. 2. The toolbar is a sibling of the WebView content area, so searching from event.source never reaches it. Fix: use rootInActiveWindow as search root and traverse the tree manually with findNodeByResourceName (compares viewIdResourceName directly). The view-based fallback IDs (url_bar_title, mozac_browser_toolbar_url_view) are retained for older Firefox versions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
If findWebView(child) returns child itself (i.e. the child IS the
matching WebView), the previous .also { child.recycle() } destroyed the
node before the caller could use it. Only recycle when the returned node
is not the child being searched.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rowser When a supported browser's URL extractor returns null, dump the full accessibility tree to logcat at debug level (tag: WebWatcher), rate-limited to once per minute per browser package. This gives bug reporters actionable data — the actual view IDs and content descriptions present — without requiring manual instrumentation. Addresses the unresolved Samsung Internet and Chrome reports from the original PR, where no logs were available to diagnose the failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR replaces the Chrome-only
Confidence Score: 3/5The core of the PR — WebWatcher's accessibility event handling — has two correctness issues in newly added code that will affect users on Android 7–12 devices. The mobile/src/main/java/net/activitywatch/android/watcher/WebWatcher.kt — specifically Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
AE[onAccessibilityEvent] --> SI{systemui?}
SI -->|yes| SKIP[return]
SI -->|no| PKG[resolve packageName]
PKG --> EX{urlExtractor exists?}
EX -->|no| WC{windowChanged?}
WC -->|yes| HU0[handleUrl null, null]
WC -->|no| SKIP
EX -->|yes| SRC[event.source != null?]
SRC -->|no| SKIP
SRC -->|yes| UE[urlExtractor event]
UE --> UNL{url == null?}
UNL -->|yes| DT[maybeDumpTree rate-limited 1/min]
UNL -->|no| HU[handleUrl newUrl, browser]
HU --> FWV[findWebView source]
FWV --> WT[handleWindowTitle]
WT --> LOG[logBrowserEvent heartbeatHelper]
DT --> RI[rootInActiveWindow dumpNode tree to logcat]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
AE[onAccessibilityEvent] --> SI{systemui?}
SI -->|yes| SKIP[return]
SI -->|no| PKG[resolve packageName]
PKG --> EX{urlExtractor exists?}
EX -->|no| WC{windowChanged?}
WC -->|yes| HU0[handleUrl null, null]
WC -->|no| SKIP
EX -->|yes| SRC[event.source != null?]
SRC -->|no| SKIP
SRC -->|yes| UE[urlExtractor event]
UE --> UNL{url == null?}
UNL -->|yes| DT[maybeDumpTree rate-limited 1/min]
UNL -->|no| HU[handleUrl newUrl, browser]
HU --> FWV[findWebView source]
FWV --> WT[handleWindowTitle]
WT --> LOG[logBrowserEvent heartbeatHelper]
DT --> RI[rootInActiveWindow dumpNode tree to logcat]
Reviews (2): Last reviewed commit: "fix: address greptile's identified bugs ..." | Re-trigger Greptile |
|
I think you mentioned wrong account :) |
- Fix memory leak in findNodeByResourceName: recycle child before returning found node - Fix NPE in exception handling: use ex.message ?: ex.toString() instead of ex.message!! - Fix Intent flag combination: use 'or' instead of '+' for proper bitwise operation - Fix queue sync bug: consume NAVIGATION_FINISHED event with poll() instead of peek() - Fix unclosed file descriptor: close ParcelFileDescriptor from executeShellCommand() Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
@ErikBjare I believe this is ready for review. I don't see how to request your review directly. |
|
@greptileai review @FractalMachinist I'll do my best to not forget about this, do ping me again if I do! Thanks a lot for contributing! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Fixes a real self-recycle bug in the Firefox node-finder that could silently break URL extraction (or throw on pre-API-33 devices), plus a handful of correctness/robustness issues found in review: - Firefox address-bar regex: was non-greedy (truncated URLs containing their own ". ") and assumed an ASCII-capitalized hint suffix (broke non-Latin locales). Now greedy and locale-independent. - Blank/cleared address-bar text is now filtered for all browsers, not just Firefox, to avoid spurious empty-URL sessions. - Page title capture no longer depends on URL extraction succeeding in the same event. - Session duration is clamped to zero against backward clock steps (NTP sync, manual clock change). - Normalized protocol-stripping across all browsers/extractors (fixes Samsung Internet reporting differently formatted URLs depending on which of its two toolbar variants matched). - Unified the three near-duplicate recursive AccessibilityNodeInfo walkers into one generic find/forEach pair, and recycled two previously-leaked rootInActiveWindow references. - Extracted the regex/text-processing and session-state-machine logic into pure, unit-testable units (UrlExtraction.kt, BrowserSessionTracker.kt) with new JVM tests. - Fixed CustomTabsWrapper's sticky-fallback/stale-event-queue bug in the instrumented test harness, and switched WebWatcherTest to the exception-safe RustInterface.getEventsJSON() accessor. Verified end-to-end: full app builds and unit tests pass, and the Firefox->URL detection path was confirmed live on a physical device against a real page load, with the resulting event persisted correctly to the on-device datastore. Also documents (TODO(maintainer) comments) two known gaps left unresolved: the release-build privacy exposure of maybeDumpTree's logcat output, and why Firefox's page title can't currently be captured (GeckoView's content isn't a descendant of the toolbar's accessibility window at all - confirmed via live tree dump). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Fixed the greptile-flagged bugs. Also normalized URL formatting across all browsers, not just Firefox — a couple of browsers' own toolbar variants disagreed with each other, which felt in-scope for "browser support" correctness. Added unit tests for the extraction/session logic while in there. Verified: unit tests pass, and a live device test confirms a real Firefox page load persists correctly end-to-end (checked the on-device datastore directly, not just logs). Needs your attention:
|
TimeToBuildBob
left a comment
TimeToBuildBob
left a comment
There was a problem hiding this comment.
Great contribution overall — the test suite is particularly strong: BrowserSessionTrackerTest uses a FakeClock pattern that makes duration math deterministic, the backward-clock clamp test is a nice edge case, and UrlExtractionTest covers Firefox-specific parsing edge cases (locale-independent separator, greedy regex regression). A few things to address before merge:
🔒 Privacy concern — should block merge
WebWatcher.maybeDumpTree logs the full accessibility tree to logcat (via Log.d) in production builds. Log.d calls are not stripped by R8/ProGuard by default, so this will emit potentially sensitive browsing content (page text, content descriptions) to logcat on end-user devices. The contributor's own TODO documents this:
"this can include page text/content-descriptions (potentially sensitive browsing content) and is not currently gated to debug builds"
Suggested fix — enable buildFeatures.buildConfig in mobile/build.gradle and gate the call:
// in mobile/build.gradle:
buildFeatures {
buildConfig = true
}// in WebWatcher.kt, before calling maybeDumpTree:
if (BuildConfig.DEBUG) {
maybeDumpTree(packageName)
}Alternative with no build config change: if (Log.isLoggable(TAG, Log.DEBUG)) respects adb log-level overrides while not logging in typical production builds.
📁 .idea/ files
.idea/misc.xml and .idea/vcs.xml contain personal Android Studio state and should be reverted. They're usually in .gitignore — if they're not, this is a good time to add them.
📦 aw-server-rust submodule bump
The submodule is bumped from dc70318e → 1e9c3580. Is this needed for the WebWatcher feature, or is it incidental? Submodule bumps mixed into feature PRs make it harder to bisect and review — better to keep them separate if possible.
Once the privacy gate is in place and the .idea/ changes are reverted, this looks ready. The multi-browser approach via AccessibilityService is the right direction, and the thorough test coverage (unit + E2E) is exactly what this kind of Accessibility-dependent code needs.
|
I strongly agree with the proposed build config. Since build settings reach beyond the ideal scope of this PR, I left it as a decision for @ErikBjare
I discuss the submodule bump in my first comment. TL;DR yes, I strongly recommend the bump, and it could be a separate PR. Erik, tell me what you want done with build configs and submodules, I've got time today. |
|
@FractalMachinist — thanks for the excellent work and thorough responses. Here's my direction on the outstanding items: Privacy gate on
|
|
Sorry for the conflicts here... I rebase-merged #139 which also builds on the same commits you have, so hopefully a rebase should skip those and the conflict not be too painful to resolve. |
AccessibilityNodeInfo Lifecycle IssueThe Greptile findings are accurate — Current code (leaks node):private fun extractTextByViewId(event: AccessibilityEvent, viewId: String): String? =
processExtractedText(
event.source?.findAccessibilityNodeInfosByViewId(viewId)?.firstOrNull()?.text?.toString()
)Problem: Fixed version:private fun extractTextByViewId(event: AccessibilityEvent, viewId: String): String? {
event.source?.let { source ->
val nodes = source.findAccessibilityNodeInfosByViewId(viewId)
try {
return processExtractedText(nodes.firstOrNull()?.text?.toString())
} finally {
nodes.forEach { it.recycle() }
}
}
return null
}This follows the same pattern as The |
Rebase onto current masterThe PR branch is conflicting due to the recent merge of #139. Here's how to rebase cleanly: # Fetch current master
git fetch origin master
# Rebase only the 8 PR-specific commits (not the entire fork history)
MERGE_BASE=$(git merge-base origin/master HEAD) # should be 9d32040
git rebase --onto origin/master $MERGE_BASE HEADConflict resolution guide for commit 2 (dep bump):The Conflict resolution guide for commit 3 (WebWatcher feature):
For For Also apply the |
6 participants
adding a generic WebWatcher that tracks browser URLs via Android's Accessibility Service. Verified Firefox working on Android 14 (Fairphone 6).
What this adds
ChromeWatcherwith aWebWatchersupporting Chrome, Firefox, Samsung Internet, Opera, and Edgeurl,title,browser,audible, andincognitofields per theweb.tab.currentevent typeFirefox Compose toolbar fix
Firefox ~v130+ migrated its address bar to Jetpack Compose. The original PR's approach (
findAccessibilityNodeInfosByViewId("ADDRESSBAR_URL_BOX")) silently returns empty results because Android requires IDs in"package:id/name"format and rejects bare Compose testTag names. Additionally, the toolbar is a sibling of the WebView content area, so searching fromevent.sourcenever reaches it.Fix: search from
rootInActiveWindowand traverse the tree manually, comparingviewIdResourceNamedirectly. The view-based fallback IDs (url_bar_title,mozac_browser_toolbar_url_view) are retained for older Firefox versions.Fixes from code review
findWebViewwherechild.recycle()was called even whenchildwas the node being returned, leaving the caller with a recycled reference (flagged by ellipsis-dev in the original PR).Testing
Verified on one physical device (Fairphone 6, Android 14) with Firefox 138. Chrome, Edge, Samsung Internet, and Opera have not been tested by the authors of this revision — the view IDs for those browsers are carried over from the original PR unchanged. The diagnostic tree logging (described below) was added specifically because we lack broad device/browser coverage. If you or a reviewer can test on Chrome or Samsung Internet and URL extraction fails, enabling debug logging (
adb logcat -s WebWatcher:D) will produce a dump of the accessibility tree that should make it straightforward to identify the correct view IDs. We believe the Firefox fix is correct because it was verified end-to-end (URLs and page titles appearing in the ActivityWatch timeline). We believe the recycle fix is correct by code inspection. For the untested browsers, confidence rests entirely on the original PR author's view IDs being stable across the versions you'll encounter.Diagnostic logging for unresolved browser issues
The original PR had unresolved reports of Chrome stopping mid-session and Samsung Internet never producing data. The Chrome issue was almost certainly the JNI emoji crash described below — the service would crash silently and stop processing events. For future reports,
WebWatchernow dumps the full accessibility tree to logcat (tag:WebWatcher, debug level) when a known browser's URL extractor returns null, rate-limited to once per minute per browser. This gives reporters actionable data without manual instrumentation.aw-server-rust submodule bump
This PR bumps
aw-server-rustto current master to pick up PR #547, which fixes a panic injstring_to_stringwhen JNI passes modified UTF-8 strings (e.g. app names containing emoji). Without this fix the service crashes silently on any heartbeat where the browser has a non-BMP character in its app name, which likely explains the "Chrome stopped working" report in the original PR thread.Happy to separate the submodule bump into a standalone PR if you prefer to take it independently.
Credits
Original implementation by @KonradKrol (incubly-oss/aw-android#138).