From: "Rémi Bernon" Subject: [PATCH v2 4/5] user32: Add generation counter to set_active_window requests. Message-Id: <20200902170102.324526-4-rbernon@codeweavers.com> Date: Wed, 2 Sep 2020 19:01:01 +0200 In-Reply-To: <20200902170102.324526-1-rbernon@codeweavers.com> References: <20200902170102.324526-1-rbernon@codeweavers.com> So that we can correctly track out of order window activation requests from other threads calling SetForegroundWindow and process or discard them correctly. A new generation is created every time the foreground thread changes its active or focus window state, so that any previously sent activation messages are discarded (with some tricky details shown by the tests). Signed-off-by: Rémi Bernon --- v2: Use a generation counter instead of discarding messages and messing with the message queues. dlls/user32/focus.c | 65 ++++++++++++++++++++++++++++++-------- dlls/user32/message.c | 2 +- dlls/user32/tests/win.c | 46 ++++++++++++--------------- dlls/user32/user_private.h | 1 + server/protocol.def | 4 +++ server/queue.c | 6 ++++ 6 files changed, 83 insertions(+), 41 deletions(-) diff --git a/dlls/user32/focus.c b/dlls/user32/focus.c index f1c883167ed..b88a711b05e 100644 --- a/dlls/user32/focus.c +++ b/dlls/user32/focus.c @@ -70,17 +70,41 @@ static HWND set_focus_window( HWND hwnd ) return previous; } +static void get_active_window( HWND *active, DWORD *active_gen ) +{ + *active = NULL; + *active_gen = 0; + SERVER_START_REQ( get_thread_input ) + { + req->tid = GetCurrentThreadId(); + if (wine_server_call_err( req )) return; + *active = wine_server_ptr_handle( reply->active ); + *active_gen = reply->active_gen; + } + SERVER_END_REQ; +} + /******************************************************************* * 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, DWORD generation ) { - HWND previous = GetActiveWindow(); + HWND previous; BOOL ret; - DWORD old_thread, new_thread; + DWORD old_thread, new_thread, previous_gen; CBTACTIVATESTRUCT cbt; + TRACE( "hwnd %p, prev %p, mouse %d, focus %d, generation %u\n", hwnd, prev, mouse, focus, generation ); + + get_active_window( &previous, &previous_gen ); + + if ((generation - previous_gen) > 1) + { + WARN( "ignoring out of order generation %u, previous_gen %u\n", generation, previous_gen ); + return FALSE; + } + if (previous == hwnd) { if (prev) *prev = hwnd; @@ -102,6 +126,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) SERVER_START_REQ( set_active_window ) { req->handle = wine_server_user_handle( hwnd ); + req->generation = generation; if ((ret = !wine_server_call_err( req ))) previous = wine_server_ptr_handle( reply->previous ); } @@ -182,6 +207,7 @@ static BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus ) */ static BOOL set_foreground_window( HWND hwnd, BOOL mouse ) { + DWORD active_gen_old, active_gen_new; BOOL ret, send_msg_old = FALSE, send_msg_new = FALSE; HWND previous = 0; @@ -192,7 +218,9 @@ static BOOL set_foreground_window( HWND hwnd, BOOL mouse ) { previous = wine_server_ptr_handle( reply->previous ); send_msg_old = reply->send_msg_old; + active_gen_old = reply->active_gen_old; send_msg_new = reply->send_msg_new; + active_gen_new = reply->active_gen_new; } } SERVER_END_REQ; @@ -200,14 +228,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, (LPARAM)active_gen_old ); 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, active_gen_old + 1 ); 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, (LPARAM)active_gen_new ); else /* new window belongs to us */ - ret = set_active_window( hwnd, NULL, mouse, TRUE ); + ret = set_active_window( hwnd, NULL, mouse, TRUE, active_gen_new + 1 ); } return ret; } @@ -229,10 +257,13 @@ BOOL FOCUS_MouseActivate( HWND hwnd ) */ HWND WINAPI SetActiveWindow( HWND hwnd ) { - HWND prev; + HWND active = NULL; + DWORD active_gen = 0; TRACE( "%p\n", hwnd ); + get_active_window( &active, &active_gen ); + if (hwnd) { LONG style; @@ -246,11 +277,13 @@ HWND WINAPI SetActiveWindow( HWND hwnd ) style = GetWindowLongW( hwnd, GWL_STYLE ); if ((style & (WS_POPUP|WS_CHILD)) == WS_CHILD) - return GetActiveWindow(); /* Windows doesn't seem to return an error here */ + return active; /* Windows doesn't seem to return an error here */ } - if (!set_active_window( hwnd, &prev, FALSE, TRUE )) return 0; - return prev; + if (GetWindowThreadProcessId( GetForegroundWindow(), NULL ) == GetCurrentThreadId()) + active_gen++; + if (!set_active_window( hwnd, &active, FALSE, TRUE, active_gen )) return 0; + return active; } @@ -259,8 +292,9 @@ HWND WINAPI SetActiveWindow( HWND hwnd ) */ HWND WINAPI SetFocus( HWND hwnd ) { - HWND hwndTop = hwnd; + HWND hwndTop = hwnd, active; HWND previous = GetFocus(); + DWORD active_gen; TRACE( "%p prev %p\n", hwnd, previous ); @@ -294,9 +328,12 @@ HWND WINAPI SetFocus( HWND hwnd ) if (HOOK_CallHooks( WH_CBT, HCBT_SETFOCUS, (WPARAM)hwnd, (LPARAM)previous, TRUE )) return 0; /* activate hwndTop if needed. */ - if (hwndTop != GetActiveWindow()) + get_active_window( &active, &active_gen ); + if (hwndTop != active) { - if (!set_active_window( hwndTop, NULL, FALSE, FALSE )) return 0; + if (GetWindowThreadProcessId( GetForegroundWindow(), NULL ) == GetCurrentThreadId()) + active_gen++; + if (!set_active_window( hwndTop, NULL, FALSE, FALSE, active_gen )) 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 9d08aa22120..1f5eebf7248 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -1875,7 +1875,7 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR case WM_WINE_SETACTIVEWINDOW: if (!wparam && GetWindowThreadProcessId( GetForegroundWindow(), NULL ) == GetCurrentThreadId()) return 0; - return (LRESULT)SetActiveWindow( (HWND)wparam ); + return (LRESULT)set_active_window( (HWND)wparam, NULL, FALSE, TRUE, (DWORD)lparam ); 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 bdc7a8962e0..75a4cdc42d5 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -3268,33 +3268,31 @@ struct test_sfw_test_desc int msgcount_before_set_foreground; BOOL todo_msgcount_after_set_foreground; int msgcount_after_set_foreground; - BOOL todo_msgcount_after_peek_message; int msgcount_after_peek_message; - BOOL todo_expected_window; int expected_window; }; static struct test_sfw_test_desc test_sfw_tests[] = { - {1, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 0, FALSE, 6, FALSE, 0}, - {1, TRUE, FALSE, FALSE, FALSE, 0, TRUE, 1, FALSE, 6, FALSE, 0}, - {1, FALSE, TRUE, FALSE, FALSE, 0, FALSE, 0, FALSE, 6, FALSE, 0}, - {1, TRUE, TRUE, FALSE, FALSE, 0, TRUE, 1, FALSE, 6, FALSE, 0}, - {1, FALSE, FALSE, TRUE, FALSE, 0, FALSE, 0, FALSE, 6, FALSE, 0}, - {1, TRUE, FALSE, TRUE, FALSE, 0, TRUE, 1, FALSE, 6, FALSE, 0}, - - {2, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 6, TRUE, 0, TRUE, 1}, - {2, TRUE, FALSE, FALSE, FALSE, 0, FALSE, 6, TRUE, 0, TRUE, 1}, - {2, FALSE, TRUE, FALSE, FALSE, 6, FALSE, 0, TRUE, 0, TRUE, 1}, - {2, TRUE, TRUE, FALSE, FALSE, 6, TRUE, 1, FALSE, 6, FALSE, 0}, - {2, FALSE, FALSE, TRUE, TRUE, 8, FALSE, 0, TRUE, 0, TRUE, 1}, - {2, TRUE, FALSE, TRUE, TRUE, 8, TRUE, 1, FALSE, 6, FALSE, 0}, - - {0, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 6, FALSE, 0, FALSE, 1}, - {0, TRUE, FALSE, FALSE, FALSE, 0, FALSE, 6, TRUE, 0, TRUE, 1}, - {0, FALSE, TRUE, FALSE, FALSE, 6, FALSE, 0, FALSE, 0, FALSE, 1}, - {0, TRUE, TRUE, FALSE, FALSE, 6, TRUE, 1, FALSE, 6, FALSE, 0}, - {0, FALSE, FALSE, TRUE, TRUE, 8, FALSE, 0, FALSE, 0, FALSE, 1}, - {0, TRUE, FALSE, TRUE, TRUE, 8, TRUE, 1, FALSE, 6, FALSE, 0}, + {1, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 0, 6, 0}, + {1, TRUE, FALSE, FALSE, FALSE, 0, TRUE, 1, 6, 0}, + {1, FALSE, TRUE, FALSE, FALSE, 0, FALSE, 0, 6, 0}, + {1, TRUE, TRUE, FALSE, FALSE, 0, TRUE, 1, 6, 0}, + {1, FALSE, FALSE, TRUE, FALSE, 0, FALSE, 0, 6, 0}, + {1, TRUE, FALSE, TRUE, FALSE, 0, TRUE, 1, 6, 0}, + + {2, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 6, 0, 1}, + {2, TRUE, FALSE, FALSE, FALSE, 0, FALSE, 6, 0, 1}, + {2, FALSE, TRUE, FALSE, FALSE, 6, FALSE, 0, 0, 1}, + {2, TRUE, TRUE, FALSE, FALSE, 6, TRUE, 1, 6, 0}, + {2, FALSE, FALSE, TRUE, TRUE, 8, FALSE, 0, 0, 1}, + {2, TRUE, FALSE, TRUE, TRUE, 8, TRUE, 1, 6, 0}, + + {0, FALSE, FALSE, FALSE, FALSE, 0, FALSE, 6, 0, 1}, + {0, TRUE, FALSE, FALSE, FALSE, 0, FALSE, 6, 0, 1}, + {0, FALSE, TRUE, FALSE, FALSE, 6, FALSE, 0, 0, 1}, + {0, TRUE, TRUE, FALSE, FALSE, 6, TRUE, 1, 6, 0}, + {0, FALSE, FALSE, TRUE, TRUE, 8, FALSE, 0, 0, 1}, + {0, TRUE, FALSE, TRUE, TRUE, 8, TRUE, 1, 6, 0}, }; static DWORD WINAPI test_sfw_thread( void *param ) @@ -3368,16 +3366,12 @@ static DWORD WINAPI test_sfw_thread( void *param ) ok( GetActiveWindow() == windows[1], "GetActiveWindow() returned %p\n", GetActiveWindow() ); ok( GetFocus() == windows[1], "GetFocus() returned %p\n", GetFocus() ); while (PeekMessageA( &msg, 0, 0, 0, PM_REMOVE )) DispatchMessageA( &msg ); - todo_wine_if( test->todo_msgcount_after_peek_message ) ok( test_sfw_msg_count == test->msgcount_after_peek_message, "%d: Unexpected number of messages received after PeekMessageA: %d\n", i, test_sfw_msg_count ); - todo_wine_if( test->todo_expected_window ) ok( GetForegroundWindow() == expected_window, "%d: GetForegroundWindow() returned %p\n", i, GetForegroundWindow() ); - todo_wine_if( test->todo_expected_window ) ok( GetActiveWindow() == expected_window, "%d: GetActiveWindow() returned %p\n", i, GetActiveWindow() ); - todo_wine_if( test->todo_expected_window ) ok( GetFocus() == expected_window, "%d: GetFocus() returned %p\n", i, GetFocus() ); res = WaitForSingleObject( test_sfw_done, INFINITE ); diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index 8fa54b9229a..aa310c65cfd 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -246,6 +246,7 @@ extern struct rawinput_thread_data *rawinput_thread_data(void); 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, DWORD generation ) 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; diff --git a/server/protocol.def b/server/protocol.def index 92290af701c..1b1fdf6fda9 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2768,6 +2768,7 @@ enum coords_relative user_handle_t focus; /* handle to the focus window */ user_handle_t capture; /* handle to the capture window */ user_handle_t active; /* handle to the active window */ + unsigned int active_gen; /* generation of the last set_active_window */ user_handle_t foreground; /* handle to the global foreground window */ user_handle_t menu_owner; /* handle to the menu owner */ user_handle_t move_size; /* handle to the moving/resizing window */ @@ -2807,7 +2808,9 @@ enum coords_relative @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 */ + unsigned int active_gen_old;/* set_active_window generation for the old window */ int send_msg_new; /* whether we have to send a msg to the new window */ + unsigned int active_gen_new;/* set_active_window generation for the new window */ @END /* Set the current thread focus window */ @@ -2820,6 +2823,7 @@ enum coords_relative /* Set the current thread active window */ @REQ(set_active_window) user_handle_t handle; /* handle to the active window */ + unsigned int generation; /* set_active_window generation number */ @REPLY user_handle_t previous; /* handle to the previous active window */ @END diff --git a/server/queue.c b/server/queue.c index c1016016051..65cc9e9eb76 100644 --- a/server/queue.c +++ b/server/queue.c @@ -104,6 +104,7 @@ struct thread_input user_handle_t focus; /* focus window */ user_handle_t capture; /* capture window */ user_handle_t active; /* active window */ + unsigned int active_gen; /* set_active_window generation */ user_handle_t menu_owner; /* current menu owner window */ user_handle_t move_size; /* current moving/resizing window */ user_handle_t caret; /* caret window */ @@ -253,6 +254,7 @@ static struct thread_input *create_thread_input( struct thread *thread ) input->focus = 0; input->capture = 0; input->active = 0; + input->active_gen = 0; input->menu_owner = 0; input->move_size = 0; input->cursor = 0; @@ -2929,6 +2931,7 @@ DECL_HANDLER(get_thread_input) reply->focus = input->focus; reply->capture = input->capture; reply->active = input->active; + reply->active_gen = input->active_gen; reply->menu_owner = input->menu_owner; reply->move_size = input->move_size; reply->caret = input->caret; @@ -3026,6 +3029,7 @@ DECL_HANDLER(set_foreground_window) reply->previous = desktop->foreground_input ? desktop->foreground_input->active : 0; reply->send_msg_old = (reply->previous && desktop->foreground_input != queue->input); reply->send_msg_new = FALSE; + reply->active_gen_old = desktop->foreground_input ? desktop->foreground_input->active_gen : 0; if (is_valid_foreground_window( req->handle ) && (thread = get_window_thread( req->handle )) && @@ -3033,6 +3037,7 @@ DECL_HANDLER(set_foreground_window) { set_foreground_input( desktop, thread->queue->input ); reply->send_msg_new = (desktop->foreground_input != queue->input); + reply->active_gen_new = desktop->foreground_input ? desktop->foreground_input->active_gen : 0; } else set_win32_error( ERROR_INVALID_WINDOW_HANDLE ); @@ -3067,6 +3072,7 @@ DECL_HANDLER(set_active_window) { reply->previous = queue->input->active; queue->input->active = get_user_full_handle( req->handle ); + queue->input->active_gen = req->generation; } else set_error( STATUS_INVALID_HANDLE ); } -- 2.28.0