Re: [PATCH v3 3/3] mshtml: Add console object stub implementation.

[Jacek Caban]

Hi Paul,

On 7/7/21 2:03 PM, Paul Gofman wrote:
> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com>
> ---
> v3:
>      - add console getter interface to HTMLWindow.
>
> dlls/mshtml/Makefile.in              |   1 +
>   dlls/mshtml/console.c                | 294 +++++++++++++++++++++++++++

We already have more files than we should in mshtml, this could go to 
omnavigator.c instead.
> +void create_console(IWineMSHTMLConsole **ret)
> +{
> +    struct console *obj;
> +
> +    obj = heap_alloc_zero(sizeof(*obj));
> +    if(!obj)
> +    {
> +        ERR("No memory.\n");
> +        return;
> +    }
> +
> +    obj->IWineMSHTMLConsole_iface.lpVtbl = &WineMSHTMLConsoleVtbl;
> +    obj->ref = 1;
> +    init_dispatch(&obj->dispex, (IUnknown*)&obj->IWineMSHTMLConsole_iface, &console_dispex, COMPAT_MODE_NONE);

Compat mode affects some basic IDispatchEx behaviour now, so it should 
be specified properly here. For that, you will probably want to delay 
object creation.
> +
> +    *ret = &obj->IWineMSHTMLConsole_iface;
> +}
> diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> index 6d248691d6b..e0c2ea2c121 100644
> --- a/dlls/mshtml/dispex.c
> +++ b/dlls/mshtml/dispex.c
> @@ -111,7 +111,7 @@ struct dispex_dynamic_data_t {
>
> #define FDEX_VERSION_MASK 0xf0000000
>
> -static ITypeLib *typelib;
> +static ITypeLib *typelib, *typelib_private;
>   static ITypeInfo *typeinfos[LAST_tid];
>   static struct list dispex_data_list = LIST_INIT(dispex_data_list);
>
> @@ -119,6 +119,8 @@ static REFIID tid_ids[] = {
>   #define XIID(iface) &IID_ ## iface,
>   #define XDIID(iface) &DIID_ ## iface,
>   TID_LIST
> +    NULL,
> +PRIVATE_TID_LIST
>   #undef XIID
>   #undef XDIID
>   };
> @@ -136,7 +138,17 @@ static HRESULT load_typelib(void)
>
> if(InterlockedCompareExchangePointer((void**)&typelib, tl, NULL))
>           ITypeLib_Release(tl);
> -    return hres;
> +
> +    hres = LoadTypeLibEx(L"mshtml.dll\\1", REGKIND_NONE, &tl);

It would be better to use something like GetModuleFileName() instead of 
hardcoding it.
> diff --git a/dlls/mshtml/mshtml_private_iface.idl b/dlls/mshtml/mshtml_private_iface.idl
> index 60ad70eede7..d721d09190a 100644
> --- a/dlls/mshtml/mshtml_private_iface.idl
> +++ b/dlls/mshtml/mshtml_private_iface.idl
> @@ -30,4 +30,119 @@ import "dispex.idl";
>   ]
>   library MSHTML_private
>   {
> +
> +importlib("stdole2.tlb");
> +
> +[
> +    odl,
> +    oleautomation,
> +    dual,
> +    hidden,
> +    uuid(fd55b4b6-2813-4fb4-829d-380099474ab1)
> +]
> +interface IWineMSHTMLConsole : IDispatch
> +{
> +    [id(1)]
> +    HRESULT assert([in] VARIANT_BOOL *assertion,
> +                   [in, optional] VARIANT *varargStart);
> +    [id(2)]
> +    HRESULT clear();
> +    [id(3)]
> +    HRESULT count([in, optional] VARIANT *label);
> +    [id(4)]
> +    HRESULT debug([in, optional] VARIANT *varargStart);
> +    [id(5)]
> +    HRESULT dir([in, optional] VARIANT *object);
> +    [id(6)]
> +    HRESULT dirxml([in, optional] VARIANT *object);
> +    [id(7)]
> +    HRESULT error([in, optional] VARIANT *varargStart);
> +    [id(8)]
> +    HRESULT group([in, optional] VARIANT *label);
> +    [id(9)]
> +    HRESULT groupCollapsed([in, optional] VARIANT *label);
> +    [id(10)]
> +    HRESULT groupEnd();
> +    [id(11)]
> +    HRESULT info([in, optional] VARIANT *varargStart);
> +    [id(12)]
> +    HRESULT log([in, optional] VARIANT *varargStart);
> +    [id(13)]
> +    HRESULT time([in, optional] VARIANT *label);
> +    [id(14)]
> +    HRESULT timeEnd([in, optional] VARIANT *label);
> +    [id(15)]
> +    HRESULT trace([in, optional] VARIANT *varargStart);
> +    [id(16)]
> +    HRESULT warn([in, optional] VARIANT *varargStart);
> +}
> +
> +[
> +    hidden,
> +    uuid(b61c3206-7c3d-4b56-bf16-a9f264747f58)
> +]
> +dispinterface DispWineMSHTMLConsole
> +{
> +properties:
> +methods:
> +    [id(1)]
> +    void assert([in] VARIANT_BOOL *assertion,
> +                [in, optional] VARIANT *varargStart);
> +    [id(2)]
> +    void clear();
> +    [id(3)]
> +    void count([in, optional] VARIANT *label);
> +    [id(4)]
> +    void debug([in, optional] VARIANT *varargStart);
> +    [id(5)]
> +    void dir([in, optional] VARIANT *object);
> +    [id(6)]
> +    void dirxml([in, optional] VARIANT *object);
> +    [id(7)]
> +    void error([in, optional] VARIANT *varargStart);
> +    [id(8)]
> +    void group([in, optional] VARIANT *label);
> +    [id(9)]
> +    void groupCollapsed([in, optional] VARIANT *label);
> +    [id(10)]
> +    void groupEnd();
> +    [id(11)]
> +    void info([in, optional] VARIANT *varargStart);
> +    [id(12)]
> +    void log([in, optional] VARIANT *varargStart);
> +    [id(13)]
> +    void time([in, optional] VARIANT *label);
> +    [id(14)]
> +    void timeEnd([in, optional] VARIANT *label);
> +    [id(15)]
> +    void trace([in, optional] VARIANT *varargStart);
> +    [id(16)]
> +    void warn([in, optional] VARIANT *varargStart);
> +}

You shouldn't need dispinterface at all.
> +
> +[
> +    odl,
> +    oleautomation,
> +    dual,
> +    hidden,
> +    uuid(1b5939fc-8f84-43f3-8d89-f9a92069fad7)
> +]
> +interface IWineConsoleGetter : IDispatch
> +{
> +    [propget, id(1)]
> +    HRESULT console([retval, out] IDispatch **console);
> +}

It would be have a more generic name for this. I was hoping that we 
could use it for requestAnimationFrame as well (which would be a nice 
excuse to introduce it in a separated patch).
> +
> +[
> +    hidden,
> +    uuid(b84d405a-8057-4cc7-86c6-e872bb6205ad)
> +]
> +dispinterface DispWineConsoleGetter
> +{
> +properties:
> +methods:
> +    [propget, id(1)]
> +    IDispatch *console();
> +}

Same here, dispinterface is not needed.
> } /* library MSHTML_private */
> diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js
> index a520253569c..e57aed5ca35 100644
> --- a/dlls/mshtml/tests/es5.js
> +++ b/dlls/mshtml/tests/es5.js
> @@ -1448,3 +1448,67 @@ sync_test("functions scope", function() {
>       ok(val == 8, "val != 8");
>       ok(w == 9, "w != 9");
>   });
> +
> +sync_test("console", function() {
> +    var except
> +
> +    window.console.log('1', '2');
> +    console.info('1', '2', '3');
> +    console.info();
> +    console.log();
> +    console.trace();
> +    console.warn();
> +    console.debug();
> +    console.error();
> +
> +    console.assert(false, '1');
> +    console.assert(true, '1');
> +    console.assert('1');
> +
> +    console.clear();
> +    console.count('1');
> +    console.count(1);
> +
> +
> +    except = false;
> +    try
> +    {
> +        console.countReset('1');
> +    }
> +    catch(e)
> +    {
> +        except = true;
> +    }
> +    ok(except, "console.countReset: expected exception");
> +    console.dir(document);
> +    console.dir();
> +    console.dirxml(document);
> +    console.group('1');
> +    console.groupCollapsed('1');
> +    console.groupEnd();
> +
> +    except = false;
> +    try
> +    {
> +        console.table(['1', '2']);
> +    }
> +    catch(e)
> +    {
> +        except = true;
> +    }
> +    ok(except, "console.table: expected exception");
> +
> +    console.time('1');
> +    console.timeEnd('1');
> +
> +    except = false;
> +    try
> +    {
> +        console.timeLog('1');
> +    }
> +    catch(e)
> +    {
> +        except = true;
> +    }
> +    ok(except, "console.timeLog: expected exception");
> +});

A test in documentmode.js "window_props" would be nice as well.


Thanks,

Jacek