On Thu, Oct 28, 2021 at 09:02:40PM +0300, Nikolay Sivov wrote:
On 10/28/21 6:17 PM, Connor McAdams wrote:
Signed-off-by: Connor McAdams cmcadams@codeweavers.com
dlls/uiautomationcore/Makefile.in | 1 + dlls/uiautomationcore/uia_main.c | 147 +++++++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 2 deletions(-)
diff --git a/dlls/uiautomationcore/Makefile.in b/dlls/uiautomationcore/Makefile.in index 71ea7b99c94..5a72ea144c4 100644 --- a/dlls/uiautomationcore/Makefile.in +++ b/dlls/uiautomationcore/Makefile.in @@ -1,5 +1,6 @@ MODULE = uiautomationcore.dll IMPORTLIB = uiautomationcore +IMPORTS = uuid ole32
EXTRADLLFLAGS = -Wb,--prefer-native
diff --git a/dlls/uiautomationcore/uia_main.c b/dlls/uiautomationcore/uia_main.c index 2dada95af80..51c220c6041 100644 --- a/dlls/uiautomationcore/uia_main.c +++ b/dlls/uiautomationcore/uia_main.c @@ -16,12 +16,150 @@
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
*/
+#define COBJMACROS
#include "uiautomation.h"
#include "wine/debug.h" +#include "wine/heap.h"
WINE_DEFAULT_DEBUG_CHANNEL(uiautomation);
+struct uia_rsrv_ftmarshaler +{
- IUnknown IUnknown_iface;
- LONG refcount;
- IUnknown *inner;
- IUnknown *rsrv_val;
+};
I think naming is confusing. 'inner' here really means marshaler, rsrv_val is the object marshaler is associated with, and the whole thing is not an ftmarshaler, but something like a host or a wrapper.
Yeah, I agree the naming is a bit confusing. I was having trouble with that, but I'll use the name you've suggested in the next version.
+static struct uia_rsrv_ftmarshaler *uia_rsrv_ns_val_ftmarshaler = NULL;
I thought you said that new instances are created every time?
+static struct uia_rsrv_ftmarshaler *impl_uia_rsrv_ftmarshaler_from_IUnknown(IUnknown *iface) +{
- return CONTAINING_RECORD(iface, struct uia_rsrv_ftmarshaler, IUnknown_iface);
+}
+static HRESULT WINAPI uia_rsrv_ftmarshaler_QueryInterface(IUnknown *iface,
REFIID riid, void **ppv)+{
- struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ftmarshaler_from_IUnknown(iface);
- return IUnknown_QueryInterface(marshaler->rsrv_val, riid, ppv);
+}
+static ULONG WINAPI uia_rsrv_ftmarshaler_AddRef(IUnknown *iface) +{
- struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ftmarshaler_from_IUnknown(iface);
- ULONG refcount = InterlockedIncrement(&marshaler->refcount);
- TRACE("%p, refcount %d\n", iface, refcount);
- return refcount;
+}
+static ULONG WINAPI uia_rsrv_ftmarshaler_Release(IUnknown *iface) +{
- struct uia_rsrv_ftmarshaler *marshaler = impl_uia_rsrv_ftmarshaler_from_IUnknown(iface);
- ULONG refcount = InterlockedDecrement(&marshaler->refcount);
- TRACE("%p, refcount %d\n", iface, refcount);
- if (!refcount)
- {
if (marshaler == uia_rsrv_ns_val_ftmarshaler)uia_rsrv_ns_val_ftmarshaler = NULL;IUnknown_Release(marshaler->inner);heap_free(marshaler);- }
- return refcount;
+}
+static const IUnknownVtbl uia_rsrv_ftmarshaler_vtbl = {
- uia_rsrv_ftmarshaler_QueryInterface,
- uia_rsrv_ftmarshaler_AddRef,
- uia_rsrv_ftmarshaler_Release,
+};
+/*
- When passing the ReservedNotSupportedValue/ReservedMixedAttributeValue
- interface pointers across apartments within the same process, create a free
- threaded marshaler so that the pointer value is preserved.
- */
+static HRESULT uia_rsrv_val_create_ftmarshaler(IUnknown *reserved, struct uia_rsrv_ftmarshaler **marshaler) +{
- struct uia_rsrv_ftmarshaler *object;
- HRESULT hr;
- TRACE("%p, %p\n", reserved, marshaler);
- object = heap_alloc(sizeof(*object));
- if (!object)
return E_OUTOFMEMORY;- object->IUnknown_iface.lpVtbl = &uia_rsrv_ftmarshaler_vtbl;
- object->refcount = 1;
- hr = CoCreateFreeThreadedMarshaler(&object->IUnknown_iface, &object->inner);
- if (FAILED(hr = CoCreateFreeThreadedMarshaler(&object->IUnknown_iface, &object->inner)))
- {
heap_free(object);return hr;- }
This one is obviously copy-paste typo.
Oof, yeah, good catch.
- *marshaler = object;
- return hr;
+}
+/*
- UiaReservedNotSupported object.
- */
+static HRESULT WINAPI uia_reserved_ns_QueryInterface(IUnknown *iface,
REFIID riid, void **ppv)+{
- *ppv = NULL;
- if (IsEqualIID(riid, &IID_IUnknown))
*ppv = iface;- else if (IsEqualIID(riid, &IID_IMarshal))
- {
if (!uia_rsrv_ns_val_ftmarshaler){if (FAILED(uia_rsrv_val_create_ftmarshaler(iface, &uia_rsrv_ns_val_ftmarshaler)))return E_NOINTERFACE;}if (FAILED(IUnknown_QueryInterface(uia_rsrv_ns_val_ftmarshaler->inner, riid, ppv)))return E_NOINTERFACE;IUnknown_Release(&uia_rsrv_ns_val_ftmarshaler->IUnknown_iface);- }
- else
return E_NOINTERFACE;- return S_OK;
+}
It's not safe to handle shared pointer like that, when you know you're going to run in multithreaded environment.
I think creating that helper wrapper/host object, and then creating marshaler on every call should be fine.
Ah, okay. In our last discussion you had asked if it was really that important that QI returns a new instance each time, so I assumed you wanted a shared instance across each call. I can go back to creating a unique one each time though like I was doing before.
+static ULONG WINAPI uia_reserved_ns_AddRef(IUnknown *iface) +{
- return 1;
+}
+static ULONG WINAPI uia_reserved_ns_Release(IUnknown *iface) +{
- return 1;
+}
That probably doesn't matter, but I'll mentioned it anyway, occasionally for such singleton objects they still maintain refcount field, that doesn't do anything, expect changing its value.
+static const IUnknownVtbl uia_reserved_ns_vtbl = {
- uia_reserved_ns_QueryInterface,
- uia_reserved_ns_AddRef,
- uia_reserved_ns_Release,
+};
+static IUnknown uia_reserved_ns_iface = {&uia_reserved_ns_vtbl};
/***********************************************************************
UiaClientsAreListening (uiautomationcore.@)*/ @@ -46,8 +184,13 @@ HRESULT WINAPI UiaGetReservedMixedAttributeValue(IUnknown **value) */ HRESULT WINAPI UiaGetReservedNotSupportedValue(IUnknown **value) {
- FIXME("(%p) stub!\n", value);
- *value = NULL;
- TRACE("(%p)\n", value);
- if (!value)
return E_INVALIDARG;- *value = &uia_reserved_ns_iface;
- return S_OK;
}
Thanks for the review.