Re: [1/4] ole32/test: Add InitCache test

[Huw Davies]

On Tue, May 02, 2017 at 06:10:44PM +0800, Wei Xie wrote:
> From 4f8b94b5a8c7739901eae8f19317ca6255911d20 Mon Sep 17 00:00:00 2001
> From: Wei xie <xiewei(a)linuxdeepin.com>
> Date: Tue, 2 May 2017 18:07:22 +0800
> Subject: [PATCH 1/4] ole32/test: Add InitCache test

ole32/test: -> ole32/tests:
> +static HRESULT WINAPI DataObject2_QueryInterface(
> +            IDataObject*     iface,
> +            REFIID           riid,
> +            void**           ppvObject)
> +{
> +    CHECK_EXPECTED_METHOD("DataObject2_QueryInterface");
> +
> +    if (IsEqualIID(riid, &IID_IDataObject) || IsEqualIID(riid, &IID_IUnknown))
> +    {
> +        *ppvObject = iface;
> +        return S_OK;
> +    }
> +    *ppvObject = NULL;
> +    return S_OK;
> +}

There's no need to create a new IDataObject implementation.
Use the existing one and add a file-scope variable to control
what _GetData does (which is all you really care about for
this test).
> +static HRESULT WINAPI DataObject2_GetData(
> +        IDataObject*     iface,
> +        LPFORMATETC      pformatetcIn,
> +        STGMEDIUM*       pmedium)
> +{
> +        *pformatetcIn = FormatEtc;
> +        *pmedium = StgMedium;
> +        return S_OK;
> +}

You're not checking this method is called, which should be
one of the goals of this test!

pfomratetcIn is an [in] variable, so assigning things to it is
useless.
> @@ -1838,10 +1994,23 @@ todo_wine {
>
> DeleteDC(hdcMem);
>
> -    todo_wine {

You remove a todo_wine here and yet this patch doesn't change
Wine's implementation, so this will produce a failure.  The
test must run cleanly on Wine (and Windows) after just this
patch is applied.  If you fix the implementation later on in
the series, then remove the todo_wine in that patch, not this
one.
> +    /* IDataObject_GetData Invalid */

I don't understand how this comment relates to the code.
> hr = IOleCache2_InitCache(pOleCache, &DataObject);
>      ok(hr == CACHE_E_NOCACHE_UPDATED, "IOleCache_InitCache should have returned CACHE_E_NOCACHE_UPDATED instead of 0x%08x\n", hr);
> -    }
> +
> +    fmtetc.ptd = NULL;
> +    fmtetc.dwAspect = DVASPECT_CONTENT;
> +    fmtetc.lindex = -1;
> +    fmtetc.cfFormat = CF_BITMAP;
> +    fmtetc.tymed = TYMED_GDI;
> +    stgmedium.tymed = TYMED_GDI;
> +    stgmedium.hBitmap = CreateCompatibleBitmap(hdcMem, 100, 100);
> +    stgmedium.pUnkForRelease = NULL;
> +    IDataObject_SetData(&DataObject2, &fmtetc, &stgmedium, FALSE);
> +
> +    /* IOleCache2_InitCache not need IOleCache2_Cache first */
> +    hr = IOleCache2_InitCache(pOleCache, &DataObject2);
> +    ok(hr == CACHE_E_NOCACHE_UPDATED, "IOleCache2_InitCache should have returned CACHE_E_NOCACHE_UPDATED instead of 0x%08x\n", hr);

Why is the hunk even necessary, you're just calling InitCache again
below a test that shows it fails.
> IPersistStorage_Release(pPS);
>      IViewObject_Release(pViewObject);
> @@ -1851,6 +2020,44 @@ todo_wine {
>      CHECK_NO_EXTRA_METHODS();
>      }
>
> +    hdcMem = CreateCompatibleDC(NULL);
> +    fmtetc.ptd = NULL;
> +    fmtetc.dwAspect = DVASPECT_CONTENT;
> +    fmtetc.lindex = -1;
> +    fmtetc.cfFormat = CF_BITMAP;
> +    fmtetc.tymed = TYMED_GDI;
> +    stgmedium.tymed = TYMED_GDI;
> +    stgmedium.hBitmap = CreateCompatibleBitmap(hdcMem, 100, 100);
> +    stgmedium.pUnkForRelease = NULL;
> +    IDataObject_SetData(&DataObject2, &fmtetc, &stgmedium, FALSE);
> +
> +    expected_method_list = methods_cacheinit;
> +
> +    hr = CreateDataCache(NULL, &CLSID_NULL, &IID_IOleCache2, (LPVOID *)&pOleCache);
> +    ok_ole_success(hr, "CreateDataCache");
> +
> +    hr = IOleCache2_QueryInterface(pOleCache, &IID_IViewObject2, (LPVOID *)&pViewObject2);
> +    ok_ole_success(hr, "IOleCache_QueryInterface(IID_IViewObject2)");
> +
> +    hr = IOleCache2_Cache(pOleCache, &fmtetc, 0, &dwConnection);
> +    ok_ole_success(hr, "IOleCache2_Cache");
> +    hr = IOleCache2_InitCache(pOleCache, &DataObject2);
> +    ok_ole_success(hr, "IOleCache2_InitCache");
> +
> +    hr = IViewObject2_SetAdvise(pViewObject2, DVASPECT_CONTENT, ADVF_PRIMEFIRST, &AdviseSink);
> +    ok_ole_success(hr, "IViewObject2_SetAdvise");
> +
> +    hr = IViewObject2_GetExtent(pViewObject2, DVASPECT_CONTENT, -1, NULL, &sz );
> +    ok_ole_success(hr, "IViewObject2_GetExtent");
> +
> +    hr = IViewObject2_Draw(pViewObject2, DVASPECT_CONTENT, -1, NULL, NULL, NULL, hdcMem, &rcBounds, NULL, draw_continue, 0xdeadbeef);
> +    ok_ole_success(hr, "IViewObject2_GetExtent");
> +
> +    IViewObject2_Release(pViewObject2);
> +    IOleCache2_Release(pOleCache);
> +
> +    DeleteDC(hdcMem);

For this bit of the test, drop the IViewObject calls.  What we care
about is testing InitCache().

Then I suggest calling _Cache twice with two different formats and
checking that _InitCache calls DataObject_GetData for those two
formats.  You'll need to make DataObject_GetData respond to these two
different formats.

It's a bit more complicated than that however because in the CF_BITMAP
case _InitCache apparently calls _GetData with CF_DIB (this is part of the
cache's ability to use dibs instead of bitmaps and vice-versa).  You
may want to avoid using CF_BITMAP to test this at this stage (we can
add CF_BITMAP <-> CF_DIB tests later).

Huw.