From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/jscript.h | 6 +- dlls/jscript/object.c | 1 + dlls/jscript/set.c | 174 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 46644b8247b..d907da3a36c 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -117,6 +117,7 @@ typedef enum { JSCLASS_JSON, JSCLASS_MAP, JSCLASS_SET, + JSCLASS_WEAKMAP, } jsclass_t;
jsdisp_t *iface_to_jsdisp(IDispatch*); @@ -418,11 +419,12 @@ struct _script_ctx_t { jsdisp_t *vbarray_constr; jsdisp_t *map_prototype; jsdisp_t *set_prototype; + jsdisp_t *weakmap_prototype; }; - jsdisp_t *global_objects[22]; + jsdisp_t *global_objects[23]; }; }; -C_ASSERT(RTL_SIZEOF_THROUGH_FIELD(script_ctx_t, set_prototype) == RTL_SIZEOF_THROUGH_FIELD(script_ctx_t, global_objects)); +C_ASSERT(RTL_SIZEOF_THROUGH_FIELD(script_ctx_t, weakmap_prototype) == RTL_SIZEOF_THROUGH_FIELD(script_ctx_t, global_objects));
void script_release(script_ctx_t*);
diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 4f6acacbc95..65b25cab239 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -51,6 +51,7 @@ static HRESULT Object_toString(script_ctx_t *ctx, jsval_t vthis, WORD flags, uns L"[object Object]", L"[object Object]", L"[object Object]", + L"[object Object]", L"[object Object]" };
diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index eca26a890f7..f2de6924468 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -614,6 +614,165 @@ static HRESULT Set_constructor(script_ctx_t *ctx, jsval_t vthis, WORD flags, uns } }
+typedef struct { + jsdisp_t dispex; + struct rb_tree map; +} WeakMapInstance; + +struct weakmap_entry { + struct rb_entry entry; + jsdisp_t *key; + jsval_t value; +}; + +static int weakmap_compare(const void *k, const struct rb_entry *e) +{ + ULONG_PTR a = (ULONG_PTR)k, b = (ULONG_PTR)WINE_RB_ENTRY_VALUE(e, const struct weakmap_entry, entry)->key; + return (a > b) - (a < b); +} + +static void release_weakmap_entry(struct weakmap_entry *entry) +{ + jsval_release(entry->value); + free(entry); +} + +static HRESULT WeakMap_clear(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static HRESULT WeakMap_delete(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static HRESULT WeakMap_get(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static HRESULT WeakMap_set(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static HRESULT WeakMap_has(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static HRESULT WeakMap_value(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + FIXME("\n"); + return E_NOTIMPL; +} + +static void WeakMap_destructor(jsdisp_t *dispex) +{ + WeakMapInstance *weakmap = (WeakMapInstance*)dispex; + struct weakmap_entry *entry, *entry2; + + RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry) + release_weakmap_entry(entry); + + free(weakmap); +} + +static HRESULT WeakMap_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) +{ + WeakMapInstance *weakmap = (WeakMapInstance*)dispex; + struct weakmap_entry *entry, *entry2; + HRESULT hres; + + if(op == GC_TRAVERSE_UNLINK) { + RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry) + release_weakmap_entry(entry); + rb_destroy(&weakmap->map, NULL, NULL); + return S_OK; + } + + RB_FOR_EACH_ENTRY(entry, &weakmap->map, struct weakmap_entry, entry) { + hres = gc_process_linked_val(gc_ctx, op, dispex, &entry->value); + if(FAILED(hres)) + return hres; + } + return S_OK; +} + +static const builtin_prop_t WeakMap_prototype_props[] = { + {L"clear", WeakMap_clear, PROPF_METHOD}, + {L"delete", WeakMap_delete, PROPF_METHOD|1}, + {L"get", WeakMap_get, PROPF_METHOD|1}, + {L"has", WeakMap_has, PROPF_METHOD|1}, + {L"set", WeakMap_set, PROPF_METHOD|2}, +}; + +static const builtin_info_t WeakMap_prototype_info = { + JSCLASS_OBJECT, + WeakMap_value, + ARRAY_SIZE(WeakMap_prototype_props), + WeakMap_prototype_props, + NULL, + NULL +}; + +static const builtin_info_t WeakMap_info = { + JSCLASS_WEAKMAP, + WeakMap_value, + 0, + NULL, + WeakMap_destructor, + NULL, + NULL, + NULL, + NULL, + WeakMap_gc_traverse, +}; + +static HRESULT WeakMap_constructor(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + WeakMapInstance *weakmap; + HRESULT hres; + + switch(flags) { + case DISPATCH_CONSTRUCT: + TRACE("\n"); + + if(!r) + return S_OK; + if(!(weakmap = calloc(1, sizeof(*weakmap)))) + return E_OUTOFMEMORY; + + hres = init_dispex(&weakmap->dispex, ctx, &WeakMap_info, ctx->weakmap_prototype); + if(FAILED(hres)) + return hres; + + rb_init(&weakmap->map, weakmap_compare); + *r = jsval_obj(&weakmap->dispex); + return S_OK; + + case DISPATCH_METHOD: + return throw_error(ctx, JS_E_WRONG_THIS, L"WeakMap"); + + default: + FIXME("unimplemented flags %x\n", flags); + return E_NOTIMPL; + } +} + HRESULT init_set_constructor(script_ctx_t *ctx) { jsdisp_t *constructor; @@ -649,5 +808,20 @@ HRESULT init_set_constructor(script_ctx_t *ctx) hres = jsdisp_define_data_property(ctx->global, L"Map", PROPF_WRITABLE, jsval_obj(constructor)); jsdisp_release(constructor); + if(FAILED(hres)) + return hres; + + hres = create_dispex(ctx, &WeakMap_prototype_info, ctx->object_prototype, &ctx->weakmap_prototype); + if(FAILED(hres)) + return hres; + + hres = create_builtin_constructor(ctx, WeakMap_constructor, L"WeakMap", NULL, + PROPF_CONSTR, ctx->weakmap_prototype, &constructor); + if(FAILED(hres)) + return hres; + + hres = jsdisp_define_data_property(ctx->global, L"WeakMap", PROPF_WRITABLE, + jsval_obj(constructor)); + jsdisp_release(constructor); return hres; }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Objects that have had a weakref will become "zombies" with zero refcount, until we clean them up during a garbage collector run. This is to prevent a new object from sharing the same address, since it refers to the old object in the WeakMap until it's cleaned. It also doesn't hurt performance like scanning the WeakMaps on every such object's release would, and the object is otherwise completely unlinked anyway, so it doesn't keep unnecessary refs in the meantime.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/dispex.c | 28 ++++++++++++++ dlls/jscript/error.c | 1 + dlls/jscript/jscript.c | 1 + dlls/jscript/jscript.h | 4 ++ dlls/jscript/jscript.rc | 1 + dlls/jscript/resource.h | 1 + dlls/jscript/set.c | 83 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index b5fc3699212..20865a30569 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -690,6 +690,26 @@ static void unlink_props(jsdisp_t *jsdisp) } }
+static void fixup_weak_refs(script_ctx_t *ctx) +{ + jsdisp_t *obj, *obj2; + + if(list_empty(&ctx->zombie_objects)) + return; + + LIST_FOR_EACH_ENTRY(obj, &ctx->objects, jsdisp_t, entry) + if(obj->builtin_info->fixup_weak_refs) + obj->builtin_info->fixup_weak_refs(obj); + + LIST_FOR_EACH_ENTRY_SAFE(obj, obj2, &ctx->zombie_objects, jsdisp_t, entry) { + if(obj->builtin_info->destructor) + obj->builtin_info->destructor(obj); + else + free(obj); + } + list_init(&ctx->zombie_objects); +} +
/* @@ -943,6 +963,7 @@ HRESULT gc_run(script_ctx_t *ctx) jsdisp_release(obj); }
+ fixup_weak_refs(ctx); ctx->gc_is_unlinking = FALSE; ctx->gc_last_tick = GetTickCount(); return S_OK; @@ -2244,6 +2265,13 @@ void jsdisp_free(jsdisp_t *obj) if(obj->prototype) jsdisp_release(obj->prototype);
+ /* If we possibly have weak refs to this object, keep it around as a zombie, + so that it only gets actually freed after the weak refs are removed. */ + if(obj->has_weak_refs) { + list_add_tail(&obj->ctx->zombie_objects, &obj->entry); + return; + } + if(obj->builtin_info->destructor) obj->builtin_info->destructor(obj); else diff --git a/dlls/jscript/error.c b/dlls/jscript/error.c index c3768668178..cf0ece42f15 100644 --- a/dlls/jscript/error.c +++ b/dlls/jscript/error.c @@ -483,6 +483,7 @@ jsdisp_t *create_builtin_error(script_ctx_t *ctx) case JS_E_OBJECT_NONEXTENSIBLE: case JS_E_NONCONFIGURABLE_REDEFINED: case JS_E_NONWRITABLE_MODIFIED: + case JS_E_KEY_NOT_OBJECT: case JS_E_PROP_DESC_MISMATCH: case JS_E_INVALID_WRITABLE_PROP_DESC: constr = ctx->type_error_constr; diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c index 401f6ca85b0..8896fff720c 100644 --- a/dlls/jscript/jscript.c +++ b/dlls/jscript/jscript.c @@ -748,6 +748,7 @@ static HRESULT WINAPI JScript_SetScriptSite(IActiveScript *iface, ctx->acc = jsval_undefined(); list_init(&ctx->named_items); list_init(&ctx->objects); + list_init(&ctx->zombie_objects); heap_pool_init(&ctx->tmp_heap);
hres = create_jscaller(ctx); diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index d907da3a36c..371b76cb6f6 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -172,6 +172,7 @@ typedef struct { HRESULT (*idx_get)(jsdisp_t*,unsigned,jsval_t*); HRESULT (*idx_put)(jsdisp_t*,unsigned,jsval_t); HRESULT (*gc_traverse)(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*); + void (*fixup_weak_refs)(jsdisp_t*); } builtin_info_t;
struct jsdisp_t { @@ -179,6 +180,7 @@ struct jsdisp_t {
LONG ref;
+ BOOLEAN has_weak_refs; BOOLEAN extensible; BOOLEAN gc_marked;
@@ -371,6 +373,7 @@ struct _script_ctx_t { struct _call_frame_t *call_ctx; struct list named_items; struct list objects; + struct list zombie_objects; IActiveScriptSite *site; IInternetHostSecurityManager *secmgr; DWORD safeopt; @@ -550,6 +553,7 @@ static inline HRESULT disp_call_value(script_ctx_t *ctx, IDispatch *disp, jsval_ #define JS_E_NONCONFIGURABLE_REDEFINED MAKE_JSERROR(IDS_NONCONFIGURABLE_REDEFINED) #define JS_E_NONWRITABLE_MODIFIED MAKE_JSERROR(IDS_NONWRITABLE_MODIFIED) #define JS_E_WRONG_THIS MAKE_JSERROR(IDS_WRONG_THIS) +#define JS_E_KEY_NOT_OBJECT MAKE_JSERROR(IDS_KEY_NOT_OBJECT) #define JS_E_PROP_DESC_MISMATCH MAKE_JSERROR(IDS_PROP_DESC_MISMATCH) #define JS_E_INVALID_WRITABLE_PROP_DESC MAKE_JSERROR(IDS_INVALID_WRITABLE_PROP_DESC)
diff --git a/dlls/jscript/jscript.rc b/dlls/jscript/jscript.rc index fbf965fb7e8..e5289a550a3 100644 --- a/dlls/jscript/jscript.rc +++ b/dlls/jscript/jscript.rc @@ -77,6 +77,7 @@ STRINGTABLE IDS_NONCONFIGURABLE_REDEFINED "Cannot redefine non-configurable property '|'" IDS_NONWRITABLE_MODIFIED "Cannot modify non-writable property '|'" IDS_WRONG_THIS "'this' is not a | object" + IDS_KEY_NOT_OBJECT "'key' is not an object" IDS_PROP_DESC_MISMATCH "Property cannot have both accessors and a value"
IDS_COMPILATION_ERROR "Microsoft JScript compilation error" diff --git a/dlls/jscript/resource.h b/dlls/jscript/resource.h index 0ac457d740d..ad771b67475 100644 --- a/dlls/jscript/resource.h +++ b/dlls/jscript/resource.h @@ -75,6 +75,7 @@ #define IDS_NONCONFIGURABLE_REDEFINED 0x13D6 #define IDS_NONWRITABLE_MODIFIED 0x13D7 #define IDS_WRONG_THIS 0x13FC +#define IDS_KEY_NOT_OBJECT 0x13FD /* FIXME: This is not compatible with native, but we would * conflict with IDS_UNSUPPORTED_ACTION otherwise */ #define IDS_PROP_DESC_MISMATCH 0x1F00 diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index f2de6924468..a92d3e42506 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -631,8 +631,31 @@ static int weakmap_compare(const void *k, const struct rb_entry *e) return (a > b) - (a < b); }
+static HRESULT get_weakmap_this(script_ctx_t *ctx, jsval_t vthis, WeakMapInstance **ret) +{ + jsdisp_t *jsdisp; + + if(!is_object_instance(vthis)) + return JS_E_OBJECT_EXPECTED; + if(!(jsdisp = to_jsdisp(get_object(vthis))) || !is_class(jsdisp, JSCLASS_WEAKMAP)) { + WARN("not a WeakMap object passed as 'this'\n"); + return throw_error(ctx, JS_E_WRONG_THIS, L"WeakMap"); + } + + *ret = CONTAINING_RECORD(jsdisp, WeakMapInstance, dispex); + return S_OK; +} + +static struct weakmap_entry *get_weakmap_entry(WeakMapInstance *weakmap, jsdisp_t *key) +{ + struct rb_entry *entry; + if(!(entry = rb_get(&weakmap->map, key))) return NULL; + return CONTAINING_RECORD(entry, struct weakmap_entry, entry); +} + static void release_weakmap_entry(struct weakmap_entry *entry) { + /* We can't clear the has_weak_refs flag on the key, because there might be other weak refs */ jsval_release(entry->value); free(entry); } @@ -661,8 +684,50 @@ static HRESULT WeakMap_get(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigne static HRESULT WeakMap_set(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *key = (argc >= 1 && is_object_instance(argv[0])) ? to_jsdisp(get_object(argv[0])) : NULL; + jsval_t value = argc >= 2 ? argv[1] : jsval_undefined(); + struct weakmap_entry *entry; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p (%p %s)\n", weakmap, key, debugstr_jsval(value)); + + if(!key) + return JS_E_KEY_NOT_OBJECT; + + if(key->ctx != ctx) { + FIXME("different ctx not supported\n"); + return JS_E_KEY_NOT_OBJECT; + } + + if((entry = get_weakmap_entry(weakmap, key))) { + jsval_t val; + hres = jsval_copy(value, &val); + if(FAILED(hres)) + return hres; + + jsval_release(entry->value); + entry->value = val; + }else { + if(!(entry = malloc(sizeof(*entry)))) + return E_OUTOFMEMORY; + + entry->key = key; + hres = jsval_copy(value, &entry->value); + if(FAILED(hres)) { + free(entry); + return hres; + } + key->has_weak_refs = TRUE; + rb_put(&weakmap->map, key, &entry->entry); + } + + if(r) *r = jsval_undefined(); + return S_OK; }
static HRESULT WeakMap_has(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, @@ -711,6 +776,19 @@ static HRESULT WeakMap_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op return S_OK; }
+static void WeakMap_fixup_weak_refs(jsdisp_t *dispex) +{ + WeakMapInstance *weakmap = (WeakMapInstance*)dispex; + struct weakmap_entry *entry, *entry2; + + RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry) { + if(!entry->key->ref) { + rb_remove(&weakmap->map, &entry->entry); + release_weakmap_entry(entry); + } + } +} + static const builtin_prop_t WeakMap_prototype_props[] = { {L"clear", WeakMap_clear, PROPF_METHOD}, {L"delete", WeakMap_delete, PROPF_METHOD|1}, @@ -739,6 +817,7 @@ static const builtin_info_t WeakMap_info = { NULL, NULL, WeakMap_gc_traverse, + WeakMap_fixup_weak_refs };
static HRESULT WeakMap_constructor(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/set.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index a92d3e42506..476da56800d 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -677,8 +677,23 @@ static HRESULT WeakMap_delete(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsi static HRESULT WeakMap_get(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *key = (argc >= 1 && is_object_instance(argv[0])) ? to_jsdisp(get_object(argv[0])) : NULL; + struct weakmap_entry *entry; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p (%p)\n", weakmap, key); + + if(!(entry = get_weakmap_entry(weakmap, key))) { + if(r) *r = jsval_undefined(); + return S_OK; + } + + return r ? jsval_copy(entry->value, r) : S_OK; }
static HRESULT WeakMap_set(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/set.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index 476da56800d..69a277920ff 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -670,8 +670,23 @@ static HRESULT WeakMap_clear(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsig static HRESULT WeakMap_delete(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *key = (argc >= 1 && is_object_instance(argv[0])) ? to_jsdisp(get_object(argv[0])) : NULL; + struct weakmap_entry *entry; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p (%p)\n", weakmap, key); + + if((entry = get_weakmap_entry(weakmap, key))) { + rb_remove(&weakmap->map, &entry->entry); + release_weakmap_entry(entry); + } + if(r) *r = jsval_bool(!!entry); + return S_OK; }
static HRESULT WeakMap_get(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/set.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index 69a277920ff..4066e0f24d1 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -663,8 +663,22 @@ static void release_weakmap_entry(struct weakmap_entry *entry) static HRESULT WeakMap_clear(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + struct weakmap_entry *entry, *entry2; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p\n", weakmap); + + RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry) + release_weakmap_entry(entry); + rb_destroy(&weakmap->map, NULL, NULL); + + if(r) *r = jsval_undefined(); + return S_OK; }
static HRESULT WeakMap_delete(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/set.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index 4066e0f24d1..2c7b2278b02 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -777,8 +777,20 @@ static HRESULT WeakMap_set(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigne static HRESULT WeakMap_has(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *key = (argc >= 1 && is_object_instance(argv[0])) ? to_jsdisp(get_object(argv[0])) : NULL; + struct weakmap_entry *entry; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p (%p)\n", weakmap, key); + + entry = get_weakmap_entry(weakmap, key); + if(r) *r = jsval_bool(!!entry); + return S_OK; }
static HRESULT WeakMap_value(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/tests/documentmode.js | 111 ++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 3533d721960..02eafd84cf6 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -470,6 +470,8 @@ sync_test("window_props", function() { test_exposed("requestAnimationFrame", v >= 10); test_exposed("Map", v >= 11); test_exposed("Set", v >= 11); + test_exposed("WeakMap", v >= 11); + test_exposed("WeakSet", false); test_exposed("performance", true); test_exposed("console", v >= 10); test_exposed("matchMedia", v >= 10); @@ -1608,6 +1610,115 @@ sync_test("map_obj", function() { ok(r === 1, "r = " + r); });
+sync_test("weakmap_obj", function() { + if(!("WeakMap" in window)) return; + + try { + var s = WeakMap(); + ok(false, "expected exception calling constructor as method"); + }catch(e) { + ok(e.number === 0xa13fc - 0x80000000, "calling constructor as method threw " + e.number); + } + + var s = new WeakMap, r, o, o2; + ok(Object.getPrototypeOf(s) === WeakMap.prototype, "unexpected WeakMap prototype"); + + function test_length(name, len) { + ok(WeakMap.prototype[name].length === len, "WeakMap.prototype." + name + " = " + WeakMap.prototype[name].length); + } + test_length("clear", 0); + test_length("delete", 1); + test_length("get", 1); + test_length("has", 1); + test_length("set", 2); + ok(!("entries" in s), "entries is in WeakMap"); + ok(!("forEach" in s), "forEach is in WeakMap"); + ok(!("keys" in s), "keys is in WeakMap"); + ok(!("size" in s), "size is in WeakMap"); + ok(!("values" in s), "values is in WeakMap"); + + r = Object.prototype.toString.call(s); + ok(r === "[object Object]", "toString returned " + r); + + r = s.get("test"); + ok(r === undefined, "get('test') returned " + r); + r = s.has("test"); + ok(r === false, "has('test') returned " + r); + + try { + r = s.set("test", 1); + ok(false, "set('test') did not throw"); + }catch(e) { + ok(e.number === 0xa13fd - 0x80000000, "set('test') threw " + e.number); + } + try { + r = s.set(external.testHostContext(true), 1); + ok(false, "set(host_obj) did not throw"); + }catch(e) { + ok(e.number === 0xa13fd - 0x80000000, "set(host_obj) threw " + e.number); + } + + r = s.set({}, 1); + ok(r === undefined, "set({}, 1) returned " + r); + + o = {}, o2 = {}; + r = s.get({}); + ok(r === undefined, "get({}) returned " + r); + r = s.has({}); + ok(r === false, "has({}) returned " + r); + + r = s.set(o, 2); + ok(r === undefined, "set(o, 2) returned " + r); + r = s.get(o); + ok(r === 2, "get(o) returned " + r); + r = s.has(o); + ok(r === true, "has(o) returned " + r); + r = s.get(o2); + ok(r === undefined, "get(o2) before set returned " + r); + r = s.has(o2); + ok(r === false, "has(o2) before set returned " + r); + r = s.set(o2, "test"); + ok(r === undefined, "set(o2, 'test') returned " + r); + r = s.get(o2); + ok(r === "test", "get(o2) returned " + r); + r = s.has(o2); + ok(r === true, "has(o2) returned " + r); + + r = s["delete"]("test"); /* using s.delete() would break parsing in quirks mode */ + ok(r === false, "delete('test') returned " + r); + r = s["delete"]({}); + ok(r === false, "delete({}) returned " + r); + r = s["delete"](o); + ok(r === true, "delete(o) returned " + r); + + r = s.get(o); + ok(r === undefined, "get(o) after delete returned " + r); + r = s.has(o); + ok(r === false, "has(o) after delete returned " + r); + r = s.get(o2); + ok(r === "test", "get(o2) after delete returned " + r); + r = s.has(o2); + ok(r === true, "has(o2) after delete returned " + r); + + r = s.set(o, undefined); + ok(r === undefined, "set(o, undefined) returned " + r); + r = s.get(o); + ok(r === undefined, "get(o) after re-set returned " + r); + r = s.has(o); + ok(r === true, "has(o) after re-set returned " + r); + + r = s.clear(); + ok(r === undefined, "clear() returned " + r); + r = s.get(o); + ok(r === undefined, "get(o) after clear returned " + r); + r = s.has(o); + ok(r === false, "has(o) after clear returned " + r); + r = s.get(o2); + ok(r === undefined, "get(o2) after clear returned " + r); + r = s.has(o2); + ok(r === false, "has(o2) after clear returned " + r); +}); + sync_test("storage", function() { var v = document.documentMode, i, r, list;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/error.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/jscript/error.c b/dlls/jscript/error.c index cf0ece42f15..f6eda62b88d 100644 --- a/dlls/jscript/error.c +++ b/dlls/jscript/error.c @@ -483,6 +483,7 @@ jsdisp_t *create_builtin_error(script_ctx_t *ctx) case JS_E_OBJECT_NONEXTENSIBLE: case JS_E_NONCONFIGURABLE_REDEFINED: case JS_E_NONWRITABLE_MODIFIED: + case JS_E_WRONG_THIS: case JS_E_KEY_NOT_OBJECT: case JS_E_PROP_DESC_MISMATCH: case JS_E_INVALID_WRITABLE_PROP_DESC:
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=134689
Your paranoid android.
=== w10pro64 (64 bit report) ===
mshtml: htmldoc.c:351: Test failed: expected Exec_SETTITLE
Jacek Caban (@jacek) commented about dlls/jscript/set.c:
+static HRESULT WeakMap_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) +{
- WeakMapInstance *weakmap = (WeakMapInstance*)dispex;
- struct weakmap_entry *entry, *entry2;
- HRESULT hres;
- if(op == GC_TRAVERSE_UNLINK) {
RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry)release_weakmap_entry(entry);rb_destroy(&weakmap->map, NULL, NULL);return S_OK;- }
- RB_FOR_EACH_ENTRY(entry, &weakmap->map, struct weakmap_entry, entry) {
hres = gc_process_linked_val(gc_ctx, op, dispex, &entry->value);
This is not exactly enough for weak references. Note that weak entry should also be released when key is released, which is an information that you don't feed to GC. For example: ``` var m = new WeakMap(); var k = {}; m.set(k, k); k = null; ``` In this example the entry may be collected, but unless I'm missing something, GC won't be able to do it as long as m is alive.
BTW, it would be nice to have some tests for leaks. We could have, for example, two new host object. One would be purely for ref counting, while the other one could opaque it with a predictable single reference and a way to query current ref count. ``` var m = new WeakMap(); var t = createTestRef(); // a builtin function returning a builtin object var k = { val: t.get() }; // t.get() returns a separated object, who's ref count we will be testing m.set(k, k); ok(t.ref > 1, "map entry not released"); // t.ref is t.get()'s ref count, there is a reference from k.val k = null; CollectGarbage(); ok(t.ref === 1, "map entry not released"); // there are no more JS references ```
It also doesn't hurt performance like scanning the WeakMaps on every such object's release would, and the object is otherwise completely unlinked anyway, so it doesn't keep unnecessary refs in the meantime.
You don't need to scan for every WeakMap if you store enough info. One solution would be to use a separate struct as a key: an object could allocate such struct on demand and store it. That struct could store a reference a weak reference to the original object and object destructor would just reset that reference. See Gecko's `nsWeakReference` and `nsSupportsWeakReference` for examples.
The downside of the above solution is that it still delays the actual weak map destruction to GC. We could extend it to store the whole list of entries where an object is used as a key to clean them directly from object destructor. (We'd probably want to try to avoid bloating `jsdisp_t` too much while implementing it, but it seems doable).
On Thu Jul 13 10:09:15 2023 +0000, Jacek Caban wrote:
This is not exactly enough for weak references. Note that weak entry should also be released when key is released, which is an information that you don't feed to GC. For example:
var m = new WeakMap(); var k = {}; m.set(k, k); k = null;In this example the entry may be collected, but unless I'm missing something, GC won't be able to do it as long as m is alive. BTW, it would be nice to have some tests for leaks. We could have, for example, two new host object. One would be purely for ref counting, while the other one could opaque it with a predictable single reference and a way to query current ref count.
var m = new WeakMap(); var t = createTestRef(); // a builtin function returning a builtin object var k = { val: t.get() }; // t.get() returns a separated object, who's ref count we will be testing m.set(k, k); ok(t.ref > 1, "map entry not released"); // t.ref is t.get()'s ref count, there is a reference from k.val k = null; CollectGarbage(); ok(t.ref === 1, "map entry not released"); // there are no more JS references
You're right, it won't get released until m is freed, even though the value is not technically accessible. I think this also applies if there's an object as the value that's holding a ref to the key, but that object is not accessible anywhere else (basically, cyclic ref with the WeakMap).
Not sure how "bad" this is, since it's not a permanent leak though, I'll have to weigh the alternatives, but I'll look into writing tests for it.
I'll see if I can do something about it, but I *really* would like to avoid increasing the size of a `jsdisp_t` at all, honestly, because this is just for `WeakMaps` out of all objects, and seems way overkill for such a small case (but it would impact literally every object). And even if we do need the GC for it (which I think we'll need to anyway, due to cyclic refs in the WeakMap's value holding ref to the key, etc), it's not as big a deal since it's only the jsdisp_t struct itself that had weak refs that will linger as a "zombie", not even its refs to other objects.
I think the easiest way to fix this in all cases (complicated cyclic refs) is to treat WeakMaps entries as *extensions* to the key object, rather than as part of the WeakMap itself.
So when traversing in the GC, traverse the entry values as part of the key object's traversal (like we traverse its props, prototype, etc), as if they were linked from the key object. Do **not** traverse them during the WeakMap traversal itself, because the WeakMap shouldn't hold a ref to the entry values, it's the key object who should, basically (we still add a ref when setting the entry, but the ref is held by the key object!).
After all, WeakMap entries cannot be accessed without the key, so it makes sense to "move" the ref to the key itself.
When unlinking the key, as an optimization, nothing special needs to be done, because we can remove the entry when fixing the weak refs later if the keys are gc_marked (the mark means the key was unlinked already, and so should its "extensions" from the WeakMaps, since we haven't).
What do you think?
On Fri Jul 14 12:00:19 2023 +0000, Gabriel Ivăncescu wrote:
I think the easiest way to fix this in all cases (complicated cyclic refs) is to treat WeakMaps entries as *extensions* to the key object, rather than as part of the WeakMap itself. So when traversing in the GC, traverse the entry values as part of the key object's traversal (like we traverse its props, prototype, etc), as if they were linked from the key object. Do **not** traverse them during the WeakMap traversal itself, because the WeakMap shouldn't hold a ref to the entry values, it's the key object who should, basically (we still add a ref when setting the entry, but the ref is held by the key object!). After all, WeakMap entries cannot be accessed without the key, so it makes sense to "move" the ref to the key itself. When unlinking the key, as an optimization, nothing special needs to be done, because we can remove the entry when fixing the weak refs later if the keys are gc_marked (the mark means the key was unlinked already, and so should its "extensions" from the WeakMaps, since we haven't). What do you think?
Treating them as an object extension only would not handle things like: ``` var m = new WeakMap(); m.set(key, m); m = null; ```
We could instead traverse map entry value in step 2 of GC only when it's marked twice: once from the map and once from object itself. Unless I'm missing something, this should get all cases right.
I *really* would like to avoid increasing the size of a `jsdisp_t` at all, honestly, because this is just for `WeakMaps` out of all objects, and seems way overkill for such a small case (but it would impact literally every object).
There are solutions that don't require anything in jsdisp_t (perhaps just a flag to know that we should expect weak refs). You could have, for example, a global map that would map jsdisp_t to a list of weak ref entries.
On Fri Jul 14 12:00:19 2023 +0000, Jacek Caban wrote:
Treating them as an object extension only would not handle things like:
var m = new WeakMap(); m.set(key, m); m = null;We could instead traverse map entry value in step 2 of GC only when it's marked twice: once from the map and once from object itself. Unless I'm missing something, this should get all cases right.
I thought about this some more, I think what we need to do is to have an extra pass over weak maps only after step 2, and traversing/unmarking only those weak maps that are alive *and* those keys which are also alive in it (alive means they're not marked for garbage collection).
However, we have to keep re-iterating over the weak maps everytime we actually unmarked something during one iteration, because each unmarking can potentially make some other path "alive", and we don't want to mistakenly unlink them. And stop when during the entire iteration nothing additional has been unmarked / made alive.
This should cover all cases, and btw thanks for the test I'll add it just to make sure.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)
-
Marvin