[PATCH v2 0/3] MR9511: Draft: server: Forbid background process window reactivation.
Pegasus Mail posts a WM_USER + 8991 message to its own window when it loses focus, then calls SetFocus on its window when processing this message. Several other applications have been seen calling SetWindowPos during WM_ACTIVATE message, which might also attempt to reactivate the window. That processing happens shortly after we have changed the foreground window to the desktop window, when focus is lost, then SetFocus tries to change the foreground window again. This SetFocus behavior is tested, and should work like this, but would only activate the window if the process is allowed to do so. Windows has various rules around this, and it seems to boil down to something like: * Allow taking focus if the process never was foreground, ie: when process is starting and gets initial focus on its windows. * Allow taking focus if the process was foreground but lost it recently because of a window being destroyed. * Forbid taking focus back if the process had foreground and called SetForegroundWindow explicitly to give it to another process. This doesn't implement all this rules, but rather keep rely on the host window management with some additional heuristics to avoid activating windows which lost foreground recently. This is mostly about keeping track of user input time, updating it on user input and on focus change. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 --- I think this should be enough to prevent spurious window reactivation, either from calls to SetWindowPos during WM_ACTIVATE messages like in https://gitlab.winehq.org/wine/wine/-/merge_requests/9398 or from calls to SetFocus from posted messages right after window deactivation as described above. Fwiw we could have tests for that, and show that this MR is fixing them but fvwm doesn't implement window minimization properly, and doesn't change focus when window is minimized, so the tests would be meaningless. -- v2: server: Forbid background process window reactivation. server: Set NULL foreground input when switching to desktop window. win32u: Introduce a NtUserSetForegroundWindowInternal call. https://gitlab.winehq.org/wine/wine/-/merge_requests/9511
From: Rémi Bernon <rbernon(a)codeweavers.com> For user drivers to forcefully change foreground window, regardless of the restriction which will be introduced next. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 --- dlls/win32u/defwnd.c | 2 +- dlls/win32u/input.c | 7 ++++--- dlls/win32u/message.c | 4 ++-- dlls/win32u/win32u_private.h | 2 +- dlls/win32u/window.c | 7 +++++-- dlls/winemac.drv/window.c | 8 ++++---- dlls/winewayland.drv/window.c | 2 +- dlls/winex11.drv/event.c | 6 +++--- include/ntuser.h | 6 ++++++ server/protocol.def | 1 + 10 files changed, 28 insertions(+), 17 deletions(-) diff --git a/dlls/win32u/defwnd.c b/dlls/win32u/defwnd.c index 870ef376e85..03662963db3 100644 --- a/dlls/win32u/defwnd.c +++ b/dlls/win32u/defwnd.c @@ -2206,7 +2206,7 @@ static LRESULT handle_nc_lbutton_down( HWND hwnd, WPARAM wparam, LPARAM lparam ) top = parent; } - if (set_foreground_window( top, TRUE ) || (get_active_window() == top)) + if (set_foreground_window( top, TRUE, FALSE ) || (get_active_window() == top)) send_message( hwnd, WM_SYSCOMMAND, SC_MOVE + HTCAPTION, lparam ); break; } diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index f431ab98a9f..57d054a9df6 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2168,7 +2168,7 @@ HWND WINAPI NtUserSetFocus( HWND hwnd ) if (call_hooks( WH_CBT, HCBT_SETFOCUS, (WPARAM)hwnd, (LPARAM)previous, 0 )) return 0; /* activate hwndTop if needed. */ - if (!(active = get_active_window()) && !set_foreground_window( hwndTop, FALSE )) return 0; + if (!(active = get_active_window()) && !set_foreground_window( hwndTop, FALSE, FALSE )) return 0; if (hwndTop != active) { if (!set_active_window( hwndTop, NULL, FALSE, FALSE, 0 )) return 0; @@ -2193,13 +2193,13 @@ HWND WINAPI NtUserSetFocus( HWND hwnd ) */ BOOL WINAPI NtUserSetForegroundWindow( HWND hwnd ) { - return set_foreground_window( hwnd, FALSE ); + return set_foreground_window( hwnd, FALSE, FALSE ); } /******************************************************************* * set_foreground_window */ -BOOL set_foreground_window( HWND hwnd, BOOL mouse ) +BOOL set_foreground_window( HWND hwnd, BOOL mouse, BOOL internal ) { BOOL ret, send_msg_old = FALSE, send_msg_new = FALSE; DWORD new_thread_id; @@ -2211,6 +2211,7 @@ BOOL set_foreground_window( HWND hwnd, BOOL mouse ) SERVER_START_REQ( set_foreground_window ) { req->handle = wine_server_user_handle( hwnd ); + req->internal = internal; if ((ret = !wine_server_call_err( req ))) { previous = wine_server_ptr_handle( reply->previous ); diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 7da66fd1494..b28ce272099 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2231,7 +2231,7 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR if (!user_driver->pGetWindowStateUpdates( hwnd, &state_cmd, &swp_flags, &window_rect, &foreground )) return 0; window_rect = map_rect_raw_to_virt( window_rect, get_thread_dpi() ); - if (foreground) NtUserSetForegroundWindow( foreground ); + if (foreground) set_foreground_window( foreground, FALSE, TRUE ); switch (state_cmd) { case SC_RESTORE: @@ -2744,7 +2744,7 @@ static BOOL process_mouse_message( MSG *msg, UINT hw_id, ULONG_PTR extra_info, H /* fall through */ case MA_ACTIVATE: case 0: - if (!set_foreground_window( hwndTop, TRUE )) eat_msg = TRUE; + if (!set_foreground_window( hwndTop, TRUE, FALSE )) eat_msg = TRUE; break; default: WARN( "unknown WM_MOUSEACTIVATE code %d\n", ret ); diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index ee76854c7e4..3b65bb25b53 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -95,7 +95,7 @@ extern DWORD get_input_state(void); extern DWORD get_last_input_time(void); extern BOOL get_async_keyboard_state( BYTE state[256] ); extern BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret ); -extern BOOL set_foreground_window( HWND hwnd, BOOL mouse ); +extern BOOL set_foreground_window( HWND hwnd, BOOL mouse, BOOL force ); extern BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus, DWORD new_active_thread_id ); extern BOOL set_ime_composition_rect( HWND hwnd, RECT rect ); extern void toggle_caret( HWND hwnd ); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 00023632d13..450381c55f3 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3943,7 +3943,7 @@ BOOL set_window_pos( WINDOWPOS *winpos, int parent_x, int parent_y ) if ((style & (WS_CHILD | WS_POPUP)) == WS_CHILD) send_message( winpos->hwnd, WM_CHILDACTIVATE, 0, 0 ); else if (!(style & WS_MINIMIZE)) - set_foreground_window( winpos->hwnd, FALSE ); + set_foreground_window( winpos->hwnd, FALSE, FALSE ); } if(!(orig_flags & SWP_DEFERERASE)) @@ -4288,7 +4288,7 @@ static void activate_other_window( HWND hwnd ) TRACE( "win = %p fg = %p\n", hwnd_to, fg ); if (!fg || hwnd == fg) { - if (set_foreground_window( hwnd_to, FALSE )) return; + if (set_foreground_window( hwnd_to, FALSE, FALSE )) return; } if (NtUserSetActiveWindow( hwnd_to )) NtUserSetActiveWindow( 0 ); } @@ -5998,6 +5998,9 @@ ULONG_PTR WINAPI NtUserCallHwnd( HWND hwnd, DWORD code ) activate_other_window( hwnd ); return 0; + case NtUserCallHwnd_SetForegroundWindowInternal: + return set_foreground_window( hwnd, FALSE, TRUE ); + case NtUserCallHwnd_GetDpiForWindow: return get_dpi_for_window( hwnd ); diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c index 26ea212043d..acb90420bcd 100644 --- a/dlls/winemac.drv/window.c +++ b/dlls/winemac.drv/window.c @@ -1762,7 +1762,7 @@ void macdrv_window_got_focus(HWND hwnd, const macdrv_event *event) if (can_window_become_foreground(hwnd) && !(style & WS_MINIMIZE)) { TRACE("setting foreground window to %p\n", hwnd); - NtUserSetForegroundWindow(hwnd); + NtUserSetForegroundWindowInternal(hwnd); return; } @@ -1786,7 +1786,7 @@ void macdrv_window_lost_focus(HWND hwnd, const macdrv_event *event) { send_message(hwnd, WM_CANCELMODE, 0, 0); if (hwnd == NtUserGetForegroundWindow()) - NtUserSetForegroundWindow(NtUserGetDesktopWindow()); + NtUserSetForegroundWindowInternal(NtUserGetDesktopWindow()); } } @@ -1815,7 +1815,7 @@ void macdrv_app_deactivated(void) if (get_active_window() == NtUserGetForegroundWindow()) { TRACE("setting fg to desktop\n"); - NtUserSetForegroundWindow(NtUserGetDesktopWindow()); + NtUserSetForegroundWindowInternal(NtUserGetDesktopWindow()); } } @@ -1978,7 +1978,7 @@ void macdrv_window_drag_begin(HWND hwnd, const macdrv_event *event) if (ma != MA_NOACTIVATEANDEAT && ma != MA_NOACTIVATE) { TRACE("setting foreground window to %p\n", hwnd); - NtUserSetForegroundWindow(hwnd); + NtUserSetForegroundWindowInternal(hwnd); } } diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 939be6e3c08..b838fd84191 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -620,7 +620,7 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) wayland_configure_window(hwnd); return 0; case WM_WAYLAND_SET_FOREGROUND: - NtUserSetForegroundWindow(hwnd); + NtUserSetForegroundWindowInternal(hwnd); return 0; default: FIXME("got window msg %x hwnd %p wp %lx lp %lx\n", msg, hwnd, (long)wp, lp); diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index f33db32839f..53c617d2d68 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -657,7 +657,7 @@ static void set_focus( Display *display, HWND focus, Time time ) if (!is_net_supported( x11drv_atom(_NET_ACTIVE_WINDOW) )) { - NtUserSetForegroundWindow( focus ); + NtUserSetForegroundWindowInternal( focus ); threadinfo.cbSize = sizeof(threadinfo); NtUserGetGUIThreadInfo( 0, &threadinfo ); @@ -879,7 +879,7 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) if (!hwnd) hwnd = x11drv_thread_data()->last_focus; if (hwnd && can_activate_window(hwnd)) set_focus( event->display, hwnd, CurrentTime ); } - else NtUserSetForegroundWindow( hwnd ); + else NtUserSetForegroundWindowInternal( hwnd ); return TRUE; } @@ -910,7 +910,7 @@ static void focus_out( Display *display , HWND hwnd ) if (hwnd == NtUserGetForegroundWindow()) { TRACE( "lost focus, setting fg to desktop\n" ); - NtUserSetForegroundWindow( NtUserGetDesktopWindow() ); + NtUserSetForegroundWindowInternal( NtUserGetDesktopWindow() ); } } } diff --git a/include/ntuser.h b/include/ntuser.h index 664bcfe459d..438fca0f52d 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -1305,6 +1305,7 @@ enum NtUserCallHwnd_IsWindowEnabled, NtUserCallHwnd_IsWindowUnicode, NtUserCallHwnd_IsWindowVisible, + NtUserCallHwnd_SetForegroundWindowInternal, /* temporary exports */ NtUserGetFullWindowHandle, NtUserIsCurrentProcessWindow, @@ -1316,6 +1317,11 @@ static inline void NtUserActivateOtherWindow( HWND hwnd ) NtUserCallHwnd( hwnd, NtUserCallHwnd_ActivateOtherWindow ); } +static inline BOOL NtUserSetForegroundWindowInternal( HWND hwnd ) +{ + return NtUserCallHwnd( hwnd, NtUserCallHwnd_SetForegroundWindowInternal ); +} + static inline void *NtUserGetDialogInfo( HWND hwnd ) { return (void *)NtUserCallHwnd( hwnd, NtUserCallHwnd_GetDialogInfo ); diff --git a/server/protocol.def b/server/protocol.def index 1881f0a628a..6712a661817 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3117,6 +3117,7 @@ enum coords_relative /* Set the system foreground window */ @REQ(set_foreground_window) user_handle_t handle; /* handle to the foreground window */ + int internal; /* foreground change comes from the host */ @REPLY user_handle_t previous; /* handle to the previous foreground window */ int send_msg_old; /* whether we have to send a msg to the old window */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9511
From: Rémi Bernon <rbernon(a)codeweavers.com> When called from the application. Changing foreground window to the desktop seems like a bad idea, but it also seem to allow later foreground changes back to application windows, as many D3D tests suggest. Currently we translate these calls into requests to activate the root window, which will usually have no effect, so allowing further changes should be safe. When the internal flag is set, we actually change the foreground to the desktop window, which should later forbid any further changes made from the application side, as this happens when a non-Wine or foreign window has been focused. --- server/queue.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/queue.c b/server/queue.c index 82d5a395957..5ced2299336 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3810,11 +3810,10 @@ DECL_HANDLER(set_foreground_window) reply->send_msg_old = (reply->previous && desktop->foreground_input != queue->input); reply->send_msg_new = FALSE; - if (is_valid_foreground_window( req->handle ) && - (thread = get_window_thread( req->handle )) && - thread->queue->input->desktop == desktop) + if (is_valid_foreground_window( req->handle ) && (thread = get_window_thread( req->handle )) && + (input = thread->queue->input) && input->desktop == desktop) { - set_foreground_input( desktop, thread->queue->input ); + set_foreground_input( desktop, req->internal || thread->process != get_top_window_owner( desktop ) ? input : NULL ); reply->send_msg_new = (desktop->foreground_input != queue->input); } else set_win32_error( ERROR_INVALID_WINDOW_HANDLE ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9511
From: Rémi Bernon <rbernon(a)codeweavers.com> Pegasus Mail posts a WM_USER + 8991 message to its own window when it loses focus, then calls SetFocus on its window when processing this message. Several other applications have been seen calling SetWindowPos during WM_ACTIVATE message, which might also attempt to reactivate the window. That processing happens shortly after we have changed the foreground window to the desktop window, when focus is lost, then SetFocus tries to change the foreground window again. This SetFocus behavior is tested, and should work like this, but would only activate the window if the process is allowed to do so. Windows has various rules around this, and it seems to boil down to something like: * Allow taking focus if the process never was foreground, ie: when process is starting and gets initial focus on its windows. * Allow taking focus if the process was foreground but lost it recently because of a window being destroyed. * Forbid taking focus back if the process had foreground and called SetForegroundWindow explicitly to give it to another process. This doesn't implement all this rules, but rather keep rely on the host window management with some additional heuristics to avoid activating windows which lost foreground recently. This is mostly about keeping track of user input time, updating it on user input and on focus change. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 --- server/process.c | 1 + server/process.h | 1 + server/queue.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- server/winstation.c | 1 + 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/server/process.c b/server/process.c index 52abaa21d75..4a06595c66e 100644 --- a/server/process.c +++ b/server/process.c @@ -682,6 +682,7 @@ struct process *create_process( int fd, struct process *parent, unsigned int fla process->is_system = 0; process->debug_children = 1; process->is_terminating = 0; + process->set_foreground = 0; process->imagelen = 0; process->image = NULL; process->job = NULL; diff --git a/server/process.h b/server/process.h index 5c136fb5103..7cfcc39c258 100644 --- a/server/process.h +++ b/server/process.h @@ -63,6 +63,7 @@ struct process unsigned int is_system:1; /* is it a system process? */ unsigned int debug_children:1;/* also debug all child processes */ unsigned int is_terminating:1;/* is process terminating? */ + unsigned int set_foreground:1;/* has process called set_foreground_window */ data_size_t imagelen; /* length of image path in bytes */ WCHAR *image; /* main exe image full path */ struct job *job; /* job object associated with this process */ diff --git a/server/queue.c b/server/queue.c index 5ced2299336..4f463d880d9 100644 --- a/server/queue.c +++ b/server/queue.c @@ -22,6 +22,7 @@ #include <assert.h> #include <stdarg.h> +#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> @@ -112,6 +113,7 @@ struct thread_input int caret_hide; /* caret hide count */ int caret_state; /* caret on/off state */ struct list msg_list; /* list of hardware messages */ + timeout_t user_time; /* time of last user input */ unsigned char desktop_keystate[256]; /* desktop keystate when keystate was synced */ input_shm_t *shared; /* thread input in session shared memory */ }; @@ -253,6 +255,7 @@ static struct thread_input *create_thread_input( struct thread *thread ) if ((input = alloc_object( &thread_input_ops ))) { list_init( &input->msg_list ); + input->user_time = 0; input->shared = NULL; if (!(input->desktop = get_thread_desktop( thread, 0 /* FIXME: access rights */ ))) @@ -665,6 +668,12 @@ static void set_foreground_input( struct desktop *desktop, struct thread_input * input_shm_t *input_shm, *old_input_shm; shared_object_t dummy_obj = {0}; + if (input) + { + input->user_time = monotonic_time; + if (debug_level) fprintf( stderr, "%04x: updating input %p user_time %ju\n", current->id, input, (uintmax_t)input->user_time); + } + if (desktop->foreground_input == input) return; input_shm = input ? input->shared : &dummy_obj.shm.input; old_input_shm = desktop->foreground_input ? desktop->foreground_input->shared : &dummy_obj.shm.input; @@ -1927,6 +1936,13 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg win = find_hardware_message_window( desktop, input, msg, &msg_code, &thread ); flags = thread ? get_rawinput_device_flags( thread->process, msg ) : 0; + if (thread) input = thread->queue->input; + if (input && (get_hardware_msg_bit( msg->msg ) & (QS_KEY | QS_MOUSEBUTTON))) + { + input->user_time = monotonic_time; + if (debug_level) fprintf( stderr, "%04x: updating input %p user_time %ju\n", current->id, input, (uintmax_t)input->user_time); + } + if (!win || !thread || (flags & RIDEV_NOLEGACY)) { if (input && !(flags & RIDEV_NOLEGACY)) update_thread_input_key_state( input, msg->msg, msg->wparam ); @@ -1935,8 +1951,6 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg return; } - input = thread->queue->input; - if (win != msg->win) always_queue = 1; if (!always_queue || merge_message( input, msg )) free_message( msg ); else @@ -2000,6 +2014,18 @@ static struct thread *get_foreground_thread( struct desktop *desktop, user_handl return NULL; } +static int is_current_process_foreground( struct desktop *desktop ) +{ + struct thread *thread; + int ret; + + if (!(thread = get_foreground_thread( desktop, 0 ))) return 0; + ret = thread->process == current->process; + release_object( thread ); + + return ret; +} + /* user32 reserves 1 & 2 for winemouse and winekeyboard, * keep this in sync with user_private.h */ #define WINE_MOUSE_HANDLE 1 @@ -3802,11 +3828,24 @@ DECL_HANDLER(set_foreground_window) struct thread_input *input; struct msg_queue *queue = get_current_queue(); - if (!(desktop = get_thread_desktop( current, 0 ))) return; + if (!queue || !(desktop = get_thread_desktop( current, 0 ))) return; if (!(input = desktop->foreground_input)) reply->previous = 0; else reply->previous = input->shared->active; + if (!req->internal) + { + if (!current->process->set_foreground) current->process->set_foreground = 1; + else if (!is_current_process_foreground( desktop ) && queue->input && input && queue->input->user_time < input->user_time) + { + if (debug_level) fprintf( stderr, "%04x: refusing set_foreground_window user_time %ju < %ju\n", current->id, + (uintmax_t)queue->input->user_time, (uintmax_t)input->user_time ); + set_win32_error( ERROR_ACCESS_DENIED ); + release_object( desktop ); + return; + } + } + reply->send_msg_old = (reply->previous && desktop->foreground_input != queue->input); reply->send_msg_new = FALSE; diff --git a/server/winstation.c b/server/winstation.c index 52c3f1ce1eb..a36253e20db 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -864,6 +864,7 @@ DECL_HANDLER(set_thread_desktop) { if (old_desktop) remove_desktop_thread( old_desktop, current ); add_desktop_thread( new_desktop, current ); + current->process->set_foreground = 0; } reply->locator = get_shared_object_locator( new_desktop->shared ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9511
v2: Tweak and reduce a bit more the conditions where the SetForegroundWindow call is refused: * Set NULL foreground input, allowing any later change, if `SetForegrdounWindow(GetDesktopWindow())` is called from the application. The `d3d8:device` and `d3d9:device` tests do that a lot (and probably shouldn't), and expect to be able to get foreground back right after. We still actually make the desktop window foreground, and forbid further changes, when the user drivers are calling it to indicate a host focus loss to a foreign window. * Allow processes to set foreground again after SetThreadDesktop has been called. This shouldn't happen very often in practice, and it fixes an `dinput:joystick8` test with and exotic cross-desktop use case. * Allow any thread from the current foreground process to set foreground to a different window. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9511#note_122831
participants (1)
-
Rémi Bernon