From 09b426a5bbf575ad23a721c50d0b374c84364b72 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Thu, 31 Oct 2019 14:47:07 -0700 Subject: [PATCH 1/3] Fix some nit suggestions from PR #3568. --- vnext/JSI/Shared/ChakraRuntime.cpp | 44 ++++++++++++++++-------------- vnext/JSI/Shared/ChakraRuntime.h | 1 + 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 04eaac21dc1..b580f7f233f 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -10,8 +10,8 @@ #include #include -#include #include +#include #include #include #include @@ -384,26 +384,29 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object // Handle to the Object.prototype.propertyIsEnumerable() Function. ChakraObjectRef objectPrototypePropertyIsEnumerable = GetProperty(objectPrototype, "propertyIsEnumerable"); + // We now traverse the object's property chain and collect all enumerable + // property names. std::vector enumerablePropNames{}; - ChakraObjectRef currentObject = GetChakraObjectRef(object); + ChakraObjectRef currentObjectOnPrototypeChain = GetChakraObjectRef(object); // 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)) { + while (!CompareJsValues(currentObjectOnPrototypeChain, objectPrototype) && + !CompareJsValues(currentObjectOnPrototypeChain, jsNull)) { JsValueRef propNamesRef = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetOwnPropertyNames(currentObject, &propNamesRef)); + VerifyJsErrorElseThrow(JsGetOwnPropertyNames(currentObjectOnPrototypeChain, &propNamesRef)); ChakraObjectRef propNames(propNamesRef); - int numPropNames = ToInteger(GetProperty(propNames, "length")); + int propNamesSize = ToInteger(GetProperty(propNames, "length")); - for (int i = 0; i < numPropNames; ++i) { + for (int i = 0; i < propNamesSize; ++i) { JsValueRef propNameRef = JS_INVALID_REFERENCE; VerifyJsErrorElseThrow(JsGetIndexedProperty(propNames, ToJsNumber(i), &propNameRef)); ChakraObjectRef propName(propNameRef); - std::vector args = {currentObject, propName}; + std::vector args = {currentObjectOnPrototypeChain, propName}; JsValueRef result; VerifyJsErrorElseThrow(JsCallFunction( @@ -417,14 +420,14 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames(const facebook::jsi::Object } JsValueRef prototype = JS_INVALID_REFERENCE; - VerifyJsErrorElseThrow(JsGetPrototype(currentObject, &prototype)); - currentObject = ChakraObjectRef(prototype); + VerifyJsErrorElseThrow(JsGetPrototype(currentObjectOnPrototypeChain, &prototype)); + currentObjectOnPrototypeChain = ChakraObjectRef(prototype); } - size_t numEnumerablePropNames = enumerablePropNames.size(); - facebook::jsi::Array result = createArray(numEnumerablePropNames); + size_t enumerablePropNamesSize = enumerablePropNames.size(); + facebook::jsi::Array result = createArray(enumerablePropNamesSize); - for (size_t i = 0; i < numEnumerablePropNames; ++i) { + for (size_t i = 0; i < enumerablePropNamesSize; ++i) { result.setValueAtIndex(*this, i, MakePointer(enumerablePropNames[i])); } @@ -446,11 +449,10 @@ facebook::jsi::Value ChakraRuntime::lockWeakObject(const facebook::jsi::WeakObje } facebook::jsi::Array ChakraRuntime::createArray(size_t length) { - assert(length <= UINT_MAX); - JsValueRef result = JS_INVALID_REFERENCE; + assert(length <= (std::numeric_limits::max)()); + JsValueRef result = JS_INVALID_REFERENCE; VerifyJsErrorElseThrow(JsCreateArray(static_cast(length), &result)); - return MakePointer(result).asArray(*this); } @@ -459,7 +461,7 @@ size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { int result = ToInteger(GetProperty(GetChakraObjectRef(arr), "length")); - if (result < 0 || result > SIZE_MAX) { + if (result < 0 || result > (std::numeric_limits::max)()) { throw facebook::jsi::JSINativeException("Invalid JS array length detected."); } @@ -471,7 +473,7 @@ size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { int result = ToInteger(GetProperty(GetChakraObjectRef(arrBuf), "bytelength")); - if (result < 0 || result > SIZE_MAX) { + if (result < 0 || result > (std::numeric_limits::max)()) { throw facebook::jsi::JSINativeException("Invalid JS array buffer bytelength detected."); } @@ -491,7 +493,7 @@ uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { facebook::jsi::Value ChakraRuntime::getValueAtIndex(const facebook::jsi::Array &arr, size_t index) { assert(isArray(arr)); - assert(index <= INT_MAX); + assert(index <= (std::numeric_limits::max)()); JsValueRef result = JS_INVALID_REFERENCE; VerifyJsErrorElseThrow(JsGetIndexedProperty(GetChakraObjectRef(arr), ToJsNumber(static_cast(index)), &result)); @@ -500,7 +502,7 @@ facebook::jsi::Value ChakraRuntime::getValueAtIndex(const facebook::jsi::Array & void ChakraRuntime::setValueAtIndexImpl(facebook::jsi::Array &arr, size_t index, const facebook::jsi::Value &value) { assert(isArray(arr)); - assert(index <= INT_MAX); + assert(index <= (std::numeric_limits::max)()); VerifyJsErrorElseThrow( JsSetIndexedProperty(GetChakraObjectRef(arr), ToJsNumber(static_cast(index)), ToChakraObjectRef(value))); @@ -558,7 +560,7 @@ facebook::jsi::Value ChakraRuntime::call( std::vector argRefs = ToChakraObjectRefs(args, count); std::vector argsWithThis = ConstructJsFunctionArguments(thisRef, argRefs); - assert(argsWithThis.size() <= USHRT_MAX); + assert(argsWithThis.size() <= (std::numeric_limits::max)()); JsValueRef result; VerifyJsErrorElseThrow(JsCallFunction( @@ -574,7 +576,7 @@ ChakraRuntime::callAsConstructor(const facebook::jsi::Function &func, const face std::vector argRefs = ToChakraObjectRefs(args, count); std::vector argsWithThis = ConstructJsFunctionArguments(undefinedRef, argRefs); - assert(argsWithThis.size() <= USHRT_MAX); + assert(argsWithThis.size() <= (std::numeric_limits::max)()); JsValueRef result; VerifyJsErrorElseThrow(JsConstructObject( diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 5d53b534796..b39022eda03 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -241,6 +241,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { // 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)))); } From a40f66188ff2239f2083de0e76c47bd40d12477b Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Thu, 31 Oct 2019 14:49:39 -0700 Subject: [PATCH 2/3] Change files --- ...eact-native-windows-2019-10-31-14-49-39-nitFixes.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 change/react-native-windows-2019-10-31-14-49-39-nitFixes.json diff --git a/change/react-native-windows-2019-10-31-14-49-39-nitFixes.json b/change/react-native-windows-2019-10-31-14-49-39-nitFixes.json new file mode 100644 index 00000000000..335ae21faa9 --- /dev/null +++ b/change/react-native-windows-2019-10-31-14-49-39-nitFixes.json @@ -0,0 +1,9 @@ +{ + "type": "none", + "comment": "Fix some nit suggestions from PR #3568.", + "packageName": "react-native-windows", + "email": "yicyao@microsoft.com", + "commit": "09b426a5bbf575ad23a721c50d0b374c84364b72", + "date": "2019-10-31T21:49:39.312Z", + "file": "D:\\Git\\hansenyy-react-native-windows-3\\change\\react-native-windows-2019-10-31-14-49-39-nitFixes.json" +} \ No newline at end of file From db51ce82f0147fa17cce995874808a4e59d1fa15 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Thu, 31 Oct 2019 16:01:22 -0700 Subject: [PATCH 3/3] Fix Universal build. --- vnext/JSI/Shared/ChakraRuntime.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index b580f7f233f..363dceb756d 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -461,7 +461,7 @@ size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { int result = ToInteger(GetProperty(GetChakraObjectRef(arr), "length")); - if (result < 0 || result > (std::numeric_limits::max)()) { + if (result < 0) { throw facebook::jsi::JSINativeException("Invalid JS array length detected."); } @@ -473,7 +473,7 @@ size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { int result = ToInteger(GetProperty(GetChakraObjectRef(arrBuf), "bytelength")); - if (result < 0 || result > (std::numeric_limits::max)()) { + if (result < 0) { throw facebook::jsi::JSINativeException("Invalid JS array buffer bytelength detected."); } @@ -493,7 +493,7 @@ uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { facebook::jsi::Value ChakraRuntime::getValueAtIndex(const facebook::jsi::Array &arr, size_t index) { assert(isArray(arr)); - assert(index <= (std::numeric_limits::max)()); + assert(index <= static_cast((std::numeric_limits::max)())); JsValueRef result = JS_INVALID_REFERENCE; VerifyJsErrorElseThrow(JsGetIndexedProperty(GetChakraObjectRef(arr), ToJsNumber(static_cast(index)), &result)); @@ -502,7 +502,7 @@ facebook::jsi::Value ChakraRuntime::getValueAtIndex(const facebook::jsi::Array & void ChakraRuntime::setValueAtIndexImpl(facebook::jsi::Array &arr, size_t index, const facebook::jsi::Value &value) { assert(isArray(arr)); - assert(index <= (std::numeric_limits::max)()); + assert(index <= static_cast((std::numeric_limits::max)())); VerifyJsErrorElseThrow( JsSetIndexedProperty(GetChakraObjectRef(arr), ToJsNumber(static_cast(index)), ToChakraObjectRef(value)));