flawless: harden PR #144 — bounded fallback (isPassword, MainThread, tests) - #147
Merged
Conversation
findFocus(FOCUS_INPUT) stays the first choice and keeps precedence, but some hosts expose the editable input deeper in the tree than findFocus reaches. When findFocus reports nothing, walk the tree for the node that is both editable and focused. The walk is bounded (500-node budget, depth cap 32) so a deep tree cannot spike the main thread, and recycles every visited node on every exit path — including when an accessor throws mid-walk — except the returned match. Extracted behind a FocusNode seam so it is unit-tested with a fake tree. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…c, double-recycle clarity, expanded tests
Contributor
📦 Preview APK — PR #147 ·
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the focused-editable node fallback in the accessibility service by introducing a unit-testable seam (FocusNode) and a bounded tree-walk fallback used only when findFocus(FOCUS_INPUT) returns null, with additional password-field filtering and expanded JVM tests.
Changes:
- Add
FocusedEditableFinderbounded DFS fallback (budget + depth cap) with aFocusNodeabstraction andAccessibilityNodeInfobridge. - Wire the two-stage fallback into
AssistantService.findFocusedEditableSource()and clarify ownership/recycling behavior in KDoc. - Add comprehensive JVM tests covering recycling, caps, null children, and exception paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/src/main/java/com/musheer360/swiftslate/service/FocusedEditableFinder.kt | Introduces bounded fallback finder + FocusNode seam and AccessibilityNodeInfo bridge. |
| app/src/main/java/com/musheer360/swiftslate/service/AssistantService.kt | Uses two-stage focused-node lookup and documents recycling/ownership behavior. |
| app/src/test/java/com/musheer360/swiftslate/service/FocusedEditableFinderTest.kt | Adds JVM tests to validate bounded walk behavior and recycling guarantees. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+360
to
+362
| * (or all on miss). The outer catch's `safeRecycle` is a safety net for the rare case where | ||
| * an exception escapes before the finder takes ownership; double-recycle is benign | ||
| * (`safeRecycle` catches `IllegalStateException`, and on API 33+ `recycle()` is a no-op). |
Comment on lines
+68
to
+72
| val childCount = node.childCount | ||
| for (i in 0 until childCount) { | ||
| val child = node.getChild(i) ?: continue | ||
| val found = walk(child, depth + 1) | ||
| if (found != null) return found |
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
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.
Supersedes #144 with flawless hardening. Original #144 by guoxpeng is MERGEABLE CLEAN and ready with nits; this branch adds defense-in-depth isPassword seam, MainThread docs, double-recycle clarity, expanded tests (12 tests, budget wide-tree, isEditable/isFocused/isPassword throw, null child, password filter). Local testDebugUnitTest BUILD SUCCESSFUL, lintDebug BUILD SUCCESSFUL, subagent READ-ONLY review: PERFECT (conditionally, after rebase). Rebased onto origin/master 50d192a so diff is only 3 files.