[PATCH v3 0/1] MR9578: win32u: Ignore statup cmd show mode for owned windows.

[Paul Gofman (＠gofman)]

Fixes a regression introduced by 8d7de32cd646f3d1f118836e643e6146c9837278.

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

--
  v3: win32u: Ignore statup cmd show mode for owned windows.

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

[PATCH v3 1/1] win32u: Ignore statup cmd show mode for owned windows.

[Paul Gofman]

From: Paul Gofman

Fixes a regression introduced by 8d7de32cd646f3d1f118836e643e6146c9837278.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59034
---
 dlls/user32/tests/win.c | 36 ++++++++++++++++++++++--------------
 dlls/win32u/window.c    |  1 +
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index 354d6ed5ca3..6073e087cb4 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -13677,20 +13677,27 @@ static const struct test_startupinfo_showwindow_test test_startupinfo_showwindow
 static void test_startupinfo_showwindow_proc( int test_id )
 {
     const struct test_startupinfo_showwindow_test *test = &test_startupinfo_showwindow_tests[test_id];
-    static const DWORD ignored_window_styles[] =
-    {
-        WS_CHILD,
-        WS_POPUP, /* WS_POPUP windows are not ignored when used with WS_CAPTION (which is WS_BORDER | WS_DLGFRAME) */
-        WS_CHILD | WS_POPUP,
-        WS_POPUP | WS_BORDER,
-        WS_POPUP | WS_DLGFRAME,
-        WS_POPUP | WS_SYSMENU | WS_THICKFRAME| WS_MINIMIZEBOX | WS_MAXIMIZEBOX,
+    static const struct
+    {
+        DWORD style;
+        BOOL parent;
+    }
+    ignored_window_styles[] =
+    {
+        { WS_CHILD, TRUE },
+        { WS_POPUP }, /* Unowned WS_POPUP windows are not ignored when used with WS_CAPTION (which is WS_BORDER | WS_DLGFRAME) */
+        { WS_CHILD | WS_POPUP, TRUE },
+        { WS_POPUP | WS_BORDER },
+        { WS_POPUP | WS_DLGFRAME },
+        { WS_POPUP | WS_SYSMENU | WS_THICKFRAME| WS_MINIMIZEBOX | WS_MAXIMIZEBOX },
+        { WS_OVERLAPPED, TRUE }, /* owned window */
+        { WS_POPUP | WS_CAPTION, TRUE }, /* owned window */
     };
     BOOL bval, expected;
     STARTUPINFOW sa;
     unsigned int i;
     DWORD style;
-    HWND hwnd;
+    HWND parent, hwnd;

GetStartupInfoW( &sa );

@@ -13705,26 +13712,27 @@ static void test_startupinfo_showwindow_proc( int test_id )
      * SW_ variants for ShowWindow() which are not altered by startup info still consume startup info usage so can
      * only be tested once per process. */

-    hwnd = CreateWindowA( "static", "parent", WS_OVERLAPPED, 0, 0, 0, 0, NULL, NULL,
+    parent = CreateWindowA( "static", "parent", WS_OVERLAPPED, 0, 0, 0, 0, NULL, NULL,
                            GetModuleHandleW( NULL ), NULL );
     pump_messages();
     for (i = 0; i < ARRAY_SIZE(ignored_window_styles); ++i)
     {
         winetest_push_context( "%u", i );
-        hwnd = CreateWindowA( "static", "overlapped", ignored_window_styles[i], 0, 0, 0, 0,
-                               ignored_window_styles[i] & WS_CHILD ? hwnd : NULL, NULL,
+        hwnd = CreateWindowA( "static", "overlapped", ignored_window_styles[i].style, 0, 0, 0, 0,
+                               ignored_window_styles[i].parent ? parent : NULL, NULL,
                                GetModuleHandleW( NULL ), NULL );
         ok( !!hwnd, "got NULL.\n" );
         ShowWindow( hwnd, SW_SHOW );
         bval = IsWindowVisible( hwnd );
-        if ((ignored_window_styles[i] & (WS_CHILD | WS_POPUP)) == WS_CHILD)
+        if ((ignored_window_styles[i].style & (WS_CHILD | WS_POPUP)) == WS_CHILD)
             ok( !bval, "unexpectedly visible.\n" );
         else
             ok( bval, "unexpectedly invisible.\n" );
         pump_messages();
+        DestroyWindow( hwnd );
         winetest_pop_context();
     }
-    DestroyWindow( hwnd );
+    DestroyWindow( parent );
     pump_messages();

style = test->style;
diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c
index 340ab31e857..083a1d1aa25 100644
--- a/dlls/win32u/window.c
+++ b/dlls/win32u/window.c
@@ -4739,6 +4739,7 @@ static BOOL show_window( HWND hwnd, INT cmd )

if ((!(style & (WS_POPUP | WS_CHILD))
          || ((style & (WS_POPUP | WS_CHILD | WS_CAPTION)) == (WS_POPUP | WS_CAPTION)))
+         && first_window && !get_window_relative( hwnd, GW_OWNER )
         && InterlockedExchange( &first_window, 0 ))
     {
         RTL_USER_PROCESS_PARAMETERS *params = NtCurrentTeb()->Peb->ProcessParameters;

[Paul Gofman (＠gofman)]

v3:
 - add test for (ignored) owned (WS_POPUP | WS_CAPTION) window;
 - stop leaking ignored windows in test.

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/win32u/window.c:
>  if ((!(style & (WS_POPUP | WS_CHILD))
>       || ((style & (WS_POPUP | WS_CHILD | WS_CAPTION)) == (WS_POPUP | WS_CAPTION)))
> -
>     && first_window && !get_window_relative( hwnd, GW_OWNER )
>    && InterlockedExchange( &first_window, 0 ))

I think first_window should be read with ReadNoFence at least, at it's written through atomic ops.

[Paul Gofman (＠gofman)]

On Tue Nov 25 10:52:53 2025 +0000, Rémi Bernon wrote:
> I think first_window should be read with ReadNoFence at least, at it's
> written through atomic ops.

Isn't it a tiny bit nicer this way? The check is redundant. In the very rare potential case when it will read some stale data it will do `get_window_relative() call but will remain functionally correct with InterlockedExchange( &first_window, 0 ). But the vast majority of cases will go without fenced memory read.

[Rémi Bernon (＠rbernon)]

On Tue Nov 25 15:41:21 2025 +0000, Paul Gofman wrote:
> Isn't it a tiny bit nicer this way? The check is redundant. In the very
> rare potential case when it will read some stale data it will do
> `get_window_relative() call but will remain functionally correct with
> InterlockedExchange( &first_window, 0 ). But the vast majority of cases
> will go without fenced memory read.

I don't think a fence here would have any cost? I would even argue that you could remove the check entirely and call `get_window_relative` unconditionally. A server call here is likely not an issue and we could probably ultimately get rid of it through window shared memory once that's implemented.

[Paul Gofman (＠gofman)]

On Tue Nov 25 15:50:23 2025 +0000, Rémi Bernon wrote:
> I don't think a fence here would have any cost? I would even argue that
> you could remove the check entirely and call `get_window_relative`
> unconditionally. A server call here is likely not an issue and we could
> probably ultimately get rid of it through window shared memory once
> that's implemented.

Ok, thanks, I will remove that pre-check entirely. Meanwhile the bug reporter says that with this change, while initial issue is fixed, the probable minimization happens later with an exit message box. While it is unlikely that this exact change is wrong and more likely it is is missing some extra conditions or hits entirely unrelated aspects, I will look into that before resending.