[PATCH v3 0/2] MR8697: combase: Don't marshal objects marked as IAgileObject in RoGetAgileReference.
[Chromium obtains an agile reference](https://source.chromium.org/chromium/chromium/src/+/main:device/bluetooth/bl...) to WinRT activation factories with RoAgileReference. As we currently do not support marshaling WinRT objects, this fails. However, as objects marked with IAgileObject don't need to be marshaled across apartments, we can trivially implement `RoAgileReference` for such objects, as most activation factories tend to be agile. -- v3: combase: Don't marshal objects that implement IAgileObject in RoGetAgileReference. https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
From: Vibhav Pant <vibhavp(a)gmail.com> Objects that implement IAgileObject don't need to be marshaled across apartment boundaries. --- dlls/combase/tests/roapi.c | 64 +++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index ab5dbff6017..fb40dc60f5a 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -374,6 +374,19 @@ static HRESULT WINAPI unk_no_marshal_QueryInterface(IUnknown *iface, REFIID riid return E_NOINTERFACE; } +static HRESULT WINAPI unk_agile_QueryInterface(IUnknown *iface, REFIID riid, void **ppv) +{ + if (IsEqualGUID(riid, &IID_IUnknown) + || IsEqualGUID(riid, &IID_IAgileObject)) + { + *ppv = iface; + IUnknown_AddRef(iface); + return S_OK; + } + + return E_NOINTERFACE; +} + static ULONG WINAPI unk_AddRef(IUnknown *iface) { struct unk_impl *impl = impl_from_IUnknown(iface); @@ -400,6 +413,13 @@ static const IUnknownVtbl unk_no_marshal_vtbl = unk_Release }; +static const IUnknownVtbl unk_agile_vtbl = +{ + unk_agile_QueryInterface, + unk_AddRef, + unk_Release +}; + struct test_RoGetAgileReference_thread_param { enum AgileReferenceOptions option; @@ -407,6 +427,7 @@ struct test_RoGetAgileReference_thread_param RO_INIT_TYPE to_type; IAgileReference *agile_reference; IUnknown *unk_obj; + BOOLEAN obj_is_agile; }; static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) @@ -423,7 +444,14 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) hr = IAgileReference_Resolve(param->agile_reference, &IID_IUnknown, (void **)&unknown); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!unknown, "Expected pointer not NULL.\n"); - if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) + if (param->obj_is_agile) + { + todo_wine_if(param->from_type == RO_INIT_SINGLETHREADED || param->to_type == RO_INIT_SINGLETHREADED) + ok(unknown == param->unk_obj, "Expected the same object.\n"); + todo_wine_if(param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) + EXPECT_REF(param->unk_obj, 4); + } + else if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) { ok(unknown == param->unk_obj, "Expected the same object.\n"); todo_wine_if(param->option == AGILEREFERENCE_DEFAULT) @@ -448,6 +476,7 @@ static void test_RoGetAgileReference(void) struct test_RoGetAgileReference_thread_param param; struct unk_impl unk_no_marshal_obj = {{&unk_no_marshal_vtbl}, 1}; struct unk_impl unk_obj = {{&unk_vtbl}, 1}; + struct unk_impl unk_agile_obj = {{&unk_agile_vtbl}, 1}; enum AgileReferenceOptions option; IAgileReference *agile_reference; RO_INIT_TYPE from_type, to_type; @@ -521,6 +550,7 @@ static void test_RoGetAgileReference(void) param.to_type = to_type; param.agile_reference = agile_reference; param.unk_obj = &unk_obj.IUnknown_iface; + param.obj_is_agile = FALSE; thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); flush_events(); ret = WaitForSingleObject(thread, 100); @@ -532,6 +562,38 @@ static void test_RoGetAgileReference(void) IAgileReference_Release(agile_reference); EXPECT_REF(&unk_obj, 1); + hr = RoGetAgileReference(option, &IID_IUnknown, &unk_agile_obj.IUnknown_iface, &agile_reference); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!!agile_reference, "Got unexpected agile_reference.\n"); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + EXPECT_REF(&unk_agile_obj, 2); + + unknown = NULL; + hr = IAgileReference_Resolve(agile_reference, &IID_IUnknown, (void **)&unknown); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!!unknown, "Expected pointer not NULL.\n"); + ok(unknown == &unk_agile_obj.IUnknown_iface, "Expected the same object.\n"); + todo_wine + EXPECT_REF(&unk_agile_obj, 3); + + for (to_type = RO_INIT_SINGLETHREADED; to_type <= RO_INIT_MULTITHREADED; to_type++) + { + param.option = option; + param.from_type = from_type; + param.to_type = to_type; + param.agile_reference = agile_reference; + param.unk_obj = &unk_agile_obj.IUnknown_iface; + param.obj_is_agile = TRUE; + thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); + flush_events(); + ret = WaitForSingleObject(thread, 100); + ok(!ret, "WaitForSingleObject failed, error %ld.\n", GetLastError()); + } + + IUnknown_Release(unknown); + IAgileReference_Release(agile_reference); + EXPECT_REF(&unk_obj, 1); + RoUninitialize(); winetest_pop_context(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/combase/roapi.c | 40 +++++++++++++++++++++++--------------- dlls/combase/tests/roapi.c | 4 ---- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c index df3a3c5c176..e1216dbb462 100644 --- a/dlls/combase/roapi.c +++ b/dlls/combase/roapi.c @@ -245,6 +245,7 @@ struct agile_reference IStream *marshal_stream; CRITICAL_SECTION cs; IUnknown *obj; + BOOLEAN is_agile; LONG ref; }; @@ -331,6 +332,9 @@ static HRESULT WINAPI agile_ref_Resolve(IAgileReference *iface, REFIID riid, voi TRACE("(%p, %s, %p)\n", iface, debugstr_guid(riid), obj); + if (impl->is_agile) + return IUnknown_QueryInterface(impl->obj, riid, obj); + EnterCriticalSection(&impl->cs); if (impl->option == AGILEREFERENCE_DELAYEDMARSHAL && impl->marshal_stream == NULL) { @@ -359,6 +363,17 @@ static const IAgileReferenceVtbl agile_ref_vtbl = agile_ref_Resolve, }; +static BOOL object_has_interface(IUnknown *obj, REFIID iid) +{ + IUnknown *unk; + HRESULT hr; + + hr = IUnknown_QueryInterface(obj, iid, (void **)&unk); + if (SUCCEEDED(hr)) + IUnknown_Release(unk); + return SUCCEEDED(hr); +} + /*********************************************************************** * RoGetAgileReference (combase.@) */ @@ -366,7 +381,6 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii IAgileReference **agile_reference) { struct agile_reference *impl; - IUnknown *unknown; HRESULT hr; TRACE("(%d, %s, %p, %p).\n", option, debugstr_guid(riid), obj, agile_reference); @@ -380,17 +394,10 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii return CO_E_NOTINITIALIZED; } - hr = IUnknown_QueryInterface(obj, riid, (void **)&unknown); - if (FAILED(hr)) + if (!object_has_interface(obj, riid)) return E_NOINTERFACE; - IUnknown_Release(unknown); - - hr = IUnknown_QueryInterface(obj, &IID_INoMarshal, (void **)&unknown); - if (SUCCEEDED(hr)) - { - IUnknown_Release(unknown); + if (object_has_interface(obj, &IID_INoMarshal)) return CO_E_NOT_SUPPORTED; - } impl = calloc(1, sizeof(*impl)); if (!impl) @@ -398,9 +405,15 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii impl->IAgileReference_iface.lpVtbl = &agile_ref_vtbl; impl->option = option; + impl->is_agile = object_has_interface(obj, &IID_IAgileObject); impl->ref = 1; - if (option == AGILEREFERENCE_DEFAULT) + if (option == AGILEREFERENCE_DELAYEDMARSHAL || impl->is_agile) + { + impl->obj = obj; + IUnknown_AddRef(impl->obj); + } + else if (option == AGILEREFERENCE_DEFAULT) { if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, obj))) { @@ -408,11 +421,6 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii return hr; } } - else if (option == AGILEREFERENCE_DELAYEDMARSHAL) - { - impl->obj = obj; - IUnknown_AddRef(impl->obj); - } InitializeCriticalSection(&impl->cs); diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index fb40dc60f5a..91421605a75 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -446,9 +446,7 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) ok(!!unknown, "Expected pointer not NULL.\n"); if (param->obj_is_agile) { - todo_wine_if(param->from_type == RO_INIT_SINGLETHREADED || param->to_type == RO_INIT_SINGLETHREADED) ok(unknown == param->unk_obj, "Expected the same object.\n"); - todo_wine_if(param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) EXPECT_REF(param->unk_obj, 4); } else if (param->from_type == RO_INIT_MULTITHREADED && param->to_type == RO_INIT_MULTITHREADED) @@ -565,7 +563,6 @@ static void test_RoGetAgileReference(void) hr = RoGetAgileReference(option, &IID_IUnknown, &unk_agile_obj.IUnknown_iface, &agile_reference); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!agile_reference, "Got unexpected agile_reference.\n"); - todo_wine_if(option == AGILEREFERENCE_DEFAULT) EXPECT_REF(&unk_agile_obj, 2); unknown = NULL; @@ -573,7 +570,6 @@ static void test_RoGetAgileReference(void) ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!unknown, "Expected pointer not NULL.\n"); ok(unknown == &unk_agile_obj.IUnknown_iface, "Expected the same object.\n"); - todo_wine EXPECT_REF(&unk_agile_obj, 3); for (to_type = RO_INIT_SINGLETHREADED; to_type <= RO_INIT_MULTITHREADED; to_type++) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
v3: * Remove tests for calling RoGetAgileReference with WinRT interfaces. * Simplify interface detection code in RoGetAgileReference. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_113889
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
participants (4)
-
Huw Davies (＠huw) -
Nikolay Sivov (＠nsivov)
-
Vibhav Pant
-
Vibhav Pant (＠vibhavp)