[PATCH v2 1/3] uiautomationcore: Implement UiaGetReservedNotSupportedValue.

[Nikolay Sivov]

On 10/28/21 6:17 PM, Connor McAdams wrote:
> Signed-off-by: Connor McAdams <cmcadams at 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.

> +
> +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.
> +
> +    *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.
> +
> +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;
>  }
>