[PATCH v4 2/4] windows.media.speech: Partially implement IAsyncOperation.
Rémi Bernon
rbernon at codeweavers.com
Tue Apr 19 07:54:40 CDT 2022
On 4/18/22 23:42, Bernhard Kölbl wrote:
> Signed-off-by: Bernhard Kölbl <besentv at gmail.com>
> ---
> dlls/windows.media.speech/async.c | 142 ++++++++++++++++++++---
> dlls/windows.media.speech/tests/speech.c | 88 +++++++-------
> 2 files changed, 172 insertions(+), 58 deletions(-)
>
> diff --git a/dlls/windows.media.speech/async.c b/dlls/windows.media.speech/async.c
> index 94d0b35d9c1..cf0da23e493 100644
> --- a/dlls/windows.media.speech/async.c
> +++ b/dlls/windows.media.speech/async.c
> @@ -23,6 +23,9 @@
>
> WINE_DEFAULT_DEBUG_CHANNEL(speech);
>
> +#define Closed 4
> +#define HANDLER_NOT_SET ((void *)~(ULONG_PTR)0)
> +
> /*
> *
> * IAsyncOperation<IInspectable*>
> @@ -35,6 +38,11 @@ struct async_operation
> IAsyncInfo IAsyncInfo_iface;
> const GUID *iid;
> LONG ref;
> +
> + IAsyncOperationCompletedHandler_IInspectable *handler;
> +
> + AsyncStatus status;
> + HRESULT hr;
> };
>
> static inline struct async_operation *impl_from_IAsyncOperation_IInspectable(IAsyncOperation_IInspectable *iface)
> @@ -84,7 +92,14 @@ static ULONG WINAPI async_operation_Release( IAsyncOperation_IInspectable *iface
> TRACE("iface %p, ref %lu.\n", iface, ref);
>
> if (!ref)
> + {
> + IAsyncInfo_Close(&impl->IAsyncInfo_iface);
> +
> + if (impl->handler && impl->handler != HANDLER_NOT_SET)
> + IAsyncOperationCompletedHandler_IInspectable_Release(impl->handler);
> +
> free(impl);
> + }
>
> return ref;
> }
> @@ -110,21 +125,79 @@ static HRESULT WINAPI async_operation_GetTrustLevel( IAsyncOperation_IInspectabl
> static HRESULT WINAPI async_operation_put_Completed( IAsyncOperation_IInspectable *iface,
> IAsyncOperationCompletedHandler_IInspectable *handler )
> {
> - FIXME("iface %p, handler %p stub!\n", iface, handler);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncOperation_IInspectable(iface);
> + HRESULT hr;
> +
> + TRACE("iface %p, handler %p.\n", iface, handler);
> +
> + if (impl->handler == HANDLER_NOT_SET)
> + {
> + /*
> + impl->hanlder can only be set once with async_operation_put_Completed,
> + so by default we set a non HANDLER_NOT_SET value, in this case NULL.
> + */
> + impl->handler = NULL;
> +
> + if (handler)
> + {
> + if (impl->status != Started)
> + {
> + IAsyncOperation_IInspectable *operation = &impl->IAsyncOperation_IInspectable_iface;
> + AsyncStatus status = impl->status;
> +
> + IAsyncOperation_IInspectable_AddRef(operation);
> + IAsyncOperationCompletedHandler_IInspectable_AddRef(handler);
> +
> + IAsyncOperationCompletedHandler_IInspectable_Invoke(handler, operation, status);
> +
> + IAsyncOperationCompletedHandler_IInspectable_Release(handler);
> + IAsyncOperation_IInspectable_Release(operation);
> + }
> + }
> +
> + return S_OK;
> + }
> + else if (impl->status == Closed)
> + hr = E_ILLEGAL_METHOD_CALL;
> + else if (impl->handler != HANDLER_NOT_SET)
> + hr = E_ILLEGAL_DELEGATE_ASSIGNMENT;
> + else
> + hr = E_UNEXPECTED;
> +
> + return hr;
> }
I think the last branch cannot be taken. Then I think that something
like that would be clearer:
> if (impl->status == Closed)
> hr = E_ILLEGAL_METHOD_CALL;
> else if (impl->handler != HANDLER_NOT_SET)
> hr = E_ILLEGAL_DELEGATE_ASSIGNMENT;
> else if ((impl->handler = handler))
> {
> IAsyncOperationCompletedHandler_IInspectable_AddRef(impl->handler);
> if (impl->status > Started)
> {
> IAsyncOperation_IInspectable *operation = &impl->IAsyncOperation_IInspectable_iface;
> AsyncStatus status = impl->status;
> impl->handler = NULL; /* prevent concurrent Invoke */
>
> IAsyncOperationCompletedHandler_IInspectable_Invoke(handler, operation, status);
> IAsyncOperationCompletedHandler_IInspectable_Release(handler);
>
> return S_OK;
> }
> }
I don't think you need to add a reference on "operation" around the call
to Invoke.
>
> static HRESULT WINAPI async_operation_get_Completed( IAsyncOperation_IInspectable *iface,
> IAsyncOperationCompletedHandler_IInspectable **handler )
> {
> - FIXME("iface %p, handler %p stub!\n", iface, handler);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncOperation_IInspectable(iface);
> + HRESULT hr;
> +
> + FIXME("iface %p, handler %p semi stub!\n", iface, handler);
> +
> + if (impl->handler != HANDLER_NOT_SET && impl->status == Closed)
> + hr = E_ILLEGAL_METHOD_CALL;
> + else
> + hr = S_OK;
> +
> + *handler = NULL; /* Hanlder *seems* to always be NULL from the tests. */
> + return hr;
> }
>
> static HRESULT WINAPI async_operation_GetResults( IAsyncOperation_IInspectable *iface, IInspectable **results )
> {
> - FIXME("iface %p, results %p stub!\n", iface, results);
> - return E_NOTIMPL;
> + /* NOTE: Despite the name, this function only returns one result! */
> + struct async_operation *impl = impl_from_IAsyncOperation_IInspectable(iface);
> + HRESULT hr;
> +
> + TRACE("iface %p, results %p.\n", iface, results);
> +
> + if (impl->status == Closed)
> + hr = E_ILLEGAL_METHOD_CALL;
> + else
> + hr = S_OK;
> +
> + return hr;
> }
Does it really succeed if status is Started? I think that the check is
more likely to be if (status != Completed) instead.
>
> static const struct IAsyncOperation_IInspectableVtbl async_operation_vtbl =
> @@ -159,26 +232,53 @@ static HRESULT WINAPI async_operation_info_get_Id( IAsyncInfo *iface, UINT32 *id
>
> static HRESULT WINAPI async_operation_info_get_Status( IAsyncInfo *iface, AsyncStatus *status )
> {
> - FIXME("iface %p, status %p stub!\n", iface, status);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncInfo(iface);
> + HRESULT hr;
> +
> + TRACE("iface %p, status %p.\n", iface, status);
> +
> + if (impl->status == Closed)
> + hr = E_ILLEGAL_METHOD_CALL;
> + else
> + hr = S_OK;
> +
> + *status = impl->status;
> + return hr;
> }
>
> static HRESULT WINAPI async_operation_info_get_ErrorCode( IAsyncInfo *iface, HRESULT *error_code )
> {
> - FIXME("iface %p, error_code %p stub!\n", iface, error_code);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncInfo(iface);
> + TRACE("iface %p, error_code %p.\n", iface, error_code);
> +
> + *error_code = impl->hr;
> + return S_OK;
> }
>
> static HRESULT WINAPI async_operation_info_Cancel( IAsyncInfo *iface )
> {
> - FIXME("iface %p stub!\n", iface);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncInfo(iface);
> + HRESULT hr;
> +
> + TRACE("iface %p.\n", iface);
> + hr = S_OK;
> +
> + if (impl->status == Closed)
> + hr = E_ILLEGAL_METHOD_CALL;
> + else if (impl->status == Started)
> + impl->status = Canceled;
> +
> + return hr;
> }
>
> static HRESULT WINAPI async_operation_info_Close( IAsyncInfo *iface )
> {
> - FIXME("iface %p stub!\n", iface);
> - return E_NOTIMPL;
> + struct async_operation *impl = impl_from_IAsyncInfo(iface);
> +
> + TRACE("iface %p.\n", iface);
> +
> + impl->status = Closed;
> + return S_OK;
> }
>
> static const struct IAsyncInfoVtbl async_operation_info_vtbl =
> @@ -202,11 +302,14 @@ static const struct IAsyncInfoVtbl async_operation_info_vtbl =
> HRESULT async_operation_create( const GUID *iid, IAsyncOperation_IInspectable **out )
> {
> struct async_operation *impl;
> + HRESULT hr;
> +
> + *out = NULL;
>
> if (!(impl = calloc(1, sizeof(*impl))))
> {
> - *out = NULL;
> - return E_OUTOFMEMORY;
> + hr = E_OUTOFMEMORY;
> + goto error;
> }
>
> impl->IAsyncOperation_IInspectable_iface.lpVtbl = &async_operation_vtbl;
> @@ -214,7 +317,14 @@ HRESULT async_operation_create( const GUID *iid, IAsyncOperation_IInspectable **
> impl->iid = iid;
> impl->ref = 1;
>
> + impl->handler = HANDLER_NOT_SET;
> + impl->status = Closed;
> +
I'm thinking that for this patch, and as it's still all stubs, it'd be
better to set the initial status to Completed. This way put_Completed
would invoke the handler, and you wouldn't have to bother adding
Wine-specific and temporary wait timeouts.
It may require a temporary tweak or two in the tests, like GetResults
which would succeed but not return any result. I think you could set it
to 0xdeadbeef and check that GetResults succeeded and returned a result
!= 0xdeadbeef (or skip the result tests).
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list