[PATCH v3 0/2] MR8702: user32/tests: Add more SendMessageCallbackA/W() tests with NULL callback.

[Zhiyi Zhang (＠zhiyi)]

For React Native.

--
  v3: win32u: Put the message in the queue when callback pointer is NULL and callback data is 1.
      user32/tests: Add more SendMessageCallbackA/W() tests with NULL callback.

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

[PATCH v3 1/2] user32/tests: Add more SendMessageCallbackA/W() tests with NULL callback.

[Zhiyi Zhang]

From: Zhiyi Zhang

The tests are mainly to show that SendMessageCallbackA/W() with NULL callback pointer and callback
data being 1 causes the message to be posted to the window, even when the window is created in the
same thread.
---
 dlls/user32/tests/msg.c | 59 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index 80d900baa1c..95fcb91cc01 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -16499,16 +16499,73 @@ static void test_EndDialog(void)
     UnregisterClassA(cls.lpszClassName, cls.hInstance);
 }

+static const struct message WmUserSeq[] =
+{
+    { WM_USER, sent },
+    { 0 }
+};
+
 static void test_nullCallback(void)
 {
+    DWORD status;
     HWND hwnd;
+    BOOL ret;

hwnd = CreateWindowExA(0, "TestWindowClass", "Test overlapped", WS_OVERLAPPEDWINDOW,
                            100, 100, 200, 200, 0, 0, 0, NULL);
     ok (hwnd != 0, "Failed to create overlapped window\n");

-    SendMessageCallbackA(hwnd,WM_NULL,0,0,NULL,0);
+    /* NULL callback and data being 0 with SendMessageCallbackA() */
+    flush_sequence();
+    ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 0);
+    ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback", FALSE);
+
+    /* NULL callback and data being 0 with SendMessageCallbackW() */
+    flush_sequence();
+    ret = SendMessageCallbackW(hwnd, WM_USER, 0, 0, NULL, 0);
+    ok(ret, "SendMessageCallbackW failed, error %ld.", GetLastError());
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback", FALSE);
+
+    /* NULL callback and data being 1 with SendMessageCallbackA(). The result suggests that the
+     * message is not directly sent to the window */
+    flush_sequence();
+    ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 1);
+    ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
+    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", TRUE);
+    flush_events();
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", TRUE);
+
+    /* NULL callback and data being 1 with SendMessageCallbackW(). The result suggests that the
+     * message is not directly sent to the window */
+    flush_sequence();
+    ret = SendMessageCallbackW(hwnd, WM_USER, 0, 0, NULL, 1);
+    ok(ret, "SendMessageCallbackW failed, error %ld.", GetLastError());
+    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", TRUE);
     flush_events();
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", TRUE);
+
+    /* NULL callback and data being 2 with SendMessageCallbackA() */
+    flush_sequence();
+    ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 2);
+    ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback", FALSE);
+
+    /* NULL callback and data being 2 with SendMessageCallbackW() */
+    flush_sequence();
+    ret = SendMessageCallbackW(hwnd, WM_USER, 0, 0, NULL, 2);
+    ok(ret, "SendMessageCallbackW failed, error %ld.", GetLastError());
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback", FALSE);
+
+    /* Check the queue status after SendMessageCallbackA() with NULL callback and data being 1 */
+    flush_events();
+    ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 1);
+    ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
+    status = GetQueueStatus(QS_ALLINPUT);
+    todo_wine
+    ok(HIWORD(status) & QS_SENDMESSAGE && LOWORD(status) & QS_SENDMESSAGE,
+       "Got unexpected status %#lx.\n", status);
+
     DestroyWindow(hwnd);
 }

[PATCH v3 2/2] win32u: Put the message in the queue when callback pointer is NULL and callback data is 1.

[Zhiyi Zhang]

From: Zhiyi Zhang

This is an undocumented behavior needed by React Native apps, based on Nikolay's findings.
---
 dlls/user32/tests/msg.c | 9 ++++-----
 dlls/win32u/message.c   | 4 +++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index 95fcb91cc01..c8a12a6f998 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -16532,18 +16532,18 @@ static void test_nullCallback(void)
     flush_sequence();
     ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 1);
     ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
-    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", TRUE);
+    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", FALSE);
     flush_events();
-    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", TRUE);
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", FALSE);

/* NULL callback and data being 1 with SendMessageCallbackW(). The result suggests that the
      * message is not directly sent to the window */
     flush_sequence();
     ret = SendMessageCallbackW(hwnd, WM_USER, 0, 0, NULL, 1);
     ok(ret, "SendMessageCallbackW failed, error %ld.", GetLastError());
-    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", TRUE);
+    ok_sequence(WmEmptySeq, "WM_USER with NULL callback", FALSE);
     flush_events();
-    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", TRUE);
+    ok_sequence(WmUserSeq, "WM_USER with NULL callback after flushing events", FALSE);

/* NULL callback and data being 2 with SendMessageCallbackA() */
     flush_sequence();
@@ -16562,7 +16562,6 @@ static void test_nullCallback(void)
     ret = SendMessageCallbackA(hwnd, WM_USER, 0, 0, NULL, 1);
     ok(ret, "SendMessageCallbackA failed, error %ld.", GetLastError());
     status = GetQueueStatus(QS_ALLINPUT);
-    todo_wine
     ok(HIWORD(status) & QS_SENDMESSAGE && LOWORD(status) & QS_SENDMESSAGE,
        "Got unexpected status %#lx.\n", status);

diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c
index 2a824dbdab3..e64e8213f93 100644
--- a/dlls/win32u/message.c
+++ b/dlls/win32u/message.c
@@ -4189,7 +4189,9 @@ static BOOL process_message( struct send_message_info *info, DWORD_PTR *res_ptr,
     thread_info->msg_source = msg_source_unavailable;
     spy_enter_message( SPY_SENDMESSAGE, info->hwnd, info->msg, info->wparam, info->lparam );

-    if (info->dest_tid != GetCurrentThreadId())
+    if (info->dest_tid != GetCurrentThreadId() ||
+        /* undocumented behavior when callback pointer is NULL and data is 1. See test_nullCallback(). */
+        (info->type == MSG_CALLBACK && !info->callback && info->data == 1))
     {
         if (dest_pid != GetCurrentProcessId() && (info->type == MSG_ASCII || info->type == MSG_UNICODE))
             info->type = MSG_OTHER_PROCESS;

[Zhiyi Zhang (＠zhiyi)]

On Mon Aug  4 09:14:13 2025 +0000, Dmitry Timoshkov wrote:
> There's no need to add another copy of SendMessageCallback() tests just
> for checking the queue status.

Okay, I've deleted it.

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 09:17:05 2025 +0000, Zhiyi Zhang wrote:
> Okay, I've deleted it.

I ment to add the queue checks in the tests you are adding. That would imply testing queue status in all cases, not just some selected ones.

[Zhiyi Zhang (＠zhiyi)]

On Mon Aug  4 09:23:48 2025 +0000, Dmitry Timoshkov wrote:
> I ment to add the queue checks in the tests you are adding. That would
> imply testing queue status in all cases, not just some selected ones.

Adding GetQueueStatus() after SendMessageCallbackA/W(hwnd, WM_USER, 0, 0, NULL, 1) has some strange behavior on Windows. For example, calling GetQueueStatus() after SendMessageCallbackA/W(hwnd, WM_USER, 0, 0, NULL, 1) can cause the WM_USER message to never arrive(not sent or posted). That's why it's added at the end of the tests.

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 09:38:48 2025 +0000, Zhiyi Zhang wrote:
> Adding GetQueueStatus() after SendMessageCallbackA/W(hwnd, WM_USER, 0,
> 0, NULL, 1) has some strange behavior on Windows. For example, calling
> GetQueueStatus() after SendMessageCallbackA/W(hwnd, WM_USER, 0, 0, NULL,
> - can cause the WM_USER message to never arrive(not sent or posted).
>
> That's why it's added at the end of the tests.

Doesn't that mean that the proposed solution is not fully correct? At least Windows behaviour would need some additional investigation I'd guess.

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 09:45:22 2025 +0000, Dmitry Timoshkov wrote:
> Doesn't that mean that the proposed solution is not fully correct? At
> least Windows behaviour would need some additional investigation I'd guess.

Does replacing WM_USER by WM_NULL change the observed behaviour under Windows?

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 09:52:06 2025 +0000, Dmitry Timoshkov wrote:
> Does replacing WM_USER by WM_NULL change the observed behaviour under Windows?

Also what actual message number the affected application uses?

[Zhiyi Zhang (＠zhiyi)]

> Also what actual message number the affected application uses?

0x60.

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 09:53:46 2025 +0000, Zhiyi Zhang wrote:
> > Also what actual message number the affected application uses?
>
> 0x60.

If you use 0x60 in the tests does it work more reliably under Windows?

[Zhiyi Zhang (＠zhiyi)]

> Does replacing WM_USER by WM_NULL change the observed behaviour under Windows?

No.
> Doesn't that mean that the proposed solution is not fully correct? At least Windows behaviour would need some additional investigation I'd guess.

Yeah, it's incomplete due to its undocumented nature. I don't see the value of figuring out why GetQueueStatus() causes the message to disappear. It's not a behavior that React Native apps need anyway.
> If you use 0x60 in the tests does it work more reliably under Windows?

No. It's still the same as WM_USER. React Native apps use 0x60 with both WPARAM and LPARAM being 5. Testing with the same message number and parameters doesn't change anything.

[Dmitry Timoshkov (＠dmitry)]

On Mon Aug  4 10:06:03 2025 +0000, Zhiyi Zhang wrote:
> > Does replacing WM_USER by WM_NULL change the observed behaviour under Windows?
>
> No.
> > Doesn't that mean that the proposed solution is not fully correct? At
>
> least Windows behaviour would need some additional investigation I'd guess.
> Yeah, it's incomplete due to its undocumented nature. I don't see the
> value of figuring out why GetQueueStatus() causes the message to
> disappear. It's not a behavior that React Native apps need anyway.
> > If you use 0x60 in the tests does it work more reliably under Windows?
>
> No. It's still the same as WM_USER. React Native apps use 0x60 with both
> WPARAM and LPARAM being 5. Testing with the same message number and
> parameters doesn't change anything.

I see. Thanks for testing and sharing the results.

[Zhiyi Zhang (＠zhiyi)]

On Mon Aug  4 10:14:29 2025 +0000, Dmitry Timoshkov wrote:
> I see. Thanks for testing and sharing the results.

Thanks for the review.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/win32u/message.c:
>  thread_info->msg_source = msg_source_unavailable;
>  spy_enter_message( SPY_SENDMESSAGE, info->hwnd, info->msg, info->wparam, info->lparam );
>
> - if (info->dest_tid != GetCurrentThreadId())
> - if (info->dest_tid != GetCurrentThreadId() ||
> -
>    /* undocumented behavior when callback pointer is NULL and data is 1. See test_nullCallback(). */

I would remove test reference from here, it's better to describe what happens instead - that it won't wait in such case. Otherwise seems fine.