[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(a)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 ); } -- 2.20.1 (Apple Git-117)
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)
participants (2)
-
Alexey Prokhin -
Marvin