[PATCH v8 1/3] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/jscript/engine.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c
index 5b96895..044e32c 100644
--- a/dlls/jscript/engine.c
+++ b/dlls/jscript/engine.c
@@ -669,6 +669,15 @@ static HRESULT identifier_eval(script_ctx_t *ctx, BSTR identifier, exprval_t *re
                 return S_OK;
             }
         }
+
+        if(ctx->call_ctx->bytecode->named_item) {
+            jsdisp_t *script_obj = ctx->call_ctx->bytecode->named_item->script_obj;
+            hres = jsdisp_get_id(script_obj, identifier, 0, &id);
+            if(SUCCEEDED(hres)) {
+                exprval_set_disp_ref(ret, to_disp(script_obj), id);
+                return S_OK;
+            }
+        }
     }

hres = jsdisp_get_id(ctx->global, identifier, 0, &id);
@@ -1254,13 +1263,17 @@ static HRESULT interp_identifier_ref(script_ctx_t *ctx, BSTR identifier, unsigne
         return hres;

if(exprval.type == EXPRVAL_INVALID && (flags & fdexNameEnsure)) {
+        jsdisp_t *script_obj = ctx->global;
         DISPID id;

-        hres = jsdisp_get_id(ctx->global, identifier, fdexNameEnsure, &id);
+        if(ctx->call_ctx->bytecode->named_item)
+            script_obj = ctx->call_ctx->bytecode->named_item->script_obj;
+
+        hres = jsdisp_get_id(script_obj, identifier, fdexNameEnsure, &id);
         if(FAILED(hres))
             return hres;

-        exprval_set_disp_ref(&exprval, to_disp(ctx->global), id);
+        exprval_set_disp_ref(&exprval, to_disp(script_obj), id);
     }

if(exprval.type == EXPRVAL_JSVAL || exprval.type == EXPRVAL_INVALID) {
@@ -2995,6 +3008,8 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
             hres = create_named_item_script_obj(ctx, bytecode->named_item);
             if(FAILED(hres)) return hres;
         }
+        if(variable_obj == ctx->global)
+            variable_obj = bytecode->named_item->script_obj;
     }

if(!ctx->ei->enter_notified) {

[PATCH v8 2/3] jscript: Retrieve the proper 'this' object for bytecodes with named items.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/jscript/engine.c        | 10 ++++++++--
 dlls/jscript/tests/jscript.c |  3 ---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c
index 044e32c..017f3d0 100644
--- a/dlls/jscript/engine.c
+++ b/dlls/jscript/engine.c
@@ -1246,8 +1246,14 @@ static HRESULT interp_this(script_ctx_t *ctx)

TRACE("\n");

-    if(!this_obj)
-        this_obj = lookup_global_host(ctx);
+    if(!this_obj) {
+        named_item_t *item = ctx->call_ctx->bytecode->named_item;
+
+        if(item)
+            this_obj = (item->flags & SCRIPTITEM_CODEONLY) ? to_disp(item->script_obj) : item->disp;
+        else
+            this_obj = lookup_global_host(ctx);
+    }

IDispatch_AddRef(this_obj);
     return stack_push(ctx, jsval_disp(this_obj));
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 5de933e..1246f2d 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1252,7 +1252,6 @@ static void test_named_items(void)
     SET_EXPECT(OnLeaveScript);
     hr = IActiveScriptParse_ParseScriptText(parse, L"this", L"visibleCodeItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISEXPRESSION, &var, NULL);
     ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr);
-    todo_wine
     ok(V_VT(&var) == VT_DISPATCH && V_DISPATCH(&var) == disp,
         "Unexpected 'this': V_VT = %d, V_DISPATCH = %p\n", V_VT(&var), V_DISPATCH(&var));
     VariantClear(&var);
@@ -1415,7 +1414,6 @@ static void test_named_items(void)
     SET_EXPECT(OnLeaveScript);
     hr = IActiveScriptParse_ParseScriptText(parse, L"this", L"visibleItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISEXPRESSION, &var, NULL);
     ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr);
-    todo_wine
     ok(V_VT(&var) == VT_DISPATCH && V_DISPATCH(&var) == &visible_named_item,
         "Unexpected 'this': V_VT = %d, V_DISPATCH = %p\n", V_VT(&var), V_DISPATCH(&var));
     VariantClear(&var);
@@ -1426,7 +1424,6 @@ static void test_named_items(void)
     SET_EXPECT(OnLeaveScript);
     hr = IActiveScriptParse_ParseScriptText(parse, L"this", L"codeOnlyItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISEXPRESSION, &var, NULL);
     ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr);
-    todo_wine
     ok(V_VT(&var) == VT_DISPATCH && V_DISPATCH(&var) == (IDispatch*)dispex2,
         "Unexpected 'this': V_VT = %d, V_DISPATCH = %p\n", V_VT(&var), V_DISPATCH(&var));
     VariantClear(&var);

[PATCH v8 2/3] jscript: Retrieve the proper 'this' object for bytecodes with named items.

[Jacek Caban]

Signed-off-by: Jacek Caban

[PATCH v8 3/3] jscript: Create separate script dispatches for each named item.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/jscript/jscript.c       | 13 +++++++++----
 dlls/jscript/tests/jscript.c | 18 ------------------
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c
index aabad7e..c474a73 100644
--- a/dlls/jscript/jscript.c
+++ b/dlls/jscript/jscript.c
@@ -110,10 +110,15 @@ static inline BOOL is_started(script_ctx_t *ctx)

HRESULT create_named_item_script_obj(script_ctx_t *ctx, named_item_t *item)
 {
-    /* FIXME: Create a separate script dispatch instead of using the global */
-    item->script_obj = ctx->global;
-    IDispatchEx_AddRef(&item->script_obj->IDispatchEx_iface);
-    return S_OK;
+    static const builtin_info_t disp_info = {
+        JSCLASS_GLOBAL,
+        {NULL, NULL, 0},
+        0, NULL,
+        NULL,
+        NULL
+    };
+
+    return create_dispex(ctx, &disp_info, NULL, &item->script_obj);
 }

static void release_named_item_script_obj(named_item_t *item)
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 1246f2d..164e6f4 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1203,7 +1203,6 @@ static void test_named_items(void)
     ok(dispex == dispex2, "get_script_dispatch returned different dispatch objects.\n");
     IDispatchEx_Release(dispex2);
     dispex2 = get_script_dispatch(script, L"codeOnlyItem");
-    todo_wine
     ok(dispex != dispex2, "get_script_dispatch returned same dispatch objects.\n");

SET_EXPECT(OnStateChange_INITIALIZED);
@@ -1335,9 +1334,7 @@ static void test_named_items(void)

id = 0;
         hr = IDispatchEx_GetDispID(dispex2, bstr, 0, &id);
-        todo_wine
         ok(hr == DISP_E_UNKNOWNNAME, "GetDispID(%s) returned %08x\n", wine_dbgstr_w(global_idents[i]), hr);
-        todo_wine
         ok(id == -1, "[%s] id = %d, expected -1\n", wine_dbgstr_w(global_idents[i]), id);
         SysFreeString(bstr);
     }
@@ -1347,9 +1344,7 @@ static void test_named_items(void)
         bstr = SysAllocString(context_idents[i]);
         id = 0;
         hr = IDispatchEx_GetDispID(dispex, bstr, 0, &id);
-        todo_wine
         ok(hr == DISP_E_UNKNOWNNAME, "GetDispID(%s) returned %08x\n", wine_dbgstr_w(context_idents[i]), hr);
-        todo_wine
         ok(id == -1, "[%s] id = %d, expected -1\n", wine_dbgstr_w(context_idents[i]), id);
         id = 0;
         hr = IDispatchEx_GetDispID(dispex2, bstr, 0, &id);
@@ -1383,12 +1378,9 @@ static void test_named_items(void)
         SET_EXPECT(OnScriptError);
         SET_EXPECT(OnLeaveScript);
         hr = IActiveScriptParse_ParseScriptText(parse, context_code_test[i], NULL, NULL, NULL, 0, 0, 0, NULL, NULL);
-        todo_wine
         ok(FAILED(hr), "ParseScriptText(%s) returned: %08x\n", wine_dbgstr_w(context_code_test[i]), hr);
         CHECK_CALLED(OnEnterScript);
-        todo_wine_if(i != 0)
         CHECK_CALLED(GetIDsOfNames);
-        todo_wine
         CHECK_CALLED(OnScriptError);
         CHECK_CALLED(OnLeaveScript);

@@ -1498,9 +1490,7 @@ static void test_named_items(void)
         bstr = SysAllocString(context_idents[i]);
         id = 0;
         hr = IDispatchEx_GetDispID(dispex, bstr, 0, &id);
-        todo_wine_if(i != 0)
         ok(hr == DISP_E_UNKNOWNNAME, "GetDispID(%s) returned %08x\n", wine_dbgstr_w(context_idents[i]), hr);
-        todo_wine_if(i != 0)
         ok(id == -1, "[%s] id = %d, expected -1\n", wine_dbgstr_w(context_idents[i]), id);
         SysFreeString(bstr);
     }
@@ -1539,9 +1529,7 @@ static void test_named_items(void)
         bstr = SysAllocString(global_idents[i]);
         id = 0;
         hr = IDispatchEx_GetDispID(dispex, bstr, 0, &id);
-        todo_wine
         ok(hr == DISP_E_UNKNOWNNAME, "GetDispID(%s) returned %08x\n", wine_dbgstr_w(global_idents[i]), hr);
-        todo_wine
         ok(id == -1, "[%s] id = %d, expected -1\n", wine_dbgstr_w(global_idents[i]), id);
         SysFreeString(bstr);
     }
@@ -1551,9 +1539,7 @@ static void test_named_items(void)
         bstr = SysAllocString(context_idents[i]);
         id = 0;
         hr = IDispatchEx_GetDispID(dispex, bstr, 0, &id);
-        todo_wine_if(i != 0)
         ok(hr == DISP_E_UNKNOWNNAME, "GetDispID(%s) returned %08x\n", wine_dbgstr_w(context_idents[i]), hr);
-        todo_wine_if(i != 0)
         ok(id == -1, "[%s] id = %d, expected -1\n", wine_dbgstr_w(context_idents[i]), id);
         SysFreeString(bstr);
     }
@@ -1581,10 +1567,8 @@ static void test_named_items(void)
         SET_EXPECT(OnScriptError);
         SET_EXPECT(OnLeaveScript);
         hr = IActiveScriptParse_ParseScriptText(parse, context_code_test[i], NULL, NULL, NULL, 0, 0, 0, NULL, NULL);
-        todo_wine_if(i != 0)
         ok(FAILED(hr), "ParseScriptText(%s) returned: %08x\n", wine_dbgstr_w(context_code_test[i]), hr);
         CHECK_CALLED(OnEnterScript);
-        todo_wine_if(i != 0)
         CHECK_CALLED(OnScriptError);
         CHECK_CALLED(OnLeaveScript);

@@ -1592,10 +1576,8 @@ static void test_named_items(void)
         SET_EXPECT(OnScriptError);
         SET_EXPECT(OnLeaveScript);
         hr = IActiveScriptParse_ParseScriptText(parse, context_code_test[i], L"codeOnlyItem", NULL, NULL, 0, 0, 0, NULL, NULL);
-        todo_wine_if(i != 0)
         ok(FAILED(hr), "ParseScriptText(%s) returned: %08x\n", wine_dbgstr_w(context_code_test[i]), hr);
         CHECK_CALLED(OnEnterScript);
-        todo_wine_if(i != 0)
         CHECK_CALLED(OnScriptError);
         CHECK_CALLED(OnLeaveScript);
     }

[Jacek Caban]

Hi Gabriel,

On 11/03/2020 14:06, Gabriel Ivăncescu wrote:
> @@ -2995,6 +3008,8 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
>               hres = create_named_item_script_obj(ctx, bytecode->named_item);
>               if(FAILED(hres)) return hres;
>           }
> -
>    if(variable_obj == ctx->global)
> -
>        variable_obj = bytecode->named_item->script_obj;

This could be a separated patch. If in this case caller can't handle 
variable_obj in many cases, maybe it shouldn't handle it at all. It 
seems to me that we could get rid of that argument. exec_source has all 
information needed to handle it by itself.

Thanks,

Jacek

[Gabriel Ivăncescu]

Hi Jacek,

On 12/03/2020 13:02, Jacek Caban wrote:
> Hi Gabriel,
>
> On 11/03/2020 14:06, Gabriel Ivăncescu wrote:
> > @@ -2995,6 +3008,8 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD 
> > flags, bytecode_t *bytecode, functi
> >               hres = create_named_item_script_obj(ctx, 
> > bytecode->named_item);
> >               if(FAILED(hres)) return hres;
> >           }
> > +        if(variable_obj == ctx->global)
> > +            variable_obj = bytecode->named_item->script_obj;
>
> This could be a separated patch. If in this case caller can't handle 
> variable_obj in many cases, maybe it shouldn't handle it at all. It 
> seems to me that we could get rid of that argument. exec_source has all 
> information needed to handle it by itself.
>
> Thanks,
>
> Jacek

I can split it of course, but I have some doubts when it comes to 
removing it. What should we do in function.c's InterpretedFunction_call?

A possible solution I came up with would be to add a new exec_flag, say 
EXEC_NEW_VARIABLE_OBJ and use it in InterpretedFunction_call -- then 
create it in exec_source if the flag is set.

In rest of cases, we take the frame->variable_obj if a frame is 
available, otherwise the ctx->global or named item's dispatch. All of 
this in exec_source of course, then we get rid of the parameter altogether.

Does that sound like a good approach?

Thanks,
Gabriel

[Jacek Caban]

On 12.03.2020 14:29, Gabriel Ivăncescu wrote:
> Hi Jacek,
>
> On 12/03/2020 13:02, Jacek Caban wrote:
> > Hi Gabriel,
> >
> > On 11/03/2020 14:06, Gabriel Ivăncescu wrote:
> > > @@ -2995,6 +3008,8 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD 
> > > flags, bytecode_t *bytecode, functi
> > >               hres = create_named_item_script_obj(ctx, 
> > > bytecode->named_item);
> > >               if(FAILED(hres)) return hres;
> > >           }
> > > +        if(variable_obj == ctx->global)
> > > +            variable_obj = bytecode->named_item->script_obj;
> >
> > This could be a separated patch. If in this case caller can't handle 
> > variable_obj in many cases, maybe it shouldn't handle it at all. It 
> > seems to me that we could get rid of that argument. exec_source has 
> > all information needed to handle it by itself.
> >
> > Thanks,
> >
> > Jacek
>
> I can split it of course, but I have some doubts when it comes to 
> removing it. What should we do in function.c's InterpretedFunction_call?
>
> A possible solution I came up with would be to add a new exec_flag, 
> say EXEC_NEW_VARIABLE_OBJ and use it in InterpretedFunction_call -- 
> then create it in exec_source if the flag is set.

Unless I'm missing something, a new variable object is needed always for 
non-global non-eval code. You don't need a new flag for that.
> In rest of cases, we take the frame->variable_obj if a frame is 
> available, otherwise the ctx->global or named item's dispatch. All of 
> this in exec_source of course, then we get rid of the parameter 
> altogether.
>
> Does that sound like a good approach?

Sounds good, yes.

Thanks,

Jacek