From: Jacek Caban <jacek@codeweavers.com>
Subject: Re: [PATCH v2] wmp: add IWMPControls stub implementation
Message-Id: <d4c391ae-ce87-b5ba-2e27-0344feadbf01@codeweavers.com>
Date: Thu, 15 Feb 2018 18:09:04 +0100
In-Reply-To: <20180212213640.25141-1-theli.ua@gmail.com>
References: <20180212213640.25141-1-theli.ua@gmail.com>

Hi Anton,

On 02/12/2018 10:36 PM, Anton Romanov wrote:
> +HRESULT WINAPI WMPControls_QueryInterface(IWMPControls *iface, REFIID riid, void **ppv)
> +{
> +    if(IsEqualGUID(riid, &IID_IDispatch)) {
> +        *ppv = iface;
> +    }else if(IsEqualGUID(riid, &IID_IWMPControls)) {
> +        *ppv = iface;
> +    }else {
> +        WARN("Unsupported interface (%s)\n", wine_dbgstr_guid(riid));
> +        *ppv = NULL;
> +        return E_NOINTERFACE;
> +    }
> +
> +    IUnknown_AddRef((IUnknown*)*ppv);
> +    return S_OK;
> +}

This breaks COM rules: if iface1->QI(IID_iface2) returns iface2, then
iface2->QI(IID_iface1) should work as well. I tried your tests and it
seems that MS violates this rule as well, but in a different way than
you do. What's even more important, interface returned by QI is not the
same as returned by get_controls(). get_controls() should return a
different object. I guess that your application needs get_controls()
working, not QI(IID_IWMPControls)? I'd suggest to implement IWMPControls
in a separated object (I guess that's what you really want to fix) and
ignore the fact that it's also returned by QI() to avoid messing with
broken behaviour.


Also, please use consistent implementation order: IUnknown functions
first followed by other parent's interfaces (like IDispatch), followed
by actual interface implementation.

Thanks,
Jacek