[PATCH v2] ole32/tests: Improve tests for OLE's default object handler.

Sergio Gómez Del Real sdelreal at codeweavers.com
Wed May 9 14:28:42 CDT 2018


On 08/05/18 03:07, Huw Davies wrote:

> On Fri, May 04, 2018 at 01:34:32PM -0500, Sergio Gómez Del Real wrote:
>> This patch reworks the structure of the tests for OLE's default object
>> handler. We want, at least, to:
>>   - Test its behaviour when object is not running.
>>   - Test its behaviour when is being aggregated by a custom handler.
>>   - Test that it delegates appropriately to datacache.
>>   - Test that it delegates appropriately to running object.
>>
>> The present patch adds a local server (EXE), ands tests that some calls
>> in some interfaces are delegated to it after calling _Run on the
>> IRunnableObject interface.
>>
>> Signed-off-by: Sergio Gómez Del Real <sdelreal at codeweavers.com>
>> ---
>>   dlls/ole32/tests/defaulthandler.c | 687 ++++++++++++++++++++++++++++++--------
>>   1 file changed, 545 insertions(+), 142 deletions(-)
>>
>> diff --git a/dlls/ole32/tests/defaulthandler.c b/dlls/ole32/tests/defaulthandler.c
>> index 60bc29c08d..776cda2eef 100644
>> --- a/dlls/ole32/tests/defaulthandler.c
>> +++ b/dlls/ole32/tests/defaulthandler.c
>> @@ -60,24 +60,31 @@
>>           expect_ ## func = called_ ## func = FALSE; \
>>       }while(0)
>>   
>> +#define CHARS_IN_GUID 39
>> +#define CLSID_KEY_LEN (CHARS_IN_GUID+6)
>> +
>>   DEFINE_EXPECT(CF_QueryInterface_ClassFactory);
>> -DEFINE_EXPECT(CF_CreateInstance);
>> -DEFINE_EXPECT(CF_QueryInterface_IMarshal);
>> +DEFINE_EXPECT(OleObject_Update);
>> +DEFINE_EXPECT(PStorage_InitNew);
>>   
>> -static HRESULT create_storage(IStorage **stg)
>> +struct TestClass
>>   {
>> -    HRESULT hr;
>> -    ILockBytes *lock_bytes;
>> +    IUnknown IUnknown_iface;
>> +    IOleObject IOleObject_iface;
>> +    IDataObject IDataObject_iface;
>> +    IPersistStorage IPersistStorage_iface;
>>   
>> -    hr = CreateILockBytesOnHGlobal(NULL, TRUE, &lock_bytes);
>> -    if(SUCCEEDED(hr))
>> -    {
>> -        hr = StgCreateDocfileOnILockBytes(lock_bytes,
>> -                  STGM_CREATE | STGM_SHARE_EXCLUSIVE | STGM_READWRITE, 0, stg);
>> -        ILockBytes_Release(lock_bytes);
>> -    }
>> -    return hr;
>> -}
>> +    LONG ref;
>> +    CLSID clsid;
>> +};
>> +typedef struct TestClass TestClass;
>> +
>> +const CLSID CLSID_Test_Server = {0x0f77e570,0x80c3,0x11e2,{0x9e,0x96,0x08,0x00,0x20,0x0c,0x9a,0x66}};
>> +static WCHAR clsid_key[CLSID_KEY_LEN] = {'C','L','S','I','D','\\',0};
>> +static int wine_argc;
>> +static char **wine_argv;
>> +static IUnknown *dfhandler_unk;
>> +static TestClass *test_class;
>>   
>>   typedef struct
>>   {
>> @@ -88,84 +95,364 @@ typedef struct
>>       DWORD moniker_size;
>>   } ole_stream_header_t;
>>   
>> -static void test_olestream(void)
>> +static HRESULT WINAPI TC_IUnknown_QueryInterface(IUnknown *iface, REFIID riid, void **ppv)
>>   {
>> -    HRESULT hr;
>> -    const CLSID non_existent_class = {0xa5f1772f, 0x3772, 0x490f, {0x9e, 0xc6, 0x77, 0x13, 0xe8, 0xb3, 0x4b, 0x5d}};
>> -    IOleObject *ole_obj;
>> -    IPersistStorage *persist;
>> -    IStorage *stg;
>> -    IStream *stm;
>> -    static const WCHAR olestream[] = {1,'O','l','e',0};
>> -    ULONG read;
>> -    ole_stream_header_t header;
>> -
>> -    hr = create_storage(&stg);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -
>> -    hr = IStorage_OpenStream(stg, olestream, NULL, STGM_SHARE_EXCLUSIVE | STGM_READ, 0, &stm);
>> -    ok(hr == STG_E_FILENOTFOUND, "got %08x\n", hr);
>> -
>> -    hr = OleCreateDefaultHandler(&non_existent_class, 0, &IID_IOleObject, (void**)&ole_obj);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -
>> -    hr = IOleObject_QueryInterface(ole_obj, &IID_IPersistStorage, (void**)&persist);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -
>> -    hr = IPersistStorage_InitNew(persist, stg);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -
>> -    hr = IStorage_OpenStream(stg, olestream, NULL, STGM_SHARE_EXCLUSIVE | STGM_READ, 0, &stm);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -    hr = IStream_Read(stm, &header, sizeof(header), &read);
>> -    ok(hr == S_OK, "got %08x\n", hr);
>> -    ok(read == sizeof(header), "read %d\n", read);
>> -    ok(header.version == 0x02000001, "got version %08x\n", header.version);
>> -    ok(header.flags == 0x0, "got flags %08x\n", header.flags);
>> -    ok(header.link_update_opt == 0x0, "got link update option %08x\n", header.link_update_opt);
>> -    ok(header.res == 0x0, "got reserved %08x\n", header.res);
>> -    ok(header.moniker_size == 0x0, "got moniker size %08x\n", header.moniker_size);
>> -
>> -    IStream_Release(stm);
>> -
>> -    IPersistStorage_Release(persist);
>> -    IOleObject_Release(ole_obj);
>> -
>> -    IStorage_Release(stg);
>> -}
>> -
>> -static HRESULT WINAPI test_class_QueryInterface(IUnknown *iface, REFIID riid, void **ppv)
>> -{
>> -    if(IsEqualGUID(riid, &IID_IUnknown)) {
>> +    if (IsEqualGUID(riid, &IID_IUnknown))
>>           *ppv = iface;
>> -        return S_OK;
>> -    }else if(IsEqualGUID(riid, &IID_IOleObject)) {
>> -        ok(0, "unexpected query for IOleObject interface\n");
>> +    else if (IsEqualGUID(riid, &IID_IOleObject))
>> +        *ppv = &test_class->IOleObject_iface;
> test_class doesn't need to be a global variable.  Please use the
> usual impl_from_xxxx().
>
>> +    else if (IsEqualGUID(riid, &IID_IDataObject))
>> +        *ppv = &test_class->IDataObject_iface;
>> +    else if (IsEqualGUID(riid, &IID_IPersistStorage))
>> +        *ppv = &test_class->IPersistStorage_iface;
>> +    else {
> Please move the opening brace to a new line.
>
>>   START_TEST(defaulthandler)
>>   {
>> +    HRESULT hres;
>> +
>> +    wine_argc = winetest_get_mainargs( &wine_argv );
>> +    if (wine_argc > 2 && !strncmp(wine_argv[2], "-Embedding", strnlen(wine_argv[2], 10)))
>> +    {
>> +        CoInitializeEx(NULL, COINIT_MULTITHREADED);
>> +        run_local_server();
>> +        CoUninitialize();
>> +        return;
>> +    }
>> +
>>       OleInitialize(NULL);
>>   
>> -    test_olestream();
>> -    test_default_handler_run();
>> +    if (register_server() != ERROR_SUCCESS)
>> +    {
>> +        win_skip("not enough permissions to create a server CLSID key\n");
>> +        OleUninitialize();
>> +        return;
>> +    }
>>   
>> +    hres = OleCreateDefaultHandler(&CLSID_Test_Server, NULL, &IID_IUnknown, (void**)&dfhandler_unk);
>> +    ok(hres == S_OK, "OleCreateDefaultHandler failed: %x\n", hres);
>> +
>> +    if (FAILED(hres)) goto fail;
>> +
>> +    test_running_object();
> I don't think you want a global dfhandle_unk, especially as you
> shutdown the server at the end of test_running_object().
>
> Please check that the other global variables are necessary.
>
> Huw.

Hi, Huw,
I fail to see why using a global IUnknown for the client would be a 
problem; the dfhandler_unk is used to refer to the object created by 
OleCreateDefaultHandler (the default handler object). When the client 
make calls through that interface, should the default handler need to 
defer to the local object, it would do the appropriate interface 
marshaling and the call would end in the interfaces' functions in the 
server instance, which would receive marshaled copies from the COM 
library. But in any case, the server doesn't (and shouldn't) know about 
this dfhandler_unk, which is used only by the client to interface with 
the default object handler.

The global variables are there merely as a convenience; the client has 
access to the pointer to the default handler object (dfhandler_unk), 
while the server uses the test_class pointer to the local object, which 
it creates in create_testclass(), but I don't see the conflict, as the 
server is receiving in its interface functions internal marshaled 
versions from OLE, not the client's dfhandler_unk. I think the 
separation is correct, as both control different objects (default 
handler object vs local server object). Maybe I am missing something.

As for the other globals, they are there also for mere convenience; not 
necessary to have them globals, for sure.

- Sergio



More information about the wine-devel mailing list