Re: [PATCH 2/2] d3drm/tests: Add tests for IDirect3DRMObject::Add/DeleteDestroyCallback (v2).

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

A few questions / suggestions:

*) What happens if you register the same callback with different context pointers?

*) The test already does some testing of the call order. I think you could extend it a bit if you store arrays of expected callback functions and context pointers in the structure and compare them when the callbacks are invoked.

*) Does DeleteDestroyCallback return an error when no matching callback is found?

*) The last one is mostly curiosity / a bad feeling: Is there a difference between callbacks added with IDirect3DRMObject (and its derived interfaces) and IDirect3DRMObject2? I am asking because Object2 is a separate interface and might be implemented in a separate object and then aggregated. If Microsoft screwed up (they never do that, right? ;-) ) then it'll have its own list of added callbacks...

Stefan

Am 2016-04-03 um 20:42 schrieb Aaryaman Vasishta:
> v2: Fixed commit message.
> Signed-off-by: Aaryaman Vasishta
>
> dlls/d3drm/tests/d3drm.c | 203 ++++++++++++++++++-----------------------------
>  1 file changed, 79 insertions(+), 124 deletions(-)
>
> diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c
> index 1abfed3..1f197b8 100644
> --- a/dlls/d3drm/tests/d3drm.c
> +++ b/dlls/d3drm/tests/d3drm.c
> @@ -1219,6 +1219,7 @@ static void test_Frame(void)
>  struct destroy_context
>  {
>      IDirect3DRMObject *obj;
> - unsigned int test_idx;
> int called;
>
> };
>
> @@ -1238,6 +1239,81 @@ static void CDECL destroy_callback1(IDirect3DRMObject *obj, void *arg)
>      ctxt->called++;
>  }
>
> +static void test_destroy_callback(unsigned int test_idx, REFCLSID clsid, REFIID iid)
> +{
> - struct destroy_context context;
> - IDirect3DRMObject *obj;
> - IUnknown *unknown;
> - IDirect3DRM *d3drm;
> - HRESULT hr;
> -
> - hr = Direct3DRMCreate(&d3drm);
> - ok(SUCCEEDED(hr), "Test %u: Cannot get IDirect3DRM interface (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRM_CreateObject(d3drm, clsid, NULL, iid, (void **)&unknown);
> - ok(hr == D3DRM_OK, "Test %u: Cannot get IDirect3DRMObject interface (hr = %x).\n", test_idx, hr);
> - hr = IUnknown_QueryInterface(unknown, &IID_IDirect3DRMObject, (void**)&obj);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> - IUnknown_Release(unknown);
> -
> - context.called = 0;
> - context.test_idx = test_idx;
> - context.obj = obj;
> -
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, NULL, &context);
> - ok(hr == D3DRMERR_BADVALUE, "Test %u: expected D3DRMERR_BADVALUE (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - /* same callback added twice */
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_DeleteDestroyCallback(obj, destroy_callback1, NULL);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_DeleteDestroyCallback(obj, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - /* add one more */
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_DeleteDestroyCallback(obj, NULL, NULL);
> - ok(hr == D3DRMERR_BADVALUE, "Test %u: expected D3DRM_BADVALUE (hr = %x).\n", test_idx, hr);
> -
> - context.called = 0;
> - IDirect3DRMObject_Release(obj);
> - ok(context.called == 3, "Test %u: got %d, expected 3.\n", test_idx, context.called);
> -
> - /* test this pattern - add cb1, add cb2, add cb1, delete cb1 */
> - hr = IDirect3DRM_CreateObject(d3drm, clsid, NULL, iid, (void **)&unknown);
> - ok(hr == D3DRM_OK, "Test %u: Cannot get IDirect3DRMObject interface (hr = %x).\n", test_idx, hr);
> - hr = IUnknown_QueryInterface(unknown, &IID_IDirect3DRMObject, (void**)&obj);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> - IUnknown_Release(unknown);
> -
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_AddDestroyCallback(obj, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - hr = IDirect3DRMObject_DeleteDestroyCallback(obj, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> -
> - context.called = 0;
> - hr = IDirect3DRMObject_QueryInterface(obj, &IID_IDirect3DRMObject, (void**)&context.obj);
> - ok(hr == D3DRM_OK, "Test %u: expected D3DRM_OK (hr = %x).\n", test_idx, hr);
> - IDirect3DRMObject_Release(context.obj);
> - IUnknown_Release(unknown);
> - ok(context.called == 2, "Test %u: got %d, expected 2.\n", test_idx, context.called);
>
> +}
> -
>
> static void test_object(void)
>  {
>      static const struct
> @@ -1292,6 +1368,9 @@ static void test_object(void)
>              ref2 = get_refcount((IUnknown *)d3drm1);
>              ok(ref2 == ref1, "Test %u: expected ref2 == ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2);
> -
>        /* test Add/Destroy callbacks */
> -
>        test_destroy_callback(i, tests[i].clsid, tests[i].iid);
> -
>        hr = IDirect3DRM2_CreateObject(d3drm2, tests[i].clsid, NULL, tests[i].iid, (void **)&unknown);
>        ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got %#x.\n", i, hr);
>        ref2 = get_refcount((IUnknown *)d3drm1);
>
> @@ -1325,7 +1404,6 @@ static void test_object(void)
>
> static void test_Viewport(void)
>  {
> - struct destroy_context context;
> IDirectDrawClipper *pClipper;
> HRESULT hr;
> IDirect3DRM *d3drm;
>
> @@ -1411,129 +1489,6 @@ static void test_Viewport(void)
>      ok(data == 1, "got %x\n", data);
>      IDirect3DRMViewport2_Release(viewport2);
> - /* destroy callback */
> - context.called = 0;
> - hr = IDirect3DRMViewport_QueryInterface(viewport, &IID_IDirect3DRMObject, (void**)&context.obj);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMObject_Release(context.obj);
> -
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, NULL, &context);
> - ok(hr == D3DRMERR_BADVALUE, "expected D3DRMERR_BADVALUE (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - /* same callback added twice */
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_DeleteDestroyCallback(viewport, destroy_callback1, NULL);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_DeleteDestroyCallback(viewport, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - /* add one more */
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_DeleteDestroyCallback(viewport, NULL, NULL);
> - ok(hr == D3DRMERR_BADVALUE, "expected D3DRM_BADVALUE (hr = %x)\n", hr);
> -
> - context.called = 0;
> - IDirect3DRMViewport_Release(viewport);
> - ok(context.called == 3, "got %d, expected 3\n", context.called);
> -
> - /* test this pattern - add cb1, add cb2, add cb1, delete cb1 */
> - hr = IDirect3DRM_CreateViewport(d3drm, device, frame, rc.left, rc.top, rc.right, rc.bottom, &viewport);
> - ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_AddDestroyCallback(viewport, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_DeleteDestroyCallback(viewport, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - context.called = 0;
> - hr = IDirect3DRMViewport_QueryInterface(viewport, &IID_IDirect3DRMObject, (void**)&context.obj);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMObject_Release(context.obj);
> - IDirect3DRMViewport_Release(viewport);
> - ok(context.called == 2, "got %d, expected 2\n", context.called);
> -
> - /* destroy from Viewport2 */
> - hr = IDirect3DRM_CreateViewport(d3drm, device, frame, rc.left, rc.top, rc.right, rc.bottom, &viewport);
> - ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_QueryInterface(viewport, &IID_IDirect3DRMViewport2, (void**)&viewport2);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMViewport_Release(viewport);
> -
> - context.called = 0;
> - hr = IDirect3DRMViewport2_QueryInterface(viewport2, &IID_IDirect3DRMObject, (void**)&context.obj);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMObject_Release(context.obj);
> -
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, NULL, &context);
> - ok(hr == D3DRMERR_BADVALUE, "expected D3DRMERR_BADVALUE (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - /* same callback added twice */
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_DeleteDestroyCallback(viewport2, destroy_callback1, NULL);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_DeleteDestroyCallback(viewport2, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - /* add one more */
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_DeleteDestroyCallback(viewport2, NULL, NULL);
> - ok(hr == D3DRMERR_BADVALUE, "expected D3DRM_BADVALUE (hr = %x)\n", hr);
> -
> - context.called = 0;
> - IDirect3DRMViewport2_Release(viewport2);
> - ok(context.called == 3, "got %d, expected 3\n", context.called);
> -
> - /* test this pattern - add cb1, add cb2, add cb1, delete cb1 */
> - hr = IDirect3DRM_CreateViewport(d3drm, device, frame, rc.left, rc.top, rc.right, rc.bottom, &viewport);
> - ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport_QueryInterface(viewport, &IID_IDirect3DRMViewport2, (void**)&viewport2);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMViewport_Release(viewport);
> -
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback1, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_AddDestroyCallback(viewport2, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - hr = IDirect3DRMViewport2_DeleteDestroyCallback(viewport2, destroy_callback, &context);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> -
> - context.called = 0;
> - hr = IDirect3DRMViewport2_QueryInterface(viewport2, &IID_IDirect3DRMObject, (void**)&context.obj);
> - ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr);
> - IDirect3DRMObject_Release(context.obj);
> - IDirect3DRMViewport2_Release(viewport2);
> - ok(context.called == 2, "got %d, expected 2\n", context.called);
> - IDirect3DRMFrame_Release(frame);
>    IDirect3DRMDevice_Release(device);
>    IDirectDrawClipper_Release(pClipper);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXAXjPAAoJEN0/YqbEcdMwaCAP/R0jPTn4638NJY+4kpdtREkT
pmpe3+RAx7XQXqJd7baFP1Ge5A8+UyH0GC5jraqneelZ4tjIePY9weFtTI0dPlbO
W5Ptty2lIqI5e79S9BC9RYn6HOVVcLqWZTbArBwfRLWCDmHFDKIvLzzg1CmgVoZQ
rL8pPiqzE0qanj7UnAZXt8/N5Dz8Uy3aY2tXimnn/ao/ET4Tpi2rp6CRuFGXo71I
n7ydIj+spggC6GSk1HmOiAsHLOoKwmmxd83XOrQE+awlYJVxe/jrKZu6Du/29eYa
/J+xfD9iHvm2oXAvQKNy8Tca3KaynCTB0aLPzZzZ1E/UXZsI3CPYWeEzXNxkyvQv
yrSVRWyUjhAegPB3DUB4JWZq8sDiEjZw6y8hRwibN3NCGTZYtjsK2AuFAK5zUmlH
H4k6BFtYRyVNvOUARA5UHO+bJ9wzr0nak14DZbiWoDYISLFKm77nieWR4EU03TNh
cZWOpQde/CngNRXIr1mth/WAabPm8WN1IXRcayDAh5GfTC3bVLiRvrjT/vlpm8AH
G+9qVEUTmH7HRpSXJpQ4vjAnZJkpJa/ixJazz9cxcNYucPD1LMg/ZbvTGLjKRFhF
1yR9UWOFKBAZyKolehBwRuDFFzw0cXpsUJW9gW/CFyoxtoL6qV+BmmZLN2Gy6Yoe
XQJAPbjrbZ4oxHyyGsGp
=BxYw
-----END PGP SIGNATURE-----

[PATCH 2/2] d3drm/tests: Add tests for IDirect3DRMObject::Add/DeleteDestroyCallback (v2).

[Aaryaman Vasishta]

On Mon, Apr 4, 2016 at 1:40 AM, Stefan Dösinger 
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi,
>
> A few questions / suggestions:
>
> *) What happens if you register the same callback with different context
> pointers?

It'll call the callback with the respective context pointer passed to it.
I'll add a test for the same.
> *) The test already does some testing of the call order. I think you could
> extend it a bit if you store arrays of expected callback functions and
> context pointers in the structure and compare them when the callbacks are
> invoked.

In what way would you want me to test the callback order? I.e. how many
callbacks and context pointers to be used and in which order should I add
them?
> *) Does DeleteDestroyCallback return an error when no matching callback is
> found?

Yes, there's a test added which checks for D3DRMERR_BADVALUE if the
callback pointer passed is NULL. Any other random value (except for a valid
matching callback pointer) e.g. 0xdeadbeef would return the same.
> *) The last one is mostly curiosity / a bad feeling: Is there a difference
> between callbacks added with IDirect3DRMObject (and its derived interfaces)
> and IDirect3DRMObject2? I am asking because Object2 is a separate interface
> and might be implemented in a separate object and then aggregated. If
> Microsoft screwed up (they never do that, right? ;-) ) then it'll have its
> own list of added callbacks...

That's not tested yet, but it could be the case that Microsoft might be
adding their own internal callbacks for handling of internal objects.

Cheers,
Aaryaman

[PATCH 2/2] d3drm/tests: Add tests for IDirect3DRMObject::Add/DeleteDestroyCallback (v2).

[Aaryaman Vasishta]

On Mon, Apr 4, 2016 at 1:40 AM, Stefan Dösinger 
wrote:
> *) The test already does some testing of the call order. I think you could
> extend it a bit if you store arrays of expected callback functions and
> context pointers in the structure and compare them when the callbacks are
> invoked.
>
> Something like this? http://pastebin.com/hwAA66Vj

It's not final yet. I could leave the existing tests alone and use a
seperate context pointer for the call order. I'm also aware that the id
member might be useless.

Cheers,
Aaryaman