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

[Rémi Bernon]

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>