[PATCH v2 0/3] MR1067: jscript: Bind function 'thisArg' and JSON.parse improvements.
-- v2: jscript: Implement `reviver` argument for JSON.parse. jscript/tests: Fix copy paste mistake in JSON test. jscript: Implement Function.prototype.bind's `thisArg` properly. https://gitlab.winehq.org/wine/wine/-/merge_requests/1067
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/function.c | 40 +++++++++++++++++++++------------------- dlls/mshtml/tests/es5.js | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index bf152e22868..8009f3a5d8b 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -57,7 +57,7 @@ typedef struct { typedef struct { FunctionInstance function; FunctionInstance *target; - IDispatch *this; + jsval_t this; unsigned argc; jsval_t args[1]; } BindFunction; @@ -70,7 +70,7 @@ typedef struct { unsigned argc; } ArgumentsInstance; -static HRESULT create_bind_function(script_ctx_t*,FunctionInstance*,IDispatch*,unsigned,jsval_t*,jsdisp_t**r); +static HRESULT create_bind_function(script_ctx_t*,FunctionInstance*,jsval_t,unsigned,jsval_t*,jsdisp_t**r); static inline FunctionInstance *function_from_jsdisp(jsdisp_t *jsdisp) { @@ -448,7 +448,7 @@ static HRESULT Function_call(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsig static HRESULT Function_bind(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - IDispatch *bound_this = NULL; + jsval_t bound_this = jsval_undefined(); FunctionInstance *function; jsdisp_t *new_function; HRESULT hres; @@ -459,18 +459,19 @@ static HRESULT Function_bind(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsig return JS_E_FUNCTION_EXPECTED; if(argc < 1) { - FIXME("no this argument\n"); - return E_NOTIMPL; - } - - if(is_object_instance(argv[0])) { - bound_this = get_object(argv[0]); - }else if(!is_null(argv[0])) { - FIXME("%s is not an object instance\n", debugstr_jsval(argv[0])); - return E_NOTIMPL; + argc = 1; + }else if(is_null(argv[0])) { + bound_this = argv[0]; + }else if(!is_undefined(argv[0])) { + IDispatch *obj; + hres = to_object(ctx, argv[0], &obj); + if(FAILED(hres)) + return hres; + bound_this = jsval_disp(obj); } hres = create_bind_function(ctx, function, bound_this, argc - 1, argv + 1, &new_function); + jsval_release(bound_this); if(FAILED(hres)) return hres; @@ -849,8 +850,7 @@ static HRESULT BindFunction_call(script_ctx_t *ctx, FunctionInstance *func, jsva memcpy(call_args + function->argc, argv, argc * sizeof(*call_args)); } - hres = function->target->vtbl->call(ctx, function->target, function->this ? jsval_disp(function->this) : jsval_null(), - flags, call_argc, call_args, r); + hres = function->target->vtbl->call(ctx, function->target, function->this, flags, call_argc, call_args, r); heap_free(call_args); return hres; @@ -877,8 +877,7 @@ static void BindFunction_destructor(FunctionInstance *func) for(i = 0; i < function->argc; i++) jsval_release(function->args[i]); jsdisp_release(&function->target->dispex); - if(function->this) - IDispatch_Release(function->this); + jsval_release(function->this); } static const function_vtbl_t BindFunctionVtbl = { @@ -888,7 +887,7 @@ static const function_vtbl_t BindFunctionVtbl = { BindFunction_destructor }; -static HRESULT create_bind_function(script_ctx_t *ctx, FunctionInstance *target, IDispatch *bound_this, unsigned argc, +static HRESULT create_bind_function(script_ctx_t *ctx, FunctionInstance *target, jsval_t bound_this, unsigned argc, jsval_t *argv, jsdisp_t **ret) { BindFunction *function; @@ -902,8 +901,11 @@ static HRESULT create_bind_function(script_ctx_t *ctx, FunctionInstance *target, jsdisp_addref(&target->dispex); function->target = target; - if(bound_this) - IDispatch_AddRef(function->this = bound_this); + hres = jsval_copy(bound_this, &function->this); + if(FAILED(hres)) { + jsdisp_release(&function->function.dispex); + return hres; + } for(function->argc = 0; function->argc < argc; function->argc++) { hres = jsval_copy(argv[function->argc], function->args + function->argc); diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js index bfaacd1bd5b..e22f610e197 100644 --- a/dlls/mshtml/tests/es5.js +++ b/dlls/mshtml/tests/es5.js @@ -1188,6 +1188,23 @@ sync_test("bind", function() { ok(t != a, "t == a"); ok(Function.prototype.bind.length === 1, "Function.prototype.bind.length = " + Function.prototype.bind.length); + + ((function() { ok(this === window, "bind() this = " + this); }).bind())(); + ((function() { ok(this === window, "bind(undefined) = " + this); }).bind(undefined))(); + ((function() { ok(this === window, "bind(nullDisp) = " + this); }).bind(external.nullDisp))(); + ((function() { + ok(typeof(this) === "object", "bind(42) typeof(this) = " + typeof(this)); + ok(this.valueOf() === 42, "bind(42) this = " + this); + }).bind(42))(); + + r = (Object.prototype.toString.bind())(); + ok(r === "[object Undefined]", "toString.bind() returned " + r); + r = (Object.prototype.toString.bind(undefined))(); + ok(r === "[object Undefined]", "toString.bind(undefined) returned " + r); + r = (Object.prototype.toString.bind(null))(); + ok(r === "[object Null]", "toString.bind(null) returned " + r); + r = (Object.prototype.toString.bind(external.nullDisp))(); + ok(r === "[object Null]", "toString.bind(nullDisp) returned " + r); }); sync_test("keys", function() { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1067
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/tests/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/jscript/tests/api.js b/dlls/jscript/tests/api.js index 123665cfdde..e47e2759abb 100644 --- a/dlls/jscript/tests/api.js +++ b/dlls/jscript/tests/api.js @@ -2025,7 +2025,7 @@ ok(isNaN(tmp), "Math.tan(-Infinity) is not NaN"); for(var prop in x) { if(!x.hasOwnProperty(prop)) continue; - if(!x.hasOwnProperty(prop)) + if(!y.hasOwnProperty(prop)) return false; if(!json_cmp(x[prop], y[prop])) return false; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1067
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- We store the hres in the ctx to minimize the amount of parameters to the recursive function, since it propagates up the call stack anyway. --- dlls/jscript/array.c | 2 +- dlls/jscript/jscript.h | 1 + dlls/jscript/json.c | 120 ++++++++++++++++++++++++++++++++++++-- dlls/jscript/tests/api.js | 57 +++++++++++++++++- 4 files changed, 173 insertions(+), 7 deletions(-) diff --git a/dlls/jscript/array.c b/dlls/jscript/array.c index 5a8131f092c..d69c9f7edf4 100644 --- a/dlls/jscript/array.c +++ b/dlls/jscript/array.c @@ -93,7 +93,7 @@ static HRESULT set_length(jsdisp_t *obj, DWORD length) return jsdisp_propput_name(obj, L"length", jsval_number(length)); } -static WCHAR *idx_to_str(DWORD idx, WCHAR *ptr) +WCHAR *idx_to_str(DWORD idx, WCHAR *ptr) { if(!idx) { *ptr = '0'; diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 0f8baea0188..757abf8aa6d 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -318,6 +318,7 @@ HRESULT variant_date_to_string(script_ctx_t*,double,jsstr_t**) DECLSPEC_HIDDEN; HRESULT decode_source(WCHAR*) DECLSPEC_HIDDEN; HRESULT double_to_string(double,jsstr_t**) DECLSPEC_HIDDEN; +WCHAR *idx_to_str(DWORD,WCHAR*) DECLSPEC_HIDDEN; static inline BOOL is_digit(WCHAR c) { diff --git a/dlls/jscript/json.c b/dlls/jscript/json.c index dfcc39daa2a..d354768b486 100644 --- a/dlls/jscript/json.c +++ b/dlls/jscript/json.c @@ -267,20 +267,104 @@ static HRESULT parse_json_value(json_parse_ctx_t *ctx, jsval_t *r) return E_FAIL; } +struct transform_json_object_ctx +{ + script_ctx_t *ctx; + IDispatch *reviver; + HRESULT hres; +}; + +static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, jsdisp_t *holder, jsstr_t *name) +{ + jsval_t res, args[2]; + const WCHAR *str; + + if(!(str = jsstr_flatten(name))) + proc_ctx->hres = E_OUTOFMEMORY; + else + proc_ctx->hres = jsdisp_propget_name(holder, str, &args[1]); + if(FAILED(proc_ctx->hres)) + return jsval_undefined(); + + if(is_object_instance(args[1])) { + jsdisp_t *obj = to_jsdisp(get_object(args[1])); + jsstr_t *jsstr; + DISPID id; + BOOL b; + + if(!obj) { + FIXME("non-JS obj in JSON object: %p\n", get_object(args[1])); + proc_ctx->hres = E_NOTIMPL; + return jsval_undefined(); + }else if(is_class(obj, JSCLASS_ARRAY)) { + unsigned i, length = array_get_length(obj); + WCHAR buf[14], *buf_end; + + buf_end = buf + ARRAY_SIZE(buf) - 1; + *buf_end-- = 0; + for(i = 0; i < length; i++) { + str = idx_to_str(i, buf_end); + if(!(jsstr = jsstr_alloc(str))) { + proc_ctx->hres = E_OUTOFMEMORY; + return jsval_undefined(); + } + res = transform_json_object(proc_ctx, obj, jsstr); + jsstr_release(jsstr); + if(is_undefined(res)) { + if(FAILED(proc_ctx->hres)) + return jsval_undefined(); + if(FAILED(jsdisp_get_id(obj, str, 0, &id))) + continue; + proc_ctx->hres = disp_delete((IDispatch*)&obj->IDispatchEx_iface, id, &b); + }else { + proc_ctx->hres = jsdisp_define_data_property(obj, str, PROPF_WRITABLE | PROPF_ENUMERABLE | PROPF_CONFIGURABLE, res); + jsval_release(res); + } + if(FAILED(proc_ctx->hres)) + return jsval_undefined(); + } + }else { + id = DISPID_STARTENUM; + for(;;) { + proc_ctx->hres = jsdisp_next_prop(obj, id, JSDISP_ENUM_OWN_ENUMERABLE, &id); + if(proc_ctx->hres == S_FALSE) + break; + if(FAILED(proc_ctx->hres) || FAILED(proc_ctx->hres = jsdisp_get_prop_name(obj, id, &jsstr))) + return jsval_undefined(); + res = transform_json_object(proc_ctx, obj, jsstr); + if(is_undefined(res)) { + if(SUCCEEDED(proc_ctx->hres)) + proc_ctx->hres = disp_delete((IDispatch*)&obj->IDispatchEx_iface, id, &b); + }else { + if(!(str = jsstr_flatten(jsstr))) + proc_ctx->hres = E_OUTOFMEMORY; + else + proc_ctx->hres = jsdisp_define_data_property(obj, str, PROPF_WRITABLE | PROPF_ENUMERABLE | PROPF_CONFIGURABLE, res); + jsval_release(res); + } + jsstr_release(jsstr); + if(FAILED(proc_ctx->hres)) + return jsval_undefined(); + } + } + } + + args[0] = jsval_string(name); + proc_ctx->hres = disp_call_value(proc_ctx->ctx, proc_ctx->reviver, (IDispatch*)&holder->IDispatchEx_iface, + DISPATCH_METHOD, ARRAY_SIZE(args), args, &res); + return FAILED(proc_ctx->hres) ? jsval_undefined() : res; +} + /* ECMA-262 5.1 Edition 15.12.2 */ static HRESULT JSON_parse(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { json_parse_ctx_t parse_ctx; const WCHAR *buf; + jsdisp_t *root; jsstr_t *str; jsval_t ret; HRESULT hres; - if(argc != 1) { - FIXME("Unsupported args\n"); - return E_INVALIDARG; - } - hres = to_flat_string(ctx, argv[0], &str, &buf); if(FAILED(hres)) return hres; @@ -293,12 +377,38 @@ static HRESULT JSON_parse(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned hres = parse_json_value(&parse_ctx, &ret); if(SUCCEEDED(hres) && skip_spaces(&parse_ctx)) { FIXME("syntax error\n"); + jsval_release(ret); hres = E_FAIL; } jsstr_release(str); if(FAILED(hres)) return hres; + /* FIXME: check IsCallable */ + if(argc > 1 && is_object_instance(argv[1])) { + hres = create_object(ctx, NULL, &root); + if(FAILED(hres)) { + jsval_release(ret); + return hres; + } + hres = jsdisp_define_data_property(root, L"", PROPF_WRITABLE | PROPF_ENUMERABLE | PROPF_CONFIGURABLE, ret); + jsval_release(ret); + + if(SUCCEEDED(hres)) { + struct transform_json_object_ctx proc_ctx = { ctx, get_object(argv[1]), S_OK }; + if(!(str = jsstr_alloc(L""))) + hres = E_OUTOFMEMORY; + else { + ret = transform_json_object(&proc_ctx, root, str); + jsstr_release(str); + hres = proc_ctx.hres; + } + } + jsdisp_release(root); + if(FAILED(hres)) + return hres; + } + if(r) *r = ret; else diff --git a/dlls/jscript/tests/api.js b/dlls/jscript/tests/api.js index e47e2759abb..c33791f1fb1 100644 --- a/dlls/jscript/tests/api.js +++ b/dlls/jscript/tests/api.js @@ -1960,7 +1960,7 @@ ok(isNaN(tmp), "Math.tan(-Infinity) is not NaN"); [[[,2,undefined,3,{prop:0},],undefined," "],"[\n null,\n 2,\n null,\n 3,\n {\n \"prop\": 0\n },\n null\n]"] ]; - var i, s, v; + var i, s, v, t; for(i=0; i < stringify_tests.length; i++) { s = JSON.stringify.apply(null, stringify_tests[i][0]); @@ -2043,6 +2043,61 @@ ok(isNaN(tmp), "Math.tan(-Infinity) is not NaN"); v = JSON.parse(parse_tests[i][0]); ok(json_cmp(v, parse_tests[i][1]), "parse[" + i + "] returned " + v + ", expected " + parse_tests[i][1]); } + + v = [ [-1, "b"], {"length": -2, "0": -4, "1": -5}, [{}], [{"x": [null]}] ]; + s = + '{' + + '"foo": true,' + + '"bar": [],' + + '"baz": "remove_me",' + + '"obj": {' + + ' "arr": [ [1, "b"], {"length": 2, "0": 4, "1": 5}, [{}], [{"x": [null]}] ],' + + ' "": "empty"' + + '},' + + '"last": false' + + '}'; + o = JSON.parse(s), t = JSON.parse(s), i = new Object(); + i[""] = t; + delete t.baz; /* baz gets removed */ + t.obj.arr = v; /* has negative values */ + + var walk_expect = [ + [ o, "foo", true ], + [ o, "bar", [] ], + [ o, "baz", "remove_me" ], + [ [1, "b"], "0", 1 ], + [ [-1, "b"], "1", "b" ], + [ [ [-1, "b"], {"length": 2, "0": 4, "1": 5}, [{}], [{"x": [null]}] ], "0", [-1, "b"] ], + [ {"length": 2, "0": 4, "1": 5}, "length", 2 ], + [ {"length": -2, "0": 4, "1": 5}, "0", 4 ], + [ {"length": -2, "0": -4, "1": 5}, "1", 5 ], + [ v, "1", {"length": -2, "0": -4, "1": -5} ], + [ [{}], "0", {} ], + [ v, "2", [{}] ], + [ [null], "0", null ], + [ {"x": [null]}, "x", [null] ], + [ [{"x": [null]}], "0", {"x": [null]} ], + [ v, "3", [{"x": [null]}] ], + [ { "arr": v, "": "empty" }, "arr", v ], + [ { "arr": v, "": "empty" }, "", "empty" ], + [ t, "obj", { "arr": v, "": "empty" } ], + [ t, "last", false ], + [ i, "", t ] + ]; + i = 0; + v = JSON.parse(s, function(prop, value) { + var a = [this, prop, value]; + ok(json_cmp(a, walk_expect[i]), "[walk step " + i + "] got [" + a + "], expected [" + walk_expect[i] + "]"); + i++; + return (typeof value === 'number') ? -value : (value === "remove_me" ? undefined : value); + }); + ok(i === walk_expect.length, "parse with reviver walked " + i + " steps, expected " + walk_expect.length); + ok(json_cmp(v, t), "parse with reviver returned wrong object"); + + v = JSON.parse('true', function(prop, value) { return prop === "" ? undefined : value; }); + ok(v === undefined, "parse with reviver removing last prop returned " + v); + v = JSON.parse('true', function(prop, value) { return prop === "" ? false : value; }); + ok(v === false, "parse with reviver setting last prop to false returned " + v); })(); var func = function (a) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1067
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=125000 Your paranoid android. === w10pro64_zh_CN (64 bit report) === mshtml: events.c:3530: Test failed: ReportResult failed: 80ef0001 === debian11 (32 bit report) === user32: input.c:2864: Test marked flaky: 15: expected WM_INPUT message input.c:2867: Test marked flaky: 15: expected RIM_INPUT message === debian11 (build log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24899. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24899. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24899.
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1067
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin