Re: [PATCH 2/2] quartz/tests: Test renderer's parent WM_DESTROY notification.

[Zebediah Figura]

On 1/9/20 9:22 AM, Gabriel Ivăncescu wrote:
> On 09/01/2020 16:46, Zebediah Figura wrote:
> > On 1/9/20 7:13 AM, Gabriel Ivăncescu wrote:
> > > Signed-off-by: Gabriel Ivăncescu
> > >
> > > dlls/quartz/tests/filtergraph.c | 35 ++++++++++++++++++++++++++++++++-
> > >   1 file changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/dlls/quartz/tests/filtergraph.c 
> > > b/dlls/quartz/tests/filtergraph.c
> > > index ca45031..726c54a 100644
> > > --- a/dlls/quartz/tests/filtergraph.c
> > > +++ b/dlls/quartz/tests/filtergraph.c
> > > @@ -4232,16 +4232,38 @@ static HWND get_renderer_hwnd(IFilterGraph2 
> > > *graph)
> > >       return hwnd;
> > >   }
> > > +static LRESULT CALLBACK parent_wndproc(HWND hwnd, UINT msg, WPARAM 
> > > wParam, LPARAM lParam)
> > > +{
> > > +    switch (msg)
> > > +    {
> > > +        case WM_PARENTNOTIFY:
> > > +            if (LOWORD(wParam) == WM_DESTROY)
> > > +                ok(0, "Received WM_PARENTNOTIFY with WM_DESTROY.\n");
> > > +        break;
> > > +    }
> > > +    return DefWindowProcA(hwnd, msg, wParam, lParam);
> > > +}
> > > -
> > >
> > > static void test_window_threading(void)
> > >   {
> > >       WCHAR *filename = load_resource(avifile);
> > >       IFilterGraph2 *graph = create_graph();
> > > +    WNDCLASSA cls = { 0 };
> > > +    HWND hwnd, parent;
> > >       HRESULT hr;
> > >       DWORD tid;
> > >       ULONG ref;
> > > -    HWND hwnd;
> > >       BOOL ret;
> > > +    cls.lpfnWndProc   = parent_wndproc;
> > > +    cls.hInstance     = GetModuleHandleA(NULL);
> > > +    cls.hCursor       = LoadCursorA(0, (const char*)IDC_ARROW);
> > > +    cls.lpszClassName = "TestParent";
> > > +    RegisterClassA(&cls);
> > > -
> > >
> > > +    parent = CreateWindowExA(0, "TestParent", NULL, 
> > > WS_OVERLAPPEDWINDOW, 50, 50, 150, 150, NULL, NULL, cls.hInstance, NULL);
> > > +    ok(parent != NULL, "Failed to create parent window.\n");
> > > -
> > >
> > > hr = IFilterGraph2_RenderFile(graph, filename, NULL);
> > >       if (FAILED(hr))
> > >       {
> > > @@ -4256,6 +4278,10 @@ static void test_window_threading(void)
> > >       {
> > >           tid = GetWindowThreadProcessId(hwnd, NULL);
> > >           ok(tid != GetCurrentThreadId(), "Window should have been 
> > > created on a separate thread.\n");
> > > +        ok(!(GetWindowLongW(hwnd, GWL_EXSTYLE) & 
> > > WS_EX_NOPARENTNOTIFY), "Window has WS_EX_NOPARENTNOTIFY.\n");
> > > -
> > >
> > > +        SetParent(hwnd, parent);
> > > +        SetWindowLongW(hwnd, GWL_STYLE, GetWindowLongW(hwnd, 
> > > GWL_STYLE) | WS_CHILD);
> >
> > Does the application actually do this, or does it use 
> > IVideoWindow::put_Owner()? If it is the former, I'd like to see a 
> > comment to that effect.
>
> I actually don't know what the app does when it sets the parent. I wrote 
> the test to see if Windows notifies the parent, and it seems that it 
> doesn't (I also set the WS_CHILD just in case), despite not having the 
> WS_EX_NOPARENTNOTIFY style in the first place. Wine would fail this test 
> without the first patch, even without put_Owner.
>
> I'll investigate what the app does. However, while this patchset fixes 
> that app, I thought a more generic solution would be preferable, since 
> that's how Windows seems to work (unless you have a better idea).

Manually fiddling with the window handle is not really something 
applications are supposed to do. I wouldn't necessarily expect anything 
about that to work. I don't know that it would cause a difference in 
this case, but it's better to test with public APIs.
> Also, by comment, you meant in the test file as C comment, right?

Yes.
> Thanks,
> Gabriel