[PATCH v2 1/2] user32: Let nulldrv return some dummy monitor information.

[Bernhard Übelacker]

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

Signed-off-by: Bernhard Übelacker 
---
Supersedes patch 151898.

v2: No changes to v1.
---
 dlls/user32/driver.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/dlls/user32/driver.c b/dlls/user32/driver.c
index 561a126bd6..03296d518c 100644
--- a/dlls/user32/driver.c
+++ b/dlls/user32/driver.c
@@ -354,7 +354,12 @@ static LONG CDECL nulldrv_ChangeDisplaySettingsEx( LPCWSTR name, LPDEVMODEW mode

static BOOL CDECL nulldrv_EnumDisplayMonitors( HDC hdc, LPRECT rect, MONITORENUMPROC proc, LPARAM lp )
 {
-    return FALSE;
+    RECT r = {0, 0, 640, 480};
+
+    TRACE("(%p, %p, %p, 0x%lx)\n", hdc, rect, proc, lp);
+
+    proc(NULL, hdc, &r, lp);
+    return TRUE;
 }

static BOOL CDECL nulldrv_EnumDisplaySettingsEx( LPCWSTR name, DWORD num, LPDEVMODEW mode, DWORD flags )
@@ -364,7 +369,17 @@ static BOOL CDECL nulldrv_EnumDisplaySettingsEx( LPCWSTR name, DWORD num, LPDEVM

static BOOL CDECL nulldrv_GetMonitorInfo( HMONITOR handle, LPMONITORINFO info )
 {
-    return FALSE;
+    RECT r = {0, 0, 640, 480};
+    const WCHAR device[] = {'W','i','n','D','i','s','c',0};
+
+    TRACE("(%p, %p)\n", handle, info);
+
+    info->rcMonitor = r;
+    info->rcWork = r;
+    info->dwFlags = MONITORINFOF_PRIMARY;
+    if (info->cbSize >= sizeof(MONITORINFOEXW))
+        lstrcpyW( ((MONITORINFOEXW *)info)->szDevice, device );
+    return TRUE;
 }

static BOOL CDECL nulldrv_CreateDesktopWindow( HWND hwnd )

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Bernhard Übelacker]

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

Signed-off-by: Bernhard Übelacker 
---
This patch is not merged with the previous one because there
is no todo_wine for service_ok.

Supersedes patch 151899.

v2: Fix test for wxppro or w2003.
---
 programs/services/tests/service.c | 36 +++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/programs/services/tests/service.c b/programs/services/tests/service.c
index 728d515f85..e4538559c0 100644
--- a/programs/services/tests/service.c
+++ b/programs/services/tests/service.c
@@ -127,6 +127,41 @@ static void test_create_window(void)
     service_ok(r, "DestroyWindow failed: %08x\n", GetLastError());
 }

+static BOOL CALLBACK monitor_enum_proc(HMONITOR hmon, HDC hdc, LPRECT lprc, LPARAM lparam)
+{
+    BOOL r;
+    MONITORINFOEXA mi;
+
+    mi.cbSize = sizeof(mi);
+
+    r = GetMonitorInfoA(hmon, (MONITORINFO*)&mi);
+    service_ok(r, "GetMonitorInfo failed.\n");
+
+    service_ok(mi.rcMonitor.left == 0 && mi.rcMonitor.top == 0 && mi.rcMonitor.right >= 640 && mi.rcMonitor.bottom >= 480,
+               "Unexepected monitor rcMonitor values: {%d,%d,%d,%d}\n",
+               mi.rcMonitor.left, mi.rcMonitor.top, mi.rcMonitor.right, mi.rcMonitor.bottom);
+
+    service_ok(mi.rcWork.left == 0 && mi.rcWork.top == 0 && mi.rcWork.right >= 640 && mi.rcWork.bottom >= 480,
+               "Unexepected monitor rcWork values: {%d,%d,%d,%d}\n",
+               mi.rcWork.left, mi.rcWork.top, mi.rcWork.right, mi.rcWork.bottom);
+
+    service_ok(!strcmp(mi.szDevice, "WinDisc") || !strcmp(mi.szDevice, "\\.\DISPLAY1"),
+               "Unexpected szDevice received: %s\n", mi.szDevice);
+
+    service_ok(mi.dwFlags & MONITORINFOF_PRIMARY, "Unexpected secondary monitor info.\n");
+
+    return TRUE;
+}
+
+/* query informations monitor information, even in non-interactive services */
+static void test_monitors(void)
+{
+    BOOL r;
+
+    r = EnumDisplayMonitors(0, 0, monitor_enum_proc, 0);
+    service_ok(r, "EnumDisplayMonitors failed.\n");
+}
+
 static DWORD WINAPI service_handler(DWORD ctrl, DWORD event_type, void *event_data, void *context)
 {
     SERVICE_STATUS status;
@@ -151,6 +186,7 @@ static DWORD WINAPI service_handler(DWORD ctrl, DWORD event_type, void *event_da
     case 128:
         test_winstation();
         test_create_window();
+        test_monitors();
         service_event("CUSTOM");
         return 0xdeadbeef;
     default:

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Jacek Caban]

Hi Bernhard,

On 28/09/2018 23:08, Bernhard Übelacker wrote:
> +/* query informations monitor information, even in non-interactive services */
> +static void test_monitors(void)
> +{
> - BOOL r;
> -
> - r = EnumDisplayMonitors(0, 0, monitor_enum_proc, 0);
> - service_ok(r, "EnumDisplayMonitors failed.\n");

Note that an implementation that will not call the callback at all will 
pass this test as well. It would be nice to make sure that callback was 
called exactly once.

Thanks,
Jacek

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Jacek Caban]

On 28/09/2018 23:08, Bernhard Übelacker wrote:
> -
> - service_ok(mi.dwFlags & MONITORINFOF_PRIMARY, "Unexpected secondary monitor info.\n");

Are those flags consistent between Windows version? It would be better 
to test all flags here.

Jacek

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Bernhard Übelacker]

Hello Jacek,
I am unsure about that point:
> > -
> >
> > +    service_ok(mi.dwFlags & MONITORINFOF_PRIMARY, "Unexpected
> > secondary monitor info.\n");
>
> Are those flags consistent between Windows version? It would be better
> to test all flags here.

In /include/winuser.h there is just that flag and I found also no other
flags used inside the other display drivers.

Also MSDN states just that only possible flag for the dwFlags member
of the tagMONITORINFO structure.

Do you know of some other flag values or can we assume that point as checked?

Kind regards,
Bernhard

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Jacek Caban]

Hi Bernhard,

On 10/01/2018 07:41 PM, Bernhard Übelacker wrote:
> Hello Jacek,
> I am unsure about that point:
> > > -
> > >
> > > +    service_ok(mi.dwFlags & MONITORINFOF_PRIMARY, "Unexpected
> > > secondary monitor info.\n");
> >
> > Are those flags consistent between Windows version? It would be better
> > to test all flags here.
>
> In /include/winuser.h there is just that flag and I found also no other
> flags used inside the other display drivers.
>
> Also MSDN states just that only possible flag for the dwFlags member
> of the tagMONITORINFO structure.
>
> Do you know of some other flag values or can we assume that point as checked?

What I suggested is really using a comparison instead of bit and
(mi.dwFlags == MONITORINFOF_PRIMARY). This way you can ensure that there
are no more flags.

Jacek

[PATCH v2 2/2] services/tests: Retrieve monitor information when executed as service.

[Bernhard Übelacker]

Am 01.10.2018 um 19:42 schrieb Jacek Caban:
> Hi Bernhard,
>
> On 10/01/2018 07:41 PM, Bernhard Übelacker wrote:
> > Hello Jacek,
> > I am unsure about that point:
> > > > -
> > > >
> > > > +    service_ok(mi.dwFlags & MONITORINFOF_PRIMARY, "Unexpected
> > > > secondary monitor info.\n");
> > >
> > > Are those flags consistent between Windows version? It would be better
> > > to test all flags here.
> >
> > In /include/winuser.h there is just that flag and I found also no other
> > flags used inside the other display drivers.
> >
> > Also MSDN states just that only possible flag for the dwFlags member
> > of the tagMONITORINFO structure.
> >
> > Do you know of some other flag values or can we assume that point as checked?
>
> What I suggested is really using a comparison instead of bit and
> (mi.dwFlags == MONITORINFOF_PRIMARY). This way you can ensure that there
> are no more flags.
>
> Jacek

Thank you, now I got it.
Will send a new version soon.

Kind regards,
Bernhard

[Jacek Caban]

Hi Bernhard,

On 28/09/2018 23:08, Bernhard Übelacker wrote:
> Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=43187
> Signed-off-by: Bernhard Übelacker
>
> Supersedes patch 151898.
> v2: No changes to v1.
>
> dlls/user32/driver.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/user32/driver.c b/dlls/user32/driver.c
> index 561a126bd6..03296d518c 100644
> --- a/dlls/user32/driver.c
> +++ b/dlls/user32/driver.c
> @@ -354,7 +354,12 @@ static LONG CDECL nulldrv_ChangeDisplaySettingsEx( LPCWSTR name, LPDEVMODEW mode
>
> static BOOL CDECL nulldrv_EnumDisplayMonitors( HDC hdc, LPRECT rect, MONITORENUMPROC proc, LPARAM lp )
>   {
> - return FALSE;
> - RECT r = {0, 0, 640, 480};
> -
> - TRACE("(%p, %p, %p, 0x%lx)\n", hdc, rect, proc, lp);
> -
> - proc(NULL, hdc, &r, lp);
> - return TRUE;
> }

NULL doesn't seem to be a good choice for monitor handle here. It's 
probably worth testing.
> static BOOL CDECL nulldrv_EnumDisplaySettingsEx( LPCWSTR name, DWORD num, LPDEVMODEW mode, DWORD flags )
> @@ -364,7 +369,17 @@ static BOOL CDECL nulldrv_EnumDisplaySettingsEx( LPCWSTR name, DWORD num, LPDEVM
>
> static BOOL CDECL nulldrv_GetMonitorInfo( HMONITOR handle, LPMONITORINFO info )
>   {
> - return FALSE;
> - RECT r = {0, 0, 640, 480};
> - const WCHAR device[] = {'W','i','n','D','i','s','c',0};

It should be static const.
> -
> - TRACE("(%p, %p)\n", handle, info);
> -
> - info->rcMonitor = r;
> - info->rcWork = r;
> - info->dwFlags = MONITORINFOF_PRIMARY;
> - if (info->cbSize >= sizeof(MONITORINFOEXW))
> -
>    lstrcpyW( ((MONITORINFOEXW *)info)->szDevice, device );
> - return TRUE;
> }

Monitor handle is not used, so maybe it's not too important, but we 
should probably validate it anyway.

Thanks,
Jacek

[Marvin]

Hi,

While running your changed tests on Windows, 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=42711

Your paranoid android.

=== debian9 (build log) ===

Task: Patch failed to apply

[Bernhard Übelacker]

Hello Jacek,
thank you very much for your feedback.
I am going to improve the patches and resend.

Kind regards,
Bernhard