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@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@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@codeweavers.com