From e04616ca17cd767672c9b637201983603fa35d4c Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Thu, 26 Apr 2018 20:05:27 +0100 Subject: [PATCH] chakrashim: implement proxy methods in chakrashim Implement value::IsProxy(), Proxy::GetTarget() and Proxy::GetHandler() in chakrashim. This enables util.types.isProxy() and util.inspect() with the showProxy flag as true to function correctly in node-chakracore. Also enable related tests for node-chakracore: * parallel/test-util-types.js * parallel/test-util-inspect.js Fixes: https://github.com/nodejs/node-chakracore/issues/488 Fixes: https://github.com/nodejs/node-chakracore/issues/523 PR-URL: https://github.com/nodejs/node-chakracore/pull/525 Reviewed-By: Kyle Farnung Reviewed-By: Jack Horton --- deps/chakrashim/lib/chakra_shim.js | 5 ----- deps/chakrashim/src/v8proxy.cc | 20 ++++++++++++++------ deps/chakrashim/src/v8value.cc | 7 ++++++- test/parallel/parallel.status | 2 -- test/parallel/test-util-types.js | 21 +++++++++++---------- 5 files changed, 31 insertions(+), 24 deletions(-) diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js index 85ca9c70b23..fff078d4c75 100644 --- a/deps/chakrashim/lib/chakra_shim.js +++ b/deps/chakrashim/lib/chakra_shim.js @@ -636,11 +636,6 @@ return microTasks.shift(); }; - utils.isProxy = function(value) { - // CHAKRA-TODO: Need to add JSRT API to detect this - return false; - }; - utils.getPropertyAttributes = function(object, value) { const descriptor = Object_getOwnPropertyDescriptor(object, value); if (descriptor === undefined) { diff --git a/deps/chakrashim/src/v8proxy.cc b/deps/chakrashim/src/v8proxy.cc index 83aa061dab0..b19ebb06842 100644 --- a/deps/chakrashim/src/v8proxy.cc +++ b/deps/chakrashim/src/v8proxy.cc @@ -22,15 +22,23 @@ namespace v8 { Local Proxy::GetTarget() { - // CHAKRA-TODO: Need to add JSRT API to get target of proxy - JsValueRef undefinedValue = jsrt::GetUndefined(); - return Local::New(undefinedValue); + JsValueRef returnValue = JS_INVALID_REFERENCE; + bool isProxy = false; + JsGetProxyProperties((JsValueRef) this, &isProxy, &returnValue, nullptr); + if (!isProxy || returnValue == JS_INVALID_REFERENCE) { + returnValue = jsrt::GetUndefined(); + } + return Local::New(returnValue); } Local Proxy::GetHandler() { - // CHAKRA-TODO: Need to add JSRT API to get handler of proxy - JsValueRef undefinedValue = jsrt::GetUndefined(); - return Local::New(undefinedValue); + JsValueRef returnValue = JS_INVALID_REFERENCE; + bool isProxy = false; + JsGetProxyProperties((JsValueRef) this, &isProxy, nullptr, &returnValue); + if (!isProxy || returnValue == JS_INVALID_REFERENCE) { + returnValue = jsrt::GetUndefined(); + } + return Local::New(returnValue); } Proxy *Proxy::Cast(v8::Value *obj) { diff --git a/deps/chakrashim/src/v8value.cc b/deps/chakrashim/src/v8value.cc index f48b3c6effc..0422812c377 100644 --- a/deps/chakrashim/src/v8value.cc +++ b/deps/chakrashim/src/v8value.cc @@ -145,6 +145,12 @@ bool Value::IsUint32() const { return trunc(value) == value; } +bool Value::IsProxy() const { + bool isProxy = false; + JsGetProxyProperties((JsValueRef) this, &isProxy, nullptr, nullptr); + return isProxy; +} + #define IS_TYPE_FUNCTION(v8ValueFunc, chakrashimFunc) \ bool Value::v8ValueFunc() const { \ JsValueRef resultRef = JS_INVALID_REFERENCE; \ @@ -163,7 +169,6 @@ IS_TYPE_FUNCTION(IsDate, isDate) IS_TYPE_FUNCTION(IsMap, isMap) IS_TYPE_FUNCTION(IsNativeError, isNativeError) IS_TYPE_FUNCTION(IsPromise, isPromise) -IS_TYPE_FUNCTION(IsProxy, isProxy) IS_TYPE_FUNCTION(IsRegExp, isRegExp) IS_TYPE_FUNCTION(IsAsyncFunction, isAsyncFunction) IS_TYPE_FUNCTION(IsSet, isSet) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 766da600fff..1850155beed 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -86,7 +86,6 @@ test-trace-events-v8 : SKIP test-url-domain-ascii-unicode : SKIP test-util : SKIP test-util-format-shared-arraybuffer : SKIP -test-util-inspect-proxy : SKIP test-v8-serdes : SKIP test-vm-attributes-property-not-on-sandbox : SKIP test-vm-cached-data : SKIP @@ -156,7 +155,6 @@ test-vm-module-errors : SKIP test-vm-module-import-meta : SKIP test-vm-module-link : SKIP test-vm-module-reevaluate : SKIP -test-util-types : SKIP # This test requires support for bigints test-util-inspect-bigint : SKIP diff --git a/test/parallel/test-util-types.js b/test/parallel/test-util-types.js index 8b8278efceb..5f03722f3e3 100644 --- a/test/parallel/test-util-types.js +++ b/test/parallel/test-util-types.js @@ -51,9 +51,7 @@ for (const [ value, _method ] of [ { value: 'foo' }) ], [ new DataView(new ArrayBuffer()) ], [ new SharedArrayBuffer() ], - // Node-ChakraCore does not support util.types.isProxy() due to - // Microsoft/ChakraCore#950 and node/node-chakracore#488 - !common.isChakraEngine ? [ new Proxy({}, {}), 'isProxy' ] : [ new Date() ], + [ new Proxy({}, {}), 'isProxy' ], [ new WebAssembly.Module(wasmBuffer), 'isWebAssemblyCompiledModule' ], ]) { const method = _method || `is${value.constructor.name}`; @@ -130,10 +128,13 @@ for (const [ value, _method ] of [ } } -(async () => { - const m = new vm.Module(''); - await m.link(() => 0); - m.instantiate(); - await m.evaluate(); - assert.ok(types.isModuleNamespaceObject(m.namespace)); -})(); +// Node-ChakraCore does not support esmodules +if (!common.isChakraEngine) { + (async () => { + const m = new vm.Module(''); + await m.link(() => 0); + m.instantiate(); + await m.evaluate(); + assert.ok(types.isModuleNamespaceObject(m.namespace)); + })(); +}