[PATCH 0/2] MR4711: Fix Toad for Oracle F3 find next function not working properly
Fix Toad for Oracle F3 find next function not working properly because its WH_KEYBOARD hook gets called too many times. There is another case that could cause WH_CBT HCBT_CLICKSKIPPED hooks to get called recursively as the tests show. I will send patches for that after the code freeze. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4711
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/user32/tests/msg.c | 167 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 166 insertions(+), 1 deletion(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 0059afcbac7..9f7da2fe5a5 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -12430,6 +12430,29 @@ todo_wine { static HWND hook_hwnd; static HHOOK recursive_hook; static int hook_depth, max_hook_depth; +static BOOL skip_WH_KEYBOARD_hook, skip_WH_MOUSE_hook; + +static void simulate_click(BOOL left, int x, int y) +{ + POINT old_pt; + INPUT input[2]; + UINT events_no; + + GetCursorPos(&old_pt); + SetCursorPos(x, y); + memset(input, 0, sizeof(input)); + input[0].type = INPUT_MOUSE; + input[0].mi.dx = x; + input[0].mi.dy = y; + input[0].mi.dwFlags = left ? MOUSEEVENTF_LEFTDOWN : MOUSEEVENTF_RIGHTDOWN; + input[1].type = INPUT_MOUSE; + input[1].mi.dx = x; + input[1].mi.dy = y; + input[1].mi.dwFlags = left ? MOUSEEVENTF_LEFTUP : MOUSEEVENTF_RIGHTUP; + events_no = SendInput(2, input, sizeof(input[0])); + ok(events_no == 2, "SendInput returned %d\n", events_no); + SetCursorPos(old_pt.x, old_pt.y); +} static LRESULT WINAPI rec_get_message_hook(int code, WPARAM w, LPARAM l) { @@ -12450,12 +12473,85 @@ static LRESULT WINAPI rec_get_message_hook(int code, WPARAM w, LPARAM l) return res; } +static LRESULT CALLBACK keyboard_recursive_hook_proc(int code, WPARAM wp, LPARAM lp) +{ + MSG msg; + + if (code < 0) + return CallNextHookEx(0, code, wp, lp); + + if (skip_WH_KEYBOARD_hook) + return 1; + + hook_depth++; + max_hook_depth = max(max_hook_depth, hook_depth); + PeekMessageW(&msg, NULL, WM_KEYFIRST, WM_KEYLAST, PM_REMOVE); + hook_depth--; + return CallNextHookEx(0, code, wp, lp); +} + +static LRESULT CALLBACK mouse_recursive_hook_proc(int code, WPARAM wp, LPARAM lp) +{ + MSG msg; + + if (code < 0) + return CallNextHookEx(0, code, wp, lp); + + if (skip_WH_MOUSE_hook) + return 1; + + hook_depth++; + max_hook_depth = max(max_hook_depth, hook_depth); + PeekMessageW(&msg, NULL, WM_MOUSEFIRST, WM_MOUSELAST, PM_REMOVE); + hook_depth--; + return CallNextHookEx(0, code, wp, lp); +} + +static LRESULT CALLBACK keyboard_recursive_cbt_hook_proc(int code, WPARAM wp, LPARAM lp) +{ + MSG msg; + + if (code < 0) + return CallNextHookEx(0, code, wp, lp); + + if (code == HCBT_KEYSKIPPED) + { + hook_depth++; + max_hook_depth = max(max_hook_depth, hook_depth); + PeekMessageW(&msg, NULL, WM_KEYFIRST, WM_KEYLAST, PM_REMOVE); + hook_depth--; + } + + return CallNextHookEx(0, code, wp, lp); +} + +static LRESULT CALLBACK mouse_recursive_cbt_hook_proc(int code, WPARAM wp, LPARAM lp) +{ + MSG msg; + + if (code < 0) + return CallNextHookEx(0, code, wp, lp); + + if (code == HCBT_CLICKSKIPPED) + { + hook_depth++; + max_hook_depth = max(max_hook_depth, hook_depth); + PeekMessageW(&msg, NULL, WM_MOUSEFIRST, WM_MOUSELAST, PM_REMOVE); + hook_depth--; + } + + return CallNextHookEx(0, code, wp, lp); +} + static void test_recursive_hook(void) { + HHOOK hook, cbt_hook; + INPUT input = {0}; MSG msg; BOOL b; - hook_hwnd = CreateWindowA("Static", NULL, WS_POPUP, 0, 0, 200, 60, NULL, NULL, NULL, NULL); + hook_hwnd = CreateWindowExA(WS_EX_TOPMOST, "Static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 200, 60, + NULL, NULL, NULL, NULL); ok(hook_hwnd != NULL, "CreateWindow failed\n"); recursive_hook = SetWindowsHookExW(WH_GETMESSAGE, rec_get_message_hook, NULL, GetCurrentThreadId()); @@ -12472,6 +12568,75 @@ static void test_recursive_hook(void) b = UnhookWindowsHookEx(recursive_hook); ok(b, "UnhokWindowsHookEx failed\n"); + /* Test possible recursive hook conditions */ + b = SetForegroundWindow(hook_hwnd); + ok(b, "SetForegroundWindow failed, error %ld.\n", GetLastError()); + + /* Test a possible recursive WH_KEYBOARD hook condition */ + max_hook_depth = 0; + hook = SetWindowsHookExA(WH_KEYBOARD, keyboard_recursive_hook_proc, NULL, GetCurrentThreadId()); + ok(!!hook, "SetWindowsHookExA failed, error %ld.\n", GetLastError()); + + flush_events(); + input.type = INPUT_KEYBOARD; + input.ki.wVk = VK_F3; + SendInput(1, &input, sizeof(INPUT)); + flush_events(); + + /* Expect the WH_KEYBOARD hook not gets called recursively */ + todo_wine + ok(max_hook_depth == 1, "Got expected %d.\n", max_hook_depth); + + /* Test a possible recursive WH_CBT HCBT_KEYSKIPPED hook condition */ + max_hook_depth = 0; + skip_WH_KEYBOARD_hook = 1; + cbt_hook = SetWindowsHookExA(WH_CBT, keyboard_recursive_cbt_hook_proc, NULL, GetCurrentThreadId()); + ok(!!cbt_hook, "SetWindowsHookExA failed, error %ld.\n", GetLastError()); + + flush_events(); + input.type = INPUT_KEYBOARD; + input.ki.wVk = VK_F3; + SendInput(1, &input, sizeof(INPUT)); + while (PeekMessageA(&msg, hook_hwnd, WM_KEYFIRST, WM_KEYLAST, 0)) DispatchMessageA(&msg); + + /* Expect the WH_CBT HCBT_KEYSKIPPED hook not gets called recursively */ + todo_wine + ok(max_hook_depth == 1, "Got expected %d.\n", max_hook_depth); + + UnhookWindowsHookEx(cbt_hook); + UnhookWindowsHookEx(hook); + + /* Test a recursive WH_MOUSE hook condition */ + SetCapture(hook_hwnd); + + max_hook_depth = 0; + hook = SetWindowsHookExA(WH_MOUSE, mouse_recursive_hook_proc, NULL, GetCurrentThreadId()); + ok(!!hook, "SetWindowsHookExA failed, error %ld.\n", GetLastError()); + + flush_events(); + simulate_click(FALSE, 50, 50); + flush_events(); + + /* Expect the WH_MOUSE hook gets called recursively */ + ok(max_hook_depth > 10, "Got expected %d.\n", max_hook_depth); + + /* Test a possible recursive WH_CBT HCBT_CLICKSKIPPED hook condition */ + max_hook_depth = 0; + skip_WH_MOUSE_hook = 1; + cbt_hook = SetWindowsHookExA(WH_CBT, mouse_recursive_cbt_hook_proc, NULL, GetCurrentThreadId()); + ok(!!cbt_hook, "SetWindowsHookExA failed, error %ld.\n", GetLastError()); + + flush_events(); + simulate_click(FALSE, 50, 50); + flush_events(); + + /* Expect the WH_CBT HCBT_CLICKSKIPPED hook not gets called recursively */ + todo_wine + ok(max_hook_depth <= 10, "Got expected %d.\n", max_hook_depth); + + UnhookWindowsHookEx(cbt_hook); + UnhookWindowsHookEx(hook); + ReleaseCapture(); DestroyWindow(hook_hwnd); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4711
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Previously, accept_hardware_message() is called after WH_KEYBOARD and WH_CBT HCBT_KEYSKIPPED hooks. So when PeekMessage() gets called in WH_KEYBOARD and WH_CBT HCBT_KEYSKIPPED hooks, the hooks will be called recursively because the message is still in the server message queue. Fix Toad for Oracle F3 find next function not working properly because its WH_KEYBOARD hook gets called too many times. --- dlls/user32/tests/msg.c | 2 -- dlls/win32u/message.c | 6 ++++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 9f7da2fe5a5..98b1dcc6f99 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -12584,7 +12584,6 @@ static void test_recursive_hook(void) flush_events(); /* Expect the WH_KEYBOARD hook not gets called recursively */ - todo_wine ok(max_hook_depth == 1, "Got expected %d.\n", max_hook_depth); /* Test a possible recursive WH_CBT HCBT_KEYSKIPPED hook condition */ @@ -12600,7 +12599,6 @@ static void test_recursive_hook(void) while (PeekMessageA(&msg, hook_hwnd, WM_KEYFIRST, WM_KEYLAST, 0)) DispatchMessageA(&msg); /* Expect the WH_CBT HCBT_KEYSKIPPED hook not gets called recursively */ - todo_wine ok(max_hook_depth == 1, "Got expected %d.\n", max_hook_depth); UnhookWindowsHookEx(cbt_hook); diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 91f836a60bd..376af30554e 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2384,15 +2384,17 @@ static BOOL process_keyboard_message( MSG *msg, UINT hw_id, HWND hwnd_filter, } } + /* if remove is TRUE, remove message first before calling hooks to avoid recursive hook calls */ + if (remove) accept_hardware_message( hw_id ); if (call_hooks( WH_KEYBOARD, remove ? HC_ACTION : HC_NOREMOVE, LOWORD(msg->wParam), msg->lParam, 0 )) { + /* if the message has not been removed, remove it */ + if (!remove) accept_hardware_message( hw_id ); /* skip this message */ call_hooks( WH_CBT, HCBT_KEYSKIPPED, LOWORD(msg->wParam), msg->lParam, 0 ); - accept_hardware_message( hw_id ); return FALSE; } - if (remove) accept_hardware_message( hw_id ); msg->pt = point_phys_to_win_dpi( msg->hwnd, msg->pt ); if (remove && msg->message == WM_KEYDOWN) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4711
participants (2)
-
Zhiyi Zhang -
Zhiyi Zhang (＠zhiyi)