Re: [PATCH 4/9] mshtml: Use builtin hooks even when calling function dispatch objects.

[Jacek Caban]

Hi Gabriel,

On 12/3/21 2:57 PM, Gabriel Ivăncescu wrote:
> This fixes a discrepancy between builtins called via a function dispatch
> object and others.
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/mshtml/dispex.c              | 37 ++++++++++++++++++++-----------
>   dlls/mshtml/htmlelem.c            |  7 +++---
>   dlls/mshtml/htmlevent.c           | 10 ++++-----
>   dlls/mshtml/mshtml_private.h      |  6 +++--
>   dlls/mshtml/tests/documentmode.js | 14 +++++++++++-
>   dlls/mshtml/xmlhttprequest.c      |  5 ++---
>   6 files changed, 50 insertions(+), 29 deletions(-)
> diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> index 9f56a56..4ba00f8 100644
> --- a/dlls/mshtml/dispex.c
> +++ b/dlls/mshtml/dispex.c
> @@ -49,7 +49,7 @@ typedef struct {
>       VARIANT default_value;
>   } func_arg_info_t;
>
> -typedef struct {
> +struct func_info_t {
>       DISPID id;
>       BSTR name;
>       tid_t tid;
> @@ -63,7 +63,7 @@ typedef struct {
>       VARTYPE prop_vt;
>       VARTYPE *arg_types;
>       func_arg_info_t *arg_info;
> -} func_info_t;
> +};

This could be kept local to dispex.c.
> struct dispex_data_t {
>       dispex_static_data_t *desc;
> @@ -837,6 +837,11 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR
>       case DISPATCH_METHOD:
>           if(!This->obj)
>               return E_UNEXPECTED;
> -
>    if(This->info->hook) {
> -
>        hres = This->info->hook(This->obj, This->info, lcid, flags, params, res, ei, caller);
> -
>        if(hres != S_FALSE)
> -
>            break;
> -
>    }
>     hres = typeinfo_invoke(This->obj, This->info, flags, params, res, ei);
>     break;

I think you could simply change typeinfo_invoke to function_invoke() here.
>   case DISPATCH_PROPERTYGET: {
>
> @@ -1163,13 +1168,17 @@ static HRESULT builtin_propput(DispatchEx *This, func_info_t *func, DISPPARAMS *
>       return hres;
>   }
>
> -static HRESULT invoke_builtin_function(DispatchEx *This, func_info_t *func, DISPPARAMS *dp, VARIANT *res, IServiceProvider *caller)
> +HRESULT invoke_builtin_function(DispatchEx *This, func_info_t *func, WORD flags, DISPPARAMS *dp, VARIANT *res,
> -
>    EXCEPINFO *ei, IServiceProvider *caller)
>
> {
>     VARIANT arg_buf[MAX_ARGS], *arg_ptrs[MAX_ARGS], *arg, retv, ret_ref, vhres;
>     unsigned i, nconv = 0;
>     IUnknown *iface;
>     HRESULT hres;
> -
>
> if(!func->call_vtbl_off)
> -
>    return typeinfo_invoke(This, func, flags, dp, res, ei);
> -
> if(dp->cNamedArgs) {
>     FIXME("Named arguments not supported\n");
>     return E_NOTIMPL;
>
> @@ -1265,8 +1274,8 @@ static HRESULT invoke_builtin_function(DispatchEx *This, func_info_t *func, DISP
>       return V_ERROR(&vhres);
>   }
>
> -static HRESULT function_invoke(DispatchEx *This, func_info_t *func, WORD flags, DISPPARAMS *dp, VARIANT *res,
> -
>    EXCEPINFO *ei, IServiceProvider *caller)
>
> +static HRESULT function_invoke(DispatchEx *This, func_info_t *func, LCID lcid, WORD flags, DISPPARAMS *dp,
> -
>    VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
> {
>     HRESULT hres;
>
> @@ -1296,10 +1305,12 @@ static HRESULT function_invoke(DispatchEx *This, func_info_t *func, WORD flags,
>               }
>           }
> -
>    if(func->call_vtbl_off)
> -
>        hres = invoke_builtin_function(This, func, dp, res, caller);
> -
>    else
> -
>        hres = typeinfo_invoke(This, func, flags, dp, res, ei);
> -
>    if(func->hook) {
> -
>        hres = func->hook(This, func, lcid, flags, dp, res, ei, caller);
> -
>        if(hres != S_FALSE)
> -
>            break;
> -
>    }
> -
>    hres = invoke_builtin_function(This, func, flags, dp, res, ei, caller);

This seems to be a good place to call the hook, but could you just keep 
typeinfo_invoke call here and don't expose invoke_builtin_function?

Thanks,

Jacek