[PATCH 1/2] user32/tests: Add a test exposing a SetActiveWindow inversion.
When calling SetForegroundWindow for a window in another thread, an internal message is posted to the thread's message queue. If this thread then calls SetForegroundWindow before processing its messages it will execute the corresponding set_active_window first, but then overwrite the active window later, when processing its internal messages. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/win.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 0f683f858a1..38fcada570c 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -3246,7 +3246,9 @@ static void test_SetActiveWindow(HWND hwnd) struct create_window_thread_params { HWND window; + HWND other_window; HANDLE window_created; + HANDLE set_foreground; HANDLE test_finished; }; @@ -3255,12 +3257,23 @@ static DWORD WINAPI create_window_thread(void *param) struct create_window_thread_params *p = param; DWORD res; BOOL ret; + MSG msg; + p->other_window = CreateWindowA("static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0); p->window = CreateWindowA("static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0); ret = SetEvent(p->window_created); ok(ret, "SetEvent failed, last error %#x.\n", GetLastError()); + res = WaitForSingleObject(p->set_foreground, INFINITE); + ok(res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError()); + + SetForegroundWindow(p->other_window); + check_active_state(p->other_window, p->other_window, p->other_window); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + todo_wine + check_active_state(p->other_window, p->other_window, p->other_window); + res = WaitForSingleObject(p->test_finished, INFINITE); ok(res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError()); @@ -3355,6 +3368,8 @@ static void test_SetForegroundWindow(HWND hwnd) thread_params.window_created = CreateEventW(NULL, FALSE, FALSE, NULL); ok(!!thread_params.window_created, "CreateEvent failed, last error %#x.\n", GetLastError()); + thread_params.set_foreground = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(!!thread_params.set_foreground, "CreateEvent failed, last error %#x.\n", GetLastError()); thread_params.test_finished = CreateEventW(NULL, FALSE, FALSE, NULL); ok(!!thread_params.test_finished, "CreateEvent failed, last error %#x.\n", GetLastError()); thread = CreateThread(NULL, 0, create_window_thread, &thread_params, 0, &tid); @@ -3388,9 +3403,19 @@ static void test_SetForegroundWindow(HWND hwnd) ok(!SetForegroundWindow(hwnd2), "SetForegroundWindow failed\n"); check_wnd_state(hwnd, hwnd, hwnd, 0); + SetForegroundWindow(hwnd); + check_active_state(hwnd, hwnd, hwnd); + SetForegroundWindow(thread_params.window); + check_active_state(0, thread_params.window, 0); + SetForegroundWindow(hwnd); + check_active_state(hwnd, hwnd, hwnd); + res = SetEvent(thread_params.set_foreground); + ok(res, "SetEvent failed, last error %#x.\n", GetLastError()); + SetEvent(thread_params.test_finished); WaitForSingleObject(thread, INFINITE); CloseHandle(thread_params.test_finished); + CloseHandle(thread_params.set_foreground); CloseHandle(thread_params.window_created); CloseHandle(thread); DestroyWindow(hwnd2); -- 2.25.0
This makes set_active_window calls process the pending internal WM_WINE_SETACTIVEWINDOW messages first. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/focus.c | 21 +++++++++++++-------- dlls/user32/message.c | 2 +- dlls/user32/tests/win.c | 1 - dlls/user32/user_private.h | 1 + 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/dlls/user32/focus.c b/dlls/user32/focus.c index f1c883167ed..f2f972ef1a9 100644 --- a/dlls/user32/focus.c +++ b/dlls/user32/focus.c @@ -74,13 +74,18 @@ static HWND set_focus_window( HWND hwnd ) /******************************************************************* * set_active_window */ -static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) +BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus, BOOL sent ) { - HWND previous = GetActiveWindow(); + MSG msg; + HWND previous; BOOL ret; DWORD old_thread, new_thread; CBTACTIVATESTRUCT cbt; + /* process pending WM_WINE_SETACTIVEWINDOW messages first */ + if (!sent) PeekMessageW( &msg, 0, 0, 0, QS_SENDMESSAGE ); + + previous = GetActiveWindow(); if (previous == hwnd) { if (prev) *prev = hwnd; @@ -200,14 +205,14 @@ static BOOL set_foreground_window( HWND hwnd, BOOL mouse ) if (ret && previous != hwnd) { if (send_msg_old) /* old window belongs to other thread */ - SendNotifyMessageW( previous, WM_WINE_SETACTIVEWINDOW, 0, 0 ); + SendNotifyMessageW( previous, WM_WINE_SETACTIVEWINDOW, 0, mouse ); else if (send_msg_new) /* old window belongs to us but new one to other thread */ - ret = set_active_window( 0, NULL, mouse, TRUE ); + ret = set_active_window( 0, NULL, mouse, TRUE, FALSE ); if (send_msg_new) /* new window belongs to other thread */ - SendNotifyMessageW( hwnd, WM_WINE_SETACTIVEWINDOW, (WPARAM)hwnd, 0 ); + SendNotifyMessageW( hwnd, WM_WINE_SETACTIVEWINDOW, (WPARAM)hwnd, mouse ); else /* new window belongs to us */ - ret = set_active_window( hwnd, NULL, mouse, TRUE ); + ret = set_active_window( hwnd, NULL, mouse, TRUE, FALSE ); } return ret; } @@ -249,7 +254,7 @@ HWND WINAPI SetActiveWindow( HWND hwnd ) return GetActiveWindow(); /* Windows doesn't seem to return an error here */ } - if (!set_active_window( hwnd, &prev, FALSE, TRUE )) return 0; + if (!set_active_window( hwnd, &prev, FALSE, TRUE, FALSE )) return 0; return prev; } @@ -296,7 +301,7 @@ HWND WINAPI SetFocus( HWND hwnd ) /* activate hwndTop if needed. */ if (hwndTop != GetActiveWindow()) { - if (!set_active_window( hwndTop, NULL, FALSE, FALSE )) return 0; + if (!set_active_window( hwndTop, NULL, FALSE, FALSE, FALSE )) return 0; if (!IsWindow( hwnd )) return 0; /* Abort if window destroyed */ /* Do not change focus if the window is no longer active */ diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 1336865112a..84de851085b 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -1874,7 +1874,7 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR return WIN_SetStyle(hwnd, wparam, lparam); case WM_WINE_SETACTIVEWINDOW: if (!wparam && GetForegroundWindow() == hwnd) return 0; - return (LRESULT)SetActiveWindow( (HWND)wparam ); + return (LRESULT)set_active_window( (HWND)wparam, NULL, lparam, TRUE, TRUE ); case WM_WINE_KEYBOARD_LL_HOOK: case WM_WINE_MOUSE_LL_HOOK: { diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 38fcada570c..de31de84617 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -3271,7 +3271,6 @@ static DWORD WINAPI create_window_thread(void *param) SetForegroundWindow(p->other_window); check_active_state(p->other_window, p->other_window, p->other_window); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - todo_wine check_active_state(p->other_window, p->other_window, p->other_window); res = WaitForSingleObject(p->test_finished, INFINITE); diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index 7e294558ef1..115503eb38d 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -231,6 +231,7 @@ struct tagWND; extern void CLIPBOARD_ReleaseOwner( HWND hwnd ) DECLSPEC_HIDDEN; extern BOOL FOCUS_MouseActivate( HWND hwnd ) DECLSPEC_HIDDEN; +extern BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus, BOOL sent ) DECLSPEC_HIDDEN; extern BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret ) DECLSPEC_HIDDEN; extern void free_dce( struct dce *dce, HWND hwnd ) DECLSPEC_HIDDEN; extern void invalidate_dce( struct tagWND *win, const RECT *rect ) DECLSPEC_HIDDEN; -- 2.25.0
On 1/22/20 6:38 PM, Rémi Bernon wrote:
diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 1336865112a..84de851085b 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -1874,7 +1874,7 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR return WIN_SetStyle(hwnd, wparam, lparam); case WM_WINE_SETACTIVEWINDOW: if (!wparam && GetForegroundWindow() == hwnd) return 0; - return (LRESULT)SetActiveWindow( (HWND)wparam ); + return (LRESULT)set_active_window( (HWND)wparam, NULL, lparam, TRUE, TRUE ); case WM_WINE_KEYBOARD_LL_HOOK: case WM_WINE_MOUSE_LL_HOOK: {
I replaced SetActiveWindow with the internal function here, I'm not entirely sure if the WIN_GetFullHandle call / child window logic that is in SetActiveWindow was actually needed here or not, if someone knows better. -- Rémi Bernon <rbernon(a)codeweavers.com>
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=63578 Your paranoid android. === w1064v1809 (32 bit report) === user32: win.c:3167: Test failed: GetActiveWindow() = 0004007C win.c:3170: Test failed: GetFocus() = 00000000 win.c:3182: Test failed: GetFocus() = 00000000 win.c:3185: Test failed: GetFocus() = 00000000 win.c:3188: Test failed: GetFocus() = 00000000 win.c:3191: Test failed: GetActiveWindow() = 0004007C win.c:3195: Test failed: GetFocus() = 00000000 win.c:3198: Test failed: GetFocus() = 00000000 win.c:3916: Test failed: hwnd 00020178/00BA029C message 0737 win.c:3921: Test failed: hwnd 00BA029C/00BA029C message 0202 win.c:3926: Test failed: hwnd 00BA029C/00BA029C message 0203 win.c:3930: Test failed: message 0202 available === w1064v1809_2scr (32 bit report) === user32: win.c:3167: Test failed: GetActiveWindow() = 000502EA win.c:3170: Test failed: GetFocus() = 00000000 win.c:3182: Test failed: GetFocus() = 00000000 win.c:3185: Test failed: GetFocus() = 00000000 win.c:3188: Test failed: GetFocus() = 00000000 win.c:3191: Test failed: GetActiveWindow() = 000502EA win.c:3195: Test failed: GetFocus() = 00000000 win.c:3198: Test failed: GetFocus() = 00000000 win.c:3916: Test failed: hwnd 0004029E/001D032C message 0737 win.c:3921: Test failed: hwnd 001D032C/001D032C message 0202 win.c:3926: Test failed: hwnd 001D032C/001D032C message 0203 win.c:3930: Test failed: message 0202 available win.c:9974: Test failed: pos = 00e700d0 win.c:9978: Test failed: pos = 00e700d0 win.c:9982: Test failed: pos = 00e700d0 === w1064v1809_ja (32 bit report) === user32: win.c:3876: Test failed: hwnd 000202DA message 7fff win.c:3955: Test failed: hwnd 000202DA/00060360 message 7fff win.c:3958: Test failed: hwnd 000202DA/00060360 message 7fff === w1064v1809_zh_CN (32 bit report) === user32: win.c:3876: Test failed: hwnd 0003030E message 0282 win.c:3955: Test failed: hwnd 0003030E/000D02CC message 0282 win.c:3958: Test failed: hwnd 0003030E/000D02CC message 0282 === w1064v1809 (64 bit report) === user32: win.c:3167: Test failed: GetActiveWindow() = 0000000000040294 win.c:3170: Test failed: GetFocus() = 0000000000000000 win.c:3182: Test failed: GetFocus() = 0000000000000000 win.c:3185: Test failed: GetFocus() = 0000000000000000 win.c:3188: Test failed: GetFocus() = 0000000000000000 win.c:3191: Test failed: GetActiveWindow() = 0000000000040294 win.c:3195: Test failed: GetFocus() = 0000000000000000 win.c:3198: Test failed: GetFocus() = 0000000000000000 win.c:3916: Test failed: hwnd 0000000000030056/00000000000B02E4 message 0737 win.c:3921: Test failed: hwnd 00000000000B02E4/00000000000B02E4 message 0202 win.c:3926: Test failed: hwnd 00000000000B02E4/00000000000B02E4 message 0203 win.c:3930: Test failed: message 0202 available === debian10 (32 bit report) === user32: msg.c:5276: Test failed: SetWindowPos:show_popup_first_show_window: 14: the msg sequence is not complete: expected 0000 - actual 0046
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=63577 Your paranoid android. === w1064v1809 (32 bit report) === user32: win.c:3167: Test failed: GetActiveWindow() = 0004004A win.c:3170: Test failed: GetFocus() = 00000000 win.c:3182: Test failed: GetFocus() = 00000000 win.c:3185: Test failed: GetFocus() = 00000000 win.c:3188: Test failed: GetFocus() = 00000000 win.c:3191: Test failed: GetActiveWindow() = 0004004A win.c:3195: Test failed: GetFocus() = 00000000 win.c:3198: Test failed: GetFocus() = 00000000 win.c:3917: Test failed: hwnd 00020102/000C01CC message 0737 win.c:3922: Test failed: hwnd 000C01CC/000C01CC message 0202 win.c:3927: Test failed: hwnd 000C01CC/000C01CC message 0203 win.c:3931: Test failed: message 0202 available === w1064v1809_ar (32 bit report) === user32: win.c:3752: Test failed: message 0738 available === w1064v1809_he (32 bit report) === user32: win.c:3752: Test failed: message 0738 available === w1064v1809_ja (32 bit report) === user32: win.c:3877: Test failed: hwnd 000202F4 message 7fff win.c:3956: Test failed: hwnd 000202F4/000A0324 message 7fff win.c:3959: Test failed: hwnd 000202F4/000A0324 message 7fff === w1064v1809_zh_CN (32 bit report) === user32: win.c:3877: Test failed: hwnd 0001038A message 0282 win.c:3956: Test failed: hwnd 0001038A/002C03EC message 0282 win.c:3959: Test failed: hwnd 0001038A/002C03EC message 0282 === w1064v1809 (64 bit report) === user32: win.c:3167: Test failed: GetActiveWindow() = 00000000000200E4 win.c:3170: Test failed: GetFocus() = 0000000000000000 win.c:3182: Test failed: GetFocus() = 0000000000000000 win.c:3185: Test failed: GetFocus() = 0000000000000000 win.c:3188: Test failed: GetFocus() = 0000000000000000 win.c:3191: Test failed: GetActiveWindow() = 00000000000200E4 win.c:3195: Test failed: GetFocus() = 0000000000000000 win.c:3198: Test failed: GetFocus() = 0000000000000000 win.c:3917: Test failed: hwnd 0000000000020102/000000000011004C message 0737 win.c:3922: Test failed: hwnd 000000000011004C/000000000011004C message 0202 win.c:3927: Test failed: hwnd 000000000011004C/000000000011004C message 0203 win.c:3931: Test failed: message 0202 available === debian10 (32 bit report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000 === debian10 (32 bit French report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000 === debian10 (32 bit Japanese:Japan report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000 === debian10 (32 bit Chinese:China report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000 === debian10 (32 bit WoW report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000 === debian10 (64 bit WoW report) === user32: win.c:9791: Test failed: GetActiveWindow() = 00000000 win.c:9791: Test failed: GetFocus() = 00000000
Looks like the new test case now leaks some internal messages and breaks a later test, the next patch fixes it but this one is broken. I'll add a PeekMessage somewhere near the end of the test case. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (2)
-
Marvin -
Rémi Bernon