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

[Sergio Gómez Del Real]

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 
---
 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;
+    else if (IsEqualGUID(riid, &IID_IDataObject))
+        *ppv = &test_class->IDataObject_iface;
+    else if (IsEqualGUID(riid, &IID_IPersistStorage))
+        *ppv = &test_class->IPersistStorage_iface;
+    else {
         *ppv = NULL;
         return E_NOINTERFACE;
     }

-    *ppv = NULL;
-    return E_NOINTERFACE;
+    IUnknown_AddRef((IUnknown *)*ppv);
+    return S_OK;
 }

-static ULONG WINAPI test_class_AddRef(IUnknown *iface)
+static ULONG WINAPI TC_IUnknown_AddRef(IUnknown *iface)
 {
-    return 2;
+    return InterlockedIncrement(&test_class->ref);
 }

-static ULONG WINAPI test_class_Release(IUnknown *iface)
+static ULONG WINAPI TC_IUnknown_Release(IUnknown *iface)
 {
-    return 1;
+    return InterlockedDecrement(&test_class->ref);
 }

-static const IUnknownVtbl test_class_vtbl = {
-    test_class_QueryInterface,
-    test_class_AddRef,
-    test_class_Release,
+static const IUnknownVtbl TC_IUnknown_Vtbl = {
+    TC_IUnknown_QueryInterface,
+    TC_IUnknown_AddRef,
+    TC_IUnknown_Release,
 };

-static IUnknown test_class = { &test_class_vtbl };
+static HRESULT WINAPI TC_IOleObject_QueryInterface(IOleObject *iface, REFIID riid, void **ppv)
+{
+    return IUnknown_QueryInterface(&test_class->IUnknown_iface, riid, ppv);
+}
+
+static ULONG WINAPI TC_IOleObject_AddRef(IOleObject *iface)
+{
+    return IUnknown_AddRef(&test_class->IUnknown_iface);
+}
+
+static ULONG WINAPI TC_IOleObject_Release(IOleObject *iface)
+{
+    return IUnknown_Release(&test_class->IUnknown_iface);
+}
+
+static HRESULT WINAPI TC_IOleObject_Advise(IOleObject *iface, IAdviseSink *advs, DWORD *conn)
+{
+    return S_OK;
+}
+
+static HRESULT WINAPI TC_IOleObject_Close(IOleObject *iface, DWORD save_opt)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_DoVerb(IOleObject *iface, LONG verb,
+        LPMSG msg, IOleClientSite *actv, LONG lindex, HWND hwnd, LPCRECT pos_rec)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_EnumAdvise(IOleObject *iface, IEnumSTATDATA **stat)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_EnumVerbs(IOleObject *iface, IEnumOLEVERB **enumole)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetClientSite(IOleObject *iface, IOleClientSite **csite)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetClipboardData(IOleObject *iface,
+        DWORD res, IDataObject **dobj)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetExtent(IOleObject *iface, DWORD draw_asp, SIZEL *size)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetMiscStatus(IOleObject *iface, DWORD asp, DWORD *stat)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetMoniker(IOleObject *iface, DWORD assign,
+        DWORD which_mon, IMoniker **mon)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetUserClassID(IOleObject *iface, CLSID *clsid)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_GetUserType(IOleObject *iface, DWORD form, LPOLESTR *utype)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_InitFromData(IOleObject *iface,
+        IDataObject *dobj, BOOL creation, DWORD res)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_IsUpToDate(IOleObject *iface)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_SetClientSite(IOleObject *iface, IOleClientSite *csite)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_SetColorScheme(IOleObject *iface, LOGPALETTE *logpal)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_SetExtent(IOleObject *iface, DWORD draw_asp, SIZEL *size)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_SetHostNames(IOleObject *iface,
+        LPCOLESTR cont_app, LPCOLESTR cont_obj)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_SetMoniker(IOleObject *iface, DWORD which_mon, IMoniker *mon)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_Unadvise(IOleObject *iface, DWORD conn)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IOleObject_Update(IOleObject *iface)
+{
+    HANDLE update_event;
+
+    update_event = CreateEventA(NULL, FALSE, FALSE, "IOleObject_Update call in local server");
+    SetEvent(update_event);
+    CloseHandle(update_event);
+    return S_OK;
+}
+
+static const IOleObjectVtbl TC_IOleObject_Vtbl = {
+    TC_IOleObject_QueryInterface,
+    TC_IOleObject_AddRef,
+    TC_IOleObject_Release,
+    TC_IOleObject_SetClientSite,
+    TC_IOleObject_GetClientSite,
+    TC_IOleObject_SetHostNames,
+    TC_IOleObject_Close,
+    TC_IOleObject_SetMoniker,
+    TC_IOleObject_GetMoniker,
+    TC_IOleObject_InitFromData,
+    TC_IOleObject_GetClipboardData,
+    TC_IOleObject_DoVerb,
+    TC_IOleObject_EnumVerbs,
+    TC_IOleObject_Update,
+    TC_IOleObject_IsUpToDate,
+    TC_IOleObject_GetUserClassID,
+    TC_IOleObject_GetUserType,
+    TC_IOleObject_SetExtent,
+    TC_IOleObject_GetExtent,
+    TC_IOleObject_Advise,
+    TC_IOleObject_Unadvise,
+    TC_IOleObject_EnumAdvise,
+    TC_IOleObject_GetMiscStatus,
+    TC_IOleObject_SetColorScheme,
+};
+
+static HRESULT WINAPI TC_IDataObject_QueryInterface(IDataObject *iface, REFIID riid, void **ppv)
+{
+    return IUnknown_QueryInterface(&test_class->IUnknown_iface, riid, ppv);
+}
+
+static ULONG WINAPI TC_IDataObject_AddRef(IDataObject *iface)
+{
+    return IUnknown_AddRef(&test_class->IUnknown_iface);
+}
+
+static ULONG WINAPI TC_IDataObject_Release(IDataObject *iface)
+{
+    return IUnknown_Release(&test_class->IUnknown_iface);
+}
+
+static HRESULT WINAPI TC_IDataObject_DAdvise(IDataObject *iface, FORMATETC *fmt,
+        DWORD adv, LPADVISESINK advs, DWORD *conn)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_DUnadvise(IDataObject *iface, DWORD conn)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_EnumDAdvise(IDataObject *iface, IEnumSTATDATA **stat)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_EnumFormatEtc(IDataObject *iface, DWORD dir, IEnumFORMATETC **fmt)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_GetCanonicalFormatEtc(IDataObject *iface, FORMATETC *in, FORMATETC *out)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_GetData(IDataObject *iface, FORMATETC *fmt, STGMEDIUM *stg)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_GetDataHere(IDataObject *iface, FORMATETC *fmt, STGMEDIUM *stg)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_QueryGetData(IDataObject *iface, FORMATETC *fmt)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IDataObject_SetData(IDataObject *iface, FORMATETC *fmt, STGMEDIUM *med, BOOL rel)
+{
+    return E_NOTIMPL;
+}
+
+static const IDataObjectVtbl TC_IDataObject_Vtbl = {
+    TC_IDataObject_QueryInterface,
+    TC_IDataObject_AddRef,
+    TC_IDataObject_Release,
+    TC_IDataObject_GetData,
+    TC_IDataObject_GetDataHere,
+    TC_IDataObject_QueryGetData,
+    TC_IDataObject_GetCanonicalFormatEtc,
+    TC_IDataObject_SetData,
+    TC_IDataObject_EnumFormatEtc,
+    TC_IDataObject_DAdvise,
+    TC_IDataObject_DUnadvise,
+    TC_IDataObject_EnumDAdvise,
+};
+
+static HRESULT WINAPI TC_IPersistStorage_QueryInterface(IPersistStorage *iface, REFIID riid, void **ppv)
+{
+    return IUnknown_QueryInterface(&test_class->IUnknown_iface, riid, ppv);
+}
+
+static ULONG WINAPI TC_IPersistStorage_AddRef(IPersistStorage *iface)
+{
+    return IUnknown_AddRef(&test_class->IUnknown_iface);
+}
+
+static ULONG WINAPI TC_IPersistStorage_Release(IPersistStorage *iface)
+{
+    return IUnknown_Release(&test_class->IUnknown_iface);
+}
+
+static HRESULT WINAPI TC_IPersistStorage_GetClassID(IPersistStorage *iface, CLSID *clsid)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_IsDirty(IPersistStorage *iface)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_InitNew(IPersistStorage *iface, IStorage *stg)
+{
+    HANDLE update_event;
+
+    update_event = CreateEventA(NULL, FALSE, FALSE, "IPersistStorage_InitNew call in local server");
+    SetEvent(update_event);
+    CloseHandle(update_event);
+
+    return S_OK;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_Load(IPersistStorage *iface, IStorage *stg)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_Save(IPersistStorage *iface, IStorage *stg, BOOL same_as_load)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_SaveCompleted(IPersistStorage *iface, IStorage *stg)
+{
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI TC_IPersistStorage_HandsOffStorage(IPersistStorage *iface)
+{
+    return E_NOTIMPL;
+}
+
+static const IPersistStorageVtbl TC_IPersistStorage_Vtbl = {
+    TC_IPersistStorage_QueryInterface,
+    TC_IPersistStorage_AddRef,
+    TC_IPersistStorage_Release,
+    TC_IPersistStorage_GetClassID,
+    TC_IPersistStorage_IsDirty,
+    TC_IPersistStorage_InitNew,
+    TC_IPersistStorage_Load,
+    TC_IPersistStorage_Save,
+    TC_IPersistStorage_SaveCompleted,
+    TC_IPersistStorage_HandsOffStorage,
+};
+
+static HRESULT create_testclass(IUnknown *pUnkOuter, REFIID riid, void **ppv)
+{
+    HRESULT hres;
+
+    test_class = HeapAlloc(GetProcessHeap(), 0, sizeof(TestClass));
+
+    if (!test_class)
+        return E_OUTOFMEMORY;
+
+    test_class->IUnknown_iface.lpVtbl = &TC_IUnknown_Vtbl;
+    test_class->IOleObject_iface.lpVtbl = &TC_IOleObject_Vtbl;
+    test_class->IDataObject_iface.lpVtbl = &TC_IDataObject_Vtbl;
+    test_class->IPersistStorage_iface.lpVtbl = &TC_IPersistStorage_Vtbl;
+    test_class->ref = 1;
+    test_class->clsid = CLSID_Test_Server;
+
+    hres = IUnknown_QueryInterface(&test_class->IUnknown_iface, riid, ppv);
+    ok(hres == S_OK, "QuereyInterface failed with error %x\n", hres);
+
+    return hres;
+}

static HRESULT WINAPI ClassFactory_QueryInterface(IClassFactory *iface, REFIID riid, void **ppv)
 {
@@ -173,11 +460,13 @@ static HRESULT WINAPI ClassFactory_QueryInterface(IClassFactory *iface, REFIID r
         *ppv = iface;
         return S_OK;
     }else if(IsEqualGUID(riid, &IID_IMarshal)) {
-        CHECK_EXPECT(CF_QueryInterface_IMarshal);
         *ppv = NULL;
         return E_NOINTERFACE;
     }else if(IsEqualGUID(riid, &IID_IClassFactory)) {
-        CHECK_EXPECT(CF_QueryInterface_ClassFactory);
+        HANDLE cf_event;
+        cf_event = CreateEventA(NULL, FALSE, FALSE, "Local server's CF");
+        SetEvent(cf_event);
+        CloseHandle(cf_event);
         *ppv = iface;
         return S_OK;
     }
@@ -200,7 +489,7 @@ static ULONG WINAPI ClassFactory_Release(IClassFactory *iface)
 static HRESULT WINAPI ClassFactory_CreateInstance(IClassFactory *iface,
         IUnknown *pUnkOuter, REFIID riid, void **ppv)
 {
-    CHECK_EXPECT(CF_CreateInstance);
+    HRESULT hres;

ok(pUnkOuter == NULL, "pUnkOuter != NULL\n");
     todo_wine ok(IsEqualGUID(riid, &IID_IUnknown), "riid = %s\n", wine_dbgstr_guid(riid));
@@ -209,8 +498,10 @@ static HRESULT WINAPI ClassFactory_CreateInstance(IClassFactory *iface,
         return E_NOINTERFACE;
     }

-    *ppv = &test_class;
-    return S_OK;
+    hres = create_testclass(pUnkOuter, riid, ppv);
+    ok(hres == S_OK, "create_testclass failed with error %x\n", hres);
+
+    return hres;
 }

static HRESULT WINAPI ClassFactory_LockServer(IClassFactory *iface, BOOL fLock)
@@ -229,91 +520,203 @@ static const IClassFactoryVtbl ClassFactoryVtbl = {

static IClassFactory ClassFactory = { &ClassFactoryVtbl };

-static void test_default_handler_run(void)
+static HRESULT create_storage(IStorage **stg)
 {
-    const CLSID test_server_clsid = {0x0f77e570,0x80c3,0x11e2,{0x9e,0x96,0x08,0x00,0x20,0x0c,0x9a,0x66}};
+    HRESULT hr;
+    ILockBytes *lock_bytes;

-    IUnknown *unk;
+    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;
+}
+
+static LONG register_server(void)
+{
+    LONG ret;
+    HKEY root;
+    char server_path[MAX_PATH];
+
+    lstrcpyA(server_path, wine_argv[0]);
+    lstrcatA(server_path, " defaulthandler");
+
+    StringFromGUID2(&CLSID_Test_Server, &clsid_key[6], CHARS_IN_GUID);
+
+    ret = RegCreateKeyExW(HKEY_CLASSES_ROOT, clsid_key, 0, NULL, 0,
+                          KEY_READ | KEY_WRITE | KEY_CREATE_SUB_KEY, NULL, &root, NULL);
+    if (ret == ERROR_SUCCESS)
+    {
+        ret = RegSetValueA(root, "LocalServer32", REG_SZ, server_path, strlen(server_path));
+        if (ret != ERROR_SUCCESS)
+            RegDeleteKeyA(root, "LocalServer32");
+        RegCloseKey(root);
+    }
+
+    return ret;
+}
+
+static void unregister_server(void)
+{
+    HKEY root;
+    LONG ret;
+
+    StringFromGUID2(&CLSID_Test_Server, &clsid_key[6], CHARS_IN_GUID);
+
+    if (RegCreateKeyExW(HKEY_CLASSES_ROOT, clsid_key, 0, NULL, 0,
+        DELETE, NULL, &root, NULL) == ERROR_SUCCESS)
+    {
+        ret = RegDeleteKeyA(root, "LocalServer32");
+        ok(ret == ERROR_SUCCESS, "RegDeleteKey error %u\n", ret);
+        ret = RegDeleteKeyA(root, "");
+        ok(ret == ERROR_SUCCESS, "RegDeleteKey error %u\n", ret);
+        RegCloseKey(root);
+    }
+}
+
+static void run_local_server(void)
+{
+    DWORD class_reg;
+    HRESULT hres;
+    HANDLE finish_event;
+
+    hres = CoRegisterClassObject(&CLSID_Test_Server, (IUnknown*)&ClassFactory,
+                                 CLSCTX_LOCAL_SERVER, 0, &class_reg);
+    ok(hres == S_OK, "CoRegisterClassObject failed: %x\n", hres);
+
+    finish_event = CreateEventA(NULL, FALSE, FALSE, "Shutdown local server");
+    WaitForSingleObject(finish_event, INFINITE);
+    CloseHandle(finish_event);
+    CoRevokeClassObject(class_reg);
+}
+
+static void test_running_object(void)
+{
     IRunnableObject *ro;
     IOleObject *oleobj;
-    IPersistStorage *persist;
-    DWORD class_reg;
+    IStorage *stg;
     HRESULT hres;
-
-    if(!GetProcAddress(GetModuleHandleA("ole32"), "CoRegisterSurrogateEx")) {
-        win_skip("skipping OleCreateDefaultHandler tests\n");
-        return;
-    }
-
-    hres = CoRegisterClassObject(&test_server_clsid, (IUnknown*)&ClassFactory,
-            CLSCTX_INPROC_SERVER, 0, &class_reg);
-    ok(hres == S_OK, "CoRegisterClassObject failed: %x\n", hres);
-
-    hres = OleCreateDefaultHandler(&test_server_clsid, NULL, &IID_IUnknown, (void**)&unk);
-    ok(hres == S_OK, "OleCreateDefaultHandler failed: %x\n", hres);
-
-    hres = IUnknown_QueryInterface(unk, &IID_IRunnableObject, (void**)&ro);
+    HANDLE update_event, finish_event, cf_event;
+    IPersistStorage *persist;
+    IStream *stm;
+    static const WCHAR olestream[] = {1,'O','l','e',0};
+    ULONG read;
+    ole_stream_header_t header;
+
+    hres = IUnknown_QueryInterface(dfhandler_unk, &IID_IRunnableObject, (void**)&ro);
     ok(hres == S_OK, "QueryInterface(IRunnableObject) failed: %x\n", hres);
-    IUnknown_Release(unk);

+    hres = IRunnableObject_IsRunning(ro);
+    ok(hres == FALSE, "Object shouldn't be running\n");
+
+    /* set object running and expect delegated calls */
+    hres = IUnknown_QueryInterface(dfhandler_unk, &IID_IOleObject, (void**)&oleobj);
+    ok(hres == S_OK, "QueryInterface(IOleObject) failed: %x\n", hres);
+    update_event = CreateEventA(NULL, FALSE, FALSE, "IOleObject_Update call in local server");
+    cf_event = CreateEventA(NULL, FALSE, FALSE, "Local server's CF");
     hres = IRunnableObject_Run(ro, NULL);
-    ok(hres == REGDB_E_CLASSNOTREG, "Run returned: %x, expected REGDB_E_CLASSNOTREG\n", hres);
-    IRunnableObject_Release(ro);
+    todo_wine ok(hres == S_OK, "Failed to run object. Returned value was %x\n", hres);
+    hres = IRunnableObject_IsRunning(ro);
+    todo_wine ok(hres == TRUE, "Object should be running\n");

-    SET_EXPECT(CF_QueryInterface_IMarshal);
-    CoRevokeClassObject(class_reg);
-    todo_wine CHECK_CALLED(CF_QueryInterface_IMarshal);
+    SET_EXPECT(OleObject_Update);
+    hres = IOleObject_Update(oleobj);
+    todo_wine ok(hres == S_OK, "IOleObject_Update failed: %x\n", hres);
+    if (WaitForSingleObject(update_event, 3000) == WAIT_OBJECT_0)
+        CHECK_EXPECT2(OleObject_Update);
+    todo_wine CHECK_CALLED(OleObject_Update);
+    CloseHandle(update_event);

-    hres = CoRegisterClassObject(&test_server_clsid, (IUnknown*)&ClassFactory,
-            CLSCTX_LOCAL_SERVER, 0, &class_reg);
-    ok(hres == S_OK, "CoRegisterClassObject failed: %x\n", hres);
+    SET_EXPECT(CF_QueryInterface_ClassFactory);
+    if (WaitForSingleObject(cf_event, 3000) == WAIT_OBJECT_0)
+        CHECK_EXPECT2(CF_QueryInterface_ClassFactory);
+    CHECK_CALLED(CF_QueryInterface_ClassFactory);
+    CloseHandle(cf_event);

-    hres = OleCreateDefaultHandler(&test_server_clsid, NULL, &IID_IUnknown, (void**)&unk);
-    ok(hres == S_OK, "OleCreateDefaultHandler failed: %x\n", hres);
+    /* test delegation to object's IPersistStorage */
+    hres = create_storage(&stg);
+    ok(hres == S_OK, "got %08x\n", hres);

-    hres = IUnknown_QueryInterface(unk, &IID_IOleObject, (void**)&oleobj);
-    ok(hres == S_OK, "QueryInterface(IID_IOleObject) failed: %x\n", hres);
+    hres = IStorage_OpenStream(stg, olestream, NULL, STGM_SHARE_EXCLUSIVE | STGM_READ, 0, &stm);
+    ok(hres == STG_E_FILENOTFOUND, "got %08x\n", hres);

hres = IOleObject_QueryInterface(oleobj, &IID_IPersistStorage, (void**)&persist);
-    ok(hres == S_OK, "QueryInterface(IID_IPersistStorage) failed: %x\n", hres);
+    ok(hres == S_OK, "got %08x\n", hres);
+
+    update_event = CreateEventA(NULL, FALSE, FALSE, "IPersistStorage_InitNew call in local server");
+    SET_EXPECT(PStorage_InitNew);
+    hres = IPersistStorage_InitNew(persist, stg);
+    ok(hres == S_OK, "got %08x\n", hres);
+    if (WaitForSingleObject(update_event, 3000) == WAIT_OBJECT_0)
+        CHECK_EXPECT2(PStorage_InitNew);
+    todo_wine CHECK_CALLED(PStorage_InitNew);
+    CloseHandle(update_event);
+
+    hres = IStorage_OpenStream(stg, olestream, NULL, STGM_SHARE_EXCLUSIVE | STGM_READ, 0, &stm);
+    ok(hres == S_OK, "got %08x\n", hres);
+    hres = IStream_Read(stm, &header, sizeof(header), &read);
+    ok(hres == S_OK, "got %08x\n", hres);
+    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(oleobj);
-
-    hres = IUnknown_QueryInterface(unk, &IID_IRunnableObject, (void**)&ro);
-    ok(hres == S_OK, "QueryInterface(IRunnableObject) failed: %x\n", hres);
-    IUnknown_Release(unk);
-
-    SET_EXPECT(CF_QueryInterface_ClassFactory);
-    SET_EXPECT(CF_CreateInstance);
-    hres = IRunnableObject_Run(ro, NULL);
-todo_wine
-    ok(hres == S_OK, "Run failed: %x\n", hres);
-    CHECK_CALLED(CF_QueryInterface_ClassFactory);
-    CHECK_CALLED(CF_CreateInstance);
     IRunnableObject_Release(ro);
+    IStorage_Release(stg);

-    SET_EXPECT(CF_QueryInterface_ClassFactory);
-    SET_EXPECT(CF_CreateInstance);
-    hres = CoCreateInstance(&test_server_clsid, NULL, CLSCTX_LOCAL_SERVER,
-                            &IID_IOleObject, (void**)&oleobj);
-todo_wine
-    ok(hres == REGDB_E_CLASSNOTREG, "expected REGDB_E_CLASSNOTREG, got %x\n", hres);
-todo_wine
-    CHECK_NOT_CALLED(CF_QueryInterface_ClassFactory);
-todo_wine
-    CHECK_NOT_CALLED(CF_CreateInstance);
-
-    SET_EXPECT(CF_QueryInterface_IMarshal);
-    CoRevokeClassObject(class_reg);
-    todo_wine CHECK_CALLED(CF_QueryInterface_IMarshal);
+    finish_event = CreateEventA(NULL, FALSE, FALSE, "Shutdown local server");
+    SetEvent(finish_event);
+    CloseHandle(finish_event);
 }

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();
+    /*
+     * TODO:
+     * - Test aggregating default handler from a custom handler.
+     * - Test delegation to datacache.
+     * - Test handler for non-running object. Some functions must
+     *   save information for later delegation to object.
+     */
+
+    hres = IUnknown_Release(dfhandler_unk);
+    ok(hres == S_OK, "Failed to release default handler's IUnknown: %x\n", hres);
+
+fail:
+    unregister_server();
     OleUninitialize();
 }

[Huw Davies]

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
>
> 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.

[Sergio Gómez Del Real]

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
> >
> > 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

[Huw Davies]

On Wed, May 09, 2018 at 02:28:42PM -0500, Sergio Gómez Del Real wrote:
> On 08/05/18 03:07, Huw Davies wrote:
> > 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.
>
> 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.

While you may think globals are convenient they can become very
inconvenient when it comes to understanding the code.  If the globals
are beening accessed by a lot of functions, then sure they're fine (in
tests).  In the case of dfhandler_unk, why not create it locally in
test_running_object()?  You can recreate it in other test functions as
needed - one of the points about the default handler is that it
shouldn't be too expensive to create.  If you really want to keep it
alive across different test functions (remembering that this will make
its state much harder to follow) then pass it as a parameter to those
functions.

Huw.