[PATCH 1/2] user32/tests: Test a recursive activation loop on WM_ACTIVATE.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Windows only sends the activation messages and hooks once, until the SetActiveWindow completes for that window. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46274 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- AFAICS these two patches help with the recent user32 tests failures on the testbot. Although I believe the patches are correct, I'm not sure the failure root cause is that windows are now activated recursively. Instead, I think that some change caused window procs to now be always called recursively when they weren't before. dlls/user32/tests/msg.c | 91 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index a2adf56565d..adb167bdad3 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -10203,6 +10203,40 @@ static LRESULT WINAPI ShowWindowProcA(HWND hwnd, UINT message, WPARAM wParam, LP return ret; } +static LRESULT WINAPI SetActiveWindowProcA(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) +{ + static LONG defwndproc_counter = 0; + struct recvd_message msg; + LRESULT ret; + + switch (message) + { + /* log only specific messages we are interested in */ + case WM_NCACTIVATE: + case WM_ACTIVATE: + case WM_SETFOCUS: + case WM_KILLFOCUS: + break; + default: + return DefWindowProcA(hwnd, message, wParam, lParam); + } + + msg.hwnd = hwnd; + msg.message = message; + msg.flags = sent|wparam|lparam; + if (defwndproc_counter) msg.flags |= defwinproc; + msg.wParam = wParam; + msg.lParam = lParam; + msg.descr = "activation"; + add_message(&msg); + + defwndproc_counter++; + ret = DefWindowProcA(hwnd, message, wParam, lParam); + defwndproc_counter--; + + return ret; +} + static LRESULT WINAPI PaintLoopProcA(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) { switch (msg) @@ -10300,6 +10334,10 @@ static BOOL RegisterWindowClasses(void) cls.lpszClassName = "ShowWindowClass"; if(!RegisterClassA(&cls)) return FALSE; + cls.lpfnWndProc = SetActiveWindowProcA; + cls.lpszClassName = "SetActiveWindowClass"; + if(!RegisterClassA(&cls)) return FALSE; + cls.lpfnWndProc = PopupMsgCheckProcA; cls.lpszClassName = "TestPopupClass"; if(!RegisterClassA(&cls)) return FALSE; @@ -10355,6 +10393,7 @@ static BOOL is_our_logged_class(HWND hwnd) { if (!lstrcmpiA(buf, "TestWindowClass") || !lstrcmpiA(buf, "ShowWindowClass") || + !lstrcmpiA(buf, "SetActiveWindowClass") || !lstrcmpiA(buf, "TestParentClass") || !lstrcmpiA(buf, "TestPopupClass") || !lstrcmpiA(buf, "SimpleWindowClass") || @@ -14758,10 +14797,46 @@ static const struct message SetActiveWindowSeq4[] = { 0 } }; +static LRESULT WINAPI test_recursive_set_active_window_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) +{ + if (message == WM_ACTIVATE && LOWORD(wparam) != WA_INACTIVE) + { + SetActiveWindow((HWND)lparam); + SetActiveWindow(hwnd); + return 0; + } + + return DefWindowProcA(hwnd, message, wparam, lparam); +} + +static void test_set_active_window_recursive(HWND hwnd, HWND hwnd2) +{ + const struct message RecursiveActivationSeq[] = + { + { HCBT_ACTIVATE, hook|wparam, (WPARAM)hwnd }, + { WM_NCACTIVATE, sent|wparam|lparam, FALSE, (LPARAM)hwnd }, + { WM_ACTIVATE, sent|wparam|lparam, WA_INACTIVE, (LPARAM)hwnd }, + { EVENT_SYSTEM_FOREGROUND, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { HCBT_ACTIVATE, hook|wparam, (WPARAM)hwnd2 }, + { EVENT_SYSTEM_FOREGROUND, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { WM_NCACTIVATE, sent|wparam|lparam, FALSE, (LPARAM)hwnd }, + { WM_ACTIVATE, sent|wparam|lparam, WA_ACTIVE, (LPARAM)hwnd }, + { 0 } + }; + SetActiveWindow(hwnd2); + + flush_sequence(); + SetActiveWindow(hwnd); + ok_sequence(RecursiveActivationSeq, "recursive activation", TRUE); + + DestroyWindow(hwnd2); + DestroyWindow(hwnd); + flush_sequence(); +} static void test_SetActiveWindow(void) { - HWND hwnd, popup, ret; + HWND hwnd, hwnd2, popup, ret; hwnd = CreateWindowExA(0, "TestWindowClass", "Test SetActiveWindow", WS_OVERLAPPEDWINDOW | WS_VISIBLE, @@ -14812,6 +14887,20 @@ static void test_SetActiveWindow(void) trace("done\n"); DestroyWindow(hwnd); + + hwnd = CreateWindowExA(0, "SimpleWindowClass", NULL, WS_OVERLAPPED|WS_VISIBLE, + 100, 100, 200, 200, 0, 0, 0, NULL); + ok(hwnd != 0, "Failed to create simple window\n"); + SetWindowLongPtrA(hwnd, GWLP_WNDPROC, (LONG_PTR)test_recursive_set_active_window_proc); + + hwnd2 = CreateWindowExA(0, "SetActiveWindowClass", NULL, WS_OVERLAPPED|WS_VISIBLE, + 10, 10, 50, 50, hwnd, 0, 0, NULL); + ok(hwnd2 != 0, "Failed to create recursive activation window\n"); + + test_set_active_window_recursive(hwnd, hwnd2); + + DestroyWindow(hwnd2); + DestroyWindow(hwnd); } static const struct message SetForegroundWindowSeq[] = -- 2.35.1
When activating a window and sending activation messages to the window procedure, Windows avoids a recursive loop by not sending more of these messages or hooks while it's still activating the window. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46274 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/msg.c | 2 +- dlls/win32u/input.c | 17 ++++++++++++----- dlls/win32u/ntuser_private.h | 1 + 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index adb167bdad3..5c7464173c0 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -14827,7 +14827,7 @@ static void test_set_active_window_recursive(HWND hwnd, HWND hwnd2) flush_sequence(); SetActiveWindow(hwnd); - ok_sequence(RecursiveActivationSeq, "recursive activation", TRUE); + ok_sequence(RecursiveActivationSeq, "recursive activation", FALSE); DestroyWindow(hwnd2); DestroyWindow(hwnd); diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 5cd2cdd0970..29c7e86036c 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -1227,6 +1227,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) return TRUE; } + if (prev) *prev = previous; + if (win_set_flags( hwnd, WIN_IS_ACTIVATING, 0 ) & WIN_IS_ACTIVATING) return TRUE; + /* call CBT hook chain */ cbt.fMouse = mouse; cbt.hWndActive = previous; @@ -1246,9 +1249,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) previous = wine_server_ptr_handle( reply->previous ); } SERVER_END_REQ; - if (!ret) return FALSE; + if (!ret) goto done; if (prev) *prev = previous; - if (previous == hwnd) return TRUE; + if (previous == hwnd) goto done; if (hwnd) { @@ -1256,7 +1259,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) if (send_message( hwnd, WM_QUERYNEWPALETTE, 0, 0 ) && user_callbacks) user_callbacks->pSendMessageTimeoutW( HWND_BROADCAST, WM_PALETTEISCHANGING, (WPARAM)hwnd, 0, SMTO_ABORTIFHUNG, 2000, NULL ); - if (!is_window(hwnd)) return FALSE; + if (!(ret = is_window( hwnd ))) goto done; } old_thread = previous ? get_window_thread( previous, NULL ) : 0; @@ -1290,7 +1293,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) if (is_window(hwnd)) { - send_message( hwnd, WM_NCACTIVATE, hwnd == NtUserGetForegroundWindow(), (LPARAM)previous ); + send_message( hwnd, WM_NCACTIVATE, + (hwnd == NtUserGetForegroundWindow()) && !(win_get_flags(previous) & WIN_IS_ACTIVATING), + (LPARAM)previous ); send_message( hwnd, WM_ACTIVATE, MAKEWPARAM( mouse ? WA_CLICKACTIVE : WA_ACTIVE, is_iconic(hwnd) ), (LPARAM)previous ); @@ -1313,7 +1318,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) } } - return TRUE; +done: + win_set_flags( hwnd, 0, WIN_IS_ACTIVATING ); + return ret; } /********************************************************************** diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index 7c03474286d..5452c722ffa 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -119,6 +119,7 @@ typedef struct tagWND #define WIN_NEEDS_SHOW_OWNEDPOPUP 0x0020 /* WM_SHOWWINDOW:SC_SHOW must be sent in the next ShowOwnedPopup call */ #define WIN_CHILDREN_MOVED 0x0040 /* children may have moved, ignore stored positions */ #define WIN_HAS_IME_WIN 0x0080 /* the window has been registered with imm32 */ +#define WIN_IS_ACTIVATING 0x0100 /* the window is being activated */ #define WND_OTHER_PROCESS ((WND *)1) /* returned by WIN_GetPtr on unknown window handles */ #define WND_DESKTOP ((WND *)2) /* returned by WIN_GetPtr on the desktop window */ -- 2.35.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=111213 Your paranoid android. === w8 (32 bit report) === user32: msg.c:15351: Test failed: unexpected message 31f msg.c:15352: Test failed: bad wparam 1 msg.c:15358: Test failed: unicode WM_CHAR: 0: the msg sequence is not complete: expected 0102 - actual 0000 === debian11 (32 bit French report) === user32: msg.c:17136: Test failed: 0: WaitForSingleObject failed msg.c:17136: Test failed: 1: WaitForSingleObject failed msg.c:17136: Test failed: 2: WaitForSingleObject failed msg.c:17136: Test failed: 3: WaitForSingleObject failed msg.c:17136: Test failed: 4: WaitForSingleObject failed msg.c:17136: Test failed: 5: WaitForSingleObject failed msg.c:17136: Test failed: 6: WaitForSingleObject failed msg.c:17136: Test failed: 7: WaitForSingleObject failed msg: Timeout === debian11 (32 bit Japanese:Japan report) === user32: msg.c:17136: Test failed: 0: WaitForSingleObject failed msg.c:17136: Test failed: 1: WaitForSingleObject failed msg.c:17136: Test failed: 2: WaitForSingleObject failed msg.c:17136: Test failed: 3: WaitForSingleObject failed msg.c:17136: Test failed: 4: WaitForSingleObject failed msg.c:17136: Test failed: 5: WaitForSingleObject failed msg.c:17136: Test failed: 6: WaitForSingleObject failed msg.c:17136: Test failed: 7: WaitForSingleObject failed msg: Timeout
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Thanks for taking the time to upstream these, I kind of forgot, but they've been in wine-staging for ages now, and I personally used them ever since they got placed there with no issues for years now. They never got any review though, back in the day.
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=111212 Your paranoid android. === debian11 (32 bit report) === user32: msg.c:17136: Test failed: 0: WaitForSingleObject failed msg.c:17136: Test failed: 1: WaitForSingleObject failed msg.c:17136: Test failed: 2: WaitForSingleObject failed msg.c:17136: Test failed: 3: WaitForSingleObject failed msg.c:17136: Test failed: 4: WaitForSingleObject failed msg.c:17136: Test failed: 5: WaitForSingleObject failed msg.c:17136: Test failed: 6: WaitForSingleObject failed msg.c:17136: Test failed: 7: WaitForSingleObject failed msg: Timeout === debian11 (32 bit Arabic:Morocco report) === user32: msg.c:17136: Test failed: 0: WaitForSingleObject failed msg.c:17136: Test failed: 1: WaitForSingleObject failed msg.c:17136: Test failed: 2: WaitForSingleObject failed msg.c:17136: Test failed: 3: WaitForSingleObject failed msg.c:17136: Test failed: 4: WaitForSingleObject failed msg.c:17136: Test failed: 5: WaitForSingleObject failed msg.c:17136: Test failed: 6: WaitForSingleObject failed msg.c:17136: Test failed: 7: WaitForSingleObject failed msg: Timeout === debian11 (32 bit Chinese:China report) === user32: msg.c:17136: Test failed: 0: WaitForSingleObject failed msg.c:17136: Test failed: 1: WaitForSingleObject failed msg.c:17136: Test failed: 2: WaitForSingleObject failed msg.c:17136: Test failed: 3: WaitForSingleObject failed msg.c:17136: Test failed: 4: WaitForSingleObject failed msg.c:17136: Test failed: 5: WaitForSingleObject failed msg.c:17136: Test failed: 6: WaitForSingleObject failed msg.c:17136: Test failed: 7: WaitForSingleObject failed msg: Timeout
participants (3)
-
Gabriel Ivăncescu -
Marvin -
Rémi Bernon