perf(cua-driver): use on-screen-only SCShareableContent query for permission probe - #1378
Conversation
…mission probe (fixes trycua#1371) Replace `SCShareableContent.current` with `excludingDesktopWindows(false, onScreenWindowsOnly: true)` in the screen-recording TCC probe. The full `.current` call must resolve app names for all windows — including thousands of off-screen ghost windows left by crashed system processes — which can block for up to 6 seconds on affected M1 machines. The lighter on-screen-only query throws the same `SCStreamError.userDeclined` when the grant is absent, so permission detection remains accurate. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
|
Someone is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Screen Recording permission probe was optimized by replacing Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift (1)
25-31: Doc comment accurately reflects the new behavior.The rationale for preferring the on-screen-only query over
SCShareableContent.current(and still overCGPreflightScreenCaptureAccess()) is clearly captured. Note thatWindowCapture.swiftstill carries a near-duplicate block of commentary aboutCGPreflightScreenCaptureAccessfalse negatives; if you want to keep the project's permission-probe rationale discoverable from a single place, consider cross-referencing this doc fromWindowCapture.captureDisplayin a follow-up. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift` around lines 25 - 31, Add a brief cross-reference from WindowCapture.captureDisplay to the Permissions.swift doc comment explaining the on-screen-only SCShareableContent probe: update the documentation/comment block inside the WindowCapture.captureDisplay implementation to mention and link (by file/section name) to the Permissions.swift rationale about using SCShareableContent.excludingDesktopWindows(_:onScreenWindowsOnly:) vs CGPreflightScreenCaptureAccess(), so readers can find the single authoritative explanation without duplicating long commentary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift`:
- Around line 25-31: Add a brief cross-reference from
WindowCapture.captureDisplay to the Permissions.swift doc comment explaining the
on-screen-only SCShareableContent probe: update the documentation/comment block
inside the WindowCapture.captureDisplay implementation to mention and link (by
file/section name) to the Permissions.swift rationale about using
SCShareableContent.excludingDesktopWindows(_:onScreenWindowsOnly:) vs
CGPreflightScreenCaptureAccess(), so readers can find the single authoritative
explanation without duplicating long commentary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfc3ace9-30cc-4624-8214-8eb295ef9a7b
📒 Files selected for processing (1)
libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift
|
closed as deprecated by #2077 |
Fixes #1371
Problem
Permissions.currentStatus()callsSCShareableContent.currentto probe the Screen Recording TCC grant. This API enumerates all windows system-wide, including off-screen ones, and resolves the display name of each window's owner process. When a macOS system process (e.g.CursorUIViewService) has crashed and left thousands of zombie windows behind, the name-resolution loop can block for 6+ seconds on M1 hardware.Solution
Replace
SCShareableContent.currentwithSCShareableContent.excludingDesktopWindows(false, onScreenWindowsOnly: true). This limits the query to on-screen windows only, skipping the heavyweight off-screen window enumeration that triggers the slowdown.The lighter call still throws
SCStreamError.userDeclinedwhen the Screen Recording TCC grant is absent, so permission detection remains fully accurate — it just no longer stalls when a rogue system process is present.Testing
VideoRecorderandWindowCapturecode paths that need full window lists continue to useSCShareableContent.currentas-is — this change is scoped to the permission probe only.Summary by CodeRabbit