[PATCH v2 2/4] windows.media.speech: Implement IInstalledVoicesStatic stub.

[Rémi Bernon]

On 3/5/21 2:51 PM, Jacek Caban wrote:
> Hi Rémi,
> 
> The series looks better now, but it has some COM problems. Also, there 
> is not much to test yet, but it would still be interesting to see some 
> basic creation and QueryInterface tests.
> 

Alright, I can add a few tests.

> On 05.03.2021 09:53, Rémi Bernon wrote:
>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>> ---
>>   dlls/windows.media.speech.dll/main.c | 104 +++++++++++++++++++++++++++
>>   1 file changed, 104 insertions(+)
>>
>> diff --git a/dlls/windows.media.speech.dll/main.c 
>> b/dlls/windows.media.speech.dll/main.c
>> index 909ea6db748..db57acac651 100644
>> --- a/dlls/windows.media.speech.dll/main.c
>> +++ b/dlls/windows.media.speech.dll/main.c
>> @@ -29,7 +29,10 @@
>>   #include "initguid.h"
>>   #include "activation.h"
>> +#define WIDL_using_Windows_Foundation
>> +#define WIDL_using_Windows_Foundation_Collections
>>   #include "windows.foundation.h"
>> +#define WIDL_using_Windows_Media_SpeechSynthesis
>>   #include "windows.media.speechsynthesis.h"
>>   WINE_DEFAULT_DEBUG_CHANNEL(speech);
>> @@ -46,6 +49,7 @@ static const char *debugstr_hstring(HSTRING hstr)
>>   struct windows_media_speech
>>   {
>>       IActivationFactory IActivationFactory_iface;
>> +    IInstalledVoicesStatic IInstalledVoicesStatic_iface;
>>       LONG ref;
>>   };
>> @@ -54,6 +58,98 @@ static inline struct windows_media_speech 
>> *impl_from_IActivationFactory(IActivat
>>       return CONTAINING_RECORD(iface, struct windows_media_speech, 
>> IActivationFactory_iface);
>>   }
>> +static inline struct windows_media_speech 
>> *impl_from_IInstalledVoicesStatic(IInstalledVoicesStatic *iface)
>> +{
>> +    return CONTAINING_RECORD(iface, struct windows_media_speech, 
>> IInstalledVoicesStatic_iface);
>> +}
>> +
>> +static HRESULT STDMETHODCALLTYPE installed_voices_static_QueryInterface(
>> +        IInstalledVoicesStatic *iface, REFIID iid, void **out)
>> +{
>> +    TRACE("iface %p, iid %s, out %p stub!\n", iface, 
>> debugstr_guid(iid), out);
>> +
>> +    if (IsEqualGUID(iid, &IID_IUnknown) ||
>> +        IsEqualGUID(iid, &IID_IAgileObject) ||
>> +        IsEqualGUID(iid, &IID_IInspectable) ||
>> +        IsEqualGUID(iid, &IID_IInstalledVoicesStatic))
>> +    {
>> +        IUnknown_AddRef(iface);
>> +        *out = iface;
>> +        return S_OK;
>> +    }
>> +
>> +    WARN("%s not implemented, returning E_NOINTERFACE.\n", 
>> debugstr_guid(iid));
>> +    *out = NULL;
>> +    return E_NOINTERFACE;
>> +}
> 
> 
> It's missing IActivationFactrory interface.
> 
> 
>> @@ -69,6 +165,13 @@ static HRESULT STDMETHODCALLTYPE 
>> windows_media_speech_QueryInterface(
>>           return S_OK;
>>       }
>> +    if (IsEqualGUID(iid, &IID_IInstalledVoicesStatic))
>> +    {
>> +        IUnknown_AddRef(iface);
>> +        *out = &impl->IInstalledVoicesStatic_iface;
>> +        return S_OK;
>> +    }
> 
> 
> It's missing IID_IAgileObject interface now and it generally just 
> duplicates installed_voices_static_QueryInterface. The usual practice is 
> to have one QueryInterface implementation per object and forward other 
> calls. Same for AddRef and Release (at least in case where we have 
> actual ref counting).
> 
> 
> Note static objects don't need ref counting at all, because you don't 
> have the destructor anyway. If there is a reason to have reference 
> counting, then patch 3 is missing AddRef call.
> 

 From basic tests it looks like it's supposed to be actually refcounted 
on Windows, and destructed if released too many times. I'm not sure if 
it's worth implementing such behavior?

> 
> Static object also don't really need a structs like 
> windows_media_speech, but it's fine to have it. However, 
> IVectorView_VoiceInformation doesn't really belong to 
> windows_media_speech object as it looks like a separate object.
> 

Yeah, that was mainly for convenience, as it was only meant to be a stub.

> 
> Thanks,
> 
> Jacek
> 

Cheers,
-- 
Rémi Bernon <rbernon at codeweavers.com>