[PATCH v6 1/8] jscript: Lookup the named item and keep a ref to it from the associated bytecode.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/compile.c       | 10 +++++++++-
 dlls/jscript/engine.h        |  3 ++-
 dlls/jscript/function.c      |  3 ++-
 dlls/jscript/global.c        |  2 +-
 dlls/jscript/jscript.c       | 22 ++++++++++++++++++++--
 dlls/jscript/tests/jscript.c |  3 +++
 6 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/dlls/jscript/compile.c b/dlls/jscript/compile.c
index 61db10c..5ac93c9 100644
--- a/dlls/jscript/compile.c
+++ b/dlls/jscript/compile.c
@@ -2250,6 +2250,8 @@ void release_bytecode(bytecode_t *code)
     for(i=0; i < code->str_cnt; i++)
         jsstr_release(code->str_pool[i]);
 
+    if(code->named_item)
+        release_named_item(code->named_item);
     heap_free(code->source);
     heap_pool_free(&code->heap);
     heap_free(code->bstr_pool);
@@ -2485,7 +2487,8 @@ static HRESULT compile_arguments(compiler_ctx_t *ctx, const WCHAR *args)
 }
 
 HRESULT compile_script(script_ctx_t *ctx, const WCHAR *code, UINT64 source_context, unsigned start_line,
-                       const WCHAR *args, const WCHAR *delimiter, BOOL from_eval, BOOL use_decode, bytecode_t **ret)
+                       const WCHAR *args, const WCHAR *delimiter, BOOL from_eval, BOOL use_decode,
+                       named_item_t *named_item, bytecode_t **ret)
 {
     compiler_ctx_t compiler = {0};
     HRESULT hres;
@@ -2526,6 +2529,11 @@ HRESULT compile_script(script_ctx_t *ctx, const WCHAR *code, UINT64 source_conte
         return DISP_E_EXCEPTION;
     }
 
+    if(named_item) {
+        compiler.code->named_item = named_item;
+        named_item->ref++;
+    }
+
     *ret = compiler.code;
     return S_OK;
 }
diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h
index 7be98c5..fe6b2a0 100644
--- a/dlls/jscript/engine.h
+++ b/dlls/jscript/engine.h
@@ -181,6 +181,7 @@ struct _bytecode_t {
     heap_pool_t heap;
 
     function_code_t global_code;
+    named_item_t *named_item;
 
     WCHAR *source;
     UINT64 source_context;
@@ -197,7 +198,7 @@ struct _bytecode_t {
     struct list entry;
 };
 
-HRESULT compile_script(script_ctx_t*,const WCHAR*,UINT64,unsigned,const WCHAR*,const WCHAR*,BOOL,BOOL,bytecode_t**) DECLSPEC_HIDDEN;
+HRESULT compile_script(script_ctx_t*,const WCHAR*,UINT64,unsigned,const WCHAR*,const WCHAR*,BOOL,BOOL,named_item_t*,bytecode_t**) DECLSPEC_HIDDEN;
 void release_bytecode(bytecode_t*) DECLSPEC_HIDDEN;
 
 static inline bytecode_t *bytecode_addref(bytecode_t *code)
diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c
index ef0de96..007aec0 100644
--- a/dlls/jscript/function.c
+++ b/dlls/jscript/function.c
@@ -982,7 +982,8 @@ static HRESULT construct_function(script_ctx_t *ctx, unsigned argc, jsval_t *arg
     if(FAILED(hres))
         return hres;
 
-    hres = compile_script(ctx, str, 0, 0, NULL, NULL, FALSE, FALSE, &code);
+    hres = compile_script(ctx, str, 0, 0, NULL, NULL, FALSE, FALSE,
+                          ctx->call_ctx ? ctx->call_ctx->bytecode->named_item : NULL, &code);
     heap_free(str);
     if(FAILED(hres))
         return hres;
diff --git a/dlls/jscript/global.c b/dlls/jscript/global.c
index e2758bf..2a1dcb2 100644
--- a/dlls/jscript/global.c
+++ b/dlls/jscript/global.c
@@ -204,7 +204,7 @@ HRESULT JSGlobal_eval(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned a
         return E_OUTOFMEMORY;
 
     TRACE("parsing %s\n", debugstr_jsval(argv[0]));
-    hres = compile_script(ctx, src, 0, 0, NULL, NULL, TRUE, FALSE, &code);
+    hres = compile_script(ctx, src, 0, 0, NULL, NULL, TRUE, FALSE, frame ? frame->bytecode->named_item : NULL, &code);
     if(FAILED(hres)) {
         WARN("parse (%s) failed: %08x\n", debugstr_jsval(argv[0]), hres);
         return hres;
diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c
index 36d9f84..694ea8b 100644
--- a/dlls/jscript/jscript.c
+++ b/dlls/jscript/jscript.c
@@ -983,6 +983,7 @@ static HRESULT WINAPI JScriptParse_ParseScriptText(IActiveScriptParse *iface,
         DWORD dwFlags, VARIANT *pvarResult, EXCEPINFO *pexcepinfo)
 {
     JScript *This = impl_from_IActiveScriptParse(iface);
+    named_item_t *item = NULL;
     bytecode_t *code;
     jsexcept_t ei;
     HRESULT hres;
@@ -994,9 +995,17 @@ static HRESULT WINAPI JScriptParse_ParseScriptText(IActiveScriptParse *iface,
     if(This->thread_id != GetCurrentThreadId() || This->ctx->state == SCRIPTSTATE_CLOSED)
         return E_UNEXPECTED;
 
+    if(pstrItemName) {
+        item = lookup_named_item(This->ctx, pstrItemName, 0);
+        if(!item) {
+            WARN("Unknown context %s\n", debugstr_w(pstrItemName));
+            return E_INVALIDARG;
+        }
+    }
+
     enter_script(This->ctx, &ei);
     hres = compile_script(This->ctx, pstrCode, dwSourceContextCookie, ulStartingLine, NULL, pstrDelimiter,
-            (dwFlags & SCRIPTTEXT_ISEXPRESSION) != 0, This->is_encode, &code);
+            (dwFlags & SCRIPTTEXT_ISEXPRESSION) != 0, This->is_encode, item, &code);
     if(FAILED(hres))
         return leave_script(This->ctx, hres);
 
@@ -1075,6 +1084,7 @@ static HRESULT WINAPI JScriptParseProcedure_ParseProcedureText(IActiveScriptPars
         CTXARG_T dwSourceContextCookie, ULONG ulStartingLineNumber, DWORD dwFlags, IDispatch **ppdisp)
 {
     JScript *This = impl_from_IActiveScriptParseProcedure2(iface);
+    named_item_t *item = NULL;
     bytecode_t *code;
     jsdisp_t *dispex;
     jsexcept_t ei;
@@ -1087,9 +1097,17 @@ static HRESULT WINAPI JScriptParseProcedure_ParseProcedureText(IActiveScriptPars
     if(This->thread_id != GetCurrentThreadId() || This->ctx->state == SCRIPTSTATE_CLOSED)
         return E_UNEXPECTED;
 
+    if(pstrItemName) {
+        item = lookup_named_item(This->ctx, pstrItemName, 0);
+        if(!item) {
+            WARN("Unknown context %s\n", debugstr_w(pstrItemName));
+            return E_INVALIDARG;
+        }
+    }
+
     enter_script(This->ctx, &ei);
     hres = compile_script(This->ctx, pstrCode, dwSourceContextCookie, ulStartingLineNumber, pstrFormalParams,
-                          pstrDelimiter, FALSE, This->is_encode, &code);
+                          pstrDelimiter, FALSE, This->is_encode, item, &code);
     if(SUCCEEDED(hres))
         hres = create_source_function(This->ctx, code, &code->global_code, NULL,  &dispex);
     release_bytecode(code);
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 8d4f6e1..56b8cff 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1267,6 +1267,9 @@ static void test_named_items(void)
     parse_script(parse, L"visibleItem.testCall();");
     CHECK_CALLED(testCall);
 
+    hr = IActiveScriptParse_ParseScriptText(parse, L"function testFunc() { }", L"CodeOnlyItem", NULL, NULL, 0, 0, 0, NULL, NULL);
+    ok(hr == E_INVALIDARG, "ParseScriptText returned: %08x\n", hr);
+
     SET_EXPECT(OnEnterScript);
     SET_EXPECT(GetIDsOfNames);
     SET_EXPECT(OnLeaveScript);
-- 
2.21.0

[PATCH v6 2/8] jscript: Add stub implementation of separate script dispatches for named items.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/engine.c  |  7 +++++++
 dlls/jscript/jscript.c | 34 ++++++++++++++++++++++++++++++++++
 dlls/jscript/jscript.h |  2 ++
 3 files changed, 43 insertions(+)

diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c
index ce7fbbc..5b96895 100644
--- a/dlls/jscript/engine.c
+++ b/dlls/jscript/engine.c
@@ -2990,6 +2990,13 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
     unsigned i;
     HRESULT hres;
 
+    if(bytecode->named_item) {
+        if(!bytecode->named_item->script_obj) {
+            hres = create_named_item_script_obj(ctx, bytecode->named_item);
+            if(FAILED(hres)) return hres;
+        }
+    }
+
     if(!ctx->ei->enter_notified) {
         ctx->ei->enter_notified = TRUE;
         IActiveScriptSite_OnEnterScript(ctx->site);
diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c
index 694ea8b..6281b20 100644
--- a/dlls/jscript/jscript.c
+++ b/dlls/jscript/jscript.c
@@ -108,6 +108,22 @@ static inline BOOL is_started(script_ctx_t *ctx)
         || ctx->state == SCRIPTSTATE_DISCONNECTED;
 }
 
+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 void release_named_item_script_obj(named_item_t *item)
+{
+    if(!item->script_obj) return;
+
+    jsdisp_release(item->script_obj);
+    item->script_obj = NULL;
+}
+
 named_item_t *lookup_named_item(script_ctx_t *ctx, const WCHAR *item_name, unsigned flags)
 {
     named_item_t *item;
@@ -115,6 +131,10 @@ named_item_t *lookup_named_item(script_ctx_t *ctx, const WCHAR *item_name, unsig
 
     LIST_FOR_EACH_ENTRY(item, &ctx->named_items, named_item_t, entry) {
         if((item->flags & flags) == flags && !wcscmp(item->name, item_name)) {
+            if(!item->script_obj && !(item->flags & SCRIPTITEM_GLOBALMEMBERS)) {
+                hr = create_named_item_script_obj(ctx, item);
+                if(FAILED(hr)) return NULL;
+            }
             if(!item->disp && (flags || !(item->flags & SCRIPTITEM_CODEONLY))) {
                 IUnknown *unk;
 
@@ -371,6 +391,15 @@ static void clear_persistent_code_list(JScript *This)
     }
 }
 
+static void release_persistent_script_objs(JScript *This)
+{
+    bytecode_t *iter;
+
+    LIST_FOR_EACH_ENTRY(iter, &This->persistent_code, bytecode_t, entry)
+        if(iter->named_item)
+            release_named_item_script_obj(iter->named_item);
+}
+
 static void exec_queued_code(JScript *This)
 {
     bytecode_t *iter;
@@ -408,6 +437,7 @@ static void decrease_state(JScript *This, SCRIPTSTATE state)
             /* FALLTHROUGH */
         case SCRIPTSTATE_INITIALIZED:
             clear_script_queue(This);
+            release_persistent_script_objs(This);
 
             while(!list_empty(&This->ctx->named_items)) {
                 named_item_t *iter = LIST_ENTRY(list_head(&This->ctx->named_items), named_item_t, entry);
@@ -415,6 +445,7 @@ static void decrease_state(JScript *This, SCRIPTSTATE state)
                 list_remove(&iter->entry);
                 if(iter->disp)
                     IDispatch_Release(iter->disp);
+                release_named_item_script_obj(iter);
                 release_named_item(iter);
             }
 
@@ -827,6 +858,7 @@ static HRESULT WINAPI JScript_AddNamedItem(IActiveScript *iface,
     item->ref = 1;
     item->disp = disp;
     item->flags = dwFlags;
+    item->script_obj = NULL;
     item->name = heap_strdupW(pstrName);
     if(!item->name) {
         if(disp)
@@ -1001,6 +1033,7 @@ static HRESULT WINAPI JScriptParse_ParseScriptText(IActiveScriptParse *iface,
             WARN("Unknown context %s\n", debugstr_w(pstrItemName));
             return E_INVALIDARG;
         }
+        if(!item->script_obj) item = NULL;
     }
 
     enter_script(This->ctx, &ei);
@@ -1103,6 +1136,7 @@ static HRESULT WINAPI JScriptParseProcedure_ParseProcedureText(IActiveScriptPars
             WARN("Unknown context %s\n", debugstr_w(pstrItemName));
             return E_INVALIDARG;
         }
+        if(!item->script_obj) item = NULL;
     }
 
     enter_script(This->ctx, &ei);
diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h
index f9d6992..804f95e 100644
--- a/dlls/jscript/jscript.h
+++ b/dlls/jscript/jscript.h
@@ -205,6 +205,7 @@ 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;
     DWORD flags;
@@ -213,6 +214,7 @@ typedef struct named_item_t {
     struct list entry;
 } named_item_t;
 
+HRESULT create_named_item_script_obj(script_ctx_t*,named_item_t*) DECLSPEC_HIDDEN;
 named_item_t *lookup_named_item(script_ctx_t*,const WCHAR*,unsigned) DECLSPEC_HIDDEN;
 void release_named_item(named_item_t*) DECLSPEC_HIDDEN;
 
-- 
2.21.0

[PATCH v6 2/8] jscript: Add stub implementation of separate script dispatches for named items.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

[PATCH v6 3/8] jscript: Retrieve the proper script dispatch from the named item in GetScriptDispatch.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/jscript.c       | 10 +++++++++-
 dlls/jscript/tests/jscript.c |  5 +++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c
index 6281b20..aabad7e 100644
--- a/dlls/jscript/jscript.c
+++ b/dlls/jscript/jscript.c
@@ -883,6 +883,7 @@ static HRESULT WINAPI JScript_GetScriptDispatch(IActiveScript *iface, LPCOLESTR
                                                 IDispatch **ppdisp)
 {
     JScript *This = impl_from_IActiveScript(iface);
+    jsdisp_t *script_obj;
 
     TRACE("(%p)->(%s %p)\n", This, debugstr_w(pstrItemName), ppdisp);
 
@@ -894,7 +895,14 @@ static HRESULT WINAPI JScript_GetScriptDispatch(IActiveScript *iface, LPCOLESTR
         return E_UNEXPECTED;
     }
 
-    *ppdisp = to_disp(This->ctx->global);
+    script_obj = This->ctx->global;
+    if(pstrItemName) {
+        named_item_t *item = lookup_named_item(This->ctx, pstrItemName, 0);
+        if(!item) return E_INVALIDARG;
+        if(item->script_obj) script_obj = item->script_obj;
+    }
+
+    *ppdisp = to_disp(script_obj);
     IDispatch_AddRef(*ppdisp);
     return S_OK;
 }
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 56b8cff..be1320f 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1183,6 +1183,11 @@ static void test_named_items(void)
     ok(visible_named_item_ref == 0, "visible_named_item_ref = %u\n", visible_named_item_ref);
     ok(visible_code_named_item_ref == 0, "visible_code_named_item_ref = %u\n", visible_code_named_item_ref);
 
+    hr = IActiveScript_GetScriptDispatch(script, L"noContext", &disp);
+    ok(hr == E_INVALIDARG, "GetScriptDispatch returned: %08x\n", hr);
+    hr = IActiveScript_GetScriptDispatch(script, L"codeONLYItem", &disp);
+    ok(hr == E_INVALIDARG, "GetScriptDispatch returned: %08x\n", hr);
+
     SET_EXPECT(GetItemInfo_global_code);
     hr = IActiveScript_AddNamedItem(script, L"globalCodeItem", SCRIPTITEM_GLOBALMEMBERS | SCRIPTITEM_CODEONLY);
     ok(hr == S_OK, "AddNamedItem failed: %08x\n", hr);
-- 
2.21.0

[PATCH v6 3/8] jscript: Retrieve the proper script dispatch from the named item in GetScriptDispatch.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

[PATCH v6 4/8] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/dispex.c |  1 +
 dlls/jscript/engine.c | 19 +++++++++++++++++--
 dlls/jscript/engine.h |  1 +
 dlls/jscript/global.c |  4 +++-
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
index e986de6..a0c8f38 100644
--- a/dlls/jscript/dispex.c
+++ b/dlls/jscript/dispex.c
@@ -1527,6 +1527,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
     }
 
     enter_script(This->ctx, &ei);
+    ei.script_obj = This;
 
     switch(wFlags) {
     case DISPATCH_METHOD|DISPATCH_PROPERTYGET:
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) {
diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h
index fe6b2a0..4057f9c 100644
--- a/dlls/jscript/engine.h
+++ b/dlls/jscript/engine.h
@@ -233,6 +233,7 @@ struct _jsexcept_t {
     jsstr_t *message;
     jsstr_t *line;
 
+    jsdisp_t *script_obj;
     bytecode_t *code;
     unsigned loc;
 
diff --git a/dlls/jscript/global.c b/dlls/jscript/global.c
index 2a1dcb2..40fa951 100644
--- a/dlls/jscript/global.c
+++ b/dlls/jscript/global.c
@@ -181,6 +181,7 @@ HRESULT JSGlobal_eval(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned a
 {
     call_frame_t *frame = ctx->call_ctx;
     DWORD exec_flags = EXEC_EVAL;
+    jsdisp_t *context;
     bytecode_t *code;
     const WCHAR *src;
     HRESULT hres;
@@ -209,13 +210,14 @@ HRESULT JSGlobal_eval(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned a
         WARN("parse (%s) failed: %08x\n", debugstr_jsval(argv[0]), hres);
         return hres;
     }
+    context = frame ? frame->variable_obj : (ctx->ei->script_obj ? ctx->ei->script_obj : ctx->global);
 
     if(!frame || (frame->flags & EXEC_GLOBAL))
         exec_flags |= EXEC_GLOBAL;
     if(flags & DISPATCH_JSCRIPT_CALLEREXECSSOURCE)
         exec_flags |= EXEC_RETURN_TO_INTERP;
     hres = exec_source(ctx, exec_flags, code, &code->global_code, frame ? frame->scope : NULL,
-            frame ? frame->this_obj : NULL, NULL, frame ? frame->variable_obj : ctx->global, 0, NULL, r);
+            frame ? frame->this_obj : NULL, NULL, context, 0, NULL, r);
     release_bytecode(code);
     return hres;
 }
-- 
2.21.0

[PATCH v6 4/8] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Jacek Caban]

Hi Gabriel,

On 07.03.2020 14:40, Gabriel Ivăncescu wrote:
> diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
> index e986de6..a0c8f38 100644
> --- a/dlls/jscript/dispex.c
> +++ b/dlls/jscript/dispex.c
> @@ -1527,6 +1527,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
>       }
>
> enter_script(This->ctx, &ei);
> +    ei.script_obj = This;

This doesn't look right. What are you trying to handle here? Note that 
IDispatchEx is used by regular JavaScript objects and using such objects 
and script_obj doesn't seem right. Even if the object is a script 
dispatch, I'm not sure this is right. In fact, I'm not yet convinced 
that we need script_obj in ei at all.


Thanks,

Jacek

[PATCH v6 4/8] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Gabriel Ivăncescu]

Hi Jacek,

On 09/03/2020 16:05, Jacek Caban wrote:
> Hi Gabriel,
>
> On 07.03.2020 14:40, Gabriel Ivăncescu wrote:
> > diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
> > index e986de6..a0c8f38 100644
> > --- a/dlls/jscript/dispex.c
> > +++ b/dlls/jscript/dispex.c
> > @@ -1527,6 +1527,7 @@ static HRESULT WINAPI 
> > DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
> >       }
> >       enter_script(This->ctx, &ei);
> > +    ei.script_obj = This;
>
> This doesn't look right. What are you trying to handle here? Note that 
> IDispatchEx is used by regular JavaScript objects and using such objects 
> and script_obj doesn't seem right. Even if the object is a script 
> dispatch, I'm not sure this is right. In fact, I'm not yet convinced 
> that we need script_obj in ei at all.
>
> Thanks,
>
> Jacek

Oh, I was just concerned about 'eval' being called somehow from 
IDispatch directly without going through exec_source, because the eval 
code does seem to assume that 'frame' (i.e. ctx->call_ctx) can be NULL, 
which would mean we can't access the bytecode's named item (this check 
was there before my patches).

I can drop it, of course, if it's not needed. Perhaps that NULL check in 
eval is redundant, then?

Thanks,
Gabriel

[PATCH v6 4/8] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Jacek Caban]

Hi Gabriel,

On 09/03/2020 16:57, Gabriel Ivăncescu wrote:
> Hi Jacek,
>
> On 09/03/2020 16:05, Jacek Caban wrote:
> > Hi Gabriel,
> >
> > On 07.03.2020 14:40, Gabriel Ivăncescu wrote:
> > > diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
> > > index e986de6..a0c8f38 100644
> > > --- a/dlls/jscript/dispex.c
> > > +++ b/dlls/jscript/dispex.c
> > > @@ -1527,6 +1527,7 @@ static HRESULT WINAPI 
> > > DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
> > >       }
> > >       enter_script(This->ctx, &ei);
> > > +    ei.script_obj = This;
> >
> > This doesn't look right. What are you trying to handle here? Note 
> > that IDispatchEx is used by regular JavaScript objects and using such 
> > objects and script_obj doesn't seem right. Even if the object is a 
> > script dispatch, I'm not sure this is right. In fact, I'm not yet 
> > convinced that we need script_obj in ei at all.
> >
> > Thanks,
> >
> > Jacek
>
> Oh, I was just concerned about 'eval' being called somehow from 
> IDispatch directly without going through exec_source, because the eval 
> code does seem to assume that 'frame' (i.e. ctx->call_ctx) can be 
> NULL, which would mean we can't access the bytecode's named item (this 
> check was there before my patches).
>
> I can drop it, of course, if it's not needed. Perhaps that NULL check 
> in eval is redundant, then?

NULL check in eval is still needed. I don't know what's the exact 
expected behavior in case of this, but treating it as global code does 
not seem wrong.


The way you set it will not do the right thing when used with a regular 
JS object:

o = { f: eval };

If you call o.f() by IDispatchEx, your patch would use o as a script object.


That said, I'm fine with treating such calls as global code, which 
simply means using NULL script object and not touching jsexcept_t. If 
you'd want to dig deeper, it would need more tests.


Thanks,

Jacek

[PATCH v6 5/8] jscript: Store the proper 'this' object for bytecodes with named items.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/engine.c        | 5 +++++
 dlls/jscript/tests/jscript.c | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c
index 044e32c..3e32ea0 100644
--- a/dlls/jscript/engine.c
+++ b/dlls/jscript/engine.c
@@ -3099,6 +3099,11 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
         frame->this_obj = this_obj;
         IDispatch_AddRef(frame->this_obj);
     }
+    else if(bytecode->named_item) {
+        frame->this_obj = (bytecode->named_item->flags & SCRIPTITEM_CODEONLY)
+                          ? to_disp(bytecode->named_item->script_obj) : bytecode->named_item->disp;
+        IDispatch_AddRef(frame->this_obj);
+    }
 
     if(function_instance)
         frame->function_instance = jsdisp_addref(function_instance);
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index be1320f..45d4620 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1250,7 +1250,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);
@@ -1412,7 +1411,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);
@@ -1423,7 +1421,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);
-- 
2.21.0

[PATCH v6 6/8] jscript: Don't lookup global members for bytecodes with a named item.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/engine.c        | 4 +++-
 dlls/jscript/tests/jscript.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c
index 3e32ea0..c8d33cb 100644
--- a/dlls/jscript/engine.c
+++ b/dlls/jscript/engine.c
@@ -3034,6 +3034,8 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
     }
 
     if(flags & (EXEC_GLOBAL | EXEC_EVAL)) {
+        BOOL lookup_globals = (flags & EXEC_GLOBAL) && !bytecode->named_item;
+
         for(i=0; i < function->var_cnt; i++) {
             TRACE("[%d] %s %d\n", i, debugstr_w(function->variables[i].name), function->variables[i].func_id);
             if(function->variables[i].func_id != -1) {
@@ -3045,7 +3047,7 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi
 
                 hres = jsdisp_propput_name(variable_obj, function->variables[i].name, jsval_obj(func_obj));
                 jsdisp_release(func_obj);
-            }else if(!(flags & EXEC_GLOBAL) || !lookup_global_members(ctx, function->variables[i].name, NULL)) {
+            }else if(!lookup_globals || !lookup_global_members(ctx, function->variables[i].name, NULL)) {
                 DISPID id = 0;
 
                 hres = jsdisp_get_id(variable_obj, function->variables[i].name, fdexNameEnsure, &id);
diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 45d4620..57d1636 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1306,7 +1306,7 @@ static void test_named_items(void)
     ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr);
     CHECK_CALLED(OnEnterScript);
     CHECK_CALLED(OnLeaveScript);
-    todo_wine CHECK_NOT_CALLED(GetIDsOfNames);
+    CHECK_NOT_CALLED(GetIDsOfNames);
 
     SET_EXPECT(OnEnterScript);
     SET_EXPECT(OnLeaveScript);
-- 
2.21.0

[PATCH v6 7/8] jscript: Create separate script dispatches for each named item.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 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 57d1636..8990257 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1201,7 +1201,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);
@@ -1332,9 +1331,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);
     }
@@ -1344,9 +1341,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);
@@ -1380,12 +1375,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);
 
@@ -1495,9 +1487,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);
     }
@@ -1536,9 +1526,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);
     }
@@ -1548,9 +1536,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);
     }
@@ -1578,10 +1564,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);
 
@@ -1589,10 +1573,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);
     }
-- 
2.21.0

[PATCH v6 8/8] jscript/tests: Add test for function constructor in named item context.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/tests/jscript.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/dlls/jscript/tests/jscript.c b/dlls/jscript/tests/jscript.c
index 8990257..ac6a5d1 100644
--- a/dlls/jscript/tests/jscript.c
+++ b/dlls/jscript/tests/jscript.c
@@ -1129,13 +1129,15 @@ static void test_named_items(void)
     static const WCHAR *context_idents[] =
     {
         L"testFunc",
-        L"testVar"
+        L"testVar",
+        L"testFuncConstr"
     };
     static const WCHAR *context_code_test[] =
     {
         L"testFunc();",
         L"if(testVar != 42) throw new Error();",
-        L"if(Math.abs(-testVar) != 42) throw new Error();"
+        L"if(Math.abs(-testVar) != 42) throw new Error();",
+        L"if(testFuncConstr() != testVar) throw new Error();"
     };
     IDispatchEx *dispex, *dispex2;
     IActiveScriptParse *parse;
@@ -1300,7 +1302,8 @@ static void test_named_items(void)
     SET_EXPECT(GetIDsOfNames);
     hr = IActiveScriptParse_ParseScriptText(parse, L""
         "var testVar = 42;\n"
-        "testVar_global = 5;\n",
+        "testVar_global = 5;\n"
+        "var testFuncConstr = new Function(\"return testVar;\");\n",
         L"codeOnlyItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISPERSISTENT, NULL, NULL);
     ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr);
     CHECK_CALLED(OnEnterScript);
-- 
2.21.0

[PATCH v6 8/8] jscript/tests: Add test for function constructor in named item context.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

---

Applies without the rest of the series.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>