[PATCH 4/6] windows.media.speech: Implement handler for IAsyncOperation.

[Bernhard Kölbl]

Yeah the many helpers aren't ideal. I'll rework the patches.

Thanks,
Bernhard

Am Fr., 1. Apr. 2022 um 11:24 Uhr schrieb Rémi Bernon <rbernon at codeweavers.com>:
>
> On 4/1/22 11:15, Rémi Bernon wrote:
> > On 3/31/22 18:17, Bernhard Kölbl wrote:
> >> Signed-off-by: Bernhard Kölbl <besentv at gmail.com>
> >> ---
> >>   dlls/windows.media.speech/async.c        | 40 ++++++++++++++++++++----
> >>   dlls/windows.media.speech/private.h      |  1 +
> >>   dlls/windows.media.speech/recognizer.c   | 10 +++++-
> >>   dlls/windows.media.speech/tests/speech.c | 25 +++++++--------
> >>   4 files changed, 55 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/dlls/windows.media.speech/async.c
> >> b/dlls/windows.media.speech/async.c
> >> index 5b74ec60be1..767b4a3a4e4 100644
> >> --- a/dlls/windows.media.speech/async.c
> >> +++ b/dlls/windows.media.speech/async.c
> >> @@ -35,6 +35,10 @@ struct async_operation
> >>       IAsyncInfo IAsyncInfo_iface;
> >>       const GUID *iid;
> >>       LONG ref;
> >> +
> >> +    IAsyncOperationCompletedHandler_IInspectable *handler;
> >> +    BOOLEAN handler_set;
> >> +    AsyncStatus status;
> >>   };
> >>   static inline struct async_operation
> >> *impl_from_IAsyncOperation_IInspectable(IAsyncOperation_IInspectable
> >> *iface)
> >> @@ -110,15 +114,26 @@ 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);
> >> +
> >> +    TRACE("iface %p, handler %p.\n", iface, handler);
> >> +
> >> +    if (impl->handler_set) return E_ILLEGAL_DELEGATE_ASSIGNMENT;
> >> +    impl->handler = handler;
> >> +    impl->handler_set = TRUE;
> >> +
> >> +    if (impl->status == Completed)
> >> +        return async_operation_notify(iface);
> >> +
> >> +    return S_OK;
> >>   }
> >>   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;
> >> +    FIXME("iface %p, handler %p semi stub!\n", iface, handler);
> >> +    *handler = NULL;
> >> +    return S_OK;
> >>   }
> >>   static HRESULT WINAPI async_operation_GetResults(
> >> IAsyncOperation_IInspectable *iface, IInspectable ***results )
> >> @@ -159,8 +174,10 @@ 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);
> >> +    TRACE("iface %p, status %p.\n", iface, status);
> >> +    *status = impl->status;
> >> +    return S_OK;
> >>   }
> >>   static HRESULT WINAPI async_operation_info_get_ErrorCode( IAsyncInfo
> >> *iface, HRESULT *error_code )
> >> @@ -218,3 +235,14 @@ HRESULT async_operation_create( const GUID *iid,
> >> IAsyncOperation_IInspectable **
> >>       TRACE("created %p\n", *out);
> >>       return S_OK;
> >>   }
> >> +
> >> +HRESULT async_operation_notify( IAsyncOperation_IInspectable
> >> *operation )
> >> +{
> >> +    struct async_operation *impl =
> >> impl_from_IAsyncOperation_IInspectable(operation);
> >> +
> >> +    impl->status = Completed;
> >> +    if (impl->handler)
> >> +
> >> IAsyncOperationCompletedHandler_IInspectable_Invoke(impl->handler,
> >> operation, impl->status);
> >> +
> >> +    return S_OK;
> >> +}
> >> \ No newline at end of file
> >
> > It might require a bit of reorganization of the patches, to introduce
> > the result first, but I think you only need one private helper overall,
> > which would be something like "async_operation_complete". I don't think
> > it makes much sense to have two separate "set_result" + "notify"
> > operations.
> >
> > It would also be interesting to add tests for the IAsyncInfo and results
> > before setting a completion handler. As far as I can tell, the async is
> > actually completed by the put_Completed call, not right when it is
> > created. This would make the "already completed" case in put_Completed
> > go away.
> >
> > Imho adding the tests first, in a separate patch, including with the
> > results tests also would make things a bit more clear in term of to
> > which direction the implementation should go.
> >
>
>
> And I'm guessing here, but I think that truly async operations could
> work with a "async_operation_start", also called from put_Completed,
> starting a task on a worker thread at that point to also avoid the
> situation where the task completes before the handler has been provided.
>
> FWIW instead (or in addition to an async_operation_complete) you could
> already introduce a "async_operation_start" helper, which would later be
> able to either queue long tasks to worker thread, or simply execute and
> complete short async tasks synchronously.
>
> --
> Rémi Bernon <rbernon at codeweavers.com>
>