[PATCH] ieframe: implement COM aggregation in WebBrowser

[Damjan Jovanovic]

Wine's WebBrowser currently just ignores the "outer" it is passed.
Support COM aggregation like Windows does instead.

Signed-off-by: Damjan Jovanovic <damjan.jov(a)gmail.com>
---
 dlls/ieframe/classinfo.c        |   6 +-
 dlls/ieframe/ieframe.h          |   2 +
 dlls/ieframe/oleobject.c        |  30 +--
 dlls/ieframe/persist.c          |  18 +-
 dlls/ieframe/tests/webbrowser.c |  42 +++++
 dlls/ieframe/view.c             |  12 +-
 dlls/ieframe/webbrowser.c       | 323 ++++++++++++++++++--------------
 7 files changed, 259 insertions(+), 174 deletions(-)

[Jacek Caban]

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

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=61363

Your paranoid android.


=== debian10 (build log) ===

error: patch failed: dlls/ieframe/ieframe.h:170
Task: Patch failed to apply

=== debian10 (build log) ===

error: patch failed: dlls/ieframe/ieframe.h:170
Task: Patch failed to apply