[PATCH v2 resend] user32: Do not call SendMessage() to show a window that is already visible.
Based on a patch made by Kimmo Myllyvirta.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=39731 Signed-off-by: Alexey Prokhin alexey@prokhin.ru --- v2: handle all ShowWindow commands, add a better test case. --- dlls/user32/tests/msg.c | 116 ++++++++++++++++++++++++++++++---------- dlls/user32/winpos.c | 64 +++++++++++++++++++--- 2 files changed, 146 insertions(+), 34 deletions(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index a5bc1c5518..8aa00d5ba0 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -5041,16 +5041,6 @@ static void test_WM_DEVICECHANGE(HWND hwnd) } }
-static DWORD CALLBACK show_window_thread(LPVOID arg) -{ - HWND hwnd = arg; - - /* function will not return if ShowWindow(SW_HIDE) calls SendMessage() */ - ok(ShowWindow(hwnd, SW_HIDE) == FALSE, "ShowWindow(SW_HIDE) expected FALSE\n"); - - return 0; -} - /* Helper function to easier test SetWindowPos messages */ #define test_msg_setpos( expected_list, flags, todo ) \ test_msg_setpos_( (expected_list), (flags), (todo), __FILE__, __LINE__) @@ -5071,8 +5061,6 @@ static void test_msg_setpos_(const struct message *expected_list, UINT flags, BO /* test if we receive the right sequence of messages */ static void test_messages(void) { - DWORD tid; - HANDLE hthread; HWND hwnd, hparent, hchild; HWND hchild2, hbutton; HMENU hmenu; @@ -5111,14 +5099,6 @@ static void test_messages(void) flush_events(); ok_sequence(WmEmptySeq, "ShowWindow(SW_HIDE):overlapped", FALSE);
- /* test ShowWindow(SW_HIDE) on a hidden window - multi-threaded */ - hthread = CreateThread(NULL, 0, show_window_thread, hwnd, 0, &tid); - ok(hthread != NULL, "CreateThread failed, error %d\n", GetLastError()); - ok(WaitForSingleObject(hthread, INFINITE) == WAIT_OBJECT_0, "WaitForSingleObject failed\n"); - CloseHandle(hthread); - flush_events(); - ok_sequence(WmEmptySeq, "ShowWindow(SW_HIDE):overlapped", FALSE); - ShowWindow(hwnd, SW_SHOW); flush_events(); ok_sequence(WmShowOverlappedSeq, "ShowWindow(SW_SHOW):overlapped", TRUE); @@ -13180,6 +13160,14 @@ static const struct message WmShowMaximized_3[] = { { 0 } };
+static const char * const sw_cmd_name[13] = +{ + "SW_HIDE", "SW_SHOWNORMAL", "SW_SHOWMINIMIZED", "SW_SHOWMAXIMIZED", + "SW_SHOWNOACTIVATE", "SW_SHOW", "SW_MINIMIZE", "SW_SHOWMINNOACTIVE", + "SW_SHOWNA", "SW_RESTORE", "SW_SHOWDEFAULT", "SW_FORCEMINIMIZE", + "SW_NORMALNA" /* 0xCC */ +}; + static void test_ShowWindow(void) { /* ShowWindow commands in random order */ @@ -13366,13 +13354,6 @@ static void test_ShowWindow(void)
for (i = 0; i < ARRAY_SIZE(sw); i++) { - static const char * const sw_cmd_name[13] = - { - "SW_HIDE", "SW_SHOWNORMAL", "SW_SHOWMINIMIZED", "SW_SHOWMAXIMIZED", - "SW_SHOWNOACTIVATE", "SW_SHOW", "SW_MINIMIZE", "SW_SHOWMINNOACTIVE", - "SW_SHOWNA", "SW_RESTORE", "SW_SHOWDEFAULT", "SW_FORCEMINIMIZE", - "SW_NORMALNA" /* 0xCC */ - }; char comment[64]; INT idx; /* index into the above array of names */
@@ -13421,6 +13402,86 @@ if (0) /* FIXME: Wine behaves completely different here */ flush_events(); }
+struct show_window_thread_data +{ + const char *comment; + HWND hwnd; + int cmd; + BOOL ret; +}; + +static DWORD CALLBACK show_window_thread(LPVOID arg) +{ + struct show_window_thread_data *d = arg; + BOOL ret; + + /* function will not return if ShowWindow() calls SendMessage() */ + ret = ShowWindow(d->hwnd, d->cmd); + ok(!ret == !d->ret, "%s: expected ret %u, got %u\n", d->comment, d->ret, ret); + + return 0; +} + +static void test_ShowWindow_multithreaded(void) +{ + static const struct + { + INT initial_cmd; /* ShowWindow command to apply before the test */ + INT cmd; /* ShowWindow command to apply in a thread */ + BOOL ret; /* ShowWindow return value */ + } sw[] = { + { SW_HIDE, SW_HIDE, FALSE }, + { SW_RESTORE, SW_SHOW, TRUE }, + { SW_RESTORE, SW_SHOWNOACTIVATE, TRUE }, + { SW_RESTORE, SW_RESTORE, TRUE }, + { SW_RESTORE, SW_SHOWNORMAL, TRUE }, + { SW_RESTORE, SW_SHOWDEFAULT, TRUE }, + { SW_MINIMIZE, SW_SHOW, TRUE }, + { SW_MINIMIZE, SW_SHOWMINNOACTIVE, TRUE }, + { SW_MINIMIZE, SW_MINIMIZE, TRUE }, + { SW_MINIMIZE, SW_SHOWMINIMIZED, TRUE }, + { SW_MINIMIZE, SW_FORCEMINIMIZE, TRUE }, + { SW_SHOWMAXIMIZED, SW_SHOW, TRUE }, + { SW_SHOWMAXIMIZED, SW_SHOWMAXIMIZED, TRUE }, + }; + + DWORD tid; + HANDLE hthread; + HWND hwnd; + int i; + struct show_window_thread_data swtd; + + hwnd = CreateWindowExA(0, "ShowWindowClass", NULL, WS_OVERLAPPEDWINDOW, + 120, 120, 90, 90, + 0, 0, 0, NULL); + assert(hwnd); + + for (i = 0; i < ARRAY_SIZE(sw); ++i) + { + char comment[64]; + sprintf(comment, "%d: ShowWindow(%s)", i+1, sw_cmd_name[sw[i].cmd]); + + ShowWindow(hwnd, sw[i].initial_cmd); + flush_events(); + flush_sequence(); + + swtd.comment = comment; + swtd.hwnd = hwnd; + swtd.cmd = sw[i].cmd; + swtd.ret = sw[i].ret; + hthread = CreateThread(NULL, 0, show_window_thread, &swtd, 0, &tid); + ok(hthread != NULL, "%d: CreateThread failed, error %d\n", i, GetLastError()); + ok(WaitForSingleObject(hthread, 3000) == WAIT_OBJECT_0, "%d: WaitForSingleObject failed\n", i); + CloseHandle(hthread); + + flush_events(); + ok_sequence(WmOptionalPaint, comment, FALSE); + } + + DestroyWindow(hwnd); + flush_events(); +} + static INT_PTR WINAPI test_dlg_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) { struct recvd_message msg; @@ -17748,6 +17809,7 @@ START_TEST(msg) test_PostMessage(); test_broadcast(); test_ShowWindow(); + test_ShowWindow_multithreaded(); test_PeekMessage(); test_PeekMessage2(); test_PeekMessage3(); diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c index 5837c179e4..96f86c4aeb 100644 --- a/dlls/user32/winpos.c +++ b/dlls/user32/winpos.c @@ -1032,6 +1032,51 @@ UINT WINPOS_MinMaximize( HWND hwnd, UINT cmd, LPRECT rect ) return swpFlags; }
+/*********************************************************************** + * is_window_state_changed + * + * A helper function for ShowWindow and ShowWindowAsync for checking if + * a window already has the requested style. + */ +static BOOL is_window_state_changed( LONG style, INT cmd ) +{ + BOOL is_visible = (style & WS_VISIBLE) != 0; + + switch (cmd) { + case SW_HIDE: + if (!is_visible) + return FALSE; + break; + case SW_SHOW: + if (is_visible) + return FALSE; + break; + case SW_SHOWMINNOACTIVE: + case SW_MINIMIZE: + case SW_FORCEMINIMIZE: + case SW_SHOWMINIMIZED: + if (is_visible && (style & WS_MINIMIZE)) + return FALSE; + break; + case SW_SHOWMAXIMIZED: /* same as SW_MAXIMIZE */ + if (is_visible && (style & WS_MAXIMIZE)) + return FALSE; + break; + case SW_SHOWNA: + break; + case SW_SHOWNOACTIVATE: + case SW_RESTORE: + case SW_SHOWNORMAL: /* same as SW_NORMAL: */ + case SW_SHOWDEFAULT: + if (is_visible && !(style & (WS_MINIMIZE | WS_MAXIMIZE))) + return FALSE; + break; + default: + return FALSE; + } + + return TRUE; +}
/*********************************************************************** * show_window @@ -1056,7 +1101,6 @@ static BOOL show_window( HWND hwnd, INT cmd ) switch(cmd) { case SW_HIDE: - if (!wasVisible) goto done; showFlag = FALSE; swp |= SWP_HIDEWINDOW | SWP_NOSIZE | SWP_NOMOVE; if (style & WS_CHILD) swp |= SWP_NOACTIVATE | SWP_NOZORDER; @@ -1070,14 +1114,12 @@ static BOOL show_window( HWND hwnd, INT cmd ) case SW_SHOWMINIMIZED: swp |= SWP_SHOWWINDOW | SWP_FRAMECHANGED; swp |= WINPOS_MinMaximize( hwnd, cmd, &newPos ); - if ((style & WS_MINIMIZE) && wasVisible) goto done; break;
case SW_SHOWMAXIMIZED: /* same as SW_MAXIMIZE */ if (!wasVisible) swp |= SWP_SHOWWINDOW; swp |= SWP_FRAMECHANGED; swp |= WINPOS_MinMaximize( hwnd, SW_MAXIMIZE, &newPos ); - if ((style & WS_MAXIMIZE) && wasVisible) goto done; break;
case SW_SHOWNA: @@ -1085,7 +1127,6 @@ static BOOL show_window( HWND hwnd, INT cmd ) if (style & WS_CHILD) swp |= SWP_NOZORDER; break; case SW_SHOW: - if (wasVisible) goto done; swp |= SWP_SHOWWINDOW | SWP_NOSIZE | SWP_NOMOVE; if (style & WS_CHILD) swp |= SWP_NOACTIVATE | SWP_NOZORDER; break; @@ -1105,7 +1146,6 @@ static BOOL show_window( HWND hwnd, INT cmd ) } else { - if (wasVisible) goto done; swp |= SWP_NOSIZE | SWP_NOMOVE; } if (style & WS_CHILD && !(swp & SWP_STATECHANGED)) swp |= SWP_NOACTIVATE | SWP_NOZORDER; @@ -1114,6 +1154,9 @@ static BOOL show_window( HWND hwnd, INT cmd ) goto done; }
+ if (!is_window_state_changed( style, cmd )) + goto done; + if ((showFlag != wasVisible || cmd == SW_SHOWNA) && cmd != SW_SHOWMAXIMIZED && !(swp & SWP_STATECHANGED)) { SendMessageW( hwnd, WM_SHOWWINDOW, showFlag, 0 ); @@ -1199,6 +1242,7 @@ done: BOOL WINAPI ShowWindowAsync( HWND hwnd, INT cmd ) { HWND full_handle; + LONG style;
if (is_broadcast(hwnd)) { @@ -1209,6 +1253,10 @@ BOOL WINAPI ShowWindowAsync( HWND hwnd, INT cmd ) if ((full_handle = WIN_IsCurrentThread( hwnd ))) return show_window( full_handle, cmd );
+ style = GetWindowLongW( hwnd, GWL_STYLE ); + if (!is_window_state_changed( style, cmd )) + return (style & WS_VISIBLE) != 0; + return SendNotifyMessageW( hwnd, WM_WINE_SHOWWINDOW, cmd, 0 ); }
@@ -1219,6 +1267,7 @@ BOOL WINAPI ShowWindowAsync( HWND hwnd, INT cmd ) BOOL WINAPI ShowWindow( HWND hwnd, INT cmd ) { HWND full_handle; + LONG style;
if (is_broadcast(hwnd)) { @@ -1228,8 +1277,9 @@ BOOL WINAPI ShowWindow( HWND hwnd, INT cmd ) if ((full_handle = WIN_IsCurrentThread( hwnd ))) return show_window( full_handle, cmd );
- if ((cmd == SW_HIDE) && !(GetWindowLongW( hwnd, GWL_STYLE ) & WS_VISIBLE)) - return FALSE; + style = GetWindowLongW( hwnd, GWL_STYLE ); + if (!is_window_state_changed( style, cmd )) + return (style & WS_VISIBLE) != 0;
return SendMessageW( hwnd, WM_WINE_SHOWWINDOW, cmd, 0 ); }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=55230
Your paranoid android.
=== wvistau64_zh_CN (32 bit report) ===
user32: msg.c:16356: Test failed: SetParent() visible WS_POPUP: 26: the msg sequence is not complete: expected 0000 - actual 0088 msg.c:5119: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088
=== wvistau64_he (32 bit report) ===
user32: msg.c:16228: Test failed: 0: WaitForSingleObject failed
=== w1064v1809 (32 bit report) ===
user32: msg.c:9314: Test failed: Shift+MouseButton press/release: 7: the msg 0x0202 was expected, but got msg 0x0007 instead msg.c:9314: Test failed: Shift+MouseButton press/release: 8: the msg 0x0202 was expected, but got msg 0x0101 instead msg.c:9314: Test failed: Shift+MouseButton press/release: 10: the msg 0x0101 should have been sent msg.c:9314: Test failed: Shift+MouseButton press/release: 11: the msg 0x0101 was expected, but got msg 0x0202 instead msg.c:9314: Test failed: Shift+MouseButton press/release: 12: the msg sequence is not complete: expected 0000 - actual 0202 msg.c:10257: Test failed: did not get expected count for minimum timeout (50 != ~100).
=== w7pro64 (64 bit report) ===
user32: msg.c:13370: Test failed: 54: ShowWindow(SW_MINIMIZE): 4: in msg 0x0047 expecting wParam 0x8170 got 0x1803 msg.c:13370: Test failed: 54: ShowWindow(SW_MINIMIZE): 5: the msg 0x0003 was expected, but got msg 0x0047 instead msg.c:13370: Test failed: 54: ShowWindow(SW_MINIMIZE): 6: the msg 0x0005 was expected, but got msg 0x0003 instead msg.c:13370: Test failed: 54: ShowWindow(SW_MINIMIZE): 7: the msg sequence is not complete: expected 0000 - actual 0005 msg.c:4292: Test failed: WM_MDICREATE for maximized visible MDI child window: 56: in msg 0x0047 expecting wParam 0x1012 got 0x101a msg.c:4292: Test failed: WM_MDICREATE for maximized visible MDI child window: 61: in msg 0x0047 expecting wParam 0x1012 got 0x101a msg.c:4309: Test failed: Destroy visible maximized MDI child window: 2: the msg 0x0046 was expected, but got msg 0x0009 instead msg.c:4342: Test failed: Maximize MDI child window with invisible parent: 31: in msg 0x0047 expecting wParam 0x33 got 0x3b
Report errors: user32:msg prints too much data (57759 bytes)
=== debian10 (32 bit report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35221 bytes)
=== debian10 (32 bit French report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35222 bytes)
=== debian10 (32 bit Japanese:Japan report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35222 bytes)
=== debian10 (32 bit Chinese:China report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35221 bytes)
=== debian10 (32 bit WoW report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35221 bytes)
=== debian10 (64 bit WoW report) ===
user32: msg.c:5125: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds
Report errors: user32:msg prints too much data (35223 bytes)
-
Alexey Prokhin -
Marvin