Re: [PATCH 7/8] jscript: Implement separate script dispatch objects for each named item.

[Gabriel Ivăncescu]

Hi Jacek,

On 20/02/2020 22:41, Jacek Caban wrote:
> Hi Gabriel,
>
> First of all, this patch breaks mshtml script tests. It's likely that 
> it's mshtml's fault, but we need to get it working.

Ok I'll take a look into it, didn't know it could.
> On 19.02.2020 17:38, Gabriel Ivăncescu wrote:
> > -void enter_script(script_ctx_t *ctx, jsexcept_t *ei)
> > +HRESULT enter_script(script_ctx_t *ctx, named_item_t *item, jsexcept_t *ei)
> >   {
> > - HRESULT hres;
> > -
> > memset(ei, 0, sizeof(*ei));
> > - if(item) {
> > -
> >    if(!item->script_obj) {
> > -
> >        hres = create_named_item_script_obj(ctx, item);
> > -
> >        if(FAILED(hres)) return hres;
> > -
> >    }
>
> Can we have it in exec_source instead? If nothing else, it would 
> simplify enter_script error handling.

I think it can, yes. Do you mean to also remove it from leave_script, 
right? (I mean, either set it in exec_source and unset at the end of the 
function, or find a way to retrieve it otherwise -- currently I use a 
"stack", but that's probably not necessary)
> > diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h
> > index 8297838..9492f3f 100644
> > --- a/dlls/jscript/jscript.h
> > +++ b/dlls/jscript/jscript.h
> > @@ -205,7 +205,9 @@ typedef HRESULT (*builtin_setter_t)(script_ctx_t*,jsdisp_t*,jsval_t);
> >   HRESULT builtin_set_const(script_ctx_t*,jsdisp_t*,jsval_t) DECLSPEC_HIDDEN;
> >
> > typedef struct named_item_t {
> > - jsdisp_t *script_obj;
> >  IDispatch *disp;
> > - unsigned ref;
>
> Maybe ref counting would make sense in a separated patch?

Right, I'll split it up.
> >   DWORD flags;
> >   LPWSTR name;
> >
> > @@ -213,6 +215,7 @@ typedef struct named_item_t {
> >   } named_item_t;
> >
> > named_item_t*lookup_named_item(script_ctx_t*,const WCHAR*,unsigned) DECLSPEC_HIDDEN;
> > +void release_named_item(named_item_t*) DECLSPEC_HIDDEN;
> >
> > typedef struct {
> >       const WCHAR *name;
> > @@ -243,6 +246,7 @@ struct jsdisp_t {
> >       DWORD prop_cnt;
> >       dispex_prop_t *props;
> >       script_ctx_t *ctx;
> > - named_item_t *named_item;
>
> I'd rather avoid having it here. I think that we should almost always be 
> getting it from bytecode. In some corner cases, jsexcept should be 
> enough to fill the gap.

The only issue I had when I tried without it is in 
dispex.c/DispatchEx_InvokeEx. Is there another way to retrieve it, or 
the bytecode, in there?
> >   jsdisp_t *prototype;
> >
> > @@ -433,6 +437,7 @@ struct _script_ctx_t {
> >       DWORD last_match_length;
> >   jsdisp_t *global;
> > - jsdisp_t *item_context;
>
> Same here, do we really need it?

Probably not, but I'm not that familiar with the code. What's the 
easiest or best way to retrieve it from the bytecode? From call_ctx? Or 
perhaps from the ei?

Thanks,
Gabriel