[PATCH v7 1/4] jscript: Lookup and ref the named item's dispatch first, during interpretion.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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) {
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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 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);
On 10.03.2020 14:04, Gabriel Ivăncescu wrote:
}
- 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);- }
Is there any reason you're doing this here instead of interp_this?
Thanks,
Jacek
On 10/03/2020 17:41, Jacek Caban wrote:
On 10.03.2020 14:04, Gabriel Ivăncescu wrote:
} + 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); + }
Is there any reason you're doing this here instead of interp_this?
Thanks,
Jacek
Probably not, I just forgot to rebase it when I did the change. I'll move it to interp_this, then, after I test it.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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 1246f2d..a5bc05a 100644 --- a/dlls/jscript/tests/jscript.c +++ b/dlls/jscript/tests/jscript.c @@ -1309,7 +1309,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);
Hi Gabriel,
On 10.03.2020 14:04, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescugabrielopcode@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);
I think that the real problem is that we pass wrong variable_obj to exec_source in this case.
Thanks,
Jacek
On 10/03/2020 17:44, Jacek Caban wrote:
Hi Gabriel,
On 10.03.2020 14:04, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescugabrielopcode@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);
I think that the real problem is that we pass wrong variable_obj to exec_source in this case.
Thanks,
Jacek
Sorry, I don't understand what you mean. The function "lookup_global_members" must not be called if we're in a named item context, since those are part of the global scope only, as evidenced by tests.
I don't see how variable_obj is used anywhere before that, except to assign it to the named item dispatch. It has to be done here, because the dispatch is potentially created here (on demand) so it may not exist before at the caller site.
Can you please clarify?
Thanks, Gabriel
On 10.03.2020 18:19, Gabriel Ivăncescu wrote:
On 10/03/2020 17:44, Jacek Caban wrote:
Hi Gabriel,
On 10.03.2020 14:04, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescugabrielopcode@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);
I think that the real problem is that we pass wrong variable_obj to exec_source in this case.
Thanks,
Jacek
Sorry, I don't understand what you mean. The function "lookup_global_members" must not be called if we're in a named item context, since those are part of the global scope only, as evidenced by tests.
I don't see how variable_obj is used anywhere before that, except to assign it to the named item dispatch. It has to be done here, because the dispatch is potentially created here (on demand) so it may not exist before at the caller site.
Okay, the patch is fine.
Thanks,
Jacek
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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 a5bc05a..ac6a5d1 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); }
-
Gabriel Ivăncescu -
Jacek Caban