[PATCH 0/3] 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. -- 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/tests/roapi.c | 66 +++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index fb40dc60f5a..a3337ed9b08 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -27,6 +27,9 @@ #include "roapi.h" #include "roerrorapi.h" +#define WIDL_using_Windows_Foundation_Metadata +#include "windows.foundation.metadata.h" + #include "wine/test.h" #define EXPECT_REF(obj,ref) _expect_ref((IUnknown*)obj, ref, __LINE__) @@ -427,6 +430,7 @@ struct test_RoGetAgileReference_thread_param RO_INIT_TYPE to_type; IAgileReference *agile_reference; IUnknown *unk_obj; + const GUID *iid; BOOLEAN obj_is_agile; }; @@ -441,7 +445,7 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) RoInitialize(param->to_type); unknown = NULL; - hr = IAgileReference_Resolve(param->agile_reference, &IID_IUnknown, (void **)&unknown); + hr = IAgileReference_Resolve(param->agile_reference, param->iid, (void **)&unknown); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ok(!!unknown, "Expected pointer not NULL.\n"); if (param->obj_is_agile) @@ -473,19 +477,26 @@ static DWORD CALLBACK test_RoGetAgileReference_thread_proc(void *arg) static void test_RoGetAgileReference(void) { + static const WCHAR *class_name = RuntimeClass_Windows_Foundation_Metadata_ApiInformation; 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}; + IApiInformationStatics *statics, *statics2; enum AgileReferenceOptions option; IAgileReference *agile_reference; RO_INIT_TYPE from_type, to_type; IUnknown *unknown, *unknown2; IAgileObject *agile_object; HANDLE thread; + HSTRING str; HRESULT hr; DWORD ret; + /* A known agile ActivationFactory */ + hr = WindowsCreateString(class_name, wcslen(class_name), &str); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + for (option = AGILEREFERENCE_DEFAULT; option <= AGILEREFERENCE_DELAYEDMARSHAL; option++) { for (from_type = RO_INIT_SINGLETHREADED; from_type <= RO_INIT_MULTITHREADED; from_type++) @@ -551,6 +562,7 @@ static void test_RoGetAgileReference(void) param.agile_reference = agile_reference; param.unk_obj = &unk_obj.IUnknown_iface; param.obj_is_agile = FALSE; + param.iid = &IID_IUnknown; thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); flush_events(); ret = WaitForSingleObject(thread, 100); @@ -583,6 +595,7 @@ static void test_RoGetAgileReference(void) param.to_type = to_type; param.agile_reference = agile_reference; param.unk_obj = &unk_agile_obj.IUnknown_iface; + param.iid = &IID_IUnknown; param.obj_is_agile = TRUE; thread = CreateThread(NULL, 0, test_RoGetAgileReference_thread_proc, ¶m, 0, NULL); flush_events(); @@ -594,10 +607,61 @@ static void test_RoGetAgileReference(void) IAgileReference_Release(agile_reference); EXPECT_REF(&unk_obj, 1); + /* Test marshaling WinRT interfaces. */ + hr = RoGetActivationFactory(str, &IID_IApiInformationStatics, (void **)&statics); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + agile_reference = NULL; + hr = RoGetAgileReference(option, &IID_IApiInformationStatics, (IUnknown *)statics, &agile_reference); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + ok(!!agile_reference, "Got unexpected agile_reference.\n"); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + EXPECT_REF(statics, 3); + + if (SUCCEEDED(hr)) + { + statics2 = NULL; + hr = IAgileReference_Resolve(agile_reference, &IID_IApiInformationStatics, (void **)&statics2); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine + ok(!!statics2, "Expected pointer not NULL.\n"); + todo_wine + ok(statics2 == statics, "Expected the same object.\n"); + todo_wine + EXPECT_REF(statics, 4); + + if (SUCCEEDED(hr)) + { + IApiInformationStatics_Release( statics2 ); + 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 = (IUnknown *)statics; + param.obj_is_agile = TRUE; + param.iid = &IID_IApiInformationStatics; + 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()); + } + } + IAgileReference_Release( agile_reference ); + } + EXPECT_REF(statics, 2); + IApiInformationStatics_Release( statics ); + + RoUninitialize(); winetest_pop_context(); } } + + WindowsDeleteString(str); } static void test_RoGetErrorReportingFlags(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/combase/roapi.c | 12 +++++++++++- dlls/combase/tests/roapi.c | 11 ----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c index df3a3c5c176..7c15dc5d148 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) { @@ -400,7 +404,13 @@ HRESULT WINAPI RoGetAgileReference(enum AgileReferenceOptions option, REFIID rii impl->option = option; impl->ref = 1; - if (option == AGILEREFERENCE_DEFAULT) + hr = IUnknown_QueryInterface(obj, &IID_IAgileObject, (void **)&unknown); + if (SUCCEEDED(hr)) + { + impl->obj = obj; + impl->is_agile = TRUE; + } + else if (option == AGILEREFERENCE_DEFAULT) { if (FAILED(hr = marshal_object_in_agile_reference(impl, riid, obj))) { diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index a3337ed9b08..02284ab6cfc 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -450,9 +450,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) @@ -577,7 +575,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; @@ -585,7 +582,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++) @@ -612,24 +608,17 @@ static void test_RoGetAgileReference(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); agile_reference = NULL; hr = RoGetAgileReference(option, &IID_IApiInformationStatics, (IUnknown *)statics, &agile_reference); - todo_wine_if(option == AGILEREFERENCE_DEFAULT) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine_if(option == AGILEREFERENCE_DEFAULT) ok(!!agile_reference, "Got unexpected agile_reference.\n"); - todo_wine_if(option == AGILEREFERENCE_DEFAULT) EXPECT_REF(statics, 3); if (SUCCEEDED(hr)) { statics2 = NULL; hr = IAgileReference_Resolve(agile_reference, &IID_IApiInformationStatics, (void **)&statics2); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - todo_wine ok(!!statics2, "Expected pointer not NULL.\n"); - todo_wine ok(statics2 == statics, "Expected the same object.\n"); - todo_wine EXPECT_REF(statics, 4); if (SUCCEEDED(hr)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8697
Nikolay Sivov (@nsivov) commented about dlls/combase/tests/roapi.c:
IAgileReference_Release(agile_reference); EXPECT_REF(&unk_obj, 1);
+ /* Test marshaling WinRT interfaces. */ + hr = RoGetActivationFactory(str, &IID_IApiInformationStatics, (void **)&statics); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + agile_reference = NULL; + hr = RoGetAgileReference(option, &IID_IApiInformationStatics, (IUnknown *)statics, &agile_reference); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + ok(!!agile_reference, "Got unexpected agile_reference.\n"); + todo_wine_if(option == AGILEREFERENCE_DEFAULT) + EXPECT_REF(statics, 3);
Why do you need to use some winrt object to demonstrate this? Does this fail because CoMarshalInterface() fails for IApiInformationStatics ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_111867
On Fri Aug 1 19:46:49 2025 +0000, Nikolay Sivov wrote:
Why do you need to use some winrt object to demonstrate this? Does this fail because CoMarshalInterface() fails for IApiInformationStatics ? Yes, we currently dont support marshaling WinRT types, so I used this as an example. At the same time, since IApiInformationStatics also supports IAgileObject, it also doesn't necessarily need to be marshaled.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_111871
On Fri Aug 1 20:38:03 2025 +0000, Vibhav Pant wrote:
Yes, we currently dont support marshaling WinRT types, so I used this as an example. At the same time, since IApiInformationStatics also supports IAgileObject, it also doesn't necessarily need to be marshaled. Ok, what do we need to have support for marshalling winrt types? Is it different from non-winrt types? (what are those types by the way)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_111872
On Fri Aug 1 20:40:52 2025 +0000, Nikolay Sivov wrote:
Ok, what do we need to have support for marshalling winrt types? Is it different from non-winrt types? (what are those types by the way) I believe it would involve adding support for generating proxy code for parameterized types to widl, and implement the HSTRING_User\* methods, for starters. That would allow us to register the generated PSFactoryBuffer class for standard marshalling.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_111873
Nikolay Sivov (@nsivov) commented about dlls/combase/roapi.c:
impl->option = option; impl->ref = 1;
- if (option == AGILEREFERENCE_DEFAULT) + hr = IUnknown_QueryInterface(obj, &IID_IAgileObject, (void **)&unknown); + if (SUCCEEDED(hr)) + { + impl->obj = obj; + impl->is_agile = TRUE; + }
I would rather combine this with AGILEREFERENCE_DELAYEDMARSHAL block, using additional helper like "bool is_object_agile()". Right now you need to look twice to see why "unknown" is not released. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_113708
Nikolay Sivov (@nsivov) commented about dlls/combase/tests/roapi.c:
IAgileReference_Release(agile_reference); EXPECT_REF(&unk_obj, 1);
+ /* Test marshaling WinRT interfaces. */ + hr = RoGetActivationFactory(str, &IID_IApiInformationStatics, (void **)&statics);
I don't think we need this test, there is nothing special about those interfaces. If IAgileObject is supported, the rest is the same for any kind of objects. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8697#note_113709
participants (3)
-
Nikolay Sivov (＠nsivov) -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)