[PATCH 1/6] user32/focus: Prevent a recursive activation loop with the activation messages
When activating a window and sending activation messages to the window procedure (i.e. WM_NCACTIVATE and WM_ACTIVATE), we have to avoid a recursive loop by not sending more of these messages or hooks while we are still activating the window. Some applications actually depend on this behavior. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46274 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- It may seem a bit strange (and I thought so as well initially), but this is exactly what's needed to match the testcase in the next patch, so this is how Windows does it (yes, including the hooks). dlls/user32/focus.c | 44 +++++++++++++++++++++++++++++--------------- dlls/user32/win.h | 1 + 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/dlls/user32/focus.c b/dlls/user32/focus.c index f1c8831..0d32d00 100644 --- a/dlls/user32/focus.c +++ b/dlls/user32/focus.c @@ -78,7 +78,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) { HWND previous = GetActiveWindow(); BOOL ret; - DWORD old_thread, new_thread; + DWORD winflags, old_thread, new_thread; CBTACTIVATESTRUCT cbt; if (previous == hwnd) @@ -87,16 +87,24 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) return TRUE; } - /* call CBT hook chain */ - cbt.fMouse = mouse; - cbt.hWndActive = previous; - if (HOOK_CallHooks( WH_CBT, HCBT_ACTIVATE, (WPARAM)hwnd, (LPARAM)&cbt, TRUE )) return FALSE; - - if (IsWindow(previous)) + /* Prevent a recursive activation loop with the activation messages */ + winflags = win_set_flags(hwnd, WIN_IS_IN_ACTIVATION, 0); + if (!(winflags & WIN_IS_IN_ACTIVATION)) { - SendMessageW( previous, WM_NCACTIVATE, FALSE, (LPARAM)hwnd ); - SendMessageW( previous, WM_ACTIVATE, - MAKEWPARAM( WA_INACTIVE, IsIconic(previous) ), (LPARAM)hwnd ); + ret = FALSE; + + /* call CBT hook chain */ + cbt.fMouse = mouse; + cbt.hWndActive = previous; + if (HOOK_CallHooks( WH_CBT, HCBT_ACTIVATE, (WPARAM)hwnd, (LPARAM)&cbt, TRUE )) + goto clear_flags; + + if (IsWindow(previous)) + { + SendMessageW( previous, WM_NCACTIVATE, FALSE, (LPARAM)hwnd ); + SendMessageW( previous, WM_ACTIVATE, + MAKEWPARAM( WA_INACTIVE, IsIconic(previous) ), (LPARAM)hwnd ); + } } SERVER_START_REQ( set_active_window ) @@ -106,9 +114,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 clear_flags; if (prev) *prev = previous; - if (previous == hwnd) return TRUE; + if (previous == hwnd) goto clear_flags; if (hwnd) { @@ -116,7 +124,11 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) if (SendMessageW( hwnd, WM_QUERYNEWPALETTE, 0, 0 )) SendMessageTimeoutW( HWND_BROADCAST, WM_PALETTEISCHANGING, (WPARAM)hwnd, 0, SMTO_ABORTIFHUNG, 2000, NULL ); - if (!IsWindow(hwnd)) return FALSE; + if (!IsWindow(hwnd)) + { + ret = FALSE; + goto clear_flags; + } } old_thread = previous ? GetWindowThreadProcessId( previous, NULL ) : 0; @@ -148,7 +160,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) } } - if (IsWindow(hwnd)) + if (!(winflags & WIN_IS_IN_ACTIVATION) && IsWindow(hwnd)) { SendMessageW( hwnd, WM_NCACTIVATE, (hwnd == GetForegroundWindow()), (LPARAM)previous ); SendMessageW( hwnd, WM_ACTIVATE, @@ -173,7 +185,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) } } - return TRUE; +clear_flags: + win_set_flags(hwnd, 0, WIN_IS_IN_ACTIVATION); + return ret; } diff --git a/dlls/user32/win.h b/dlls/user32/win.h index f3bdfd3..fa5d1b6 100644 --- a/dlls/user32/win.h +++ b/dlls/user32/win.h @@ -80,6 +80,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_IN_ACTIVATION 0x0100 /* the window is in an activation process */ /* Window functions */ extern HWND get_hwnd_message_parent(void) DECLSPEC_HIDDEN; -- 2.19.1
Windows only sends the activation messages and hooks once, until the SetActiveWindow completes for that window. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Note that without the prior patch that fixes this, Wine would enter a long loop here. dlls/user32/tests/msg.c | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 366a138..3d9c0b7 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -4772,6 +4772,39 @@ static void test_showwindow(void) flush_sequence(); } +static void test_recursive_activation(void) +{ + static const struct message seq[] = + { + { HCBT_ACTIVATE, hook }, + { WM_NCACTIVATE, sent|wparam, TRUE }, + { WM_ACTIVATE, sent|wparam, WA_ACTIVE }, + { HCBT_ACTIVATE, hook }, + { WM_NCACTIVATE, sent|wparam, FALSE }, + { WM_ACTIVATE, sent|wparam, WA_INACTIVE }, + { WM_SETFOCUS, sent|optional }, + { 0 } + }; + HWND hwnd, recursive; + + 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"); + + recursive = CreateWindowExA(0, "RecursiveActivationClass", NULL, WS_OVERLAPPED|WS_VISIBLE, + 10, 10, 50, 50, hwnd, 0, 0, NULL); + ok(recursive != 0, "Failed to create recursive activation window\n"); + SetActiveWindow(hwnd); + + flush_sequence(); + SetActiveWindow(recursive); + ok_sequence(seq, "Recursive Activation", FALSE); + + DestroyWindow(recursive); + DestroyWindow(hwnd); + flush_sequence(); +} + static void test_sys_menu(void) { HWND hwnd; @@ -9699,6 +9732,48 @@ static LRESULT WINAPI ShowWindowProcA(HWND hwnd, UINT message, WPARAM wParam, LP return ret; } +static LRESULT WINAPI recursive_activation_wndprocA(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 = "recursive_activation"; + add_message(&msg); + + /* recursively activate ourselves by first losing activation and changing it back */ + if (message == WM_ACTIVATE && LOWORD(wParam) != WA_INACTIVE) + { + SetActiveWindow((HWND)lParam); + SetActiveWindow(hwnd); + return 0; + } + + 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) @@ -9796,6 +9871,10 @@ static BOOL RegisterWindowClasses(void) cls.lpszClassName = "ShowWindowClass"; if(!RegisterClassA(&cls)) return FALSE; + cls.lpfnWndProc = recursive_activation_wndprocA; + cls.lpszClassName = "RecursiveActivationClass"; + if(!RegisterClassA(&cls)) return FALSE; + cls.lpfnWndProc = PopupMsgCheckProcA; cls.lpszClassName = "TestPopupClass"; if(!RegisterClassA(&cls)) return FALSE; @@ -9851,6 +9930,7 @@ static BOOL is_our_logged_class(HWND hwnd) { if (!lstrcmpiA(buf, "TestWindowClass") || !lstrcmpiA(buf, "ShowWindowClass") || + !lstrcmpiA(buf, "RecursiveActivationClass") || !lstrcmpiA(buf, "TestParentClass") || !lstrcmpiA(buf, "TestPopupClass") || !lstrcmpiA(buf, "SimpleWindowClass") || @@ -17617,6 +17697,7 @@ START_TEST(msg) test_messages(); test_setwindowpos(); test_showwindow(); + test_recursive_activation(); invisible_parent_tests(); test_mdi_messages(); test_button_messages(); -- 2.19.1
On Windows, if WM_NCACTIVATE returns FALSE, the window should not be able to be deactivated. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/focus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/focus.c b/dlls/user32/focus.c index 0d32d00..834ceb1 100644 --- a/dlls/user32/focus.c +++ b/dlls/user32/focus.c @@ -101,7 +101,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) if (IsWindow(previous)) { - SendMessageW( previous, WM_NCACTIVATE, FALSE, (LPARAM)hwnd ); + if (!SendMessageW( previous, WM_NCACTIVATE, FALSE, (LPARAM)hwnd )) goto clear_flags; SendMessageW( previous, WM_ACTIVATE, MAKEWPARAM( WA_INACTIVE, IsIconic(previous) ), (LPARAM)hwnd ); } -- 2.19.1
Hi, While running your changed tests on Windows, 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=46796 Your paranoid android. === debian9 (32 bit Chinese:China report) === user32: clipboard.c:833: Test failed: 6: gle 5 clipboard.c:838: Test failed: 6.0: got 0000 instead of 0008 clipboard.c:868: Test failed: 6: gle 1418 === debian9 (64 bit WoW report) === user32: msg.c:8746: Test failed: WaitForSingleObject failed 102 msg.c:8752: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8752: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8752: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
On Windows, if WM_NCACTIVATE returns FALSE, the window doesn't get deactivated. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/tests/win.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 94eff5e..83ba1de 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -899,6 +899,17 @@ static LRESULT WINAPI tool_window_procA(HWND hwnd, UINT msg, WPARAM wparam, LPAR return DefWindowProcA(hwnd, msg, wparam, lparam); } +static LRESULT WINAPI no_deactivation_procA(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + switch (msg) + { + case WM_NCACTIVATE: + DefWindowProcA(hwnd, msg, wparam, lparam); + return FALSE; + } + return DefWindowProcA(hwnd, msg, wparam, lparam); +} + static const WCHAR mainclassW[] = {'M','a','i','n','W','i','n','d','o','w','C','l','a','s','s','W',0}; static BOOL RegisterWindowClasses(void) @@ -3160,6 +3171,18 @@ static void test_SetActiveWindow(HWND hwnd) check_active_state(hwnd, hwnd, hwnd); } DestroyWindow(hwnd2); + + /* a window can disallow being deactivated by returning FALSE from WM_NCACTIVATE */ + hwnd2 = CreateWindowExA(0, "MainWindowClass", "No Deactivation window", WS_OVERLAPPED | WS_VISIBLE, + 10, 10, 50, 50, hwnd, 0, GetModuleHandleA(NULL), NULL); + ok(hwnd2 != NULL, "failed to create No Deactivation window: 0x%08x\n", GetLastError()); + SetWindowLongPtrA(hwnd2, GWLP_WNDPROC, (LONG_PTR)no_deactivation_procA); + + SetActiveWindow(hwnd2); + check_wnd_state(hwnd2, hwnd2, hwnd2, 0); + SetActiveWindow(hwnd); + check_wnd_state(hwnd2, hwnd2, hwnd2, 0); + DestroyWindow(hwnd2); } struct create_window_thread_params -- 2.19.1
Hi, While running your changed tests on Windows, 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=46797 Your paranoid android. === debian9 (32 bit WoW report) === user32: menu.c:2354: Test failed: test 30 menu: Timeout
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- See tests in next patch; changing the activation from within the WA_INACTIVE message is kept by Windows. dlls/user32/focus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/user32/focus.c b/dlls/user32/focus.c index 834ceb1..1fac45d 100644 --- a/dlls/user32/focus.c +++ b/dlls/user32/focus.c @@ -104,6 +104,9 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) if (!SendMessageW( previous, WM_NCACTIVATE, FALSE, (LPARAM)hwnd )) goto clear_flags; SendMessageW( previous, WM_ACTIVATE, MAKEWPARAM( WA_INACTIVE, IsIconic(previous) ), (LPARAM)hwnd ); + + /* Give up if the activation was changed */ + if (previous != GetActiveWindow()) goto clear_flags; } } -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/tests/win.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 83ba1de..2b73661 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -910,6 +910,17 @@ static LRESULT WINAPI no_deactivation_procA(HWND hwnd, UINT msg, WPARAM wparam, return DefWindowProcA(hwnd, msg, wparam, lparam); } +static LRESULT WINAPI change_activation_from_wa_inactive_procA(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + switch (msg) + { + case WM_ACTIVATE: + if (LOWORD(wparam) == WA_INACTIVE) + SetActiveWindow((HWND)GetWindowLongPtrA(hwnd, GWLP_USERDATA)); + } + return DefWindowProcA(hwnd, msg, wparam, lparam); +} + static const WCHAR mainclassW[] = {'M','a','i','n','W','i','n','d','o','w','C','l','a','s','s','W',0}; static BOOL RegisterWindowClasses(void) @@ -3092,7 +3103,7 @@ todo_wine static void test_SetActiveWindow(HWND hwnd) { - HWND hwnd2, ret; + HWND hwnd2, hwnd3, ret; flush_events( TRUE ); ShowWindow(hwnd, SW_HIDE); @@ -3183,6 +3194,24 @@ static void test_SetActiveWindow(HWND hwnd) SetActiveWindow(hwnd); check_wnd_state(hwnd2, hwnd2, hwnd2, 0); DestroyWindow(hwnd2); + + /* changing the activated window from within WM_ACTIVATE's WA_INACTIVE should work */ + hwnd2 = CreateWindowExA(0, "MainWindowClass", "No Deactivation window", WS_OVERLAPPED | WS_VISIBLE, + 10, 10, 50, 50, hwnd, 0, GetModuleHandleA(NULL), NULL); + ok(hwnd2 != NULL, "failed to create window: 0x%08x\n", GetLastError()); + hwnd3 = CreateWindowExA(0, "MainWindowClass", "No Deactivation window", WS_OVERLAPPED | WS_VISIBLE, + 70, 70, 50, 50, hwnd, 0, GetModuleHandleA(NULL), NULL); + ok(hwnd3 != NULL, "failed to create window: 0x%08x\n", GetLastError()); + + SetActiveWindow(hwnd2); + check_wnd_state(hwnd2, hwnd2, hwnd2, 0); + SetWindowLongPtrA(hwnd2, GWLP_USERDATA, (LONG_PTR)hwnd3); + SetWindowLongPtrA(hwnd2, GWLP_WNDPROC, (LONG_PTR)change_activation_from_wa_inactive_procA); + SetActiveWindow(hwnd); + check_wnd_state(hwnd3, hwnd3, hwnd3, 0); + + DestroyWindow(hwnd2); + DestroyWindow(hwnd3); } struct create_window_thread_params -- 2.19.1
participants (2)
-
Gabriel Ivăncescu -
Marvin