[PATCH 0/2] MR4493: user32/tests: Add a simpler test for SetParent's internal message.

[Esme Povirk (＠madewokherd)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54037

test_SendMessage_other_thread(2), which is supposed to show that
Windows doesn't use an internal sent message for SetParent, tacks
this onto another more complex test, which introduces a race
condition (see Comment 2 on the bug). I am not sure if it can be
fixed in the existing function, perhaps it can and I just can't
keep track of all the moving parts well enough, but I see no need
to make it so difficult.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4493

[PATCH 1/2] user32/tests: Add a simpler test for SetParent's internal message.

[Esme Povirk]

From: Esme Povirk <esme(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54037

test_SendMessage_other_thread(2), which is supposed to show that
Windows doesn't use an internal sent message for SetParent, tacks
this onto another more complex test, which introduces a race
condition (see Comment 2 on the bug). I am not sure if it can be
fixed in the existing function, perhaps it can and I just can't
keep track of all the moving parts well enough, but I see no need
to make it so difficult.
---
 dlls/user32/tests/msg.c | 50 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index a9b9a009576..9098c7bf926 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -19856,6 +19856,55 @@ static void test_SendMessage_other_thread(int thread_n)
     CloseHandle(wnd_event.getmessage_complete);
 }
 
+static DWORD WINAPI SetParent_thread(void *param)
+{
+    struct wnd_event *wnd_event = param;
+
+    if (winetest_debug > 1) trace("thread: started\n");
+    SetEvent(wnd_event->start_event);
+
+    /* this leads to sending an internal message under Wine */
+    if (winetest_debug > 1) trace("thread: call SetParent\n");
+    SetParent(wnd_event->hwnd, wnd_event->hwnd);
+
+    return 0;
+}
+
+static void test_setparent_status(void)
+{
+    HANDLE hthread;
+    struct wnd_event wnd_event;
+    DWORD ret;
+
+    wnd_event.start_event = CreateEventA(NULL, 0, 0, NULL);
+
+    wnd_event.hwnd = CreateWindowExA(0, "TestWindowClass", NULL, WS_OVERLAPPEDWINDOW,
+                           100, 100, 200, 200, 0, 0, 0, NULL);
+    ok(wnd_event.hwnd != 0, "CreateWindowEx failed\n");
+
+    ret = GetQueueStatus(QS_SENDMESSAGE);
+    ok(ret == 0, "wrong status %08lx\n", ret);
+
+    hthread = CreateThread(NULL, 0, SetParent_thread, &wnd_event, 0, NULL);
+    ok(hthread != NULL, "CreateThread failed, error %ld\n", GetLastError());
+
+    WaitForSingleObject(wnd_event.start_event, INFINITE);
+
+    /* background thread's SetParent should complete allowing the thread to exit */
+    ret = MsgWaitForMultipleObjects(1, &hthread, FALSE, 1000, QS_SENDMESSAGE);
+    todo_wine ok(ret == WAIT_OBJECT_0, "MsgWaitForMultipleObjects returned %08lx\n", ret);
+
+    /* QS_SENDMESSAGE status should not have been set by SetParent */
+    ret = GetQueueStatus(QS_SENDMESSAGE);
+    todo_wine ok(ret == 0, "wrong status %08lx\n", ret);
+
+    DestroyWindow(wnd_event.hwnd);
+
+    WaitForSingleObject(hthread, INFINITE);
+    CloseHandle(hthread);
+    CloseHandle(wnd_event.start_event);
+}
+
 static LRESULT CALLBACK insendmessage_wnd_proc( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp )
 {
     DWORD flags = InSendMessageEx( NULL );
@@ -20248,6 +20297,7 @@ START_TEST(msg)
     test_winevents();
     test_SendMessage_other_thread(1);
     test_SendMessage_other_thread(2);
+    test_setparent_status();
     test_InSendMessage();
     test_SetFocus();
     test_SetParent();
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/4493

[PATCH 2/2] user32/tests: Remove test_SendMessage_other_thread(2).

[Esme Povirk]

From: Esme Povirk <esme(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54037
---
 dlls/user32/tests/msg.c | 48 ++++++-----------------------------------
 1 file changed, 6 insertions(+), 42 deletions(-)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index 9098c7bf926..1b728002858 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -19732,37 +19732,7 @@ static DWORD WINAPI SendMessage_thread_1(void *param)
     return 0;
 }
 
-static DWORD WINAPI SendMessage_thread_2(void *param)
-{
-    struct wnd_event *wnd_event = param;
-    DWORD ret;
-
-    if (winetest_debug > 1) trace("thread: starting\n");
-    WaitForSingleObject(wnd_event->start_event, INFINITE);
-
-    if (winetest_debug > 1) trace("thread: call PostMessage\n");
-    PostMessageA(wnd_event->hwnd, WM_USER, 0, 0);
-
-    if (winetest_debug > 1) trace("thread: call PostMessage\n");
-    PostMessageA(wnd_event->hwnd, WM_USER+1, 0, 0);
-
-    /* this leads to sending an internal message under Wine */
-    if (winetest_debug > 1) trace("thread: call SetParent\n");
-    SetParent(wnd_event->hwnd, wnd_event->hwnd);
-
-    if (winetest_debug > 1) trace("thread: call SendMessage\n");
-    SendMessageA(wnd_event->hwnd, WM_USER+2, 0, 0);
-    SetEvent(wnd_event->stop_event);
-    ret = WaitForSingleObject(wnd_event->getmessage_complete, 100);
-    ok(ret == WAIT_OBJECT_0, "WaitForSingleObject failed, ret:%lx\n", ret);
-
-    if (winetest_debug > 1) trace("thread: call SendMessage\n");
-    SendMessageA(wnd_event->hwnd, WM_USER+3, 0, 0);
-
-    return 0;
-}
-
-static void test_SendMessage_other_thread(int thread_n)
+static void test_SendMessage_other_thread(void)
 {
     DWORD qs_all_input = QS_ALLINPUT & ~QS_RAWINPUT;
     HANDLE hthread;
@@ -19770,8 +19740,6 @@ static void test_SendMessage_other_thread(int thread_n)
     DWORD tid, ret;
     MSG msg;
 
-    winetest_push_context("thread_%i", thread_n);
-
     wnd_event.start_event = CreateEventA(NULL, 0, 0, NULL);
     wnd_event.stop_event = CreateEventA(NULL, 0, 0, NULL);
     wnd_event.getmessage_complete = CreateEventA(NULL, 0, 0, NULL);
@@ -19780,7 +19748,7 @@ static void test_SendMessage_other_thread(int thread_n)
                                      100, 100, 200, 200, 0, 0, 0, NULL);
     ok(wnd_event.hwnd != 0, "CreateWindowEx failed\n");
 
-    hthread = CreateThread(NULL, 0, thread_n == 1 ? SendMessage_thread_1 : SendMessage_thread_2, &wnd_event, 0, &tid);
+    hthread = CreateThread(NULL, 0, SendMessage_thread_1, &wnd_event, 0, &tid);
     ok(hthread != NULL, "CreateThread failed, error %ld\n", GetLastError());
     CloseHandle(hthread);
 
@@ -19807,12 +19775,11 @@ static void test_SendMessage_other_thread(int thread_n)
     GetMessageA(&msg, 0, 0, 0);
     ok(msg.message == WM_USER, "expected WM_USER, got %04x\n", msg.message);
     DispatchMessageA(&msg);
-    ok_sequence(send_message_1, "SendMessage from other thread 1", thread_n == 2);
+    ok_sequence(send_message_1, "SendMessage from other thread 1", FALSE);
 
     SetEvent(wnd_event.getmessage_complete);
 
     ret = WaitForSingleObject(wnd_event.stop_event, 100);
-    todo_wine_if (thread_n == 2)
     ok(ret == WAIT_OBJECT_0, "WaitForSingleObject failed, ret:%lx\n", ret);
 
     /* intentionally yield */
@@ -19829,7 +19796,7 @@ static void test_SendMessage_other_thread(int thread_n)
     ok(PeekMessageA(&msg, 0, 0, 0, PM_REMOVE), "PeekMessage should not fail\n");
     ok(msg.message == WM_USER+1, "expected WM_USER+1, got %04x\n", msg.message);
     DispatchMessageA(&msg);
-    ok_sequence(send_message_3, "SendMessage from other thread 3", thread_n == 2);
+    ok_sequence(send_message_3, "SendMessage from other thread 3", FALSE);
 
     /* intentionally yield */
     MsgWaitForMultipleObjects(0, NULL, FALSE, 100, qs_all_input);
@@ -19838,7 +19805,7 @@ static void test_SendMessage_other_thread(int thread_n)
     while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) {
         ok(ignore_message(msg.message), "got unexpected message %04x from PeekMessageA\n", msg.message);
     }
-    ok_sequence(send_message_5, "SendMessage from other thread 5", thread_n == 2);
+    ok_sequence(send_message_5, "SendMessage from other thread 5", FALSE);
 
     ret = GetQueueStatus(QS_SENDMESSAGE|QS_POSTMESSAGE);
     ok(ret == 0, "wrong status %08lx\n", ret);
@@ -19849,8 +19816,6 @@ static void test_SendMessage_other_thread(int thread_n)
     flush_events();
     flush_sequence();
 
-    winetest_pop_context();
-
     CloseHandle(wnd_event.start_event);
     CloseHandle(wnd_event.stop_event);
     CloseHandle(wnd_event.getmessage_complete);
@@ -20295,8 +20260,7 @@ START_TEST(msg)
     if (!hCBT_hook) win_skip( "cannot set global hook, will skip hook tests\n" );
 
     test_winevents();
-    test_SendMessage_other_thread(1);
-    test_SendMessage_other_thread(2);
+    test_SendMessage_other_thread();
     test_setparent_status();
     test_InSendMessage();
     test_SetFocus();
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4493

[PATCH 2/2] user32/tests: Remove test_SendMessage_other_thread(2).

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=140306

Your paranoid android.


=== build (build log) ===

error: patch failed: dlls/user32/tests/msg.c:20295
Task: Patch failed to apply

=== debian11 (build log) ===

error: patch failed: dlls/user32/tests/msg.c:20295
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/user32/tests/msg.c:20295
Task: Patch failed to apply