[PATCH v2 1/8] user32/tests: Add concurrency tests for SetForegroundWindow.

Rémi Bernon rbernon at codeweavers.com
Thu Oct 1 05:19:57 CDT 2020


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.

This is not always the correct behavior and these tests help determine
what should actually be done in various situations.

This aims to check the following sequences, with A being a separate
thread that created three windows, and B being the main test thread
with some windows initially in background:

* window A0, A1, or A2 is foreground, then:
  * B sets foreground to window A0
  * A sets foreground to window A1

As well as these sequences where foreground is also temporarily switched
to window B0:

* window A0, A1, or A2 is foreground, then:
  * B sets foreground to window B0
  * B sets foreground to window A0
  * B sets foreground to window B0
  * A sets foreground to window A1

In addition, we also do tests with additional SetActiveWindow / SetFocus
calls to check their influence.

Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
---

Another try at this. Back to message filtering, as it's a more generic
solution than last time counter.

It still needs to be able to filter these internal messages so we have
to change get_message request handler a bit. We don't provide any way
to specifically remove messages though.

v2: * Remove cross-process test and the related debugger patch, the
      results are the same as cross-thread.

    * Trace counted messages in the test so we can compare traces, add
      more test traces too.

    * Send a WM_USER message too, to validate that we shouldn't process
      sent messages while filtering internal messages.

    * Filter internal WM_WINE_SETACTIVEWINDOW messages using a thread
      local flag and a call to peek_message.

    * Add a fix for the additional focus messages received when calling
      SetFocus, even if the window already has focus, fixing the last
      remaining todo in the test.

Supersedes: 191845-191849

 dlls/user32/tests/win.c | 290 +++++++++++++++++++++++++++++++++-------
 1 file changed, 238 insertions(+), 52 deletions(-)

diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index 843da8900f1..bbc3fbfb1d9 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -3240,40 +3240,180 @@ static void test_SetActiveWindow(HWND hwnd)
     DestroyWindow(hwnd2);
 }
 
-struct create_window_thread_params
+static int test_sfw_msg_count;
+
+static LRESULT WINAPI test_sfw_wndproc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam )
 {
-    HWND window;
-    HANDLE window_created;
-    HANDLE test_finished;
+    switch (msg)
+    {
+    case WM_NCACTIVATE: trace("%p WM_NCACTIVATE %d %p\n", hwnd, (DWORD)wparam, (HWND)lparam); test_sfw_msg_count++; break;
+    case WM_ACTIVATE: trace("%p WM_ACTIVATE %d %p\n", hwnd, (DWORD)wparam, (HWND)lparam); test_sfw_msg_count++; break;
+    case WM_SETFOCUS: trace("%p WM_SETFOCUS %p %d\n", hwnd, (HWND)wparam, (DWORD)lparam); test_sfw_msg_count++; break;
+    case WM_KILLFOCUS: trace("%p WM_KILLFOCUS %p %d\n", hwnd, (HWND)wparam, (DWORD)lparam); test_sfw_msg_count++; break;
+    case WM_USER: trace("%p WM_USER %d %d\n", hwnd, (DWORD)wparam, (DWORD)lparam); test_sfw_msg_count++; break;
+    }
+
+    return DefWindowProcA( hwnd, msg, wparam, lparam );
+}
+
+struct test_sfw_test_desc
+{
+    int  initial_window;
+    BOOL steal_foreground;
+    BOOL call_set_active_window;
+    BOOL call_set_focus;
+
+    BOOL todo_msgcount_before_set_foreground;
+    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, 7, FALSE, 0},
+    {1, TRUE, FALSE, FALSE,   FALSE, 0,  TRUE, 1,  TRUE, 7,  TRUE, 0},
+    {1, FALSE, TRUE, FALSE,   FALSE, 0, FALSE, 0, FALSE, 7, FALSE, 0},
+    {1, TRUE, TRUE, FALSE,    FALSE, 0,  TRUE, 1,  TRUE, 7,  TRUE, 0},
+    {1, FALSE, FALSE, TRUE,   FALSE, 0, FALSE, 0, FALSE, 7, FALSE, 0},
+    {1, TRUE, FALSE, TRUE,    FALSE, 0,  TRUE, 1,  TRUE, 7,  TRUE, 0},
+
+    {2, FALSE, FALSE, FALSE,  FALSE, 0, FALSE, 6,  TRUE, 1,  TRUE, 1},
+    {2, TRUE, FALSE, FALSE,   FALSE, 0, FALSE, 6,  TRUE, 1,  TRUE, 1},
+    {2, FALSE, TRUE, FALSE,   FALSE, 6, FALSE, 0,  TRUE, 1,  TRUE, 1},
+    {2, TRUE, TRUE, FALSE,    FALSE, 6,  TRUE, 1,  TRUE, 7,  TRUE, 0},
+    {2, FALSE, FALSE, TRUE,    TRUE, 8, FALSE, 0,  TRUE, 1,  TRUE, 1},
+    {2, TRUE, FALSE, TRUE,     TRUE, 8,  TRUE, 1,  TRUE, 7,  TRUE, 0},
+
+    {0, FALSE, FALSE, FALSE,  FALSE, 0, FALSE, 6, FALSE, 1, FALSE, 1},
+    {0, TRUE, FALSE, FALSE,   FALSE, 0, FALSE, 6,  TRUE, 1,  TRUE, 1},
+    {0, FALSE, TRUE, FALSE,   FALSE, 6, FALSE, 0, FALSE, 1, FALSE, 1},
+    {0, TRUE, TRUE, FALSE,    FALSE, 6,  TRUE, 1, FALSE, 7, FALSE, 0},
+    {0, FALSE, FALSE, TRUE,    TRUE, 8, FALSE, 0, FALSE, 1, FALSE, 1},
+    {0, TRUE, FALSE, TRUE,     TRUE, 8,  TRUE, 1, FALSE, 7, FALSE, 0},
 };
 
-static DWORD WINAPI create_window_thread(void *param)
+static DWORD WINAPI test_sfw_thread( void *param )
 {
-    struct create_window_thread_params *p = param;
+    HANDLE test_sfw_ready, test_sfw_start, test_sfw_done;
+    WNDPROC wndprocs[3];
+    HWND windows[3];
     DWORD res;
     BOOL ret;
+    MSG msg;
+    int i;
+
+    test_sfw_ready = OpenEventA( EVENT_ALL_ACCESS, FALSE, "test_sfw_ready" );
+    test_sfw_start = OpenEventA( EVENT_ALL_ACCESS, FALSE, "test_sfw_start" );
+    test_sfw_done = OpenEventA( EVENT_ALL_ACCESS, FALSE, "test_sfw_done" );
 
-    p->window = CreateWindowA("static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0);
+    windows[1] = CreateWindowA( "static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0 );
+    windows[2] = CreateWindowA( "static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0 );
+    windows[0] = CreateWindowA( "static", "test_sfw_window", WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0 );
+    trace( "window[0]:%p windows[1]:%p windows[2]:%p\n", windows[0], windows[1], windows[2] );
 
-    ret = SetEvent(p->window_created);
+    ret = SetEvent( test_sfw_ready );
     ok(ret, "SetEvent failed, last error %#x.\n", GetLastError());
 
-    res = WaitForSingleObject(p->test_finished, INFINITE);
+    /* wait for the initial state to be clean */
+
+    res = WaitForSingleObject( test_sfw_start, INFINITE );
     ok(res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError());
+    ret = ResetEvent( test_sfw_start );
+    ok( ret, "ResetEvent failed, last error %#x.\n", GetLastError() );
+
+    for (i = 0; i < ARRAY_SIZE(windows); ++i)
+        wndprocs[i] = (WNDPROC)SetWindowLongPtrA( windows[i], GWLP_WNDPROC, (LONG_PTR)test_sfw_wndproc );
+
+    flush_events( TRUE );
+
+    for (i = 0; i < ARRAY_SIZE(test_sfw_tests); ++i)
+    {
+        struct test_sfw_test_desc *test = test_sfw_tests + i;
+        HWND initial_window = windows[test->initial_window];
+        HWND expected_window = windows[test->expected_window];
+        trace( "running test %d\n", i );
+
+        SetForegroundWindow( initial_window );
+        flush_events( TRUE );
+        check_wnd_state( initial_window, initial_window, initial_window, 0 );
+
+        ret = SetEvent( test_sfw_ready );
+        ok( ret, "SetEvent failed, last error %#x.\n", GetLastError() );
+
+        res = WaitForSingleObject( test_sfw_start, INFINITE );
+        ok( res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError() );
+        ret = ResetEvent( test_sfw_start );
+        ok( ret, "ResetEvent failed, last error %#x.\n", GetLastError() );
+
+        test_sfw_msg_count = 0;
+        trace( "%d: before SetForegroundWindow\n", i );
+        if (test->call_set_active_window) SetActiveWindow( windows[1] );
+        if (test->call_set_focus) SetFocus( windows[1] );
+        todo_wine_if( test->todo_msgcount_before_set_foreground )
+        ok( test_sfw_msg_count == test->msgcount_before_set_foreground,
+            "%d: Unexpected number of messages received before SetForegroundWindow: %d\n", i, test_sfw_msg_count );
+
+        test_sfw_msg_count = 0;
+        trace( "%d: calling SetForegroundWindow\n", i );
+        SetForegroundWindow( windows[1] );
+        todo_wine_if( test->todo_msgcount_after_set_foreground )
+        ok( test_sfw_msg_count == test->msgcount_after_set_foreground,
+            "%d: Unexpected number of messages received after SetForegroundWindow: %d\n", i, test_sfw_msg_count );
+
+        ok( GetForegroundWindow() == windows[1], "%d: GetForegroundWindow() returned %p\n", i,
+            GetForegroundWindow() );
+        ok( GetActiveWindow() == windows[1], "%d: GetActiveWindow() returned %p\n", i, GetActiveWindow() );
+        ok( GetFocus() == windows[1], "%d: GetFocus() returned %p\n", i, GetFocus() );
+
+        test_sfw_msg_count = 0;
+        trace( "%d: calling PeekMessageA\n", i );
+        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() );
+        trace( "%d: done\n", i );
+
+        res = WaitForSingleObject( test_sfw_done, INFINITE );
+        ok( res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError() );
+        ret = ResetEvent( test_sfw_done );
+        ok( ret, "ResetEvent failed, last error %#x.\n", GetLastError() );
+    }
+
+    for (i = 0; i < ARRAY_SIZE(windows); ++i)
+        SetWindowLongPtrA( windows[i], GWLP_WNDPROC, (LONG_PTR)wndprocs[i] );
+
+    for (i = 0; i < ARRAY_SIZE(windows); ++i) DestroyWindow( windows[i] );
+
+    CloseHandle( test_sfw_ready );
+    CloseHandle( test_sfw_start );
+    CloseHandle( test_sfw_done );
 
-    DestroyWindow(p->window);
     return 0;
 }
 
-static void test_SetForegroundWindow(HWND hwnd)
+static void test_SetForegroundWindow( HWND hwnd )
 {
-    struct create_window_thread_params thread_params;
-    HANDLE thread;
+    HANDLE thread = 0;
+    HANDLE test_sfw_ready, test_sfw_start, test_sfw_done;
     DWORD res, tid;
-    BOOL ret;
+    HWND test_sfw_window;
     HWND hwnd2;
-    MSG msg;
     LONG style;
+    BOOL ret;
+    MSG msg;
+    int i;
 
     flush_events( TRUE );
     ShowWindow(hwnd, SW_HIDE);
@@ -3347,50 +3487,96 @@ static void test_SetForegroundWindow(HWND hwnd)
     DestroyWindow(hwnd2);
     check_wnd_state(hwnd, hwnd, hwnd, 0);
 
-    hwnd2 = CreateWindowA("static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0);
-    check_wnd_state(hwnd2, hwnd2, hwnd2, 0);
+    hwnd2 = CreateWindowA( "static", NULL, WS_POPUP | WS_VISIBLE, 0, 0, 0, 0, 0, 0, 0, 0 );
+    check_wnd_state( hwnd2, hwnd2, hwnd2, 0 );
 
-    thread_params.window_created = CreateEventW(NULL, FALSE, FALSE, NULL);
-    ok(!!thread_params.window_created, "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);
-    ok(!!thread, "Failed to create thread, last error %#x.\n", GetLastError());
-    res = WaitForSingleObject(thread_params.window_created, INFINITE);
-    ok(res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError());
-    check_wnd_state(hwnd2, thread_params.window, hwnd2, 0);
+    test_sfw_ready = CreateEventA( NULL, FALSE, FALSE, "test_sfw_ready" );
+    ok( !!test_sfw_ready, "CreateEvent failed, last error %#x.\n", GetLastError() );
+    test_sfw_start = CreateEventA( NULL, FALSE, FALSE, "test_sfw_start" );
+    ok( !!test_sfw_start, "CreateEvent failed, last error %#x.\n", GetLastError() );
+    test_sfw_done = CreateEventA( NULL, FALSE, FALSE, "test_sfw_done" );
+    ok( !!test_sfw_done, "CreateEvent failed, last error %#x.\n", GetLastError() );
 
-    SetForegroundWindow(hwnd2);
-    check_wnd_state(hwnd2, hwnd2, hwnd2, 0);
+    thread = CreateThread( NULL, 0, test_sfw_thread, NULL, 0, &tid );
+    ok( !!thread, "Failed to create thread, last error %#x.\n", GetLastError() );
 
-    while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg);
-    if (0) check_wnd_state(hwnd2, hwnd2, hwnd2, 0);
+    res = WaitForSingleObject( test_sfw_ready, INFINITE );
+    ok( res == WAIT_OBJECT_0, "Wait failed (%#x), last error %#x.\n", res, GetLastError() );
+    ret = ResetEvent( test_sfw_ready );
+    ok( ret, "ResetEvent failed, last error %#x.\n", GetLastError() );
+
+    test_sfw_window = FindWindowA( "static", "test_sfw_window" );
+    ok( test_sfw_window != NULL, "FindWindowA failed to find test window, last error %#x\n", GetLastError() );
+    check_wnd_state( hwnd2, test_sfw_window, hwnd2, 0 );
+
+    /* ensure initial state is consistent, with hwnd as active / foreground / focus window */
+
+    SetForegroundWindow( hwnd2 );
+    check_wnd_state( hwnd2, hwnd2, hwnd2, 0 );
+
+    while (PeekMessageA( &msg, 0, 0, 0, PM_REMOVE )) DispatchMessageA( &msg );
+    if (0) check_wnd_state( hwnd2, hwnd2, hwnd2, 0 );
 
     /* FIXME: these tests are failing because of a race condition
-     * between internal focus state applied immediately and X11 focus
-     * message coming late */
-    todo_wine ok(GetActiveWindow() == hwnd2, "Expected active window %p, got %p.\n", hwnd2, GetActiveWindow());
-    todo_wine ok(GetFocus() == hwnd2, "Expected focus window %p, got %p.\n", hwnd2, GetFocus());
+     * between internal process focus state applied immediately and
+     * X11 focus message coming late */
+    todo_wine
+    ok( GetActiveWindow() == hwnd2, "Expected active window %p, got %p.\n", hwnd2, GetActiveWindow() );
+    todo_wine
+    ok( GetFocus() == hwnd2, "Expected focus window %p, got %p.\n", hwnd2, GetFocus() );
 
-    SetForegroundWindow(hwnd);
-    check_wnd_state(hwnd, hwnd, hwnd, 0);
-    style = GetWindowLongA(hwnd2, GWL_STYLE) | WS_CHILD;
-    ok(SetWindowLongA(hwnd2, GWL_STYLE, style), "SetWindowLong failed\n");
-    ok(SetForegroundWindow(hwnd2), "SetForegroundWindow failed\n");
-    check_wnd_state(hwnd2, hwnd2, hwnd2, 0);
+    SetForegroundWindow( hwnd );
+    check_wnd_state( hwnd, hwnd, hwnd, 0 );
+    style = GetWindowLongA( hwnd2, GWL_STYLE ) | WS_CHILD;
+    ok( SetWindowLongA( hwnd2, GWL_STYLE, style ), "SetWindowLong failed\n" );
+    ok( SetForegroundWindow( hwnd2 ), "SetForegroundWindow failed\n" );
+    check_wnd_state( hwnd2, hwnd2, hwnd2, 0 );
 
-    SetForegroundWindow(hwnd);
-    check_wnd_state(hwnd, hwnd, hwnd, 0);
-    ok(SetWindowLongA(hwnd2, GWL_STYLE, style & (~WS_POPUP)), "SetWindowLong failed\n");
-    ok(!SetForegroundWindow(hwnd2), "SetForegroundWindow failed\n");
-    check_wnd_state(hwnd, hwnd, hwnd, 0);
+    SetForegroundWindow( hwnd );
+    check_wnd_state( hwnd, hwnd, hwnd, 0 );
+    ok( SetWindowLongA( hwnd2, GWL_STYLE, style & (~WS_POPUP) ), "SetWindowLong failed\n" );
+    ok( !SetForegroundWindow( hwnd2 ), "SetForegroundWindow failed\n" );
+    check_wnd_state( hwnd, hwnd, hwnd, 0 );
 
-    SetEvent(thread_params.test_finished);
-    WaitForSingleObject(thread, INFINITE);
-    CloseHandle(thread_params.test_finished);
-    CloseHandle(thread_params.window_created);
-    CloseHandle(thread);
-    DestroyWindow(hwnd2);
+    res = SetEvent( test_sfw_start );
+    ok( res, "SetEvent failed, last error %#x.\n", GetLastError() );
+
+    /* now run the tests */
+
+    for (i = 0; i < ARRAY_SIZE(test_sfw_tests); ++i)
+    {
+        struct test_sfw_test_desc *test = test_sfw_tests + i;
+
+        while (MsgWaitForMultipleObjects( 1, &test_sfw_ready, FALSE, INFINITE, QS_SENDMESSAGE ) != WAIT_OBJECT_0)
+        {
+            while (PeekMessageA( &msg, 0, 0, 0, PM_REMOVE | PM_QS_SENDMESSAGE ))
+                DispatchMessageA( &msg );
+        }
+
+        ret = ResetEvent( test_sfw_ready );
+        ok( ret, "ResetEvent failed, last error %#x.\n", GetLastError() );
+
+        if (test->steal_foreground) SetForegroundWindow( hwnd );
+        SetForegroundWindow( test_sfw_window );
+        if (test->steal_foreground) SetForegroundWindow( hwnd );
+        SendNotifyMessageW( test_sfw_window, WM_USER, 0, 0 );
+
+        res = SetEvent( test_sfw_start );
+        ok( res, "SetEvent failed, last error %#x.\n", GetLastError() );
+
+        ret = SetEvent( test_sfw_done );
+        ok( res, "SetEvent failed, last error %#x.\n", GetLastError() );
+    }
+
+    WaitForSingleObject( thread, INFINITE );
+    CloseHandle( thread );
+
+    CloseHandle( test_sfw_start );
+    CloseHandle( test_sfw_done );
+    CloseHandle( test_sfw_ready );
+
+    DestroyWindow( hwnd2 );
+    flush_events( TRUE );
 }
 
 static WNDPROC old_button_proc;
@@ -12040,7 +12226,7 @@ START_TEST(win)
     test_Expose();
     test_layered_window();
 
-    test_SetForegroundWindow(hwndMain);
+    test_SetForegroundWindow( hwndMain );
     test_handles( hwndMain );
     test_winregion();
     test_map_points();
-- 
2.28.0




More information about the wine-devel mailing list