Missing WM_CAPTURECHANGED message

[Christopher Thielen]

Hi wine-devel,

My investigation of https://bugs.winehq.org/show_bug.cgi?id=13683 has 
determined a missing WM_CAPTURECHANGED message is to blame.

At some point in early execution, a WM_CAPTURECHANGED message is sent by 
Windows (but not Wine) which apparently sets off a course of calls that 
prevents this bug.

I wrote some simple WM_CAPTURECHANGED tests but I have not found any 
meaningful differences in execution between Windows and Wine. Put 
another way, I've been unable to trigger u9.exe's specific behavior.

Does anyone have recommendations for debugging Win32 messaging, 
specifically in the case of messages "missing" in Wine? Since I can see 
the missing message being sent by Windows with Spy++, presumably I can 
look back through API calls made before that point and try to replicate 
a test case, but that debugging method sounds fairly lengthy. Is there a 
good tool to capture and replicate all Win32 API calls perhaps?

Any advice would be appreciated, thanks!

- Christopher Thielen

[Christopher Thielen]

I figured out this behavior. It seems if one calls SetCapture() twice it 
will cause Windows to send a WM_CAPTURECHANGED message to the same 
window given as the parameter of SetCapture(). Wine does not currently 
follow this behavior.

The following patch fixes the behavior, though I'm not sure if it's the 
right way to fix the issue as I don't know why the original author(s) 
made the removed check in the first place. Does anyone have any ideas?

(Generated against wine.git on 11/15/15 at 19:28 PST):

diff --git a/dlls/user32/input.c b/dlls/user32/input.c
index 40e35a9..63fae67 100644
--- a/dlls/user32/input.c
+++ b/dlls/user32/input.c
@@ -108,7 +108,7 @@ BOOL set_capture_window( HWND hwnd, UINT gui_flags, 
HWND *prev_ret )
      {
          USER_Driver->pSetCapture( hwnd, gui_flags );

-        if (previous && previous != hwnd)
+        if (previous)
              SendMessageW( previous, WM_CAPTURECHANGED, 0, (LPARAM)hwnd );

if (prev_ret) *prev_ret = previous;

On 11/07/2015 11:30 AM, Christopher Thielen wrote:
> Hi wine-devel,
>
> My investigation of https://bugs.winehq.org/show_bug.cgi?id=13683 has
> determined a missing WM_CAPTURECHANGED message is to blame.
>
> At some point in early execution, a WM_CAPTURECHANGED message is sent by
> Windows (but not Wine) which apparently sets off a course of calls that
> prevents this bug.
>
> I wrote some simple WM_CAPTURECHANGED tests but I have not found any
> meaningful differences in execution between Windows and Wine. Put
> another way, I've been unable to trigger u9.exe's specific behavior.
>
> Does anyone have recommendations for debugging Win32 messaging,
> specifically in the case of messages "missing" in Wine? Since I can see
> the missing message being sent by Windows with Spy++, presumably I can
> look back through API calls made before that point and try to replicate
> a test case, but that debugging method sounds fairly lengthy. Is there a
> good tool to capture and replicate all Win32 API calls perhaps?
>
> Any advice would be appreciated, thanks!
> - Christopher Thielen

[Ken Thomases]

On Nov 15, 2015, at 9:40 PM, Christopher Thielen  wrote:
> I figured out this behavior. It seems if one calls SetCapture() twice it will cause Windows to send a WM_CAPTURECHANGED message to the same window given as the parameter of SetCapture(). Wine does not currently follow this behavior.
>
> The following patch fixes the behavior, though I'm not sure if it's the right way to fix the issue as I don't know why the original author(s) made the removed check in the first place. Does anyone have any ideas?
>
> (Generated against wine.git on 11/15/15 at 19:28 PST):
>
> diff --git a/dlls/user32/input.c b/dlls/user32/input.c
> index 40e35a9..63fae67 100644
> --- a/dlls/user32/input.c
> +++ b/dlls/user32/input.c
> @@ -108,7 +108,7 @@ BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret )
>     {
>         USER_Driver->pSetCapture( hwnd, gui_flags );
> -
>    if (previous && previous != hwnd)
> -
>    if (previous)
>       SendMessageW( previous, WM_CAPTURECHANGED, 0, (LPARAM)hwnd );
>
>   if (prev_ret) *prev_ret = previous;

Have you run the user32 tests with this change in place?  Does it break any tests?

Can you write a test for the behavior that you're seeing?  Does that test pass on the testbot's Windows VMs?

The history of this code is that it was consolidated from multiple similar code paths that all had that check against the previous and new windows being the same.  In particular, it's used by implicit captures, such as with menu tracking and window moving and resizing, as well as explicit captures.  You'd want to be sure that those other paths don't break as a result of your change.

Regards,
Ken

[Christopher Thielen]

Thanks Ken. I do have a test in mind that I'll submit with my patch but 
first ... do all the tests in dlls/user32/tests/ pass normally?

I've got the latest git as of 7f6634b2bd916b530e30ef8a850fefbc0dbdf687 
(Nov 16 22:36 GMT+1) and I receive the following when I run 'make test' 
in that directory (this is without my patch):

../../../tools/runtest -q -P wine -T ../../.. -M user32.dll -p 
user32_test.exe.so win && touch win.ok
fixme:msg:pack_message msg 138 (WM_CTLCOLORSTATIC) not supported yet
err:win:DefWindowProcW called for other process window 0x4005c
fixme:msg:pack_message msg 135 (WM_CTLCOLORBTN) not supported yet
fixme:msg:pack_message msg 135 (WM_CTLCOLORBTN) not supported yet
fixme:class:CLASS_GetClassLong offset -12 (GCLP_HCURSOR) not supported 
on other process window 0x200c8
fixme:class:CLASS_GetClassLong offset -12 (GCLP_HCURSOR) not supported 
on other process window 0x200c8
fixme:msg:pack_message msg 135 (WM_CTLCOLORBTN) not supported yet
fixme:msg:pack_message msg 135 (WM_CTLCOLORBTN) not supported yet
fixme:msg:pack_message msg 135 (WM_CTLCOLORBTN) not supported yet
fixme:win:FlashWindowEx 0x33fbfc - semi-stub
fixme:win:FlashWindowEx 0x33fbfc - semi-stub
fixme:win:FlashWindowEx 0x33fbfc - semi-stub
win.c:2337: Test succeeded inside todo block: GetActiveWindow() = 0x50056
win.c:2337: Test succeeded inside todo block: GetForegroundWindow() = 
0x50056
win.c:2337: Test succeeded inside todo block: GetFocus() = 0x50056
win.c:2561: Test failed: 0x50056: expected prev (nil), got 0x1d00cc
win.c:2569: Test failed: 0xa00a8: expected next 0x50056, got 0x1d00cc
win.c:2574: Test failed: 0x50056: expected prev (nil), got 0x1d00cc
win.c:2583: Test failed: 0x50056: expected prev 0x9004c, got 0x1d00cc
win.c:2584: Test failed: 0x9004c: expected next 0x50056, got 0x1d00cc
win.c:2593: Test failed: 0x50056: expected prev 0x9004c, got 0x1d00cc
win.c:2594: Test failed: 0x9004c: expected next 0x50056, got 0x1d00cc
win.c:2604: Test failed: 0x50056: expected prev 0x9004c, got 0x1d00cc
win.c:2605: Test failed: 0x9004c: expected next 0x50056, got 0x1d00cc
win.c:2614: Test failed: 0x50056: expected prev 0xa00a8, got 0x1d00cc
win.c:2615: Test failed: 0xa00a8: expected next 0x50056, got 0x1d00cc
win.c:2639: Test failed: 0x50056: expected prev 0xa00a8, got 0x1d00cc
win.c:2640: Test failed: 0xa00a8: expected next 0x50056, got 0x1d00cc
win.c:2561: Test failed: 0x50056: expected prev (nil), got 0x1d00cc
win.c:2569: Test failed: 0xb00a8: expected next 0x50056, got 0x1d00cc
win.c:2574: Test failed: 0x50056: expected prev (nil), got 0x1d00cc
win.c:2583: Test failed: 0x50056: expected prev 0xf00aa, got 0x1d00cc
win.c:2584: Test failed: 0xf00aa: expected next 0x50056, got 0x1d00cc
win.c:2593: Test failed: 0x50056: expected prev 0xf00aa, got 0x1d00cc
win.c:2594: Test failed: 0xf00aa: expected next 0x50056, got 0x1d00cc
win.c:2604: Test failed: 0x50056: expected prev 0xf00aa, got 0x1d00cc
win.c:2605: Test failed: 0xf00aa: expected next 0x50056, got 0x1d00cc
win.c:2614: Test failed: 0x50056: expected prev 0xb00a8, got 0x1d00cc
win.c:2615: Test failed: 0xb00a8: expected next 0x50056, got 0x1d00cc
win.c:2639: Test failed: 0x50056: expected prev 0xb00a8, got 0x1d00cc
win.c:2640: Test failed: 0xb00a8: expected next 0x50056, got 0x1d00cc
win.c:5851: Test failed: expected a nonempty window text
win.c:5852: Test failed: got wrong window text 'Default - Wine desktop'
win.c:2983: Test succeeded inside todo block: Expected active window 
0x8200b0, got 0x8200b0.
win.c:2984: Test succeeded inside todo block: Expected focus window 
0x8200b0, got 0x8200b0.
Makefile:621: recipe for target 'win.ok' failed
make: *** [win.ok] Error 33

On 11/15/2015 11:09 PM, Ken Thomases wrote:
> On Nov 15, 2015, at 9:40 PM, Christopher Thielen  wrote:
> > I figured out this behavior. It seems if one calls SetCapture() twice it will cause Windows to send a WM_CAPTURECHANGED message to the same window given as the parameter of SetCapture(). Wine does not currently follow this behavior.
> >
> > The following patch fixes the behavior, though I'm not sure if it's the right way to fix the issue as I don't know why the original author(s) made the removed check in the first place. Does anyone have any ideas?
> >
> > (Generated against wine.git on 11/15/15 at 19:28 PST):
> >
> > diff --git a/dlls/user32/input.c b/dlls/user32/input.c
> > index 40e35a9..63fae67 100644
> > --- a/dlls/user32/input.c
> > +++ b/dlls/user32/input.c
> > @@ -108,7 +108,7 @@ BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret )
> >      {
> >          USER_Driver->pSetCapture( hwnd, gui_flags );
> > -
> >    if (previous && previous != hwnd)
> > -
> >    if (previous)
> >        SendMessageW( previous, WM_CAPTURECHANGED, 0, (LPARAM)hwnd );
> >
> >    if (prev_ret) *prev_ret = previous;
>
> Have you run the user32 tests with this change in place?  Does it break any tests?
>
> Can you write a test for the behavior that you're seeing?  Does that test pass on the testbot's Windows VMs?
>
> The history of this code is that it was consolidated from multiple similar code paths that all had that check against the previous and new windows being the same.  In particular, it's used by implicit captures, such as with menu tracking and window moving and resizing, as well as explicit captures.  You'd want to be sure that those other paths don't break as a result of your change.
>
> Regards,
> Ken

[Ken Thomases]

On Nov 18, 2015, at 8:59 PM, Christopher Thielen  wrote:
> Thanks Ken. I do have a test in mind that I'll submit with my patch but first ... do all the tests in dlls/user32/tests/ pass normally?

Well, not for everybody. ;)  It can be sensitive to things like desktop environment, window manager, and even graphics driver.

However, it seems that the user32:win tests generally pass pretty widely on Linux: https://test.winehq.org/data/7f6634b2bd916b530e30ef8a850fefbc0dbdf687/index_...

In your results, this:
> win.c:5852: Test failed: got wrong window text 'Default - Wine desktop'

makes me think you're running the tests in an unclean wineprefix.  In particular, it seems it's set to use virtual desktop mode.  Try setting WINEPREFIX to a fresh directory.

Regards,
Ken

[Christopher Thielen]

Thanks again Ken. WINEPREFIX and a window manager switch helped get the 
existing tests passing.

My patch doesn't break any existing tests in user32 but I'm having 
trouble writing my own test.

The crux of the bug fix is that calling SetCapture() twice in a row 
passing your own HWND both times should result in a WM_CAPTURECHANGED 
message in which the lParam == your HWND. Below is my attempt at writing 
a test for that but the test never receives the WM_CAPTURECHANGED message.

My own Win32 example code (a regular program, not a test) demonstrates 
this behavior as posted at the original bug report: 
https://bugs.winehq.org/show_bug.cgi?id=13683 (see the latest attachment).

My guess is there's some difference between the regular win proc handler 
I'm used to writing and the internal wait_for_message() debug function. 
Any ideas?

static void test_SetCapture(void)
{
     BOOL got_capture_changed;
     HWND capture_win;
     MSG msg;
     WNDCLASSA  wclass;
     HANDLE hInstance = GetModuleHandleA( NULL );

wclass.lpszClassName = "capture";
     wclass.style         = CS_HREDRAW | CS_VREDRAW;
     wclass.lpfnWndProc   = WndProc;
     wclass.hInstance     = hInstance;
     wclass.hIcon         = LoadIconA( 0, (LPCSTR)IDI_APPLICATION );
     wclass.hCursor       = LoadCursorA( NULL, (LPCSTR)IDC_ARROW );
     wclass.hbrBackground = (HBRUSH)( COLOR_WINDOW + 1 );
     wclass.lpszMenuName = 0;
     wclass.cbClsExtra    = 0;
     wclass.cbWndExtra    = 0;
     RegisterClassA( &wclass );

capture_win = CreateWindowA("capture", "capture", WS_VISIBLE | 
WS_POPUP,
             100, 100, 100, 100, 0, NULL, NULL, NULL);
     ok(capture_win != 0, "CreateWindow failed\n");

ShowWindow(capture_win, SW_SHOW);

/* flush pending messages */
     while (PeekMessageA( &msg, 0, 0, 0, PM_REMOVE )) DispatchMessageA( 
&msg );

/* simple SetCapture() twice test */
     SetCapture(capture_win);
     SetCapture(capture_win);

got_capture_changed = FALSE;
     while (wait_for_message(&msg))
     {
         DispatchMessageA(&msg);

if (msg.message == WM_CAPTURECHANGED)
         {
             // do additional checking here to ensure lParam == capture_win
             got_capture_changed = TRUE;
         }
     }
     ok(got_capture_changed, "expected WM_CAPTURECHANGED message\n");
}

On 11/18/2015 11:00 PM, Ken Thomases wrote:
> On Nov 18, 2015, at 8:59 PM, Christopher Thielen  wrote:
> > Thanks Ken. I do have a test in mind that I'll submit with my patch but first ... do all the tests in dlls/user32/tests/ pass normally?
>
> Well, not for everybody. ;)  It can be sensitive to things like desktop environment, window manager, and even graphics driver.
>
> However, it seems that the user32:win tests generally pass pretty widely on Linux: https://test.winehq.org/data/7f6634b2bd916b530e30ef8a850fefbc0dbdf687/index_...
>
> In your results, this:
> > win.c:5852: Test failed: got wrong window text 'Default - Wine desktop'
>
> makes me think you're running the tests in an unclean wineprefix.  In particular, it seems it's set to use virtual desktop mode.  Try setting WINEPREFIX to a fresh directory.
>
> Regards,
> Ken

[Dmitry Timoshkov]

Christopher Thielen  wrote:
> The crux of the bug fix is that calling SetCapture() twice in a row 
> passing your own HWND both times should result in a WM_CAPTURECHANGED 
> message in which the lParam == your HWND. Below is my attempt at writing 
> a test for that but the test never receives the WM_CAPTURECHANGED message.

Message loop can see only posted messages, sent messages go directly
to the message handling procedure. Please add a message test instead
(tests/msg.c).