[PATCH v8 1/3] jscript: Lookup and ref the named item's dispatch first, during interpretion.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- 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) { -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- 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); -- 2.21.0
Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
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 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); } -- 2.21.0
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
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
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
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban