[PATCH v2 1/8] jscript: Implement proxy objects along with a private interface.
Native mshtml objects act like JS objects in IE9 modes and up (the JS engine is implemented in mshtml). This implements proxy JS objects for certain dispatch objects that export a new private (internal to Wine) interface in versions ES5 and up, to achieve the same behavior. It delegates as much as possible to mshtml to implement all quirks there. Props are queried by the underlying dispatch and accesses to them forwarded to the dispatch object. Since it's purely internal, the private interface is not part of any typelib, and for simplicity, it extends (inherits from) IDispatchEx, so we can just cast it as needed. Given certain conditions, we query IDispatch objects on demand for the interface. If it's available, we either create a JS proxy object that is associated with the dispatch object exposing the interface, or re-acquire another one (possibly dangling). It is also associated on the other end, so that we keep a 1:1 mapping between the JS proxy object and the actual dispatch object, to avoid having multiple JS proxy objects for the same mshtml object (which would result in different dispids on them, and subtle bugs). This keeps them both in sync at all times, on demand, without having to pre-create any objects, and deals with dynamic props and such automatically. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- There's only one caveat here: since the JS proxy object references the dispatch object, while the dispatch object is also associated to the JS proxy object, the JS proxy object must be kept alive even when its refcount is zero. We can't hold a reference from the dispatch object, because that would create a circular reference. In effect, we *need* to know *when* the JS proxy object has no references from jscript part, to release the dispatch object. But the dispatch object itself might still remain alive (if kept alive by mshtml refs), and the JS proxy object must also remain associated in this case, hence we keep it alive even when refcount == 0, but remove the proxy association on its side. This is slightly ugly, but it is the best approach I could think of, to avoid bugs or having to rely on more complicated hash table lookups. Keep in mind that we have to keep the JS proxy object alive as long as it is associated with the dispatch object, because it could have proxy props retrieved, and they must always match among multiple references of this object, if they refer to the same underlying mshtml object (we can't have different dispids or "reset" them). dlls/jscript/dispex.c | 210 +++++++++++++++++++++++++++++++++++--- dlls/jscript/engine.c | 17 ++- dlls/jscript/enumerator.c | 10 +- dlls/jscript/function.c | 98 +++++++++++++++++- dlls/jscript/jscript.h | 27 +++++ dlls/jscript/jsutils.c | 21 ++-- dlls/jscript/jsval.h | 2 +- dlls/jscript/object.c | 22 ++-- dlls/jscript/vbarray.c | 4 +- 9 files changed, 368 insertions(+), 43 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index c7e4ba9..0b834d8 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -33,6 +33,7 @@ static const GUID GUID_JScriptTypeInfo = {0xc59c6b12,0xf6c1,0x11cf,{0x88,0x35,0x typedef enum { PROP_JSVAL, PROP_BUILTIN, + PROP_PROXY, PROP_PROTREF, PROP_ACCESSOR, PROP_DELETED, @@ -50,6 +51,10 @@ struct _dispex_prop_t { const builtin_prop_t *p; DWORD ref; unsigned idx; + struct { + DISPID id; + DWORD flags; + } proxy; struct { jsdisp_t *getter; jsdisp_t *setter; @@ -199,6 +204,30 @@ static inline dispex_prop_t* alloc_prop(jsdisp_t *This, const WCHAR *name, prop_ return prop; } +static inline dispex_prop_t* alloc_proxy_prop(jsdisp_t *This, dispex_prop_t *prop, const WCHAR *name, DISPID id) +{ + unsigned flags, prop_flags; + + flags = prop_flags = This->proxy->lpVtbl->GetPropFlags(This->proxy, id); + + if(flags & PROPF_METHOD) + flags |= PROPF_WRITABLE | PROPF_CONFIGURABLE; + flags &= PROPF_ALL; + + if(prop) { + prop->type = PROP_PROXY; + prop->flags = flags; + }else { + prop = alloc_prop(This, name, PROP_PROXY, flags); + if(!prop) + return NULL; + } + + prop->u.proxy.id = id; + prop->u.proxy.flags = prop_flags; + return prop; +} + static dispex_prop_t *alloc_protref(jsdisp_t *This, const WCHAR *name, DWORD ref) { dispex_prop_t *ret; @@ -235,6 +264,23 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, pos = This->props[pos].bucket_next; } + if(This->proxy) { + DISPID id; + + if(FAILED(IDispatchEx_GetDispID((IDispatchEx*)This->proxy, (WCHAR*)name, make_grfdex(This->ctx, fdexNameCaseSensitive), &id))) + id = DISPID_UNKNOWN; + if(id == DISPID_UNKNOWN) { + *ret = NULL; + return S_OK; + } + prop = alloc_proxy_prop(This, NULL, name, id); + if(!prop) + return E_OUTOFMEMORY; + + *ret = prop; + return S_OK; + } + builtin = find_builtin_prop(This, name); if(builtin) { unsigned flags = builtin->flags; @@ -314,11 +360,28 @@ static HRESULT find_prop_name_prot(jsdisp_t *This, unsigned hash, const WCHAR *n static HRESULT ensure_prop_name(jsdisp_t *This, const WCHAR *name, DWORD create_flags, dispex_prop_t **ret) { + unsigned hash = string_hash(name); dispex_prop_t *prop; HRESULT hres; - hres = find_prop_name_prot(This, string_hash(name), name, &prop); + hres = find_prop_name_prot(This, hash, name, &prop); if(SUCCEEDED(hres) && (!prop || prop->type == PROP_DELETED)) { + if(This->proxy) { + DWORD fdex = make_grfdex(This->ctx, fdexNameEnsure | fdexNameCaseSensitive); + DISPID id; + + TRACE("creating proxy prop %s\n", debugstr_w(name)); + + hres = IDispatchEx_GetDispID((IDispatchEx*)This->proxy, (WCHAR*)name, fdex, &id); + if(FAILED(hres)) + return hres; + prop = alloc_proxy_prop(This, prop, name, id); + if(!prop) + return E_OUTOFMEMORY; + *ret = prop; + return S_OK; + } + TRACE("creating prop %s flags %x\n", debugstr_w(name), create_flags); if(prop) { @@ -356,7 +419,7 @@ static IDispatch *get_this(DISPPARAMS *dp) return NULL; } -static HRESULT convert_params(const DISPPARAMS *dp, jsval_t *buf, unsigned *argc, jsval_t **ret) +static HRESULT convert_params(script_ctx_t *ctx, const DISPPARAMS *dp, jsval_t *buf, unsigned *argc, jsval_t **ret) { jsval_t *argv; unsigned cnt; @@ -374,7 +437,7 @@ static HRESULT convert_params(const DISPPARAMS *dp, jsval_t *buf, unsigned *argc } for(i = 0; i < cnt; i++) { - hres = variant_to_jsval(dp->rgvarg+dp->cArgs-i-1, argv+i); + hres = variant_to_jsval(ctx, dp->rgvarg+dp->cArgs-i-1, argv+i); if(FAILED(hres)) { while(i--) jsval_release(argv[i]); @@ -419,6 +482,23 @@ static HRESULT prop_get(jsdisp_t *This, dispex_prop_t *prop, jsval_t *r) *r = jsval_obj(obj); } break; + case PROP_PROXY: + if(!(prop->u.proxy.flags & PROPF_METHOD)) { + hres = disp_propget(This->ctx, (IDispatch*)prop_obj->proxy, prop->u.proxy.id, r); + }else { + jsdisp_t *obj; + + hres = create_proxy_function(This->ctx, (IDispatchEx*)prop_obj->proxy, prop->u.proxy.id, prop->u.proxy.flags, &obj); + if(FAILED(hres)) + break; + + prop->type = PROP_JSVAL; + prop->u.val = jsval_obj(obj); + + jsdisp_addref(obj); + *r = jsval_obj(obj); + } + break; case PROP_JSVAL: hres = jsval_copy(prop->u.val, r); break; @@ -438,6 +518,8 @@ static HRESULT prop_get(jsdisp_t *This, dispex_prop_t *prop, jsval_t *r) ERR("type %d\n", prop->type); return E_FAIL; } + if(SUCCEEDED(hres)) + hres = convert_to_proxy(This->ctx, r); if(FAILED(hres)) { TRACE("fail %08x\n", hres); @@ -472,6 +554,10 @@ static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) return S_OK; } return prop->u.p->setter(This->ctx, This, val); + case PROP_PROXY: + if(!(prop->flags & PROPF_WRITABLE)) + return S_OK; + return disp_propput(This->ctx, (IDispatch*)This->proxy, prop->u.proxy.id, val); case PROP_PROTREF: case PROP_DELETED: if(!This->extensible) @@ -521,7 +607,8 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t HRESULT hres; switch(prop->type) { - case PROP_BUILTIN: { + case PROP_BUILTIN: + case PROP_PROXY: if(flags == DISPATCH_CONSTRUCT && (prop->flags & PROPF_METHOD)) { WARN("%s is not a constructor\n", debugstr_w(prop->name)); return E_INVALIDARG; @@ -530,6 +617,17 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t if(prop->name || This->builtin_info->class != JSCLASS_FUNCTION) { vdisp_t vthis; + if(prop->type == PROP_PROXY) { + jsdisp_t *jsdisp = to_jsdisp(jsthis); + if(jsdisp) + jsthis = (IDispatch*)jsdisp->proxy; + if(jsthis != (IDispatch*)This->proxy) { + WARN("Incompatible this %p for proxy %p, id %d\n", jsthis, This->proxy, prop->u.proxy.id); + return E_UNEXPECTED; + } + return disp_call(This->ctx, jsthis, prop->u.proxy.id, flags & ~DISPATCH_JSCRIPT_INTERNAL_MASK, argc, argv, r); + } + if(This->builtin_info->class != JSCLASS_FUNCTION && prop->u.p->invoke != JSGlobal_eval) flags &= ~DISPATCH_JSCRIPT_INTERNAL_MASK; if(jsthis) @@ -542,8 +640,9 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t /* Function object calls are special case */ hres = Function_invoke(This, jsthis, flags, argc, argv, r); } + if(SUCCEEDED(hres)) + hres = convert_to_proxy(This->ctx, r); return hres; - } case PROP_PROTREF: return invoke_prop_func(This->prototype, jsthis ? jsthis : (IDispatch *)&This->IDispatchEx_iface, This->prototype->props+prop->u.ref, flags, argc, argv, r, caller); @@ -1563,7 +1662,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc jsval_t *argv, buf[6], r; unsigned argc; - hres = convert_params(pdp, buf, &argc, &argv); + hres = convert_params(This->ctx, pdp, buf, &argc, &argv); if(FAILED(hres)) break; @@ -1601,7 +1700,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc break; } - hres = variant_to_jsval(pdp->rgvarg+i, &val); + hres = variant_to_jsval(This->ctx, pdp->rgvarg+i, &val); if(FAILED(hres)) break; @@ -1618,8 +1717,19 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc return leave_script(This->ctx, hres); } -static HRESULT delete_prop(dispex_prop_t *prop, BOOL *ret) +static HRESULT delete_prop(jsdisp_t *prop_obj, dispex_prop_t *prop, BOOL *ret) { + if(prop->type == PROP_PROXY) { + BOOL deleted; + HRESULT hres = prop_obj->proxy->lpVtbl->DeleteProp(prop_obj->proxy, prop->u.proxy.id, &deleted); + if(SUCCEEDED(hres)) { + *ret = (hres == S_OK); + if(deleted) + prop->type = PROP_DELETED; + } + return hres; + } + if(!(prop->flags & PROPF_CONFIGURABLE)) { *ret = FALSE; return S_OK; @@ -1656,7 +1766,7 @@ static HRESULT WINAPI DispatchEx_DeleteMemberByName(IDispatchEx *iface, BSTR bst return S_OK; } - return delete_prop(prop, &b); + return delete_prop(This, prop, &b); } static HRESULT WINAPI DispatchEx_DeleteMemberByDispID(IDispatchEx *iface, DISPID id) @@ -1673,7 +1783,7 @@ static HRESULT WINAPI DispatchEx_DeleteMemberByDispID(IDispatchEx *iface, DISPID return DISP_E_MEMBERNOTFOUND; } - return delete_prop(prop, &b); + return delete_prop(This, prop, &b); } static HRESULT WINAPI DispatchEx_GetMemberProperties(IDispatchEx *iface, DISPID id, DWORD grfdexFetch, DWORD *pgrfdex) @@ -1808,10 +1918,73 @@ HRESULT create_dispex(script_ctx_t *ctx, const builtin_info_t *builtin_info, jsd return S_OK; } +static const builtin_info_t proxy_dispex_info = { + JSCLASS_OBJECT, + {NULL, NULL, 0}, + 0, NULL, + NULL, + NULL +}; + +HRESULT convert_to_proxy(script_ctx_t *ctx, jsval_t *val) +{ + IWineDispatchProxyPrivate *proxy; + jsdisp_t **jsdisp_ref; + IDispatch *obj; + HRESULT hres; + + if(ctx->version < SCRIPTLANGUAGEVERSION_ES5 || !val || !is_object_instance(*val)) + return S_OK; + obj = get_object(*val); + if(!obj || to_jsdisp(obj)) + return S_OK; + + if(FAILED(IDispatch_QueryInterface(obj, &IID_IWineDispatchProxyPrivate, (void**)&proxy)) || !proxy) + return S_OK; + IDispatch_Release(obj); + + jsdisp_ref = proxy->lpVtbl->GetProxyFieldRef(proxy); + + if(*jsdisp_ref) { + /* Re-acquire the proxy if it's an old dangling proxy */ + if((*jsdisp_ref)->ref == 0) + (*jsdisp_ref)->proxy = proxy; + else + IDispatchEx_Release((IDispatchEx*)proxy); + + *val = jsval_obj(jsdisp_addref(*jsdisp_ref)); + return S_OK; + } + + hres = create_dispex(ctx, &proxy_dispex_info, ctx->object_prototype, jsdisp_ref); + if(FAILED(hres)) { + IDispatchEx_Release((IDispatchEx*)proxy); + return hres; + } + + (*jsdisp_ref)->props[0].type = PROP_PROXY; + (*jsdisp_ref)->props[0].u.proxy.id = DISPID_VALUE; + (*jsdisp_ref)->props[0].u.proxy.flags = proxy->lpVtbl->GetPropFlags(proxy, DISPID_VALUE); + (*jsdisp_ref)->proxy = proxy; + + *val = jsval_obj(*jsdisp_ref); + return S_OK; +} + void jsdisp_free(jsdisp_t *obj) { dispex_prop_t *prop; + /* If we are a proxy, stay alive and keep it associated with the disp, since + we can be re-acquired at some later point. When the underlying disp is + actually destroyed, it should let go and destroy us for real. */ + if(obj->proxy) { + IDispatchEx *disp = (IDispatchEx*)obj->proxy; + obj->proxy = NULL; + IDispatchEx_Release(disp); + return; + } + TRACE("(%p)\n", obj); for(prop = obj->props; prop < obj->props+obj->prop_cnt; prop++) { @@ -1931,6 +2104,8 @@ HRESULT jsdisp_call_value(jsdisp_t *jsfunc, IDispatch *jsthis, WORD flags, unsig if(is_class(jsfunc, JSCLASS_FUNCTION)) { hres = Function_invoke(jsfunc, jsthis, flags, argc, argv, r); + }else if(jsfunc->proxy) { + hres = disp_call_value(jsfunc->ctx, (IDispatch*)jsfunc->proxy, jsthis, flags, argc, argv, r); }else { vdisp_t vdisp; @@ -1944,6 +2119,8 @@ HRESULT jsdisp_call_value(jsdisp_t *jsfunc, IDispatch *jsthis, WORD flags, unsig hres = jsfunc->builtin_info->value_prop.invoke(jsfunc->ctx, &vdisp, flags, argc, argv, r); vdisp_release(&vdisp); } + if(SUCCEEDED(hres)) + hres = convert_to_proxy(jsfunc->ctx, r); return hres; } @@ -2082,7 +2259,7 @@ HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, uns heap_free(dp.rgvarg); if(SUCCEEDED(hres) && ret) - hres = variant_to_jsval(&retv, ret); + hres = variant_to_jsval(ctx, &retv, ret); VariantClear(&retv); return hres; } @@ -2148,7 +2325,7 @@ HRESULT disp_call_value(script_ctx_t *ctx, IDispatch *disp, IDispatch *jsthis, W if(!r) return S_OK; - hres = variant_to_jsval(&retv, r); + hres = variant_to_jsval(ctx, &retv, r); VariantClear(&retv); return hres; } @@ -2316,7 +2493,7 @@ HRESULT disp_propget(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t *val V_VT(&var) = VT_EMPTY; hres = disp_invoke(ctx, disp, id, INVOKE_PROPERTYGET, &dp, &var); if(SUCCEEDED(hres)) { - hres = variant_to_jsval(&var, val); + hres = variant_to_jsval(ctx, &var, val); VariantClear(&var); } return hres; @@ -2335,7 +2512,7 @@ HRESULT jsdisp_delete_idx(jsdisp_t *obj, DWORD idx) if(FAILED(hres) || !prop) return hres; - hres = delete_prop(prop, &b); + hres = delete_prop(obj, prop, &b); if(FAILED(hres)) return hres; return b ? S_OK : JS_E_INVALID_ACTION; @@ -2353,7 +2530,7 @@ HRESULT disp_delete(IDispatch *disp, DISPID id, BOOL *ret) prop = get_prop(jsdisp, id); if(prop) - hres = delete_prop(prop, ret); + hres = delete_prop(jsdisp, prop, ret); else hres = DISP_E_MEMBERNOTFOUND; @@ -2424,7 +2601,7 @@ HRESULT disp_delete_name(script_ctx_t *ctx, IDispatch *disp, jsstr_t *name, BOOL hres = find_prop_name(jsdisp, string_hash(ptr), ptr, &prop); if(prop) { - hres = delete_prop(prop, ret); + hres = delete_prop(jsdisp, prop, ret); }else { *ret = TRUE; hres = S_OK; @@ -2480,6 +2657,7 @@ HRESULT jsdisp_get_own_property(jsdisp_t *obj, const WCHAR *name, BOOL flags_onl switch(prop->type) { case PROP_BUILTIN: + case PROP_PROXY: case PROP_JSVAL: desc->mask |= PROPF_WRITABLE; desc->explicit_value = TRUE; diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 47b4e56..4ff4297 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -491,15 +491,24 @@ static HRESULT disp_cmp(IDispatch *disp1, IDispatch *disp2, BOOL *ret) { IObjectIdentity *identity; IUnknown *unk1, *unk2; + jsdisp_t *jsdisp; HRESULT hres; - if(disp1 == disp2) { - *ret = TRUE; + if(!disp1 || !disp2) { + *ret = (disp1 == disp2); return S_OK; } - if(!disp1 || !disp2) { - *ret = FALSE; + jsdisp = to_jsdisp(disp1); + if(jsdisp && jsdisp->proxy) + disp1 = (IDispatch*)jsdisp->proxy; + + jsdisp = to_jsdisp(disp2); + if(jsdisp && jsdisp->proxy) + disp2 = (IDispatch*)jsdisp->proxy; + + if(disp1 == disp2) { + *ret = TRUE; return S_OK; } diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index dea1940..eb6baf6 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -48,7 +48,7 @@ static inline EnumeratorInstance *enumerator_this(vdisp_t *jsthis) return is_vclass(jsthis, JSCLASS_ENUMERATOR) ? enumerator_from_vdisp(jsthis) : NULL; } -static inline HRESULT enumvar_get_next_item(EnumeratorInstance *This) +static inline HRESULT enumvar_get_next_item(EnumeratorInstance *This, script_ctx_t *ctx) { HRESULT hres; VARIANT nextitem; @@ -64,7 +64,7 @@ static inline HRESULT enumvar_get_next_item(EnumeratorInstance *This) hres = IEnumVARIANT_Next(This->enumvar, 1, &nextitem, NULL); if (hres == S_OK) { - hres = variant_to_jsval(&nextitem, &This->item); + hres = variant_to_jsval(ctx, &nextitem, &This->item); VariantClear(&nextitem); if (FAILED(hres)) { @@ -138,7 +138,7 @@ static HRESULT Enumerator_moveFirst(script_ctx_t *ctx, vdisp_t *jsthis, WORD fla return hres; This->atend = FALSE; - hres = enumvar_get_next_item(This); + hres = enumvar_get_next_item(This, ctx); if(FAILED(hres)) return hres; } @@ -161,7 +161,7 @@ static HRESULT Enumerator_moveNext(script_ctx_t *ctx, vdisp_t *jsthis, WORD flag if (This->enumvar) { - hres = enumvar_get_next_item(This); + hres = enumvar_get_next_item(This, ctx); if (FAILED(hres)) return hres; } @@ -276,7 +276,7 @@ static HRESULT create_enumerator(script_ctx_t *ctx, jsval_t *argv, jsdisp_t **re enumerator->enumvar = enumvar; enumerator->atend = !enumvar; - hres = enumvar_get_next_item(enumerator); + hres = enumvar_get_next_item(enumerator, ctx); if (FAILED(hres)) { jsdisp_release(&enumerator->dispex); diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 318d6be..7cd83a3 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -54,6 +54,12 @@ typedef struct { const WCHAR *name; } NativeFunction; +typedef struct { + FunctionInstance function; + IDispatchEx *proxy; + DISPID id; +} ProxyFunction; + typedef struct { FunctionInstance function; FunctionInstance *target; @@ -340,7 +346,7 @@ static HRESULT Function_apply(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, un TRACE("\n"); - if(!(function = function_this(jsthis)) && (jsthis->flags & VDISP_JSDISP)) + if(!(function = function_this(jsthis)) && is_jsdisp(jsthis) && !get_jsdisp(jsthis)->proxy) return JS_E_FUNCTION_EXPECTED; if(argc) { @@ -707,6 +713,96 @@ HRESULT create_builtin_constructor(script_ctx_t *ctx, builtin_invoke_t value_pro return S_OK; } +static HRESULT ProxyFunction_call(script_ctx_t *ctx, FunctionInstance *func, IDispatch *this_disp, unsigned flags, + unsigned argc, jsval_t *argv, jsval_t *r) +{ + ProxyFunction *function = (ProxyFunction*)func; + jsdisp_t *jsdisp; + + if(!this_disp) + this_disp = lookup_global_host(ctx); + + jsdisp = to_jsdisp(this_disp); + if(jsdisp) + this_disp = (IDispatch*)jsdisp->proxy; + if(this_disp != (IDispatch*)function->proxy) { + WARN("Incompatible this %p for proxy %p, id %d\n", this_disp, function->proxy, function->id); + return E_UNEXPECTED; + } + + return disp_call(ctx, this_disp, function->id, flags & ~DISPATCH_JSCRIPT_INTERNAL_MASK, argc, argv, r); +} + +static HRESULT ProxyFunction_toString(FunctionInstance *func, jsstr_t **ret) +{ + ProxyFunction *function = (ProxyFunction*)func; + BSTR name = NULL; + DWORD name_len; + jsstr_t *str; + WCHAR *ptr; + + static const WCHAR proxy_prefixW[] = L"\nfunction "; + static const WCHAR proxy_suffixW[] = L"() {\n [native code]\n}\n"; + + if(FAILED(IDispatchEx_GetMemberName(function->proxy, function->id, &name))) + name = NULL; + + name_len = name ? SysStringLen(name) : 0; + str = jsstr_alloc_buf(ARRAY_SIZE(proxy_prefixW) + ARRAY_SIZE(proxy_suffixW) + name_len - 2, &ptr); + if(!str) + return E_OUTOFMEMORY; + + memcpy(ptr, proxy_prefixW, sizeof(proxy_prefixW)); + ptr += ARRAY_SIZE(proxy_prefixW) - 1; + memcpy(ptr, name, name_len*sizeof(WCHAR)); + ptr += name_len; + memcpy(ptr, proxy_suffixW, sizeof(proxy_suffixW)); + + SysFreeString(name); + *ret = str; + return S_OK; +} + +static function_code_t *ProxyFunction_get_code(FunctionInstance *func) +{ + return NULL; +} + +static void ProxyFunction_destructor(FunctionInstance *func) +{ + ProxyFunction *function = (ProxyFunction*)func; + IDispatchEx_Release(function->proxy); +} + +static const function_vtbl_t ProxyFunctionVtbl = { + ProxyFunction_call, + ProxyFunction_toString, + ProxyFunction_get_code, + ProxyFunction_destructor +}; + +HRESULT create_proxy_function(script_ctx_t *ctx, IDispatchEx *proxy, DISPID id, DWORD flags, jsdisp_t **ret) +{ + ProxyFunction *function; + HRESULT hres; + + hres = create_function(ctx, NULL, &ProxyFunctionVtbl, sizeof(ProxyFunction), flags, FALSE, NULL, (void**)&function); + if(FAILED(hres)) + return hres; + + if(FAILED(hres)) { + jsdisp_release(&function->function.dispex); + return hres; + } + + function->proxy = proxy; + function->id = id; + IDispatchEx_AddRef(function->proxy); + + *ret = &function->function.dispex; + return S_OK; +} + static HRESULT InterpretedFunction_call(script_ctx_t *ctx, FunctionInstance *func, IDispatch *this_obj, unsigned flags, unsigned argc, jsval_t *argv, jsval_t *r) { diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 90a5e15..59f7cfc 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -47,6 +47,29 @@ #define SCRIPTLANGUAGEVERSION_ES5 0x102 #define SCRIPTLANGUAGEVERSION_ES6 0x103 +/* + * This is Wine jscript extension, used for mshtml objects so they act like JS objects. + * Extends IDispatchEx. + * + * NOTE: Keep in sync with mshtml_private.h in mshtml.dll + */ +DEFINE_GUID(IID_IWineDispatchProxyPrivate, 0xd359f2fe,0x5531,0x741b,0xa4,0x1a,0x5c,0xf9,0x2e,0xdc,0x97,0x1b); +typedef struct _IWineDispatchProxyPrivate IWineDispatchProxyPrivate; + +typedef struct { + IDispatchExVtbl dispex; + void* (STDMETHODCALLTYPE *GetProxyFieldRef)(IWineDispatchProxyPrivate *This); + DWORD (STDMETHODCALLTYPE *GetPropFlags)(IWineDispatchProxyPrivate *This, DISPID id); + HRESULT (STDMETHODCALLTYPE *DeleteProp)(IWineDispatchProxyPrivate *This, DISPID id, BOOL *deleted); + HRESULT (STDMETHODCALLTYPE *ToString)(IWineDispatchProxyPrivate *This, BSTR *string); +} IWineDispatchProxyPrivateVtbl; + +struct _IWineDispatchProxyPrivate { + const IWineDispatchProxyPrivateVtbl *lpVtbl; +}; + + + typedef struct _jsval_t jsval_t; typedef struct _jsstr_t jsstr_t; typedef struct _jsexcept_t jsexcept_t; @@ -91,6 +114,7 @@ typedef struct jsdisp_t jsdisp_t; extern HINSTANCE jscript_hinstance DECLSPEC_HIDDEN; HRESULT get_dispatch_typeinfo(ITypeInfo**) DECLSPEC_HIDDEN; +/* NOTE: Keep in sync with mshtml_private.h in mshtml.dll */ #define PROPF_ARGMASK 0x00ff #define PROPF_METHOD 0x0100 #define PROPF_CONSTR 0x0200 @@ -253,6 +277,7 @@ struct jsdisp_t { BOOL extensible; jsdisp_t *prototype; + IWineDispatchProxyPrivate *proxy; const builtin_info_t *builtin_info; }; @@ -302,6 +327,7 @@ enum jsdisp_enum_type { HRESULT create_dispex(script_ctx_t*,const builtin_info_t*,jsdisp_t*,jsdisp_t**) DECLSPEC_HIDDEN; HRESULT init_dispex(jsdisp_t*,script_ctx_t*,const builtin_info_t*,jsdisp_t*) DECLSPEC_HIDDEN; HRESULT init_dispex_from_constr(jsdisp_t*,script_ctx_t*,const builtin_info_t*,jsdisp_t*) DECLSPEC_HIDDEN; +HRESULT convert_to_proxy(script_ctx_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT disp_call(script_ctx_t*,IDispatch*,DISPID,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT disp_call_value(script_ctx_t*,IDispatch*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; @@ -333,6 +359,7 @@ HRESULT create_builtin_function(script_ctx_t*,builtin_invoke_t,const WCHAR*,cons jsdisp_t*,jsdisp_t**) DECLSPEC_HIDDEN; HRESULT create_builtin_constructor(script_ctx_t*,builtin_invoke_t,const WCHAR*,const builtin_info_t*,DWORD, jsdisp_t*,jsdisp_t**) DECLSPEC_HIDDEN; +HRESULT create_proxy_function(script_ctx_t*,IDispatchEx*,DISPID,DWORD,jsdisp_t**) DECLSPEC_HIDDEN; HRESULT Function_invoke(jsdisp_t*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT Function_value(script_ctx_t*,vdisp_t*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/jsutils.c b/dlls/jscript/jsutils.c index 3c3cd01..1b1b904 100644 --- a/dlls/jscript/jsutils.c +++ b/dlls/jscript/jsutils.c @@ -246,7 +246,7 @@ HRESULT jsval_copy(jsval_t v, jsval_t *r) return E_FAIL; } -HRESULT variant_to_jsval(VARIANT *var, jsval_t *r) +HRESULT variant_to_jsval(script_ctx_t *ctx, VARIANT *var, jsval_t *r) { if(V_VT(var) == (VT_VARIANT|VT_BYREF)) var = V_VARIANTREF(var); @@ -285,7 +285,7 @@ HRESULT variant_to_jsval(VARIANT *var, jsval_t *r) if(V_DISPATCH(var)) IDispatch_AddRef(V_DISPATCH(var)); *r = jsval_disp(V_DISPATCH(var)); - return S_OK; + return convert_to_proxy(ctx, r); } case VT_I1: *r = jsval_number(V_I1(var)); @@ -329,7 +329,7 @@ HRESULT variant_to_jsval(VARIANT *var, jsval_t *r) hres = IUnknown_QueryInterface(V_UNKNOWN(var), &IID_IDispatch, (void**)&disp); if(SUCCEEDED(hres)) { *r = jsval_disp(disp); - return S_OK; + return convert_to_proxy(ctx, r); } }else { *r = jsval_disp(NULL); @@ -343,6 +343,8 @@ HRESULT variant_to_jsval(VARIANT *var, jsval_t *r) HRESULT jsval_to_variant(jsval_t val, VARIANT *retv) { + IDispatch *disp; + switch(jsval_type(val)) { case JSV_UNDEFINED: V_VT(retv) = VT_EMPTY; @@ -352,9 +354,14 @@ HRESULT jsval_to_variant(jsval_t val, VARIANT *retv) return S_OK; case JSV_OBJECT: V_VT(retv) = VT_DISPATCH; - if(get_object(val)) - IDispatch_AddRef(get_object(val)); - V_DISPATCH(retv) = get_object(val); + disp = get_object(val); + if(disp) { + jsdisp_t *jsdisp = to_jsdisp(disp); + if(jsdisp && jsdisp->proxy) + disp = (IDispatch*)jsdisp->proxy; + IDispatch_AddRef(disp); + } + V_DISPATCH(retv) = disp; return S_OK; case JSV_STRING: V_VT(retv) = VT_BSTR; @@ -884,7 +891,7 @@ HRESULT variant_change_type(script_ctx_t *ctx, VARIANT *dst, VARIANT *src, VARTY jsval_t val; HRESULT hres; - hres = variant_to_jsval(src, &val); + hres = variant_to_jsval(ctx, src, &val); if(FAILED(hres)) return hres; diff --git a/dlls/jscript/jsval.h b/dlls/jscript/jsval.h index 9f451b1..c79bef4 100644 --- a/dlls/jscript/jsval.h +++ b/dlls/jscript/jsval.h @@ -241,7 +241,7 @@ static inline BOOL get_bool(jsval_t v) return __JSVAL_BOOL(v); } -HRESULT variant_to_jsval(VARIANT*,jsval_t*) DECLSPEC_HIDDEN; +HRESULT variant_to_jsval(script_ctx_t*,VARIANT*,jsval_t*) DECLSPEC_HIDDEN; HRESULT jsval_to_variant(jsval_t,VARIANT*) DECLSPEC_HIDDEN; void jsval_release(jsval_t) DECLSPEC_HIDDEN; HRESULT jsval_copy(jsval_t,jsval_t*) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 169b47c..c107ed7 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -29,6 +29,8 @@ static HRESULT Object_toString(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, u { jsdisp_t *jsdisp; const WCHAR *str; + BSTR bstr = NULL; + jsstr_t *ret; /* Keep in sync with jsclass_t enum */ static const WCHAR *names[] = { @@ -54,9 +56,17 @@ static HRESULT Object_toString(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, u TRACE("\n"); + if(!r) + return S_OK; + jsdisp = get_jsdisp(jsthis); if(!jsdisp) { str = L"[object Object]"; + }else if(jsdisp->proxy) { + HRESULT hres = jsdisp->proxy->lpVtbl->ToString(jsdisp->proxy, &bstr); + if(FAILED(hres)) + return hres; + str = bstr; }else if(names[jsdisp->builtin_info->class]) { str = names[jsdisp->builtin_info->class]; }else { @@ -65,13 +75,11 @@ static HRESULT Object_toString(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, u return E_FAIL; } - if(r) { - jsstr_t *ret; - ret = jsstr_alloc(str); - if(!ret) - return E_OUTOFMEMORY; - *r = jsval_string(ret); - } + ret = jsstr_alloc(str); + SysFreeString(bstr); + if(!ret) + return E_OUTOFMEMORY; + *r = jsval_string(ret); return S_OK; } diff --git a/dlls/jscript/vbarray.c b/dlls/jscript/vbarray.c index 69a77f1..3294ee6 100644 --- a/dlls/jscript/vbarray.c +++ b/dlls/jscript/vbarray.c @@ -96,7 +96,7 @@ static HRESULT VBArray_getItem(script_ctx_t *ctx, vdisp_t *vthis, WORD flags, un return hres; if(r) { - hres = variant_to_jsval(&out, r); + hres = variant_to_jsval(ctx, &out, r); VariantClear(&out); } return hres; @@ -166,7 +166,7 @@ static HRESULT VBArray_toArray(script_ctx_t *ctx, vdisp_t *vthis, WORD flags, un } for(i=0; i<size; i++) { - hres = variant_to_jsval(v, &val); + hres = variant_to_jsval(ctx, v, &val); if(SUCCEEDED(hres)) { hres = jsdisp_propput_idx(array, i, val); jsval_release(val); -- 2.31.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 4ff4297..bac855b 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -662,12 +662,27 @@ static BOOL lookup_global_members(script_ctx_t *ctx, BSTR identifier, exprval_t LIST_FOR_EACH_ENTRY(item, &ctx->named_items, named_item_t, entry) { if(item->flags & SCRIPTITEM_GLOBALMEMBERS) { - hres = disp_get_id(ctx, item->disp, identifier, identifier, 0, &id); + IDispatch *disp = item->disp; + + IDispatch_AddRef(disp); + if(ret) { + jsval_t val = jsval_disp(disp); + + hres = convert_to_proxy(ctx, &val); + if(SUCCEEDED(hres)) + disp = get_object(val); + else + IDispatch_AddRef(disp); + } + + hres = disp_get_id(ctx, disp, identifier, identifier, 0, &id); if(SUCCEEDED(hres)) { if(ret) - exprval_set_disp_ref(ret, item->disp, id); + exprval_set_disp_ref(ret, disp, id); + IDispatch_Release(disp); return TRUE; } + IDispatch_Release(disp); } } @@ -779,7 +794,7 @@ static HRESULT identifier_eval(script_ctx_t *ctx, BSTR identifier, exprval_t *re IDispatch_AddRef(item->disp); ret->type = EXPRVAL_JSVAL; ret->u.val = jsval_disp(item->disp); - return S_OK; + return convert_to_proxy(ctx, &ret->u.val); } if(lookup_global_members(ctx, identifier, ret)) -- 2.31.1
This implements the internal private interface for mshtml objects, so they can be used as JS proxy objects from jscript. Note that we need to keep an association also from these objects back to the JS proxy objects. We keep a weak ref, though, to avoid circular references, and because the JS proxy object needs to know when all the jscript refs to it are gone (so we can't hold an actual ref to it). Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- As explained in the previous patch, the JS proxy object and dispatch objects are both associated with one another in both directions. This allows lookups to "old dangling" JS proxy objects to return the same object with same props (and dispids) and avoid subtle bugs, which makes them as though they were directly implemented in mshtml, just like native. Note that we *do not* keep a ref to the JS proxy object, but a weak ref. However, this is not a problem, because the JS proxy object remains alive even with refcount == 0 (as explained in previous patch, because it needs to be re-acquired with same dispids as long as the builtin dispatch object didn't die yet). This does mean we have to somehow retrigger its "release" if the builtin object is actually destroyed when the JS proxy object has refcount == 0. Since the refcount is 0, we achieve this by AddRef and then Release to it. It is a semi hack, but it avoids further complications. The JS proxy object knows this situation because it lets go of the dispatch when refcount == 0, and re-acquires it if it has to, on demand. dlls/mshtml/dispex.c | 91 ++++++++++++++++++++++++++++++- dlls/mshtml/htmlwindow.c | 55 ++++++++++++++++++- dlls/mshtml/mshtml_private.h | 28 ++++++++++ dlls/mshtml/tests/documentmode.js | 2 +- 4 files changed, 171 insertions(+), 5 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 96a776d..6314087 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -1886,7 +1886,75 @@ static HRESULT WINAPI DispatchEx_GetNameSpaceParent(IDispatchEx *iface, IUnknown return E_NOTIMPL; } -static IDispatchExVtbl DispatchExVtbl = { +static inline DispatchEx *impl_from_IWineDispatchProxyPrivate(IWineDispatchProxyPrivate *iface) +{ + return impl_from_IDispatchEx((IDispatchEx*)iface); +} + +static void* WINAPI WineDispatchProxyPrivate_GetProxyFieldRef(IWineDispatchProxyPrivate *iface) +{ + DispatchEx *This = impl_from_IWineDispatchProxyPrivate(iface); + return &This->proxy; +} + +static DWORD WINAPI WineDispatchProxyPrivate_GetPropFlags(IWineDispatchProxyPrivate *iface, DISPID id) +{ + DispatchEx *This = impl_from_IWineDispatchProxyPrivate(iface); + func_info_t *func; + + TRACE("(%p)->(%x)\n", This, id); + + if(is_dynamic_dispid(id)) + return PROPF_WRITABLE | PROPF_CONFIGURABLE | PROPF_ENUMERABLE; + + if(is_custom_dispid(id)) + return PROPF_WRITABLE; + + if(FAILED(get_builtin_func(This->info, id, &func))) + return 0; + + if(func->func_disp_idx != -1) { + if(This->dynamic_data && This->dynamic_data->func_disps + && This->dynamic_data->func_disps[func->func_disp_idx].func_obj) { + func_obj_entry_t *entry = This->dynamic_data->func_disps + func->func_disp_idx; + + if((IDispatch*)&entry->func_obj->dispex.IDispatchEx_iface != V_DISPATCH(&entry->val)) + return PROPF_WRITABLE | PROPF_CONFIGURABLE; + } + return PROPF_METHOD | func->argc; + } + + /* FIXME: Don't add PROPF_ENUMERABLE to hidden properties */ + return PROPF_ENUMERABLE | ((func->put_vtbl_off) ? PROPF_WRITABLE : 0); +} + +static HRESULT WINAPI WineDispatchProxyPrivate_DeleteProp(IWineDispatchProxyPrivate *iface, DISPID id, BOOL *deleted) +{ + DispatchEx *This = impl_from_IWineDispatchProxyPrivate(iface); + HRESULT hres; + + hres = DispatchEx_DeleteMemberByDispID(&This->IDispatchEx_iface, id); + if(FAILED(hres)) + return hres; + + *deleted = FALSE; + if(is_dynamic_dispid(id)) { + DWORD idx = id - DISPID_DYNPROP_0; + + if(get_dynamic_data(This) && idx < This->dynamic_data->prop_cnt) + *deleted = TRUE; + } + return hres; +} + +static HRESULT WINAPI WineDispatchProxyPrivate_ToString(IWineDispatchProxyPrivate *iface, BSTR *string) +{ + DispatchEx *This = impl_from_IWineDispatchProxyPrivate(iface); + return dispex_to_string(This, string); +} + +static IWineDispatchProxyPrivateVtbl WineDispatchProxyPrivateVtbl = { + { DispatchEx_QueryInterface, DispatchEx_AddRef, DispatchEx_Release, @@ -1902,6 +1970,13 @@ static IDispatchExVtbl DispatchExVtbl = { DispatchEx_GetMemberName, DispatchEx_GetNextDispID, DispatchEx_GetNameSpaceParent + }, + + /* IWineDispatchProxyPrivate extension */ + WineDispatchProxyPrivate_GetProxyFieldRef, + WineDispatchProxyPrivate_GetPropFlags, + WineDispatchProxyPrivate_DeleteProp, + WineDispatchProxyPrivate_ToString }; BOOL dispex_query_interface(DispatchEx *This, REFIID riid, void **ppv) @@ -1910,6 +1985,8 @@ BOOL dispex_query_interface(DispatchEx *This, REFIID riid, void **ppv) *ppv = &This->IDispatchEx_iface; else if(IsEqualGUID(&IID_IDispatchEx, riid)) *ppv = &This->IDispatchEx_iface; + else if(IsEqualGUID(&IID_IWineDispatchProxyPrivate, riid)) + *ppv = &This->IDispatchEx_iface; else if(IsEqualGUID(&IID_IDispatchJS, riid)) *ppv = NULL; else if(IsEqualGUID(&IID_UndocumentedScriptIface, riid)) @@ -1945,6 +2022,15 @@ void dispex_unlink(DispatchEx *This) { dynamic_prop_t *prop; + /* Even though we hold a weak ref to the proxy, it is possible that it is a + dangling proxy associated with us (refcount == 0), so we need to trigger + its release again so that it is destroyed under such conditions. */ + if(This->proxy) { + IDispatch *proxy = This->proxy; + IDispatch_AddRef(proxy); + IDispatch_Release(proxy); + } + if(!This->dynamic_data) return; @@ -1998,8 +2084,9 @@ void init_dispatch(DispatchEx *dispex, IUnknown *outer, dispex_static_data_t *da { assert(compat_mode < COMPAT_MODE_CNT); - dispex->IDispatchEx_iface.lpVtbl = &DispatchExVtbl; + dispex->IDispatchEx_iface.lpVtbl = (const IDispatchExVtbl*)&WineDispatchProxyPrivateVtbl; dispex->outer = outer; + dispex->proxy = NULL; dispex->dynamic_data = NULL; if(data->vtbl && data->vtbl->get_compat_mode) { diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 0b25e71..0a10244 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -161,6 +161,8 @@ static HRESULT WINAPI HTMLWindow2_QueryInterface(IHTMLWindow2 *iface, REFIID rii *ppv = &This->IHTMLWindow2_iface; }else if(IsEqualGUID(&IID_IDispatchEx, riid)) { *ppv = &This->IDispatchEx_iface; + }else if(IsEqualGUID(&IID_IWineDispatchProxyPrivate, riid)) { + *ppv = &This->IDispatchEx_iface; }else if(IsEqualGUID(&IID_IHTMLFramesCollection2, riid)) { *ppv = &This->IHTMLWindow2_iface; }else if(IsEqualGUID(&IID_IHTMLWindow2, riid)) { @@ -3541,7 +3543,49 @@ static HRESULT WINAPI WindowDispEx_GetNameSpaceParent(IDispatchEx *iface, IUnkno return S_OK; } -static const IDispatchExVtbl WindowDispExVtbl = { +static inline HTMLWindow *impl_from_IWineDispatchProxyPrivate(IWineDispatchProxyPrivate *iface) +{ + return impl_from_IDispatchEx((IDispatchEx*)iface); +} + +static void* WINAPI WindowWineDispProxyPrivate_GetProxyFieldRef(IWineDispatchProxyPrivate *iface) +{ + HTMLWindow *This = impl_from_IWineDispatchProxyPrivate(iface); + IWineDispatchProxyPrivate *itf = (IWineDispatchProxyPrivate*)&This->inner_window->event_target.dispex.IDispatchEx_iface; + + return itf->lpVtbl->GetProxyFieldRef(itf); +} + +static DWORD WINAPI WindowWineDispProxyPrivate_GetPropFlags(IWineDispatchProxyPrivate *iface, DISPID id) +{ + HTMLWindow *This = impl_from_IWineDispatchProxyPrivate(iface); + IWineDispatchProxyPrivate *itf = (IWineDispatchProxyPrivate*)&This->inner_window->event_target.dispex.IDispatchEx_iface; + + switch(id) { + case DISPID_IHTMLWINDOW2_LOCATION: return PROPF_WRITABLE | PROPF_ENUMERABLE; + } + + return itf->lpVtbl->GetPropFlags(itf, id); +} + +static HRESULT WINAPI WindowWineDispProxyPrivate_DeleteProp(IWineDispatchProxyPrivate *iface, DISPID id, BOOL *deleted) +{ + HTMLWindow *This = impl_from_IWineDispatchProxyPrivate(iface); + IWineDispatchProxyPrivate *itf = (IWineDispatchProxyPrivate*)&This->inner_window->event_target.dispex.IDispatchEx_iface; + + return itf->lpVtbl->DeleteProp(itf, id, deleted); +} + +static HRESULT WINAPI WindowWineDispProxyPrivate_ToString(IWineDispatchProxyPrivate *iface, BSTR *string) +{ + HTMLWindow *This = impl_from_IWineDispatchProxyPrivate(iface); + IWineDispatchProxyPrivate *itf = (IWineDispatchProxyPrivate*)&This->inner_window->event_target.dispex.IDispatchEx_iface; + + return itf->lpVtbl->ToString(itf, string); +} + +static const IWineDispatchProxyPrivateVtbl WindowDispExVtbl = { + { WindowDispEx_QueryInterface, WindowDispEx_AddRef, WindowDispEx_Release, @@ -3557,6 +3601,13 @@ static const IDispatchExVtbl WindowDispExVtbl = { WindowDispEx_GetMemberName, WindowDispEx_GetNextDispID, WindowDispEx_GetNameSpaceParent + }, + + /* IWineDispatchProxyPrivate extension */ + WindowWineDispProxyPrivate_GetProxyFieldRef, + WindowWineDispProxyPrivate_GetPropFlags, + WindowWineDispProxyPrivate_DeleteProp, + WindowWineDispProxyPrivate_ToString }; static inline HTMLWindow *impl_from_IServiceProvider(IServiceProvider *iface) @@ -3801,7 +3852,7 @@ static void *alloc_window(size_t size) window->IHTMLWindow6_iface.lpVtbl = &HTMLWindow6Vtbl; window->IHTMLWindow7_iface.lpVtbl = &HTMLWindow7Vtbl; window->IHTMLPrivateWindow_iface.lpVtbl = &HTMLPrivateWindowVtbl; - window->IDispatchEx_iface.lpVtbl = &WindowDispExVtbl; + window->IDispatchEx_iface.lpVtbl = (const IDispatchExVtbl*)&WindowDispExVtbl; window->IServiceProvider_iface.lpVtbl = &ServiceProviderVtbl; window->ITravelLogClient_iface.lpVtbl = &TravelLogClientVtbl; window->IObjectIdentity_iface.lpVtbl = &ObjectIdentityVtbl; diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 5cf53bb..ebadd62 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -44,6 +44,33 @@ #include <assert.h> +/* NOTE: Keep in sync with jscript.h in jscript.dll */ +DEFINE_GUID(IID_IWineDispatchProxyPrivate, 0xd359f2fe,0x5531,0x741b,0xa4,0x1a,0x5c,0xf9,0x2e,0xdc,0x97,0x1b); +typedef struct _IWineDispatchProxyPrivate IWineDispatchProxyPrivate; + +typedef struct { + IDispatchExVtbl dispex; + void* (STDMETHODCALLTYPE *GetProxyFieldRef)(IWineDispatchProxyPrivate *This); + DWORD (STDMETHODCALLTYPE *GetPropFlags)(IWineDispatchProxyPrivate *This, DISPID id); + HRESULT (STDMETHODCALLTYPE *DeleteProp)(IWineDispatchProxyPrivate *This, DISPID id, BOOL *deleted); + HRESULT (STDMETHODCALLTYPE *ToString)(IWineDispatchProxyPrivate *This, BSTR *string); +} IWineDispatchProxyPrivateVtbl; + +struct _IWineDispatchProxyPrivate { + const IWineDispatchProxyPrivateVtbl *lpVtbl; +}; + +#define PROPF_ARGMASK 0x00ff +#define PROPF_METHOD 0x0100 +#define PROPF_CONSTR 0x0200 + +#define PROPF_ENUMERABLE 0x0400 +#define PROPF_WRITABLE 0x0800 +#define PROPF_CONFIGURABLE 0x1000 +#define PROPF_ALL (PROPF_ENUMERABLE | PROPF_WRITABLE | PROPF_CONFIGURABLE) + + + #define NS_ERROR_GENERATE_FAILURE(module,code) \ ((nsresult) (((UINT32)(1u<<31)) | ((UINT32)(module+0x45)<<16) | ((UINT32)(code)))) #define NS_ERROR_GENERATE_SUCCESS(module,code) \ @@ -349,6 +376,7 @@ struct DispatchEx { IDispatchEx IDispatchEx_iface; IUnknown *outer; + void *proxy; dispex_data_t *info; dispex_dynamic_data_t *dynamic_data; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 3307f2c..4356b93 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -154,7 +154,7 @@ sync_test("builtin_toString", function() { ok(s === (tostr ? tostr : (v < 9 ? "[object]" : "[object " + name + "]")), msg + " toString returned " + s); } s = Object.prototype.toString.call(obj); - todo_wine_if(v >= 9 && name != "Object"). + todo_wine_if(name !== "HTMLElement" && s === "[object HTMLElement]"). ok(s === (v < 9 ? "[object Object]" : "[object " + name + "]"), msg + " Object.toString returned " + s); } -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=99979 Your paranoid android. === w1064_tsign (32 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w10pro64_ar (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS htmldoc.c:350: Test failed: expected Exec_SETTITLE htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE === w7u_adm (32 bit report) === mshtml: script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1634137608143 expected 1634137608205"
For document modes <= IE8, which don't use the JS object proxies, native exposes some of the function prototype properties and methods, and semi-implements the methods for typical use. This implements the same limitations on them. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Tests follow. dlls/mshtml/dispex.c | 172 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 168 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 6314087..649de16 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -34,6 +34,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(mshtml); #define MAX_ARGS 16 +#define JS_E_INVALID_PROPERTY 0x800a01b6 static CRITICAL_SECTION cs_dispex_static_data; static CRITICAL_SECTION_DEBUG cs_dispex_static_data_dbg = @@ -762,6 +763,19 @@ static HRESULT typeinfo_invoke(DispatchEx *This, func_info_t *func, WORD flags, return hres; } +static inline BOOL same_disp(IDispatch *disp, DispatchEx *obj) +{ + IDispatch *tmp; + + if(!disp || FAILED(IDispatch_QueryInterface(disp, &IID_IDispatch, (void**)&disp)) || !disp) + return FALSE; + IDispatch_Release(disp); + if(FAILED(IDispatchEx_QueryInterface(&obj->IDispatchEx_iface, &IID_IDispatch, (void**)&tmp))) + return FALSE; + IDispatch_Release(tmp); + return disp == tmp; +} + static inline func_disp_t *impl_from_IUnknown(IUnknown *iface) { return CONTAINING_RECORD(iface, func_disp_t, IUnknown_iface); @@ -818,6 +832,113 @@ static const IUnknownVtbl FunctionUnkVtbl = { Function_Release }; +static HRESULT function_apply(func_disp_t *func, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei) +{ + DISPPARAMS params = { 0 }; + IDispatch *disp; + DISPID dispid; + VARIANT *arg; + HRESULT hres; + + if(!func->obj) + return E_UNEXPECTED; + + if(dp->cNamedArgs) { + FIXME("named args not supported\n"); + return E_NOTIMPL; + } + + arg = dp->rgvarg + dp->cArgs - 1; + if(dp->cArgs < 1 || V_VT(arg) != VT_DISPATCH || !same_disp(V_DISPATCH(arg), func->obj)) + return CTL_E_ILLEGALFUNCTIONCALL; + + if(dp->cArgs >= 2) { + WCHAR buf[12], *name; + ULONG i, err = 0; + + arg--; + if((V_VT(arg) & ~VT_BYREF) != VT_DISPATCH) + return CTL_E_ILLEGALFUNCTIONCALL; + disp = (V_VT(arg) & VT_BYREF) ? *(IDispatch**)(V_BYREF(arg)) : V_DISPATCH(arg); + + /* get the array length */ + name = buf; + memcpy(buf, L"length", sizeof(L"length")); + hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, &dispid); + if(FAILED(hres) || dispid == DISPID_UNKNOWN) + return CTL_E_ILLEGALFUNCTIONCALL; + + hres = IDispatch_Invoke(disp, dispid, &IID_NULL, 0, DISPATCH_PROPERTYGET, NULL, res, ei, &err); + if(FAILED(hres)) + return hres; + if(FAILED(VariantChangeType(res, res, 0, VT_UI4))) { + VariantClear(res); + return CTL_E_ILLEGALFUNCTIONCALL; + } + params.cArgs = V_UI4(res); + V_VT(res) = VT_EMPTY; + + /* alloc new params */ + if(params.cArgs) { + if(!(params.rgvarg = heap_alloc(params.cArgs * sizeof(VARIANTARG)))) + return E_OUTOFMEMORY; + for(i = 0; i < params.cArgs; i++) { + arg = params.rgvarg + params.cArgs - i - 1; + + name = buf; + swprintf(buf, ARRAY_SIZE(buf), L"%u", i); + hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, &dispid); + if(FAILED(hres) || dispid == DISPID_UNKNOWN) { + hres = CTL_E_ILLEGALFUNCTIONCALL; + break; + } + hres = IDispatch_Invoke(disp, dispid, &IID_NULL, 0, DISPATCH_PROPERTYGET, NULL, arg, ei, &err); + if(FAILED(hres)) + break; + } + if(FAILED(hres)) { + while(i--) VariantClear(params.rgvarg + params.cArgs - i - 1); + return hres; + } + } + } + + hres = typeinfo_invoke(func->obj, func->info, DISPATCH_METHOD, ¶ms, res, ei); + + heap_free(params.rgvarg); + return hres; +} + +static HRESULT function_call(func_disp_t *func, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei) +{ + DISPPARAMS params = { dp->rgvarg, NULL, dp->cArgs - 1, 0 }; + VARIANT *arg; + + if(!func->obj) + return E_UNEXPECTED; + + if(dp->cNamedArgs) { + FIXME("named args not supported\n"); + return E_NOTIMPL; + } + + arg = dp->rgvarg + dp->cArgs - 1; + if(dp->cArgs < 1 || V_VT(arg) != VT_DISPATCH || !same_disp(V_DISPATCH(arg), func->obj)) + return CTL_E_ILLEGALFUNCTIONCALL; + + return typeinfo_invoke(func->obj, func->info, DISPATCH_METHOD, ¶ms, res, ei); +} + +static const struct { + const WCHAR *name; + HRESULT (*method)(func_disp_t*,DISPPARAMS*,VARIANT*,EXCEPINFO*); +} function_props[] = { + { L"apply", function_apply }, + { L"arguments", NULL }, + { L"call", function_call }, + { L"length", NULL } +}; + static inline func_disp_t *impl_from_DispatchEx(DispatchEx *iface) { return CONTAINING_RECORD(iface, func_disp_t, dispex); @@ -878,10 +999,53 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR return hres; } +static HRESULT function_get_dispid(DispatchEx *dispex, BSTR name, DWORD flags, DISPID *dispid) +{ + DWORD i; + + for(i = 0; i < ARRAY_SIZE(function_props); i++) { + if(!wcsicmp(name, function_props[i].name)) { + if((flags & fdexNameCaseSensitive) && wcscmp(name, function_props[i].name)) + return DISP_E_UNKNOWNNAME; + *dispid = MSHTML_DISPID_CUSTOM_MIN + i; + return S_OK; + } + } + return DISP_E_UNKNOWNNAME; +} + +static HRESULT function_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD flags, DISPPARAMS *params, + VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller) +{ + func_disp_t *This = impl_from_DispatchEx(dispex); + DWORD idx = id - MSHTML_DISPID_CUSTOM_MIN; + + if(idx >= ARRAY_SIZE(function_props)) + return DISP_E_UNKNOWNNAME; + + switch(flags) { + case DISPATCH_METHOD|DISPATCH_PROPERTYGET: + if(!res) + return E_INVALIDARG; + /* fall through */ + case DISPATCH_METHOD: + if(function_props[idx].method) + return function_props[idx].method(This, params, res, ei); + return JS_E_INVALID_PROPERTY; + case DISPATCH_PROPERTYGET: + V_VT(res) = VT_EMPTY; /* IE returns undefined */ + break; + default: + return JS_E_INVALID_PROPERTY; + } + + return S_OK; +} + static const dispex_static_data_vtbl_t function_dispex_vtbl = { function_value, - NULL, - NULL, + function_get_dispid, + function_invoke, NULL }; @@ -1265,7 +1429,7 @@ static HRESULT invoke_builtin_function(DispatchEx *This, func_info_t *func, DISP return V_ERROR(&vhres); } -static HRESULT function_invoke(DispatchEx *This, func_info_t *func, WORD flags, DISPPARAMS *dp, VARIANT *res, +static HRESULT func_invoke(DispatchEx *This, func_info_t *func, WORD flags, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller) { HRESULT hres; @@ -1371,7 +1535,7 @@ static HRESULT invoke_builtin_prop(DispatchEx *This, DISPID id, LCID lcid, WORD } if(func->func_disp_idx != -1) - return function_invoke(This, func, flags, dp, res, ei, caller); + return func_invoke(This, func, flags, dp, res, ei, caller); switch(flags) { case DISPATCH_PROPERTYPUT: -- 2.31.1
For document modes <= IE8, native allows to retrieve the semi-implemented builtin methods, and then use them to retrieve their value props (and string) or the other props (which always return undefined), but it doesn't allow calling them or otherwise doing anything else with them anymore (e.g. chaining them like f.call.call). This implements it the same way with the same limitations. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Tests follow. dlls/mshtml/dispex.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 649de16..45a9803 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -91,7 +91,10 @@ typedef struct { DispatchEx dispex; IUnknown IUnknown_iface; LONG ref; - DispatchEx *obj; + union { + DispatchEx *obj; + DWORD idx; /* index into function_props, used when info == NULL */ + }; func_info_t *info; } func_disp_t; @@ -126,6 +129,8 @@ PRIVATE_TID_LIST #undef XDIID }; +static func_disp_t *create_func_disp(DispatchEx*,func_info_t*); + static HRESULT load_typelib(void) { WCHAR module_path[MAX_PATH + 3]; @@ -818,7 +823,7 @@ static ULONG WINAPI Function_Release(IUnknown *iface) TRACE("(%p) ref=%d\n", This, ref); if(!ref) { - assert(!This->obj); + assert(!This->info || !This->obj); release_dispex(&This->dispex); heap_free(This); } @@ -956,6 +961,8 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR return E_INVALIDARG; /* fall through */ case DISPATCH_METHOD: + if(!This->info) + return JS_E_INVALID_PROPERTY; if(!This->obj) return E_UNEXPECTED; hres = typeinfo_invoke(This->obj, This->info, flags, params, res, ei); @@ -974,7 +981,7 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR if(!caller) return E_ACCESSDENIED; - name_len = SysStringLen(This->info->name); + name_len = This->info ? SysStringLen(This->info->name) : wcslen(function_props[This->idx].name); ptr = str = SysAllocStringLen(NULL, name_len + ARRAY_SIZE(func_prefixW) + ARRAY_SIZE(func_suffixW)); if(!str) return E_OUTOFMEMORY; @@ -982,7 +989,7 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR memcpy(ptr, func_prefixW, sizeof(func_prefixW)); ptr += ARRAY_SIZE(func_prefixW); - memcpy(ptr, This->info->name, name_len*sizeof(WCHAR)); + memcpy(ptr, This->info ? This->info->name : function_props[This->idx].name, name_len*sizeof(WCHAR)); ptr += name_len; memcpy(ptr, func_suffixW, sizeof(func_suffixW)); @@ -1001,12 +1008,18 @@ static HRESULT function_value(DispatchEx *dispex, LCID lcid, WORD flags, DISPPAR static HRESULT function_get_dispid(DispatchEx *dispex, BSTR name, DWORD flags, DISPID *dispid) { + func_disp_t *This = impl_from_DispatchEx(dispex); DWORD i; for(i = 0; i < ARRAY_SIZE(function_props); i++) { if(!wcsicmp(name, function_props[i].name)) { if((flags & fdexNameCaseSensitive) && wcscmp(name, function_props[i].name)) return DISP_E_UNKNOWNNAME; + + /* methods (apply, call) can't be chained */ + if(!This->info && function_props[i].method) + return DISP_E_UNKNOWNNAME; + *dispid = MSHTML_DISPID_CUSTOM_MIN + i; return S_OK; } @@ -1023,6 +1036,10 @@ static HRESULT function_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD fl if(idx >= ARRAY_SIZE(function_props)) return DISP_E_UNKNOWNNAME; + /* methods (apply, call) can't be chained */ + if(!This->info && function_props[idx].method) + return DISP_E_UNKNOWNNAME; + switch(flags) { case DISPATCH_METHOD|DISPATCH_PROPERTYGET: if(!res) @@ -1033,6 +1050,16 @@ static HRESULT function_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD fl return function_props[idx].method(This, params, res, ei); return JS_E_INVALID_PROPERTY; case DISPATCH_PROPERTYGET: + if(function_props[idx].method) { + func_disp_t *disp = create_func_disp(dispex, NULL); + if(!disp) + return E_OUTOFMEMORY; + disp->idx = idx; + + V_VT(res) = VT_DISPATCH; + V_DISPATCH(res) = (IDispatch*)&disp->dispex.IDispatchEx_iface; + break; + } V_VT(res) = VT_EMPTY; /* IE returns undefined */ break; default: -- 2.31.1
It is a prop, not a method. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- This is needed for the tests in the next patch to pass properly, otherwise an exception is generated, because mshtml handles DISPATCH_METHOD differently (as it should). dlls/jscript/enumerator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index eb6baf6..bdbebf7 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -244,7 +244,7 @@ static HRESULT create_enumerator(script_ctx_t *ctx, jsval_t *argv, jsdisp_t **re /* Try to get a IEnumVARIANT by _NewEnum */ VariantInit(&varresult); hres = IDispatch_Invoke(obj, DISPID_NEWENUM, &IID_NULL, LOCALE_NEUTRAL, - DISPATCH_METHOD, &dispparams, &varresult, NULL, NULL); + DISPATCH_PROPERTYGET, &dispparams, &varresult, NULL, NULL); if (FAILED(hres)) { WARN("Enumerator: no DISPID_NEWENUM.\n"); -- 2.31.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/documentmode.js | 137 ++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 4356b93..9ea940c 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1025,3 +1025,140 @@ sync_test("elem_attr", function() { r = elem.getAttribute("className"); ok(r === "cls3", "className attr = " + r); }); + +sync_test("builtin_obj", function() { + var v = document.documentMode; + var f = document.createElement; + var e; + + if(v < 9) { + ok(!(window instanceof Object), "window instance of Object"); + ok(!(document instanceof Object), "document instance of Object"); + ok(!(f.apply instanceof Function), "f.apply instance of Function"); + ok(!(f.call instanceof Function), "f.call instance of Function"); + ok(f.arguments === undefined, "f.arguments = " + f.arguments); + ok(f.length === undefined, "f.length = " + f.length); + e = 0; + try { + f.toString(); + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.toString] e = " + e); + } + else { + ok(window instanceof Object, "window not instance of Object"); + ok(document instanceof Object, "document not instance of Object"); + ok(Object.isExtensible(window), "window is not extensible"); + ok(Object.isExtensible(document), "document is not extensible"); + + ok(f.toString() === "\nfunction createElement() {\n [native code]\n}\n", "f.toString() = " + f.toString()); + ok(Object.getPrototypeOf(f) === Function.prototype, "unexpected document.createElement prototype"); + ok(Object.getPrototypeOf(f.apply) === Function.prototype, "unexpected f.apply prototype"); + ok(Object.getPrototypeOf(f.call) === Function.prototype, "unexpected f.call prototype"); + } + + e = 0; + try { + f.call(Object, "div"); + }catch(ex) { + e = ex.number; + } + ok(e === (v < 9 ? 0xa0005 : 0x0ffff) - 0x80000000, "[f.call(Object, 'div')] e = " + e); + + var elem = f.call(document, "div"); + elem.setAttribute("class", "cls"); + elem.setAttribute("className", "cls"); + ok(elem.className === "cls", "elem.className = " + elem.className); + + if(v < 9) { + e = 0; + try { + elem = f.call.call(f, document, "div"); + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[elem = f.call.call(f, document, 'div')] e = " + e); + e = 0; + try { + f = f.bind(document); + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.bind(document)] e = " + e); + elem = f.apply(document, ["style"]); + document.body.appendChild(elem); + + var enumerator = new Enumerator(document.getElementsByTagName("style")); + var enum_elem = enumerator.item(); + enumerator.moveNext(); + ok(enum_elem === elem, "enum_elem = " + enum_elem); + ok(enumerator.atEnd(), "enumerator not at end"); + + e = 0; + try { + f.apply = 0; + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.apply = 0] e = " + e); + e = 0; + try { + f.call = function() { }; + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.call = function() { }] e = " + e); + e = 0; + try { + f.arguments = 0; + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.arguments = 0] e = " + e); + e = 0; + try { + f.length = 0; + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.length = 0] e = " + e); + + f = f.apply; + ok(f.arguments === undefined, "f.apply.arguments = " + f.arguments); + ok(f.length === undefined, "f.apply.length = " + f.length); + e = 0; + try { + f.toString(); + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.apply.toString] e = " + e); + e = 0; + try { + f(document, ["style"]); + }catch(ex) { + e = ex.number; + } + ok(e === 0xa01b6 - 0x80000000, "[f.apply() indirect] e = " + e); + } + else { + elem = f.call.call(f, document, "div"); + f = f.bind(document); + elem = f.apply(null, ["style"]); + document.body.appendChild(elem); + + try { + var enumerator = new Enumerator(document.getElementsByTagName("style")); + }catch(ex) { + e = ex.number; + } + todo_wine. + ok(e === 0xa01c3 - 0x80000000, "[style Enumerator] e = " + e); + + f.apply = 0; + f.call = function() { }; + ok(f.apply === 0, "changed f.apply = ", f.apply); + ok(f.call instanceof Function, "changed f.call not instance of Function"); + } +}); -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=99984 Your paranoid android. === w8adm (32 bit report) === mshtml: htmldoc.c:3084: Test failed: Incorrect error code: -2146697211 htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:5861: Test failed: expected OnChanged_1012 htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV htmldoc.c:5864: Test failed: expected Exec_SETTITLE htmldoc.c:5905: Test failed: expected FireNavigateComplete2 === w10pro64_ar (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS htmldoc.c:350: Test failed: expected Exec_SETTITLE htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/enumerator.c | 4 ++-- dlls/jscript/error.c | 1 + dlls/jscript/jscript.h | 1 + dlls/jscript/jscript.rc | 1 + dlls/jscript/resource.h | 1 + dlls/mshtml/tests/documentmode.js | 1 - 6 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index bdbebf7..5255448 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -248,7 +248,7 @@ static HRESULT create_enumerator(script_ctx_t *ctx, jsval_t *argv, jsdisp_t **re if (FAILED(hres)) { WARN("Enumerator: no DISPID_NEWENUM.\n"); - return E_INVALIDARG; + return JS_E_OBJECT_NOT_COLLECTION; } if ((V_VT(&varresult) == VT_DISPATCH) || (V_VT(&varresult) == VT_UNKNOWN)) @@ -259,7 +259,7 @@ static HRESULT create_enumerator(script_ctx_t *ctx, jsval_t *argv, jsdisp_t **re else { FIXME("Enumerator: NewEnum unexpected type of varresult (%d).\n", V_VT(&varresult)); - hres = E_INVALIDARG; + hres = JS_E_OBJECT_NOT_COLLECTION; } VariantClear(&varresult); if (FAILED(hres)) diff --git a/dlls/jscript/error.c b/dlls/jscript/error.c index bb9e5a2..63c957f 100644 --- a/dlls/jscript/error.c +++ b/dlls/jscript/error.c @@ -461,6 +461,7 @@ jsdisp_t *create_builtin_error(script_ctx_t *ctx) case JS_E_INVALID_PROPERTY: case JS_E_INVALID_ACTION: case JS_E_MISSING_ARG: + case JS_E_OBJECT_NOT_COLLECTION: case JS_E_FUNCTION_EXPECTED: case JS_E_DATE_EXPECTED: case JS_E_NUMBER_EXPECTED: diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 59f7cfc..d0d04cd 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -574,6 +574,7 @@ static inline DWORD make_grfdex(script_ctx_t *ctx, DWORD flags) #define JS_E_INVALID_PROPERTY MAKE_JSERROR(IDS_NO_PROPERTY) #define JS_E_INVALID_ACTION MAKE_JSERROR(IDS_UNSUPPORTED_ACTION) #define JS_E_MISSING_ARG MAKE_JSERROR(IDS_ARG_NOT_OPT) +#define JS_E_OBJECT_NOT_COLLECTION MAKE_JSERROR(IDS_OBJECT_NOT_COLLECTION) #define JS_E_SYNTAX MAKE_JSERROR(IDS_SYNTAX_ERROR) #define JS_E_MISSING_SEMICOLON MAKE_JSERROR(IDS_SEMICOLON) #define JS_E_MISSING_LBRACKET MAKE_JSERROR(IDS_LBRACKET) diff --git a/dlls/jscript/jscript.rc b/dlls/jscript/jscript.rc index 50e2c30..45e1e27 100644 --- a/dlls/jscript/jscript.rc +++ b/dlls/jscript/jscript.rc @@ -33,6 +33,7 @@ STRINGTABLE IDS_NO_PROPERTY "Object doesn't support this property or method" IDS_UNSUPPORTED_ACTION "Object doesn't support this action" IDS_ARG_NOT_OPT "Argument not optional" + IDS_OBJECT_NOT_COLLECTION "Object not a collection" IDS_SYNTAX_ERROR "Syntax error" IDS_SEMICOLON "Expected ';'" IDS_LBRACKET "Expected '('" diff --git a/dlls/jscript/resource.h b/dlls/jscript/resource.h index b17f9fb..68e88db 100644 --- a/dlls/jscript/resource.h +++ b/dlls/jscript/resource.h @@ -31,6 +31,7 @@ #define IDS_NO_PROPERTY 0x01B6 #define IDS_UNSUPPORTED_ACTION 0x01BD #define IDS_ARG_NOT_OPT 0x01c1 +#define IDS_OBJECT_NOT_COLLECTION 0x01c3 #define IDS_SYNTAX_ERROR 0x03EA #define IDS_SEMICOLON 0x03EC #define IDS_LBRACKET 0x03ED diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 9ea940c..db431a8 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1153,7 +1153,6 @@ sync_test("builtin_obj", function() { }catch(ex) { e = ex.number; } - todo_wine. ok(e === 0xa01c3 - 0x80000000, "[style Enumerator] e = " + e); f.apply = 0; -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=99985 Your paranoid android. === w8adm (32 bit report) === mshtml: htmldoc.c:3084: Test failed: Incorrect error code: -2146697211 htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:5861: Test failed: expected OnChanged_1012 htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV htmldoc.c:5864: Test failed: expected Exec_SETTITLE htmldoc.c:5905: Test failed: expected FireNavigateComplete2
Hi Gabriel, The main idea behind this patch looks good, but I think some more consideration would be nice to make it fit better the bigger picture. Right now, we use IDispatch[Ex] all over jscript (that's how it's stored in jsval_t, for example) and we often needs three code paths: for IDispatch, IDispatchEx and jsdisp_t. With your patches, that we'd have four code paths, even if it's often hidden behind helpers. While we ultimately need separated code paths, I feel like we could do it architecturally better and it would be good to consider that. One way to achieve that is to abstract JS objects enough so that we'd operate on a single type everywhere outside the code dealing with the difference (dispex.c, more or less). Note that pure external IDispatch and IDispatchEx could be represented by jsdisp_t restricted to have only proxy properties. IDispatch -> jsdisp_t mapping would be a bit tricky in this case, but some sort of rb tree should do. If we had that, we could replace all IDispatch usage with jsdisp_t, including jsval_t. jsdisp_t (or a new jsobj_t type) could then internally sort of differences between those, maybe with an internal vtbl. Another thing that we will need to support is proper prototype chain support for MSHTML objects. Eg. those functions should not really be properties of objects, but rather their prototypes. I think that it will be possible to express by extending something like this extension, but that's something to keep in mind. I'm not expecting you to implement all of that, I'm not even sure about details of it without trying it, but let's make sure we won't make it harder in the future. On 10/13/21 4:11 PM, Gabriel Ivăncescu wrote:
Native mshtml objects act like JS objects in IE9 modes and up (the JS engine is implemented in mshtml). This implements proxy JS objects for certain dispatch objects that export a new private (internal to Wine) interface in versions ES5 and up, to achieve the same behavior. It delegates as much as possible to mshtml to implement all quirks there. Props are queried by the underlying dispatch and accesses to them forwarded to the dispatch object.
Since it's purely internal, the private interface is not part of any typelib, and for simplicity, it extends (inherits from) IDispatchEx, so we can just cast it as needed.
Given certain conditions, we query IDispatch objects on demand for the interface. If it's available, we either create a JS proxy object that is associated with the dispatch object exposing the interface, or re-acquire another one (possibly dangling). It is also associated on the other end, so that we keep a 1:1 mapping between the JS proxy object and the actual dispatch object, to avoid having multiple JS proxy objects for the same mshtml object (which would result in different dispids on them, and subtle bugs).
This keeps them both in sync at all times, on demand, without having to pre-create any objects, and deals with dynamic props and such automatically.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> ---
There's only one caveat here: since the JS proxy object references the dispatch object, while the dispatch object is also associated to the JS proxy object, the JS proxy object must be kept alive even when its refcount is zero. We can't hold a reference from the dispatch object, because that would create a circular reference.
In effect, we *need* to know *when* the JS proxy object has no references from jscript part, to release the dispatch object. But the dispatch object itself might still remain alive (if kept alive by mshtml refs), and the JS proxy object must also remain associated in this case, hence we keep it alive even when refcount == 0, but remove the proxy association on its side.
This is slightly ugly, but it is the best approach I could think of, to avoid bugs or having to rely on more complicated hash table lookups.
While I'd hope for a bit cleaner solution, there is one more problem with moving all properties to jscript: cycle collector will no longer be able to deal with them. See dispex_traverse. Without cycle collector knowing about object properties, we will leak on cycles (we already don't traverse through builtin jscript objects, so there are leaks already, but this will make it worse).
+typedef struct { + FunctionInstance function; + IDispatchEx *proxy; + DISPID id; +} ProxyFunction;
Binding this to one exact object doesn't seem right. For example document.body.click.call(div_elem) should probably work fine and call on div_elem. Maybe it's better to store such functions as a pair of (IID,DISPID)? Thanks, Jacek
Hi Jacek, On 15/10/2021 22:27, Jacek Caban wrote:
Hi Gabriel,
The main idea behind this patch looks good, but I think some more consideration would be nice to make it fit better the bigger picture.
Right now, we use IDispatch[Ex] all over jscript (that's how it's stored in jsval_t, for example) and we often needs three code paths: for IDispatch, IDispatchEx and jsdisp_t. With your patches, that we'd have four code paths, even if it's often hidden behind helpers. While we ultimately need separated code paths, I feel like we could do it architecturally better and it would be good to consider that. One way to achieve that is to abstract JS objects enough so that we'd operate on a single type everywhere outside the code dealing with the difference (dispex.c, more or less).
Note that pure external IDispatch and IDispatchEx could be represented by jsdisp_t restricted to have only proxy properties. IDispatch -> jsdisp_t mapping would be a bit tricky in this case, but some sort of rb tree should do. If we had that, we could replace all IDispatch usage with jsdisp_t, including jsval_t. jsdisp_t (or a new jsobj_t type) could then internally sort of differences between those, maybe with an internal vtbl.
I have to admit I'm a bit worried about this. Please bear with me, I'll try to be brief, but there's just a lot to talk about. Initially, I tried something like that, trying to fit everything into jsdisp. Unfortunately, I gave up on that path because I realized it would not make the code much cleaner, if anything it would make it worse in some places—simply put, the existing jscript tests blew up, because the current implementation, despite all the code paths, is completely correct from behavioral perspective. That's because external Dispatch objects do *not* behave like JS objects at all. They're "special". In contrast, the proxies that this patch implements for mshtml objects *do* behave almost like JS objects, which is why it works and makes sense to have them in jsdisp. mshtml proxies have prototypes, have props, and stuff like extensible/frozen, they *do not* work with Enumerator, just like JS objects don't, and so on. Basically they make use of all the fields in jsdisp_t, more or less, and so can use the jsdisp code path just fine (with few additional paths for proxy-specific things or props). In contrast, external Dispatch objects *do not* have these. They do not have prototypes, they don't have props stored on jscript side (in fact, that leads to a ton of test failures, just that alone), have no concept of jscript extensibility, they also *work* with Enumerator if they expose _NewEnum, unlike both mshtml proxies and standard JS objects, and have no builtin props or any other data on jscript side. Excluding dispex.c, the different code paths now happen all throughout the code, but many of those in engine.c, object.c etc will still be needed simply to act differently because dispatch objects are *very* different from JS objects. To me, it doesn't make much sense to store them in jsdisp, when they'll (have to) literally ignore every single field in the jsdisp and behavior, except for the class in builtin_info perhaps. Any code that touches jsdisp fields will be *wrong* for dispatch objects and will require a different code path anyway. Although instead of checking for "to_jsdisp" being NULL or not, we'd probably check for a JSCLASS_DISPATCH class, but I don't see that as an improvement since it's still a check into a different (and necessary) code path. In fact, I'd argue it adds more potential complications to the code. First, as you mentioned, we'll need some sort of rbtree to map dispatches. This adds complexity to the code, but also runtime overhead, not just for lookups, but also for the rbtree entry, which uses 4 pointer-sized fields already for every object (5 if you count the pointer to the Dispatch). Then we'll have to add *more* special case handling in things like variant_to_jsval, to treat dispatch objects and map them, but also in jsval_to_variant, because external code that *expects* the original dispatch pointer (and this is tested) and not something wrapped in a jsdisp dispatch, otherwise we'd break the existing tests! This patch already does this for proxies, but (1) it doesn't require rbtrees and (2) the only reason it does it in jsval_to_variant is because mshtml builtin functions expect passed parameters to have specific IIDs in some cases, so they need to have the original dispatch passed to them. Having them wrapped in a JS object dispatch doesn't work in this case, but that's it. If there's a way to avoid that, I can drop the need to convert them back to their actual inner dispatch. Overall I think it's not worth simply because dispatch objects are just very different from JS objects, behavior-wise, and would require most of the different code paths anyway. In fact, I believe that the way it's currently done is pretty straightforward and correct, because the dispatch itself is the "lowest common divisor" and makes sense to store that in a jsval for example. I mean, I could conceivably add another container for all objects, that's a common divisor and avoid the pitfalls described above, but I don't see much point when IDispatch already does this. Sure you need checks for jsdisp in some cases but that's no different than using something like QueryInterface on a private interface for jsdisps. Or I could add some "ops" or "vtable" to the container and have a ton of methods in there to handle all the code paths that require different logic between them, but is that really much of an improvement though? Issue is that it will have methods dealing with vastly different parts of the code that require different code paths (some with stuff in object.c, some with stuff in function.c, engine.c, etc). Lastly, I want to give another example on a patch I haven't sent yet, and was planning to send after these are in, but will also be affected by this. In Function_apply we test for array or arguments class, but this is only correct in normal jscript, other modes have more quirks (that some js libraries examine or abuse). In mshtml before ES5, external dispatch objects are *allowed* as argument to apply() as long as they expose "length" as a prop. However, JS objects that have a "length" prop *are not* allowed. ES5 makes this different, allowing JS objects as well, but not throwing errors when "length" is not found, but giving an undefined array. So in this case, we'd still have to check there for jsdisp vs Dispatch object, because we have to reject JS objects unless they're array or arguments class, even if they expose "length", which is not the case with external Dispatch objects (in html mode only). This is yet another example of a different code path needed in a place different from dispex.c, simply due to how native works. How would this even look like if Dispatch objects were merged into jsdisp? Would it not still require a check of some sort and code path? Overall I'm just worried that it would be very hard not to break the existing (and future) tests without complicating the code even more than it currently is, with checks, considering that dispatch objects do not behave like jsdisps at all, and in fact any code path dealing with jsdisps and their fields will simply be wrong for Dispatch objects.
Another thing that we will need to support is proper prototype chain support for MSHTML objects. Eg. those functions should not really be properties of objects, but rather their prototypes. I think that it will be possible to express by extending something like this extension, but that's something to keep in mind. I'm not expecting you to implement all of that, I'm not even sure about details of it without trying it, but let's make sure we won't make it harder in the future.
Good point, but it shouldn't be hard to deal with; they are made to act like JS objects more or less and have prototypes (though I set them to object prototype, which is not correct I guess, but good enough for a first implementation). So, to fix this we'd just need to set them to proper prototypes in convert_to_proxy without really touching much else, so it can be easily done, in theory. It would probably need an internal interface GetPrototype method or something, but that's not a problem.
While I'd hope for a bit cleaner solution, there is one more problem with moving all properties to jscript: cycle collector will no longer be able to deal with them. See dispex_traverse. Without cycle collector knowing about object properties, we will leak on cycles (we already don't traverse through builtin jscript objects, so there are leaks already, but this will make it worse).
I don't really understand specifics here, but it goes through dynamic props, so it should work just fine I believe? The current proxy implementation delegates all the handling to mshtml (because it has specific quirks). The props on jscript side are just "indices" into the mshtml props (dynamic or not). So traversing the dynamic props will yield the same results as before; they're still there. What does note_cc_edge do? Does it somehow remove the prop? If so, I'll probably have to add a way for mshtml to notify jscript to also remove the prop index, but it's not necessary for correctness because an invalid index (that points to a non-existed DISPID or a deleted dynamic prop) is the same as a deleted one right now, it just takes a slot and some memory. Even if not, it shouldn't be hard using an internal interface here to notify it. But that depends on what note_cc_edge does.
+typedef struct { + FunctionInstance function; + IDispatchEx *proxy; + DISPID id; +} ProxyFunction;
Binding this to one exact object doesn't seem right. For example document.body.click.call(div_elem) should probably work fine and call on div_elem. Maybe it's better to store such functions as a pair of (IID,DISPID)?
Interesting, I'll have to test for this. I hadn't considered it, although I do test for incompatible objects now (and it throws the proper error, i.e. E_UNEXPECTED). But yeah, they're incompatible, so need to add tests for compatible objects of same type and see what happens. Thanks, Gabriel
Well unfortunately it looks like I'll have to look into the prototypes implementation somehow, as they are expected in practice as well by scripts. For IE9 and above, quick testing shows that all objects with the name [object X] (where X is the name) have an equivalent constructor literally called X and is even available from the js code (e.g. prototype.js uses window.Event.prototype to get the __proto__ for all events, since window.Event is the constructor of all events). Such prototypes are called [object XPrototype]. So I guess I'll rethink this a bit. Hopefully I find a way to synthesize the constructors with dynamic code from their names, because I would really like to avoid having to create separate constructors for every single object manually and duplicating so much code in mshtml. IE8 is a special case. It doesn't support "js-like objects" (with proxies in my impl), but it does have an artificial "prototype" property for all objects, just like builtin functions have artificial "apply" and "call" methods, which try to emulate javascript behavior but are *not* javascript objects or the actual javascript builtin props at all. Which is fine, because they don't use proxies. The IE8 objects are based on the IHTMLDOMConstructorCollection interface and its props—that's all it exposes. Note that all builtin object prototypes are probably the same since they all report [Interface prototype object] for IE8 mode. Anyway I'll leave IE8 aside for the moment. For IE7 and below modes nothing needs to be changed since the current behavior (not supporting prototypes at all) is correct. It does have 4 constructors (we already have them special cased) like Image and XMLHttpRequest, but those are implemented. Please feel free to offer suggestions here, if I missed anything. Even just things to check for. Anyway, since these mshtml proxies are basically acting like jsdisps, and will become even more different now than normal dispatch objects due to the constructors/prototypes (for reference, IE8 and below use normal dispatch objects, which is why they work so differently than jsdisps), I still think it's not a good idea at all to somehow make them use the same code path. In fact, the proxies use the jsdisp code path for the most part, with only a few exceptions, and PROP_PROXY for proxy props (but still within jsdisp codepath; the jsdisp fields usage is the same). This is completely unlike dispatch objects which must use a completely different code path, as it is now, because of how they work, and we have tests for that (especially for normal jscript code, not mshtml related). IMO, keeping the current dispatch-object behavior is needed to be correct for anything other than ES5 and up. In the meantime, I'll probably try to upstream some other patches I got, that shouldn't be dependent on proxies, that implement some other missing stuff that will be needed for later anyway. Hope that's fine. BTW I fixed the Proxy Functions and now work with compatible objects as you mentioned, with tests added. I don't store the IID on jscript side, though. I retrieve the mshtml func_disp and store it, and forward to its simulated "call" prop (I moved that patch that implements it earlier, which is needed anyway for IE8 and below), so all the IID checks are done on mshtml side, which is neat and avoids duplication/exposing details, and works across all modes (as tested to). Thanks, Gabriel
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban -
Marvin