[PATCH v21 0/4] MR10393: Draft: vbscript: Check named item dispatch for Dim variable names.
When declaring variables with Dim in a visible named item context, check the named item's IDispatch for each variable name via GetIDsOfNames. This matches Windows VBScript behavior where Dim probes the host object before creating the variable. -- v21: vbscript: Use a separate dispatch for top-level declaration probes. vbscript/tests: Cover named-item dim-probe IDispatch fetching corner cases. https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> When parsing top-level declarations (Dim variables, Subs, Functions, Classes) in a visible named item context, the first such parse release-and-refetches the host IDispatch and probes each declared name on it via GetIDsOfNames. This matches Windows VBScript behavior; the variable/sub/function/class is always created regardless of whether the name exists on the host dispatch. Subsequent parses on the same named item reuse the cached dispatch and only probe newly declared names. --- dlls/vbscript/vbscript.c | 58 ++++++++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 1 + 2 files changed, 59 insertions(+) diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index ff28b3b17ef..a758caa9d40 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -93,6 +93,8 @@ static inline BOOL is_started(VBScript *This) || This->state == SCRIPTSTATE_DISCONNECTED; } +static HRESULT retrieve_named_item_disp(IActiveScriptSite *site, named_item_t *item); + HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL extern_caller) { ScriptDisp *obj = ctx->script_obj; @@ -138,6 +140,61 @@ HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL obj->global_funcs_size = cnt; } + /* For visible named items, the first exec_global_code with top-level + declarations release-and-refetches the host dispatch, then probes + each declared name (Dim variables, Subs, Functions, Classes) via + GetIDsOfNames. Subsequent parses reuse the cache. Names are always + declared regardless of whether they exist on the host dispatch. + TODO: It is unclear why Windows refetches the host dispatch on the + first declaration when the result is not used to skip name + creation. Perhaps it is used for some side effect or diagnostic + purpose that we have not yet identified. */ + if(code->named_item && !(code->named_item->flags & SCRIPTITEM_CODEONLY) + && (code->main_code.var_cnt || code->funcs || code->classes)) + { + if(!code->named_item->dim_disp_probed) { + if(code->named_item->disp) { + IDispatch_Release(code->named_item->disp); + code->named_item->disp = NULL; + } + retrieve_named_item_disp(ctx->site, code->named_item); + code->named_item->dim_disp_probed = TRUE; + } + } + + if(code->named_item && code->named_item->disp + && !(code->named_item->flags & SCRIPTITEM_CODEONLY)) + { + class_desc_t *class_iter; + + /* Probe top-level declared names (Dim vars, Subs/Functions, + Classes) on the host dispatch. */ + for (i = 0; i < code->main_code.var_cnt; i++) { + BSTR name = SysAllocString(code->main_code.vars[i].name); + if(name) { + DISPID id; + disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } + } + for (func_iter = code->funcs; func_iter; func_iter = func_iter->next) { + BSTR name = SysAllocString(func_iter->name); + if(name) { + DISPID id; + disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } + } + for (class_iter = code->classes; class_iter; class_iter = class_iter->next) { + BSTR name = SysAllocString(class_iter->name); + if(name) { + DISPID id; + disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } + } + } + for (i = 0; i < code->main_code.var_cnt; i++) { if (script_disp_find_var(obj, code->main_code.vars[i].name)) @@ -869,6 +926,7 @@ static HRESULT WINAPI VBScript_AddNamedItem(IActiveScript *iface, LPCOLESTR pstr item->disp = disp; item->flags = dwFlags; item->script_obj = NULL; + item->dim_disp_probed = FALSE; item->name = wcsdup(pstrName); if(!item->name) { if(disp) diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 1420e2da0d1..b0a674cc67f 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -163,6 +163,7 @@ typedef struct named_item_t { unsigned ref; DWORD flags; LPWSTR name; + BOOL dim_disp_probed; struct list entry; } named_item_t; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> Native VBScript calls GetIDsOfNames once per top-level declared name (Dim variable, Sub, Function, Class). Cover Dim, Sub, Function, and Class shapes plus a multi-Dim and dim+sub combination, asserting the exact host probe count for each. --- dlls/vbscript/tests/vbscript.c | 88 +++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 7500ec30a8d..2e2bbef2a1b 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -2088,8 +2088,8 @@ static void test_named_items(void) hres = IActiveScriptParse_ParseScriptText(parse, L"dim abc\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); CHECK_CALLED(OnEnterScript); - todo_wine CHECK_CALLED(GetItemInfo_visible); - todo_wine CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(GetItemInfo_visible); + CHECK_CALLED(GetIDsOfNames_visible); CHECK_CALLED(OnLeaveScript); SET_EXPECT(OnEnterScript); SET_EXPECT(OnLeaveScript); @@ -2113,6 +2113,90 @@ static void test_named_items(void) CHECK_CALLED(OnEnterScript); CHECK_CALLED(OnLeaveScript); + /* Probe: multiple names in a single Dim statement. Each name should + * be probed against the host dispatch via GetIDsOfNames. The host + * dispatch is already cached from the earlier "dim abc" parse, so + * GetItemInfo is not refetched. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT_MULTI(GetIDsOfNames_visible, 3); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_a, probe_b, probe_c\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED_MULTI(GetIDsOfNames_visible, 3); + CHECK_CALLED(OnLeaveScript); + + /* Probe: Dim with explicit array bounds. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_arr(5)\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + /* Native probes the host with GetIDsOfNames once per top-level + * declared name, which includes bare Subs, Functions, and Classes + * (in addition to Dim variables). Inner Dims inside a Sub body are + * not probed. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"sub probe_sub\ndim probe_local\nend sub\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"class probe_cls\npublic probe_prop\nend class\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"sub probe_named_sub\nend sub\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"function probe_fn\nend function\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + SET_EXPECT(OnEnterScript); + SET_EXPECT_MULTI(GetIDsOfNames_visible, 2); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_top\nsub probe_s3\nend sub\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED_MULTI(GetIDsOfNames_visible, 2); + CHECK_CALLED(OnLeaveScript); + + /* Probe: a second top-level Dim parse on the same named item. The + * host dispatch is already cached, so GetItemInfo should not fire + * a second time. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_second\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + SET_EXPECT(OnEnterScript); SET_EXPECT(OnLeaveScript); hres = IActiveScriptParse_ParseScriptText(parse, L"set global_me = me\n", L"globalItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISPERSISTENT, NULL, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> Adds three tests probing the host-dispatch fetches done by native VBScript at top-level declaration parse time: - test_named_item_dim_first_use_no_double_fetch: when a dim parse is the first use of an ISVISIBLE item, native makes exactly one GetItemInfo call. Wine currently fetches twice (compile_script's lookup_named_item plus exec_global_code's release-and-refetch). - test_named_item_dim_two_slot_rotating: with a host that returns a different IDispatch on consecutive GetItemInfo calls, native keeps the first-fetched dispatch as the runtime cache and uses a separate refetch for the dim probe; subsequent qualified access stays on the original. Wine merges the two into a single slot, so the probe refetch overwrites the runtime cache. - test_named_item_globalmembers_dim_no_refetch: a dim parse on a GLOBALMEMBERS item does not trigger any GetItemInfo refetch; this documents existing behavior to guard against regressions. --- dlls/vbscript/tests/vbscript.c | 288 +++++++++++++++++++++++++++++++++ 1 file changed, 288 insertions(+) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 2e2bbef2a1b..3aac33b52da 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -105,6 +105,12 @@ DEFINE_EXPECT(GetItemInfo_visible); DEFINE_EXPECT(GetItemInfo_visible_code); DEFINE_EXPECT(GetItemInfo_persistent); DEFINE_EXPECT(testCall); +DEFINE_EXPECT(GetItemInfo_rotating); +DEFINE_EXPECT(GetIDsOfNames_rotating); +DEFINE_EXPECT(testCall_rotating_a); +DEFINE_EXPECT(testCall_rotating_b); + +static int rotating_get_item_idx; DEFINE_GUID(CLSID_VBScript, 0xb54f3741, 0x5b07, 0x11cf, 0xa4,0xb0, 0x00,0xaa,0x00,0x4a,0x55,0xe8); DEFINE_GUID(CLSID_VBScriptRegExp, 0x3f4daca4, 0x160d, 0x11d2, 0xa8,0xe9, 0x00,0x10,0x4b,0x36,0x5c,0x9f); @@ -301,6 +307,57 @@ static const IDispatchVtbl persistent_named_item_vtbl = { static IDispatch persistent_named_item = { &persistent_named_item_vtbl }; +static HRESULT WINAPI rotating_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *names, UINT name_cnt, + LCID lcid, DISPID *ids) +{ + ok(name_cnt == 1, "name_cnt = %u\n", name_cnt); + if(!wcscmp(names[0], L"testCall")) { + *ids = 1; + return S_OK; + } + CHECK_EXPECT2(GetIDsOfNames_rotating); + return DISP_E_UNKNOWNNAME; +} + +static HRESULT WINAPI rotating_a_Invoke(IDispatch *iface, DISPID id, REFIID riid, LCID lcid, WORD flags, + DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, UINT *err) +{ + CHECK_EXPECT(testCall_rotating_a); + ok(id == 1, "id = %lu\n", id); + return S_OK; +} + +static HRESULT WINAPI rotating_b_Invoke(IDispatch *iface, DISPID id, REFIID riid, LCID lcid, WORD flags, + DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, UINT *err) +{ + CHECK_EXPECT(testCall_rotating_b); + ok(id == 1, "id = %lu\n", id); + return S_OK; +} + +static const IDispatchVtbl rotating_a_vtbl = { + Dispatch_QueryInterface, + Dispatch_AddRef, + Dispatch_Release, + Dispatch_GetTypeInfoCount, + Dispatch_GetTypeInfo, + rotating_GetIDsOfNames, + rotating_a_Invoke +}; + +static const IDispatchVtbl rotating_b_vtbl = { + Dispatch_QueryInterface, + Dispatch_AddRef, + Dispatch_Release, + Dispatch_GetTypeInfoCount, + Dispatch_GetTypeInfo, + rotating_GetIDsOfNames, + rotating_b_Invoke +}; + +static IDispatch rotating_a_named_item = { &rotating_a_vtbl }; +static IDispatch rotating_b_named_item = { &rotating_b_vtbl }; + static HRESULT WINAPI ActiveScriptSite_QueryInterface(IActiveScriptSite *iface, REFIID riid, void **ppv) { *ppv = NULL; @@ -366,6 +423,17 @@ static HRESULT WINAPI ActiveScriptSite_GetItemInfo(IActiveScriptSite *iface, LPC *item_unk = (IUnknown*)&persistent_named_item; return S_OK; } + if(!wcscmp(name, L"rotatingItem")) { + CHECK_EXPECT2(GetItemInfo_rotating); + if(rotating_get_item_idx++ == 0) { + IDispatch_AddRef(&rotating_a_named_item); + *item_unk = (IUnknown*)&rotating_a_named_item; + } else { + IDispatch_AddRef(&rotating_b_named_item); + *item_unk = (IUnknown*)&rotating_b_named_item; + } + return S_OK; + } ok(0, "unexpected call %s\n", wine_dbgstr_w(name)); return E_NOTIMPL; } @@ -2604,6 +2672,223 @@ static void test_named_items(void) ok(!ref, "ref = %ld\n", ref); } +static void test_named_item_dim_first_use_no_double_fetch(void) +{ + IActiveScriptParse *parse; + IActiveScript *script; + HRESULT hres; + LONG ref; + + /* When a top-level dim parse is the FIRST use of an ISVISIBLE named item, + * native VBScript fetches the host IDispatch via GetItemInfo exactly once + * (the probe-time fetch). Wine currently fetches twice: once at compile + * time via lookup_named_item and again in exec_global_code's release- + * and-refetch. */ + + script = create_vbscript(); + + hres = IActiveScript_QueryInterface(script, &IID_IActiveScriptParse, (void**)&parse); + ok(hres == S_OK, "Could not get IActiveScriptParse: %08lx\n", hres); + + SET_EXPECT(GetLCID); + hres = IActiveScript_SetScriptSite(script, &ActiveScriptSite); + ok(hres == S_OK, "SetScriptSite failed: %08lx\n", hres); + CHECK_CALLED(GetLCID); + + hres = IActiveScript_AddNamedItem(script, L"visibleItem", SCRIPTITEM_ISVISIBLE); + ok(hres == S_OK, "AddNamedItem failed: %08lx\n", hres); + + SET_EXPECT(OnStateChange_INITIALIZED); + hres = IActiveScriptParse_InitNew(parse); + ok(hres == S_OK, "InitNew failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hres = IActiveScript_SetScriptState(script, SCRIPTSTATE_CONNECTED); + ok(hres == S_OK, "SetScriptState failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_CONNECTED); + + SET_EXPECT_MULTI(GetItemInfo_visible, 2); + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim foo\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + todo_wine ok(called_GetItemInfo_visible == 1, + "GetItemInfo_visible called %d times, expected 1\n", called_GetItemInfo_visible); + expect_GetItemInfo_visible = 0; + called_GetItemInfo_visible = 0; + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hres = IActiveScript_Close(script); + ok(hres == S_OK, "Close failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parse); + ref = IActiveScript_Release(script); + ok(!ref, "ref = %ld\n", ref); +} + +static void test_named_item_dim_two_slot_rotating(void) +{ + IActiveScriptParse *parse; + IActiveScript *script; + HRESULT hres; + LONG ref; + + /* Native VBScript maintains separate IDispatch references for the dim + * probe and runtime access on an ISVISIBLE item. With a host that + * returns a different IDispatch on each GetItemInfo call, native uses + * the first returned dispatch for runtime (cached on first runtime use) + * and a second dispatch for the dim probe; subsequent qualified access + * keeps using the original runtime dispatch. Wine currently has a + * single slot, so the dim refetch overwrites the runtime cache. */ + + rotating_get_item_idx = 0; + + script = create_vbscript(); + hres = IActiveScript_QueryInterface(script, &IID_IActiveScriptParse, (void**)&parse); + ok(hres == S_OK, "Could not get IActiveScriptParse: %08lx\n", hres); + + SET_EXPECT(GetLCID); + hres = IActiveScript_SetScriptSite(script, &ActiveScriptSite); + ok(hres == S_OK, "SetScriptSite failed: %08lx\n", hres); + CHECK_CALLED(GetLCID); + + hres = IActiveScript_AddNamedItem(script, L"rotatingItem", SCRIPTITEM_ISVISIBLE); + ok(hres == S_OK, "AddNamedItem failed: %08lx\n", hres); + + SET_EXPECT(OnStateChange_INITIALIZED); + hres = IActiveScriptParse_InitNew(parse); + ok(hres == S_OK, "InitNew failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hres = IActiveScript_SetScriptState(script, SCRIPTSTATE_CONNECTED); + ok(hres == S_OK, "SetScriptState failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_CONNECTED); + + /* First runtime use: GetItemInfo returns rotating_a (idx 0). */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetItemInfo_rotating); + SET_EXPECT(testCall_rotating_a); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"rotatingItem.testCall\n", NULL, NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetItemInfo_rotating); + CHECK_CALLED(testCall_rotating_a); + CHECK_CALLED(OnLeaveScript); + + /* First dim parse on the same item: native fetches a separate probe + * dispatch (rotating_b, idx 1) and probes "foo" on it. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetItemInfo_rotating); + SET_EXPECT(GetIDsOfNames_rotating); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim foo\n", L"rotatingItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetItemInfo_rotating); + CHECK_CALLED(GetIDsOfNames_rotating); + CHECK_CALLED(OnLeaveScript); + + /* Second runtime access: native uses cached rotating_a (the runtime + * slot is independent of the dim probe slot). Wine routes to + * rotating_b because the dim refetch overwrote the single cached + * dispatch. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(testCall_rotating_a); + SET_EXPECT(testCall_rotating_b); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"rotatingItem.testCall\n", NULL, NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + todo_wine ok(called_testCall_rotating_a, "expected testCall on the runtime-cached (first) dispatch\n"); + todo_wine ok(!called_testCall_rotating_b, "wine routed testCall to the dim-refetched dispatch\n"); + expect_testCall_rotating_a = 0; called_testCall_rotating_a = 0; + expect_testCall_rotating_b = 0; called_testCall_rotating_b = 0; + CHECK_CALLED(OnLeaveScript); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hres = IActiveScript_Close(script); + ok(hres == S_OK, "Close failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parse); + ref = IActiveScript_Release(script); + ok(!ref, "ref = %ld\n", ref); +} + +static void test_named_item_globalmembers_dim_no_refetch(void) +{ + IActiveScriptParse *parse; + IActiveScript *script; + HRESULT hres; + LONG ref; + + /* Native VBScript does not refetch the host dispatch via GetItemInfo on + * a dim parse with a SCRIPTITEM_GLOBALMEMBERS item: the eager fetch at + * AddNamedItem time is the only GetItemInfo call. */ + + script = create_vbscript(); + + hres = IActiveScript_QueryInterface(script, &IID_IActiveScriptParse, (void**)&parse); + ok(hres == S_OK, "Could not get IActiveScriptParse: %08lx\n", hres); + + SET_EXPECT(GetLCID); + hres = IActiveScript_SetScriptSite(script, &ActiveScriptSite); + ok(hres == S_OK, "SetScriptSite failed: %08lx\n", hres); + CHECK_CALLED(GetLCID); + + SET_EXPECT(GetItemInfo_global); + hres = IActiveScript_AddNamedItem(script, L"globalItem", SCRIPTITEM_GLOBALMEMBERS); + ok(hres == S_OK, "AddNamedItem failed: %08lx\n", hres); + CHECK_CALLED(GetItemInfo_global); + + SET_EXPECT(OnStateChange_INITIALIZED); + hres = IActiveScriptParse_InitNew(parse); + ok(hres == S_OK, "InitNew failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hres = IActiveScript_SetScriptState(script, SCRIPTSTATE_CONNECTED); + ok(hres == S_OK, "SetScriptState failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_CONNECTED); + + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim foo\n", L"globalItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + ok(!called_GetItemInfo_global, "GetItemInfo refetched on GLOBALMEMBERS dim parse\n"); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hres = IActiveScript_Close(script); + ok(hres == S_OK, "Close failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parse); + ref = IActiveScript_Release(script); + ok(!ref, "ref = %ld\n", ref); +} + static void test_RegExp(void) { IRegExp2 *regexp; @@ -2826,6 +3111,9 @@ START_TEST(vbscript) test_vbscript_initializing(); test_param_ids(); test_named_items(); + test_named_item_dim_first_use_no_double_fetch(); + test_named_item_dim_two_slot_rotating(); + test_named_item_globalmembers_dim_no_refetch(); test_scriptdisp(); test_code_persistence(); test_script_typeinfo(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> Track the probe IDispatch in named_item_t::dim_probe_disp so qualified runtime access keeps using the original named_item_t::disp. Also stop lookup_named_item from eagerly fetching when called with no required- flags filter, which avoided a redundant compile-time GetItemInfo. --- dlls/vbscript/tests/vbscript.c | 8 +-- dlls/vbscript/vbscript.c | 106 +++++++++++++++++---------------- dlls/vbscript/vbscript.h | 1 + 3 files changed, 61 insertions(+), 54 deletions(-) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 3aac33b52da..1dd39d39338 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -2717,8 +2717,8 @@ static void test_named_item_dim_first_use_no_double_fetch(void) CHECK_CALLED(OnEnterScript); CHECK_CALLED(GetIDsOfNames_visible); CHECK_CALLED(OnLeaveScript); - todo_wine ok(called_GetItemInfo_visible == 1, - "GetItemInfo_visible called %d times, expected 1\n", called_GetItemInfo_visible); + ok(called_GetItemInfo_visible == 1, + "GetItemInfo_visible called %d times, expected 1\n", called_GetItemInfo_visible); expect_GetItemInfo_visible = 0; called_GetItemInfo_visible = 0; @@ -2811,8 +2811,8 @@ static void test_named_item_dim_two_slot_rotating(void) hres = IActiveScriptParse_ParseScriptText(parse, L"rotatingItem.testCall\n", NULL, NULL, NULL, 0, 0, 0, NULL, NULL); ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); CHECK_CALLED(OnEnterScript); - todo_wine ok(called_testCall_rotating_a, "expected testCall on the runtime-cached (first) dispatch\n"); - todo_wine ok(!called_testCall_rotating_b, "wine routed testCall to the dim-refetched dispatch\n"); + ok(called_testCall_rotating_a, "expected testCall on the runtime-cached (first) dispatch\n"); + ok(!called_testCall_rotating_b, "wine routed testCall to the dim-refetched dispatch\n"); expect_testCall_rotating_a = 0; called_testCall_rotating_a = 0; expect_testCall_rotating_b = 0; called_testCall_rotating_b = 0; CHECK_CALLED(OnLeaveScript); diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index a758caa9d40..41338ea96de 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -94,6 +94,7 @@ static inline BOOL is_started(VBScript *This) } static HRESULT retrieve_named_item_disp(IActiveScriptSite *site, named_item_t *item); +static HRESULT fetch_named_item_disp(IActiveScriptSite *site, const WCHAR *name, IDispatch **out); HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL extern_caller) { @@ -140,57 +141,48 @@ HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL obj->global_funcs_size = cnt; } - /* For visible named items, the first exec_global_code with top-level - declarations release-and-refetches the host dispatch, then probes - each declared name (Dim variables, Subs, Functions, Classes) via - GetIDsOfNames. Subsequent parses reuse the cache. Names are always - declared regardless of whether they exist on the host dispatch. - TODO: It is unclear why Windows refetches the host dispatch on the - first declaration when the result is not used to skip name - creation. Perhaps it is used for some side effect or diagnostic - purpose that we have not yet identified. */ + /* For visible named items, native VBScript fetches a dedicated host + IDispatch via GetItemInfo on the first top-level declaration parse + and probes each declared name (Dim variables, Subs, Functions, + Classes) via GetIDsOfNames on it. The probe IDispatch is independent + of the runtime cache (named_item->disp) so subsequent qualified + access keeps using the runtime cache. The probe results are never + used to skip name creation; declarations always succeed. */ if(code->named_item && !(code->named_item->flags & SCRIPTITEM_CODEONLY) && (code->main_code.var_cnt || code->funcs || code->classes)) { if(!code->named_item->dim_disp_probed) { - if(code->named_item->disp) { - IDispatch_Release(code->named_item->disp); - code->named_item->disp = NULL; - } - retrieve_named_item_disp(ctx->site, code->named_item); + fetch_named_item_disp(ctx->site, code->named_item->name, &code->named_item->dim_probe_disp); code->named_item->dim_disp_probed = TRUE; } - } - if(code->named_item && code->named_item->disp - && !(code->named_item->flags & SCRIPTITEM_CODEONLY)) - { - class_desc_t *class_iter; - - /* Probe top-level declared names (Dim vars, Subs/Functions, - Classes) on the host dispatch. */ - for (i = 0; i < code->main_code.var_cnt; i++) { - BSTR name = SysAllocString(code->main_code.vars[i].name); - if(name) { - DISPID id; - disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); - SysFreeString(name); + if(code->named_item->dim_probe_disp) { + class_desc_t *class_iter; + IDispatch *probe = code->named_item->dim_probe_disp; + + for (i = 0; i < code->main_code.var_cnt; i++) { + BSTR name = SysAllocString(code->main_code.vars[i].name); + if(name) { + DISPID id; + disp_get_id(probe, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } } - } - for (func_iter = code->funcs; func_iter; func_iter = func_iter->next) { - BSTR name = SysAllocString(func_iter->name); - if(name) { - DISPID id; - disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); - SysFreeString(name); + for (func_iter = code->funcs; func_iter; func_iter = func_iter->next) { + BSTR name = SysAllocString(func_iter->name); + if(name) { + DISPID id; + disp_get_id(probe, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } } - } - for (class_iter = code->classes; class_iter; class_iter = class_iter->next) { - BSTR name = SysAllocString(class_iter->name); - if(name) { - DISPID id; - disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); - SysFreeString(name); + for (class_iter = code->classes; class_iter; class_iter = class_iter->next) { + BSTR name = SysAllocString(class_iter->name); + if(name) { + DISPID id; + disp_get_id(probe, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } } } } @@ -273,25 +265,32 @@ static void exec_queued_code(script_ctx_t *ctx) } } -static HRESULT retrieve_named_item_disp(IActiveScriptSite *site, named_item_t *item) +static HRESULT fetch_named_item_disp(IActiveScriptSite *site, const WCHAR *name, IDispatch **out) { IUnknown *unk; HRESULT hres; - hres = IActiveScriptSite_GetItemInfo(site, item->name, SCRIPTINFO_IUNKNOWN, &unk, NULL); + *out = NULL; + if(!site) + return E_UNEXPECTED; + + hres = IActiveScriptSite_GetItemInfo(site, name, SCRIPTINFO_IUNKNOWN, &unk, NULL); if(FAILED(hres)) { WARN("GetItemInfo failed: %08lx\n", hres); return hres; } - hres = IUnknown_QueryInterface(unk, &IID_IDispatch, (void**)&item->disp); + hres = IUnknown_QueryInterface(unk, &IID_IDispatch, (void**)out); IUnknown_Release(unk); - if(FAILED(hres)) { + if(FAILED(hres)) WARN("object does not implement IDispatch\n"); - return hres; - } - return S_OK; + return hres; +} + +static HRESULT retrieve_named_item_disp(IActiveScriptSite *site, named_item_t *item) +{ + return fetch_named_item_disp(site, item->name, &item->disp); } named_item_t *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned flags) @@ -306,7 +305,7 @@ named_item_t *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned f if(FAILED(hres)) return NULL; } - if(!item->disp && (flags || !(item->flags & SCRIPTITEM_CODEONLY))) { + if(!item->disp && flags) { hres = retrieve_named_item_disp(ctx->site, item); if(FAILED(hres)) continue; } @@ -365,6 +364,12 @@ static void release_script(script_ctx_t *ctx) IDispatch_Release(item->disp); item->disp = NULL; } + if(item->dim_probe_disp) + { + IDispatch_Release(item->dim_probe_disp); + item->dim_probe_disp = NULL; + } + item->dim_disp_probed = FALSE; release_named_item_script_obj(item); if(!(item->flags & SCRIPTITEM_ISPERSISTENT)) { @@ -924,6 +929,7 @@ static HRESULT WINAPI VBScript_AddNamedItem(IActiveScript *iface, LPCOLESTR pstr item->ref = 1; item->disp = disp; + item->dim_probe_disp = NULL; item->flags = dwFlags; item->script_obj = NULL; item->dim_disp_probed = FALSE; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index b0a674cc67f..c8aa84d5562 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -160,6 +160,7 @@ typedef struct { typedef struct named_item_t { ScriptDisp *script_obj; IDispatch *disp; + IDispatch *dim_probe_disp; unsigned ref; DWORD flags; LPWSTR name; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)