From 4a054b8e668b72020494654e56986b755f8d860d Mon Sep 17 00:00:00 2001 From: Serhii Bykov Date: Fri, 14 Aug 2026 23:08:33 +0200 Subject: [PATCH 1/2] refactor(event-tap): collapse dual taps into one --- .../CaptureController/CaptureController.swift | 14 +- .../Capture/EventTap/EventTap+Error.swift | 4 +- .../EventTap/EventTap+InstalledTap.swift | 17 -- .../Capture/EventTap/EventTap+Kind.swift | 34 --- .../Capture/EventTap/EventTap+State.swift | 8 +- .../Platform/Capture/EventTap/EventTap.swift | 157 ++++++------ .../Capture/EventTap/EventTapping.swift | 21 ++ .../CoreGraphics/CGEventType+Mask.swift | 24 ++ .../Capture/CaptureControllerTests.swift | 223 ++++++++++++++++++ .../CoreGraphics/CGEventTypeMaskTests.swift | 46 ++++ 10 files changed, 403 insertions(+), 145 deletions(-) delete mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+InstalledTap.swift delete mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Kind.swift create mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift create mode 100644 Apps/Keyty/Sources/Keyty/Support/Extensions/CoreGraphics/CGEventType+Mask.swift create mode 100644 Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift create mode 100644 Apps/Keyty/Tests/KeytyTests/Support/Extensions/CoreGraphics/CGEventTypeMaskTests.swift diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift index 08c5387d..f6c938a9 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift @@ -17,7 +17,7 @@ final class CaptureController { private var tapDisableCount: Int = 0 private let maxTapDisableCountBeforeReinstall = 3 - private let eventTap = EventTap() + private let eventTap: any EventTapping private let eventProcessor = EventProcessor() private let pointerVisualizersManager: PointerVisualizersManager private let keyboardVisualizer: KeyboardVisualizer @@ -27,11 +27,13 @@ final class CaptureController { init( pointerVisualizersManager: PointerVisualizersManager, keyboardVisualizer: KeyboardVisualizer, - permissionsService: any PermissionsService + permissionsService: any PermissionsService, + eventTap: any EventTapping = EventTap() ) { self.pointerVisualizersManager = pointerVisualizersManager self.keyboardVisualizer = keyboardVisualizer self.permissionsService = permissionsService + self.eventTap = eventTap self.eventTap.onOutput = { [weak self] output in self?.handle(output) } @@ -119,7 +121,6 @@ private extension CaptureController { // MARK: - Capture Lifecycle private extension CaptureController { func applyCapturing(_ capturing: Bool) { - guard !capturing || self.eventTap.isInstalled else { return } let wasCapturing = self.isCapturing Task { @MainActor [pointerVisualizersManager = self.pointerVisualizersManager, keyboardVisualizer = self.keyboardVisualizer] in pointerVisualizersManager.isPresentationActive = capturing @@ -200,8 +201,13 @@ private extension CaptureController { self.updateTapState(from: state) switch state { - case .idle, .installed: + case .idle: self.tapDisableCount = 0 + case .installed: + // `.installed` also follows every automatic re-enable, so it must not reset the + // count; otherwise repeated disables never reach the reinstall threshold. A real + // reinstall clears it by way of `remove()` driving the tap back to `.idle`. + break case .temporarilyDisabled: self.tapDisableCount += 1 guard self.tapDisableCount >= self.maxTapDisableCountBeforeReinstall else { return } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift index 5f30e6a1..e77da3ea 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift @@ -10,8 +10,8 @@ import Cocoa extension EventTap { enum Error: LocalizedError, Equatable { - /// A tap could not be created. - case creationFailed(Kind) + /// The tap could not be created. + case creationFailed var errorDescription: String? { L10n.EventTap.creationFailed diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+InstalledTap.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+InstalledTap.swift deleted file mode 100644 index 2f1d7b3d..00000000 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+InstalledTap.swift +++ /dev/null @@ -1,17 +0,0 @@ -// -// EventTap+InstalledTap.swift -// Keyty -// -// SPDX-FileCopyrightText: 2026 Serhii Bykov -// SPDX-License-Identifier: BSD-3-Clause -// - -import Cocoa - -extension EventTap { - /// A mach port and its run loop source, kept together so they are torn down together. - struct InstalledTap { - let machPort: CFMachPort - let runLoopSource: CFRunLoopSource - } -} diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Kind.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Kind.swift deleted file mode 100644 index f332eab1..00000000 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Kind.swift +++ /dev/null @@ -1,34 +0,0 @@ -// -// EventTap+Kind.swift -// Keyty -// -// SPDX-FileCopyrightText: 2026 Serhii Bykov -// SPDX-License-Identifier: BSD-3-Clause -// - -import Cocoa - -extension EventTap { - enum Kind: CaseIterable { - case key - case mouseAndFlags - } -} - -extension EventTap.Kind { - var eventsOfInterest: CGEventMask { - switch self { - case .key: - return Self.mask(.keyDown, .keyUp, .systemDefined) - case .mouseAndFlags: - return Self.mask(.leftMouseDown, .leftMouseUp, .rightMouseDown, .rightMouseUp, - .leftMouseDragged, .rightMouseDragged, - .otherMouseDown, .otherMouseUp, .otherMouseDragged, - .scrollWheel, .flagsChanged) - } - } - - private static func mask(_ types: CGEventType...) -> CGEventMask { - types.reduce(0) { $0 | CGEventMask(1 << $1.rawValue) } - } -} diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift index 6da41c88..2e174b7c 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift @@ -8,16 +8,16 @@ extension EventTap { enum State: Equatable { - /// No taps are installed, either before the first `install()` or after `remove()`. + /// No tap is installed, either before the first `install()` or after `remove()`. case idle - /// Both taps are installed and the system is delivering events. + /// The tap is installed and the system is delivering events. case installed - /// Still installed, but the system turned a tap off; it is re-enabled automatically. + /// Still installed, but the system turned the tap off; it is re-enabled automatically. case temporarilyDisabled(EventTap.DisableReason) - /// A tap could not be created, so nothing is installed. + /// The tap could not be created, so nothing is installed. case failed(EventTap.Error) } } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift index 3eacd973..527aa356 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift @@ -21,89 +21,108 @@ final class EventTap { } } - var isInstalled: Bool { - if case .installed = state { - return true - } - return false - } - - fileprivate var installedTaps: [Kind: InstalledTap] = [:] + /// Non-nil exactly while the tap is installed. `state` cannot stand in for this: + /// it reports `.temporarilyDisabled` while the tap is still installed. + private var handle: Handle? deinit { - if self.isInstalled { self.remove() } + if self.handle != nil { self.remove() } } } // MARK: - Public API extension EventTap { func install() throws(EventTap.Error) { - guard !self.isInstalled else { return } + guard self.handle == nil else { return } - for kind in Kind.allCases { - guard let installedTap = self.makeTap(kind) else { - self.resetInstalledResources() - self.state = .failed(.creationFailed(kind)) - throw .creationFailed(kind) - } - self.installedTaps[kind] = installedTap + guard let handle = Handle( + owner: self, + eventsOfInterest: Self.eventsOfInterest + ) else { + self.state = .failed(.creationFailed) + throw .creationFailed } + self.handle = handle self.state = .installed } func remove() { guard self.state != .idle else { return } - self.resetInstalledResources() + self.handle?.invalidate() + self.handle = nil self.state = .idle } } // MARK: - Tap Resources private extension EventTap { - func makeTap(_ kind: Kind) -> InstalledTap? { - guard let machPort = CGEvent.tapCreate( - tap: .cgSessionEventTap, - place: .headInsertEventTap, - options: .listenOnly, - eventsOfInterest: kind.eventsOfInterest, - callback: tapCallback(for: kind), - userInfo: Unmanaged.passUnretained(self).toOpaque() - ) else { - return nil - } + /// Every event type the app visualizes. Creation fails as a whole when the + /// Accessibility grant is missing, so a `nil` port is a truthful capability signal. + static let eventsOfInterest: CGEventMask = [ + .keyDown, .keyUp, .systemDefined, .flagsChanged, + .leftMouseDown, .leftMouseUp, .rightMouseDown, .rightMouseUp, + .leftMouseDragged, .rightMouseDragged, + .otherMouseDown, .otherMouseUp, .otherMouseDragged, + .scrollWheel + ].eventMask + + func reenableTap() { + guard let handle = self.handle else { return } + handle.enable() + self.state = .installed + } +} - guard let runLoopSource = CFMachPortCreateRunLoopSource(nil, machPort, 0) else { - CFMachPortInvalidate(machPort) - return nil - } +// MARK: - Tap Handle +private extension EventTap { + /// The tap's mach port and its run loop source. Owns both for their whole lifetime, + /// so they are always created and invalidated as a unit. + struct Handle { + let machPort: CFMachPort + let runLoopSource: CFRunLoopSource + + /// Fails when the tap cannot be created, which is how a missing Accessibility + /// grant surfaces. `owner` is captured unretained as the callback's `userInfo`. + init?(owner: EventTap, eventsOfInterest: CGEventMask) { + guard let machPort = CGEvent.tapCreate( + tap: .cgSessionEventTap, + place: .headInsertEventTap, + options: .listenOnly, + eventsOfInterest: eventsOfInterest, + callback: tapCallback, + userInfo: Unmanaged.passUnretained(owner).toOpaque() + ) else { + return nil + } - CFRunLoopAddSource(CFRunLoopGetCurrent(), runLoopSource, .commonModes) - return InstalledTap(machPort: machPort, runLoopSource: runLoopSource) - } + guard let runLoopSource = CFMachPortCreateRunLoopSource(nil, machPort, 0) else { + CFMachPortInvalidate(machPort) + return nil + } - func resetInstalledResources() { - for installedTap in self.installedTaps.values { - CFRunLoopSourceInvalidate(installedTap.runLoopSource) + CFRunLoopAddSource(CFRunLoopGetCurrent(), runLoopSource, .commonModes) + self.machPort = machPort + self.runLoopSource = runLoopSource } - self.installedTaps.removeAll() - } - func reenableTap(_ kind: Kind) { - guard let installedTap = self.installedTaps[kind] else { return } - CGEvent.tapEnable(tap: installedTap.machPort, enable: true) - self.state = .installed + func invalidate() { + CFRunLoopSourceInvalidate(self.runLoopSource) + CFMachPortInvalidate(self.machPort) + } + + func enable() { + CGEvent.tapEnable(tap: self.machPort, enable: true) + } } } // MARK: - Event Routing extension EventTap { - /// Single entry point for both taps. `kind` matters only for disable notifications, - /// which report the same event type whichever tap the system turned off. - fileprivate func handleTapCallback(type: CGEventType, event: CGEvent, kind: Kind) { + fileprivate func handleTapCallback(type: CGEventType, event: CGEvent) { switch type { case .tapDisabledByTimeout, .tapDisabledByUserInput: - self.handleTapDisabled(type, kind: kind) + self.handleTapDisabled(type) case .keyDown, .keyUp: self.handleKeyEvent(event) case .systemDefined: @@ -153,7 +172,7 @@ private extension EventTap { self.onOutput?(.mediaKey(MediaKeyEvent(nsEvent: nsEvent))) } - func handleTapDisabled(_ type: CGEventType, kind: Kind) { + func handleTapDisabled(_ type: CGEventType) { let reason: EventTap.DisableReason switch type { case .tapDisabledByTimeout: @@ -164,50 +183,20 @@ private extension EventTap { return } self.state = .temporarilyDisabled(reason) - self.reenableTap(kind) + self.reenableTap() } } -// MARK: - C event tap callbacks -// Must be file-scope functions (no captures) to satisfy the @convention(c) requirement, -// so each tap gets a trampoline that supplies its own `Kind`. - -private func tapCallback(for kind: EventTap.Kind) -> CGEventTapCallBack { - switch kind { - case .key: - return keyTapCallback - case .mouseAndFlags: - return mouseFlagsTapCallback - } -} - -private func keyTapCallback( - proxy: CGEventTapProxy, - type: CGEventType, - event: CGEvent, - refcon: UnsafeMutableRawPointer? -) -> Unmanaged? { - dispatchTapCallback(type: type, event: event, refcon: refcon, kind: .key) -} - -private func mouseFlagsTapCallback( +// MARK: - C event tap callback +private func tapCallback( proxy: CGEventTapProxy, type: CGEventType, event: CGEvent, refcon: UnsafeMutableRawPointer? -) -> Unmanaged? { - dispatchTapCallback(type: type, event: event, refcon: refcon, kind: .mouseAndFlags) -} - -private func dispatchTapCallback( - type: CGEventType, - event: CGEvent, - refcon: UnsafeMutableRawPointer?, - kind: EventTap.Kind ) -> Unmanaged? { if let refcon { let tap = Unmanaged.fromOpaque(refcon).takeUnretainedValue() - tap.handleTapCallback(type: type, event: event, kind: kind) + tap.handleTapCallback(type: type, event: event) } return .passUnretained(event) } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift new file mode 100644 index 00000000..2d862126 --- /dev/null +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift @@ -0,0 +1,21 @@ +// +// EventTapping.swift +// Keyty +// +// SPDX-FileCopyrightText: 2026 Serhii Bykov +// SPDX-License-Identifier: BSD-3-Clause +// + +import Foundation + +/// The capture surface `CaptureController` drives. Installing a real tap requires the +/// Accessibility grant, so the state machine is only testable behind this abstraction. +protocol EventTapping: AnyObject { + /// Receives every captured event and lifecycle change. + var onOutput: ((EventTap.Output) -> Void)? { get set } + + func install() throws(EventTap.Error) + func remove() +} + +extension EventTap: EventTapping {} diff --git a/Apps/Keyty/Sources/Keyty/Support/Extensions/CoreGraphics/CGEventType+Mask.swift b/Apps/Keyty/Sources/Keyty/Support/Extensions/CoreGraphics/CGEventType+Mask.swift new file mode 100644 index 00000000..0b81a370 --- /dev/null +++ b/Apps/Keyty/Sources/Keyty/Support/Extensions/CoreGraphics/CGEventType+Mask.swift @@ -0,0 +1,24 @@ +// +// CGEventType+Mask.swift +// Keyty +// +// SPDX-FileCopyrightText: 2026 Serhii Bykov +// SPDX-License-Identifier: BSD-3-Clause +// + +import Cocoa + +public extension CGEventType { + /// The single-bit event-tap mask matching this event type. + var mask: CGEventMask { + CGEventMask(1) << CGEventMask(self.rawValue) + } +} + +public extension Sequence where Element == CGEventType { + /// The combined event-tap mask matching every event type in the sequence, + /// in the form `CGEvent.tapCreate(eventsOfInterest:)` expects. + var eventMask: CGEventMask { + self.reduce(0) { $0 | $1.mask } + } +} diff --git a/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift b/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift new file mode 100644 index 00000000..c151b1a4 --- /dev/null +++ b/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift @@ -0,0 +1,223 @@ +// +// CaptureControllerTests.swift +// KeytyTests +// +// SPDX-FileCopyrightText: 2026 Serhii Bykov +// SPDX-License-Identifier: BSD-3-Clause +// + +import XCTest +@testable import Keyty + +@MainActor +final class CaptureControllerTests: XCTestCase { + private var store: InMemoryKeyValueStore! + private var keyboardVisualizer: KeyboardVisualizer! + private var pointerVisualizersManager: PointerVisualizersManager! + private var permissionsService: TestPermissionsService! + private var eventTap: TestEventTap! + private var controller: CaptureController! + + override func setUp() { + super.setUp() + self.store = InMemoryKeyValueStore() + let settings = KeyboardVisualizerSettings(store: self.store) + settings.registerDefaults() + self.keyboardVisualizer = KeyboardVisualizer(settings: settings) + self.pointerVisualizersManager = PointerVisualizersManager() + self.permissionsService = TestPermissionsService() + self.eventTap = TestEventTap() + self.controller = CaptureController( + pointerVisualizersManager: self.pointerVisualizersManager, + keyboardVisualizer: self.keyboardVisualizer, + permissionsService: self.permissionsService, + eventTap: self.eventTap + ) + } + + override func tearDown() { + self.controller = nil + self.eventTap = nil + self.permissionsService = nil + self.pointerVisualizersManager = nil + self.keyboardVisualizer = nil + self.store = nil + super.tearDown() + } + + // MARK: - Permission Gating + + func testStartInstallsTapWhenPermissionIsGranted() { + self.permissionsService.currentStatus = .granted + + self.controller.start() + + XCTAssertEqual(self.eventTap.installCount, 1) + XCTAssertTrue(self.controller.isCapturing) + } + + func testStartDoesNotInstallTapWhenPermissionIsNotGranted() { + self.permissionsService.currentStatus = .notGranted + + self.controller.start() + + XCTAssertEqual(self.eventTap.installCount, 0) + XCTAssertFalse(self.controller.isCapturing) + } + + func testStartDoesNotPromptForPermission() { + self.permissionsService.currentStatus = .notGranted + + self.controller.start() + + XCTAssertEqual(self.permissionsService.requestedPermissions, []) + } + + func testCapturingStartsOncePermissionIsGrantedLater() { + self.permissionsService.currentStatus = .notGranted + self.controller.start() + XCTAssertFalse(self.controller.isCapturing) + + self.permissionsService.currentStatus = .granted + self.permissionsService.notifyChange() + + XCTAssertEqual(self.eventTap.installCount, 1) + XCTAssertTrue(self.controller.isCapturing) + } + + func testCapturingStopsWhenTapCannotBeInstalled() { + self.permissionsService.currentStatus = .granted + self.eventTap.installError = .creationFailed + + self.controller.start() + + XCTAssertFalse(self.controller.isCapturing) + } + + func testStopCapturingRemovesTap() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.controller.stopCapturing() + + XCTAssertEqual(self.eventTap.removeCount, 1) + XCTAssertFalse(self.controller.isCapturing) + } + + // MARK: - Tap Recovery + + func testTapIsNotReinstalledBelowTheDisableThreshold() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateTemporaryDisable() + + XCTAssertEqual(self.eventTap.installCount, 1) + XCTAssertTrue(self.controller.isCapturing) + } + + func testTapIsReinstalledAfterRepeatedDisables() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateTemporaryDisable() + + XCTAssertEqual(self.eventTap.installCount, 2) + XCTAssertTrue(self.controller.isCapturing) + } + + func testDisableCountResetsAfterAReinstall() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + for _ in 0..<3 { self.eventTap.simulateTemporaryDisable() } + XCTAssertEqual(self.eventTap.installCount, 2) + + // A fresh run of disables must be needed before the next reinstall. + self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateTemporaryDisable() + XCTAssertEqual(self.eventTap.installCount, 2) + + self.eventTap.simulateTemporaryDisable() + XCTAssertEqual(self.eventTap.installCount, 3) + } + + func testCapturingStopsWhenTapReportsFailure() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.eventTap.simulateFailure() + + XCTAssertFalse(self.controller.isCapturing) + } +} + +// MARK: - Test Doubles + +private final class TestEventTap: EventTapping { + var onOutput: ((EventTap.Output) -> Void)? + var installError: EventTap.Error? + + private(set) var isInstalled = false + private(set) var installCount = 0 + private(set) var removeCount = 0 + + func install() throws(EventTap.Error) { + if let installError = self.installError { throw installError } + guard !self.isInstalled else { return } + self.isInstalled = true + self.installCount += 1 + self.onOutput?(.stateChanged(.installed)) + } + + func remove() { + guard self.isInstalled else { return } + self.isInstalled = false + self.removeCount += 1 + self.onOutput?(.stateChanged(.idle)) + } + + /// Mirrors `EventTap`: the system disables the tap and it re-enables itself immediately. + /// The trailing `.installed` is suppressed when the owner reinstalled in response, + /// matching the real tap's de-duplicated state changes. + func simulateTemporaryDisable(_ reason: EventTap.DisableReason = .timeout) { + let installCountBeforeDisable = self.installCount + self.onOutput?(.stateChanged(.temporarilyDisabled(reason))) + guard self.isInstalled, self.installCount == installCountBeforeDisable else { return } + self.onOutput?(.stateChanged(.installed)) + } + + func simulateFailure() { + self.isInstalled = false + self.onOutput?(.stateChanged(.failed(.creationFailed))) + } +} + +private final class TestPermissionsService: PermissionsService { + var currentStatus: Permission.Status = .granted + private(set) var requestedPermissions: [Permission] = [] + private var observers: [UUID: () -> Void] = [:] + + func status(for permission: Permission) -> Permission.Status { + self.currentStatus + } + + func request(_ permission: Permission) { + self.requestedPermissions.append(permission) + } + + func observeChanges(handler: @escaping () -> Void) -> PermissionObservationToken { + let id = UUID() + self.observers[id] = handler + return PermissionObservationToken { [weak self] in + self?.observers.removeValue(forKey: id) + } + } + + func notifyChange() { + self.observers.values.forEach { $0() } + } +} diff --git a/Apps/Keyty/Tests/KeytyTests/Support/Extensions/CoreGraphics/CGEventTypeMaskTests.swift b/Apps/Keyty/Tests/KeytyTests/Support/Extensions/CoreGraphics/CGEventTypeMaskTests.swift new file mode 100644 index 00000000..d821494d --- /dev/null +++ b/Apps/Keyty/Tests/KeytyTests/Support/Extensions/CoreGraphics/CGEventTypeMaskTests.swift @@ -0,0 +1,46 @@ +// +// CGEventTypeMaskTests.swift +// KeytyTests +// +// SPDX-FileCopyrightText: 2026 Serhii Bykov +// SPDX-License-Identifier: BSD-3-Clause +// + +import XCTest +@testable import Keyty + +final class CGEventTypeMaskTests: XCTestCase { + func testMaskSetsTheBitMatchingTheEventTypeRawValue() { + XCTAssertEqual(CGEventType.keyDown.mask, 1 << CGEventMask(CGEventType.keyDown.rawValue)) + XCTAssertEqual(CGEventType.scrollWheel.mask, 1 << CGEventMask(CGEventType.scrollWheel.rawValue)) + } + + func testMaskSetsExactlyOneBit() { + for type in [CGEventType.keyDown, .keyUp, .flagsChanged, .systemDefined, .scrollWheel] { + XCTAssertEqual(type.mask.nonzeroBitCount, 1) + } + } + + func testEventMaskCombinesEveryEventType() { + let mask = [CGEventType.keyDown, .keyUp].eventMask + + XCTAssertEqual(mask, CGEventType.keyDown.mask | CGEventType.keyUp.mask) + XCTAssertNotEqual(mask & CGEventType.keyDown.mask, 0) + XCTAssertNotEqual(mask & CGEventType.keyUp.mask, 0) + XCTAssertEqual(mask & CGEventType.flagsChanged.mask, 0) + } + + func testEventMaskOfEmptySequenceIsZero() { + XCTAssertEqual([CGEventType]().eventMask, 0) + } + + func testEventMaskIgnoresRepeatedEventTypes() { + XCTAssertEqual([CGEventType.keyDown, .keyDown].eventMask, CGEventType.keyDown.mask) + } + + func testEventMaskCoversSystemDefinedWhoseRawValueIsNotAStandardCase() { + let mask = [CGEventType.systemDefined].eventMask + + XCTAssertEqual(mask, 1 << CGEventMask(NX_SYSDEFINED)) + } +} From 4407c5444588f9dd6fdb5833881dffa079972210 Mon Sep 17 00:00:00 2001 From: Serhii Bykov Date: Fri, 14 Aug 2026 23:27:54 +0200 Subject: [PATCH 2/2] refactor(event-tap): event tap changed ownership --- .../CaptureController+TapState.swift | 16 ---- .../CaptureController/CaptureController.swift | 49 ++++-------- .../EventTap/EventTap+DisableReason.swift | 14 ++++ .../Capture/EventTap/EventTap+Error.swift | 7 +- .../Capture/EventTap/EventTap+Event.swift | 18 +++++ .../Capture/EventTap/EventTap+Output.swift | 23 ------ .../Capture/EventTap/EventTap+State.swift | 4 +- .../Platform/Capture/EventTap/EventTap.swift | 76 ++++++++---------- .../Capture/EventTap/EventTapping.swift | 11 ++- .../Capture/CaptureControllerTests.swift | 77 +++++++++++++------ 10 files changed, 148 insertions(+), 147 deletions(-) delete mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController+TapState.swift create mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Event.swift delete mode 100644 Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Output.swift diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController+TapState.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController+TapState.swift deleted file mode 100644 index a885a952..00000000 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController+TapState.swift +++ /dev/null @@ -1,16 +0,0 @@ -// -// CaptureController+TapState.swift -// Keyty -// -// SPDX-FileCopyrightText: 2026 Serhii Bykov -// SPDX-License-Identifier: BSD-3-Clause -// - -extension CaptureController { - enum TapState { - case idle - case active - case recovering - case failed - } -} diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift index f6c938a9..1c18403f 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/CaptureController/CaptureController.swift @@ -13,7 +13,6 @@ final class CaptureController { var onCapturingChanged: ((Bool) -> Void)? private var shouldCapture: Bool = true private var state: State = .idle - private var tapState: TapState = .idle private var tapDisableCount: Int = 0 private let maxTapDisableCountBeforeReinstall = 3 @@ -34,8 +33,11 @@ final class CaptureController { self.keyboardVisualizer = keyboardVisualizer self.permissionsService = permissionsService self.eventTap = eventTap - self.eventTap.onOutput = { [weak self] output in - self?.handle(output) + self.eventTap.onEvent = { [weak self] event in + self?.handle(event) + } + self.eventTap.onStateChanged = { [weak self] state in + self?.handleTapStateChange(state) } self.eventProcessor.onItemProduced = { [keyboardVisualizer] item in keyboardVisualizer.display(item) @@ -154,42 +156,25 @@ private extension CaptureController { self.stopCapture() self.state = .blockedByTapFailure } - - func updateTapState(from state: EventTap.State) { - switch state { - case .idle: - self.tapState = .idle - case .installed: - self.tapState = .active - case .temporarilyDisabled: - self.tapState = .recovering - case .failed: - self.tapState = .failed - } - } } -// MARK: - Event Tap Output +// MARK: - Captured Events private extension CaptureController { - func handle(_ output: EventTap.Output) { - switch output { - case .stateChanged(let state): - self.handleTapStateChange(state) + func handle(_ event: EventTap.Event) { + guard self.isCapturing else { return } + switch event { case .keystroke(let keystroke): - guard self.isCapturing else { return } self.eventProcessor.processKeystroke(keystroke) case .modifierFlags(let flags): - guard self.isCapturing else { return } self.eventProcessor.processFlagsChanged(flags) case .mediaKey(let mediaKey): - guard self.isCapturing, mediaKey.isRecognized else { return } + guard mediaKey.isRecognized else { return } self.eventProcessor.processMediaKey(mediaKey) case .mouse(let mouseEvent): - guard self.isCapturing else { return } Task { @MainActor [pointerVisualizersManager = self.pointerVisualizersManager] in pointerVisualizersManager.display(mouseEvent) } @@ -198,20 +183,18 @@ private extension CaptureController { } func handleTapStateChange(_ state: EventTap.State) { - self.updateTapState(from: state) - switch state { case .idle: self.tapDisableCount = 0 case .installed: - // `.installed` also follows every automatic re-enable, so it must not reset the - // count; otherwise repeated disables never reach the reinstall threshold. A real - // reinstall clears it by way of `remove()` driving the tap back to `.idle`. break - case .temporarilyDisabled: + case .disabled: self.tapDisableCount += 1 - guard self.tapDisableCount >= self.maxTapDisableCountBeforeReinstall else { return } - self.reinstallEventTap() + if self.tapDisableCount >= self.maxTapDisableCountBeforeReinstall { + self.reinstallEventTap() + } else { + self.eventTap.reenable() + } case .failed: self.handleTapFailure() } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+DisableReason.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+DisableReason.swift index f744e006..06d232d5 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+DisableReason.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+DisableReason.swift @@ -6,6 +6,8 @@ // SPDX-License-Identifier: BSD-3-Clause // +import Cocoa + extension EventTap { enum DisableReason: Equatable { /// The system disabled the tap because the callback stopped responding quickly enough. @@ -13,5 +15,17 @@ extension EventTap { /// The system disabled the tap after user input re-enabled secure or direct input handling. case userInput + + /// `nil` for any event type that is not a tap-disabled notification. + init?(eventType: CGEventType) { + switch eventType { + case .tapDisabledByTimeout: + self = .timeout + case .tapDisabledByUserInput: + self = .userInput + default: + return nil + } + } } } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift index e77da3ea..f6c60320 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Error.swift @@ -10,8 +10,11 @@ import Cocoa extension EventTap { enum Error: LocalizedError, Equatable { - /// The tap could not be created. - case creationFailed + /// The system refused to create the tap's mach port, which is how a missing Accessibility grant surfaces. + case portCreationFailed + + /// The mach port could not be attached to a run loop. + case runLoopSourceCreationFailed var errorDescription: String? { L10n.EventTap.creationFailed diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Event.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Event.swift new file mode 100644 index 00000000..83707b48 --- /dev/null +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Event.swift @@ -0,0 +1,18 @@ +// +// EventTap+Event.swift +// Keyty +// +// SPDX-FileCopyrightText: 2026 Serhii Bykov +// SPDX-License-Identifier: BSD-3-Clause +// + +import AppKit + +extension EventTap { + enum Event { + case keystroke(StandardKeyEvent) + case mouse(MouseEvent) + case mediaKey(MediaKeyEvent) + case modifierFlags(NSEvent.ModifierFlags) + } +} diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Output.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Output.swift deleted file mode 100644 index 11678ec0..00000000 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+Output.swift +++ /dev/null @@ -1,23 +0,0 @@ -// -// EventTap+Output.swift -// Keyty -// -// SPDX-FileCopyrightText: 2026 Serhii Bykov -// SPDX-License-Identifier: BSD-3-Clause -// - -import AppKit - -extension EventTap { - /// Everything an installed tap reports to its owner. - /// - /// Captured input and lifecycle changes share one channel so that a consumer - /// handles them in a single ordered switch rather than across several callbacks. - enum Output { - case keystroke(StandardKeyEvent) - case mouse(MouseEvent) - case mediaKey(MediaKeyEvent) - case modifierFlags(NSEvent.ModifierFlags) - case stateChanged(EventTap.State) - } -} diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift index 2e174b7c..88636182 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap+State.swift @@ -14,8 +14,8 @@ extension EventTap { /// The tap is installed and the system is delivering events. case installed - /// Still installed, but the system turned the tap off; it is re-enabled automatically. - case temporarilyDisabled(EventTap.DisableReason) + /// Installed, but the system turned the tap off. + case disabled(EventTap.DisableReason) /// The tap could not be created, so nothing is installed. case failed(EventTap.Error) diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift index 527aa356..c32093e5 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTap.swift @@ -10,19 +10,19 @@ import Cocoa import IOKit.hidsystem final class EventTap { - /// Receives every captured event and lifecycle change. Called on the run loop - /// the tap was installed on, never on a background queue. - var onOutput: ((Output) -> Void)? + /// Sends every captured event. + var onEvent: ((Event) -> Void)? + + /// Send every lifecycle change + var onStateChanged: ((State) -> Void)? private(set) var state: EventTap.State = .idle { didSet { guard oldValue != self.state else { return } - self.onOutput?(.stateChanged(self.state)) + self.onStateChanged?(self.state) } } - - /// Non-nil exactly while the tap is installed. `state` cannot stand in for this: - /// it reports `.temporarilyDisabled` while the tap is still installed. + private var handle: Handle? deinit { @@ -35,15 +35,13 @@ extension EventTap { func install() throws(EventTap.Error) { guard self.handle == nil else { return } - guard let handle = Handle( - owner: self, - eventsOfInterest: Self.eventsOfInterest - ) else { - self.state = .failed(.creationFailed) - throw .creationFailed + do { + self.handle = try Handle(owner: self, eventsOfInterest: Self.eventsOfInterest) + } catch { + self.state = .failed(error) + throw error } - self.handle = handle self.state = .installed } @@ -53,6 +51,12 @@ extension EventTap { self.handle = nil self.state = .idle } + + func reenable() { + guard case .disabled = self.state, let handle = self.handle else { return } + handle.enable() + self.state = .installed + } } // MARK: - Tap Resources @@ -66,25 +70,19 @@ private extension EventTap { .otherMouseDown, .otherMouseUp, .otherMouseDragged, .scrollWheel ].eventMask - - func reenableTap() { - guard let handle = self.handle else { return } - handle.enable() - self.state = .installed - } } // MARK: - Tap Handle private extension EventTap { - /// The tap's mach port and its run loop source. Owns both for their whole lifetime, - /// so they are always created and invalidated as a unit. + /// Tap's `CFMachPort` and `CFRunLoopSource`. + /// + /// Owns them for their whole lifetime, so they are always created and invalidated as a unit. struct Handle { let machPort: CFMachPort let runLoopSource: CFRunLoopSource - /// Fails when the tap cannot be created, which is how a missing Accessibility - /// grant surfaces. `owner` is captured unretained as the callback's `userInfo`. - init?(owner: EventTap, eventsOfInterest: CGEventMask) { + /// `owner` is captured unretained as the callback's `userInfo`. + init(owner: EventTap, eventsOfInterest: CGEventMask) throws(EventTap.Error) { guard let machPort = CGEvent.tapCreate( tap: .cgSessionEventTap, place: .headInsertEventTap, @@ -93,12 +91,12 @@ private extension EventTap { callback: tapCallback, userInfo: Unmanaged.passUnretained(owner).toOpaque() ) else { - return nil + throw .portCreationFailed } guard let runLoopSource = CFMachPortCreateRunLoopSource(nil, machPort, 0) else { CFMachPortInvalidate(machPort) - return nil + throw .runLoopSourceCreationFailed } CFRunLoopAddSource(CFRunLoopGetCurrent(), runLoopSource, .commonModes) @@ -144,23 +142,20 @@ extension EventTap { private extension EventTap { func handleKeyEvent(_ cgEvent: CGEvent) { guard let nsEvent = NSEvent(cgEvent: cgEvent) else { return } - self.onOutput?(.keystroke(StandardKeyEvent(nsEvent: nsEvent))) + self.onEvent?(.keystroke(StandardKeyEvent(nsEvent: nsEvent))) } func handleFlagsChanged(_ cgEvent: CGEvent) { let flags = NSEvent.ModifierFlags(cgEventFlags: cgEvent.flags) - self.onOutput?(.modifierFlags(flags)) + self.onEvent?(.modifierFlags(flags)) } func handleMouseEvent(_ cgEvent: CGEvent) { guard let nsEvent = NSEvent(cgEvent: cgEvent) else { return } - self.onOutput?(.mouse(MouseEvent(nsEvent: nsEvent, cgEvent: cgEvent))) + self.onEvent?(.mouse(MouseEvent(nsEvent: nsEvent, cgEvent: cgEvent))) } func handleSystemDefined(_ cgEvent: CGEvent) { - // Media keys arrive as system-defined events with the aux-control-buttons subtype. - // Other system-defined subtypes (screen changes, etc.) are ignored, - // and reading `data1` is only safe once the subtype is confirmed. guard let nsEvent = NSEvent(cgEvent: cgEvent), nsEvent.type == .systemDefined, @@ -169,21 +164,12 @@ private extension EventTap { return } - self.onOutput?(.mediaKey(MediaKeyEvent(nsEvent: nsEvent))) + self.onEvent?(.mediaKey(MediaKeyEvent(nsEvent: nsEvent))) } func handleTapDisabled(_ type: CGEventType) { - let reason: EventTap.DisableReason - switch type { - case .tapDisabledByTimeout: - reason = .timeout - case .tapDisabledByUserInput: - reason = .userInput - default: - return - } - self.state = .temporarilyDisabled(reason) - self.reenableTap() + guard let reason = EventTap.DisableReason(eventType: type) else { return } + self.state = .disabled(reason) } } diff --git a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift index 2d862126..9243baa8 100644 --- a/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift +++ b/Apps/Keyty/Sources/Keyty/Platform/Capture/EventTap/EventTapping.swift @@ -8,14 +8,17 @@ import Foundation -/// The capture surface `CaptureController` drives. Installing a real tap requires the -/// Accessibility grant, so the state machine is only testable behind this abstraction. +/// Abstraction to mock `EventTap` protocol EventTapping: AnyObject { - /// Receives every captured event and lifecycle change. - var onOutput: ((EventTap.Output) -> Void)? { get set } + /// Receives every captured event. + var onEvent: ((EventTap.Event) -> Void)? { get set } + + /// Receives every lifecycle change. + var onStateChanged: ((EventTap.State) -> Void)? { get set } func install() throws(EventTap.Error) func remove() + func reenable() } extension EventTap: EventTapping {} diff --git a/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift b/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift index c151b1a4..a9335104 100644 --- a/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift +++ b/Apps/Keyty/Tests/KeytyTests/Platform/Capture/CaptureControllerTests.swift @@ -87,7 +87,7 @@ final class CaptureControllerTests: XCTestCase { func testCapturingStopsWhenTapCannotBeInstalled() { self.permissionsService.currentStatus = .granted - self.eventTap.installError = .creationFailed + self.eventTap.installError = .portCreationFailed self.controller.start() @@ -110,20 +110,43 @@ final class CaptureControllerTests: XCTestCase { self.permissionsService.currentStatus = .granted self.controller.start() - self.eventTap.simulateTemporaryDisable() - self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() XCTAssertEqual(self.eventTap.installCount, 1) XCTAssertTrue(self.controller.isCapturing) } + func testTapIsReenabledBelowTheDisableThreshold() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() + + XCTAssertEqual(self.eventTap.reenableCount, 2) + XCTAssertFalse(self.eventTap.isDisabled) + } + + func testTapIsReinstalledRatherThanReenabledAtTheDisableThreshold() { + self.permissionsService.currentStatus = .granted + self.controller.start() + + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() + + XCTAssertEqual(self.eventTap.reenableCount, 2) + XCTAssertEqual(self.eventTap.installCount, 2) + } + func testTapIsReinstalledAfterRepeatedDisables() { self.permissionsService.currentStatus = .granted self.controller.start() - self.eventTap.simulateTemporaryDisable() - self.eventTap.simulateTemporaryDisable() - self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() XCTAssertEqual(self.eventTap.installCount, 2) XCTAssertTrue(self.controller.isCapturing) @@ -133,15 +156,15 @@ final class CaptureControllerTests: XCTestCase { self.permissionsService.currentStatus = .granted self.controller.start() - for _ in 0..<3 { self.eventTap.simulateTemporaryDisable() } + for _ in 0..<3 { self.eventTap.simulateDisable() } XCTAssertEqual(self.eventTap.installCount, 2) // A fresh run of disables must be needed before the next reinstall. - self.eventTap.simulateTemporaryDisable() - self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateDisable() + self.eventTap.simulateDisable() XCTAssertEqual(self.eventTap.installCount, 2) - self.eventTap.simulateTemporaryDisable() + self.eventTap.simulateDisable() XCTAssertEqual(self.eventTap.installCount, 3) } @@ -158,41 +181,51 @@ final class CaptureControllerTests: XCTestCase { // MARK: - Test Doubles private final class TestEventTap: EventTapping { - var onOutput: ((EventTap.Output) -> Void)? + var onEvent: ((EventTap.Event) -> Void)? + var onStateChanged: ((EventTap.State) -> Void)? var installError: EventTap.Error? private(set) var isInstalled = false + private(set) var isDisabled = false private(set) var installCount = 0 private(set) var removeCount = 0 + private(set) var reenableCount = 0 func install() throws(EventTap.Error) { if let installError = self.installError { throw installError } guard !self.isInstalled else { return } self.isInstalled = true + self.isDisabled = false self.installCount += 1 - self.onOutput?(.stateChanged(.installed)) + self.onStateChanged?(.installed) } func remove() { guard self.isInstalled else { return } self.isInstalled = false + self.isDisabled = false self.removeCount += 1 - self.onOutput?(.stateChanged(.idle)) + self.onStateChanged?(.idle) + } + + func reenable() { + guard self.isInstalled, self.isDisabled else { return } + self.isDisabled = false + self.reenableCount += 1 + self.onStateChanged?(.installed) } - /// Mirrors `EventTap`: the system disables the tap and it re-enables itself immediately. - /// The trailing `.installed` is suppressed when the owner reinstalled in response, - /// matching the real tap's de-duplicated state changes. - func simulateTemporaryDisable(_ reason: EventTap.DisableReason = .timeout) { - let installCountBeforeDisable = self.installCount - self.onOutput?(.stateChanged(.temporarilyDisabled(reason))) - guard self.isInstalled, self.installCount == installCountBeforeDisable else { return } - self.onOutput?(.stateChanged(.installed)) + /// The system turns the tap off; it stays off until the owner acts. + func simulateDisable(_ reason: EventTap.DisableReason = .timeout) { + guard self.isInstalled else { return } + self.isDisabled = true + self.onStateChanged?(.disabled(reason)) } func simulateFailure() { self.isInstalled = false - self.onOutput?(.stateChanged(.failed(.creationFailed))) + self.isDisabled = false + self.onStateChanged?(.failed(.portCreationFailed)) } }