[PATCH] ieframe: implement COM aggregation in WebBrowser

Jacek Caban jacek at codeweavers.com
Wed Dec 4 10:46:51 CST 2019


Hi Damjan,

Most of the patch looks good, with a few comments bellow.

On 12/2/19 3:44 AM, Damjan Jovanovic wrote:
>
> diff --git a/dlls/ieframe/ieframe.h b/dlls/ieframe/ieframe.h
> index 2b4861d79b..290bc8fee4 100644
> --- a/dlls/ieframe/ieframe.h
> +++ b/dlls/ieframe/ieframe.h
> @@ -170,6 +170,8 @@ struct DocHost {
>   };
>   
>   struct WebBrowser {
> +    IUnknown                 IUnknown_inner;
> +    IUnknown                *outer_unk;


We already have outer inside HlinkFrame, maybe we could just use that 
instead?


> +static void test_Aggregation(void)
> +{
> +    HRESULT hr;
> +    IUnknown *pUnknown = NULL;
> +
> +    hr = CoCreateInstance(&CLSID_WebBrowser, &unknown, CLSCTX_ALL, &IID_IUnknown, (void**)&pUnknown);
> +    ok(hr == S_OK, "could not create instance of CLSID_WebBrowser with IID_IUnknown, hr = 0x%x\n", hr);
> +    if (pUnknown)
> +        IUnknown_Release(pUnknown);
> +}


A bit more tests would be nice. See test_com_aggregation() from MSHTML 
tests for an example.


> +static const struct IUnknownVtbl internal_unk_vtbl =
> +{
> +    Internal_QueryInterface,
> +    Internal_AddRef,
> +    Internal_Release
> +};


Those functions could use a better name at very least to make logs more 
informative. How about using WebBrowser_* names and rename existing 
IUnknown implementation of IWebBrowser2 interface to, say, WebBrowser2_*?


Thanks,

Jacek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20191204/689faf5b/attachment-0001.htm>


More information about the wine-devel mailing list