From b6d0cb4bca263f2a3641b82a3f2811146424bb35 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Tue, 29 Oct 2019 18:08:25 -0700 Subject: [PATCH 1/4] Initial native implementation of ChakraRuntime::getPropertyNames. --- vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp | 86 +++++++++++- vnext/JSI/Shared/ChakraObjectRef.cpp | 6 + vnext/JSI/Shared/ChakraObjectRef.h | 9 +- vnext/JSI/Shared/ChakraRuntime.cpp | 136 +++++++++++++++---- vnext/JSI/Shared/ChakraRuntime.h | 3 + 5 files changed, 209 insertions(+), 31 deletions(-) diff --git a/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp b/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp index b1675814198..80c8dcfeb38 100644 --- a/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp +++ b/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp @@ -1,10 +1,92 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +#include "JSI/Shared/ChakraRuntimeArgs.h" +#include "JSI/Shared/ChakraRuntimeFactory.h" +#include "MemoryTracker.h" + +#include + #include #include +using facebook::jsi::Runtime; +using facebook::react::CreateMemoryTracker; +using facebook::react::MessageQueueThread; +using Microsoft::JSI::ChakraRuntimeArgs; +using Microsoft::JSI::makeChakraRuntime; +using Microsoft::React::Test::TestMessageQueueThread; + int main(int argc, char **argv) { - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); + + system("pause"); + + ChakraRuntimeArgs args{}; + args.jsQueue = std::make_shared(); + + std::shared_ptr memoryTrackerCallbackQueue = std::make_shared(); + + args.memoryTracker = CreateMemoryTracker(std::move(memoryTrackerCallbackQueue)); + + std::unique_ptr runtime = makeChakraRuntime(std::move(args)); + + std::shared_ptr leoScript = std::make_shared( + "(function () {\n" + " function Mammal() {\n" + " this.bloodTemp = 'warm';\n" + " }\n" + " function Carnivore() {}\n" + " function Lion(name) {\n" + " Mammal.call(this);\n" + " this.name = name;\n" + " }\n" + " Mammal.prototype.growHair = function() {\n" + " console.log('my hair is growing');\n" + " }\n" + " Carnivore.prototype = Object.create(Mammal.prototype);\n" + " Carnivore.prototype.eatMeat = function() {\n" + " console.log('Mmm.Meat');\n" + " };\n" + " Lion.prototype = Object.create(Carnivore.prototype);\n" + " Lion.prototype.pride = function() {\n" + " console.log('im king of the jungle');\n" + " };\n" + " var leo = new Lion('leo');\n" + " Object.defineProperty(leo, 'son', {\n" + " enumerable: false,\n" + " value: 20\n" + " });\n" + " return leo;\n" + //" function Animal (name) {\n" + //" this.name = name\n" + //" }\n" + //" Animal.prototype.sleep = function () {\n" + //" console.log('${this.name} is sleeping.')\n" + //" }\n" + //" var leo = new Animal('Leo')\n" + //" return leo\n" + "})()"); + facebook::jsi::Object leo = runtime->evaluateJavaScript(leoScript, "").getObject(*runtime); + std::cout << leo.getPropertyNames(*runtime).size(*runtime) << std::endl; + + std::shared_ptr strScript = std::make_shared( + "(function () {\n" + " var str = new String('abc')\n" + " return str\n" + "})()"); + facebook::jsi::Object str = runtime->evaluateJavaScript(strScript, "").getObject(*runtime); + std::cout << str.getPropertyNames(*runtime).size(*runtime) << std::endl; + + std::shared_ptr arrScript = std::make_shared( + "(function () {\n" + " var arr = [1, 2]\n" + " return arr\n" + "})()"); + facebook::jsi::Object arr = runtime->evaluateJavaScript(arrScript, "").getObject(*runtime); + std::cout << arr.getPropertyNames(*runtime).size(*runtime) << std::endl; + + + + //::testing::InitGoogleTest(&argc, argv); + //return RUN_ALL_TESTS(); } diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index f60647712ae..58b38f7bc18 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -220,6 +220,12 @@ ChakraObjectRef ToJsString(const ChakraObjectRef &ref) { return ChakraObjectRef(str); } +int ToInteger(const ChakraObjectRef& jsNumber) { + int result = 0; + VerifyChakraErrorElseThrow(JsNumberToInt(jsNumber, &result)); + return result; +} + ChakraObjectRef ToJsNumber(int num) { JsValueRef result = JS_INVALID_REFERENCE; VerifyChakraErrorElseThrow(JsIntToNumber(num, &result)); diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 3ccab6bb4d5..2784f23a99d 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -148,13 +148,20 @@ ChakraObjectRef ToJsString(const std::string_view &utf8); ChakraObjectRef ToJsString(const std::wstring_view &utf16); /** - * @param jsValue A ChakraObjectRef mananing a JsValueRef. + * @param jsValue A ChakraObjectRef managing a JsValueRef. * * @returns A ChakraObjectRef managing the return value of the JS .toString * function. */ ChakraObjectRef ToJsString(const ChakraObjectRef &jsValue); +/** + * @param jsNumber A ChakraObjectRef managing a JS number. + * + * @returns The value of jsNumber converted to an integer. + */ +int ToInteger(const ChakraObjectRef &jsNumber); + /** * @returns A ChakraObjectRef managing a JS number. */ diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index d8e3b81c2bc..69be238083f 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -118,8 +118,7 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScript( if (!runtimeArgs().scriptStore) { if (!buffer) throw facebook::jsi::JSINativeException("Script buffer is empty!"); - evaluateJavaScriptSimple(*buffer, sourceURL); - return facebook::jsi::Value::undefined(); + return evaluateJavaScriptSimple(*buffer, sourceURL); } uint64_t scriptVersion = 0; @@ -140,8 +139,7 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScript( // Simple evaluate if script version can't be computed. if (scriptVersion == 0) { - evaluateJavaScriptSimple(*scriptBuffer, sourceURL); - return facebook::jsi::Value::undefined(); + return evaluateJavaScriptSimple(*scriptBuffer, sourceURL); } auto sharedScriptBuffer = std::shared_ptr(std::move(scriptBuffer)); @@ -305,9 +303,7 @@ facebook::jsi::HostFunctionType &ChakraRuntime::getHostFunction(const facebook:: facebook::jsi::Value ChakraRuntime::getProperty( const facebook::jsi::Object &obj, const facebook::jsi::PropNameID &name) { - JsValueRef result = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name), &result)); - return ToJsiValue(ChakraObjectRef(result)); + return ToJsiValue(GetProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name))); } facebook::jsi::Value ChakraRuntime::getProperty(const facebook::jsi::Object &obj, const facebook::jsi::String &name) { @@ -376,28 +372,88 @@ bool ChakraRuntime::isHostFunction(const facebook::jsi::Function &obj) const { } facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object &object) { - JsValueRef propertyNamesArrayRef; - VerifyJsErrorElseThrow(JsGetOwnPropertyNames(GetChakraObjectRef(object), &propertyNamesArrayRef)); + // Handle to the Object constructor. + ChakraObjectRef objectConstructor = GetProperty(GetChakraObjectRef(global()), "Object"); + + // Handle to the Object.prototype Object. + ChakraObjectRef objectPrototype = GetProperty(objectConstructor, "prototype"); + + // Handle to the Object.prototype.propertyIsEnumerable() Function. + ChakraObjectRef objectPrototypePropertyIsEnumerable = GetProperty(objectPrototype, "propertyIsEnumerable"); + + std::vector enumerablePropNames{}; + + ChakraObjectRef currentObject = GetChakraObjectRef(object); + while (!CompareJsValues(objectPrototype, currentObject)) { + JsValueRef propNames = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetOwnPropertyNames(currentObject, &propNames)); + ChakraObjectRef propNamesRef(propNames); + + int numPropNames = ToInteger(GetProperty(propNamesRef, "length")); - JsPropertyIdRef propertyId; - VerifyJsErrorElseThrow(JsGetPropertyIdFromName(L"length", &propertyId)); - JsValueRef countRef; - VerifyJsErrorElseThrow(JsGetProperty(propertyNamesArrayRef, propertyId, &countRef)); - int count; + for (int i = 0; i < numPropNames; ++i) { + JsValueRef propName = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetIndexedProperty(propNamesRef, ToJsNumber(i), &propName)); - VerifyJsErrorElseThrow(JsNumberToInt(countRef, &count)); + std::vector args{}; + args.emplace_back(propName); - auto result = createArray(count); - for (int i = 0; i < count; i++) { - JsValueRef index; - VerifyJsErrorElseThrow(JsIntToNumber(i, &index)); - JsValueRef propertyName; - VerifyJsErrorElseThrow(JsGetIndexedProperty(propertyNamesArrayRef, index, &propertyName)); + // TODO (yicyao): There is some duplicater code here (see call()). + // Consider refactoring. + std::vector argsWithThis = ConstructJsFunctionArguments(currentObject, args); + assert(argsWithThis.size() <= USHRT_MAX); - result.setValueAtIndex(*this, i, MakePointer(propertyName)); + JsValueRef result; + VerifyJsErrorElseThrow(JsCallFunction( + objectPrototypePropertyIsEnumerable, + argsWithThis.data(), + static_cast(argsWithThis.size()), + &result)); + + // TODO (yicyao): Evaluate the efficiency of this. + bool propIsEnumerable = ToJsiValue(ChakraObjectRef(result)).getBool(); + + if (propIsEnumerable) { + enumerablePropNames.emplace_back(propName); + } + } + + JsValueRef prototype = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetPrototype(currentObject, &prototype)); + currentObject = ChakraObjectRef(prototype); + } + + size_t numEnumerablePropNames = enumerablePropNames.size(); + facebook::jsi::Array result = createArray(numEnumerablePropNames); + + for (size_t i = 0; i < numEnumerablePropNames; ++i) { + result.setValueAtIndex(*this, i, MakePointer(enumerablePropNames[i])); } return result; + + // JsValueRef propertyNamesArrayRef; + // VerifyJsErrorElseThrow(JsGetOwnPropertyNames(GetChakraObjectRef(object), &propertyNamesArrayRef)); + + // JsPropertyIdRef propertyId; + // VerifyJsErrorElseThrow(JsGetPropertyIdFromName(L"length", &propertyId)); + // JsValueRef countRef; + // VerifyJsErrorElseThrow(JsGetProperty(propertyNamesArrayRef, propertyId, &countRef)); + // int count; + + // VerifyJsErrorElseThrow(JsNumberToInt(countRef, &count)); + + // auto result = createArray(count); + // for (int i = 0; i < count; i++) { + // JsValueRef index; + // VerifyJsErrorElseThrow(JsIntToNumber(i, &index)); + // JsValueRef propertyName; + // VerifyJsErrorElseThrow(JsGetIndexedProperty(propertyNamesArrayRef, index, &propertyName)); + + // result.setValueAtIndex(*this, i, MakePointer(propertyName)); + //} + + // return result; } // Only ChakraCore supports weak reference semantics, so ChakraRuntime @@ -425,12 +481,26 @@ facebook::jsi::Array ChakraRuntime::createArray(size_t length) { size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { assert(isArray(arr)); - return static_cast(GetProperty(arr, "length").asNumber()); + + int result = ToInteger(GetProperty(GetChakraObjectRef(arr), "length")); + + if (result < 0 || result > SIZE_MAX) { + throw facebook::jsi::JSINativeException("Invalid JS array length detected."); + } + + return static_cast(result); } size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { assert(isArrayBuffer(arrBuf)); - return static_cast(GetProperty(arrBuf, "bytelength").asNumber()); + + int result = ToInteger(GetProperty(GetChakraObjectRef(arrBuf), "bytelength")); + + if (result < 0 || result > SIZE_MAX) { + throw facebook::jsi::JSINativeException("Invalid JS array buffer bytelength detected."); + } + + return static_cast(result); } uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { @@ -566,11 +636,21 @@ bool ChakraRuntime::instanceOf(const facebook::jsi::Object &obj, const facebook: #pragma endregion Functions_inherited_from_Runtime +ChakraObjectRef ChakraRuntime::GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id) { + JsValueRef result = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetProperty(obj, id, &result)); + return ChakraObjectRef(result); +} + +ChakraObjectRef ChakraRuntime::GetProperty(const ChakraObjectRef &obj, const char *const name) { + return GetProperty(obj, GetChakraObjectRef(createPropNameIDFromAscii(name, strlen(name)))); +} + facebook::jsi::Value ChakraRuntime::GetProperty(const facebook::jsi::Object &obj, const char *const name) const { - // We have to use const_casts here because createPropNameIDFromAscii and - // getProperty are not marked as const. - facebook::jsi::PropNameID propId = const_cast(this)->createPropNameIDFromAscii(name, strlen(name)); - return const_cast(this)->getProperty(obj, propId); + // We have to use const_casts here because ToJsiValue and GetProperty cannnot + // be marked as const. + return const_cast(this)->ToJsiValue( + const_cast(this)->GetProperty(GetChakraObjectRef(obj), name)); } void ChakraRuntime::VerifyJsErrorElseThrow(JsErrorCode error) { diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 9dbdf49c029..809db70a317 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -154,6 +154,9 @@ class ChakraRuntime : public facebook::jsi::Runtime { } private: + ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id); + ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const char *const name); + // Since the function // Object::getProperty(Runtime& runtime, const char* name) // causes mulitple copies of name, we do not want to use it when implementing From 2f2b2d9874f9b72a57fdf44587b29c8f2ace5fea Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 30 Oct 2019 15:53:30 -0700 Subject: [PATCH 2/4] Check for null when traversing property chain in ChakraRuntime::getPropertyNames. --- .../JsiRuntimeUnitTests.cpp | 2 +- vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp | 86 +------------------ vnext/JSI/Shared/ChakraRuntime.cpp | 83 ++++++------------ vnext/JSI/Shared/ChakraRuntime.h | 37 ++++---- 4 files changed, 50 insertions(+), 158 deletions(-) diff --git a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp index 15cf9744dc1..372adb6e7d0 100644 --- a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp +++ b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp @@ -79,7 +79,7 @@ TEST_P(JsiRuntimeUnitTests, StringTest) { EXPECT_EQ(movedQuux.utf8(rt), "quux2"); } -TEST_P(JsiRuntimeUnitTests, DISABLED_ObjectTest) { +TEST_P(JsiRuntimeUnitTests, ObjectTest) { eval("x = {1:2, '3':4, 5:'six', 'seven':['eight', 'nine']}"); Object x = rt.global().getPropertyAsObject(rt, "x"); EXPECT_EQ(x.getPropertyNames(rt).size(rt), 4); diff --git a/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp b/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp index 80c8dcfeb38..b1675814198 100644 --- a/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp +++ b/vnext/JSI.Desktop.UnitTests/RunUnitTests.cpp @@ -1,92 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#include "JSI/Shared/ChakraRuntimeArgs.h" -#include "JSI/Shared/ChakraRuntimeFactory.h" -#include "MemoryTracker.h" - -#include - #include #include -using facebook::jsi::Runtime; -using facebook::react::CreateMemoryTracker; -using facebook::react::MessageQueueThread; -using Microsoft::JSI::ChakraRuntimeArgs; -using Microsoft::JSI::makeChakraRuntime; -using Microsoft::React::Test::TestMessageQueueThread; - int main(int argc, char **argv) { - - system("pause"); - - ChakraRuntimeArgs args{}; - args.jsQueue = std::make_shared(); - - std::shared_ptr memoryTrackerCallbackQueue = std::make_shared(); - - args.memoryTracker = CreateMemoryTracker(std::move(memoryTrackerCallbackQueue)); - - std::unique_ptr runtime = makeChakraRuntime(std::move(args)); - - std::shared_ptr leoScript = std::make_shared( - "(function () {\n" - " function Mammal() {\n" - " this.bloodTemp = 'warm';\n" - " }\n" - " function Carnivore() {}\n" - " function Lion(name) {\n" - " Mammal.call(this);\n" - " this.name = name;\n" - " }\n" - " Mammal.prototype.growHair = function() {\n" - " console.log('my hair is growing');\n" - " }\n" - " Carnivore.prototype = Object.create(Mammal.prototype);\n" - " Carnivore.prototype.eatMeat = function() {\n" - " console.log('Mmm.Meat');\n" - " };\n" - " Lion.prototype = Object.create(Carnivore.prototype);\n" - " Lion.prototype.pride = function() {\n" - " console.log('im king of the jungle');\n" - " };\n" - " var leo = new Lion('leo');\n" - " Object.defineProperty(leo, 'son', {\n" - " enumerable: false,\n" - " value: 20\n" - " });\n" - " return leo;\n" - //" function Animal (name) {\n" - //" this.name = name\n" - //" }\n" - //" Animal.prototype.sleep = function () {\n" - //" console.log('${this.name} is sleeping.')\n" - //" }\n" - //" var leo = new Animal('Leo')\n" - //" return leo\n" - "})()"); - facebook::jsi::Object leo = runtime->evaluateJavaScript(leoScript, "").getObject(*runtime); - std::cout << leo.getPropertyNames(*runtime).size(*runtime) << std::endl; - - std::shared_ptr strScript = std::make_shared( - "(function () {\n" - " var str = new String('abc')\n" - " return str\n" - "})()"); - facebook::jsi::Object str = runtime->evaluateJavaScript(strScript, "").getObject(*runtime); - std::cout << str.getPropertyNames(*runtime).size(*runtime) << std::endl; - - std::shared_ptr arrScript = std::make_shared( - "(function () {\n" - " var arr = [1, 2]\n" - " return arr\n" - "})()"); - facebook::jsi::Object arr = runtime->evaluateJavaScript(arrScript, "").getObject(*runtime); - std::cout << arr.getPropertyNames(*runtime).size(*runtime) << std::endl; - - - - //::testing::InitGoogleTest(&argc, argv); - //return RUN_ALL_TESTS(); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); } diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 69be238083f..a492a0c7cad 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -372,6 +372,9 @@ bool ChakraRuntime::isHostFunction(const facebook::jsi::Function &obj) const { } facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object &object) { + // Handle to the null JS value. + ChakraObjectRef jsNull = ToChakraObjectRef(facebook::jsi::Value::null()); + // Handle to the Object constructor. ChakraObjectRef objectConstructor = GetProperty(GetChakraObjectRef(global()), "Object"); @@ -382,35 +385,33 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object ChakraObjectRef objectPrototypePropertyIsEnumerable = GetProperty(objectPrototype, "propertyIsEnumerable"); std::vector enumerablePropNames{}; - ChakraObjectRef currentObject = GetChakraObjectRef(object); - while (!CompareJsValues(objectPrototype, currentObject)) { - JsValueRef propNames = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetOwnPropertyNames(currentObject, &propNames)); - ChakraObjectRef propNamesRef(propNames); - int numPropNames = ToInteger(GetProperty(propNamesRef, "length")); + // We have a small optimization here where we stop traversing the prototype + // chain as soon as we hit Object.prototype. However, we still need to check + // for null here, as one can create an Object with no prototype through + // Object.create(null). + while (!CompareJsValues(currentObject, objectPrototype) && !CompareJsValues(currentObject, jsNull)) { + JsValueRef propNamesRef = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetOwnPropertyNames(currentObject, &propNamesRef)); + ChakraObjectRef propNames(propNamesRef); - for (int i = 0; i < numPropNames; ++i) { - JsValueRef propName = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetIndexedProperty(propNamesRef, ToJsNumber(i), &propName)); + int numPropNames = ToInteger(GetProperty(propNames, "length")); - std::vector args{}; - args.emplace_back(propName); + for (int i = 0; i < numPropNames; ++i) { + JsValueRef propNameRef = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetIndexedProperty(propNames, ToJsNumber(i), &propNameRef)); + ChakraObjectRef propName(propNameRef); - // TODO (yicyao): There is some duplicater code here (see call()). - // Consider refactoring. - std::vector argsWithThis = ConstructJsFunctionArguments(currentObject, args); - assert(argsWithThis.size() <= USHRT_MAX); + std::vector args = {currentObject, propName}; JsValueRef result; VerifyJsErrorElseThrow(JsCallFunction( objectPrototypePropertyIsEnumerable, - argsWithThis.data(), - static_cast(argsWithThis.size()), + args.data(), + static_cast(args.size()), &result)); - // TODO (yicyao): Evaluate the efficiency of this. bool propIsEnumerable = ToJsiValue(ChakraObjectRef(result)).getBool(); if (propIsEnumerable) { @@ -431,29 +432,6 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object } return result; - - // JsValueRef propertyNamesArrayRef; - // VerifyJsErrorElseThrow(JsGetOwnPropertyNames(GetChakraObjectRef(object), &propertyNamesArrayRef)); - - // JsPropertyIdRef propertyId; - // VerifyJsErrorElseThrow(JsGetPropertyIdFromName(L"length", &propertyId)); - // JsValueRef countRef; - // VerifyJsErrorElseThrow(JsGetProperty(propertyNamesArrayRef, propertyId, &countRef)); - // int count; - - // VerifyJsErrorElseThrow(JsNumberToInt(countRef, &count)); - - // auto result = createArray(count); - // for (int i = 0; i < count; i++) { - // JsValueRef index; - // VerifyJsErrorElseThrow(JsIntToNumber(i, &index)); - // JsValueRef propertyName; - // VerifyJsErrorElseThrow(JsGetIndexedProperty(propertyNamesArrayRef, index, &propertyName)); - - // result.setValueAtIndex(*this, i, MakePointer(propertyName)); - //} - - // return result; } // Only ChakraCore supports weak reference semantics, so ChakraRuntime @@ -636,23 +614,6 @@ bool ChakraRuntime::instanceOf(const facebook::jsi::Object &obj, const facebook: #pragma endregion Functions_inherited_from_Runtime -ChakraObjectRef ChakraRuntime::GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id) { - JsValueRef result = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetProperty(obj, id, &result)); - return ChakraObjectRef(result); -} - -ChakraObjectRef ChakraRuntime::GetProperty(const ChakraObjectRef &obj, const char *const name) { - return GetProperty(obj, GetChakraObjectRef(createPropNameIDFromAscii(name, strlen(name)))); -} - -facebook::jsi::Value ChakraRuntime::GetProperty(const facebook::jsi::Object &obj, const char *const name) const { - // We have to use const_casts here because ToJsiValue and GetProperty cannnot - // be marked as const. - return const_cast(this)->ToJsiValue( - const_cast(this)->GetProperty(GetChakraObjectRef(obj), name)); -} - void ChakraRuntime::VerifyJsErrorElseThrow(JsErrorCode error) { switch (error) { case JsNoError: { @@ -777,6 +738,12 @@ std::vector ChakraRuntime::ToChakraObjectRefs(const facebook::j return result; } +ChakraObjectRef ChakraRuntime::GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id) { + JsValueRef result = JS_INVALID_REFERENCE; + VerifyJsErrorElseThrow(JsGetProperty(obj, id, &result)); + return ChakraObjectRef(result); +} + JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( JsValueRef callee, bool isConstructCall, diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 809db70a317..5d53b534796 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -154,18 +154,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { } private: - ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id); - ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const char *const name); - - // Since the function - // Object::getProperty(Runtime& runtime, const char* name) - // causes mulitple copies of name, we do not want to use it when implementing - // ChakraRuntime methods. This function does the same thing as - // Object::getProperty, but without the extra overhead. This function is - // declared as const so that it can be used when implementing - // isHostFunction and isHostObject. - facebook::jsi::Value GetProperty(const facebook::jsi::Object &obj, const char *const name) const; - void VerifyJsErrorElseThrow(JsErrorCode error); // ChakraPointerValue is needed for working with Facebook's jsi::Pointer class @@ -251,7 +239,27 @@ class ChakraRuntime : public facebook::jsi::Runtime { ChakraObjectRef ToChakraObjectRef(const facebook::jsi::Value &value); std::vector ToChakraObjectRefs(const facebook::jsi::Value *value, size_t count); - // Host function and host object helpers + // Convenience functions for property access. + ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const ChakraObjectRef &id); + inline ChakraObjectRef GetProperty(const ChakraObjectRef &obj, const char *const name) { + return GetProperty(obj, GetChakraObjectRef(createPropNameIDFromAscii(name, strlen(name)))); + } + + // Since the function + // Object::getProperty(Runtime& runtime, const char* name) + // causes mulitple copies of name, we do not want to use it when implementing + // ChakraRuntime methods. This function does the same thing as + // Object::getProperty, but without the extra overhead. This function is + // declared as const so that it can be used when implementing + // isHostFunction and isHostObject. + inline facebook::jsi::Value GetProperty(const facebook::jsi::Object &obj, const char *const name) const { + // We have to use const_casts here because ToJsiValue and GetProperty cannnot + // be marked as const. + return const_cast(this)->ToJsiValue( + const_cast(this)->GetProperty(GetChakraObjectRef(obj), name)); + } + + // Host function helper static JsValueRef CALLBACK HostFunctionCall( JsValueRef callee, bool isConstructCall, @@ -259,7 +267,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { unsigned short argumentCountIncThis, void *callbackState); - // For the following functions, runtime must be referring to a ChakraRuntime. + // Host object helpers; runtime must be referring to a ChakraRuntime. static facebook::jsi::Value HostObjectGetTrap( Runtime &runtime, const facebook::jsi::Value & /*thisVal*/, @@ -275,7 +283,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { const facebook::jsi::Value & /*thisVal*/, const facebook::jsi::Value *args, size_t count); - facebook::jsi::Object createHostObjectProxyHandler() noexcept; // Promise Helpers From 7c96aa9beae65248b1b4c4c668b08ea56e4b4392 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 30 Oct 2019 16:44:05 -0700 Subject: [PATCH 3/4] Yarn format. --- vnext/JSI/Shared/ChakraObjectRef.cpp | 2 +- vnext/JSI/Shared/ChakraRuntime.cpp | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index 58b38f7bc18..ebbb0643e56 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -220,7 +220,7 @@ ChakraObjectRef ToJsString(const ChakraObjectRef &ref) { return ChakraObjectRef(str); } -int ToInteger(const ChakraObjectRef& jsNumber) { +int ToInteger(const ChakraObjectRef &jsNumber) { int result = 0; VerifyChakraErrorElseThrow(JsNumberToInt(jsNumber, &result)); return result; diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index a492a0c7cad..04eaac21dc1 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -407,10 +407,7 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object JsValueRef result; VerifyJsErrorElseThrow(JsCallFunction( - objectPrototypePropertyIsEnumerable, - args.data(), - static_cast(args.size()), - &result)); + objectPrototypePropertyIsEnumerable, args.data(), static_cast(args.size()), &result)); bool propIsEnumerable = ToJsiValue(ChakraObjectRef(result)).getBool(); From d0f10f5de7a65088b98b6418eda0a5ef38031886 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 30 Oct 2019 16:44:35 -0700 Subject: [PATCH 4/4] Change files --- ...-windows-2019-10-30-16-44-34-FixGetPropertyNames.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 change/react-native-windows-2019-10-30-16-44-34-FixGetPropertyNames.json diff --git a/change/react-native-windows-2019-10-30-16-44-34-FixGetPropertyNames.json b/change/react-native-windows-2019-10-30-16-44-34-FixGetPropertyNames.json new file mode 100644 index 00000000000..5c41b07616e --- /dev/null +++ b/change/react-native-windows-2019-10-30-16-44-34-FixGetPropertyNames.json @@ -0,0 +1,9 @@ +{ + "type": "none", + "comment": "Implement ChakraRuntime::getPropertyNames() properly in native.", + "packageName": "react-native-windows", + "email": "yicyao@microsoft.com", + "commit": "7c96aa9beae65248b1b4c4c668b08ea56e4b4392", + "date": "2019-10-30T23:44:34.909Z", + "file": "D:\\Git\\hansenyy-react-native-windows-3\\change\\react-native-windows-2019-10-30-16-44-34-FixGetPropertyNames.json" +} \ No newline at end of file