From 9ca4c3c7fe5659271baad99cb0a71af05b10f17e Mon Sep 17 00:00:00 2001 From: dijunkun Date: Fri, 6 Nov 2026 15:00:59 +0800 Subject: [PATCH] [fix] preserve Windows modifier state across keyboard capture recovery, refs #142 --- .../input/keyboard/keyboard_capturer.cpp | 13 +- .../tests/windows_keyboard_capturer_test.cpp | 237 ++++++++++++++++++ apps/desktop/xmake/targets.lua | 12 + 3 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 apps/desktop/tests/windows_keyboard_capturer_test.cpp diff --git a/apps/desktop/src/platform/windows/input/keyboard/keyboard_capturer.cpp b/apps/desktop/src/platform/windows/input/keyboard/keyboard_capturer.cpp index fa660acd..da0913af 100644 --- a/apps/desktop/src/platform/windows/input/keyboard/keyboard_capturer.cpp +++ b/apps/desktop/src/platform/windows/input/keyboard/keyboard_capturer.cpp @@ -112,10 +112,13 @@ int PlatformKeyboardCapturer::Unhook() { } void PlatformKeyboardCapturer::SnapshotLocalKeys() { + // Keep local release ownership even if focus changed during installation + // or recovery. Focus may return before the UI stops capture; forgetting a + // held local modifier here would swallow its release and leave Windows stuck. local_keys_down_.fill(false); for (int code = VK_BACK; code < 0xFF; ++code) local_keys_down_[code] = (GetAsyncKeyState(code) & 0x8000) != 0; - ForwardLocalModifiers(); + if (GetForegroundWindow() == capture_window_) ForwardLocalModifiers(); } void PlatformKeyboardCapturer::ForwardKey(int code, bool down, @@ -187,10 +190,7 @@ bool PlatformKeyboardCapturer::RenewKeyboardHook() { // Events may have escaped while the hook was unavailable or a backend // re-enabled raw input. Cancel uncertain presses and adopt local state. ReleaseCapturedKeys(); - if (GetForegroundWindow() == capture_window_) - SnapshotLocalKeys(); - else - local_keys_down_.fill(false); + SnapshotLocalKeys(); LOG_WARN("Reset captured keys after keyboard capture recovery"); } last_hook_pump_tick_.store(GetTickCount64()); @@ -227,8 +227,7 @@ void PlatformKeyboardCapturer::CaptureThreadMain() { GetLastError()); } hook_active_.store(capture_started); - if (capture_started && GetForegroundWindow() == capture_window_) - SnapshotLocalKeys(); + if (capture_started) SnapshotLocalKeys(); { std::lock_guard lock(capture_state_mutex_); capture_thread_id_ = thread_id; diff --git a/apps/desktop/tests/windows_keyboard_capturer_test.cpp b/apps/desktop/tests/windows_keyboard_capturer_test.cpp new file mode 100644 index 00000000..8a40e150 --- /dev/null +++ b/apps/desktop/tests/windows_keyboard_capturer_test.cpp @@ -0,0 +1,237 @@ +// Run the real capture thread and callback against a scripted Windows API. +// These tests never install a system hook or inject desktop input. +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "keyboard_state.h" + +namespace mock { + +const HWND stream = reinterpret_cast(1); +const HWND local = reinterpret_cast(2); +HWND foreground = stream; +HOOKPROC callback = nullptr; +std::array windows_down{}; +bool raw_registered = false; +bool old_hook_expired = false; +bool leave_during_install = false; +bool ok = true; +std::string scenario; +std::vector forwarded; +std::vector> steps; +size_t next_step = 0; +std::promise finished; + +void Expect(bool condition, const char* message) { + if (!condition) { + std::cerr << "FAIL " << scenario << ": " << message << '\n'; + ok = false; + } +} + +HWND Foreground() { return foreground; } +DWORD WindowProcess(HWND, LPDWORD process) { + *process = GetCurrentProcessId(); + return GetCurrentThreadId(); +} +SHORT AsyncKey(int code) { return windows_down[code] ? SHORT(0x8000) : 0; } +HHOOK Install(int, HOOKPROC proc, HINSTANCE, DWORD) { + callback = proc; + if (leave_during_install) { + foreground = local; + leave_during_install = false; + } + return reinterpret_cast(3); +} +BOOL Remove(HHOOK) { + if (old_hook_expired) { + old_hook_expired = false; + SetLastError(ERROR_INVALID_HOOK_HANDLE); + return FALSE; + } + return TRUE; +} +LRESULT NextHook(HHOOK, int, WPARAM, LPARAM) { return 0; } +UINT Registrations(PRAWINPUTDEVICE devices, PUINT count, UINT) { + if (!devices) { + *count = raw_registered ? 1 : 0; + return 0; + } + if (!raw_registered) return 0; + devices[0] = {1, 6, 0, nullptr}; + return 1; +} +BOOL Register(PCRAWINPUTDEVICE devices, UINT, UINT) { + raw_registered = (devices[0].dwFlags & RIDEV_REMOVE) == 0; + return TRUE; +} +BOOL Peek(LPMSG, HWND, UINT, UINT, UINT) { return FALSE; } +UINT_PTR Timer(HWND, UINT_PTR, UINT, TIMERPROC) { return 1; } +BOOL KillTimer(HWND, UINT_PTR) { return TRUE; } +BOOL Post(DWORD, UINT, WPARAM, LPARAM) { return TRUE; } +BOOL GetMessage(LPMSG message, HWND, UINT, UINT) { + if (next_step == steps.size()) { + finished.set_value(); + return 0; + } + *message = {}; + steps[next_step++](*message); + return TRUE; +} +void Forward(int key, bool down, uint32_t scan, bool extended, void*) { + forwarded.push_back({key, down, scan, extended}); +} + +void Event(int key, bool down, HWND window, bool should_swallow, + bool generic_modifier = false) { + foreground = window; + const UINT scan = MapVirtualKeyW(key, MAPVK_VK_TO_VSC_EX); + DWORD vk = key; + if (generic_modifier) { + if (key == VK_LCONTROL || key == VK_RCONTROL) vk = VK_CONTROL; + if (key == VK_LSHIFT || key == VK_RSHIFT) vk = VK_SHIFT; + if (key == VK_LMENU || key == VK_RMENU) vk = VK_MENU; + } + const KBDLLHOOKSTRUCT event{vk, scan & 0xFF, + (scan & 0xFF00) ? LLKHF_EXTENDED : 0u, 0, 0}; + const bool swallowed = callback(HC_ACTION, down ? WM_KEYDOWN : WM_KEYUP, + reinterpret_cast(&event)) != 0; + // The hook decides whether Windows receives this edge. + if (!swallowed) windows_down[key] = down; + Expect(swallowed == should_swallow, + down ? "unexpected key-down routing" : "unexpected key-up routing"); +} + +void Reset(int key, const std::string& name) { + foreground = stream; + callback = nullptr; + windows_down.fill(false); + raw_registered = old_hook_expired = leave_during_install = false; + scenario = name + " key=" + std::to_string(key); + forwarded.clear(); + steps.clear(); + next_step = 0; + finished = std::promise(); +} + +} // namespace mock + +// Windows declarations are already loaded. Redirect only the capture code's +// OS boundary, including the inline raw-input guard, to the scripted desktop. +#define GetForegroundWindow mock::Foreground +#define GetWindowThreadProcessId mock::WindowProcess +#define GetAsyncKeyState mock::AsyncKey +#define SetWindowsHookExW mock::Install +#define UnhookWindowsHookEx mock::Remove +#define CallNextHookEx mock::NextHook +#define GetRegisteredRawInputDevices mock::Registrations +#define RegisterRawInputDevices mock::Register +#define PeekMessageW mock::Peek +#define SetTimer mock::Timer +#define KillTimer mock::KillTimer +#define PostThreadMessageW mock::Post +#define GetMessageW mock::GetMessage + +// Logging is not part of the routing contract and needs no global logger here. +#define _RD_LOG_H_ +#define LOG_INFO(...) ((void)0) +#define LOG_WARN(...) ((void)0) +#include "../src/platform/windows/input/keyboard/keyboard_capturer.cpp" + +namespace crossdesk { +UINT SendInputOnUserDesktop(const INPUT&) { std::abort(); } +UINT SendInputOnUserDesktop(INPUT*, UINT) { std::abort(); } +} // namespace crossdesk + +namespace { + +void Run(int key) { + crossdesk::PlatformKeyboardCapturer capturer; + auto finished = mock::finished.get_future(); + const int result = capturer.Hook(&mock::Forward, nullptr); + mock::Expect(result == 0, "capture must start"); + if (result != 0) return; + finished.wait(); + capturer.Unhook(); + mock::Expect(!mock::windows_down[key], "Windows modifier must be released"); + mock::Expect(!mock::forwarded.empty() && !mock::forwarded.back().is_down, + "remote modifier must be released"); +} + +} // namespace + +int main() { + int scenarios = 0; + for (const int key : {VK_LCONTROL, VK_RCONTROL, VK_LSHIFT, VK_RSHIFT, + VK_LMENU, VK_RMENU, VK_LWIN, VK_RWIN}) { + mock::Reset(key, "captured press and release"); + mock::steps.push_back([=](MSG&) { + mock::Event(key, true, mock::stream, true); + mock::Event(key, false, mock::stream, true); + }); + Run(key); + ++scenarios; + + for (bool leave : {false, true}) { + mock::Reset(key, leave ? "focus changes during startup" : "inherited press"); + mock::windows_down[key] = true; + mock::leave_during_install = leave; + mock::steps.push_back([=](MSG&) { + if (leave) mock::Expect(mock::forwarded.empty(), + "startup must not forward background modifiers"); + mock::Event(key, false, mock::stream, false, true); + }); + Run(key); + ++scenarios; + } + + for (bool background : {false, true}) { + // 0: normal renewal, 1: raw input resumed, 2: hook silently expired. + for (int recovery = 0; recovery < 3; ++recovery) { + mock::Reset(key, std::string(background ? "background" : "foreground") + + " renewal=" + std::to_string(recovery)); + if (!background) mock::windows_down[key] = true; + mock::steps.push_back([=](MSG& message) { + if (background) mock::Event(key, true, mock::local, false); + mock::raw_registered = recovery == 1; + mock::old_hook_expired = recovery == 2; + message.message = WM_TIMER; + message.wParam = 1; + }); + mock::steps.push_back([=](MSG&) { + if (background) mock::Expect(mock::forwarded.empty(), + "recovery must not forward background modifiers"); + mock::Event(key, false, mock::stream, false, true); + }); + Run(key); + ++scenarios; + } + } + + mock::Reset(key, "press missed while hook was unavailable"); + mock::steps.push_back([=](MSG& message) { + mock::foreground = mock::local; + mock::windows_down[key] = true; + mock::old_hook_expired = true; + message.message = WM_TIMER; + message.wParam = 1; + }); + mock::steps.push_back([=](MSG&) { + mock::Expect(mock::forwarded.empty(), + "recovered background press must not be forwarded yet"); + mock::Event(key, false, mock::stream, false); + }); + Run(key); + ++scenarios; + } + std::cout << scenarios << " keyboard capture scenarios: " + << (mock::ok ? "PASS" : "FAIL") << '\n'; + return mock::ok ? 0 : 1; +} diff --git a/apps/desktop/xmake/targets.lua b/apps/desktop/xmake/targets.lua index c27d9283..9aeeab4d 100644 --- a/apps/desktop/xmake/targets.lua +++ b/apps/desktop/xmake/targets.lua @@ -198,6 +198,18 @@ function setup_targets() set_default(false) add_files("apps/desktop/tests/windows_mouse_controller_safety_test.cpp") + if is_os("windows") then + target("windows_keyboard_capturer_test") + set_kind("binary") + set_default(false) + add_includedirs("apps/desktop/src/platform/windows/input", + "apps/desktop/src/platform/common/input", + "apps/desktop/src/device_controller", "apps/desktop/src/common", + "apps/desktop/src/log", "libs/wire/include") + add_files("apps/desktop/tests/windows_keyboard_capturer_test.cpp") + add_syslinks("user32") + end + target("windows_sas_guard_test") set_kind("binary") set_default(false)