[PATCH 0/10] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/es5.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js index 353d0b0b5c0..57fde4d7c9d 100644 --- a/dlls/mshtml/tests/es5.js +++ b/dlls/mshtml/tests/es5.js @@ -829,6 +829,16 @@ sync_test("defineProperty", function() { test_accessor_prop_desc(child.prototype, "funcprop_prot", desc); ok(obj.funcprop_prot(100) === 10, "obj.funcprop_prot() = " + obj.funcprop_prot(100)); + (function() { + ok(arguments.length === 3, "arguments.length = " + arguments.length); + ok(arguments[0] === 1, "arguments[0] = " + arguments[0]); + ok(arguments[1] === 2, "arguments[1] = " + arguments[1]); + ok(arguments[2] === 3, "arguments[2] = " + arguments[2]); + Object.defineProperty(arguments, "1", {value: "foobar", writable: false, enumerable: true, configurable: false}); + test_own_data_prop_desc(arguments, "1", false, true, false); + ok(arguments[1] === "foobar", "arguments[1] after defineProperty = " + arguments[1]); + })(1, 2, 3); + expect_exception(function() { Object.defineProperty(null, "funcprop", desc); }, JS_E_OBJECT_EXPECTED); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 175 +++++++++++++++++++++++++---------------- dlls/jscript/jscript.h | 15 ++++ 2 files changed, 124 insertions(+), 66 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 57882bf499e..5f41443f83d 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -60,6 +60,13 @@ struct _dispex_prop_t { int bucket_next; }; +static const struct prop_desc_vtbl dispex_prop_vtbl; + +static inline DWORD prop_id_to_idx(DISPID id) +{ + return id - 1; +} + static void fix_protref_prop(jsdisp_t *jsdisp, dispex_prop_t *prop) { DWORD ref; @@ -84,15 +91,21 @@ static inline DISPID prop_to_id(jsdisp_t *This, dispex_prop_t *prop) return prop - This->props + 1; } -static inline dispex_prop_t *get_prop(jsdisp_t *This, DISPID id) +static inline BOOL get_prop(jsdisp_t *This, DISPID id, struct prop_desc *ret) { - DWORD idx = id - 1; + DWORD idx = prop_id_to_idx(id); if(idx >= This->prop_cnt) - return NULL; + return FALSE; fix_protref_prop(This, &This->props[idx]); - return This->props[idx].type == PROP_DELETED ? NULL : &This->props[idx]; + if(This->props[idx].type == PROP_DELETED) + return FALSE; + + ret->vtbl = &dispex_prop_vtbl; + ret->jsdisp = This; + ret->id = id; + return TRUE; } static inline BOOL is_function_prop(dispex_prop_t *prop) @@ -603,6 +616,69 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t return E_FAIL; } +static HRESULT delete_prop(dispex_prop_t *prop, BOOL *ret) +{ + if(prop->type == PROP_PROTREF) { + *ret = TRUE; + return S_OK; + } + + if(!(prop->flags & PROPF_CONFIGURABLE)) { + *ret = FALSE; + return S_OK; + } + + *ret = TRUE; + + if(prop->type == PROP_JSVAL) + jsval_release(prop->u.val); + if(prop->type == PROP_ACCESSOR) { + if(prop->u.accessor.getter) + jsdisp_release(prop->u.accessor.getter); + if(prop->u.accessor.setter) + jsdisp_release(prop->u.accessor.setter); + } + prop->type = PROP_DELETED; + return S_OK; +} + +static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) +{ + return prop_get(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], r); +} + +static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) +{ + return prop_put(prop->jsdisp, &prop->jsdisp->props[prop_id_to_idx(prop->id)], val); +} + +static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WORD flags, + unsigned argc, jsval_t *argv, jsval_t *r, IServiceProvider *caller) +{ + return invoke_prop_func(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], flags, argc, argv, r, caller); +} + +static HRESULT dispex_prop_delete(struct prop_desc *prop, BOOL *ret) +{ + return delete_prop(&prop->jsdisp->props[prop_id_to_idx(prop->id)], ret); +} + +static void *dispex_prop_get_name(struct prop_desc *prop, BOOL bstr) +{ + const WCHAR *name = prop->jsdisp->props[prop_id_to_idx(prop->id)].name; + if(bstr) + return SysAllocString(name); + return jsstr_alloc(name); +} + +static const struct prop_desc_vtbl dispex_prop_vtbl = { + dispex_prop_get, + dispex_prop_put, + dispex_prop_invoke, + dispex_prop_delete, + dispex_prop_get_name +}; + HRESULT builtin_set_const(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) { TRACE("%p %s\n", jsthis, debugstr_jsval(value)); @@ -1916,7 +1992,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc { jsdisp_t *This = impl_from_IDispatchEx(iface); IServiceProvider *prev_caller; - dispex_prop_t *prop; + struct prop_desc prop; jsexcept_t ei; HRESULT hres; @@ -1925,8 +2001,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc if(pvarRes) V_VT(pvarRes) = VT_EMPTY; - prop = get_prop(This, id); - if(!prop && id != DISPID_VALUE) { + if(!get_prop(This, id, &prop) && id != DISPID_VALUE) { TRACE("invalid id\n"); return DISP_E_MEMBERNOTFOUND; } @@ -1953,8 +2028,8 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc break; passed_this = get_this(pdp); - if(prop) - hres = invoke_prop_func(This, passed_this, prop, wFlags, argc, argv, pvarRes ? &r : NULL, pspCaller); + if(id != DISPID_VALUE) + hres = prop.vtbl->invoke(&prop, passed_this, wFlags, argc, argv, pvarRes ? &r : NULL, pspCaller); else hres = jsdisp_call_value(This, passed_this ? jsval_disp(passed_this) : jsval_undefined(), wFlags, argc, argv, pvarRes ? &r : NULL); @@ -1971,8 +2046,8 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc case DISPATCH_PROPERTYGET: { jsval_t r; - if(prop) - hres = prop_get(This, to_disp(This), prop, &r); + if(id != DISPID_VALUE) + hres = prop.vtbl->get(&prop, to_disp(This), &r); else { hres = to_primitive(This->ctx, jsval_obj(This), &r, NO_HINT); if(hres == JS_E_TO_PRIMITIVE) @@ -1991,7 +2066,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc jsval_t val; DWORD i; - if(!prop) { + if(id == DISPID_VALUE) { hres = DISP_E_MEMBERNOTFOUND; break; } @@ -2011,7 +2086,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc if(FAILED(hres)) break; - hres = prop_put(This, prop, val); + prop.vtbl->put(&prop, val); jsval_release(val); break; } @@ -2027,32 +2102,6 @@ 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) -{ - if(prop->type == PROP_PROTREF) { - *ret = TRUE; - return S_OK; - } - - if(!(prop->flags & PROPF_CONFIGURABLE)) { - *ret = FALSE; - return S_OK; - } - - *ret = TRUE; - - if(prop->type == PROP_JSVAL) - jsval_release(prop->u.val); - if(prop->type == PROP_ACCESSOR) { - if(prop->u.accessor.getter) - jsdisp_release(prop->u.accessor.getter); - if(prop->u.accessor.setter) - jsdisp_release(prop->u.accessor.setter); - } - prop->type = PROP_DELETED; - return S_OK; -} - static HRESULT WINAPI DispatchEx_DeleteMemberByName(IDispatchEx *iface, BSTR bstrName, DWORD grfdex) { jsdisp_t *This = impl_from_IDispatchEx(iface); @@ -2079,18 +2128,17 @@ static HRESULT WINAPI DispatchEx_DeleteMemberByName(IDispatchEx *iface, BSTR bst static HRESULT WINAPI DispatchEx_DeleteMemberByDispID(IDispatchEx *iface, DISPID id) { jsdisp_t *This = impl_from_IDispatchEx(iface); - dispex_prop_t *prop; + struct prop_desc prop; BOOL b; TRACE("(%p)->(%lx)\n", This, id); - prop = get_prop(This, id); - if(!prop) { + if(!get_prop(This, id, &prop)) { WARN("invalid id\n"); return DISP_E_MEMBERNOTFOUND; } - return delete_prop(prop, &b); + return prop.vtbl->delete(&prop, &b); } static HRESULT WINAPI DispatchEx_GetMemberProperties(IDispatchEx *iface, DISPID id, DWORD grfdexFetch, DWORD *pgrfdex) @@ -2103,15 +2151,14 @@ static HRESULT WINAPI DispatchEx_GetMemberProperties(IDispatchEx *iface, DISPID static HRESULT WINAPI DispatchEx_GetMemberName(IDispatchEx *iface, DISPID id, BSTR *pbstrName) { jsdisp_t *This = impl_from_IDispatchEx(iface); - dispex_prop_t *prop; + struct prop_desc prop; TRACE("(%p)->(%lx %p)\n", This, id, pbstrName); - prop = get_prop(This, id); - if(!prop) + if(!get_prop(This, id, &prop)) return DISP_E_MEMBERNOTFOUND; - *pbstrName = SysAllocString(prop->name); + *pbstrName = prop.vtbl->get_name(&prop, TRUE); if(!*pbstrName) return E_OUTOFMEMORY; @@ -2388,13 +2435,12 @@ HRESULT jsdisp_call_value(jsdisp_t *jsfunc, jsval_t vthis, WORD flags, unsigned HRESULT jsdisp_call(jsdisp_t *disp, DISPID id, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - dispex_prop_t *prop; + struct prop_desc prop; - prop = get_prop(disp, id); - if(!prop) + if(!get_prop(disp, id, &prop)) return DISP_E_MEMBERNOTFOUND; - return invoke_prop_func(disp, to_disp(disp), prop, flags, argc, argv, r, &disp->ctx->jscaller->IServiceProvider_iface); + return prop.vtbl->invoke(&prop, to_disp(disp), flags, argc, argv, r, &disp->ctx->jscaller->IServiceProvider_iface); } HRESULT jsdisp_call_name(jsdisp_t *disp, const WCHAR *name, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) @@ -2672,11 +2718,10 @@ HRESULT disp_propput(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t val) jsdisp = iface_to_jsdisp(disp); if(jsdisp && jsdisp->ctx == ctx) { - dispex_prop_t *prop; + struct prop_desc prop; - prop = get_prop(jsdisp, id); - if(prop) - hres = prop_put(jsdisp, prop, val); + if(get_prop(jsdisp, id, &prop)) + hres = prop.vtbl->put(&prop, val); else hres = DISP_E_MEMBERNOTFOUND; @@ -2778,13 +2823,12 @@ HRESULT jsdisp_get_idx(jsdisp_t *obj, DWORD idx, jsval_t *r) HRESULT jsdisp_propget(jsdisp_t *jsdisp, DISPID id, jsval_t *val) { - dispex_prop_t *prop; + struct prop_desc prop; - prop = get_prop(jsdisp, id); - if(!prop) + if(!get_prop(jsdisp, id, &prop)) return DISP_E_MEMBERNOTFOUND; - return prop_get(jsdisp, to_disp(jsdisp), prop, val); + return prop.vtbl->get(&prop, to_disp(jsdisp), val); } HRESULT disp_propget(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t *val) @@ -2839,11 +2883,10 @@ HRESULT disp_delete(IDispatch *disp, DISPID id, BOOL *ret) jsdisp = iface_to_jsdisp(disp); if(jsdisp) { - dispex_prop_t *prop; + struct prop_desc prop; - prop = get_prop(jsdisp, id); - if(prop) - hres = delete_prop(prop, ret); + if(get_prop(jsdisp, id, &prop)) + hres = prop.vtbl->delete(&prop, ret); else hres = DISP_E_MEMBERNOTFOUND; @@ -3199,11 +3242,11 @@ BOOL jsdisp_is_frozen(jsdisp_t *obj, BOOL sealed) HRESULT jsdisp_get_prop_name(jsdisp_t *obj, DISPID id, jsstr_t **r) { - dispex_prop_t *prop = get_prop(obj, id); + struct prop_desc prop; - if(!prop) + if(!get_prop(obj, id, &prop)) return DISP_E_MEMBERNOTFOUND; - *r = jsstr_alloc(prop->name); + *r = prop.vtbl->get_name(&prop, FALSE); return *r ? S_OK : E_OUTOFMEMORY; } diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 1a9446893a1..06be65fd153 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -53,6 +53,7 @@ typedef struct _jsexcept_t jsexcept_t; typedef struct _script_ctx_t script_ctx_t; typedef struct _dispex_prop_t dispex_prop_t; typedef struct _property_desc_t property_desc_t; +struct prop_desc_vtbl; typedef struct { void **blocks; @@ -247,6 +248,20 @@ void jsdisp_release(jsdisp_t*); #endif +struct prop_desc { + const struct prop_desc_vtbl *vtbl; + jsdisp_t *jsdisp; + DISPID id; +}; + +struct prop_desc_vtbl { + HRESULT (*get)(struct prop_desc*,IDispatch*,jsval_t*); + HRESULT (*put)(struct prop_desc*,jsval_t); + HRESULT (*invoke)(struct prop_desc*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*,IServiceProvider*); + HRESULT (*delete)(struct prop_desc*,BOOL*); + void *(*get_name)(struct prop_desc*,BOOL); +}; + enum jsdisp_enum_type { JSDISP_ENUM_ALL, JSDISP_ENUM_OWN, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> is_dispex_prop_id will be more useful later when we'll also have indexed props with their own vtbl, for now it avoids DISPID_VALUE. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 507 ++++++++++++++++++++++------------------- dlls/jscript/jscript.h | 7 + 2 files changed, 277 insertions(+), 237 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 5f41443f83d..fc7ca939294 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -257,7 +257,7 @@ static dispex_prop_t *alloc_protref(jsdisp_t *This, const WCHAR *name, DWORD ref return ret; } -static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, BOOL case_insens, dispex_prop_t **ret) +static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, BOOL case_insens, struct prop_desc *ret) { const builtin_prop_t *builtin; unsigned bucket, pos, prev = ~0; @@ -274,8 +274,8 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, This->props[bucket].bucket_head = pos; } - *ret = &This->props[pos]; - return S_OK; + prop = &This->props[pos]; + goto ret_prop; } prev = pos; @@ -300,8 +300,7 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, prop->type = PROP_JSVAL; prop->u.val = jsval_obj(obj); - *ret = prop; - return S_OK; + goto ret_prop; }else if(builtin->setter) flags |= PROPF_WRITABLE; flags &= PROPF_ENUMERABLE | PROPF_WRITABLE | PROPF_CONFIGURABLE; @@ -310,8 +309,7 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, return E_OUTOFMEMORY; prop->u.p = builtin; - *ret = prop; - return S_OK; + goto ret_prop; } if(This->builtin_info->idx_length) { @@ -329,79 +327,95 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, return E_OUTOFMEMORY; prop->u.idx = idx; - *ret = prop; - return S_OK; + goto ret_prop; } } - *ret = NULL; - return S_OK; + ret->id = 0; + return S_FALSE; + +ret_prop: + ret->vtbl = &dispex_prop_vtbl; + ret->jsdisp = This; + ret->id = prop_to_id(This, prop); + return prop->type != PROP_DELETED ? S_OK : S_FALSE; } -static HRESULT find_prop_name_prot(jsdisp_t *This, unsigned hash, const WCHAR *name, BOOL case_insens, dispex_prop_t **ret) +static HRESULT find_prop_name_prot(jsdisp_t *This, unsigned hash, const WCHAR *name, BOOL case_insens, struct prop_desc *ret) { - dispex_prop_t *prop, *del=NULL; + dispex_prop_t *prop, *del = NULL; HRESULT hres; - hres = find_prop_name(This, hash, name, case_insens, &prop); + hres = find_prop_name(This, hash, name, case_insens, ret); if(FAILED(hres)) return hres; - if(prop && prop->type==PROP_DELETED) { - del = prop; - } else if(prop) { + + if(hres == S_OK) { + prop = &This->props[prop_id_to_idx(ret->id)]; fix_protref_prop(This, prop); - *ret = prop; - return S_OK; + return prop->type != PROP_DELETED ? S_OK : S_FALSE; } + if(is_dispex_prop_id(ret->id)) + del = &This->props[prop_id_to_idx(ret->id)]; + if(This->prototype) { - hres = find_prop_name_prot(This->prototype, hash, name, case_insens, &prop); + hres = find_prop_name_prot(This->prototype, hash, name, case_insens, ret); if(FAILED(hres)) return hres; - if(prop && prop->type != PROP_DELETED) { + if(hres == S_OK) { if(del) { del->type = PROP_PROTREF; - del->u.ref = prop - This->prototype->props; + del->u.ref = prop_id_to_idx(ret->id); prop = del; }else { - prop = alloc_protref(This, prop->name, prop - This->prototype->props); + prop = alloc_protref(This, name, prop_id_to_idx(ret->id)); if(!prop) return E_OUTOFMEMORY; } - - *ret = prop; - return S_OK; + goto ret_prop; } } - *ret = del; - return S_OK; + if(!del) { + ret->id = 0; + return S_FALSE; + } + prop = del; + +ret_prop: + ret->vtbl = &dispex_prop_vtbl; + ret->jsdisp = This; + ret->id = prop_to_id(This, prop); + return prop->type != PROP_DELETED ? S_OK : S_FALSE; } -static HRESULT ensure_prop_name(jsdisp_t *This, const WCHAR *name, DWORD create_flags, BOOL case_insens, dispex_prop_t **ret) +static HRESULT ensure_prop_name(jsdisp_t *This, const WCHAR *name, DWORD create_flags, BOOL case_insens, struct prop_desc *ret) { dispex_prop_t *prop; HRESULT hres; - hres = find_prop_name_prot(This, string_hash(name), name, case_insens, &prop); - if(SUCCEEDED(hres) && (!prop || prop->type == PROP_DELETED)) { - TRACE("creating prop %s flags %lx\n", debugstr_w(name), create_flags); - - if(prop) { - prop->type = PROP_JSVAL; - prop->flags = create_flags; - prop->u.val = jsval_undefined(); - }else { - prop = alloc_prop(This, name, PROP_JSVAL, create_flags); - if(!prop) - return E_OUTOFMEMORY; - } + hres = find_prop_name_prot(This, string_hash(name), name, case_insens, ret); + if(hres != S_FALSE) + return hres; - prop->u.val = jsval_undefined(); + if(is_dispex_prop_id(ret->id)) { + prop = &This->props[prop_id_to_idx(ret->id)]; + prop->type = PROP_JSVAL; + prop->flags = create_flags; + }else { + prop = alloc_prop(This, name, PROP_JSVAL, create_flags); + if(!prop) + return E_OUTOFMEMORY; } - *ret = prop; - return hres; + TRACE("creating prop %s flags %lx\n", debugstr_w(name), create_flags); + + prop->u.val = jsval_undefined(); + ret->vtbl = &dispex_prop_vtbl; + ret->jsdisp = This; + ret->id = prop_to_id(This, prop); + return S_OK; } static IDispatch *get_this(DISPPARAMS *dp) @@ -663,6 +677,38 @@ static HRESULT dispex_prop_delete(struct prop_desc *prop, BOOL *ret) return delete_prop(&prop->jsdisp->props[prop_id_to_idx(prop->id)], ret); } +static HRESULT dispex_prop_get_desc(struct prop_desc *prop, BOOL flags_only, property_desc_t *desc) +{ + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + HRESULT hres; + + switch(p->type) { + case PROP_BUILTIN: + case PROP_JSVAL: + case PROP_IDX: + desc->explicit_value = TRUE; + if(!flags_only) { + hres = dispex_prop_get(prop, to_disp(prop->jsdisp), &desc->value); + if(FAILED(hres)) + return hres; + } + break; + case PROP_ACCESSOR: + desc->mask = PROPF_ENUMERABLE | PROPF_CONFIGURABLE; + desc->explicit_getter = desc->explicit_setter = TRUE; + if(!flags_only) { + desc->getter = p->u.accessor.getter ? jsdisp_addref(p->u.accessor.getter) : NULL; + desc->setter = p->u.accessor.setter ? jsdisp_addref(p->u.accessor.setter) : NULL; + } + break; + default: + return DISP_E_UNKNOWNNAME; + } + + desc->flags = p->flags & (PROPF_ENUMERABLE | PROPF_WRITABLE | PROPF_CONFIGURABLE); + return S_OK; +} + static void *dispex_prop_get_name(struct prop_desc *prop, BOOL bstr) { const WCHAR *name = prop->jsdisp->props[prop_id_to_idx(prop->id)].name; @@ -671,12 +717,135 @@ static void *dispex_prop_get_name(struct prop_desc *prop, BOOL bstr) return jsstr_alloc(name); } +static HRESULT dispex_prop_define(struct prop_desc *prop, property_desc_t *desc) +{ + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + script_ctx_t *ctx = prop->jsdisp->ctx; + HRESULT hres; + + if(p->type == PROP_DELETED || p->type == PROP_PROTREF) { + if(p->type == PROP_PROTREF && !prop->jsdisp->extensible) + return throw_error(ctx, JS_E_OBJECT_NONEXTENSIBLE, p->name); + + p->flags = desc->flags; + if(desc->explicit_getter || desc->explicit_setter) { + p->type = PROP_ACCESSOR; + p->u.accessor.getter = desc->getter ? jsdisp_addref(desc->getter) : NULL; + p->u.accessor.setter = desc->setter ? jsdisp_addref(desc->setter) : NULL; + TRACE("%s = accessor { get: %p set: %p }\n", debugstr_w(p->name), + p->u.accessor.getter, p->u.accessor.setter); + }else { + p->type = PROP_JSVAL; + if(desc->explicit_value) { + hres = jsval_copy(desc->value, &p->u.val); + if(FAILED(hres)) + return hres; + }else { + p->u.val = jsval_undefined(); + } + TRACE("%s = %s\n", debugstr_w(p->name), debugstr_jsval(p->u.val)); + } + return S_OK; + } + + TRACE("existing prop %s prop flags %lx desc flags %x desc mask %x\n", debugstr_w(p->name), + p->flags, desc->flags, desc->mask); + + if(!(p->flags & PROPF_CONFIGURABLE)) { + if(((desc->mask & PROPF_CONFIGURABLE) && (desc->flags & PROPF_CONFIGURABLE)) + || ((desc->mask & PROPF_ENUMERABLE) + && ((desc->flags & PROPF_ENUMERABLE) != (p->flags & PROPF_ENUMERABLE)))) + return throw_error(ctx, JS_E_NONCONFIGURABLE_REDEFINED, p->name); + } + + if(desc->explicit_value || (desc->mask & PROPF_WRITABLE)) { + if(p->type == PROP_ACCESSOR) { + if(!(p->flags & PROPF_CONFIGURABLE)) + return throw_error(ctx, JS_E_NONCONFIGURABLE_REDEFINED, p->name); + if(p->u.accessor.getter) + jsdisp_release(p->u.accessor.getter); + if(p->u.accessor.setter) + jsdisp_release(p->u.accessor.setter); + + p->type = PROP_JSVAL; + hres = jsval_copy(desc->value, &p->u.val); + if(FAILED(hres)) { + p->u.val = jsval_undefined(); + return hres; + } + }else { + if(!(p->flags & PROPF_CONFIGURABLE) && !(p->flags & PROPF_WRITABLE)) { + if((desc->mask & PROPF_WRITABLE) && (desc->flags & PROPF_WRITABLE)) + return throw_error(ctx, JS_E_NONWRITABLE_MODIFIED, p->name); + if(desc->explicit_value) { + if(p->type == PROP_JSVAL) { + BOOL eq; + hres = jsval_strict_equal(desc->value, p->u.val, &eq); + if(FAILED(hres)) + return hres; + if(!eq) + return throw_error(ctx, JS_E_NONWRITABLE_MODIFIED, p->name); + }else { + FIXME("redefinition of property type %d\n", p->type); + } + } + } + if(desc->explicit_value) { + if(p->type == PROP_JSVAL) + jsval_release(p->u.val); + else + p->type = PROP_JSVAL; + hres = jsval_copy(desc->value, &p->u.val); + if(FAILED(hres)) { + p->u.val = jsval_undefined(); + return hres; + } + } + } + }else if(desc->explicit_getter || desc->explicit_setter) { + if(p->type != PROP_ACCESSOR) { + if(!(p->flags & PROPF_CONFIGURABLE)) + return throw_error(ctx, JS_E_NONCONFIGURABLE_REDEFINED, p->name); + if(p->type == PROP_JSVAL) + jsval_release(p->u.val); + p->type = PROP_ACCESSOR; + p->u.accessor.getter = p->u.accessor.setter = NULL; + }else if(!(p->flags & PROPF_CONFIGURABLE)) { + if((desc->explicit_getter && desc->getter != p->u.accessor.getter) + || (desc->explicit_setter && desc->setter != p->u.accessor.setter)) + return throw_error(ctx, JS_E_NONCONFIGURABLE_REDEFINED, p->name); + } + + if(desc->explicit_getter) { + if(p->u.accessor.getter) { + jsdisp_release(p->u.accessor.getter); + p->u.accessor.getter = NULL; + } + if(desc->getter) + p->u.accessor.getter = jsdisp_addref(desc->getter); + } + if(desc->explicit_setter) { + if(p->u.accessor.setter) { + jsdisp_release(p->u.accessor.setter); + p->u.accessor.setter = NULL; + } + if(desc->setter) + p->u.accessor.setter = jsdisp_addref(desc->setter); + } + } + + p->flags = (p->flags & ~desc->mask) | (desc->flags & desc->mask); + return S_OK; +} + static const struct prop_desc_vtbl dispex_prop_vtbl = { dispex_prop_get, dispex_prop_put, dispex_prop_invoke, dispex_prop_delete, - dispex_prop_get_name + dispex_prop_get_desc, + dispex_prop_get_name, + dispex_prop_define }; HRESULT builtin_set_const(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) @@ -687,7 +856,7 @@ HRESULT builtin_set_const(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) static HRESULT fill_props(jsdisp_t *obj) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; if(obj->builtin_info->idx_length) { @@ -707,7 +876,8 @@ static HRESULT fill_props(jsdisp_t *obj) static HRESULT fill_protrefs(jsdisp_t *This) { - dispex_prop_t *iter, *prop; + struct prop_desc prop; + dispex_prop_t *iter; HRESULT hres; hres = fill_props(This); @@ -725,15 +895,14 @@ static HRESULT fill_protrefs(jsdisp_t *This) hres = find_prop_name(This, iter->hash, iter->name, FALSE, &prop); if(FAILED(hres)) return hres; - if(!prop || prop->type==PROP_DELETED) { - if(prop) { - prop->type = PROP_PROTREF; - prop->flags = 0; - prop->u.ref = iter - This->prototype->props; - }else { - prop = alloc_protref(This, iter->name, iter - This->prototype->props); - if(!prop) - return E_OUTOFMEMORY; + if(hres != S_OK) { + if(is_dispex_prop_id(prop.id)) { + dispex_prop_t *p = &This->props[prop_id_to_idx(prop.id)]; + p->type = PROP_PROTREF; + p->flags = 0; + p->u.ref = iter - This->prototype->props; + }else if(!alloc_protref(This, iter->name, iter - This->prototype->props)) { + return E_OUTOFMEMORY; } } } @@ -2105,7 +2274,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc static HRESULT WINAPI DispatchEx_DeleteMemberByName(IDispatchEx *iface, BSTR bstrName, DWORD grfdex) { jsdisp_t *This = impl_from_IDispatchEx(iface); - dispex_prop_t *prop; + struct prop_desc prop; BOOL b; HRESULT hres; @@ -2117,12 +2286,12 @@ static HRESULT WINAPI DispatchEx_DeleteMemberByName(IDispatchEx *iface, BSTR bst hres = find_prop_name(This, string_hash(bstrName), bstrName, grfdex & fdexNameCaseInsensitive, &prop); if(FAILED(hres)) return hres; - if(!prop) { + if(hres != S_OK) { TRACE("not found\n"); return S_OK; } - return delete_prop(prop, &b); + return prop.vtbl->delete(&prop, &b); } static HRESULT WINAPI DispatchEx_DeleteMemberByDispID(IDispatchEx *iface, DISPID id) @@ -2343,15 +2512,15 @@ void jsdisp_release(jsdisp_t *jsdisp) HRESULT init_dispex_from_constr(jsdisp_t *dispex, script_ctx_t *ctx, const builtin_info_t *builtin_info, jsdisp_t *constr) { + struct prop_desc prop; jsdisp_t *prot = NULL; - dispex_prop_t *prop; HRESULT hres; hres = find_prop_name_prot(constr, string_hash(L"prototype"), L"prototype", FALSE, &prop); - if(SUCCEEDED(hres) && prop && prop->type!=PROP_DELETED) { + if(hres == S_OK) { jsval_t val; - hres = prop_get(constr, to_disp(constr), prop, &val); + hres = prop.vtbl->get(&prop, to_disp(constr), &val); if(FAILED(hres)) { ERR("Could not get prototype\n"); return hres; @@ -2381,7 +2550,7 @@ jsdisp_t *iface_to_jsdisp(IDispatch *iface) HRESULT jsdisp_get_id(jsdisp_t *jsdisp, const WCHAR *name, DWORD flags, DISPID *id) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; if(jsdisp->extensible && (flags & fdexNameEnsure)) @@ -2392,8 +2561,8 @@ HRESULT jsdisp_get_id(jsdisp_t *jsdisp, const WCHAR *name, DWORD flags, DISPID * if(FAILED(hres)) return hres; - if(prop && prop->type!=PROP_DELETED) { - *id = prop_to_id(jsdisp, prop); + if(hres == S_OK) { + *id = prop.id; return S_OK; } @@ -2445,17 +2614,14 @@ HRESULT jsdisp_call(jsdisp_t *disp, DISPID id, WORD flags, unsigned argc, jsval_ HRESULT jsdisp_call_name(jsdisp_t *disp, const WCHAR *name, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; hres = find_prop_name_prot(disp, string_hash(name), name, FALSE, &prop); - if(FAILED(hres)) - return hres; - - if(!prop || prop->type == PROP_DELETED) - return JS_E_INVALID_PROPERTY; + if(hres != S_OK) + return FAILED(hres) ? hres : JS_E_INVALID_PROPERTY; - return invoke_prop_func(disp, to_disp(disp), prop, flags, argc, argv, r, &disp->ctx->jscaller->IServiceProvider_iface); + return prop.vtbl->invoke(&prop, to_disp(disp), flags, argc, argv, r, &disp->ctx->jscaller->IServiceProvider_iface); } static HRESULT disp_invoke(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, DISPPARAMS *params, VARIANT *r, @@ -2683,7 +2849,7 @@ HRESULT disp_call_value_with_caller(script_ctx_t *ctx, IDispatch *disp, jsval_t HRESULT jsdisp_propput(jsdisp_t *obj, const WCHAR *name, DWORD flags, BOOL throw, jsval_t val) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; if(obj->extensible) @@ -2692,10 +2858,10 @@ HRESULT jsdisp_propput(jsdisp_t *obj, const WCHAR *name, DWORD flags, BOOL throw hres = find_prop_name(obj, string_hash(name), name, FALSE, &prop); if(FAILED(hres)) return hres; - if(!prop || (prop->type == PROP_DELETED && !obj->extensible)) + if(hres != S_OK && (!prop.id || !obj->extensible)) return throw ? JS_E_INVALID_ACTION : S_OK; - return prop_put(obj, prop, val); + return prop.vtbl->put(&prop, val); } HRESULT jsdisp_propput_name(jsdisp_t *obj, const WCHAR *name, jsval_t val) @@ -2786,25 +2952,25 @@ HRESULT disp_propput_name(script_ctx_t *ctx, IDispatch *disp, const WCHAR *name, HRESULT jsdisp_propget_name(jsdisp_t *obj, const WCHAR *name, jsval_t *val) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; hres = find_prop_name_prot(obj, string_hash(name), name, FALSE, &prop); if(FAILED(hres)) return hres; - if(!prop || prop->type==PROP_DELETED) { + if(hres != S_OK) { *val = jsval_undefined(); return S_OK; } - return prop_get(obj, to_disp(obj), prop, val); + return prop.vtbl->get(&prop, to_disp(obj), val); } HRESULT jsdisp_get_idx(jsdisp_t *obj, DWORD idx, jsval_t *r) { + struct prop_desc prop; WCHAR name[12]; - dispex_prop_t *prop; HRESULT hres; swprintf(name, ARRAY_SIZE(name), L"%d", idx); @@ -2813,12 +2979,12 @@ HRESULT jsdisp_get_idx(jsdisp_t *obj, DWORD idx, jsval_t *r) if(FAILED(hres)) return hres; - if(!prop || prop->type==PROP_DELETED) { + if(hres != S_OK) { *r = jsval_undefined(); return DISP_E_UNKNOWNNAME; } - return prop_get(obj, to_disp(obj), prop, r); + return prop.vtbl->get(&prop, to_disp(obj), r); } HRESULT jsdisp_propget(jsdisp_t *jsdisp, DISPID id, jsval_t *val) @@ -2858,18 +3024,18 @@ HRESULT disp_propget(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t *val HRESULT jsdisp_delete_idx(jsdisp_t *obj, DWORD idx) { + struct prop_desc prop; WCHAR buf[12]; - dispex_prop_t *prop; BOOL b; HRESULT hres; swprintf(buf, ARRAY_SIZE(buf), L"%d", idx); hres = find_prop_name(obj, string_hash(buf), buf, FALSE, &prop); - if(FAILED(hres) || !prop) - return hres; + if(hres != S_OK) + return FAILED(hres) ? hres : S_OK; - hres = delete_prop(prop, &b); + hres = prop.vtbl->delete(&prop, &b); if(FAILED(hres)) return hres; return b ? S_OK : JS_E_INVALID_ACTION; @@ -2951,7 +3117,7 @@ HRESULT disp_delete_name(script_ctx_t *ctx, IDispatch *disp, jsstr_t *name, BOOL jsdisp = iface_to_jsdisp(disp); if(jsdisp) { - dispex_prop_t *prop; + struct prop_desc prop; const WCHAR *ptr; ptr = jsstr_flatten(name); @@ -2961,8 +3127,8 @@ HRESULT disp_delete_name(script_ctx_t *ctx, IDispatch *disp, jsstr_t *name, BOOL } hres = find_prop_name(jsdisp, string_hash(ptr), ptr, FALSE, &prop); - if(prop) { - hres = delete_prop(prop, ret); + if(hres == S_OK) { + hres = prop.vtbl->delete(&prop, ret); }else { *ret = TRUE; hres = S_OK; @@ -3004,173 +3170,40 @@ HRESULT disp_delete_name(script_ctx_t *ctx, IDispatch *disp, jsstr_t *name, BOOL HRESULT jsdisp_get_own_property(jsdisp_t *obj, const WCHAR *name, BOOL flags_only, property_desc_t *desc) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; hres = find_prop_name(obj, string_hash(name), name, FALSE, &prop); - if(FAILED(hres)) - return hres; - - if(!prop) - return DISP_E_UNKNOWNNAME; + if(hres != S_OK) + return FAILED(hres) ? hres : DISP_E_UNKNOWNNAME; memset(desc, 0, sizeof(*desc)); - - switch(prop->type) { - case PROP_BUILTIN: - case PROP_JSVAL: - case PROP_IDX: - desc->mask |= PROPF_WRITABLE; - desc->explicit_value = TRUE; - if(!flags_only) { - hres = prop_get(obj, to_disp(obj), prop, &desc->value); - if(FAILED(hres)) - return hres; - } - break; - case PROP_ACCESSOR: - desc->explicit_getter = desc->explicit_setter = TRUE; - if(!flags_only) { - desc->getter = prop->u.accessor.getter - ? jsdisp_addref(prop->u.accessor.getter) : NULL; - desc->setter = prop->u.accessor.setter - ? jsdisp_addref(prop->u.accessor.setter) : NULL; - } - break; - default: - return DISP_E_UNKNOWNNAME; - } - - desc->flags = prop->flags & (PROPF_ENUMERABLE | PROPF_WRITABLE | PROPF_CONFIGURABLE); - desc->mask |= PROPF_ENUMERABLE | PROPF_CONFIGURABLE; - return S_OK; + desc->mask = PROPF_ENUMERABLE | PROPF_WRITABLE | PROPF_CONFIGURABLE; + return prop.vtbl->get_desc(&prop, flags_only, desc); } HRESULT jsdisp_define_property(jsdisp_t *obj, const WCHAR *name, property_desc_t *desc) { - dispex_prop_t *prop; + struct prop_desc prop; HRESULT hres; hres = find_prop_name(obj, string_hash(name), name, FALSE, &prop); if(FAILED(hres)) return hres; - if((!prop || prop->type == PROP_DELETED || prop->type == PROP_PROTREF) && !obj->extensible) + if(hres != S_OK && !obj->extensible) return throw_error(obj->ctx, JS_E_OBJECT_NONEXTENSIBLE, name); - if(!prop && !(prop = alloc_prop(obj, name, PROP_DELETED, 0))) - return E_OUTOFMEMORY; - - if(prop->type == PROP_DELETED || prop->type == PROP_PROTREF) { - prop->flags = desc->flags; - if(desc->explicit_getter || desc->explicit_setter) { - prop->type = PROP_ACCESSOR; - prop->u.accessor.getter = desc->getter ? jsdisp_addref(desc->getter) : NULL; - prop->u.accessor.setter = desc->setter ? jsdisp_addref(desc->setter) : NULL; - TRACE("%s = accessor { get: %p set: %p }\n", debugstr_w(name), - prop->u.accessor.getter, prop->u.accessor.setter); - }else { - prop->type = PROP_JSVAL; - if(desc->explicit_value) { - hres = jsval_copy(desc->value, &prop->u.val); - if(FAILED(hres)) - return hres; - }else { - prop->u.val = jsval_undefined(); - } - TRACE("%s = %s\n", debugstr_w(name), debugstr_jsval(prop->u.val)); - } - return S_OK; - } - - TRACE("existing prop %s prop flags %lx desc flags %x desc mask %x\n", debugstr_w(name), - prop->flags, desc->flags, desc->mask); - - if(!(prop->flags & PROPF_CONFIGURABLE)) { - if(((desc->mask & PROPF_CONFIGURABLE) && (desc->flags & PROPF_CONFIGURABLE)) - || ((desc->mask & PROPF_ENUMERABLE) - && ((desc->flags & PROPF_ENUMERABLE) != (prop->flags & PROPF_ENUMERABLE)))) - return throw_error(obj->ctx, JS_E_NONCONFIGURABLE_REDEFINED, name); - } - - if(desc->explicit_value || (desc->mask & PROPF_WRITABLE)) { - if(prop->type == PROP_ACCESSOR) { - if(!(prop->flags & PROPF_CONFIGURABLE)) - return throw_error(obj->ctx, JS_E_NONCONFIGURABLE_REDEFINED, name); - if(prop->u.accessor.getter) - jsdisp_release(prop->u.accessor.getter); - if(prop->u.accessor.setter) - jsdisp_release(prop->u.accessor.setter); - - prop->type = PROP_JSVAL; - hres = jsval_copy(desc->value, &prop->u.val); - if(FAILED(hres)) { - prop->u.val = jsval_undefined(); - return hres; - } - }else { - if(!(prop->flags & PROPF_CONFIGURABLE) && !(prop->flags & PROPF_WRITABLE)) { - if((desc->mask & PROPF_WRITABLE) && (desc->flags & PROPF_WRITABLE)) - return throw_error(obj->ctx, JS_E_NONWRITABLE_MODIFIED, name); - if(desc->explicit_value) { - if(prop->type == PROP_JSVAL) { - BOOL eq; - hres = jsval_strict_equal(desc->value, prop->u.val, &eq); - if(FAILED(hres)) - return hres; - if(!eq) - return throw_error(obj->ctx, JS_E_NONWRITABLE_MODIFIED, name); - }else { - FIXME("redefinition of property type %d\n", prop->type); - } - } - } - if(desc->explicit_value) { - if(prop->type == PROP_JSVAL) - jsval_release(prop->u.val); - else - prop->type = PROP_JSVAL; - hres = jsval_copy(desc->value, &prop->u.val); - if(FAILED(hres)) { - prop->u.val = jsval_undefined(); - return hres; - } - } - } - }else if(desc->explicit_getter || desc->explicit_setter) { - if(prop->type != PROP_ACCESSOR) { - if(!(prop->flags & PROPF_CONFIGURABLE)) - return throw_error(obj->ctx, JS_E_NONCONFIGURABLE_REDEFINED, name); - if(prop->type == PROP_JSVAL) - jsval_release(prop->u.val); - prop->type = PROP_ACCESSOR; - prop->u.accessor.getter = prop->u.accessor.setter = NULL; - }else if(!(prop->flags & PROPF_CONFIGURABLE)) { - if((desc->explicit_getter && desc->getter != prop->u.accessor.getter) - || (desc->explicit_setter && desc->setter != prop->u.accessor.setter)) - return throw_error(obj->ctx, JS_E_NONCONFIGURABLE_REDEFINED, name); - } - - if(desc->explicit_getter) { - if(prop->u.accessor.getter) { - jsdisp_release(prop->u.accessor.getter); - prop->u.accessor.getter = NULL; - } - if(desc->getter) - prop->u.accessor.getter = jsdisp_addref(desc->getter); - } - if(desc->explicit_setter) { - if(prop->u.accessor.setter) { - jsdisp_release(prop->u.accessor.setter); - prop->u.accessor.setter = NULL; - } - if(desc->setter) - prop->u.accessor.setter = jsdisp_addref(desc->setter); - } + if(!prop.id) { + dispex_prop_t *p = alloc_prop(obj, name, PROP_DELETED, 0); + if(!p) + return E_OUTOFMEMORY; + prop.vtbl = &dispex_prop_vtbl; + prop.jsdisp = obj; + prop.id = prop_to_id(obj, p); } - prop->flags = (prop->flags & ~desc->mask) | (desc->flags & desc->mask); - return S_OK; + return prop.vtbl->define(&prop, desc); } HRESULT jsdisp_define_data_property(jsdisp_t *obj, const WCHAR *name, unsigned flags, jsval_t value) diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 06be65fd153..66de77856ad 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -259,9 +259,16 @@ struct prop_desc_vtbl { HRESULT (*put)(struct prop_desc*,jsval_t); HRESULT (*invoke)(struct prop_desc*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*,IServiceProvider*); HRESULT (*delete)(struct prop_desc*,BOOL*); + HRESULT (*get_desc)(struct prop_desc*,BOOL,property_desc_t*); void *(*get_name)(struct prop_desc*,BOOL); + HRESULT (*define)(struct prop_desc*,property_desc_t*); }; +static inline BOOL is_dispex_prop_id(DISPID id) +{ + return id > 0; +} + enum jsdisp_enum_type { JSDISP_ENUM_ALL, JSDISP_ENUM_OWN, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> It's only used in dispex_prop_delete now. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 51 ++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index fc7ca939294..f45838f3109 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -630,32 +630,6 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t return E_FAIL; } -static HRESULT delete_prop(dispex_prop_t *prop, BOOL *ret) -{ - if(prop->type == PROP_PROTREF) { - *ret = TRUE; - return S_OK; - } - - if(!(prop->flags & PROPF_CONFIGURABLE)) { - *ret = FALSE; - return S_OK; - } - - *ret = TRUE; - - if(prop->type == PROP_JSVAL) - jsval_release(prop->u.val); - if(prop->type == PROP_ACCESSOR) { - if(prop->u.accessor.getter) - jsdisp_release(prop->u.accessor.getter); - if(prop->u.accessor.setter) - jsdisp_release(prop->u.accessor.setter); - } - prop->type = PROP_DELETED; - return S_OK; -} - static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { return prop_get(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], r); @@ -674,7 +648,30 @@ static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WOR static HRESULT dispex_prop_delete(struct prop_desc *prop, BOOL *ret) { - return delete_prop(&prop->jsdisp->props[prop_id_to_idx(prop->id)], ret); + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + + if(p->type == PROP_PROTREF) { + *ret = TRUE; + return S_OK; + } + + if(!(p->flags & PROPF_CONFIGURABLE)) { + *ret = FALSE; + return S_OK; + } + + *ret = TRUE; + + if(p->type == PROP_JSVAL) + jsval_release(p->u.val); + if(p->type == PROP_ACCESSOR) { + if(p->u.accessor.getter) + jsdisp_release(p->u.accessor.getter); + if(p->u.accessor.setter) + jsdisp_release(p->u.accessor.setter); + } + p->type = PROP_DELETED; + return S_OK; } static HRESULT dispex_prop_get_desc(struct prop_desc *prop, BOOL flags_only, property_desc_t *desc) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> So it can work on any prop in the future, not just dispex props. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 68 ++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index f45838f3109..ad31f37c9fb 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -108,6 +108,16 @@ static inline BOOL get_prop(jsdisp_t *This, DISPID id, struct prop_desc *ret) return TRUE; } +static inline struct prop_desc get_prop_from_protref(jsdisp_t *prot, DWORD ref) +{ + struct prop_desc prop; + + prop.vtbl = &dispex_prop_vtbl; + prop.jsdisp = prot; + prop.id = ref + 1; + return prop; +} + static inline BOOL is_function_prop(dispex_prop_t *prop) { BOOL ret = FALSE; @@ -579,40 +589,54 @@ static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) return S_OK; } -static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t *prop, WORD flags, - unsigned argc, jsval_t *argv, jsval_t *r, IServiceProvider *caller) +static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { + return prop_get(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], r); +} + +static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) +{ + return prop_put(prop->jsdisp, &prop->jsdisp->props[prop_id_to_idx(prop->id)], val); +} + +static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WORD flags, + unsigned argc, jsval_t *argv, jsval_t *r, IServiceProvider *caller) +{ + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + jsdisp_t *jsdisp = prop->jsdisp; HRESULT hres; - switch(prop->type) { + switch(p->type) { case PROP_BUILTIN: return JS_E_FUNCTION_EXPECTED; - 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); + case PROP_PROTREF: { + struct prop_desc prot_prop = get_prop_from_protref(jsdisp->prototype, p->u.ref); + return prot_prop.vtbl->invoke(&prot_prop, jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface, + flags, argc, argv, r, caller); + } case PROP_JSVAL: { - if(!is_object_instance(prop->u.val)) { - FIXME("invoke %s\n", debugstr_jsval(prop->u.val)); + if(!is_object_instance(p->u.val)) { + FIXME("invoke %s\n", debugstr_jsval(p->u.val)); return E_FAIL; } - TRACE("call %s %p\n", debugstr_w(prop->name), get_object(prop->u.val)); + TRACE("call %s %p\n", debugstr_w(p->name), get_object(p->u.val)); - return disp_call_value_with_caller(This->ctx, get_object(prop->u.val), - jsval_disp(jsthis ? jsthis : (IDispatch*)&This->IDispatchEx_iface), + return disp_call_value_with_caller(jsdisp->ctx, get_object(p->u.val), + jsval_disp(jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface), flags, argc, argv, r, caller); } case PROP_ACCESSOR: case PROP_IDX: { jsval_t val; - hres = prop_get(This, jsthis ? jsthis : (IDispatch *)&This->IDispatchEx_iface, prop, &val); + hres = dispex_prop_get(prop, jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface, &val); if(FAILED(hres)) return hres; if(is_object_instance(val)) { - hres = disp_call_value_with_caller(This->ctx, get_object(val), - jsval_disp(jsthis ? jsthis : (IDispatch*)&This->IDispatchEx_iface), + hres = disp_call_value_with_caller(jsdisp->ctx, get_object(val), + jsval_disp(jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface), flags, argc, argv, r, caller); }else { FIXME("invoke %s\n", debugstr_jsval(val)); @@ -630,22 +654,6 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t return E_FAIL; } -static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) -{ - return prop_get(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], r); -} - -static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) -{ - return prop_put(prop->jsdisp, &prop->jsdisp->props[prop_id_to_idx(prop->id)], val); -} - -static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WORD flags, - unsigned argc, jsval_t *argv, jsval_t *r, IServiceProvider *caller) -{ - return invoke_prop_func(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], flags, argc, argv, r, caller); -} - static HRESULT dispex_prop_delete(struct prop_desc *prop, BOOL *ret) { dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 84 +++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index ad31f37c9fb..b14b58ad6ae 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -479,49 +479,6 @@ static HRESULT convert_params(script_ctx_t *ctx, const DISPPARAMS *dp, jsval_t * return S_OK; } -static HRESULT prop_get(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t *prop, jsval_t *r) -{ - jsdisp_t *prop_obj = This; - HRESULT hres; - - while(prop->type == PROP_PROTREF) { - prop_obj = prop_obj->prototype; - prop = prop_obj->props + prop->u.ref; - } - - switch(prop->type) { - case PROP_BUILTIN: - hres = prop->u.p->getter(This->ctx, prop_obj, r); - break; - case PROP_JSVAL: - hres = jsval_copy(prop->u.val, r); - break; - case PROP_ACCESSOR: - if(prop->u.accessor.getter) { - hres = jsdisp_call_value(prop->u.accessor.getter, jsval_disp(jsthis), - DISPATCH_METHOD, 0, NULL, r); - }else { - *r = jsval_undefined(); - hres = S_OK; - } - break; - case PROP_IDX: - hres = prop_obj->builtin_info->idx_get(prop_obj, prop->u.idx, r); - break; - default: - ERR("type %d\n", prop->type); - return E_FAIL; - } - - if(FAILED(hres)) { - TRACE("fail %08lx\n", hres); - return hres; - } - - TRACE("%p.%s ret %s\n", This, debugstr_w(prop->name), debugstr_jsval(*r)); - return hres; -} - static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) { HRESULT hres; @@ -591,7 +548,46 @@ static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { - return prop_get(prop->jsdisp, jsthis, &prop->jsdisp->props[prop_id_to_idx(prop->id)], r); + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + jsdisp_t *prop_obj = prop->jsdisp; + HRESULT hres; + + while(p->type == PROP_PROTREF) { + prop_obj = prop_obj->prototype; + p = prop_obj->props + p->u.ref; + } + + switch(p->type) { + case PROP_BUILTIN: + hres = p->u.p->getter(prop->jsdisp->ctx, prop_obj, r); + break; + case PROP_JSVAL: + hres = jsval_copy(p->u.val, r); + break; + case PROP_ACCESSOR: + if(p->u.accessor.getter) { + hres = jsdisp_call_value(p->u.accessor.getter, jsval_disp(jsthis), + DISPATCH_METHOD, 0, NULL, r); + }else { + *r = jsval_undefined(); + hres = S_OK; + } + break; + case PROP_IDX: + hres = prop_obj->builtin_info->idx_get(prop_obj, p->u.idx, r); + break; + default: + ERR("type %d\n", p->type); + return E_FAIL; + } + + if(FAILED(hres)) { + TRACE("fail %08lx\n", hres); + return hres; + } + + TRACE("%p.%s ret %s\n", prop->jsdisp, debugstr_w(p->name), debugstr_jsval(*r)); + return hres; } static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 133 +++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 68 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index b14b58ad6ae..da8c31fc278 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -479,73 +479,6 @@ static HRESULT convert_params(script_ctx_t *ctx, const DISPPARAMS *dp, jsval_t * return S_OK; } -static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) -{ - HRESULT hres; - - if(prop->type == PROP_PROTREF) { - dispex_prop_t *prop_iter = prop; - jsdisp_t *prototype_iter = This; - - do { - prototype_iter = prototype_iter->prototype; - prop_iter = prototype_iter->props + prop_iter->u.ref; - } while(prop_iter->type == PROP_PROTREF); - - if(prop_iter->type == PROP_ACCESSOR) - prop = prop_iter; - } - - switch(prop->type) { - case PROP_BUILTIN: - if(!prop->u.p->setter) { - TRACE("getter with no setter\n"); - return S_OK; - } - return prop->u.p->setter(This->ctx, This, val); - case PROP_PROTREF: - case PROP_DELETED: - if(!This->extensible) - return S_OK; - prop->type = PROP_JSVAL; - prop->flags = PROPF_ENUMERABLE | PROPF_CONFIGURABLE | PROPF_WRITABLE; - prop->u.val = jsval_undefined(); - break; - case PROP_JSVAL: - if(!(prop->flags & PROPF_WRITABLE)) - return S_OK; - - jsval_release(prop->u.val); - break; - case PROP_ACCESSOR: - if(!prop->u.accessor.setter) { - TRACE("no setter\n"); - return S_OK; - } - return jsdisp_call_value(prop->u.accessor.setter, jsval_obj(This), DISPATCH_METHOD, 1, &val, NULL); - case PROP_IDX: - if(!This->builtin_info->idx_put) { - TRACE("no put_idx\n"); - return S_OK; - } - return This->builtin_info->idx_put(This, prop->u.idx, val); - default: - ERR("type %d\n", prop->type); - return E_FAIL; - } - - TRACE("%p.%s = %s\n", This, debugstr_w(prop->name), debugstr_jsval(val)); - - hres = jsval_copy(val, &prop->u.val); - if(FAILED(hres)) - return hres; - - if(This->builtin_info->on_put) - This->builtin_info->on_put(This, prop->name); - - return S_OK; -} - static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; @@ -592,7 +525,71 @@ static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_ static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) { - return prop_put(prop->jsdisp, &prop->jsdisp->props[prop_id_to_idx(prop->id)], val); + dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; + jsdisp_t *jsdisp = prop->jsdisp; + HRESULT hres; + + if(p->type == PROP_PROTREF) { + dispex_prop_t *prop_iter = p; + jsdisp_t *prototype_iter = jsdisp; + + do { + prototype_iter = prototype_iter->prototype; + prop_iter = prototype_iter->props + prop_iter->u.ref; + } while(prop_iter->type == PROP_PROTREF); + + if(prop_iter->type == PROP_ACCESSOR) + p = prop_iter; + } + + switch(p->type) { + case PROP_BUILTIN: + if(!p->u.p->setter) { + TRACE("getter with no setter\n"); + return S_OK; + } + return p->u.p->setter(jsdisp->ctx, jsdisp, val); + case PROP_PROTREF: + case PROP_DELETED: + if(!jsdisp->extensible) + return S_OK; + p->type = PROP_JSVAL; + p->flags = PROPF_ENUMERABLE | PROPF_CONFIGURABLE | PROPF_WRITABLE; + p->u.val = jsval_undefined(); + break; + case PROP_JSVAL: + if(!(p->flags & PROPF_WRITABLE)) + return S_OK; + + jsval_release(p->u.val); + break; + case PROP_ACCESSOR: + if(!p->u.accessor.setter) { + TRACE("no setter\n"); + return S_OK; + } + return jsdisp_call_value(p->u.accessor.setter, jsval_obj(jsdisp), DISPATCH_METHOD, 1, &val, NULL); + case PROP_IDX: + if(!jsdisp->builtin_info->idx_put) { + TRACE("no put_idx\n"); + return S_OK; + } + return jsdisp->builtin_info->idx_put(jsdisp, p->u.idx, val); + default: + ERR("type %d\n", p->type); + return E_FAIL; + } + + TRACE("%p.%s = %s\n", jsdisp, debugstr_w(p->name), debugstr_jsval(val)); + + hres = jsval_copy(val, &p->u.val); + if(FAILED(hres)) + return hres; + + if(jsdisp->builtin_info->on_put) + jsdisp->builtin_info->on_put(jsdisp, p->name); + + return S_OK; } static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WORD flags, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- It's the only thing it was used for anyway, this will simplify next patch. --- dlls/jscript/dispex.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index da8c31fc278..7fa80fe51cd 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -131,7 +131,7 @@ static inline BOOL is_function_prop(dispex_prop_t *prop) return ret; } -static DWORD get_flags(jsdisp_t *This, dispex_prop_t *prop) +static BOOL is_enumerable(jsdisp_t *This, dispex_prop_t *prop) { if(prop->type == PROP_PROTREF) { dispex_prop_t *parent = NULL; @@ -141,13 +141,13 @@ static DWORD get_flags(jsdisp_t *This, dispex_prop_t *prop) if(!parent || parent->type == PROP_DELETED) { prop->type = PROP_DELETED; - return 0; + return FALSE; } - return get_flags(This->prototype, parent); + return is_enumerable(This->prototype, parent); } - return prop->flags; + return !!(prop->flags & PROPF_ENUMERABLE); } static const builtin_prop_t *find_builtin_prop(jsdisp_t *This, const WCHAR *name, BOOL case_insens) @@ -3094,7 +3094,7 @@ HRESULT jsdisp_next_prop(jsdisp_t *obj, DISPID id, enum jsdisp_enum_type enum_ty continue; if(enum_type != JSDISP_ENUM_ALL && iter->type == PROP_PROTREF) continue; - if(enum_type != JSDISP_ENUM_OWN && !(get_flags(obj, iter) & PROPF_ENUMERABLE)) + if(enum_type != JSDISP_ENUM_OWN && !is_enumerable(obj, iter)) continue; *ret = prop_to_id(obj, iter); return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 7fa80fe51cd..d17e0c04228 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -872,9 +872,27 @@ static HRESULT fill_props(jsdisp_t *obj) return S_OK; } -static HRESULT fill_protrefs(jsdisp_t *This) +static HRESULT fill_protref(jsdisp_t *This, unsigned hash, const WCHAR *name, DWORD ref) { struct prop_desc prop; + HRESULT hres; + + hres = find_prop_name(This, hash, name, FALSE, &prop); + if(hres != S_FALSE) + return hres; + if(is_dispex_prop_id(prop.id)) { + dispex_prop_t *p = &This->props[prop_id_to_idx(prop.id)]; + p->type = PROP_PROTREF; + p->flags = 0; + p->u.ref = ref; + }else if(!alloc_protref(This, name, ref)) { + return E_OUTOFMEMORY; + } + return S_OK; +} + +static HRESULT fill_protrefs(jsdisp_t *This) +{ dispex_prop_t *iter; HRESULT hres; @@ -890,19 +908,9 @@ static HRESULT fill_protrefs(jsdisp_t *This) return hres; for(iter = This->prototype->props; iter < This->prototype->props+This->prototype->prop_cnt; iter++) { - hres = find_prop_name(This, iter->hash, iter->name, FALSE, &prop); - if(FAILED(hres)) + hres = fill_protref(This, iter->hash, iter->name, iter - This->prototype->props); + if(hres != S_OK) return hres; - if(hres != S_OK) { - if(is_dispex_prop_id(prop.id)) { - dispex_prop_t *p = &This->props[prop_id_to_idx(prop.id)]; - p->type = PROP_PROTREF; - p->flags = 0; - p->u.ref = iter - This->prototype->props; - }else if(!alloc_protref(This, iter->name, iter - This->prototype->props)) { - return E_OUTOFMEMORY; - } - } } return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/arraybuf.c | 4 - dlls/jscript/dispex.c | 218 ++++++++++++++++++++++++++++---------- dlls/jscript/engine.c | 53 ++++++--- dlls/jscript/enumerator.c | 2 - dlls/jscript/function.c | 87 ++++++++++----- dlls/jscript/jscript.h | 24 ++++- dlls/jscript/jsregexp.c | 4 - dlls/jscript/set.c | 6 -- dlls/jscript/string.c | 68 ++++++++---- 9 files changed, 332 insertions(+), 134 deletions(-) diff --git a/dlls/jscript/arraybuf.c b/dlls/jscript/arraybuf.c index 62dfe614d65..fe20558fe50 100644 --- a/dlls/jscript/arraybuf.c +++ b/dlls/jscript/arraybuf.c @@ -617,8 +617,6 @@ static const builtin_info_t DataView_info = { DataView_destructor, NULL, NULL, - NULL, - NULL, DataView_gc_traverse }; @@ -630,8 +628,6 @@ static const builtin_info_t DataViewInst_info = { DataView_destructor, NULL, NULL, - NULL, - NULL, DataView_gc_traverse }; diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index d17e0c04228..ddd78a8fad6 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -36,7 +36,6 @@ typedef enum { PROP_PROTREF, PROP_ACCESSOR, PROP_DELETED, - PROP_IDX } prop_type_t; struct _dispex_prop_t { @@ -49,7 +48,6 @@ struct _dispex_prop_t { jsval_t val; const builtin_prop_t *p; DWORD ref; - unsigned idx; struct { jsdisp_t *getter; jsdisp_t *setter; @@ -67,6 +65,15 @@ static inline DWORD prop_id_to_idx(DISPID id) return id - 1; } +static inline const struct prop_desc_vtbl *get_prop_vtbl(jsdisp_t *jsdisp, DISPID id) +{ + if(prop_id_to_idx(id) < jsdisp->prop_cnt) + return &dispex_prop_vtbl; + if(is_dispex_prop_id(id) || !jsdisp->builtin_info->get_prop_vtbl) + return NULL; + return jsdisp->builtin_info->get_prop_vtbl(jsdisp, id); +} + static void fix_protref_prop(jsdisp_t *jsdisp, dispex_prop_t *prop) { DWORD ref; @@ -76,7 +83,12 @@ static void fix_protref_prop(jsdisp_t *jsdisp, dispex_prop_t *prop) ref = prop->u.ref; while((jsdisp = jsdisp->prototype)) { - if(ref >= jsdisp->prop_cnt || jsdisp->props[ref].type == PROP_DELETED) + if(ref >= jsdisp->prop_cnt) { + if(get_prop_vtbl(jsdisp, ref + 1)) + return; + break; + } + if(jsdisp->props[ref].type == PROP_DELETED) break; if(jsdisp->props[ref].type != PROP_PROTREF) return; @@ -93,16 +105,17 @@ static inline DISPID prop_to_id(jsdisp_t *This, dispex_prop_t *prop) static inline BOOL get_prop(jsdisp_t *This, DISPID id, struct prop_desc *ret) { - DWORD idx = prop_id_to_idx(id); - - if(idx >= This->prop_cnt) - return FALSE; - fix_protref_prop(This, &This->props[idx]); + const struct prop_desc_vtbl *vtbl = get_prop_vtbl(This, id); - if(This->props[idx].type == PROP_DELETED) + if(!vtbl) return FALSE; + if(is_dispex_prop_id(id)) { + fix_protref_prop(This, &This->props[prop_id_to_idx(id)]); - ret->vtbl = &dispex_prop_vtbl; + if(This->props[prop_id_to_idx(id)].type == PROP_DELETED) + return FALSE; + } + ret->vtbl = vtbl; ret->jsdisp = This; ret->id = id; return TRUE; @@ -112,7 +125,7 @@ static inline struct prop_desc get_prop_from_protref(jsdisp_t *prot, DWORD ref) { struct prop_desc prop; - prop.vtbl = &dispex_prop_vtbl; + prop.vtbl = is_dispex_prop_id(ref + 1) ? &dispex_prop_vtbl : prot->builtin_info->get_prop_vtbl(prot, ref + 1); prop.jsdisp = prot; prop.id = ref + 1; return prop; @@ -138,6 +151,8 @@ static BOOL is_enumerable(jsdisp_t *This, dispex_prop_t *prop) if(prop->u.ref < This->prototype->prop_cnt) parent = &This->prototype->props[prop->u.ref]; + else if(get_prop_vtbl(This->prototype, prop->u.ref + 1)) + return TRUE; if(!parent || parent->type == PROP_DELETED) { prop->type = PROP_DELETED; @@ -322,22 +337,18 @@ static HRESULT find_prop_name(jsdisp_t *This, unsigned hash, const WCHAR *name, goto ret_prop; } - if(This->builtin_info->idx_length) { + if(This->builtin_info->get_prop_vtbl) { + const struct prop_desc_vtbl *vtbl; const WCHAR *ptr; unsigned idx = 0; for(ptr = name; is_digit(*ptr) && idx < 0x10000; ptr++) idx = idx*10 + (*ptr-'0'); - if(!*ptr && idx < This->builtin_info->idx_length(This)) { - unsigned flags = PROPF_ENUMERABLE; - if(This->builtin_info->idx_put) - flags |= PROPF_WRITABLE; - prop = alloc_prop(This, name, PROP_IDX, flags); - if(!prop) - return E_OUTOFMEMORY; - - prop->u.idx = idx; - goto ret_prop; + if(!*ptr && (vtbl = This->builtin_info->get_prop_vtbl(This, indexed_prop_idx_to_id(idx)))) { + ret->vtbl = vtbl; + ret->jsdisp = This; + ret->id = indexed_prop_idx_to_id(idx); + return S_OK; } } @@ -357,7 +368,7 @@ static HRESULT find_prop_name_prot(jsdisp_t *This, unsigned hash, const WCHAR *n HRESULT hres; hres = find_prop_name(This, hash, name, case_insens, ret); - if(FAILED(hres)) + if(FAILED(hres) || is_indexed_prop_id(ret->id)) return hres; if(hres == S_OK) { @@ -406,7 +417,7 @@ static HRESULT ensure_prop_name(jsdisp_t *This, const WCHAR *name, DWORD create_ HRESULT hres; hres = find_prop_name_prot(This, string_hash(name), name, case_insens, ret); - if(hres != S_FALSE) + if(hres != S_FALSE || is_indexed_prop_id(ret->id)) return hres; if(is_dispex_prop_id(ret->id)) { @@ -483,10 +494,17 @@ static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_ { dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; jsdisp_t *prop_obj = prop->jsdisp; + const WCHAR *name = p->name; + struct prop_desc prot_prop; HRESULT hres; while(p->type == PROP_PROTREF) { prop_obj = prop_obj->prototype; + prot_prop = get_prop_from_protref(prop_obj, p->u.ref); + if(!is_dispex_prop_id(prot_prop.id)) { + hres = prot_prop.vtbl->get(&prot_prop, jsthis, r); + goto ret; + } p = prop_obj->props + p->u.ref; } @@ -506,9 +524,6 @@ static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_ hres = S_OK; } break; - case PROP_IDX: - hres = prop_obj->builtin_info->idx_get(prop_obj, p->u.idx, r); - break; default: ERR("type %d\n", p->type); return E_FAIL; @@ -519,7 +534,8 @@ static HRESULT dispex_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_ return hres; } - TRACE("%p.%s ret %s\n", prop->jsdisp, debugstr_w(p->name), debugstr_jsval(*r)); +ret: + TRACE("%p.%s ret %s\n", prop->jsdisp, debugstr_w(name), debugstr_jsval(*r)); return hres; } @@ -527,6 +543,7 @@ static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) { dispex_prop_t *p = &prop->jsdisp->props[prop_id_to_idx(prop->id)]; jsdisp_t *jsdisp = prop->jsdisp; + struct prop_desc prot_prop; HRESULT hres; if(p->type == PROP_PROTREF) { @@ -535,6 +552,10 @@ static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) do { prototype_iter = prototype_iter->prototype; + prot_prop = get_prop_from_protref(prototype_iter, prop_iter->u.ref); + if(!is_dispex_prop_id(prot_prop.id)) + return prot_prop.vtbl->put(&prot_prop, val); + prop_iter = prototype_iter->props + prop_iter->u.ref; } while(prop_iter->type == PROP_PROTREF); @@ -569,12 +590,6 @@ static HRESULT dispex_prop_put(struct prop_desc *prop, jsval_t val) return S_OK; } return jsdisp_call_value(p->u.accessor.setter, jsval_obj(jsdisp), DISPATCH_METHOD, 1, &val, NULL); - case PROP_IDX: - if(!jsdisp->builtin_info->idx_put) { - TRACE("no put_idx\n"); - return S_OK; - } - return jsdisp->builtin_info->idx_put(jsdisp, p->u.idx, val); default: ERR("type %d\n", p->type); return E_FAIL; @@ -619,8 +634,7 @@ static HRESULT dispex_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WOR jsval_disp(jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface), flags, argc, argv, r, caller); } - case PROP_ACCESSOR: - case PROP_IDX: { + case PROP_ACCESSOR: { jsval_t val; hres = dispex_prop_get(prop, jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface, &val); @@ -683,7 +697,6 @@ static HRESULT dispex_prop_get_desc(struct prop_desc *prop, BOOL flags_only, pro switch(p->type) { case PROP_BUILTIN: case PROP_JSVAL: - case PROP_IDX: desc->explicit_value = TRUE; if(!flags_only) { hres = dispex_prop_get(prop, to_disp(prop->jsdisp), &desc->value); @@ -846,30 +859,103 @@ static const struct prop_desc_vtbl dispex_prop_vtbl = { dispex_prop_define }; -HRESULT builtin_set_const(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) +HRESULT indexed_prop_invoke(struct prop_desc *prop, IDispatch *jsthis, WORD flags, + unsigned argc, jsval_t *argv, jsval_t *r, IServiceProvider *caller) { - TRACE("%p %s\n", jsthis, debugstr_jsval(value)); + jsdisp_t *jsdisp = prop->jsdisp; + HRESULT hres; + jsval_t val; + + hres = prop->vtbl->get(prop, jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface, &val); + if(FAILED(hres)) + return hres; + + if(is_object_instance(val)) { + hres = disp_call_value_with_caller(jsdisp->ctx, get_object(val), + jsval_disp(jsthis ? jsthis : (IDispatch*)&jsdisp->IDispatchEx_iface), + flags, argc, argv, r, caller); + }else { + FIXME("invoke %s\n", debugstr_jsval(val)); + hres = E_NOTIMPL; + } + + jsval_release(val); + return hres; +} + +HRESULT indexed_prop_delete(struct prop_desc *prop, BOOL *ret) +{ + /* indexed props are not configurable */ + *ret = FALSE; return S_OK; } -static HRESULT fill_props(jsdisp_t *obj) +void *indexed_prop_get_name(struct prop_desc *prop, BOOL bstr) { - struct prop_desc prop; + WCHAR buf[11]; + unsigned len; + + len = swprintf(buf, ARRAY_SIZE(buf), L"%u", indexed_prop_id_to_idx(prop->id)); + if(bstr) + return SysAllocStringLen(buf, len); + return jsstr_alloc_len(buf, len); +} + +HRESULT indexed_prop_define(struct prop_desc *prop, property_desc_t *desc) +{ + property_desc_t prop_desc; + WCHAR buf[11]; HRESULT hres; + jsval_t val; + BOOL eq; + + TRACE("existing prop L\"%lu\" desc flags %x desc mask %x\n", indexed_prop_id_to_idx(prop->id), desc->flags, desc->mask); - if(obj->builtin_info->idx_length) { - unsigned i = 0, len = obj->builtin_info->idx_length(obj); - WCHAR name[12]; + if((desc->mask & desc->flags & (PROPF_CONFIGURABLE | PROPF_ENUMERABLE)) != (desc->mask & PROPF_ENUMERABLE)) { + hres = JS_E_NONCONFIGURABLE_REDEFINED; + goto throw; + } - for(i = 0; i < len; i++) { - swprintf(name, ARRAY_SIZE(name), L"%u", i); - hres = find_prop_name(obj, string_hash(name), name, FALSE, &prop); + if(desc->explicit_value || (desc->mask & PROPF_WRITABLE)) { + if(prop->vtbl->get_desc(prop, TRUE, &prop_desc) == S_OK && !(prop_desc.flags & PROPF_WRITABLE)) { + if(desc->mask & desc->flags & PROPF_WRITABLE) { + hres = JS_E_NONWRITABLE_MODIFIED; + goto throw; + } + if(desc->explicit_value) { + hres = prop->vtbl->get(prop, to_disp(prop->jsdisp), &val); + if(FAILED(hres)) + return hres; + hres = jsval_strict_equal(desc->value, val, &eq); + jsval_release(val); + if(FAILED(hres)) + return hres; + if(!eq) { + hres = JS_E_NONWRITABLE_MODIFIED; + goto throw; + } + } + }else if(desc->explicit_value) { + HRESULT hres = prop->vtbl->put(prop, desc->value); if(FAILED(hres)) return hres; } + }else if(desc->explicit_getter || desc->explicit_setter) { + hres = JS_E_NONCONFIGURABLE_REDEFINED; + goto throw; } return S_OK; + +throw: + swprintf(buf, ARRAY_SIZE(buf), L"%u", indexed_prop_id_to_idx(prop->id)); + return throw_error(prop->jsdisp->ctx, hres, buf); +} + +HRESULT builtin_set_const(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) +{ + TRACE("%p %s\n", jsthis, debugstr_jsval(value)); + return S_OK; } static HRESULT fill_protref(jsdisp_t *This, unsigned hash, const WCHAR *name, DWORD ref) @@ -880,6 +966,8 @@ static HRESULT fill_protref(jsdisp_t *This, unsigned hash, const WCHAR *name, DW hres = find_prop_name(This, hash, name, FALSE, &prop); if(hres != S_FALSE) return hres; + if(is_indexed_prop_id(prop.id)) + return S_OK; if(is_dispex_prop_id(prop.id)) { dispex_prop_t *p = &This->props[prop_id_to_idx(prop.id)]; p->type = PROP_PROTREF; @@ -896,10 +984,6 @@ static HRESULT fill_protrefs(jsdisp_t *This) dispex_prop_t *iter; HRESULT hres; - hres = fill_props(This); - if(FAILED(hres)) - return hres; - if(!This->prototype) return S_OK; @@ -907,6 +991,18 @@ static HRESULT fill_protrefs(jsdisp_t *This) if(FAILED(hres)) return hres; + if(This->prototype->builtin_info->get_prop_vtbl) { + WCHAR buf[11]; + DWORD i; + + for(i = 0; This->prototype->builtin_info->get_prop_vtbl(This->prototype, indexed_prop_idx_to_id(i)); i++) { + swprintf(buf, ARRAY_SIZE(buf), L"%u", i); + hres = fill_protref(This, string_hash(buf), buf, prop_id_to_idx(indexed_prop_idx_to_id(i))); + if(hres != S_OK) + return hres; + } + } + for(iter = This->prototype->props; iter < This->prototype->props+This->prototype->prop_cnt; iter++) { hres = fill_protref(This, iter->hash, iter->name, iter - This->prototype->props); if(hres != S_OK) @@ -3084,15 +3180,25 @@ HRESULT disp_delete(IDispatch *disp, DISPID id, BOOL *ret) HRESULT jsdisp_next_prop(jsdisp_t *obj, DISPID id, enum jsdisp_enum_type enum_type, DISPID *ret) { dispex_prop_t *iter; - DWORD idx = id; + BOOL fill = FALSE; HRESULT hres; + DWORD idx; + + id = (id == DISPID_STARTENUM) ? indexed_prop_idx_to_id(0) : id + 1; + if(is_indexed_prop_id(id)) { + if(obj->builtin_info->get_prop_vtbl && obj->builtin_info->get_prop_vtbl(obj, id)) { + *ret = id; + return S_OK; + } + fill = TRUE; + id = 1; + } + idx = prop_id_to_idx(id); - if(id == DISPID_STARTENUM || idx >= obj->prop_cnt) { - hres = (enum_type == JSDISP_ENUM_ALL) ? fill_protrefs(obj) : fill_props(obj); + if(fill || idx >= obj->prop_cnt) { + hres = (enum_type == JSDISP_ENUM_ALL) ? fill_protrefs(obj) : S_OK; if(FAILED(hres)) return hres; - if(id == DISPID_STARTENUM) - idx = 0; if(idx >= obj->prop_cnt) return S_FALSE; } diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 76e6a271bdf..b75d8faa1b0 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -479,23 +479,16 @@ static void scope_destructor(jsdisp_t *dispex) free(scope); } -static unsigned scope_idx_length(jsdisp_t *dispex) +static HRESULT scope_indexed_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { - scope_chain_t *scope = scope_from_dispex(dispex); - - return scope->detached_vars->argc; -} - -static HRESULT scope_idx_get(jsdisp_t *dispex, unsigned idx, jsval_t *r) -{ - scope_chain_t *scope = scope_from_dispex(dispex); + scope_chain_t *scope = scope_from_dispex(prop->jsdisp); - return jsval_copy(scope->detached_vars->var[idx], r); + return jsval_copy(scope->detached_vars->var[indexed_prop_id_to_idx(prop->id)], r); } -static HRESULT scope_idx_put(jsdisp_t *dispex, unsigned idx, jsval_t val) +static HRESULT scope_indexed_prop_put(struct prop_desc *prop, jsval_t val) { - scope_chain_t *scope = scope_from_dispex(dispex); + scope_chain_t *scope = scope_from_dispex(prop->jsdisp); jsval_t copy, *ref; HRESULT hres; @@ -503,12 +496,42 @@ static HRESULT scope_idx_put(jsdisp_t *dispex, unsigned idx, jsval_t val) if(FAILED(hres)) return hres; - ref = &scope->detached_vars->var[idx]; + ref = &scope->detached_vars->var[indexed_prop_id_to_idx(prop->id)]; jsval_release(*ref); *ref = copy; return S_OK; } +static HRESULT scope_indexed_prop_get_desc(struct prop_desc *prop, BOOL flags_only, property_desc_t *desc) +{ + if(!flags_only) { + HRESULT hres = scope_indexed_prop_get(prop, to_disp(prop->jsdisp), &desc->value); + if(FAILED(hres)) + return hres; + } + + desc->explicit_value = TRUE; + desc->flags = PROPF_ENUMERABLE | PROPF_WRITABLE; + return S_OK; +} + +static const struct prop_desc_vtbl scope_indexed_prop_vtbl = { + scope_indexed_prop_get, + scope_indexed_prop_put, + indexed_prop_invoke, + indexed_prop_delete, + scope_indexed_prop_get_desc, + indexed_prop_get_name, + indexed_prop_define +}; + +static const struct prop_desc_vtbl *scope_get_prop_vtbl(jsdisp_t *dispex, DISPID id) +{ + scope_chain_t *scope = scope_from_dispex(dispex); + + return indexed_prop_id_to_idx(id) < scope->detached_vars->argc ? &scope_indexed_prop_vtbl : NULL; +} + static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { scope_chain_t *scope = scope_from_dispex(dispex); @@ -551,9 +574,7 @@ static const builtin_info_t scope_info = { NULL, scope_destructor, NULL, - scope_idx_length, - scope_idx_get, - scope_idx_put, + scope_get_prop_vtbl, scope_gc_traverse }; diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index d962c65d229..ee80341696e 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -198,8 +198,6 @@ static const builtin_info_t EnumeratorInst_info = { Enumerator_destructor, NULL, NULL, - NULL, - NULL, Enumerator_gc_traverse }; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 68f841b38a1..a0ae48e4b1a 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -68,6 +68,7 @@ typedef struct { jsval_t *buf; scope_chain_t *scope; unsigned argc; + BYTE readonly_flags[]; } ArgumentsInstance; static HRESULT create_bind_function(script_ctx_t*,FunctionInstance*,jsval_t,unsigned,jsval_t*,jsdisp_t**r); @@ -119,12 +120,6 @@ static void Arguments_destructor(jsdisp_t *jsdisp) free(arguments); } -static unsigned Arguments_idx_length(jsdisp_t *jsdisp) -{ - ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); - return arguments->argc; -} - static jsval_t *get_argument_ref(ArgumentsInstance *arguments, unsigned idx) { if(arguments->buf) @@ -134,22 +129,24 @@ static jsval_t *get_argument_ref(ArgumentsInstance *arguments, unsigned idx) return arguments->scope->detached_vars->var + idx; } -static HRESULT Arguments_idx_get(jsdisp_t *jsdisp, unsigned idx, jsval_t *r) +static HRESULT Arguments_indexed_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { - ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); + ArgumentsInstance *arguments = arguments_from_jsdisp(prop->jsdisp); + DWORD idx = indexed_prop_id_to_idx(prop->id); - TRACE("%p[%u]\n", arguments, idx); + TRACE("%p[%lu]\n", arguments, idx); return jsval_copy(*get_argument_ref(arguments, idx), r); } -static HRESULT Arguments_idx_put(jsdisp_t *jsdisp, unsigned idx, jsval_t val) +static HRESULT Arguments_indexed_prop_put(struct prop_desc *prop, jsval_t val) { - ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); + ArgumentsInstance *arguments = arguments_from_jsdisp(prop->jsdisp); + DWORD idx = indexed_prop_id_to_idx(prop->id); jsval_t copy, *ref; HRESULT hres; - TRACE("%p[%u] = %s\n", arguments, idx, debugstr_jsval(val)); + TRACE("%p[%lu] = %s\n", arguments, idx, debugstr_jsval(val)); hres = jsval_copy(val, ©); if(FAILED(hres)) @@ -161,6 +158,58 @@ static HRESULT Arguments_idx_put(jsdisp_t *jsdisp, unsigned idx, jsval_t val) return S_OK; } +static HRESULT Arguments_indexed_prop_get_desc(struct prop_desc *prop, BOOL flags_only, property_desc_t *desc) +{ + ArgumentsInstance *arguments = arguments_from_jsdisp(prop->jsdisp); + DWORD idx = indexed_prop_id_to_idx(prop->id); + + if(!flags_only) { + HRESULT hres = Arguments_indexed_prop_get(prop, to_disp(prop->jsdisp), &desc->value); + if(FAILED(hres)) + return hres; + } + + desc->explicit_value = TRUE; + desc->flags = PROPF_ENUMERABLE; + if(!(arguments->readonly_flags[idx / 8] & (1u << idx % 8))) + desc->flags |= PROPF_WRITABLE; + + return S_OK; +} + +static HRESULT Arguments_indexed_prop_define(struct prop_desc *prop, property_desc_t *desc) +{ + ArgumentsInstance *arguments = arguments_from_jsdisp(prop->jsdisp); + DWORD idx = indexed_prop_id_to_idx(prop->id); + HRESULT hres; + + hres = indexed_prop_define(prop, desc); + if(FAILED(hres)) + return hres; + + /* Need to keep track of props made read-only */ + if((desc->mask & PROPF_WRITABLE) && !(desc->flags & PROPF_WRITABLE)) + arguments->readonly_flags[idx / 8] |= 1u << idx % 8; + + return hres; +} + +static const struct prop_desc_vtbl Arguments_indexed_prop_vtbl = { + Arguments_indexed_prop_get, + Arguments_indexed_prop_put, + indexed_prop_invoke, + indexed_prop_delete, + Arguments_indexed_prop_get_desc, + indexed_prop_get_name, + Arguments_indexed_prop_define +}; + +static const struct prop_desc_vtbl *Arguments_get_prop_vtbl(jsdisp_t *jsdisp, DISPID id) +{ + ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); + return indexed_prop_id_to_idx(id) < arguments->argc ? &Arguments_indexed_prop_vtbl : NULL; +} + static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *jsdisp) { ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); @@ -190,9 +239,7 @@ static const builtin_info_t Arguments_info = { 0, NULL, Arguments_destructor, NULL, - Arguments_idx_length, - Arguments_idx_get, - Arguments_idx_put, + Arguments_get_prop_vtbl, Arguments_gc_traverse }; @@ -201,7 +248,7 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) ArgumentsInstance *args; HRESULT hres; - args = calloc(1, sizeof(*args)); + args = calloc(1, FIELD_OFFSET(ArgumentsInstance, readonly_flags[(frame->argc + 7) / 8])); if(!args) return E_OUTOFMEMORY; @@ -609,8 +656,6 @@ static const builtin_info_t Function_info = { Function_destructor, NULL, NULL, - NULL, - NULL, Function_gc_traverse }; @@ -628,8 +673,6 @@ static const builtin_info_t FunctionInst_info = { Function_destructor, NULL, NULL, - NULL, - NULL, Function_gc_traverse }; @@ -815,8 +858,6 @@ static const builtin_info_t InterpretedFunction_info = { Function_destructor, NULL, NULL, - NULL, - NULL, Function_gc_traverse }; @@ -947,8 +988,6 @@ static const builtin_info_t BindFunction_info = { Function_destructor, NULL, NULL, - NULL, - NULL, Function_gc_traverse }; diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 66de77856ad..a525d33d1f5 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -186,9 +186,7 @@ typedef struct { const builtin_prop_t *props; void (*destructor)(jsdisp_t*); void (*on_put)(jsdisp_t*,const WCHAR*); - unsigned (*idx_length)(jsdisp_t*); - HRESULT (*idx_get)(jsdisp_t*,unsigned,jsval_t*); - HRESULT (*idx_put)(jsdisp_t*,unsigned,jsval_t); + const struct prop_desc_vtbl *(*get_prop_vtbl)(jsdisp_t*,DISPID); HRESULT (*gc_traverse)(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*); } builtin_info_t; @@ -269,6 +267,26 @@ static inline BOOL is_dispex_prop_id(DISPID id) return id > 0; } +static inline BOOL is_indexed_prop_id(DISPID id) +{ + return id < 0; +} + +static inline DWORD indexed_prop_id_to_idx(DISPID id) +{ + return (DWORD)id - 0x80000000u; +} + +static inline DISPID indexed_prop_idx_to_id(DWORD idx) +{ + return idx + 0x80000000u; +} + +HRESULT indexed_prop_invoke(struct prop_desc*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*,IServiceProvider*); +HRESULT indexed_prop_delete(struct prop_desc*,BOOL*); +void *indexed_prop_get_name(struct prop_desc*,BOOL); +HRESULT indexed_prop_define(struct prop_desc*,property_desc_t*); + enum jsdisp_enum_type { JSDISP_ENUM_ALL, JSDISP_ENUM_OWN, diff --git a/dlls/jscript/jsregexp.c b/dlls/jscript/jsregexp.c index 70673ec190e..31654176ab6 100644 --- a/dlls/jscript/jsregexp.c +++ b/dlls/jscript/jsregexp.c @@ -577,8 +577,6 @@ static const builtin_info_t RegExp_info = { RegExp_destructor, NULL, NULL, - NULL, - NULL, RegExp_gc_traverse }; @@ -598,8 +596,6 @@ static const builtin_info_t RegExpInst_info = { RegExp_destructor, NULL, NULL, - NULL, - NULL, RegExp_gc_traverse }; diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index 7e49e31a5d5..990461ea1cf 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -417,8 +417,6 @@ static const builtin_info_t Map_info = { Map_destructor, NULL, NULL, - NULL, - NULL, Map_gc_traverse }; @@ -576,8 +574,6 @@ static const builtin_info_t Set_info = { Map_destructor, NULL, NULL, - NULL, - NULL, Map_gc_traverse }; @@ -873,8 +869,6 @@ static const builtin_info_t WeakMap_info = { WeakMap_destructor, NULL, NULL, - NULL, - NULL, WeakMap_gc_traverse }; diff --git a/dlls/jscript/string.c b/dlls/jscript/string.c index 4033f0a2b56..f089f0634e7 100644 --- a/dlls/jscript/string.c +++ b/dlls/jscript/string.c @@ -1502,35 +1502,66 @@ static void String_destructor(jsdisp_t *dispex) free(This); } -static unsigned String_idx_length(jsdisp_t *jsdisp) +static HRESULT String_indexed_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { - StringInstance *string = string_from_jsdisp(jsdisp); - - /* - * NOTE: For invoke version < 2, indexed array is not implemented at all. - * Newer jscript.dll versions implement it on string type, not class, - * which is not how it should work according to spec. IE9 implements it - * properly, but it uses its own JavaScript engine inside MSHTML. We - * implement it here, but in the way IE9 and spec work. - */ - return string->dispex.ctx->version < 2 ? 0 : jsstr_length(string->str); -} - -static HRESULT String_idx_get(jsdisp_t *jsdisp, unsigned idx, jsval_t *r) -{ - StringInstance *string = string_from_jsdisp(jsdisp); + StringInstance *string = string_from_jsdisp(prop->jsdisp); + DWORD idx = indexed_prop_id_to_idx(prop->id); jsstr_t *ret; ret = jsstr_substr(string->str, idx, 1); if(!ret) return E_OUTOFMEMORY; - TRACE("%p[%u] = %s\n", string, idx, debugstr_jsstr(ret)); + TRACE("%p[%lu] = %s\n", string, idx, debugstr_jsstr(ret)); *r = jsval_string(ret); return S_OK; } +static HRESULT String_indexed_prop_put(struct prop_desc *prop, jsval_t val) +{ + return S_OK; +} + +static HRESULT String_indexed_prop_get_desc(struct prop_desc *prop, BOOL flags_only, property_desc_t *desc) +{ + if(!flags_only) { + HRESULT hres = String_indexed_prop_get(prop, to_disp(prop->jsdisp), &desc->value); + if(FAILED(hres)) + return hres; + } + + desc->explicit_value = TRUE; + desc->flags = PROPF_ENUMERABLE; + return S_OK; +} + +static const struct prop_desc_vtbl String_indexed_prop_vtbl = { + String_indexed_prop_get, + String_indexed_prop_put, + indexed_prop_invoke, + indexed_prop_delete, + String_indexed_prop_get_desc, + indexed_prop_get_name, + indexed_prop_define +}; + +static const struct prop_desc_vtbl *String_get_prop_vtbl(jsdisp_t *jsdisp, DISPID id) +{ + StringInstance *string = string_from_jsdisp(jsdisp); + + /* + * NOTE: For invoke version < 2, indexed array is not implemented at all. + * Newer jscript.dll versions implement it on string type, not class, + * which is not how it should work according to spec. IE9 implements it + * properly, but it uses its own JavaScript engine inside MSHTML. We + * implement it here, but in the way IE9 and spec work. + */ + if(string->dispex.ctx->version < 2 || indexed_prop_id_to_idx(id) >= jsstr_length(string->str)) + return NULL; + return &String_indexed_prop_vtbl; +} + static const builtin_prop_t String_props[] = { {L"anchor", String_anchor, PROPF_METHOD|1}, {L"big", String_big, PROPF_METHOD}, @@ -1588,8 +1619,7 @@ static const builtin_info_t StringInst_info = { StringInst_props, String_destructor, NULL, - String_idx_length, - String_idx_get + String_get_prop_vtbl }; /* ECMA-262 3rd Edition 15.5.3.2 */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5444
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=144571 Your paranoid android. === w10pro64_ar (64 bit report) === mshtml: script.c:1223: Test failed: L"/index.html?xhr.js: async_xhr2.readyState = 2" script.c:1223: Test failed: L"/index.html?xhr.js: async_xhr2.readyState = 2"
Jacek Caban (@jacek) commented about dlls/jscript/jscript.h:
#endif
+struct prop_desc { + const struct prop_desc_vtbl *vtbl; + jsdisp_t *jsdisp; + DISPID id; +};
Do we need this struct? dispex.c callers already know jsdisp and id and the usual way to pass them to vtbl functions is by passing two arguments. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66816
Jacek Caban (@jacek) commented about dlls/jscript/jscript.h:
#endif
+struct prop_desc { + const struct prop_desc_vtbl *vtbl; + jsdisp_t *jsdisp; + DISPID id; +}; + +struct prop_desc_vtbl { + HRESULT (*get)(struct prop_desc*,IDispatch*,jsval_t*); + HRESULT (*put)(struct prop_desc*,jsval_t); + HRESULT (*invoke)(struct prop_desc*,IDispatch*,WORD,unsigned,jsval_t*,jsval_t*,IServiceProvider*); + HRESULT (*delete)(struct prop_desc*,BOOL*); + void *(*get_name)(struct prop_desc*,BOOL); +};
Why is it a separate vtbl from `builtin_info_t`? Do you expect a single object to use different vtbls for different properties? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66817
You mentioned earlier that it will be useful for future MSHTML bindings, could you elaborate on that? It would be good to know what are requirements for the interface. For example, will this need to support arbitrary names (not only numeric array indexes)? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66818
On Wed Apr 3 20:21:51 2024 +0000, Jacek Caban wrote:
Do we need this struct? dispex.c callers already know jsdisp and id and the usual way to pass them to vtbl functions is by passing two arguments. I used this design to simplify the prop handling in the rest of the code, I thought that was the point. (same with below, explaining there)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66941
On Wed Apr 3 20:21:51 2024 +0000, Jacek Caban wrote:
Why is it a separate vtbl from `builtin_info_t`? Do you expect a single object to use different vtbls for different properties? The goal was to simplify the design for prop handlers in the rest of the code. The simplest for me was just like this for common cases:
if(!get_prop(..., &prop))
/* not found */
prop.vtbl->method(...)
In this design I wanted to have the vtbl tied to the specific prop. This is obviously needed for this: even an object with indexed props needs at least two operations on different props, the indexed ones and the dispex (named) props, which have different methods. If I put it in `builtin_info_t`, then I'd either have to special case named props (they apply to all objects), or duplicate that in objects that have indexed props. If I make it optional, I'd also have to check for the props methods being NULL and fall back to named props (unless I don't make it optional so that every object needs to have one). I mean, think of all the objects not having indexed props. Probably with helpers, else it becomes tedious. But honestly, if this is the case, the whole thing seems a little pointless. Wasn't the point of this to *avoid* checks like that everywhere? Right now all props are filled with the vtbl in only few places, the rest of code doesn't care. Even for objects not having indexed props. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66942
On Thu Apr 4 14:53:04 2024 +0000, Jacek Caban wrote:
You mentioned earlier that it will be useful for future MSHTML bindings, could you elaborate on that? It would be good to know what are requirements for the interface. For example, will this need to support arbitrary names (not only numeric array indexes)? Yeah. It's for the mshtml objects that have "async" props, like sessionStorage. By "async" I mean that the underlying props can pop in and out of existence at any point during script execution (of course we have tests for this but it's logical anyway), and we only know this at the time of lookup. Other such objects are, for instance, window or document (accessing elements as props), though those probably not as important as there's other ways of implementing it there.
But yes, we'll need it for normal named props too. That said, what basically needs to be done in most cases is to "hook" it in a few places and override the behavior: 1. At the start of `prop_get`. 2. At the start of `delete_prop`. 3. In `ensure_prop_name` at the end (we override the "definition" of a new prop). 4. Same as above, at the start of `jsdisp_define_property` to override the definition of a new prop. 5. There's a couple other areas that might need special casing (finding a prop by hash/name, or in get_prop, or when enumerating), but at least get_prop is workable without special casing it, by adding another method to the prop desc vtbl or the object's. It will require a bit more changes to the vtbl and how it works, I didn't put it in this MR because I tried to keep it as simple as possible. But in short, I plan to do something like: override every method above, including named props, and return a different vtbl with methods having the "override" hook at the beginning, and later forward to normal named prop method. At least it would be workable without adding checks for such objects in the code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_66943
On Thu Apr 4 14:44:01 2024 +0000, Gabriel Ivăncescu wrote:
The goal was to simplify the design for prop handlers in the rest of the code. The simplest for me was just like this for common cases: ```c if(!get_prop(..., &prop)) /* not found */ prop.vtbl->method(...) ``` In this design I wanted to have the vtbl tied to the specific prop. This is obviously needed for this: even an object with indexed props needs at least two operations on different props, the indexed ones and the dispex (named) props, which have different methods. If I put it in `builtin_info_t`, then I'd either have to special case named props (they apply to all objects), or duplicate that in objects that have indexed props. If I make it optional, I'd also have to check for the props methods being NULL and fall back to named props (unless I don't make it optional so that every object needs to have one). I mean, think of all the objects not having indexed props. Probably with helpers, else it becomes tedious. But honestly, if this is the case, the whole thing seems a little pointless. Wasn't the point of this to *avoid* checks like that everywhere? Right now all props are filled with the vtbl in only few places, the rest of code doesn't care. Even for objects not having indexed props. There is a whole range of solutions between checking for class ID in dispex functions and per-property vtbl. By following your logic, from dispex.c point of view, you may have at most two types of properties: internal or external. Representing that distinction doesn't need a per-property vtbl.
Looking at it from a different perspective, we already handle multiple types of properties represented by `prop_type_t`. One way of thinking about external properties is that it's another type on that list, except there is no `dispex_prop_t` to store it, so it's a meaning of NULL prop instead. (Maybe we'd even want to optionally offer storage for external implementations, for example for overrides, in which case we may even want `dispex_prop_t` entry for that.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67441
On Thu Apr 4 14:53:03 2024 +0000, Gabriel Ivăncescu wrote:
Yeah. It's for the mshtml objects that have "async" props, like sessionStorage. By "async" I mean that the underlying props can pop in and out of existence at any point during script execution (of course we have tests for this but it's logical anyway), and we only know this at the time of lookup. Other such objects are, for instance, window or document (accessing elements as props), though those probably not as important as there's other ways of implementing it there. But yes, we'll need it for normal named props too. That said, what basically needs to be done in most cases is to "hook" it in a few places and override the behavior: 1. At the start of `prop_get`. 2. At the start of `delete_prop`. 3. In `ensure_prop_name` at the end (we override the "definition" of a new prop). 4. Same as above, at the start of `jsdisp_define_property` to override the definition of a new prop. 5. There's a couple other areas that might need special casing (finding a prop by hash/name, or in get_prop, or when enumerating), but at least get_prop is workable without special casing it, by adding another method to the prop desc vtbl or the object's. It will require a bit more changes to the vtbl and how it works, I didn't put it in this MR because I tried to keep it as simple as possible. But in short, I plan to do something like: override every method above, including named props, and return a different vtbl with methods having the "override" hook at the beginning, and later forward to normal named prop method. At least it would be workable without adding checks for such objects in the code. Yeah, MSHTML bindings make it a lot more interesting. In fact, I think we should concentrate on getting that right first. If we have a good enough solution for arbitrary properties, it should be good enough for typed arrays too. The same is not true the other way around.
If we introduce an interface with such capabilities, it sounds to me like it should be able to handle builtin MSHTML properties too. Should we plan for that to be the same interface? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67442
On Tue Apr 9 22:57:09 2024 +0000, Jacek Caban wrote:
There is a whole range of solutions between checking for class ID in dispex functions and per-property vtbl. By following your logic, from dispex.c point of view, you may have at most two types of properties: internal or external. Representing that distinction doesn't need a per-property vtbl. Looking at it from a different perspective, we already handle multiple types of properties represented by `prop_type_t`. One way of thinking about external properties is that it's another type on that list, except there is no `dispex_prop_t` to store it, so it's a meaning of NULL prop instead. (Maybe we'd even want to optionally offer storage for external implementations, for example for overrides, in which case we may even want `dispex_prop_t` entry for that.) Right, but that doesn't really help when we don't allocate them at all. The other issue is how to deal with the mshtml objects (well, not related to mshtml itself, just happens for those object types). I'll comment on that below.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67522
On Tue Apr 9 22:57:09 2024 +0000, Jacek Caban wrote:
Yeah, MSHTML bindings make it a lot more interesting. In fact, I think we should concentrate on getting that right first. If we have a good enough solution for arbitrary properties, it should be good enough for typed arrays too. The same is not true the other way around. If we introduce an interface with such capabilities, it sounds to me like it should be able to handle builtin MSHTML properties too. Should we plan for that to be the same interface? Yeah, I usually kept this in mind when designing this interface per prop.
They don't necessarily have to be mshtml objects, if jscript had them itself, we'd need the same thing; i.e. it's not an interoperability issue, but rather object-specific. Anyway, for those objects, my plan was to override the dispex props too with another vtbl. This would change "get_prop_vtbl" to apply to *all* props of course; I haven't done that in this MR because I didn't want to complicate it, but it is doable for the most part. Most objects would just return the dispex prop vtbl for dispex props, but those objects would return a different vtbl that first overrides certain operations (for the async props), and then forwards to the dispex props. This way we avoid special checks for those objects. Of course, indexed props would remain the same as in this MR. Note that the "prop_desc" and the per-property vtbl isn't really an object owning an actual reference, it's just to simplify the code (we don't even hold ref to the object). It's basically just saying how the prop can operate. Do you still want to move them to the object vtbl? Doing so would require adding checks everywhere where it's used for the dispex props, unless I make them mandatory, and then fill most objects with the dispex methods. The latter might keep the caller side clean. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67523
On Wed Apr 10 15:44:56 2024 +0000, Gabriel Ivăncescu wrote:
Yeah, I usually kept this in mind when designing this interface per prop. They don't necessarily have to be mshtml objects, if jscript had them itself, we'd need the same thing; i.e. it's not an interoperability issue, but rather object-specific. Anyway, for those objects, my plan was to override the dispex props too with another vtbl. This would change "get_prop_vtbl" to apply to *all* props of course; I haven't done that in this MR because I didn't want to complicate it, but it is doable for the most part. Most objects would just return the dispex prop vtbl for dispex props, but those objects would return a different vtbl that first overrides certain operations (for the async props), and then forwards to the dispex props. This way we avoid special checks for those objects. Of course, indexed props would remain the same as in this MR. Note that the "prop_desc" and the per-property vtbl isn't really an object owning an actual reference, it's just to simplify the code (we don't even hold ref to the object). It's basically just saying how the prop can operate. Do you still want to move them to the object vtbl? Doing so would require adding checks everywhere where it's used for the dispex props, unless I make them mandatory, and then fill most objects with the dispex methods. The latter might keep the caller side clean. This ultimately is an interop issue with MSHML. We will need an interface that exposes more dispex capabilities by this MR (by allowing using arbitrary names) outside jscript. I imagine that internally, in jscript, we will have a similar interface on dispex side. For jscript objects, they would operate on that internal interface. For external (MSHTML) object, those could be routed on `builtin_info_t` layer to the public interface (or something like that). Is that right? This means that those two interfaces need to be compatible.
It's hard to comment on slices of the design without a big picture. For the big picture, the most important thing seems to be how the interop interface will look like. Could you draft the interface that we could discus it? Just a (not necessarily even compiling) header or IDL snippet should be enough for now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67564
On Wed Apr 10 21:18:15 2024 +0000, Jacek Caban wrote:
This ultimately is an interop issue with MSHML. We will need an interface that exposes more dispex capabilities by this MR (by allowing using arbitrary names) outside jscript. I imagine that internally, in jscript, we will have a similar interface on dispex side. For jscript objects, they would operate on that internal interface. For external (MSHTML) object, those could be routed on `builtin_info_t` layer to the public interface (or something like that). Is that right? This means that those two interfaces need to be compatible. It's hard to comment on slices of the design without a big picture. For the big picture, the most important thing seems to be how the interop interface will look like. Could you draft the interface that we could discus it? Just a (not necessarily even compiling) header or IDL snippet should be enough for now. Well yes, in a way it does require interop, but that's not the point I'm trying to make. As in, this specific part of the code—dealing with "async" props—has to be done regardless.
Imagine that localStorage was a jscript object, no mshtml at all. So, we write to the local file, add some keys and values. Now *every* localStorage object needs to have that key as a prop. So, my point was only about hooks in this case for stuff like prop_get, delete_prop, definining new props (to see if an old one now exists from the file) and looking up props (by name or DISPID). Hooks that we would need *even if the localStorage object was in jscript*. That's because we have to look at them regardless of whether we have a jscript prop or not. Now for the other topic, for mshtml interop, so far I have it like this, and btw it works so it's not a dead end: ```idl interface IWineJSDispatchHost : IDispatchEx { IWineJSDispatch** GetProxyFieldRef(); HRESULT PropFixOverride([out] struct proxy_prop_info *info); HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); HRESULT PropDefineOverride([out] struct proxy_prop_info *info); HRESULT PropGetInfo([in] const WCHAR *name, [in] BOOL case_insens, [retval, out] struct proxy_prop_info *info); HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); HRESULT PropDelete([in] DISPID id); HRESULT PropEnum(); HRESULT ToString([retval, out] BSTR *string); } ``` Most of those after `PropGetInfo` are self-explanatory and out of scope for this. The ones dealing with the async hooks are `PropOverride` and `PropFixOverride`. `PropOverride` is the one dealing with the hooks I outlined above; right now, of course without this MR, in prop_get for instance I have this at the top: ```c if((proxy_target = get_proxy_target(prop_obj))) { hres = IWineJSDispatchHost_PropOverride(proxy_target, prop->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(This->ctx, &var, r); VariantClear(&var); } goto done; } } ``` As you can see, it redirects the get to check if an async prop exists now first. **Even if** this was a jscript object, something similar would be required (I mean, a hook/check here for such object). In my idea, along with this MR, it would be to simply have the prop_get method in the vtbl (now in the per-prop vtbl) handle this without any special checks. That is, for jscript proxy objects, we'd have something like: ```c static HRESULT proxy_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { /* Do checks here if it's dispex prop etc */ ... hres = IWineJSDispatchHost_PropOverride(proxy_target, p->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(ctx, &var, r); VariantClear(&var); } return hres; } /* Forward to normal dispex method */ return dispex_prop_get(prop, jsthis, r); } ``` And the rest of the code that uses prop.vtbl->get is completely unchanged, no special casing needed. Similar is for stuff like delete_prop or defining new props. Now `PropFixOverride` is different. It "fixes" them up similar to fix_protref_prop. It will need new stuff in the vtbl, because it's used when looking up the prop (get_prop and find_prop_name). Obviously I'm not going to include that, and dead code, in this MR yet. It's not needed by Typed Arrays either. And maybe there's a better way with it, too. Anyway, I should probably start to rewrite this so it uses mandatory methods in the object vtbl, right? It's probably hard to see how it would look, but one thing I want to avoid is to add special cases and checks, since the whole point of the vtbl approach was to get rid of them… -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67620
On Thu Apr 11 15:05:09 2024 +0000, Gabriel Ivăncescu wrote:
Well yes, in a way it does require interop, but that's not the point I'm trying to make. As in, this specific part of the code—dealing with "async" props—has to be done regardless. Imagine that localStorage was a jscript object, no mshtml at all. So, we write to the local file, add some keys and values. Now *every* localStorage object needs to have that key as a prop. So, my point was only about hooks in this case for stuff like prop_get, delete_prop, definining new props (to see if an old one now exists from the file) and looking up props (by name or DISPID). Hooks that we would need *even if the localStorage object was in jscript*. That's because we have to look at them regardless of whether we have a jscript prop or not. Now for the other topic, for mshtml interop, so far I have it like this, and btw it works so it's not a dead end: ```idl interface IWineJSDispatchHost : IDispatchEx { IWineJSDispatch** GetProxyFieldRef(); HRESULT PropFixOverride([out] struct proxy_prop_info *info); HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); HRESULT PropDefineOverride([out] struct proxy_prop_info *info); HRESULT PropGetInfo([in] const WCHAR *name, [in] BOOL case_insens, [retval, out] struct proxy_prop_info *info); HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); HRESULT PropDelete([in] DISPID id); HRESULT PropEnum(); HRESULT ToString([retval, out] BSTR *string); } ``` Most of those after `PropGetInfo` are self-explanatory and out of scope for this. The ones dealing with the async hooks are `PropOverride` and `PropFixOverride`. `PropOverride` is the one dealing with the hooks I outlined above; right now, of course without this MR, in prop_get for instance I have this at the top: ```c if((proxy_target = get_proxy_target(prop_obj))) { hres = IWineJSDispatchHost_PropOverride(proxy_target, prop->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(This->ctx, &var, r); VariantClear(&var); } goto done; } } ``` As you can see, it redirects the get to check if an async prop exists now first. **Even if** this was a jscript object, something similar would be required (I mean, a hook/check here for such object). In my idea, along with this MR, it would be to simply have the prop_get method in the vtbl (now in the per-prop vtbl) handle this without any special checks. That is, for jscript proxy objects, we'd have something like: ```c static HRESULT proxy_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { /* Do checks here if it's dispex prop etc */ ... hres = IWineJSDispatchHost_PropOverride(proxy_target, p->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(ctx, &var, r); VariantClear(&var); } return hres; } /* Forward to normal dispex method */ return dispex_prop_get(prop, jsthis, r); } ``` And the rest of the code that uses prop.vtbl->get is completely unchanged, no special casing needed. Similar is for stuff like delete_prop or defining new props. Now `PropFixOverride` is different. It "fixes" them up similar to fix_protref_prop. It will need new stuff in the vtbl, because it's used when looking up the prop (get_prop and find_prop_name). Obviously I'm not going to include that, and dead code, in this MR yet. It's not needed by Typed Arrays either. And maybe there's a better way with it, too. Anyway, I should probably start to rewrite this so it uses mandatory methods in the object vtbl, right? It's probably hard to see how it would look, but one thing I want to avoid is to add special cases and checks, since the whole point of the vtbl approach was to get rid of them… Generally, I think we should emphasize exposing properties as the intent of the interface, not "hooking" or "proxing". In this spirit:
``` IWineJSDispatch** GetProxyFieldRef(); ``` It will need a better solution, but I guess we may discuss it separately and keep this thread for properties. ``` HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); ``` This is essentially a property getter mixed with some "override" logic, right? Why not just use something like `PropGetInfo` paired with property getter call when appropriate? ``` HRESULT PropDefineOverride([out] struct proxy_prop_info *info); ``` I don't know what it's for, hopefully it's redundant. ``` HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); ``` Do we really need `this_obj` param? This really makes sense only on "call on the object" (aka. DISPID_VALUE). But I guess that we will want MSHTML functions to be represented by jscript function objects, right? In that case it shouldn't be needed to pass that to MSHTML. Calling a member like `InvokeEx` does is not really the way JavaScript calls work (at least not abstractly, as in spec). Member calls are really `func = propget(obj, name); call(func, this=obj, args...);`, this could be handled on jscript side with little MSHTML involvement. Calling a member in `IDispatch` style is really just an optimization to avoid allocating all functions objects that are not used as objects. With MSHTML using prototypes, it's less of the concern, so maybe we don't need to worry about that and just allocate them on demand? Or if we still want the optimization, we could provide a way for property getter to return something like (iid, dispid) pair that jscript would treat like a function or something along those lines. I'm also don't feel like `flags` argument is needed. I'd expect getters and setters to fit better in separate functions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67722
On Fri Apr 12 12:33:28 2024 +0000, Jacek Caban wrote:
Generally, I think we should emphasize exposing properties as the intent of the interface, not "hooking" or "proxing". In this spirit: ``` IWineJSDispatch** GetProxyFieldRef(); ``` It will need a better solution, but I guess we may discuss it separately and keep this thread for properties. ``` HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); ``` This is essentially a property getter mixed with some "override" logic, right? Why not just use something like `PropGetInfo` paired with property getter call when appropriate? ``` HRESULT PropDefineOverride([out] struct proxy_prop_info *info); ``` I don't know what it's for, hopefully it's redundant. ``` HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); ``` Do we really need `this_obj` param? This really makes sense only on "call on the object" (aka. DISPID_VALUE). But I guess that we will want MSHTML functions to be represented by jscript function objects, right? In that case it shouldn't be needed to pass that to MSHTML. Calling a member like `InvokeEx` does is not really the way JavaScript calls work (at least not abstractly, as in spec). Member calls are really `func = propget(obj, name); call(func, this=obj, args...);`, this could be handled on jscript side with little MSHTML involvement. Calling a member in `IDispatch` style is really just an optimization to avoid allocating all functions objects that are not used as objects. With MSHTML using prototypes, it's less of the concern, so maybe we don't need to worry about that and just allocate them on demand? Or if we still want the optimization, we could provide a way for property getter to return something like (iid, dispid) pair that jscript would treat like a function or something along those lines. I'm also don't feel like `flags` argument is needed. I'd expect getters and setters to fit better in separate functions. Ah, and `PropFixOverride` looks redundant too, it should be possible to avoid that by changing prototype handling on jscript side.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67723
On Fri Apr 12 12:34:40 2024 +0000, Jacek Caban wrote:
Ah, and `PropFixOverride` looks redundant too, it should be possible to avoid that by changing prototype handling on jscript side. The `GetProxyFieldRef` was the easiest I could find to handle all the cyclic ref special cases, plus it's also actually useful from mshtml itself even without jscript (for mshtml hooking to obtain the dispex for the hook…), but yeah let's leave that aside for now, will probably need to see the code. :-D
`PropInvoke` basically mostly forwards the arguments on mshtml side, I thought that's the easiest? Note that I **already** store the functions or accessors as *actual jscript functions*. The PROPERTYGET and PROPERTYPUT are used only on actual props on mshtml side (including special ones like on localStorage). And yes, it is also used for calling the DISPID_VALUE (both in `invoke_prop_func` and `jsdisp_call_value`). I thought having one method that mostly forwards args in mshtml is the most elegant, especially because we already use those interfaces on mshtml for IE8 and below modes… `PropDefineOverride` is similar to `PropOverride` except it's for defining instead of getter. Maybe I can use the same method for both, I don't know. There's probably room for improvement for sure, but my point was that I do believe we still need a way to hook into the getter/define ops on jscript side at the very least, which this MR is doing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_67740
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin