From 746b982bcf8457e1eddc7d90cd6d99633378c8d0 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Fri, 11 Oct 2019 17:01:45 -0700 Subject: [PATCH 01/16] Add ChakraObjectRef. --- vnext/JSI/Shared/ByteArrayBuffer.h | 3 +- vnext/JSI/Shared/ChakraObjectRef.cpp | 118 +++++++++++++++++++ vnext/JSI/Shared/ChakraObjectRef.h | 64 ++++++++++ vnext/JSI/Shared/JSI.Shared.vcxitems | 2 + vnext/JSI/Shared/JSI.Shared.vcxitems.filters | 6 + 5 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 vnext/JSI/Shared/ChakraObjectRef.cpp create mode 100644 vnext/JSI/Shared/ChakraObjectRef.h diff --git a/vnext/JSI/Shared/ByteArrayBuffer.h b/vnext/JSI/Shared/ByteArrayBuffer.h index 50a22f8d9bf..1ef740cfd1c 100644 --- a/vnext/JSI/Shared/ByteArrayBuffer.h +++ b/vnext/JSI/Shared/ByteArrayBuffer.h @@ -3,9 +3,10 @@ #pragma once -#include #include "jsi/jsi.h" +#include + namespace Microsoft::JSI { class ByteArrayBuffer final : public facebook::jsi::Buffer { diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp new file mode 100644 index 00000000000..ab0e0ca13e4 --- /dev/null +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#include "ChakraObjectRef.h" + +#include "jsi//jsi.h" + +#include +#include + +namespace Microsoft::JSI { + +namespace { + +inline void CheckedJsAddRef(JsRef ref) { + ThrowUponChakraError(JsAddRef(ref, nullptr), "JsAddRef"); +} + +inline void CheckedJsRelease(JsRef ref) { + ThrowUponChakraError(JsRelease(ref, nullptr), "JsRelease"); +} + +} // namespace + +void ThrowUponChakraError(JsErrorCode error, const char *const chakraApiName) { + if (error != JsNoError) { + std::ostringstream errorString; + errorString << "A call to " + << (chakraApiName ? chakraApiName : "Chakra(Core) API") + << " returned error code 0x" << std::hex << error << '.'; + throw facebook::jsi::JSINativeException(errorString.str()); + } +} + +ChakraObjectRef::ChakraObjectRef(const ChakraObjectRef &other) noexcept + : m_ref{other.m_ref}, m_state{other.m_state} { + if (m_state == State::Initialized) { + assert(m_ref); + CheckedJsAddRef(m_ref); + } else { + assert(!m_ref); + } +} + +ChakraObjectRef::ChakraObjectRef(ChakraObjectRef &&other) noexcept { + std::swap(*this, other); +} + +ChakraObjectRef &ChakraObjectRef::operator=( + const ChakraObjectRef &rhs) noexcept { + ChakraObjectRef rhsCopy(rhs); + std::swap(*this, rhsCopy); + return *this; +} + +ChakraObjectRef &ChakraObjectRef::operator=(ChakraObjectRef &&rhs) noexcept { + std::swap(*this, rhs); + return *this; +} + +ChakraObjectRef::~ChakraObjectRef() noexcept { + if (m_state == State::Initialized) { + assert(m_ref); + CheckedJsRelease(m_ref); + } else { + assert(!m_ref); + } +} + +void ChakraObjectRef::Initialize(JsRef ref) { + assert(!m_ref); + + if (m_state != State::Uninitialized) { + throw facebook::jsi::JSINativeException( + "A ChakraObjectRef can only be initialzed once."); + } + + if (!ref) { + throw facebook::jsi::JSINativeException( + "Cannot initialize a ChakraObjectRef with a null reference."); + } + + CheckedJsAddRef(ref); + m_ref = ref; + m_state = State::Initialized; +} + +void ChakraObjectRef::Invalidate() { + switch (m_state) { + case State::Uninitialized: { + assert(!m_ref); + throw facebook::jsi::JSINativeException( + "Cannot invalidate a ChakraObjectRef that has not been initialized."); + break; + } + case State::Initialized: { + assert(m_ref); + CheckedJsRelease(m_ref); + m_ref = nullptr; + m_state = State::Invalidated; + break; + } + case State::Invalidated: { + assert(!m_ref); + throw facebook::jsi::JSINativeException( + "Cannot invalidate a ChakraObjectRef that has already been " + "invalidated."); + break; + } + default: { + // Control flow should never reach here. + std::terminate(); + break; + } + } +} + +} // namespace Microsoft::JSI diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h new file mode 100644 index 00000000000..f7a051fb9e6 --- /dev/null +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#pragma once + +#ifdef CHAKRACORE +#include "ChakraCore.h" +#else +#ifndef USE_EDGEMODE_JSRT +#define USE_EDGEMODE_JSRT +#endif +#include +#endif + +#include + +namespace Microsoft::JSI { + +inline void CrashUponChakraError(JsErrorCode error) { + if (error != JsNoError) { + std::terminate(); + } +} + +void ThrowUponChakraError( + JsErrorCode error, + const char *const chakraApiName = nullptr); + +// An shared_ptr like RAII Wrapper for JsRefs, which are references to objects +// owned by the garbage collector. These include JsContextRef, JsValueRef, and +// JsPropertyIdRef, etc. ChakraObjectRef ensures that JsAddRef and JsRelease +// are called upon initialization and invalidation, respectively. It also allows +// users to implicitly convert it into a JsRef. A ChakraObjectRef must only be +// initialized once and invalidated once. +class ChakraObjectRef { + public: + ChakraObjectRef() noexcept {} + inline explicit ChakraObjectRef(JsRef ref) { + Initialize(ref); + } + + ChakraObjectRef(const ChakraObjectRef &other) noexcept; + ChakraObjectRef(ChakraObjectRef &&other) noexcept; + + ChakraObjectRef &operator=(const ChakraObjectRef &rhs) noexcept; + ChakraObjectRef &operator=(ChakraObjectRef &&rhs) noexcept; + + ~ChakraObjectRef() noexcept; + + void Initialize(JsRef ref); + void Invalidate(); + + inline operator JsRef() const noexcept { + return m_ref; + } + + private: + enum class State { Uninitialized, Initialized, Invalidated }; + + JsRef m_ref = nullptr; + State m_state = State::Uninitialized; +}; + +} // namespace Microsoft::JSI diff --git a/vnext/JSI/Shared/JSI.Shared.vcxitems b/vnext/JSI/Shared/JSI.Shared.vcxitems index 8b4e9d778f4..f2e4ac6186f 100644 --- a/vnext/JSI/Shared/JSI.Shared.vcxitems +++ b/vnext/JSI/Shared/JSI.Shared.vcxitems @@ -14,10 +14,12 @@ + + diff --git a/vnext/JSI/Shared/JSI.Shared.vcxitems.filters b/vnext/JSI/Shared/JSI.Shared.vcxitems.filters index 9023c2bc4cb..ce60891f4a3 100644 --- a/vnext/JSI/Shared/JSI.Shared.vcxitems.filters +++ b/vnext/JSI/Shared/JSI.Shared.vcxitems.filters @@ -21,10 +21,16 @@ Header Files + + Header Files + Source Files + + Source Files + \ No newline at end of file From e7d6f4b51948d0df38473eef5033195636655b42 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Fri, 11 Oct 2019 18:08:59 -0700 Subject: [PATCH 02/16] Add some ChakraObjectRef helper functions. --- vnext/JSI/Shared/ChakraObjectRef.cpp | 129 +++++++++++++++++++++++++++ vnext/JSI/Shared/ChakraObjectRef.h | 52 +++++++++++ 2 files changed, 181 insertions(+) diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index ab0e0ca13e4..fad12db1bc9 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -3,6 +3,8 @@ #include "ChakraObjectRef.h" +#include "Unicode.h" + #include "jsi//jsi.h" #include @@ -115,4 +117,131 @@ void ChakraObjectRef::Invalidate() { } } +JsValueType getValueType(const ChakraObjectRef &jsValue) { + JsValueType type; + ThrowUponChakraError(JsGetValueType(jsValue, &type), "JsGetValueType"); + return type; +} + +JsPropertyIdType getPropertyIdType(const ChakraObjectRef &jsPropId) { + JsPropertyIdType type; + ThrowUponChakraError( + JsGetPropertyIdType(jsPropId, &type), "JsGetPropertyIdType"); + return type; +} + +std::wstring getPropertyName(const ChakraObjectRef &id) { + if (getPropertyIdType(id) != JsPropertyIdTypeString) { + throw facebook::jsi::JSINativeException( + "It is llegal to retrieve the name of a property symbol."); + } + const wchar_t *propertyName; + ThrowUponChakraError( + JsGetPropertyNameFromId(id, &propertyName), "JsGetPropertyNameFromId"); + return std::wstring{propertyName}; +} + +ChakraObjectRef getPropertySymbol(const ChakraObjectRef &id) { + if (getPropertyIdType(id) != JsPropertyIdTypeSymbol) { + throw facebook::jsi::JSINativeException( + "It is llegal to retrieve the symbol associated with a property name."); + } + JsValueRef symbol = nullptr; + ThrowUponChakraError( + JsGetSymbolFromPropertyId(id, &symbol), "JsGetSymbolFromPropertyId"); + return ChakraObjectRef{symbol}; +} + +ChakraObjectRef getPropertyId(const wchar_t *const name) { + JsPropertyIdRef id = nullptr; + ThrowUponChakraError( + JsGetPropertyIdFromName(name, &id), "JsGetPropertyIdFromName"); + return ChakraObjectRef(id); +} + +std::string toStdString(const ChakraObjectRef &jsString) { + // TODO (yicyao) + return ""; +} + +std::wstring toStdWstring(const ChakraObjectRef &jsString) { + assert(getValueType(jsString) == JsString); + + const wchar_t *utf16; + size_t length; + ThrowUponChakraError( + JsStringToPointer(jsString, &utf16, &length), "JsStringToPointer"); + + return std::wstring(utf16, length); +} + +ChakraObjectRef toJsString(const char *utf8, size_t length) { + JsValueRef result; + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion + // using ChakraCore's JsCreateString API. +#ifdef CHAKRACORE + ThrowUponChakraError(JsCreateString(utf8, length, &result), "JsCreateString"); +#else + std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); + ThrowUponChakraError( + JsPointerToString(utf16.c_str(), utf16.length(), &result), + "JsPointerToString"); +#endif + return ChakraObjectRef(result); +} + +ChakraObjectRef toJsString(const wchar_t *const utf16, size_t length) { + // TODO (yicyao) + return ChakraObjectRef{}; +} + +ChakraObjectRef toJsString(const ChakraObjectRef &ref) { + JsValueRef str = nullptr; + ThrowUponChakraError( + JsConvertValueToString(ref, &str), "JsConvertValueToString"); + return ChakraObjectRef(str); +} + +ChakraObjectRef toJsExternalArrayBuffer( + const std::shared_ptr &buffer) { + // TODO (yicyao) + return ChakraObjectRef{}; +} + +bool compareJsValues( + const ChakraObjectRef &jsValue1, + const ChakraObjectRef &jsValue2) { + bool result = false; + // Note that JsStrictEquals should only be used for JsValueRefs and not for + // other types of JsRefs (e.g. JsPropertyIdRef, etc.). + ThrowUponChakraError( + JsStrictEquals(jsValue1, jsValue2, &result), "JsStrictEquals"); + return result; +} + +bool compareJsPropertyIds( + const ChakraObjectRef &jsPropId1, + const ChakraObjectRef &jsPropId2) { + JsPropertyIdType type1 = getPropertyIdType(jsPropId1); + JsPropertyIdType type2 = getPropertyIdType(jsPropId2); + + if (type1 != type2) { + return false; + } + + if (type1 == JsPropertyIdTypeString) { + assert(type2 == JsPropertyIdTypeString); + return getPropertyName(jsPropId1) == getPropertyName(jsPropId2); + } + + if (type1 == JsPropertyIdTypeSymbol) { + assert(type2 == JsPropertyIdTypeSymbol); + return compareJsValues( + getPropertySymbol(jsPropId1), getPropertySymbol(jsPropId2)); + } + + // Control should never reach here. + std::terminate(); +} + } // namespace Microsoft::JSI diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index f7a051fb9e6..41e8a4313d6 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -3,6 +3,8 @@ #pragma once +#include "jsi/jsi.h" + #ifdef CHAKRACORE #include "ChakraCore.h" #else @@ -13,6 +15,7 @@ #endif #include +#include namespace Microsoft::JSI { @@ -61,4 +64,53 @@ class ChakraObjectRef { State m_state = State::Uninitialized; }; +// jsValue must be managing a JsValueRef. +JsValueType getValueType(const ChakraObjectRef &jsValue); + +// jsPropId must be managing a JsPropertyIdRef. +JsPropertyIdType getPropertyIdType(const ChakraObjectRef &jsPropId); + +// jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeString. +std::wstring getPropertyName(const ChakraObjectRef &jsPropId); + +// jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeSymbol. +// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS Symbol. +ChakraObjectRef getPropertySymbol(const ChakraObjectRef &jsPropId); + +// name must be null terminated. +ChakraObjectRef getPropertyId(const wchar_t *const name); + +// jsString must be managing a JsValueRef pointing to a JS string. The returned +// std::string/std::wstring is UTF-8/UTF-16 encoded. These functions copy the JS +// string buffer into the returned std::string/std::wstring. +std::string toStdString(const ChakraObjectRef &jsString); +std::wstring toStdWstring(const ChakraObjectRef &jsString); + +// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS string. +// utf8/utf16 must point to a UTF-8/UTF-16 encoded buffer. These buffers do not +// have to be null terminated and are copied to JS engine owned memory. +ChakraObjectRef toJsString(const char *const utf8, size_t length); +ChakraObjectRef toJsString(const wchar_t *const utf16, size_t length); + +// jsValue must be mananing a JsValueRef. Returns a ChakraObjectRef managing a +// JsValueRef that points the return value of the JS .toString function. +ChakraObjectRef toJsString(const ChakraObjectRef &jsValue); + +// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS ArrayBuffer. +// This ArrayBuffer is backed by buffer and keeps buffer alive till the garbage +// collector finalizes it. +ChakraObjectRef toJsExternalArrayBuffer( + const std::shared_ptr &buffer); + +// Both jsValue1 and jsValue2 must be managing a JsValueRef. Returns whether +// jsValue1 anf jsValue2 are strictly equal. +bool compareJsValues( + const ChakraObjectRef &jsValue1, + const ChakraObjectRef &jsValue2); + +// Both jsPropId1 and jsPropId2 must be managing a JsPropertyIdRef. +bool compareJsPropertyIds( + const ChakraObjectRef &jsPropId1, + const ChakraObjectRef &jsPropId2); + } // namespace Microsoft::JSI From 1e93842459377514899c93a33f4883155ec9c6f0 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Mon, 14 Oct 2019 11:11:50 -0700 Subject: [PATCH 03/16] Add more ChakraObjectRef helpers. --- vnext/JSI/Shared/ChakraObjectRef.cpp | 63 +++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index fad12db1bc9..566d7a2a3e5 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -4,9 +4,11 @@ #include "ChakraObjectRef.h" #include "Unicode.h" +#include "Utilities.h" #include "jsi//jsi.h" +#include #include #include @@ -160,8 +162,21 @@ ChakraObjectRef getPropertyId(const wchar_t *const name) { } std::string toStdString(const ChakraObjectRef &jsString) { - // TODO (yicyao) - return ""; + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion + // using ChakraCore's JsCreateString API. +#ifdef CHAKRACORE + size_t length = 0; + ThrowUponChakraError( + JsCopyString(jsString, nullptr, 0, &length), "JsCopyString"); + std::string result('a', length); + ThrowUponChakraError( + JsCopyString(jsString, result.data(), result.length(), &length), + "JsCopyString"); + assert(length == result.length()); + return result; +#else + return Common::Unicode::Utf16ToUtf8(toStdWstring(jsString)); +#endif } std::wstring toStdWstring(const ChakraObjectRef &jsString) { @@ -181,18 +196,18 @@ ChakraObjectRef toJsString(const char *utf8, size_t length) { // using ChakraCore's JsCreateString API. #ifdef CHAKRACORE ThrowUponChakraError(JsCreateString(utf8, length, &result), "JsCreateString"); + return ChakraObjectRef(result); #else std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); - ThrowUponChakraError( - JsPointerToString(utf16.c_str(), utf16.length(), &result), - "JsPointerToString"); + return toJsString(utf16.c_str(), utf16.length()); #endif - return ChakraObjectRef(result); } ChakraObjectRef toJsString(const wchar_t *const utf16, size_t length) { - // TODO (yicyao) - return ChakraObjectRef{}; + JsValueRef result; + ThrowUponChakraError( + JsPointerToString(utf16, length, &result), "JsPointerToString"); + return ChakraObjectRef(result); } ChakraObjectRef toJsString(const ChakraObjectRef &ref) { @@ -204,8 +219,36 @@ ChakraObjectRef toJsString(const ChakraObjectRef &ref) { ChakraObjectRef toJsExternalArrayBuffer( const std::shared_ptr &buffer) { - // TODO (yicyao) - return ChakraObjectRef{}; + size_t size = buffer->size(); + assert(size < UINT_MAX); + + JsValueRef arrayBuffer = nullptr; + auto bufferRef = + std::make_unique>(buffer); + + // We allocate a copy of buffer on the heap, a shared_ptr which is deleted + // when the JavaScript garbage collecotr releases the created external array + // buffer. This ensures that buffer stays alive while the JavaScript engine is + // using it. + ThrowUponChakraError( + JsCreateExternalArrayBuffer( + Common::Utilities::CheckedReinterpretCast( + const_cast(buffer->data())), + static_cast(size), + [](void *bufferToDestroy) { + // We wrap bufferToDestroy in a unique_ptr to avoid calling delete + // explicitly. + std::unique_ptr> + wrapper{ + static_cast *>( + bufferToDestroy)}; + }, + bufferRef.get(), + &arrayBuffer), + "JsCreateExternalArrayBuffer"); + + bufferRef.release(); + return ChakraObjectRef(arrayBuffer); } bool compareJsValues( From d6f050abedfab995ea217ed8642f34c0b508b352 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Mon, 14 Oct 2019 18:25:40 -0700 Subject: [PATCH 04/16] Introduce ChakraPointerValue and reimplement many Runtime functions with it. --- vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp | 17 - vnext/JSI/Shared/ChakraObjectRef.cpp | 78 ++- vnext/JSI/Shared/ChakraObjectRef.h | 37 +- vnext/JSI/Shared/ChakraRuntime.cpp | 634 +++++++++++--------- vnext/JSI/Shared/ChakraRuntime.h | 174 ++++-- 5 files changed, 525 insertions(+), 415 deletions(-) diff --git a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp index 2fde9c6f258..901d0a7bda5 100644 --- a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp +++ b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp @@ -48,23 +48,6 @@ JsValueRef ChakraRuntime::strongObjectRef( return strongRef; } -// Note :: ChakraCore header provides an API which takes 8-bit string .. which -// is not available in edge mode. -JsValueRef ChakraRuntime::createJSString(const char *data, size_t length) { - JsValueRef value; - JsCreateString(reinterpret_cast(data), length, &value); - return value; -} - -// Note :: ChakraCore header provides an API which takes 8-bit string .. which -// is not available in edge mode. -JsValueRef ChakraRuntime::createJSPropertyId(const char *data, size_t length) { - JsValueRef propIdRef; - if (JsNoError != JsCreatePropertyId(data, length, &propIdRef)) - std::terminate(); - return propIdRef; -} - // ES6 Promise callback void CALLBACK ChakraRuntime::PromiseContinuationCallback( JsValueRef funcRef, diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index 566d7a2a3e5..2515e31530c 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -119,32 +119,32 @@ void ChakraObjectRef::Invalidate() { } } -JsValueType getValueType(const ChakraObjectRef &jsValue) { +JsValueType GetValueType(const ChakraObjectRef &jsValue) { JsValueType type; ThrowUponChakraError(JsGetValueType(jsValue, &type), "JsGetValueType"); return type; } -JsPropertyIdType getPropertyIdType(const ChakraObjectRef &jsPropId) { +JsPropertyIdType GetPropertyIdType(const ChakraObjectRef &jsPropId) { JsPropertyIdType type; ThrowUponChakraError( JsGetPropertyIdType(jsPropId, &type), "JsGetPropertyIdType"); return type; } -std::wstring getPropertyName(const ChakraObjectRef &id) { - if (getPropertyIdType(id) != JsPropertyIdTypeString) { +std::wstring GetPropertyName(const ChakraObjectRef &id) { + if (GetPropertyIdType(id) != JsPropertyIdTypeString) { throw facebook::jsi::JSINativeException( "It is llegal to retrieve the name of a property symbol."); } - const wchar_t *propertyName; + const wchar_t *propertyName = nullptr; ThrowUponChakraError( JsGetPropertyNameFromId(id, &propertyName), "JsGetPropertyNameFromId"); return std::wstring{propertyName}; } -ChakraObjectRef getPropertySymbol(const ChakraObjectRef &id) { - if (getPropertyIdType(id) != JsPropertyIdTypeSymbol) { +ChakraObjectRef GetPropertySymbol(const ChakraObjectRef &id) { + if (GetPropertyIdType(id) != JsPropertyIdTypeSymbol) { throw facebook::jsi::JSINativeException( "It is llegal to retrieve the symbol associated with a property name."); } @@ -154,14 +154,28 @@ ChakraObjectRef getPropertySymbol(const ChakraObjectRef &id) { return ChakraObjectRef{symbol}; } -ChakraObjectRef getPropertyId(const wchar_t *const name) { +ChakraObjectRef GetPropertyId(const char *const utf8, size_t length){ + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion + // using ChakraCore's JsCreateString API. +#ifdef CHAKRACORE + JsPropertyIdRef id = nullptr; + ThrowUponChakraError( + JsCreatePropertyId(utf8, length, &id), "JsCreatePropertyId"); + return ChakraObjectRef(id); +#else + std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8); + return GetPropertyId(utf16); +#endif +} + +ChakraObjectRef GetPropertyId(const std::wstring &utf16) { JsPropertyIdRef id = nullptr; ThrowUponChakraError( - JsGetPropertyIdFromName(name, &id), "JsGetPropertyIdFromName"); + JsGetPropertyIdFromName(utf16.c_str(), &id), "JsGetPropertyIdFromName"); return ChakraObjectRef(id); } -std::string toStdString(const ChakraObjectRef &jsString) { +std::string ToStdString(const ChakraObjectRef &jsString) { // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCreateString API. #ifdef CHAKRACORE @@ -175,49 +189,55 @@ std::string toStdString(const ChakraObjectRef &jsString) { assert(length == result.length()); return result; #else - return Common::Unicode::Utf16ToUtf8(toStdWstring(jsString)); + return Common::Unicode::Utf16ToUtf8(ToStdWstring(jsString)); #endif } -std::wstring toStdWstring(const ChakraObjectRef &jsString) { - assert(getValueType(jsString) == JsString); +std::wstring ToStdWstring(const ChakraObjectRef &jsString) { + assert(GetValueType(jsString) == JsString); - const wchar_t *utf16; - size_t length; + const wchar_t *utf16 = nullptr; + size_t length = 0; ThrowUponChakraError( JsStringToPointer(jsString, &utf16, &length), "JsStringToPointer"); return std::wstring(utf16, length); } -ChakraObjectRef toJsString(const char *utf8, size_t length) { - JsValueRef result; +ChakraObjectRef ToJsString(const char *utf8, size_t length) { // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCreateString API. #ifdef CHAKRACORE + JsValueRef result = nullptr; ThrowUponChakraError(JsCreateString(utf8, length, &result), "JsCreateString"); return ChakraObjectRef(result); #else std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); - return toJsString(utf16.c_str(), utf16.length()); + return ToJsString(utf16.c_str(), utf16.length()); #endif } -ChakraObjectRef toJsString(const wchar_t *const utf16, size_t length) { - JsValueRef result; +ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length) { + JsValueRef result = nullptr; ThrowUponChakraError( JsPointerToString(utf16, length, &result), "JsPointerToString"); return ChakraObjectRef(result); } -ChakraObjectRef toJsString(const ChakraObjectRef &ref) { +ChakraObjectRef ToJsString(const ChakraObjectRef &ref) { JsValueRef str = nullptr; ThrowUponChakraError( JsConvertValueToString(ref, &str), "JsConvertValueToString"); return ChakraObjectRef(str); } -ChakraObjectRef toJsExternalArrayBuffer( +ChakraObjectRef ToJsNumber(int num) { + JsValueRef result = nullptr; + ThrowUponChakraError(JsIntToNumber(num, &result), "JsIntToNumber"); + return ChakraObjectRef(result); +} + +ChakraObjectRef ToJsExternalArrayBuffer( const std::shared_ptr &buffer) { size_t size = buffer->size(); assert(size < UINT_MAX); @@ -251,7 +271,7 @@ ChakraObjectRef toJsExternalArrayBuffer( return ChakraObjectRef(arrayBuffer); } -bool compareJsValues( +bool CompareJsValues( const ChakraObjectRef &jsValue1, const ChakraObjectRef &jsValue2) { bool result = false; @@ -262,11 +282,11 @@ bool compareJsValues( return result; } -bool compareJsPropertyIds( +bool CompareJsPropertyIds( const ChakraObjectRef &jsPropId1, const ChakraObjectRef &jsPropId2) { - JsPropertyIdType type1 = getPropertyIdType(jsPropId1); - JsPropertyIdType type2 = getPropertyIdType(jsPropId2); + JsPropertyIdType type1 = GetPropertyIdType(jsPropId1); + JsPropertyIdType type2 = GetPropertyIdType(jsPropId2); if (type1 != type2) { return false; @@ -274,13 +294,13 @@ bool compareJsPropertyIds( if (type1 == JsPropertyIdTypeString) { assert(type2 == JsPropertyIdTypeString); - return getPropertyName(jsPropId1) == getPropertyName(jsPropId2); + return GetPropertyName(jsPropId1) == GetPropertyName(jsPropId2); } if (type1 == JsPropertyIdTypeSymbol) { assert(type2 == JsPropertyIdTypeSymbol); - return compareJsValues( - getPropertySymbol(jsPropId1), getPropertySymbol(jsPropId2)); + return CompareJsValues( + GetPropertySymbol(jsPropId1), GetPropertySymbol(jsPropId2)); } // Control should never reach here. diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 41e8a4313d6..82403ed91fc 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -65,51 +65,56 @@ class ChakraObjectRef { }; // jsValue must be managing a JsValueRef. -JsValueType getValueType(const ChakraObjectRef &jsValue); +JsValueType GetValueType(const ChakraObjectRef &jsValue); // jsPropId must be managing a JsPropertyIdRef. -JsPropertyIdType getPropertyIdType(const ChakraObjectRef &jsPropId); +JsPropertyIdType GetPropertyIdType(const ChakraObjectRef &jsPropId); // jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeString. -std::wstring getPropertyName(const ChakraObjectRef &jsPropId); +std::wstring GetPropertyName(const ChakraObjectRef &jsPropId); // jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeSymbol. // Returns a ChakraObjectRef managing a JsValueRef pointing to a JS Symbol. -ChakraObjectRef getPropertySymbol(const ChakraObjectRef &jsPropId); +ChakraObjectRef GetPropertySymbol(const ChakraObjectRef &jsPropId); -// name must be null terminated. -ChakraObjectRef getPropertyId(const wchar_t *const name); +// utf8 does not have to be null terminated. +ChakraObjectRef GetPropertyId(const char *const utf8, size_t length); + +ChakraObjectRef GetPropertyId(const std::wstring &utf16); // jsString must be managing a JsValueRef pointing to a JS string. The returned // std::string/std::wstring is UTF-8/UTF-16 encoded. These functions copy the JS // string buffer into the returned std::string/std::wstring. -std::string toStdString(const ChakraObjectRef &jsString); -std::wstring toStdWstring(const ChakraObjectRef &jsString); +std::string ToStdString(const ChakraObjectRef &jsString); +std::wstring ToStdWstring(const ChakraObjectRef &jsString); // Returns a ChakraObjectRef managing a JsValueRef pointing to a JS string. -// utf8/utf16 must point to a UTF-8/UTF-16 encoded buffer. These buffers do not -// have to be null terminated and are copied to JS engine owned memory. -ChakraObjectRef toJsString(const char *const utf8, size_t length); -ChakraObjectRef toJsString(const wchar_t *const utf16, size_t length); +// utf8 and utf16 do not have to be null terminated and are copied to JS engine +// owned memory. +ChakraObjectRef ToJsString(const char *const utf8, size_t length); +ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length); // jsValue must be mananing a JsValueRef. Returns a ChakraObjectRef managing a // JsValueRef that points the return value of the JS .toString function. -ChakraObjectRef toJsString(const ChakraObjectRef &jsValue); +ChakraObjectRef ToJsString(const ChakraObjectRef &jsValue); + +// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS number. +ChakraObjectRef ToJsNumber(int num); // Returns a ChakraObjectRef managing a JsValueRef pointing to a JS ArrayBuffer. // This ArrayBuffer is backed by buffer and keeps buffer alive till the garbage // collector finalizes it. -ChakraObjectRef toJsExternalArrayBuffer( +ChakraObjectRef ToJsExternalArrayBuffer( const std::shared_ptr &buffer); // Both jsValue1 and jsValue2 must be managing a JsValueRef. Returns whether // jsValue1 anf jsValue2 are strictly equal. -bool compareJsValues( +bool CompareJsValues( const ChakraObjectRef &jsValue1, const ChakraObjectRef &jsValue2); // Both jsPropId1 and jsPropId2 must be managing a JsPropertyIdRef. -bool compareJsPropertyIds( +bool CompareJsPropertyIds( const ChakraObjectRef &jsPropId1, const ChakraObjectRef &jsPropId2); diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 983d92df414..c40f336f879 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -2,18 +2,17 @@ // Licensed under the MIT License. #include "ChakraRuntime.h" -#include "ChakraRuntimeArgs.h" + +#include "Unicode.h" +#include "Utilities.h" #include #include +#include +#include #include #include -#include "Unicode.h" - -#include - -using namespace facebook::react; namespace Microsoft::JSI { @@ -81,25 +80,25 @@ ChakraRuntime::ChakraRuntime(ChakraRuntimeArgs &&args) noexcept : m_args{std::move(args)} { JsRuntimeAttributes runtimeAttributes = JsRuntimeAttributeNone; - if (!runtimeArgs().enableJITCompilation) { + if (!m_args.enableJITCompilation) { runtimeAttributes = static_cast( runtimeAttributes | JsRuntimeAttributeDisableNativeCodeGeneration | JsRuntimeAttributeDisableExecutablePageAllocation); } - if (JsCreateRuntime(runtimeAttributes, nullptr, &m_runtime) != JsNoError) { - std::terminate(); - } + ThrowUponChakraError( + JsCreateRuntime(runtimeAttributes, nullptr, &m_runtime), + "JsCreateRuntime"); setupMemoryTracker(); - // Create an execution context - JsCreateContext(m_runtime, &m_ctx); - JsAddRef(m_ctx, nullptr); + JsContextRef context = nullptr; + ThrowUponChakraError(JsCreateContext(m_runtime, &context), "JsCreateContext"); + m_context.Initialize(context); // Note :: We currently assume that the runtime will be created and // exclusively used in a single thread. - JsSetCurrentContext(m_ctx); + ThrowUponChakraError(JsSetCurrentContext(m_context), "JsSetCurrentContext"); startDebuggingIfNeeded(); @@ -111,12 +110,13 @@ ChakraRuntime::ChakraRuntime(ChakraRuntimeArgs &&args) noexcept ChakraRuntime::~ChakraRuntime() noexcept { stopDebuggingIfNeeded(); - JsSetCurrentContext(JS_INVALID_REFERENCE); - JsRelease(m_ctx, nullptr); + ThrowUponChakraError( + JsSetCurrentContext(JS_INVALID_REFERENCE), "JsSetCurrentContext"); + m_context.Invalidate(); JsSetRuntimeMemoryAllocationCallback(m_runtime, nullptr, nullptr); - JsDisposeRuntime(m_runtime); + ThrowUponChakraError(JsDisposeRuntime(m_runtime), "JsDisposeRuntime"); } #pragma region Functions_inherited_from_Runtime @@ -228,110 +228,93 @@ bool ChakraRuntime::isInspectable() { } facebook::jsi::Runtime::PointerValue *ChakraRuntime::cloneSymbol( - const facebook::jsi::Runtime::PointerValue *) { - throw facebook::jsi::JSINativeException("Not implemented!"); + const facebook::jsi::Runtime::PointerValue *pointerValue) { + return CloneChakraPointerValue(pointerValue); } facebook::jsi::Runtime::PointerValue *ChakraRuntime::cloneString( - const facebook::jsi::Runtime::PointerValue *pv) { - if (!pv) { - return nullptr; - } - const ChakraStringValue *string = static_cast(pv); - return makeStringValue(string->m_str); + const facebook::jsi::Runtime::PointerValue *pointerValue) { + return CloneChakraPointerValue(pointerValue); } facebook::jsi::Runtime::PointerValue *ChakraRuntime::cloneObject( - const facebook::jsi::Runtime::PointerValue *pv) { - if (!pv) { - return nullptr; - } - - const ChakraObjectValue *object = static_cast(pv); - return makeObjectValue(object->m_obj); + const facebook::jsi::Runtime::PointerValue *pointerValue) { + return CloneChakraPointerValue(pointerValue); } facebook::jsi::Runtime::PointerValue *ChakraRuntime::clonePropNameID( - const facebook::jsi::Runtime::PointerValue *pv) { - if (!pv) { - return nullptr; - } - - const ChakraPropertyIdValue *propId = - static_cast(pv); - return makePropertyIdValue(propId->m_propId); + const facebook::jsi::Runtime::PointerValue *pointerValue) { + return CloneChakraPointerValue(pointerValue); } facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromAscii( const char *str, size_t length) { - JsValueRef propIdRef = createJSPropertyId(str, length); - - auto res = createPropNameID(propIdRef); - return res; + // Unfortunately due to the typing used by JSI and Chakra(Core), we have to do + // a double reinterpret cast here. + return createPropNameIDFromUtf8( + Common::Utilities::CheckedReinterpretCast(str), length); } facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromUtf8( const uint8_t *utf8, size_t length) { - JsValueRef prpoIdRef = - createJSPropertyId(reinterpret_cast(utf8), length); - - auto res = createPropNameID(prpoIdRef); - return res; + ChakraObjectRef propId = GetPropertyId( + Common::Utilities::CheckedReinterpretCast(utf8), length); + return MakePointer(std::move(propId)); } facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromString( const facebook::jsi::String &str) { - std::string propNameString = JSStringToSTLString(stringRef(str)); - return createPropNameIDFromUtf8( - reinterpret_cast(propNameString.c_str()), - propNameString.length()); + // We don not use the functions: + // std::string ChakraRuntime::utf8(const String& str), and + // std::string ToStdString(const ChakraObjectRef &jsString) + // here to avoud excessive Unicode conversions. + std::wstring propName = ToStdWstring(GetChakraObjectRef(str)); + return MakePointer(GetPropertyId(propName)); } -std::string ChakraRuntime::utf8(const facebook::jsi::PropNameID &sym) { - const wchar_t *name; - checkException(JsGetPropertyNameFromId(propIdRef(sym), &name)); - return Microsoft::Common::Unicode::Utf16ToUtf8(name, wcslen(name)); +std::string ChakraRuntime::utf8(const facebook::jsi::PropNameID &id) { + return Common::Unicode::Utf16ToUtf8(GetPropertyName(GetChakraObjectRef(id))); } bool ChakraRuntime::compare( - const facebook::jsi::PropNameID &a, - const facebook::jsi::PropNameID &b) { - bool result; - JsEquals(propIdRef(a), propIdRef(b), &result); - return result; + const facebook::jsi::PropNameID &lhs, + const facebook::jsi::PropNameID &rhs) { + return CompareJsPropertyIds(GetChakraObjectRef(lhs), GetChakraObjectRef(rhs)); } -std::string ChakraRuntime::symbolToString(const facebook::jsi::Symbol &) { - throw facebook::jsi::JSINativeException("Not implemented!"); +std::string ChakraRuntime::symbolToString(const facebook::jsi::Symbol &s) { + return ToStdString(ToJsString(GetChakraObjectRef(s))); } facebook::jsi::String ChakraRuntime::createStringFromAscii( const char *str, size_t length) { - // Yes we end up double casting for semantic reasons (UTF8 contains ASCII, - // not the other way around) - return this->createStringFromUtf8( - reinterpret_cast(str), length); + // Unfortunately due to the typing used by JSI and Chakra, we have to do a + // double reinterpret cast here. + return createStringFromUtf8( + Common::Utilities::CheckedReinterpretCast(str), length); } facebook::jsi::String ChakraRuntime::createStringFromUtf8( const uint8_t *str, size_t length) { - JsValueRef stringRef = - createJSString(reinterpret_cast(str), length); - return createString(stringRef); + return MakePointer(ToJsString( + Common::Utilities::CheckedReinterpretCast(str), length)); } std::string ChakraRuntime::utf8(const facebook::jsi::String &str) { - return JSStringToSTLString(stringRef(str)); + return ToStdString(GetChakraObjectRef(str)); } facebook::jsi::Object ChakraRuntime::createObject() { - return createObject(static_cast(nullptr)); + JsValueRef obj = nullptr; + ThrowUponJsError(JsCreateObject(&obj), "JsCreateObject"); + return MakePointer(ChakraObjectRef(obj)); } +// TODO (yicyao) facebook::jsi::Object ChakraRuntime::createObject( std::shared_ptr hostObject) { facebook::jsi::Object proxyTarget = @@ -340,6 +323,7 @@ facebook::jsi::Object ChakraRuntime::createObject( return createProxy(std::move(proxyTarget), createHostObjectProxyHandler()); } +// TODO (yicyao) std::shared_ptr ChakraRuntime::getHostObject( const facebook::jsi::Object &obj) { if (!isHostObject(obj)) @@ -362,6 +346,7 @@ std::shared_ptr ChakraRuntime::getHostObject( return externalData->getHostObject(); } +// TODO (yicyao) facebook::jsi::HostFunctionType &ChakraRuntime::getHostFunction( const facebook::jsi::Function &obj) { throw std::runtime_error( @@ -370,86 +355,73 @@ facebook::jsi::HostFunctionType &ChakraRuntime::getHostFunction( facebook::jsi::Value ChakraRuntime::getProperty( const facebook::jsi::Object &obj, - const facebook::jsi::String &name) { - JsValueRef objRef = objectRef(obj); - - std::wstring propName = JSStringToSTLWString(stringRef(name)); - JsPropertyIdRef propIdRef; - checkException(JsGetPropertyIdFromName(propName.c_str(), &propIdRef)); - - JsValueRef value; - checkException(JsGetProperty(objectRef(obj), propIdRef, &value)); - return createValue(value); + const facebook::jsi::PropNameID &name) { + JsValueRef result = nullptr; + ThrowUponJsError( + JsGetProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name), &result), + "JsGetProperty"); + return ToJsiValue(ChakraObjectRef(result)); } facebook::jsi::Value ChakraRuntime::getProperty( const facebook::jsi::Object &obj, - const facebook::jsi::PropNameID &name) { - JsValueRef objRef = objectRef(obj); - JsValueRef exc = nullptr; - JsValueRef res; - checkException(JsGetProperty(objRef, propIdRef(name), &res)); - return createValue(res); + const facebook::jsi::String &name) { + return getProperty(obj, createPropNameIDFromString(name)); } bool ChakraRuntime::hasProperty( const facebook::jsi::Object &obj, - const facebook::jsi::String &name) { - std::wstring propName = JSStringToSTLWString(stringRef(name)); - - JsPropertyIdRef propId = JS_INVALID_REFERENCE; - checkException(JsGetPropertyIdFromName(propName.c_str(), &propId)); - - bool hasProperty; - checkException(JsHasProperty(objectRef(obj), propId, &hasProperty)); - return hasProperty; + const facebook::jsi::PropNameID &name) { + bool result = false; + ThrowUponJsError( + JsHasProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name), &result), + "JsHasProperty"); + return result; } bool ChakraRuntime::hasProperty( const facebook::jsi::Object &obj, - const facebook::jsi::PropNameID &name) { - bool hasProperty; - checkException(JsHasProperty(objectRef(obj), propIdRef(name), &hasProperty)); - return hasProperty; + const facebook::jsi::String &name) { + return hasProperty(obj, createPropNameIDFromString(name)); } void ChakraRuntime::setPropertyValue( facebook::jsi::Object &object, const facebook::jsi::PropNameID &name, const facebook::jsi::Value &value) { - checkException( - JsSetProperty(objectRef(object), propIdRef(name), valueRef(value), true)); + // We use strict rules for property assignments here, so any assignment that + // silently fails in normal code (assignment to a non-writable global or + // property, assignment to a getter-only property, assignment to a new + // property on a non-extensible object) will throw. + ThrowUponJsError( + JsSetProperty( + GetChakraObjectRef(object), + GetChakraObjectRef(name), + ToChakraObjectRef(value), + true /* useStrictRules */), + "JsSetProperty"); } void ChakraRuntime::setPropertyValue( facebook::jsi::Object &object, const facebook::jsi::String &name, const facebook::jsi::Value &value) { - std::wstring propName = JSStringToSTLWString(stringRef(name)); - - JsPropertyIdRef propId = JS_INVALID_REFERENCE; - checkException(JsGetPropertyIdFromName(propName.c_str(), &propId)); - - checkException( - JsSetProperty(objectRef(object), propId, valueRef(value), true)); + setPropertyValue(object, createPropNameIDFromString(name), value); } -bool ChakraRuntime::isArray(const facebook::jsi::Object &obj) const { - JsValueType type; - JsGetValueType(objectRef(obj), &type); - return type == JsValueType::JsArray; +bool ChakraRuntime::isArray(const facebook::jsi::Object &object) const { + return GetValueType(GetChakraObjectRef(object)) == JsArray; } -bool ChakraRuntime::isArrayBuffer(const facebook::jsi::Object & /*obj*/) const { - throw std::runtime_error("Unsupported"); +bool ChakraRuntime::isArrayBuffer(const facebook::jsi::Object &object) const { + return GetValueType(GetChakraObjectRef(object)) == JsArrayBuffer; } bool ChakraRuntime::isFunction(const facebook::jsi::Object &obj) const { - JsValueType type; - JsGetValueType(objectRef(obj), &type); - return type == JsValueType::JsFunction; + return GetValueType(GetChakraObjectRef(obj)) == JsFunction; } +// TODO (yicyao) bool ChakraRuntime::isHostObject(const facebook::jsi::Object &obj) const { facebook::jsi::Value val = obj.getProperty( const_cast(*this), s_proxyIsHostObjectPropName); @@ -459,96 +431,143 @@ bool ChakraRuntime::isHostObject(const facebook::jsi::Object &obj) const { return false; } +// TODO (yicyao) bool ChakraRuntime::isHostFunction(const facebook::jsi::Function &obj) const { throw std::runtime_error("ChakraRuntime::isHostFunction is not implemented."); } facebook::jsi::Array ChakraRuntime::getPropertyNames( - const facebook::jsi::Object &obj) { - JsValueRef propertyNamesArrayRef; - checkException(JsGetOwnPropertyNames(objectRef(obj), &propertyNamesArrayRef)); - - JsPropertyIdRef propertyId; - checkException(JsGetPropertyIdFromName(L"length", &propertyId)); - JsValueRef countRef; - checkException(JsGetProperty(propertyNamesArrayRef, propertyId, &countRef)); - int count; - checkException(JsNumberToInt(countRef, &count)); - - auto result = createArray(count); - for (int i = 0; i < count; i++) { - JsValueRef index; - checkException(JsIntToNumber(i, &index)); - JsValueRef propertyName; - checkException( - JsGetIndexedProperty(propertyNamesArrayRef, index, &propertyName)); - result.setValueAtIndex(*this, i, createString(propertyName)); - } - - return result; -} + const facebook::jsi::Object &object) { + constexpr const char *const jsGetPropertyNamesSource = + "(function()\n" + "{\n" + " return function(obj)\n" + " {\n" + " var propertyNames = []\n" + " for (propertyName in obj) \n" + " {\n" + " propertyNames.push(propertyName)\n" + " }\n" + " return propertyNames\n" + " }\n" + "})()"; + + // TODO (yicyao): Comment on why this variable is not static. + std::shared_ptr jsGetPropertyNamesSourceBuffer = + std::make_shared(jsGetPropertyNamesSource); + + // TODO (yicyao): Need some refactoring here to ensure lifetime correctness, + // reduce duplicate code, and improve efficiency. + + facebook::jsi::Function jsGetPropertyNames = + evaluateJavaScript(jsGetPropertyNamesSourceBuffer, "") + .asObject(*this) + .asFunction(*this); + + facebook::jsi::Value objAsValue(*this, object); + return call( + jsGetPropertyNames, + facebook::jsi::Value::undefined(), + &objAsValue, + 1) + .asObject(*this) + .asArray(*this); +} + +// Only ChakraCore supports weak reference semantics, so ChakraRuntime +// WeakObjects are in fact strong references. facebook::jsi::WeakObject ChakraRuntime::createWeakObject( - const facebook::jsi::Object &obj) { + const facebook::jsi::Object &object) { return make( - makeWeakRefValue(newWeakObjectRef(obj))); + CloneChakraPointerValue(getPointerValue(object))); } facebook::jsi::Value ChakraRuntime::lockWeakObject( - const facebook::jsi::WeakObject &weakObj) { - return createValue(strongObjectRef(weakObj)); + const facebook::jsi::WeakObject &weakObject) { + // We need to make a copy of the ChakraObjectRef held within weakObj's + // member PointerValue for the returned jsi::Value here. + ChakraObjectRef ref = GetChakraObjectRef(weakObject); + return ToJsiValue(std::move(ref)); } facebook::jsi::Array ChakraRuntime::createArray(size_t length) { - JsValueRef result; - checkException(JsCreateArray(static_cast(length), &result)); - return createObject(result).getArray(*this); + assert(length <= UINT_MAX); + JsValueRef result = nullptr; + + ThrowUponJsError( + JsCreateArray(static_cast(length), &result), + "JsCreateArray"); + + return MakePointer(ChakraObjectRef(result)) + .asArray(*this); } size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { - std::string lengthStr = "length"; + assert(isArray(arr)); + + constexpr const uint8_t propName[] = {'l', 'e', 'n', 'g', 't', 'h'}; - JsPropertyIdRef propId = - createJSPropertyId(lengthStr.c_str(), lengthStr.length()); + facebook::jsi::PropNameID propId = createPropNameIDFromUtf8( + propName, Common::Utilities::ArraySize(propName)); - JsValueRef valueObject; - checkException(JsGetProperty(objectRef(arr), propId, &valueObject)); + return static_cast(getProperty(arr, propId).asNumber()); +} - JsValueRef numberValue; - checkException(JsConvertValueToNumber(valueObject, &numberValue)); +size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { + assert(isArrayBuffer(arrBuf)); - int intValue; - checkException(JsNumberToInt(numberValue, &intValue)); + constexpr const uint8_t propName[] = { + 'b', 'y', 't', 'e', 'l', 'e', 'n', 'g', 't', 'h'}; - return intValue; -} + facebook::jsi::PropNameID propId = createPropNameIDFromUtf8( + propName, Common::Utilities::ArraySize(propName)); -uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer & /*obj*/) { - throw std::runtime_error("Unsupported"); + return static_cast(getProperty(arrBuf, propId).asNumber()); } -size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer & /*obj*/) { - throw std::runtime_error("Unsupported"); +uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { + assert(isArrayBuffer(arrBuf)); + + uint8_t *buffer = nullptr; + unsigned int size = 0; + + ThrowUponJsError( + JsGetArrayBufferStorage(GetChakraObjectRef(arrBuf), &buffer, &size), + "JsGetArrayBufferStorage"); + + return buffer; } facebook::jsi::Value ChakraRuntime::getValueAtIndex( const facebook::jsi::Array &arr, - size_t i) { - JsValueRef index; - JsIntToNumber(static_cast(i), &index); - JsValueRef property; - checkException(JsGetIndexedProperty(objectRef(arr), index, &property)); - return createValue(property); + size_t index) { + assert(isArray(arr)); + assert(index <= INT_MAX); + + JsValueRef result = nullptr; + ThrowUponJsError( + JsGetIndexedProperty( + GetChakraObjectRef(arr), + ToJsNumber(static_cast(index)), + &result), + "JsGetIndexedProperty"); + return ToJsiValue(ChakraObjectRef(result)); } void ChakraRuntime::setValueAtIndexImpl( facebook::jsi::Array &arr, - size_t i, + size_t index, const facebook::jsi::Value &value) { - JsValueRef index; - JsIntToNumber(static_cast(i), &index); + assert(isArray(arr)); + assert(index <= INT_MAX); - checkException(JsSetIndexedProperty(objectRef(arr), index, valueRef(value))); + ThrowUponJsError( + JsSetIndexedProperty( + GetChakraObjectRef(arr), + ToJsNumber(static_cast(index)), + ToChakraObjectRef(value)), + "JsSetIndexedProperty"); } facebook::jsi::Function ChakraRuntime::createFunctionFromHostFunction( @@ -577,7 +596,7 @@ facebook::jsi::Value ChakraRuntime::call( size_t count) { JsValueRef result = nullptr; checkException(JsCallFunction( - objectRef(f), + GetChakraObjectRef(f), ArgsConverterForCall( *this, jsThis.isUndefined() ? nullptr : objectRef(jsThis.getObject(*this)), @@ -610,37 +629,142 @@ void ChakraRuntime::popScope(Runtime::ScopeState *state) { checkException(JsCollectGarbage(m_runtime), "JsCollectGarbage"); } +bool ChakraRuntime::strictEquals( + const facebook::jsi::Symbol &a, + const facebook::jsi::Symbol &b) const { + return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); +} + bool ChakraRuntime::strictEquals( const facebook::jsi::String &a, const facebook::jsi::String &b) const { - bool result; - JsStrictEquals(stringRef(a), stringRef(b), &result); - return result; + return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); } bool ChakraRuntime::strictEquals( const facebook::jsi::Object &a, const facebook::jsi::Object &b) const { + return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); +} + +bool ChakraRuntime::instanceOf( + const facebook::jsi::Object &obj, + const facebook::jsi::Function &func) { bool result; - JsStrictEquals(objectRef(a), objectRef(b), &result); + ThrowUponJsError( + JsInstanceOf(GetChakraObjectRef(obj), GetChakraObjectRef(func), &result), + "JsInstanceOf"); return result; } -bool ChakraRuntime::strictEquals( - const facebook::jsi::Symbol &, - const facebook::jsi::Symbol &) const { - throw facebook::jsi::JSINativeException("Not implemented!"); +#pragma endregion Functions_inherited_from_Runtime + +ChakraObjectRef ChakraRuntime::ToChakraObjectRef( + const facebook::jsi::Value &value) { + if (value.isUndefined()) { + JsValueRef ref; + ThrowUponJsError(JsGetUndefinedValue(&ref), "JsGetUndefinedValue"); + return ChakraObjectRef(ref); + + } else if (value.isNull()) { + JsValueRef ref; + ThrowUponJsError(JsGetNullValue(&ref), "JsGetNullValue"); + return ChakraObjectRef(ref); + + } else if (value.isBool()) { + JsValueRef ref; + ThrowUponJsError(JsBoolToBoolean(value.getBool(), &ref), "JsBoolToBoolean"); + return ChakraObjectRef(ref); + + } else if (value.isNumber()) { + JsValueRef ref; + ThrowUponJsError( + JsDoubleToNumber(value.asNumber(), &ref), "JsDoubleToNumber"); + return ChakraObjectRef(ref); + + } else if (value.isSymbol()) { + return GetChakraObjectRef(value.asSymbol(*this)); + + } else if (value.isString()) { + return GetChakraObjectRef(value.asString(*this)); + + } else if (value.isObject()) { + return GetChakraObjectRef(value.asObject(*this)); + + } else { + // Control flow should never reach here. + std::terminate(); + } } -bool ChakraRuntime::instanceOf( - const facebook::jsi::Object &o, - const facebook::jsi::Function &f) { - bool res; - checkException(JsInstanceOf(objectRef(o), objectRef(f), &res)); - return res; +std::vector ChakraRuntime::ToChakraObjectRefs( + const facebook::jsi::Value *value, + size_t count) { + std::vector result{}; + + for (unsigned int i = 0; i < count; ++i) { + result.emplace_back(ToChakraObjectRef(*value)); + ++value; + } + + return result; } -#pragma endregion Functions_inherited_from_Runtime +facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { + JsValueType type = GetValueType(ref); + + switch (type) { + case JsUndefined: { + return facebook::jsi::Value(); + break; + } + case JsNull: { + return facebook::jsi::Value(nullptr); + break; + } + case JsNumber: { + double number; + ThrowUponJsError(JsNumberToDouble(ref, &number), "JsNumberToDouble"); + return facebook::jsi::Value(number); + break; + } + case JsString: { + return facebook::jsi::Value( + *this, MakePointer(std::move(ref))); + break; + } + case JsBoolean: { + bool b; + ThrowUponJsError(JsBooleanToBool(ref, &b), "JsBooleanToBool"); + return facebook::jsi::Value(b); + break; + } + case JsSymbol: { + return facebook::jsi::Value( + *this, MakePointer(std::move(ref))); + break; + } + case JsObject: + case JsFunction: + case JsError: + case JsArray: + case JsArrayBuffer: + case JsTypedArray: + case JsDataView: { + return facebook::jsi::Value( + *this, MakePointer(std::move(ref))); + break; + } + default: { + // Control flow should never reach here. + std::terminate(); + break; + } + } + + // Control flow should never reach here. + std::terminate(); +} facebook::jsi::Value ChakraRuntime::createValue(JsValueRef value) const { JsValueType type; @@ -722,59 +846,6 @@ JsValueRef ChakraRuntime::valueRef(const facebook::jsi::Value &valueIn) { } } -ChakraRuntime::ChakraPropertyIdValue::~ChakraPropertyIdValue() { - JsRelease(m_propId, nullptr); -} - -void ChakraRuntime::ChakraPropertyIdValue::invalidate() { - delete this; -} - -ChakraRuntime::ChakraPropertyIdValue::ChakraPropertyIdValue( - JsPropertyIdRef propIdRef) - : m_propId(propIdRef) { - JsAddRef(propIdRef, nullptr); -} - -ChakraRuntime::ChakraStringValue::ChakraStringValue(JsValueRef str) - : m_str(str) { - JsAddRef(str, nullptr); -} - -void ChakraRuntime::ChakraStringValue::invalidate() { - delete this; -} - -ChakraRuntime::ChakraStringValue::~ChakraStringValue() { - JsRelease(m_str, nullptr); -} - -ChakraRuntime::ChakraObjectValue::ChakraObjectValue(JsValueRef obj) - : m_obj(obj) { - JsAddRef(m_obj, nullptr); -} - -void ChakraRuntime::ChakraObjectValue::invalidate() { - delete this; -} - -ChakraRuntime::ChakraObjectValue::~ChakraObjectValue() { - JsRelease(m_obj, nullptr); -} - -ChakraRuntime::ChakraWeakRefValue::ChakraWeakRefValue(JsWeakRef obj) - : m_obj(obj) { - JsAddRef(m_obj, nullptr); -} - -void ChakraRuntime::ChakraWeakRefValue::invalidate() { - delete this; -} - -ChakraRuntime::ChakraWeakRefValue::~ChakraWeakRefValue() { - JsRelease(m_obj, nullptr); -} - template /*static */ facebook::jsi::Object ChakraRuntime::ObjectWithExternalData< T>::create(ChakraRuntime &rt, T *externalData) { @@ -851,30 +922,42 @@ void ChakraRuntime::checkException(JsErrorCode result, const char *message) { *this, createStringFromAscii(errorString.c_str(), errorString.length())); } -JsValueRef ChakraRuntime::stringRef(const facebook::jsi::String &str) { - return static_cast(getPointerValue(str))->m_str; -} - -JsPropertyIdRef ChakraRuntime::propIdRef(const facebook::jsi::PropNameID &sym) { - return static_cast(getPointerValue(sym)) - ->m_propId; -} +void ChakraRuntime::ThrowUponJsError( + JsErrorCode error, + const char *const chakraApiName) { + switch (error) { + case JsNoError: { + return; + break; + } -JsValueRef ChakraRuntime::objectRef(const facebook::jsi::Object &obj) { - return static_cast(getPointerValue(obj))->m_obj; -} + case JsErrorScriptException: { + JsValueRef jsError; + CrashUponChakraError(JsGetAndClearException(&jsError)); + throw facebook::jsi::JSError( + "A JavaScript Error was thrown.", + *this, + ToJsiValue(ChakraObjectRef(jsError))); + break; + } -JsWeakRef ChakraRuntime::objectRef(const facebook::jsi::WeakObject &obj) { - return static_cast(getPointerValue(obj))->m_obj; -} + default: { + ThrowUponChakraError(error, chakraApiName); + break; + } + } // switch (error) -facebook::jsi::String ChakraRuntime::createString(JsValueRef str) const { - return make(makeStringValue(str)); + // Control flow should never reach here. + std::terminate(); } -facebook::jsi::PropNameID ChakraRuntime::createPropNameID(JsValueRef str) { - return make(makePropertyIdValue(str)); -} +// facebook::jsi::String ChakraRuntime::createString(JsValueRef str) const { +// return make(makeStringValue(str)); +//} +// +// facebook::jsi::PropNameID ChakraRuntime::createPropNameID(JsValueRef str) { +// return make(makePropertyIdValue(str)); +//} template facebook::jsi::Object ChakraRuntime::createObject( @@ -1069,31 +1152,6 @@ facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeWeakRefValue( return new ChakraWeakRefValue(objWeakRef); } -std::wstring ChakraRuntime::JSStringToSTLWString(JsValueRef str) { - const wchar_t *value; - size_t length; - - if (JsNoError != JsStringToPointer(str, &value, &length)) { - std::terminate(); - } - - // Note: Copying the string out of JsString, as required. - return std::wstring(value, length); -} - -std::string ChakraRuntime::JSStringToSTLString(JsValueRef str) { - const wchar_t *value; - size_t length; - - if (JsNoError != JsStringToPointer(str, &value, &length)) { - std::terminate(); - } - - // Note: This results in multiple buffer copyings. We should look for - // optimization. - return Microsoft::Common::Unicode::Utf16ToUtf8(std::wstring(value, length)); -} - void ChakraRuntime::setupMemoryTracker() noexcept { if (runtimeArgs().memoryTracker) { size_t initialMemoryUsage = 0; diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 1219b0f0422..cd2d7e86f86 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -4,6 +4,7 @@ #pragma once #include "ByteArrayBuffer.h" +#include "ChakraObjectRef.h" #include "ChakraRuntimeArgs.h" #include "jsi/jsi.h" @@ -25,7 +26,6 @@ #if !defined(CHAKRACORE) class DebugProtocolHandler {}; class DebugService {}; -using JsWeakRef = JsValueRef; #endif namespace Microsoft::JSI { @@ -59,10 +59,17 @@ class ChakraRuntime : public facebook::jsi::Runtime { // Instrumentation instance which returns no metrics. private: - PointerValue *cloneSymbol(const PointerValue *pv) override; - PointerValue *cloneString(const PointerValue *pv) override; - PointerValue *cloneObject(const PointerValue *pv) override; - PointerValue *clonePropNameID(const PointerValue *pv) override; + // Despite the name "clone" suggesting a deep copy, a return value of these + // functions points to a new heap allocated ChakraPointerValue whose memeber + // ChakraObjectRef refers to the same JavaScript object as the member + // ChakraObjectRef of *pointerValue. This behavior is consistent with that of + // HermesRuntime and JSCRuntime. Also, Like all ChakraPointerValues, the + // return value must only be used as an argument to the constructor of + // jsi::Pointer or one of its derived classes. + PointerValue *cloneSymbol(const PointerValue *pointerValue) override; + PointerValue *cloneString(const PointerValue *pointerValue) override; + PointerValue *cloneObject(const PointerValue *pointerValue) override; + PointerValue *clonePropNameID(const PointerValue *pointerValue) override; facebook::jsi::PropNameID createPropNameIDFromAscii( const char *str, @@ -72,7 +79,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { size_t length) override; facebook::jsi::PropNameID createPropNameIDFromString( const facebook::jsi::String &str) override; - std::string utf8(const facebook::jsi::PropNameID &str) override; + std::string utf8(const facebook::jsi::PropNameID &id) override; bool compare( const facebook::jsi::PropNameID &lhs, const facebook::jsi::PropNameID &rhs) override; @@ -119,6 +126,8 @@ class ChakraRuntime : public facebook::jsi::Runtime { bool isFunction(const facebook::jsi::Object &obj) const override; bool isHostObject(const facebook::jsi::Object &obj) const override; bool isHostFunction(const facebook::jsi::Function &func) const override; + // Returns the names of all enumerable properties of an object. This + // corresponds the properties iterated through by the JavaScript for..in loop. facebook::jsi::Array getPropertyNames( const facebook::jsi::Object &obj) override; @@ -130,6 +139,9 @@ class ChakraRuntime : public facebook::jsi::Runtime { facebook::jsi::Array createArray(size_t length) override; size_t size(const facebook::jsi::Array &arr) override; size_t size(const facebook::jsi::ArrayBuffer &arrBuf) override; + // The lifetime of the buffer returned is the same as the lifetime of the + // ArrayBuffer. The returned buffer pointer does not count as a reference to + // the ArrayBuffer for the purpose of garbage collection. uint8_t *data(const facebook::jsi::ArrayBuffer &arrBuf) override; facebook::jsi::Value getValueAtIndex( const facebook::jsi::Array &arr, @@ -169,15 +181,22 @@ class ChakraRuntime : public facebook::jsi::Runtime { const facebook::jsi::Object &b) const override; bool instanceOf( - const facebook::jsi::Object &o, - const facebook::jsi::Function &f) override; + const facebook::jsi::Object &obj, + const facebook::jsi::Function &func) override; #pragma endregion Functions_inherited_from_Runtime public: + // These three functions only performs shallow copies. + ChakraObjectRef ToChakraObjectRef(const facebook::jsi::Value &value); + std::vector ToChakraObjectRefs( + const facebook::jsi::Value *value, + size_t count); + facebook::jsi::Value ToJsiValue(ChakraObjectRef &&ref); + + // TODO (yicyao): // JsValueRef->JSValue (needs make.*Value so it must be member function) facebook::jsi::Value createValue(JsValueRef value) const; - // Value->JsValueRef (similar to above) JsValueRef valueRef(const facebook::jsi::Value &value); @@ -187,50 +206,86 @@ class ChakraRuntime : public facebook::jsi::Runtime { } private: - class ChakraPropertyIdValue final : public PointerValue { - ChakraPropertyIdValue(JsPropertyIdRef str); - ~ChakraPropertyIdValue(); - - void invalidate() override; - - JsPropertyIdRef m_propId; + // ChakraPointerValue is needed for working with Facebook's jsi::Pointer class + // and must only be used for this purpose. Every instance of + // ChakraPointerValue should be allocated on the heap and be used as an + // argument to the constructor of jsi::Pointer or one of its derived classes. + // Pointer makes sure that invalidate(), which frees the heap allocated + // ChakraPointerValue, is called upon destruction. Since the constructor of + // jsi::Pointer is protected, we usually have to invoke it through + // jsi::Runtime::make. The code should look something like: + // + // make(new ChakraPointerValue(...)); + // + // or you can use the helper function MakePointer(), as defined below. + template + struct ChakraPointerValueTemplate : PointerValue { + public: + ChakraPointerValueTemplate(const T &ref) noexcept : m_ref{ref} { + static_assert( + std::is_same::value || + // Since only ChakraCore offers the JsWeakRef type alias, we + // cannot use it here; so void* is the best alternative we can use + // here. + std::is_same::value, + "ChakraPointerValueTemplate should only be instantiated for " + "ChakraObjectRef and JsWeakRef."); + } - protected: - friend class ChakraRuntime; - }; + ChakraPointerValueTemplate(T &&ref) noexcept : m_ref{std::move(ref)} {} + + // Declaring ~ChakraPointerValueTemplate() private prevents the compiler + // from implicitly generating the following functions, so we have to tell + // the compiler to do so. + ChakraPointerValueTemplate( + const ChakraPointerValueTemplate &other) noexcept = default; + ChakraPointerValueTemplate(ChakraPointerValueTemplate &&other) noexcept = + default; + ChakraPointerValueTemplate &operator=( + const ChakraPointerValueTemplate &rhs) noexcept = default; + ChakraPointerValueTemplate &operator=( + ChakraPointerValueTemplate &&rhs) noexcept = default; + + inline void invalidate() noexcept override { + delete this; + } - class ChakraStringValue final : public PointerValue { - ChakraStringValue(JsValueRef str); - ~ChakraStringValue(); + inline const T &GetRef() const noexcept { + return m_ref; + } - void invalidate() override; + private: + // ~ChakraPointerValueTemplate() should only be invoked by invalidate(). + // Hence we make it private. + ~ChakraPointerValueTemplate() noexcept = default; - protected: - friend class ChakraRuntime; - JsValueRef m_str; + T m_ref; }; - class ChakraObjectValue final : public PointerValue { - ChakraObjectValue(JsValueRef obj); - ~ChakraObjectValue(); - - void invalidate() override; + using ChakraPointerValue = ChakraPointerValueTemplate; - protected: - friend class ChakraRuntime; - JsValueRef m_obj; - }; - - class ChakraWeakRefValue final : public PointerValue { - ChakraWeakRefValue(JsWeakRef obj); - ~ChakraWeakRefValue(); + template + inline T MakePointer(ChakraObjectRef &&ref) { + static_assert( + std::is_base_of::value, + "MakePointer should only be instantiated for classes derived from " + "facebook::jsi::Pointer."); + return make(new ChakraPointerValue(std::move(ref))); + } - void invalidate() override; + // The pointer passed to this function must point to a ChakraPointerValue. + inline static ChakraPointerValue *CloneChakraPointerValue( + const PointerValue *pointerValue) { + return new ChakraPointerValue( + *(static_cast(pointerValue))); + } - protected: - friend class ChakraRuntime; - JsWeakRef m_obj; - }; + // The jsi::Pointer passed to this function must hold a ChakraPointerValue. + inline static const ChakraObjectRef &GetChakraObjectRef( + const facebook::jsi::Pointer &p) { + return static_cast(getPointerValue(p)) + ->GetRef(); + } class HostObjectProxy { public: @@ -282,21 +337,19 @@ class ChakraRuntime : public facebook::jsi::Runtime { template friend class ObjectWithExternalData; + // TODO (yicyao) inline void checkException(JsErrorCode res); inline void checkException(JsErrorCode res, const char *msg); + void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); - // Basically convenience casts - static JsValueRef stringRef(const facebook::jsi::String &str); - static JsPropertyIdRef propIdRef(const facebook::jsi::PropNameID &sym); - static JsValueRef objectRef(const facebook::jsi::Object &obj); - static JsWeakRef objectRef(const facebook::jsi::WeakObject &obj); - - static JsWeakRef newWeakObjectRef(const facebook::jsi::Object &obj); - static JsValueRef strongObjectRef(const facebook::jsi::WeakObject &obj); + // TODO (yicyao) + // static JsWeakRef newWeakObjectRef(const facebook::jsi::Object &obj); + // static JsValueRef strongObjectRef(const facebook::jsi::WeakObject &obj); + // TODO (yicyao) // Factory methods for creating String/Object - facebook::jsi::String createString(JsValueRef stringRef) const; - facebook::jsi::PropNameID createPropNameID(JsValueRef stringRef); + // facebook::jsi::String createString(JsValueRef stringRef) const; + // facebook::jsi::PropNameID createPropNameID(JsValueRef stringRef); template facebook::jsi::Object createObject(JsValueRef objectRef, T *externalData) @@ -309,27 +362,18 @@ class ChakraRuntime : public facebook::jsi::Runtime { facebook::jsi::Function createProxyConstructor() noexcept; facebook::jsi::Object createHostObjectProxyHandler() noexcept; + // TODO (yicyao) // Used by factory methods and clone methods facebook::jsi::Runtime::PointerValue *makeStringValue(JsValueRef str) const; - template facebook::jsi::Runtime::PointerValue *makeObjectValue( JsValueRef obj, T *externaldata) const; facebook::jsi::Runtime::PointerValue *makeObjectValue(JsValueRef obj) const; - facebook::jsi::Runtime::PointerValue *makePropertyIdValue( JsPropertyIdRef propId) const; - facebook::jsi::Runtime::PointerValue *makeWeakRefValue(JsWeakRef obj) const; - // String helpers - static std::wstring JSStringToSTLWString(JsValueRef str); - static std::string JSStringToSTLString(JsValueRef str); - - static JsValueRef createJSString(const char *data, size_t length); - static JsValueRef createJSPropertyId(const char *data, size_t length); - // Promise Helpers static void CALLBACK PromiseContinuationCallback(JsValueRef funcRef, void *callbackState) noexcept; @@ -394,7 +438,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { ChakraRuntimeArgs m_args; JsRuntimeHandle m_runtime; - JsContextRef m_ctx; + ChakraObjectRef m_context; // Note: For simplicity, We are pinning the script and serialized script // buffers in the facebook::jsi::Runtime instance assuming as these buffers From b173de5cb7f4affdb3d2099d781958633b006241 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Tue, 15 Oct 2019 17:30:25 -0700 Subject: [PATCH 05/16] Make ChakraJsiRuntime_core.cpp build. --- vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp | 48 ++-- vnext/JSI/Shared/ChakraObjectRef.cpp | 2 +- vnext/JSI/Shared/ChakraRuntime.cpp | 294 +++++--------------- vnext/JSI/Shared/ChakraRuntime.h | 23 -- 4 files changed, 93 insertions(+), 274 deletions(-) diff --git a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp index 901d0a7bda5..30cb431c3fb 100644 --- a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp +++ b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp @@ -3,9 +3,10 @@ #include "ChakraRuntime.h" #include "ChakraRuntimeFactory.h" -#include "Unicode.h" #include +#include "ByteArrayBuffer.h" +#include "Unicode.h" // This file contains non-edge-mode (or win32) implementations. #if !defined(USE_EDGEMODE_JSRT) @@ -35,18 +36,21 @@ struct FileVersionInfoResource { } // namespace -JsWeakRef ChakraRuntime::newWeakObjectRef(const facebook::jsi::Object &obj) { - JsWeakRef weakRef; - JsCreateWeakReference(objectRef(obj), &weakRef); - return weakRef; -} - -JsValueRef ChakraRuntime::strongObjectRef( - const facebook::jsi::WeakObject &obj) { - JsValueRef strongRef; - JsGetWeakReferenceValue(objectRef(obj), &strongRef); - return strongRef; -} +// TODO (yicyao): We temporarily removed weak reference semantics from +// ChakraCore based jsi::Runtime. + +// JsWeakRef ChakraRuntime::newWeakObjectRef(const facebook::jsi::Object &obj) { +// JsWeakRef weakRef; +// JsCreateWeakReference(objectRef(obj), &weakRef); +// return weakRef; +//} +// +// JsValueRef ChakraRuntime::strongObjectRef( +// const facebook::jsi::WeakObject &obj) { +// JsValueRef strongRef; +// JsGetWeakReferenceValue(objectRef(obj), &strongRef); +// return strongRef; +//} // ES6 Promise callback void CALLBACK ChakraRuntime::PromiseContinuationCallback( @@ -71,7 +75,9 @@ void ChakraRuntime::PromiseContinuation(JsValueRef funcRef) noexcept { runtimeArgs().jsQueue->runOnQueue([this, funcRef]() { JsValueRef undefinedValue; JsGetUndefinedValue(&undefinedValue); - checkException(JsCallFunction(funcRef, &undefinedValue, 1, nullptr)); + ThrowUponJsError( + JsCallFunction(funcRef, &undefinedValue, 1, nullptr), + "JsCallFunction"); JsRelease(funcRef, nullptr); }); } @@ -94,7 +100,7 @@ void ChakraRuntime::PromiseRejectionTracker( JsValueRef stackStrValue; error = JsConvertValueToString(stack, &stackStrValue); if (error == JsNoError) { - errorStream << JSStringToSTLString(stackStrValue); + errorStream << ToStdString(ChakraObjectRef(stackStrValue)); } } } @@ -104,7 +110,7 @@ void ChakraRuntime::PromiseRejectionTracker( JsValueRef strValue; error = JsConvertValueToString(reason, &strValue); if (error == JsNoError) { - errorStream << JSStringToSTLString(strValue); + errorStream << ToStdString(ChakraObjectRef(strValue)); } } @@ -308,16 +314,16 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScriptSimple( &sourceURLRef); JsValueRef result; - checkException( + ThrowUponJsError( JsRun( sourceRef, 0, sourceURLRef, JsParseScriptAttributes::JsParseScriptAttributeNone, &result), - sourceURL.c_str()); + "JsRun"); - return createValue(result); + return ToJsiValue(ChakraObjectRef(result)); } // TODO :: Return result @@ -334,7 +340,7 @@ bool ChakraRuntime::evaluateSerializedScript( &bytecodeArrayBuffer) == JsNoError) { JsValueRef sourceURLRef = nullptr; if (!sourceURL.empty()) { - sourceURLRef = createJSString( + sourceURLRef = ToJsString( reinterpret_cast(sourceURL.c_str()), sourceURL.size()); } @@ -366,7 +372,7 @@ bool ChakraRuntime::evaluateSerializedScript( } else if (result == JsErrorBadSerializedScript) { return false; } else { - checkException(result); + ThrowUponChakraError(result); } } diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index 2515e31530c..b3cfc740c7e 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -182,7 +182,7 @@ std::string ToStdString(const ChakraObjectRef &jsString) { size_t length = 0; ThrowUponChakraError( JsCopyString(jsString, nullptr, 0, &length), "JsCopyString"); - std::string result('a', length); + std::string result(length, 'a'); ThrowUponChakraError( JsCopyString(jsString, result.data(), result.length(), &length), "JsCopyString"); diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index c40f336f879..a88642e947e 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -38,41 +38,18 @@ class HostFunctionProxy { ChakraRuntime &m_runtime; }; -class ArgsConverterForCall { - public: - ArgsConverterForCall( - ChakraRuntime &rt, - JsValueRef thisObj, - const facebook::jsi::Value *args, - size_t count) { - JsValueRef *destination = inline_; - if (count + 1 > maxStackArgs) { - outOfLine_ = std::make_unique(count + 1); - destination = outOfLine_.get(); - } - - if (thisObj == nullptr) { - JsValueRef undefinedValue; - JsGetUndefinedValue(&undefinedValue); - destination[0] = undefinedValue; - } else { - destination[0] = thisObj; - } - - for (size_t i = 0; i < count; ++i) { - destination[i + 1] = rt.valueRef(args[i]); - } +// Callers of this functions must make sure that jsThis and args are alive when +// using the return value of this function. +std::vector constructJsFunctionArguments( + const ChakraObjectRef &jsThis, + const std::vector &args) { + std::vector result; + result.push_back(JsRef(jsThis)); + for (const ChakraObjectRef &ref : args) { + result.push_back(JsRef(ref)); } - - operator JsValueRef *() { - return outOfLine_ ? outOfLine_.get() : inline_; - } - - private: - constexpr static unsigned maxStackArgs = 8; - JsValueRef inline_[maxStackArgs]; - std::unique_ptr outOfLine_; -}; + return result; +} } // namespace @@ -579,45 +556,68 @@ facebook::jsi::Function ChakraRuntime::createFunctionFromHostFunction( HostFunctionProxy *hostFuncProxy = new HostFunctionProxy(func, *this); JsValueRef funcRef; - checkException(JsCreateFunction( - ChakraRuntime::HostFunctionCall, hostFuncProxy, &funcRef)); - checkException(JsSetObjectBeforeCollectCallback( - funcRef, hostFuncProxy, [](JsRef ref, void *hostFuncProxy) { - delete hostFuncProxy; - })); + ThrowUponJsError( + JsCreateFunction( + ChakraRuntime::HostFunctionCall, hostFuncProxy, &funcRef), + "JsCreateFunction"); + + ThrowUponJsError( + JsSetObjectBeforeCollectCallback( + funcRef, + hostFuncProxy, + [](JsRef ref, void *hostFuncProxy) { delete hostFuncProxy; }), + "JsSetObjectBeforeCollectCallback"); return createObject(funcRef).getFunction(*this); } facebook::jsi::Value ChakraRuntime::call( - const facebook::jsi::Function &f, + const facebook::jsi::Function &func, const facebook::jsi::Value &jsThis, const facebook::jsi::Value *args, size_t count) { - JsValueRef result = nullptr; - checkException(JsCallFunction( - GetChakraObjectRef(f), - ArgsConverterForCall( - *this, - jsThis.isUndefined() ? nullptr : objectRef(jsThis.getObject(*this)), - args, - count), - static_cast(count + 1), - &result)); - return createValue(result); + // We must store these ChakraObjectRefs on the stack to make sure that they do + // not go out of scope when JsCallFunction is called. + ChakraObjectRef thisRef = ToChakraObjectRef(jsThis); + std::vector argRefs = ToChakraObjectRefs(args, count); + + std::vector argsWithThis = + constructJsFunctionArguments(thisRef, argRefs); + assert(argsWithThis.size() <= USHRT_MAX); + + JsValueRef result; + ThrowUponChakraError( + JsCallFunction( + GetChakraObjectRef(func), + argsWithThis.data(), + static_cast(argsWithThis.size()), + &result), + "JsCallFunction"); + return ToJsiValue(ChakraObjectRef(result)); } facebook::jsi::Value ChakraRuntime::callAsConstructor( - const facebook::jsi::Function &f, + const facebook::jsi::Function &func, const facebook::jsi::Value *args, size_t count) { - JsValueRef result = nullptr; - checkException(JsConstructObject( - objectRef(f), - ArgsConverterForCall(*this, nullptr, args, count), - static_cast(count), - &result)); - return createValue(result); + // We must store these ChakraObjectRefs on the stack to make sure that they do + // not go out of scope when JsConstructObject is called. + ChakraObjectRef undefinedRef = ToChakraObjectRef(facebook::jsi::Value()); + std::vector argRefs = ToChakraObjectRefs(args, count); + + std::vector argsWithThis = + constructJsFunctionArguments(undefinedRef, argRefs); + assert(argsWithThis.size() <= USHRT_MAX); + + JsValueRef result; + ThrowUponJsError( + JsConstructObject( + GetChakraObjectRef(func), + argsWithThis.data(), + static_cast(argsWithThis.size()), + &result), + "JsConstructObject"); + return ToJsiValue(ChakraObjectRef(result)); } facebook::jsi::Runtime::ScopeState *ChakraRuntime::pushScope() { @@ -626,7 +626,7 @@ facebook::jsi::Runtime::ScopeState *ChakraRuntime::pushScope() { void ChakraRuntime::popScope(Runtime::ScopeState *state) { assert(state == nullptr); - checkException(JsCollectGarbage(m_runtime), "JsCollectGarbage"); + ThrowUponJsError(JsCollectGarbage(m_runtime), "JsCollectGarbage"); } bool ChakraRuntime::strictEquals( @@ -766,86 +766,6 @@ facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { std::terminate(); } -facebook::jsi::Value ChakraRuntime::createValue(JsValueRef value) const { - JsValueType type; - JsGetValueType(value, &type); - - switch (type) { - case JsUndefined: - return facebook::jsi::Value(); - - case JsNull: - return facebook::jsi::Value(nullptr); - - case JsNumber: { - double doubleValue; - JsNumberToDouble(value, &doubleValue); - facebook::jsi::Value val(doubleValue); - return val; - } - - case JsString: { - std::string utf8str = JSStringToSTLString(value); - return facebook::jsi::String::createFromUtf8( - *const_cast( - reinterpret_cast(this)), - reinterpret_cast(utf8str.c_str()), - utf8str.size()); - break; - } - - case JsBoolean: { - bool boolValue; - JsBooleanToBool(value, &boolValue); - facebook::jsi::Value val(boolValue); - return val; - } - - case JsObject: - case JsFunction: - case JsArray: { - return facebook::jsi::Value(createObject(value)); - break; - } - - case JsError: - case JsSymbol: - case JsArrayBuffer: - case JsTypedArray: - case JsDataView: - default: - std::terminate(); - break; - } -} - -JsValueRef ChakraRuntime::valueRef(const facebook::jsi::Value &valueIn) { - if (valueIn.isUndefined()) { - JsValueRef value; - JsGetUndefinedValue(&value); - return value; - } else if (valueIn.isNull()) { - JsValueRef value; - JsGetNullValue(&value); - return value; - } else if (valueIn.isBool()) { - JsValueRef value; - JsBoolToBoolean(valueIn.getBool(), &value); - return value; - } else if (valueIn.isNumber()) { - JsValueRef value; - JsDoubleToNumber(valueIn.getNumber(), &value); - return value; - } else if (valueIn.isString()) { - return stringRef(valueIn.getString(*this)); - } else if (valueIn.isObject()) { - return objectRef(valueIn.getObject(*this)); - } else { - // What are you? - abort(); - } -} - template /*static */ facebook::jsi::Object ChakraRuntime::ObjectWithExternalData< T>::create(ChakraRuntime &rt, T *externalData) { @@ -869,59 +789,6 @@ T *ChakraRuntime::ObjectWithExternalData::getExternalData() { return externalData; } -void ChakraRuntime::checkException(JsErrorCode result) { - bool hasException = false; - if (result == JsNoError && (JsHasException(&hasException), !hasException)) - return; - - checkException(result, nullptr); -} - -void ChakraRuntime::checkException(JsErrorCode result, const char *message) { - bool hasException = false; - if (result == JsNoError && (JsHasException(&hasException), !hasException)) - return; - - std::ostringstream errorStream; - - if (message != nullptr) - errorStream << message << ". "; - - if (result != JsNoError) { - errorStream << "ChakraCore API Error :" << std::hex << result << ". "; - } - - if (hasException || result == JsErrorScriptException) { - errorStream << "JS exception found: "; - - JsValueRef exn; - JsGetAndClearException(&exn); - - JsPropertyIdRef messageName; - JsGetPropertyIdFromName(L"stack", &messageName); - - JsValueRef messageValue; - if (JsGetProperty(exn, messageName, &messageValue) == JsNoError) { - if (JsConvertValueToString(exn, &messageValue) != JsNoError) { - errorStream << "Unable to retrieve JS exception stack"; - } else { - errorStream << JSStringToSTLString(messageValue); - } - } else { - JsValueRef exnJStr; - if (JsConvertValueToString(exn, &exnJStr) != JsNoError) { - errorStream << "Unable to describe JS exception"; - } else { - errorStream << JSStringToSTLString(exnJStr); - } - } - } - - std::string errorString = errorStream.str(); - throw facebook::jsi::JSError( - *this, createStringFromAscii(errorString.c_str(), errorString.length())); -} - void ChakraRuntime::ThrowUponJsError( JsErrorCode error, const char *const chakraApiName) { @@ -951,14 +818,6 @@ void ChakraRuntime::ThrowUponJsError( std::terminate(); } -// facebook::jsi::String ChakraRuntime::createString(JsValueRef str) const { -// return make(makeStringValue(str)); -//} -// -// facebook::jsi::PropNameID ChakraRuntime::createPropNameID(JsValueRef str) { -// return make(makePropertyIdValue(str)); -//} - template facebook::jsi::Object ChakraRuntime::createObject( JsValueRef objectRef, @@ -1094,15 +953,6 @@ facebook::jsi::Object ChakraRuntime::createHostObjectProxyHandler() noexcept { return handlerObj; } -facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeStringValue( - JsValueRef stringRef) const { - if (!stringRef) { - JsValueRef emptyJsValue = createJSString("", 0); - stringRef = emptyJsValue; - } - return new ChakraStringValue(stringRef); -} - facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeObjectValue( JsValueRef objectRef) const { if (!objectRef) { @@ -1137,21 +987,6 @@ facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeObjectValue( return chakraObjValue; } -facebook::jsi::Runtime::PointerValue *ChakraRuntime::makePropertyIdValue( - JsPropertyIdRef propIdRef) const { - if (!propIdRef) { - std::terminate(); - } - return new ChakraPropertyIdValue(propIdRef); -} - -facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeWeakRefValue( - JsWeakRef objWeakRef) const { - if (!objWeakRef) - std::terminate(); - return new ChakraWeakRefValue(objWeakRef); -} - void ChakraRuntime::setupMemoryTracker() noexcept { if (runtimeArgs().memoryTracker) { size_t initialMemoryUsage = 0; @@ -1167,7 +1002,8 @@ void ChakraRuntime::setupMemoryTracker() noexcept { [](void *callbackState, JsMemoryEventType allocationEvent, size_t allocationSize) -> bool { - auto memoryTrackerPtr = static_cast(callbackState); + auto memoryTrackerPtr = + static_cast(callbackState); switch (allocationEvent) { case JsMemoryAllocate: memoryTrackerPtr->OnAllocation(allocationSize); @@ -1230,12 +1066,12 @@ JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( } catch (const std::exception &ex) { std::string exwhat(ex.what()); JsValueRef exn; - exn = createJSString(exwhat.c_str(), exwhat.size()); + exn = ToJsString(exwhat.c_str(), exwhat.size()); JsSetException(exn); } catch (...) { std::string exceptionString("Exception in HostFunction: "); JsValueRef exn; - exn = createJSString(exceptionString.c_str(), exceptionString.size()); + exn = ToJsString(exceptionString.c_str(), exceptionString.size()); JsSetException(exn); } diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index cd2d7e86f86..4ddc47dfa29 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -3,7 +3,6 @@ #pragma once -#include "ByteArrayBuffer.h" #include "ChakraObjectRef.h" #include "ChakraRuntimeArgs.h" @@ -194,12 +193,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { size_t count); facebook::jsi::Value ToJsiValue(ChakraObjectRef &&ref); - // TODO (yicyao): - // JsValueRef->JSValue (needs make.*Value so it must be member function) - facebook::jsi::Value createValue(JsValueRef value) const; - // Value->JsValueRef (similar to above) - JsValueRef valueRef(const facebook::jsi::Value &value); - protected: ChakraRuntimeArgs &runtimeArgs() { return m_args; @@ -337,20 +330,8 @@ class ChakraRuntime : public facebook::jsi::Runtime { template friend class ObjectWithExternalData; - // TODO (yicyao) - inline void checkException(JsErrorCode res); - inline void checkException(JsErrorCode res, const char *msg); void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); - // TODO (yicyao) - // static JsWeakRef newWeakObjectRef(const facebook::jsi::Object &obj); - // static JsValueRef strongObjectRef(const facebook::jsi::WeakObject &obj); - - // TODO (yicyao) - // Factory methods for creating String/Object - // facebook::jsi::String createString(JsValueRef stringRef) const; - // facebook::jsi::PropNameID createPropNameID(JsValueRef stringRef); - template facebook::jsi::Object createObject(JsValueRef objectRef, T *externalData) const; @@ -364,15 +345,11 @@ class ChakraRuntime : public facebook::jsi::Runtime { // TODO (yicyao) // Used by factory methods and clone methods - facebook::jsi::Runtime::PointerValue *makeStringValue(JsValueRef str) const; template facebook::jsi::Runtime::PointerValue *makeObjectValue( JsValueRef obj, T *externaldata) const; facebook::jsi::Runtime::PointerValue *makeObjectValue(JsValueRef obj) const; - facebook::jsi::Runtime::PointerValue *makePropertyIdValue( - JsPropertyIdRef propId) const; - facebook::jsi::Runtime::PointerValue *makeWeakRefValue(JsWeakRef obj) const; // Promise Helpers static void CALLBACK From 9ff342cb862ced38e690d4e620a1b6ec33d27004 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 16:15:22 -0700 Subject: [PATCH 06/16] Fix Desktop build. --- vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp | 3 +- vnext/JSI/Shared/ChakraObjectRef.cpp | 24 ++- vnext/JSI/Shared/ChakraObjectRef.h | 44 +++-- vnext/JSI/Shared/ChakraRuntime.cpp | 200 ++++++++------------ vnext/JSI/Shared/ChakraRuntime.h | 95 +++++----- 5 files changed, 180 insertions(+), 186 deletions(-) diff --git a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp index 30cb431c3fb..f954d3aa43c 100644 --- a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp +++ b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp @@ -37,7 +37,8 @@ struct FileVersionInfoResource { } // namespace // TODO (yicyao): We temporarily removed weak reference semantics from -// ChakraCore based jsi::Runtime. +// ChakraCore based jsi::Runtime. Renable these when we switch to class +// hierarchy. // JsWeakRef ChakraRuntime::newWeakObjectRef(const facebook::jsi::Object &obj) { // JsWeakRef weakRef; diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index b3cfc740c7e..d485c0a45b5 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -154,14 +154,17 @@ ChakraObjectRef GetPropertySymbol(const ChakraObjectRef &id) { return ChakraObjectRef{symbol}; } -ChakraObjectRef GetPropertyId(const char *const utf8, size_t length){ +ChakraObjectRef GetPropertyId(const char *const utf8, size_t length) { + assert(utf8); + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion - // using ChakraCore's JsCreateString API. + // using ChakraCore's JsCreatePropertyId API. #ifdef CHAKRACORE JsPropertyIdRef id = nullptr; ThrowUponChakraError( JsCreatePropertyId(utf8, length, &id), "JsCreatePropertyId"); return ChakraObjectRef(id); + #else std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8); return GetPropertyId(utf16); @@ -177,7 +180,7 @@ ChakraObjectRef GetPropertyId(const std::wstring &utf16) { std::string ToStdString(const ChakraObjectRef &jsString) { // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion - // using ChakraCore's JsCreateString API. + // using ChakraCore's JsCopyString API. #ifdef CHAKRACORE size_t length = 0; ThrowUponChakraError( @@ -188,6 +191,7 @@ std::string ToStdString(const ChakraObjectRef &jsString) { "JsCopyString"); assert(length == result.length()); return result; + #else return Common::Unicode::Utf16ToUtf8(ToStdWstring(jsString)); #endif @@ -205,12 +209,15 @@ std::wstring ToStdWstring(const ChakraObjectRef &jsString) { } ChakraObjectRef ToJsString(const char *utf8, size_t length) { + assert(utf8); + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCreateString API. #ifdef CHAKRACORE JsValueRef result = nullptr; ThrowUponChakraError(JsCreateString(utf8, length, &result), "JsCreateString"); return ChakraObjectRef(result); + #else std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); return ToJsString(utf16.c_str(), utf16.length()); @@ -218,6 +225,7 @@ ChakraObjectRef ToJsString(const char *utf8, size_t length) { } ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length) { + assert(utf16); JsValueRef result = nullptr; ThrowUponChakraError( JsPointerToString(utf16, length, &result), "JsPointerToString"); @@ -237,13 +245,15 @@ ChakraObjectRef ToJsNumber(int num) { return ChakraObjectRef(result); } -ChakraObjectRef ToJsExternalArrayBuffer( +ChakraObjectRef ToJsArrayBuffer( const std::shared_ptr &buffer) { + assert(buffer); + size_t size = buffer->size(); assert(size < UINT_MAX); JsValueRef arrayBuffer = nullptr; - auto bufferRef = + auto bufferWrapper = std::make_unique>(buffer); // We allocate a copy of buffer on the heap, a shared_ptr which is deleted @@ -263,11 +273,11 @@ ChakraObjectRef ToJsExternalArrayBuffer( static_cast *>( bufferToDestroy)}; }, - bufferRef.get(), + bufferWrapper.get(), &arrayBuffer), "JsCreateExternalArrayBuffer"); - bufferRef.release(); + bufferWrapper.release(); return ChakraObjectRef(arrayBuffer); } diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 82403ed91fc..99d47b066c2 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -82,29 +82,47 @@ ChakraObjectRef GetPropertyId(const char *const utf8, size_t length); ChakraObjectRef GetPropertyId(const std::wstring &utf16); -// jsString must be managing a JsValueRef pointing to a JS string. The returned -// std::string/std::wstring is UTF-8/UTF-16 encoded. These functions copy the JS -// string buffer into the returned std::string/std::wstring. +// jsString must be managing a JS string. The returned std::string/std::wstring +// is UTF-8/UTF-16 encoded. These functions copy the JS string buffer into the +// returned std::string/std::wstring. std::string ToStdString(const ChakraObjectRef &jsString); std::wstring ToStdWstring(const ChakraObjectRef &jsString); -// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS string. -// utf8 and utf16 do not have to be null terminated and are copied to JS engine -// owned memory. +// Returns a ChakraObjectRef managing a JS string. utf8 and utf16 do not have to +// be null terminated and are copied to JS engine owned memory. ChakraObjectRef ToJsString(const char *const utf8, size_t length); ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length); -// jsValue must be mananing a JsValueRef. Returns a ChakraObjectRef managing a -// JsValueRef that points the return value of the JS .toString function. +// jsValue must be mananing a JsValueRef. Returns a ChakraObjectRef managing the +// return value of the JS .toString function. ChakraObjectRef ToJsString(const ChakraObjectRef &jsValue); -// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS number. +// Returns a ChakraObjectRef managing a JS number. ChakraObjectRef ToJsNumber(int num); -// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS ArrayBuffer. -// This ArrayBuffer is backed by buffer and keeps buffer alive till the garbage -// collector finalizes it. -ChakraObjectRef ToJsExternalArrayBuffer( +// Returns a ChakraObjectRef managing a JS Object. This Object is backed by data +// and keeps data alive till the garbage collector finalizes it. +template +ChakraObjectRef ToJsObject(std::unique_ptr &&data) { + assert(data); + + JsValueRef obj = nullptr; + ThrowUponChakraError(JsCreateExternalObject( + data.get(), + [](void *dataToDestroy) { + // We wrap dataToDestroy in a unique_ptr to avoid calling delete + // explicitly. + std::unique_ptr wrapper{static_cast(dataToDestroy)}; + }, + &obj)); + + data.release(); + return ChakraObjectRef(obj); +} + +// Returns a ChakraObjectRef managing a JS ArrayBuffer. This ArrayBuffer is +// backed by buffer and keeps buffer alive till the GC finalizes it. +ChakraObjectRef ToJsArrayBuffer( const std::shared_ptr &buffer); // Both jsValue1 and jsValue2 must be managing a JsValueRef. Returns whether diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index a88642e947e..4140122e91c 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -191,9 +191,9 @@ facebook::jsi::Value ChakraRuntime::evaluatePreparedJavaScript( } facebook::jsi::Object ChakraRuntime::global() { - JsValueRef value; - JsGetGlobalObject(&value); - return createObject(value); + JsValueRef global; + ThrowUponJsError(JsGetGlobalObject(&global), "JsGetGlobalObject"); + return MakePointer(global); } std::string ChakraRuntime::description() { @@ -286,9 +286,9 @@ std::string ChakraRuntime::utf8(const facebook::jsi::String &str) { } facebook::jsi::Object ChakraRuntime::createObject() { - JsValueRef obj = nullptr; + JsValueRef obj; ThrowUponJsError(JsCreateObject(&obj), "JsCreateObject"); - return MakePointer(ChakraObjectRef(obj)); + return MakePointer(obj); } // TODO (yicyao) @@ -296,7 +296,7 @@ facebook::jsi::Object ChakraRuntime::createObject( std::shared_ptr hostObject) { facebook::jsi::Object proxyTarget = ObjectWithExternalData::create( - *this, new HostObjectProxy(*this, hostObject)); + *this, std::make_unique(*this, hostObject)); return createProxy(std::move(proxyTarget), createHostObjectProxyHandler()); } @@ -476,8 +476,7 @@ facebook::jsi::Array ChakraRuntime::createArray(size_t length) { JsCreateArray(static_cast(length), &result), "JsCreateArray"); - return MakePointer(ChakraObjectRef(result)) - .asArray(*this); + return MakePointer(result).asArray(*this); } size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { @@ -568,7 +567,7 @@ facebook::jsi::Function ChakraRuntime::createFunctionFromHostFunction( [](JsRef ref, void *hostFuncProxy) { delete hostFuncProxy; }), "JsSetObjectBeforeCollectCallback"); - return createObject(funcRef).getFunction(*this); + return MakePointer(funcRef).getFunction(*this); } facebook::jsi::Value ChakraRuntime::call( @@ -766,29 +765,6 @@ facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { std::terminate(); } -template -/*static */ facebook::jsi::Object ChakraRuntime::ObjectWithExternalData< - T>::create(ChakraRuntime &rt, T *externalData) { - return rt.createObject(static_cast(nullptr), externalData); -} - -template -/*static */ ChakraRuntime::ObjectWithExternalData -ChakraRuntime::ObjectWithExternalData::fromExisting( - ChakraRuntime &rt, - facebook::jsi::Object &&obj) { - return ObjectWithExternalData(rt.cloneObject(getPointerValue(obj))); -} - -template -T *ChakraRuntime::ObjectWithExternalData::getExternalData() { - T *externalData; - JsGetExternalData( - static_cast(getPointerValue(*this))->m_obj, - reinterpret_cast(&externalData)); - return externalData; -} - void ChakraRuntime::ThrowUponJsError( JsErrorCode error, const char *const chakraApiName) { @@ -818,15 +794,33 @@ void ChakraRuntime::ThrowUponJsError( std::terminate(); } -template -facebook::jsi::Object ChakraRuntime::createObject( - JsValueRef objectRef, - T *externalData) const { - return make(makeObjectValue(objectRef, externalData)); +// clang-format off +template +/* static */ facebook::jsi::Object +ChakraRuntime::ObjectWithExternalData::create( + ChakraRuntime &runtime, + std::unique_ptr &&externalData) { + return runtime.MakePointer>( + ToJsObject(std::move(externalData))); +} + +template +/* static */ ChakraRuntime::ObjectWithExternalData +ChakraRuntime::ObjectWithExternalData::fromExisting( + ChakraRuntime &runtime, + facebook::jsi::Object &&obj) { + return ObjectWithExternalData(runtime.cloneObject(getPointerValue(obj))); } +// clang-format on -facebook::jsi::Object ChakraRuntime::createObject(JsValueRef obj) const { - return make(makeObjectValue(obj)); +template +T *ChakraRuntime::ObjectWithExternalData::getExternalData() { + T *externalData; + ThrowUponChakraError( + JsGetExternalData( + GetChakraObjectRef(*this), reinterpret_cast(&externalData)), + "JsGetExternalData"); + return externalData; } facebook::jsi::Object ChakraRuntime::createProxy( @@ -953,76 +947,6 @@ facebook::jsi::Object ChakraRuntime::createHostObjectProxyHandler() noexcept { return handlerObj; } -facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeObjectValue( - JsValueRef objectRef) const { - if (!objectRef) { - JsCreateObject(&objectRef); - } - - ChakraObjectValue *chakraObjValue = new ChakraObjectValue(objectRef); - return chakraObjValue; -} - -template -facebook::jsi::Runtime::PointerValue *ChakraRuntime::makeObjectValue( - JsValueRef objectRef, - T *externaldata) const { - if (!externaldata) { - return makeObjectValue(objectRef); - } - - // Note :: We explicitly delete the external data proxy when the JS value is - // finalized. The proxy is expected to do the right thing in destructor, for - // e.g. decrease the ref count of a shared resource. - if (!objectRef) { - JsCreateExternalObject( - externaldata, [](void *data) { delete data; }, &objectRef); - } else { - JsSetExternalData( - objectRef, externaldata); // TODO : Is there an API to listen to - // finalization of arbitrary objects ? - } - - ChakraObjectValue *chakraObjValue = new ChakraObjectValue(objectRef); - return chakraObjValue; -} - -void ChakraRuntime::setupMemoryTracker() noexcept { - if (runtimeArgs().memoryTracker) { - size_t initialMemoryUsage = 0; - JsGetRuntimeMemoryUsage(m_runtime, &initialMemoryUsage); - runtimeArgs().memoryTracker->Initialize(initialMemoryUsage); - - if (runtimeArgs().runtimeMemoryLimit > 0) - JsSetRuntimeMemoryLimit(m_runtime, runtimeArgs().runtimeMemoryLimit); - - JsSetRuntimeMemoryAllocationCallback( - m_runtime, - runtimeArgs().memoryTracker.get(), - [](void *callbackState, - JsMemoryEventType allocationEvent, - size_t allocationSize) -> bool { - auto memoryTrackerPtr = - static_cast(callbackState); - switch (allocationEvent) { - case JsMemoryAllocate: - memoryTrackerPtr->OnAllocation(allocationSize); - break; - - case JsMemoryFree: - memoryTrackerPtr->OnDeallocation(allocationSize); - break; - - case JsMemoryFailure: - default: - break; - } - - return true; - }); - } -} - JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( JsValueRef callee, bool isConstructCall, @@ -1042,32 +966,40 @@ JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( if (argumentCount > maxStackArgCount) { heapArgs = std::make_unique(argumentCount); + for (size_t i = 1; i < argumentCountIncThis; i++) { - heapArgs[i - 1] = - hostFuncProxy.getRuntime().createValue(argumentsIncThis[i]); + heapArgs[i - 1] = hostFuncProxy.getRuntime().ToJsiValue( + ChakraObjectRef(argumentsIncThis[i])); } args = heapArgs.get(); + } else { for (size_t i = 1; i < argumentCountIncThis; i++) { - stackArgs[i - 1] = - hostFuncProxy.getRuntime().createValue(argumentsIncThis[i]); + stackArgs[i - 1] = hostFuncProxy.getRuntime().ToJsiValue( + ChakraObjectRef(argumentsIncThis[i])); } args = stackArgs; } + JsValueRef res{JS_INVALID_REFERENCE}; facebook::jsi::Value thisVal( - hostFuncProxy.getRuntime().createObject(argumentsIncThis[0])); + hostFuncProxy.getRuntime().MakePointer( + argumentsIncThis[0])); + try { facebook::jsi::Value retVal = hostFuncProxy.getHostFunction()( hostFuncProxy.getRuntime(), thisVal, args, argumentCount); - res = hostFuncProxy.getRuntime().valueRef(retVal); + res = hostFuncProxy.getRuntime().ToChakraObjectRef(retVal); + } catch (const facebook::jsi::JSError &error) { - JsSetException(hostFuncProxy.getRuntime().valueRef(error.value())); + JsSetException(hostFuncProxy.getRuntime().ToChakraObjectRef(error.value())); + } catch (const std::exception &ex) { std::string exwhat(ex.what()); JsValueRef exn; exn = ToJsString(exwhat.c_str(), exwhat.size()); JsSetException(exn); + } catch (...) { std::string exceptionString("Exception in HostFunction: "); JsValueRef exn; @@ -1078,6 +1010,42 @@ JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( return res; } +void ChakraRuntime::setupMemoryTracker() noexcept { + if (runtimeArgs().memoryTracker) { + size_t initialMemoryUsage = 0; + JsGetRuntimeMemoryUsage(m_runtime, &initialMemoryUsage); + runtimeArgs().memoryTracker->Initialize(initialMemoryUsage); + + if (runtimeArgs().runtimeMemoryLimit > 0) + JsSetRuntimeMemoryLimit(m_runtime, runtimeArgs().runtimeMemoryLimit); + + JsSetRuntimeMemoryAllocationCallback( + m_runtime, + runtimeArgs().memoryTracker.get(), + [](void *callbackState, + JsMemoryEventType allocationEvent, + size_t allocationSize) -> bool { + auto memoryTrackerPtr = + static_cast(callbackState); + switch (allocationEvent) { + case JsMemoryAllocate: + memoryTrackerPtr->OnAllocation(allocationSize); + break; + + case JsMemoryFree: + memoryTrackerPtr->OnDeallocation(allocationSize); + break; + + case JsMemoryFailure: + default: + break; + } + + return true; + }); + } +} + /*static*/ std::once_flag ChakraRuntime::s_runtimeVersionInitFlag; /*static*/ uint64_t ChakraRuntime::s_runtimeVersion = 0; diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 4ddc47dfa29..4f36033d748 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -199,6 +199,8 @@ class ChakraRuntime : public facebook::jsi::Runtime { } private: + void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); + // ChakraPointerValue is needed for working with Facebook's jsi::Pointer class // and must only be used for this purpose. Every instance of // ChakraPointerValue should be allocated on the heap and be used as an @@ -257,6 +259,11 @@ class ChakraRuntime : public facebook::jsi::Runtime { using ChakraPointerValue = ChakraPointerValueTemplate; + template + inline T MakePointer(JsValueRef ref) { + return MakePointer(ChakraObjectRef(ref)); + } + template inline T MakePointer(ChakraObjectRef &&ref) { static_assert( @@ -280,76 +287,72 @@ class ChakraRuntime : public facebook::jsi::Runtime { ->GetRef(); } + // HostObject and HostFunction helpers + template + class ObjectWithExternalData : public facebook::jsi::Object { + public: + static facebook::jsi::Object create( + ChakraRuntime &runtime, + std::unique_ptr &&externalData); + + static ObjectWithExternalData fromExisting( + ChakraRuntime &runtime, + facebook::jsi::Object &&obj); + + public: + T *getExternalData(); + ObjectWithExternalData(const Runtime::PointerValue *value) + : Object(const_cast(value)) { + } // TODO :: const_cast + + ObjectWithExternalData(ObjectWithExternalData &&other) = default; + ObjectWithExternalData &operator=(ObjectWithExternalData &&other) = default; + }; + + template + friend class ObjectWithExternalData; + class HostObjectProxy { public: facebook::jsi::Value Get(const facebook::jsi::PropNameID &propNameId) { - return hostObject_->get(runtime_, propNameId); + return m_hostObject->get(m_runtime, propNameId); } void Set( const facebook::jsi::PropNameID &propNameId, const facebook::jsi::Value &value) { - hostObject_->set(runtime_, propNameId, value); + m_hostObject->set(m_runtime, propNameId, value); } std::vector Enumerator() { - return hostObject_->getPropertyNames(runtime_); + return m_hostObject->getPropertyNames(m_runtime); } HostObjectProxy( ChakraRuntime &rt, const std::shared_ptr &hostObject) - : runtime_(rt), hostObject_(hostObject) {} + : m_runtime(rt), m_hostObject(hostObject) {} std::shared_ptr getHostObject() { - return hostObject_; + return m_hostObject; } private: - ChakraRuntime &runtime_; - std::shared_ptr hostObject_; - }; - - template - class ObjectWithExternalData : public facebook::jsi::Object { - public: - static facebook::jsi::Object create(ChakraRuntime &rt, T *externalData); - static ObjectWithExternalData fromExisting( - ChakraRuntime &rt, - facebook::jsi::Object &&obj); - - public: - T *getExternalData(); - ObjectWithExternalData(const Runtime::PointerValue *value) - : Object(const_cast(value)) { - } // TODO :: const_cast - - ObjectWithExternalData(ObjectWithExternalData &&) = default; - ObjectWithExternalData &operator=(ObjectWithExternalData &&) = default; + ChakraRuntime &m_runtime; + std::shared_ptr m_hostObject; }; - template - friend class ObjectWithExternalData; - - void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); - - template - facebook::jsi::Object createObject(JsValueRef objectRef, T *externalData) - const; - facebook::jsi::Object createObject(JsValueRef objectRef) const; - facebook::jsi::Object createProxy( facebook::jsi::Object &&target, facebook::jsi::Object &&handler) noexcept; facebook::jsi::Function createProxyConstructor() noexcept; facebook::jsi::Object createHostObjectProxyHandler() noexcept; - // TODO (yicyao) - // Used by factory methods and clone methods - template - facebook::jsi::Runtime::PointerValue *makeObjectValue( - JsValueRef obj, - T *externaldata) const; - facebook::jsi::Runtime::PointerValue *makeObjectValue(JsValueRef obj) const; + static JsValueRef CALLBACK HostFunctionCall( + JsValueRef callee, + bool isConstructCall, + JsValueRef *argumentsIncThis, + unsigned short argumentCountIncThis, + void *callbackState); // Promise Helpers static void CALLBACK @@ -390,7 +393,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { return s_runtimeVersion; } - // Miscellaneous + // JavaScript evaluation helpers std::unique_ptr generatePreparedScript( const std::string &sourceURL, const facebook::jsi::Buffer &sourceBuffer) noexcept; @@ -401,12 +404,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { const facebook::jsi::Buffer &scriptBuffer, const facebook::jsi::Buffer &serializedScriptBuffer, const std::string &sourceURL); - static JsValueRef CALLBACK HostFunctionCall( - JsValueRef callee, - bool isConstructCall, - JsValueRef *argumentsIncThis, - unsigned short argumentCountIncThis, - void *callbackState); static std::once_flag s_runtimeVersionInitFlag; static uint64_t s_runtimeVersion; From ca60dd1e11e5c54735c6954e24c534c50d5ca975 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 17:29:51 -0700 Subject: [PATCH 07/16] Bug fixes. --- .../JsiRuntimeUnitTests.cpp | 19 ++++++------------- vnext/JSI/Shared/ChakraObjectRef.cpp | 17 +++++++++++------ vnext/JSI/Shared/ChakraObjectRef.h | 6 ++++-- vnext/JSI/Shared/ChakraRuntime.cpp | 12 ++++-------- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp index 2d3344d5040..b6c44889d95 100644 --- a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp +++ b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp @@ -30,7 +30,7 @@ TEST_P(JsiRuntimeUnitTests, RuntimeTest) { // TODO (yicyao) #2703: Currently, comparison of property IDs is broken for // ChakraRuntime. Enable this test once we fix it. -TEST_P(JsiRuntimeUnitTests, DISABLED_PropNameIDTest) { +TEST_P(JsiRuntimeUnitTests, PropNameIDTest) { // This is a little weird to test, because it doesn't really exist // in JS yet. All I can do is create them, compare them, and // receive one as an argument to a HostObject. @@ -926,9 +926,6 @@ TEST_P(JsiRuntimeUnitTests, DISABLED_ExceptionStackTraceTest) { EXPECT_NE(stack.find("world"), std::string::npos); } -// TODO (T28293178) Remove this once exceptions are supported in all builds. -#ifndef JSI_NO_EXCEPTION_TESTS - namespace { unsigned countOccurences(const std::string &of, const std::string &in) { @@ -944,7 +941,7 @@ unsigned countOccurences(const std::string &of, const std::string &in) { } // namespace TEST_P(JsiRuntimeUnitTests, JSErrorsArePropagatedNicely) { - unsigned callsBeoreError = 5; + unsigned callsBeforeError = 5; Function sometimesThrows = function( "function sometimesThrows(shouldThrow, callback) {" @@ -958,25 +955,21 @@ TEST_P(JsiRuntimeUnitTests, JSErrorsArePropagatedNicely) { rt, PropNameID::forAscii(rt, "callback"), 0, - [&sometimesThrows, &callsBeoreError]( + [&sometimesThrows, &callsBeforeError]( Runtime &rt, const Value &thisVal, const Value *args, size_t count) { - return sometimesThrows.call(rt, --callsBeoreError == 0, args[0]); + return sometimesThrows.call(rt, --callsBeforeError == 0, args[0]); }); try { sometimesThrows.call(rt, false, callback); } catch (JSError &error) { - // TODO (yicyao): Need to fix getMessage(). - // EXPECT_EQ(error.getMessage(), "Omg, what a nasty exception"); - - // TODO (yicyao): Need to fix getStack(). - // EXPECT_EQ(countOccurences("sometimesThrows", error.getStack()), 6); + EXPECT_EQ(error.getMessage(), "Omg, what a nasty exception"); + EXPECT_EQ(countOccurences("sometimesThrows", error.getStack()), 6); // system JSC JSI does not implement host function names // EXPECT_EQ(countOccurences("callback", error.getStack(rt)), 5); } } -#endif TEST_P(JsiRuntimeUnitTests, JSErrorsCanBeConstructedWithStack) { auto err = JSError(rt, "message", "stack"); diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index d485c0a45b5..fa7217a9c97 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -36,8 +36,8 @@ void ThrowUponChakraError(JsErrorCode error, const char *const chakraApiName) { } } -ChakraObjectRef::ChakraObjectRef(const ChakraObjectRef &other) noexcept - : m_ref{other.m_ref}, m_state{other.m_state} { +ChakraObjectRef::ChakraObjectRef(const ChakraObjectRef &original) noexcept + : m_ref{original.m_ref}, m_state{original.m_state} { if (m_state == State::Initialized) { assert(m_ref); CheckedJsAddRef(m_ref); @@ -46,19 +46,19 @@ ChakraObjectRef::ChakraObjectRef(const ChakraObjectRef &other) noexcept } } -ChakraObjectRef::ChakraObjectRef(ChakraObjectRef &&other) noexcept { - std::swap(*this, other); +ChakraObjectRef::ChakraObjectRef(ChakraObjectRef &&original) noexcept { + swap(original); } ChakraObjectRef &ChakraObjectRef::operator=( const ChakraObjectRef &rhs) noexcept { ChakraObjectRef rhsCopy(rhs); - std::swap(*this, rhsCopy); + swap(rhsCopy); return *this; } ChakraObjectRef &ChakraObjectRef::operator=(ChakraObjectRef &&rhs) noexcept { - std::swap(*this, rhs); + swap(rhs); return *this; } @@ -119,6 +119,11 @@ void ChakraObjectRef::Invalidate() { } } +void ChakraObjectRef::swap(ChakraObjectRef &other) { + std::swap(m_ref, other.m_ref); + std::swap(m_state, other.m_state); +} + JsValueType GetValueType(const ChakraObjectRef &jsValue) { JsValueType type; ThrowUponChakraError(JsGetValueType(jsValue, &type), "JsGetValueType"); diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 99d47b066c2..00874e1e1b6 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -42,8 +42,8 @@ class ChakraObjectRef { Initialize(ref); } - ChakraObjectRef(const ChakraObjectRef &other) noexcept; - ChakraObjectRef(ChakraObjectRef &&other) noexcept; + ChakraObjectRef(const ChakraObjectRef &original) noexcept; + ChakraObjectRef(ChakraObjectRef &&original) noexcept; ChakraObjectRef &operator=(const ChakraObjectRef &rhs) noexcept; ChakraObjectRef &operator=(ChakraObjectRef &&rhs) noexcept; @@ -58,6 +58,8 @@ class ChakraObjectRef { } private: + void swap(ChakraObjectRef &other); + enum class State { Uninitialized, Initialized, Invalidated }; JsRef m_ref = nullptr; diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 4140122e91c..ec51037a471 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -429,15 +429,11 @@ facebook::jsi::Array ChakraRuntime::getPropertyNames( " }\n" "})()"; - // TODO (yicyao): Comment on why this variable is not static. - std::shared_ptr jsGetPropertyNamesSourceBuffer = - std::make_shared(jsGetPropertyNamesSource); - - // TODO (yicyao): Need some refactoring here to ensure lifetime correctness, - // reduce duplicate code, and improve efficiency. + static facebook::jsi::StringBuffer jsGetPropertyNamesSourceBuffer{ + jsGetPropertyNamesSource}; facebook::jsi::Function jsGetPropertyNames = - evaluateJavaScript(jsGetPropertyNamesSourceBuffer, "") + evaluateJavaScriptSimple(jsGetPropertyNamesSourceBuffer, "") .asObject(*this) .asFunction(*this); @@ -585,7 +581,7 @@ facebook::jsi::Value ChakraRuntime::call( assert(argsWithThis.size() <= USHRT_MAX); JsValueRef result; - ThrowUponChakraError( + ThrowUponJsError( JsCallFunction( GetChakraObjectRef(func), argsWithThis.data(), From af2be74461b33fa6fdcafbb53d4f4388b16a98d6 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 17:48:10 -0700 Subject: [PATCH 08/16] Fix Universal build. --- vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp | 2 +- .../Universal/ChakraJsiRuntime_edgemode.cpp | 46 +++++-------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp index f954d3aa43c..5ed1653dfac 100644 --- a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp +++ b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp @@ -373,7 +373,7 @@ bool ChakraRuntime::evaluateSerializedScript( } else if (result == JsErrorBadSerializedScript) { return false; } else { - ThrowUponChakraError(result); + ThrowUponChakraError(result, "JsRunSerialized"); } } diff --git a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp index 2251a9b6b5f..e6603ebece6 100644 --- a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp +++ b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp @@ -2,6 +2,8 @@ // Licensed under the MIT License. #include "ChakraRuntime.h" + +#include "ByteArrayBuffer.h" #include "Unicode.h" #if !defined(CHAKRACORE) @@ -9,32 +11,6 @@ namespace Microsoft::JSI { -JsWeakRef ChakraRuntime::newWeakObjectRef(const facebook::jsi::Object &obj) { - return objectRef(obj); -} - -JsValueRef ChakraRuntime::strongObjectRef( - const facebook::jsi::WeakObject &obj) { - return objectRef(obj); // Return the original strong ref. -} - -JsValueRef ChakraRuntime::createJSString(const char *data, size_t length) { - const std::wstring script16 = Microsoft::Common::Unicode::Utf8ToUtf16( - reinterpret_cast(data), length); - JsValueRef value; - JsPointerToString(script16.c_str(), script16.size(), &value); - return value; -} - -JsValueRef ChakraRuntime::createJSPropertyId(const char *data, size_t length) { - JsValueRef propIdRef; - const std::wstring name16 = Microsoft::Common::Unicode::Utf8ToUtf16( - reinterpret_cast(data), length); - if (JsNoError != JsGetPropertyIdFromName(name16.c_str(), &propIdRef)) - std::terminate(); - return propIdRef; -} - void ChakraRuntime::setupNativePromiseContinuation() noexcept { // NOP } @@ -87,13 +63,15 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScriptSimple( throw facebook::jsi::JSINativeException("Script URL can't be empty."); JsValueRef result; - checkException(JsRunScript( - script16.c_str(), - JS_SOURCE_CONTEXT_NONE /*sourceContext*/, - url16.c_str(), - &result)); - - return createValue(result); + ThrowUponJsError( + JsRunScript( + script16.c_str(), + JS_SOURCE_CONTEXT_NONE /*sourceContext*/, + url16.c_str(), + &result), + "JsRunScript"); + + return ToJsiValue(ChakraObjectRef(result)); } // TODO :: Return result @@ -119,7 +97,7 @@ bool ChakraRuntime::evaluateSerializedScript( } else if (ret == JsErrorBadSerializedScript) { return false; } else { - checkException(ret); + ThrowUponChakraError(ret, "JsRunSerializedScript"); return true; } } From 7b8cdac9aa90b5a79b8b7a7303cff04db40da3b2 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 17:50:01 -0700 Subject: [PATCH 09/16] Change files --- ...tive-windows-2019-10-16-17-50-01-CharkaObjectRef.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 change/react-native-windows-2019-10-16-17-50-01-CharkaObjectRef.json diff --git a/change/react-native-windows-2019-10-16-17-50-01-CharkaObjectRef.json b/change/react-native-windows-2019-10-16-17-50-01-CharkaObjectRef.json new file mode 100644 index 00000000000..1dd624cd858 --- /dev/null +++ b/change/react-native-windows-2019-10-16-17-50-01-CharkaObjectRef.json @@ -0,0 +1,9 @@ +{ + "type": "none", + "comment": "Introduce ChakraPointerValue and reimplement many Runtime functions with it.", + "packageName": "react-native-windows", + "email": "yicyao@microsoft.com", + "commit": "af2be74461b33fa6fdcafbb53d4f4388b16a98d6", + "date": "2019-10-17T00:50:01.129Z", + "file": "D:\\Git\\hansenyy-react-native-windows-3\\change\\react-native-windows-2019-10-16-17-50-01-CharkaObjectRef.json" +} \ No newline at end of file From de930cce72b87e6adaf2ddfe7b1e001dee5a9997 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 17:59:25 -0700 Subject: [PATCH 10/16] Remove some useless comments --- vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp | 2 -- vnext/JSI/Shared/ChakraRuntime.cpp | 5 ----- 2 files changed, 7 deletions(-) diff --git a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp index b6c44889d95..6cc5826fdf7 100644 --- a/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp +++ b/vnext/JSI.Desktop.UnitTests/JsiRuntimeUnitTests.cpp @@ -28,8 +28,6 @@ TEST_P(JsiRuntimeUnitTests, RuntimeTest) { EXPECT_EQ(rt.global().getProperty(rt, "x").getNumber(), 1); } -// TODO (yicyao) #2703: Currently, comparison of property IDs is broken for -// ChakraRuntime. Enable this test once we fix it. TEST_P(JsiRuntimeUnitTests, PropNameIDTest) { // This is a little weird to test, because it doesn't really exist // in JS yet. All I can do is create them, compare them, and diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index ec51037a471..e826469dbca 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -291,7 +291,6 @@ facebook::jsi::Object ChakraRuntime::createObject() { return MakePointer(obj); } -// TODO (yicyao) facebook::jsi::Object ChakraRuntime::createObject( std::shared_ptr hostObject) { facebook::jsi::Object proxyTarget = @@ -300,7 +299,6 @@ facebook::jsi::Object ChakraRuntime::createObject( return createProxy(std::move(proxyTarget), createHostObjectProxyHandler()); } -// TODO (yicyao) std::shared_ptr ChakraRuntime::getHostObject( const facebook::jsi::Object &obj) { if (!isHostObject(obj)) @@ -323,7 +321,6 @@ std::shared_ptr ChakraRuntime::getHostObject( return externalData->getHostObject(); } -// TODO (yicyao) facebook::jsi::HostFunctionType &ChakraRuntime::getHostFunction( const facebook::jsi::Function &obj) { throw std::runtime_error( @@ -398,7 +395,6 @@ bool ChakraRuntime::isFunction(const facebook::jsi::Object &obj) const { return GetValueType(GetChakraObjectRef(obj)) == JsFunction; } -// TODO (yicyao) bool ChakraRuntime::isHostObject(const facebook::jsi::Object &obj) const { facebook::jsi::Value val = obj.getProperty( const_cast(*this), s_proxyIsHostObjectPropName); @@ -408,7 +404,6 @@ bool ChakraRuntime::isHostObject(const facebook::jsi::Object &obj) const { return false; } -// TODO (yicyao) bool ChakraRuntime::isHostFunction(const facebook::jsi::Function &obj) const { throw std::runtime_error("ChakraRuntime::isHostFunction is not implemented."); } From 8b9c4d6cb28022e90fc0f64f2698af06f5c3d1e4 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Wed, 16 Oct 2019 18:19:48 -0700 Subject: [PATCH 11/16] Revert some reordering for more reviewable diff. --- vnext/JSI/Shared/ChakraRuntime.cpp | 258 ++++++++++++++--------------- vnext/JSI/Shared/ChakraRuntime.h | 70 ++++---- 2 files changed, 163 insertions(+), 165 deletions(-) diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index e826469dbca..0137ada7936 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -481,18 +481,6 @@ size_t ChakraRuntime::size(const facebook::jsi::Array &arr) { return static_cast(getProperty(arr, propId).asNumber()); } -size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { - assert(isArrayBuffer(arrBuf)); - - constexpr const uint8_t propName[] = { - 'b', 'y', 't', 'e', 'l', 'e', 'n', 'g', 't', 'h'}; - - facebook::jsi::PropNameID propId = createPropNameIDFromUtf8( - propName, Common::Utilities::ArraySize(propName)); - - return static_cast(getProperty(arrBuf, propId).asNumber()); -} - uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { assert(isArrayBuffer(arrBuf)); @@ -506,6 +494,18 @@ uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { return buffer; } +size_t ChakraRuntime::size(const facebook::jsi::ArrayBuffer &arrBuf) { + assert(isArrayBuffer(arrBuf)); + + constexpr const uint8_t propName[] = { + 'b', 'y', 't', 'e', 'l', 'e', 'n', 'g', 't', 'h'}; + + facebook::jsi::PropNameID propId = createPropNameIDFromUtf8( + propName, Common::Utilities::ArraySize(propName)); + + return static_cast(getProperty(arrBuf, propId).asNumber()); +} + facebook::jsi::Value ChakraRuntime::getValueAtIndex( const facebook::jsi::Array &arr, size_t index) { @@ -619,12 +619,6 @@ void ChakraRuntime::popScope(Runtime::ScopeState *state) { ThrowUponJsError(JsCollectGarbage(m_runtime), "JsCollectGarbage"); } -bool ChakraRuntime::strictEquals( - const facebook::jsi::Symbol &a, - const facebook::jsi::Symbol &b) const { - return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); -} - bool ChakraRuntime::strictEquals( const facebook::jsi::String &a, const facebook::jsi::String &b) const { @@ -637,6 +631,12 @@ bool ChakraRuntime::strictEquals( return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); } +bool ChakraRuntime::strictEquals( + const facebook::jsi::Symbol &a, + const facebook::jsi::Symbol &b) const { + return CompareJsValues(GetChakraObjectRef(a), GetChakraObjectRef(b)); +} + bool ChakraRuntime::instanceOf( const facebook::jsi::Object &obj, const facebook::jsi::Function &func) { @@ -649,57 +649,6 @@ bool ChakraRuntime::instanceOf( #pragma endregion Functions_inherited_from_Runtime -ChakraObjectRef ChakraRuntime::ToChakraObjectRef( - const facebook::jsi::Value &value) { - if (value.isUndefined()) { - JsValueRef ref; - ThrowUponJsError(JsGetUndefinedValue(&ref), "JsGetUndefinedValue"); - return ChakraObjectRef(ref); - - } else if (value.isNull()) { - JsValueRef ref; - ThrowUponJsError(JsGetNullValue(&ref), "JsGetNullValue"); - return ChakraObjectRef(ref); - - } else if (value.isBool()) { - JsValueRef ref; - ThrowUponJsError(JsBoolToBoolean(value.getBool(), &ref), "JsBoolToBoolean"); - return ChakraObjectRef(ref); - - } else if (value.isNumber()) { - JsValueRef ref; - ThrowUponJsError( - JsDoubleToNumber(value.asNumber(), &ref), "JsDoubleToNumber"); - return ChakraObjectRef(ref); - - } else if (value.isSymbol()) { - return GetChakraObjectRef(value.asSymbol(*this)); - - } else if (value.isString()) { - return GetChakraObjectRef(value.asString(*this)); - - } else if (value.isObject()) { - return GetChakraObjectRef(value.asObject(*this)); - - } else { - // Control flow should never reach here. - std::terminate(); - } -} - -std::vector ChakraRuntime::ToChakraObjectRefs( - const facebook::jsi::Value *value, - size_t count) { - std::vector result{}; - - for (unsigned int i = 0; i < count; ++i) { - result.emplace_back(ToChakraObjectRef(*value)); - ++value; - } - - return result; -} - facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { JsValueType type = GetValueType(ref); @@ -756,33 +705,55 @@ facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { std::terminate(); } -void ChakraRuntime::ThrowUponJsError( - JsErrorCode error, - const char *const chakraApiName) { - switch (error) { - case JsNoError: { - return; - break; - } +ChakraObjectRef ChakraRuntime::ToChakraObjectRef( + const facebook::jsi::Value &value) { + if (value.isUndefined()) { + JsValueRef ref; + ThrowUponJsError(JsGetUndefinedValue(&ref), "JsGetUndefinedValue"); + return ChakraObjectRef(ref); - case JsErrorScriptException: { - JsValueRef jsError; - CrashUponChakraError(JsGetAndClearException(&jsError)); - throw facebook::jsi::JSError( - "A JavaScript Error was thrown.", - *this, - ToJsiValue(ChakraObjectRef(jsError))); - break; - } + } else if (value.isNull()) { + JsValueRef ref; + ThrowUponJsError(JsGetNullValue(&ref), "JsGetNullValue"); + return ChakraObjectRef(ref); - default: { - ThrowUponChakraError(error, chakraApiName); - break; - } - } // switch (error) + } else if (value.isBool()) { + JsValueRef ref; + ThrowUponJsError(JsBoolToBoolean(value.getBool(), &ref), "JsBoolToBoolean"); + return ChakraObjectRef(ref); - // Control flow should never reach here. - std::terminate(); + } else if (value.isNumber()) { + JsValueRef ref; + ThrowUponJsError( + JsDoubleToNumber(value.asNumber(), &ref), "JsDoubleToNumber"); + return ChakraObjectRef(ref); + + } else if (value.isSymbol()) { + return GetChakraObjectRef(value.asSymbol(*this)); + + } else if (value.isString()) { + return GetChakraObjectRef(value.asString(*this)); + + } else if (value.isObject()) { + return GetChakraObjectRef(value.asObject(*this)); + + } else { + // Control flow should never reach here. + std::terminate(); + } +} + +std::vector ChakraRuntime::ToChakraObjectRefs( + const facebook::jsi::Value *value, + size_t count) { + std::vector result{}; + + for (unsigned int i = 0; i < count; ++i) { + result.emplace_back(ToChakraObjectRef(*value)); + ++value; + } + + return result; } // clang-format off @@ -814,6 +785,35 @@ T *ChakraRuntime::ObjectWithExternalData::getExternalData() { return externalData; } +void ChakraRuntime::ThrowUponJsError( + JsErrorCode error, + const char *const chakraApiName) { + switch (error) { + case JsNoError: { + return; + break; + } + + case JsErrorScriptException: { + JsValueRef jsError; + CrashUponChakraError(JsGetAndClearException(&jsError)); + throw facebook::jsi::JSError( + "A JavaScript Error was thrown.", + *this, + ToJsiValue(ChakraObjectRef(jsError))); + break; + } + + default: { + ThrowUponChakraError(error, chakraApiName); + break; + } + } // switch (error) + + // Control flow should never reach here. + std::terminate(); +} + facebook::jsi::Object ChakraRuntime::createProxy( facebook::jsi::Object &&target, facebook::jsi::Object &&handler) noexcept { @@ -938,6 +938,42 @@ facebook::jsi::Object ChakraRuntime::createHostObjectProxyHandler() noexcept { return handlerObj; } +void ChakraRuntime::setupMemoryTracker() noexcept { + if (runtimeArgs().memoryTracker) { + size_t initialMemoryUsage = 0; + JsGetRuntimeMemoryUsage(m_runtime, &initialMemoryUsage); + runtimeArgs().memoryTracker->Initialize(initialMemoryUsage); + + if (runtimeArgs().runtimeMemoryLimit > 0) + JsSetRuntimeMemoryLimit(m_runtime, runtimeArgs().runtimeMemoryLimit); + + JsSetRuntimeMemoryAllocationCallback( + m_runtime, + runtimeArgs().memoryTracker.get(), + [](void *callbackState, + JsMemoryEventType allocationEvent, + size_t allocationSize) -> bool { + auto memoryTrackerPtr = + static_cast(callbackState); + switch (allocationEvent) { + case JsMemoryAllocate: + memoryTrackerPtr->OnAllocation(allocationSize); + break; + + case JsMemoryFree: + memoryTrackerPtr->OnDeallocation(allocationSize); + break; + + case JsMemoryFailure: + default: + break; + } + + return true; + }); + } +} + JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( JsValueRef callee, bool isConstructCall, @@ -1001,42 +1037,6 @@ JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( return res; } -void ChakraRuntime::setupMemoryTracker() noexcept { - if (runtimeArgs().memoryTracker) { - size_t initialMemoryUsage = 0; - JsGetRuntimeMemoryUsage(m_runtime, &initialMemoryUsage); - runtimeArgs().memoryTracker->Initialize(initialMemoryUsage); - - if (runtimeArgs().runtimeMemoryLimit > 0) - JsSetRuntimeMemoryLimit(m_runtime, runtimeArgs().runtimeMemoryLimit); - - JsSetRuntimeMemoryAllocationCallback( - m_runtime, - runtimeArgs().memoryTracker.get(), - [](void *callbackState, - JsMemoryEventType allocationEvent, - size_t allocationSize) -> bool { - auto memoryTrackerPtr = - static_cast(callbackState); - switch (allocationEvent) { - case JsMemoryAllocate: - memoryTrackerPtr->OnAllocation(allocationSize); - break; - - case JsMemoryFree: - memoryTrackerPtr->OnDeallocation(allocationSize); - break; - - case JsMemoryFailure: - default: - break; - } - - return true; - }); - } -} - /*static*/ std::once_flag ChakraRuntime::s_runtimeVersionInitFlag; /*static*/ uint64_t ChakraRuntime::s_runtimeVersion = 0; diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 4f36033d748..365648e6908 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -187,11 +187,11 @@ class ChakraRuntime : public facebook::jsi::Runtime { public: // These three functions only performs shallow copies. + facebook::jsi::Value ToJsiValue(ChakraObjectRef &&ref); ChakraObjectRef ToChakraObjectRef(const facebook::jsi::Value &value); std::vector ToChakraObjectRefs( const facebook::jsi::Value *value, size_t count); - facebook::jsi::Value ToJsiValue(ChakraObjectRef &&ref); protected: ChakraRuntimeArgs &runtimeArgs() { @@ -199,8 +199,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { } private: - void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); - // ChakraPointerValue is needed for working with Facebook's jsi::Pointer class // and must only be used for this purpose. Every instance of // ChakraPointerValue should be allocated on the heap and be used as an @@ -287,31 +285,6 @@ class ChakraRuntime : public facebook::jsi::Runtime { ->GetRef(); } - // HostObject and HostFunction helpers - template - class ObjectWithExternalData : public facebook::jsi::Object { - public: - static facebook::jsi::Object create( - ChakraRuntime &runtime, - std::unique_ptr &&externalData); - - static ObjectWithExternalData fromExisting( - ChakraRuntime &runtime, - facebook::jsi::Object &&obj); - - public: - T *getExternalData(); - ObjectWithExternalData(const Runtime::PointerValue *value) - : Object(const_cast(value)) { - } // TODO :: const_cast - - ObjectWithExternalData(ObjectWithExternalData &&other) = default; - ObjectWithExternalData &operator=(ObjectWithExternalData &&other) = default; - }; - - template - friend class ObjectWithExternalData; - class HostObjectProxy { public: facebook::jsi::Value Get(const facebook::jsi::PropNameID &propNameId) { @@ -341,19 +314,38 @@ class ChakraRuntime : public facebook::jsi::Runtime { std::shared_ptr m_hostObject; }; + template + class ObjectWithExternalData : public facebook::jsi::Object { + public: + static facebook::jsi::Object create( + ChakraRuntime &runtime, + std::unique_ptr &&externalData); + + static ObjectWithExternalData fromExisting( + ChakraRuntime &runtime, + facebook::jsi::Object &&obj); + + public: + T *getExternalData(); + ObjectWithExternalData(const Runtime::PointerValue *value) + : Object(const_cast(value)) { + } // TODO :: const_cast + + ObjectWithExternalData(ObjectWithExternalData &&other) = default; + ObjectWithExternalData &operator=(ObjectWithExternalData &&other) = default; + }; + + template + friend class ObjectWithExternalData; + + void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); + facebook::jsi::Object createProxy( facebook::jsi::Object &&target, facebook::jsi::Object &&handler) noexcept; facebook::jsi::Function createProxyConstructor() noexcept; facebook::jsi::Object createHostObjectProxyHandler() noexcept; - static JsValueRef CALLBACK HostFunctionCall( - JsValueRef callee, - bool isConstructCall, - JsValueRef *argumentsIncThis, - unsigned short argumentCountIncThis, - void *callbackState); - // Promise Helpers static void CALLBACK PromiseContinuationCallback(JsValueRef funcRef, void *callbackState) noexcept; @@ -393,7 +385,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { return s_runtimeVersion; } - // JavaScript evaluation helpers + // Miscellaneous std::unique_ptr generatePreparedScript( const std::string &sourceURL, const facebook::jsi::Buffer &sourceBuffer) noexcept; @@ -404,6 +396,12 @@ class ChakraRuntime : public facebook::jsi::Runtime { const facebook::jsi::Buffer &scriptBuffer, const facebook::jsi::Buffer &serializedScriptBuffer, const std::string &sourceURL); + static JsValueRef CALLBACK HostFunctionCall( + JsValueRef callee, + bool isConstructCall, + JsValueRef *argumentsIncThis, + unsigned short argumentCountIncThis, + void *callbackState); static std::once_flag s_runtimeVersionInitFlag; static uint64_t s_runtimeVersion; From 74687160d729e187c847eb55585c8f1341bec2f9 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Thu, 17 Oct 2019 19:06:03 -0700 Subject: [PATCH 12/16] Fix E2E test app. --- vnext/JSI/Shared/ChakraObjectRef.cpp | 2 +- vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index fa7217a9c97..342d4fa2ede 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -171,7 +171,7 @@ ChakraObjectRef GetPropertyId(const char *const utf8, size_t length) { return ChakraObjectRef(id); #else - std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8); + std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); return GetPropertyId(utf16); #endif } diff --git a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp index e6603ebece6..e8ebf9d5c90 100644 --- a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp +++ b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp @@ -59,8 +59,6 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScriptSimple( throw facebook::jsi::JSINativeException("Script can't be empty."); const std::wstring url16 = Microsoft::Common::Unicode::Utf8ToUtf16(sourceURL); - if (url16.empty()) - throw facebook::jsi::JSINativeException("Script URL can't be empty."); JsValueRef result; ThrowUponJsError( From 7daa7d23b5bfcadd52ec0ddef81d3caeee37a378 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Fri, 18 Oct 2019 16:00:14 -0700 Subject: [PATCH 13/16] Address feedback from PR #3445. --- vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp | 41 ++--- vnext/JSI/Shared/ChakraObjectRef.cpp | 183 ++++++++++---------- vnext/JSI/Shared/ChakraObjectRef.h | 165 +++++++++++++----- vnext/JSI/Shared/ChakraRuntime.cpp | 176 ++++++++----------- vnext/JSI/Shared/ChakraRuntime.h | 4 +- 5 files changed, 297 insertions(+), 272 deletions(-) diff --git a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp index 5ed1653dfac..1014ee0c5fa 100644 --- a/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp +++ b/vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp @@ -37,21 +37,7 @@ struct FileVersionInfoResource { } // namespace // TODO (yicyao): We temporarily removed weak reference semantics from -// ChakraCore based jsi::Runtime. Renable these when we switch to class -// hierarchy. - -// JsWeakRef ChakraRuntime::newWeakObjectRef(const facebook::jsi::Object &obj) { -// JsWeakRef weakRef; -// JsCreateWeakReference(objectRef(obj), &weakRef); -// return weakRef; -//} -// -// JsValueRef ChakraRuntime::strongObjectRef( -// const facebook::jsi::WeakObject &obj) { -// JsValueRef strongRef; -// JsGetWeakReferenceValue(objectRef(obj), &strongRef); -// return strongRef; -//} +// ChakraCore based jsi::Runtime. // ES6 Promise callback void CALLBACK ChakraRuntime::PromiseContinuationCallback( @@ -76,9 +62,8 @@ void ChakraRuntime::PromiseContinuation(JsValueRef funcRef) noexcept { runtimeArgs().jsQueue->runOnQueue([this, funcRef]() { JsValueRef undefinedValue; JsGetUndefinedValue(&undefinedValue); - ThrowUponJsError( - JsCallFunction(funcRef, &undefinedValue, 1, nullptr), - "JsCallFunction"); + VerifyJsErrorElseThrow( + JsCallFunction(funcRef, &undefinedValue, 1, nullptr)); JsRelease(funcRef, nullptr); }); } @@ -315,14 +300,12 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScriptSimple( &sourceURLRef); JsValueRef result; - ThrowUponJsError( - JsRun( - sourceRef, - 0, - sourceURLRef, - JsParseScriptAttributes::JsParseScriptAttributeNone, - &result), - "JsRun"); + VerifyJsErrorElseThrow(JsRun( + sourceRef, + 0, + sourceURLRef, + JsParseScriptAttributes::JsParseScriptAttributeNone, + &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -341,8 +324,8 @@ bool ChakraRuntime::evaluateSerializedScript( &bytecodeArrayBuffer) == JsNoError) { JsValueRef sourceURLRef = nullptr; if (!sourceURL.empty()) { - sourceURLRef = ToJsString( - reinterpret_cast(sourceURL.c_str()), sourceURL.size()); + sourceURLRef = ToJsString(std::string_view{ + reinterpret_cast(sourceURL.c_str()), sourceURL.size()}); } JsValueRef value = nullptr; @@ -373,7 +356,7 @@ bool ChakraRuntime::evaluateSerializedScript( } else if (result == JsErrorBadSerializedScript) { return false; } else { - ThrowUponChakraError(result, "JsRunSerialized"); + VerifyChakraErrorElseThrow(result); } } diff --git a/vnext/JSI/Shared/ChakraObjectRef.cpp b/vnext/JSI/Shared/ChakraObjectRef.cpp index 342d4fa2ede..1772905dd41 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.cpp +++ b/vnext/JSI/Shared/ChakraObjectRef.cpp @@ -14,24 +14,11 @@ namespace Microsoft::JSI { -namespace { - -inline void CheckedJsAddRef(JsRef ref) { - ThrowUponChakraError(JsAddRef(ref, nullptr), "JsAddRef"); -} - -inline void CheckedJsRelease(JsRef ref) { - ThrowUponChakraError(JsRelease(ref, nullptr), "JsRelease"); -} - -} // namespace - -void ThrowUponChakraError(JsErrorCode error, const char *const chakraApiName) { +void VerifyChakraErrorElseThrow(JsErrorCode error) { if (error != JsNoError) { std::ostringstream errorString; - errorString << "A call to " - << (chakraApiName ? chakraApiName : "Chakra(Core) API") - << " returned error code 0x" << std::hex << error << '.'; + errorString << "A call to Chakra(Core) API returned error code 0x" + << std::hex << error << '.'; throw facebook::jsi::JSINativeException(errorString.str()); } } @@ -39,41 +26,33 @@ void ThrowUponChakraError(JsErrorCode error, const char *const chakraApiName) { ChakraObjectRef::ChakraObjectRef(const ChakraObjectRef &original) noexcept : m_ref{original.m_ref}, m_state{original.m_state} { if (m_state == State::Initialized) { - assert(m_ref); - CheckedJsAddRef(m_ref); - } else { - assert(!m_ref); + VerifyChakraErrorElseThrow(JsAddRef(m_ref, nullptr)); } } ChakraObjectRef::ChakraObjectRef(ChakraObjectRef &&original) noexcept { - swap(original); + Swap(original); } ChakraObjectRef &ChakraObjectRef::operator=( const ChakraObjectRef &rhs) noexcept { ChakraObjectRef rhsCopy(rhs); - swap(rhsCopy); + Swap(rhsCopy); return *this; } ChakraObjectRef &ChakraObjectRef::operator=(ChakraObjectRef &&rhs) noexcept { - swap(rhs); + Swap(rhs); return *this; } ChakraObjectRef::~ChakraObjectRef() noexcept { if (m_state == State::Initialized) { - assert(m_ref); - CheckedJsRelease(m_ref); - } else { - assert(!m_ref); + VerifyChakraErrorElseThrow(JsRelease(m_ref, nullptr)); } } void ChakraObjectRef::Initialize(JsRef ref) { - assert(!m_ref); - if (m_state != State::Uninitialized) { throw facebook::jsi::JSINativeException( "A ChakraObjectRef can only be initialzed once."); @@ -84,7 +63,7 @@ void ChakraObjectRef::Initialize(JsRef ref) { "Cannot initialize a ChakraObjectRef with a null reference."); } - CheckedJsAddRef(ref); + VerifyChakraErrorElseThrow(JsAddRef(ref, nullptr)); m_ref = ref; m_state = State::Initialized; } @@ -92,20 +71,17 @@ void ChakraObjectRef::Initialize(JsRef ref) { void ChakraObjectRef::Invalidate() { switch (m_state) { case State::Uninitialized: { - assert(!m_ref); throw facebook::jsi::JSINativeException( "Cannot invalidate a ChakraObjectRef that has not been initialized."); break; } case State::Initialized: { - assert(m_ref); - CheckedJsRelease(m_ref); + VerifyChakraErrorElseThrow(JsRelease(m_ref, nullptr)); m_ref = nullptr; m_state = State::Invalidated; break; } case State::Invalidated: { - assert(!m_ref); throw facebook::jsi::JSINativeException( "Cannot invalidate a ChakraObjectRef that has already been " "invalidated."); @@ -119,21 +95,20 @@ void ChakraObjectRef::Invalidate() { } } -void ChakraObjectRef::swap(ChakraObjectRef &other) { +void ChakraObjectRef::Swap(ChakraObjectRef &other) { std::swap(m_ref, other.m_ref); std::swap(m_state, other.m_state); } JsValueType GetValueType(const ChakraObjectRef &jsValue) { JsValueType type; - ThrowUponChakraError(JsGetValueType(jsValue, &type), "JsGetValueType"); + VerifyChakraErrorElseThrow(JsGetValueType(jsValue, &type)); return type; } JsPropertyIdType GetPropertyIdType(const ChakraObjectRef &jsPropId) { JsPropertyIdType type; - ThrowUponChakraError( - JsGetPropertyIdType(jsPropId, &type), "JsGetPropertyIdType"); + VerifyChakraErrorElseThrow(JsGetPropertyIdType(jsPropId, &type)); return type; } @@ -143,8 +118,7 @@ std::wstring GetPropertyName(const ChakraObjectRef &id) { "It is llegal to retrieve the name of a property symbol."); } const wchar_t *propertyName = nullptr; - ThrowUponChakraError( - JsGetPropertyNameFromId(id, &propertyName), "JsGetPropertyNameFromId"); + VerifyChakraErrorElseThrow(JsGetPropertyNameFromId(id, &propertyName)); return std::wstring{propertyName}; } @@ -154,47 +128,56 @@ ChakraObjectRef GetPropertySymbol(const ChakraObjectRef &id) { "It is llegal to retrieve the symbol associated with a property name."); } JsValueRef symbol = nullptr; - ThrowUponChakraError( - JsGetSymbolFromPropertyId(id, &symbol), "JsGetSymbolFromPropertyId"); + VerifyChakraErrorElseThrow(JsGetSymbolFromPropertyId(id, &symbol)); return ChakraObjectRef{symbol}; } -ChakraObjectRef GetPropertyId(const char *const utf8, size_t length) { - assert(utf8); +ChakraObjectRef GetPropertyId(const std::string_view &utf8) { + if (!utf8.data()) { + throw facebook::jsi::JSINativeException( + "Property name cannot be a nullptr."); + } // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCreatePropertyId API. #ifdef CHAKRACORE JsPropertyIdRef id = nullptr; - ThrowUponChakraError( - JsCreatePropertyId(utf8, length, &id), "JsCreatePropertyId"); + VerifyChakraErrorElseThrow( + JsCreatePropertyId(utf8.data(), utf8.length(), &id)); return ChakraObjectRef(id); #else - std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); + std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8.data(), utf8.length()); return GetPropertyId(utf16); #endif } ChakraObjectRef GetPropertyId(const std::wstring &utf16) { JsPropertyIdRef id = nullptr; - ThrowUponChakraError( - JsGetPropertyIdFromName(utf16.c_str(), &id), "JsGetPropertyIdFromName"); + VerifyChakraErrorElseThrow(JsGetPropertyIdFromName(utf16.c_str(), &id)); return ChakraObjectRef(id); } std::string ToStdString(const ChakraObjectRef &jsString) { + if (GetValueType(jsString) != JsString) { + throw facebook::jsi::JSINativeException( + "Cannot convert a non JS string ChakraObjectRef to a std::string."); + } + // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCopyString API. #ifdef CHAKRACORE size_t length = 0; - ThrowUponChakraError( - JsCopyString(jsString, nullptr, 0, &length), "JsCopyString"); + VerifyChakraErrorElseThrow(JsCopyString(jsString, nullptr, 0, &length)); + std::string result(length, 'a'); - ThrowUponChakraError( - JsCopyString(jsString, result.data(), result.length(), &length), - "JsCopyString"); - assert(length == result.length()); + VerifyChakraErrorElseThrow( + JsCopyString(jsString, result.data(), result.length(), &length)); + + if (length != result.length()) { + throw facebook::jsi::JSINativeException( + "Failed to convert a JS string to a std::string."); + } return result; #else @@ -203,59 +186,76 @@ std::string ToStdString(const ChakraObjectRef &jsString) { } std::wstring ToStdWstring(const ChakraObjectRef &jsString) { - assert(GetValueType(jsString) == JsString); + if (GetValueType(jsString) != JsString) { + throw facebook::jsi::JSINativeException( + "Cannot convert a non JS string ChakraObjectRef to a std::wstring."); + } const wchar_t *utf16 = nullptr; size_t length = 0; - ThrowUponChakraError( - JsStringToPointer(jsString, &utf16, &length), "JsStringToPointer"); + VerifyChakraErrorElseThrow(JsStringToPointer(jsString, &utf16, &length)); return std::wstring(utf16, length); } -ChakraObjectRef ToJsString(const char *utf8, size_t length) { - assert(utf8); +ChakraObjectRef ToJsString(const std::string_view &utf8) { + if (!utf8.data()) { + throw facebook::jsi::JSINativeException( + "Cannot convert a nullptr to a JS string."); + } // We use a #ifdef here because we can avoid a UTF-8 to UTF-16 conversion // using ChakraCore's JsCreateString API. #ifdef CHAKRACORE JsValueRef result = nullptr; - ThrowUponChakraError(JsCreateString(utf8, length, &result), "JsCreateString"); + VerifyChakraErrorElseThrow( + JsCreateString(utf8.data(), utf8.length(), &result)); return ChakraObjectRef(result); #else - std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8, length); - return ToJsString(utf16.c_str(), utf16.length()); + std::wstring utf16 = Common::Unicode::Utf8ToUtf16(utf8.data(), utf8.length()); + return ToJsString(std::wstring_view{utf16.c_str(), utf16.length()}); #endif } -ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length) { - assert(utf16); +ChakraObjectRef ToJsString(const std::wstring_view &utf16) { + if (!utf16.data()) { + throw facebook::jsi::JSINativeException( + "Cannot convert a nullptr to a JS string."); + } + JsValueRef result = nullptr; - ThrowUponChakraError( - JsPointerToString(utf16, length, &result), "JsPointerToString"); + VerifyChakraErrorElseThrow( + JsPointerToString(utf16.data(), utf16.length(), &result)); + return ChakraObjectRef(result); } ChakraObjectRef ToJsString(const ChakraObjectRef &ref) { JsValueRef str = nullptr; - ThrowUponChakraError( - JsConvertValueToString(ref, &str), "JsConvertValueToString"); + VerifyChakraErrorElseThrow(JsConvertValueToString(ref, &str)); return ChakraObjectRef(str); } ChakraObjectRef ToJsNumber(int num) { JsValueRef result = nullptr; - ThrowUponChakraError(JsIntToNumber(num, &result), "JsIntToNumber"); + VerifyChakraErrorElseThrow(JsIntToNumber(num, &result)); return ChakraObjectRef(result); } ChakraObjectRef ToJsArrayBuffer( const std::shared_ptr &buffer) { - assert(buffer); + if (!buffer) { + throw facebook::jsi::JSINativeException( + "Cannot create an external JS ArrayBuffer without backing buffer."); + } size_t size = buffer->size(); - assert(size < UINT_MAX); + + if (size > UINT_MAX) { + throw facebook::jsi::JSINativeException( + "The external backing buffer for a JS ArrayBuffer is too large."); + } JsValueRef arrayBuffer = nullptr; auto bufferWrapper = @@ -265,23 +265,23 @@ ChakraObjectRef ToJsArrayBuffer( // when the JavaScript garbage collecotr releases the created external array // buffer. This ensures that buffer stays alive while the JavaScript engine is // using it. - ThrowUponChakraError( - JsCreateExternalArrayBuffer( - Common::Utilities::CheckedReinterpretCast( - const_cast(buffer->data())), - static_cast(size), - [](void *bufferToDestroy) { - // We wrap bufferToDestroy in a unique_ptr to avoid calling delete - // explicitly. - std::unique_ptr> - wrapper{ - static_cast *>( - bufferToDestroy)}; - }, - bufferWrapper.get(), - &arrayBuffer), - "JsCreateExternalArrayBuffer"); - + VerifyChakraErrorElseThrow(JsCreateExternalArrayBuffer( + Common::Utilities::CheckedReinterpretCast( + const_cast(buffer->data())), + static_cast(size), + [](void *bufferToDestroy) { + // We wrap bufferToDestroy in a unique_ptr to avoid calling delete + // explicitly. + std::unique_ptr> wrapper{ + static_cast *>( + bufferToDestroy)}; + }, + bufferWrapper.get(), + &arrayBuffer)); + + // We only call bufferWrapper.release() after JsCreateExternalObject succeeds. + // Otherwise, when JsCreateExternalObject fails and an exception is thrown, + // the shared_ptr that bufferWrapper used to own will be leaked. bufferWrapper.release(); return ChakraObjectRef(arrayBuffer); } @@ -292,8 +292,7 @@ bool CompareJsValues( bool result = false; // Note that JsStrictEquals should only be used for JsValueRefs and not for // other types of JsRefs (e.g. JsPropertyIdRef, etc.). - ThrowUponChakraError( - JsStrictEquals(jsValue1, jsValue2, &result), "JsStrictEquals"); + VerifyChakraErrorElseThrow(JsStrictEquals(jsValue1, jsValue2, &result)); return result; } @@ -308,12 +307,10 @@ bool CompareJsPropertyIds( } if (type1 == JsPropertyIdTypeString) { - assert(type2 == JsPropertyIdTypeString); return GetPropertyName(jsPropId1) == GetPropertyName(jsPropId2); } if (type1 == JsPropertyIdTypeSymbol) { - assert(type2 == JsPropertyIdTypeSymbol); return CompareJsValues( GetPropertySymbol(jsPropId1), GetPropertySymbol(jsPropId2)); } diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 00874e1e1b6..2fb19aeeeb7 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -8,33 +8,33 @@ #ifdef CHAKRACORE #include "ChakraCore.h" #else -#ifndef USE_EDGEMODE_JSRT #define USE_EDGEMODE_JSRT -#endif #include #endif #include #include +#include namespace Microsoft::JSI { -inline void CrashUponChakraError(JsErrorCode error) { +inline void VerifyChakraErrorElseCrash(JsErrorCode error) { if (error != JsNoError) { std::terminate(); } } -void ThrowUponChakraError( - JsErrorCode error, - const char *const chakraApiName = nullptr); - -// An shared_ptr like RAII Wrapper for JsRefs, which are references to objects -// owned by the garbage collector. These include JsContextRef, JsValueRef, and -// JsPropertyIdRef, etc. ChakraObjectRef ensures that JsAddRef and JsRelease -// are called upon initialization and invalidation, respectively. It also allows -// users to implicitly convert it into a JsRef. A ChakraObjectRef must only be -// initialized once and invalidated once. +void VerifyChakraErrorElseThrow(JsErrorCode error); + +/** + * @brief An shared_ptr like RAII Wrapper for JsRefs. + * + * JsRefs are references to objects owned by the garbage collector and include + * JsContextRef, JsValueRef, and JsPropertyIdRef, etc. ChakraObjectRef ensures + * that JsAddRef and JsRelease are called upon initialization and invalidation, + * respectively. It also allows users to implicitly convert it into a JsRef. A + * ChakraObjectRef must only be initialized once and invalidated once. + */ class ChakraObjectRef { public: ChakraObjectRef() noexcept {} @@ -58,7 +58,7 @@ class ChakraObjectRef { } private: - void swap(ChakraObjectRef &other); + void Swap(ChakraObjectRef &other); enum class State { Uninitialized, Initialized, Invalidated }; @@ -66,50 +66,113 @@ class ChakraObjectRef { State m_state = State::Uninitialized; }; -// jsValue must be managing a JsValueRef. +/** + * @param jsValue A ChakraObjectRef managing a JsValueRef. + */ JsValueType GetValueType(const ChakraObjectRef &jsValue); -// jsPropId must be managing a JsPropertyIdRef. +/** + * @param jsPropId A ChakraObjectRef managing a JsPropertyIdRef. + */ JsPropertyIdType GetPropertyIdType(const ChakraObjectRef &jsPropId); -// jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeString. +/** + * @param jsPropId A ChakraObjectRef managing a JsPropertyIdRef of type + * JsPropertyIdTypeString. + */ std::wstring GetPropertyName(const ChakraObjectRef &jsPropId); -// jsPropId must be managing a JsPropertyIdRef of type JsPropertyIdTypeSymbol. -// Returns a ChakraObjectRef managing a JsValueRef pointing to a JS Symbol. +/** + * @param jsPropId A ChakraObjectRef managing a JsPropertyIdRef of type + * JsPropertyIdTypeSymbol. + * + * @returns A ChakraObjectRef managing a JS Symbol. + */ ChakraObjectRef GetPropertySymbol(const ChakraObjectRef &jsPropId); -// utf8 does not have to be null terminated. -ChakraObjectRef GetPropertyId(const char *const utf8, size_t length); - +/** + * @param utf8 A std::string_view to a UTF-8 encoded char array. + * + * @returns A ChakraObjectRef managing a JsPropertyIdRef. + */ +ChakraObjectRef GetPropertyId(const std::string_view &utf8); + +/** + * @param utf16 A UTF-16 encoded std::wstring. + * + * @returns A ChakraObjectRef managing a JsPropertyIdRef. + */ ChakraObjectRef GetPropertyId(const std::wstring &utf16); -// jsString must be managing a JS string. The returned std::string/std::wstring -// is UTF-8/UTF-16 encoded. These functions copy the JS string buffer into the -// returned std::string/std::wstring. +/** + * @param jsString A ChakraObjectRef managing a JS string. + * + * @returns A std::string that is UTF-8 encoded. + * + * @remarks This function copies the JS string buffer into the returned + * std::string. When using Chakra instead of ChakraCore, this function incurs + * a UTF-16 to UTF-8 conversion. + */ std::string ToStdString(const ChakraObjectRef &jsString); -std::wstring ToStdWstring(const ChakraObjectRef &jsString); -// Returns a ChakraObjectRef managing a JS string. utf8 and utf16 do not have to -// be null terminated and are copied to JS engine owned memory. -ChakraObjectRef ToJsString(const char *const utf8, size_t length); -ChakraObjectRef ToJsString(const wchar_t *const utf16, size_t length); +/** + * @param jsString A ChakraObjectRef managing a JS string. + * + * @returns A std::wstring that is UTF-16 encoded. + * + * @remarks This functions copies the JS string buffer into the returned + * std::wstring. + */ +std::wstring ToStdWstring(const ChakraObjectRef &jsString); -// jsValue must be mananing a JsValueRef. Returns a ChakraObjectRef managing the -// return value of the JS .toString function. +/** + * @param utf8 A std::string_view to a UTF-8 encoded char array. + * + * @returns A ChakraObjectRef managing a JS string. + * + * @remarks The content of utf8 is copied into JS engine owned memory. When + * using Chakra instead of ChakraCore, this function incurs a UTF-8 to UTF-16 + * conversion. + */ +ChakraObjectRef ToJsString(const std::string_view &utf8); + +/** + * @param utf16 A std::wstring_view to a UTF-16 encoded wchar_t array. + * + * @returns A ChakraObjectRef managing a JS string. + * + * @remarks The content of utf16 is copied into JS engine owned memory. + */ +ChakraObjectRef ToJsString(const std::wstring_view &utf16); + +/** + * @param jsValue A ChakraObjectRef mananing a JsValueRef. + * + * @returns A ChakraObjectRef managing the return value of the JS .toString + * function. + */ ChakraObjectRef ToJsString(const ChakraObjectRef &jsValue); -// Returns a ChakraObjectRef managing a JS number. +/** + * @returns A ChakraObjectRef managing a JS number. + */ ChakraObjectRef ToJsNumber(int num); -// Returns a ChakraObjectRef managing a JS Object. This Object is backed by data -// and keeps data alive till the garbage collector finalizes it. +/** + * @returns A ChakraObjectRef managing a JS Object. + * + * @remarks The returned Object is backed by data and keeps data alive till the + * garbage collector finalizes it. + */ template ChakraObjectRef ToJsObject(std::unique_ptr &&data) { - assert(data); + if (!data) { + throw facebook::jsi::JSINativeException( + "Cannot create an external JS Object without backing data."); + } JsValueRef obj = nullptr; - ThrowUponChakraError(JsCreateExternalObject( + VerifyChakraErrorElseThrow(JsCreateExternalObject( data.get(), [](void *dataToDestroy) { // We wrap dataToDestroy in a unique_ptr to avoid calling delete @@ -118,22 +181,40 @@ ChakraObjectRef ToJsObject(std::unique_ptr &&data) { }, &obj)); + // We only call data.release() after JsCreateExternalObject succeeds. + // Otherwise, when JsCreateExternalObject fails and an exception is thrown, + // the buffer that data used to own will be leaked. data.release(); return ChakraObjectRef(obj); } -// Returns a ChakraObjectRef managing a JS ArrayBuffer. This ArrayBuffer is -// backed by buffer and keeps buffer alive till the GC finalizes it. +/** + * @returns A ChakraObjectRef managing a JS ArrayBuffer. + * + * @remarks The returned ArrayBuffer is backed by buffer and keeps buffer alive + * till the garbage collector finalizes it. + */ ChakraObjectRef ToJsArrayBuffer( const std::shared_ptr &buffer); -// Both jsValue1 and jsValue2 must be managing a JsValueRef. Returns whether -// jsValue1 anf jsValue2 are strictly equal. +/** + * @param jsValue1 A ChakraObjectRef managing a JsValueRef. + * @param jsValue2 A ChakraObjectRef managing a JsValueRef. + * + * @returns A boolean indicating whether jsValue1 and jsValue2 are strictly + * equal. + */ bool CompareJsValues( const ChakraObjectRef &jsValue1, const ChakraObjectRef &jsValue2); -// Both jsPropId1 and jsPropId2 must be managing a JsPropertyIdRef. +/** + * @param jsPropId1 A ChakraObjectRef managing a JsPropertyIdRef. + * @param jsPropId2 A ChakraObjectRef managing a JsPropertyIdRef. + * + * @returns A boolean indicating whether jsPropId1 and jsPropId2 are strictly + * equal. + */ bool CompareJsPropertyIds( const ChakraObjectRef &jsPropId1, const ChakraObjectRef &jsPropId2); diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 0137ada7936..6d373bbeb64 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -63,19 +63,18 @@ ChakraRuntime::ChakraRuntime(ChakraRuntimeArgs &&args) noexcept JsRuntimeAttributeDisableExecutablePageAllocation); } - ThrowUponChakraError( - JsCreateRuntime(runtimeAttributes, nullptr, &m_runtime), - "JsCreateRuntime"); + VerifyChakraErrorElseThrow( + JsCreateRuntime(runtimeAttributes, nullptr, &m_runtime)); setupMemoryTracker(); JsContextRef context = nullptr; - ThrowUponChakraError(JsCreateContext(m_runtime, &context), "JsCreateContext"); + VerifyChakraErrorElseThrow(JsCreateContext(m_runtime, &context)); m_context.Initialize(context); // Note :: We currently assume that the runtime will be created and // exclusively used in a single thread. - ThrowUponChakraError(JsSetCurrentContext(m_context), "JsSetCurrentContext"); + VerifyChakraErrorElseThrow(JsSetCurrentContext(m_context)); startDebuggingIfNeeded(); @@ -87,13 +86,12 @@ ChakraRuntime::ChakraRuntime(ChakraRuntimeArgs &&args) noexcept ChakraRuntime::~ChakraRuntime() noexcept { stopDebuggingIfNeeded(); - ThrowUponChakraError( - JsSetCurrentContext(JS_INVALID_REFERENCE), "JsSetCurrentContext"); + VerifyChakraErrorElseThrow(JsSetCurrentContext(JS_INVALID_REFERENCE)); m_context.Invalidate(); JsSetRuntimeMemoryAllocationCallback(m_runtime, nullptr, nullptr); - ThrowUponChakraError(JsDisposeRuntime(m_runtime), "JsDisposeRuntime"); + VerifyChakraErrorElseThrow(JsDisposeRuntime(m_runtime)); } #pragma region Functions_inherited_from_Runtime @@ -192,7 +190,7 @@ facebook::jsi::Value ChakraRuntime::evaluatePreparedJavaScript( facebook::jsi::Object ChakraRuntime::global() { JsValueRef global; - ThrowUponJsError(JsGetGlobalObject(&global), "JsGetGlobalObject"); + VerifyJsErrorElseThrow(JsGetGlobalObject(&global)); return MakePointer(global); } @@ -227,18 +225,15 @@ facebook::jsi::Runtime::PointerValue *ChakraRuntime::clonePropNameID( facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromAscii( const char *str, size_t length) { - // Unfortunately due to the typing used by JSI and Chakra(Core), we have to do - // a double reinterpret cast here. - return createPropNameIDFromUtf8( - Common::Utilities::CheckedReinterpretCast(str), length); + ChakraObjectRef propId = GetPropertyId(std::string_view{str, length}); + return MakePointer(std::move(propId)); } facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromUtf8( const uint8_t *utf8, size_t length) { - ChakraObjectRef propId = GetPropertyId( + return createPropNameIDFromAscii( Common::Utilities::CheckedReinterpretCast(utf8), length); - return MakePointer(std::move(propId)); } facebook::jsi::PropNameID ChakraRuntime::createPropNameIDFromString( @@ -277,8 +272,8 @@ facebook::jsi::String ChakraRuntime::createStringFromAscii( facebook::jsi::String ChakraRuntime::createStringFromUtf8( const uint8_t *str, size_t length) { - return MakePointer(ToJsString( - Common::Utilities::CheckedReinterpretCast(str), length)); + return MakePointer(ToJsString(std::string_view{ + Common::Utilities::CheckedReinterpretCast(str), length})); } std::string ChakraRuntime::utf8(const facebook::jsi::String &str) { @@ -287,7 +282,7 @@ std::string ChakraRuntime::utf8(const facebook::jsi::String &str) { facebook::jsi::Object ChakraRuntime::createObject() { JsValueRef obj; - ThrowUponJsError(JsCreateObject(&obj), "JsCreateObject"); + VerifyJsErrorElseThrow(JsCreateObject(&obj)); return MakePointer(obj); } @@ -331,9 +326,8 @@ facebook::jsi::Value ChakraRuntime::getProperty( const facebook::jsi::Object &obj, const facebook::jsi::PropNameID &name) { JsValueRef result = nullptr; - ThrowUponJsError( - JsGetProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name), &result), - "JsGetProperty"); + VerifyJsErrorElseThrow(JsGetProperty( + GetChakraObjectRef(obj), GetChakraObjectRef(name), &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -347,9 +341,8 @@ bool ChakraRuntime::hasProperty( const facebook::jsi::Object &obj, const facebook::jsi::PropNameID &name) { bool result = false; - ThrowUponJsError( - JsHasProperty(GetChakraObjectRef(obj), GetChakraObjectRef(name), &result), - "JsHasProperty"); + VerifyJsErrorElseThrow(JsHasProperty( + GetChakraObjectRef(obj), GetChakraObjectRef(name), &result)); return result; } @@ -367,13 +360,11 @@ void ChakraRuntime::setPropertyValue( // silently fails in normal code (assignment to a non-writable global or // property, assignment to a getter-only property, assignment to a new // property on a non-extensible object) will throw. - ThrowUponJsError( - JsSetProperty( - GetChakraObjectRef(object), - GetChakraObjectRef(name), - ToChakraObjectRef(value), - true /* useStrictRules */), - "JsSetProperty"); + VerifyJsErrorElseThrow(JsSetProperty( + GetChakraObjectRef(object), + GetChakraObjectRef(name), + ToChakraObjectRef(value), + true /* useStrictRules */)); } void ChakraRuntime::setPropertyValue( @@ -463,9 +454,8 @@ facebook::jsi::Array ChakraRuntime::createArray(size_t length) { assert(length <= UINT_MAX); JsValueRef result = nullptr; - ThrowUponJsError( - JsCreateArray(static_cast(length), &result), - "JsCreateArray"); + VerifyJsErrorElseThrow( + JsCreateArray(static_cast(length), &result)); return MakePointer(result).asArray(*this); } @@ -487,9 +477,8 @@ uint8_t *ChakraRuntime::data(const facebook::jsi::ArrayBuffer &arrBuf) { uint8_t *buffer = nullptr; unsigned int size = 0; - ThrowUponJsError( - JsGetArrayBufferStorage(GetChakraObjectRef(arrBuf), &buffer, &size), - "JsGetArrayBufferStorage"); + VerifyJsErrorElseThrow( + JsGetArrayBufferStorage(GetChakraObjectRef(arrBuf), &buffer, &size)); return buffer; } @@ -513,12 +502,8 @@ facebook::jsi::Value ChakraRuntime::getValueAtIndex( assert(index <= INT_MAX); JsValueRef result = nullptr; - ThrowUponJsError( - JsGetIndexedProperty( - GetChakraObjectRef(arr), - ToJsNumber(static_cast(index)), - &result), - "JsGetIndexedProperty"); + VerifyJsErrorElseThrow(JsGetIndexedProperty( + GetChakraObjectRef(arr), ToJsNumber(static_cast(index)), &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -529,12 +514,10 @@ void ChakraRuntime::setValueAtIndexImpl( assert(isArray(arr)); assert(index <= INT_MAX); - ThrowUponJsError( - JsSetIndexedProperty( - GetChakraObjectRef(arr), - ToJsNumber(static_cast(index)), - ToChakraObjectRef(value)), - "JsSetIndexedProperty"); + VerifyJsErrorElseThrow(JsSetIndexedProperty( + GetChakraObjectRef(arr), + ToJsNumber(static_cast(index)), + ToChakraObjectRef(value))); } facebook::jsi::Function ChakraRuntime::createFunctionFromHostFunction( @@ -543,20 +526,18 @@ facebook::jsi::Function ChakraRuntime::createFunctionFromHostFunction( facebook::jsi::HostFunctionType func) { // Currently, we are allocating this proxy object in heap .. and deleting it // whenever the JS object is garbage collected. + // TODO (yicyao): We should get rid of these naked new and delete calls using + // the same trick in ToJsObject and ToJsArrayBuffer. HostFunctionProxy *hostFuncProxy = new HostFunctionProxy(func, *this); JsValueRef funcRef; - ThrowUponJsError( - JsCreateFunction( - ChakraRuntime::HostFunctionCall, hostFuncProxy, &funcRef), - "JsCreateFunction"); - - ThrowUponJsError( - JsSetObjectBeforeCollectCallback( - funcRef, - hostFuncProxy, - [](JsRef ref, void *hostFuncProxy) { delete hostFuncProxy; }), - "JsSetObjectBeforeCollectCallback"); + VerifyJsErrorElseThrow(JsCreateFunction( + ChakraRuntime::HostFunctionCall, hostFuncProxy, &funcRef)); + + VerifyJsErrorElseThrow(JsSetObjectBeforeCollectCallback( + funcRef, hostFuncProxy, [](JsRef ref, void *hostFuncProxy) { + delete hostFuncProxy; + })); return MakePointer(funcRef).getFunction(*this); } @@ -576,13 +557,11 @@ facebook::jsi::Value ChakraRuntime::call( assert(argsWithThis.size() <= USHRT_MAX); JsValueRef result; - ThrowUponJsError( - JsCallFunction( - GetChakraObjectRef(func), - argsWithThis.data(), - static_cast(argsWithThis.size()), - &result), - "JsCallFunction"); + VerifyJsErrorElseThrow(JsCallFunction( + GetChakraObjectRef(func), + argsWithThis.data(), + static_cast(argsWithThis.size()), + &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -600,13 +579,11 @@ facebook::jsi::Value ChakraRuntime::callAsConstructor( assert(argsWithThis.size() <= USHRT_MAX); JsValueRef result; - ThrowUponJsError( - JsConstructObject( - GetChakraObjectRef(func), - argsWithThis.data(), - static_cast(argsWithThis.size()), - &result), - "JsConstructObject"); + VerifyJsErrorElseThrow(JsConstructObject( + GetChakraObjectRef(func), + argsWithThis.data(), + static_cast(argsWithThis.size()), + &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -616,7 +593,7 @@ facebook::jsi::Runtime::ScopeState *ChakraRuntime::pushScope() { void ChakraRuntime::popScope(Runtime::ScopeState *state) { assert(state == nullptr); - ThrowUponJsError(JsCollectGarbage(m_runtime), "JsCollectGarbage"); + VerifyJsErrorElseThrow(JsCollectGarbage(m_runtime)); } bool ChakraRuntime::strictEquals( @@ -641,9 +618,8 @@ bool ChakraRuntime::instanceOf( const facebook::jsi::Object &obj, const facebook::jsi::Function &func) { bool result; - ThrowUponJsError( - JsInstanceOf(GetChakraObjectRef(obj), GetChakraObjectRef(func), &result), - "JsInstanceOf"); + VerifyJsErrorElseThrow( + JsInstanceOf(GetChakraObjectRef(obj), GetChakraObjectRef(func), &result)); return result; } @@ -663,7 +639,7 @@ facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { } case JsNumber: { double number; - ThrowUponJsError(JsNumberToDouble(ref, &number), "JsNumberToDouble"); + VerifyJsErrorElseThrow(JsNumberToDouble(ref, &number)); return facebook::jsi::Value(number); break; } @@ -674,7 +650,7 @@ facebook::jsi::Value ChakraRuntime::ToJsiValue(ChakraObjectRef &&ref) { } case JsBoolean: { bool b; - ThrowUponJsError(JsBooleanToBool(ref, &b), "JsBooleanToBool"); + VerifyJsErrorElseThrow(JsBooleanToBool(ref, &b)); return facebook::jsi::Value(b); break; } @@ -709,37 +685,26 @@ ChakraObjectRef ChakraRuntime::ToChakraObjectRef( const facebook::jsi::Value &value) { if (value.isUndefined()) { JsValueRef ref; - ThrowUponJsError(JsGetUndefinedValue(&ref), "JsGetUndefinedValue"); + VerifyJsErrorElseThrow(JsGetUndefinedValue(&ref)); return ChakraObjectRef(ref); } else if (value.isNull()) { JsValueRef ref; - ThrowUponJsError(JsGetNullValue(&ref), "JsGetNullValue"); + VerifyJsErrorElseThrow(JsGetNullValue(&ref)); return ChakraObjectRef(ref); } else if (value.isBool()) { JsValueRef ref; - ThrowUponJsError(JsBoolToBoolean(value.getBool(), &ref), "JsBoolToBoolean"); + VerifyJsErrorElseThrow(JsBoolToBoolean(value.getBool(), &ref)); return ChakraObjectRef(ref); } else if (value.isNumber()) { JsValueRef ref; - ThrowUponJsError( - JsDoubleToNumber(value.asNumber(), &ref), "JsDoubleToNumber"); + VerifyJsErrorElseThrow(JsDoubleToNumber(value.asNumber(), &ref)); return ChakraObjectRef(ref); - } else if (value.isSymbol()) { - return GetChakraObjectRef(value.asSymbol(*this)); - - } else if (value.isString()) { - return GetChakraObjectRef(value.asString(*this)); - - } else if (value.isObject()) { + } else { // value.isSymbol() || value.isString() || value.isObject() return GetChakraObjectRef(value.asObject(*this)); - - } else { - // Control flow should never reach here. - std::terminate(); } } @@ -778,16 +743,12 @@ ChakraRuntime::ObjectWithExternalData::fromExisting( template T *ChakraRuntime::ObjectWithExternalData::getExternalData() { T *externalData; - ThrowUponChakraError( - JsGetExternalData( - GetChakraObjectRef(*this), reinterpret_cast(&externalData)), - "JsGetExternalData"); + VerifyChakraErrorElseThrow(JsGetExternalData( + GetChakraObjectRef(*this), reinterpret_cast(&externalData))); return externalData; } -void ChakraRuntime::ThrowUponJsError( - JsErrorCode error, - const char *const chakraApiName) { +void ChakraRuntime::VerifyJsErrorElseThrow(JsErrorCode error) { switch (error) { case JsNoError: { return; @@ -796,7 +757,7 @@ void ChakraRuntime::ThrowUponJsError( case JsErrorScriptException: { JsValueRef jsError; - CrashUponChakraError(JsGetAndClearException(&jsError)); + VerifyChakraErrorElseThrow(JsGetAndClearException(&jsError)); throw facebook::jsi::JSError( "A JavaScript Error was thrown.", *this, @@ -805,7 +766,7 @@ void ChakraRuntime::ThrowUponJsError( } default: { - ThrowUponChakraError(error, chakraApiName); + VerifyChakraErrorElseThrow(error); break; } } // switch (error) @@ -1024,13 +985,14 @@ JsValueRef CALLBACK ChakraRuntime::HostFunctionCall( } catch (const std::exception &ex) { std::string exwhat(ex.what()); JsValueRef exn; - exn = ToJsString(exwhat.c_str(), exwhat.size()); + exn = ToJsString(std::string_view{exwhat.c_str(), exwhat.size()}); JsSetException(exn); } catch (...) { std::string exceptionString("Exception in HostFunction: "); JsValueRef exn; - exn = ToJsString(exceptionString.c_str(), exceptionString.size()); + exn = ToJsString( + std::string_view{exceptionString.c_str(), exceptionString.size()}); JsSetException(exn); } diff --git a/vnext/JSI/Shared/ChakraRuntime.h b/vnext/JSI/Shared/ChakraRuntime.h index 365648e6908..33d7a471b97 100644 --- a/vnext/JSI/Shared/ChakraRuntime.h +++ b/vnext/JSI/Shared/ChakraRuntime.h @@ -85,6 +85,8 @@ class ChakraRuntime : public facebook::jsi::Runtime { std::string symbolToString(const facebook::jsi::Symbol &s) override; + // Despite its name, createPropNameIDFromAscii is the same function as + // createStringFromUtf8. facebook::jsi::String createStringFromAscii(const char *str, size_t length) override; facebook::jsi::String createStringFromUtf8(const uint8_t *utf8, size_t length) @@ -338,7 +340,7 @@ class ChakraRuntime : public facebook::jsi::Runtime { template friend class ObjectWithExternalData; - void ThrowUponJsError(JsErrorCode error, const char *const chakraApiName); + void VerifyJsErrorElseThrow(JsErrorCode error); facebook::jsi::Object createProxy( facebook::jsi::Object &&target, From bf167315d8aacb2f286244bf857351352c3a5415 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Fri, 18 Oct 2019 16:00:38 -0700 Subject: [PATCH 14/16] yarn format. --- vnext/JSI/Shared/ChakraObjectRef.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index 2fb19aeeeb7..d379ff4ad51 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -34,7 +34,7 @@ void VerifyChakraErrorElseThrow(JsErrorCode error); * that JsAddRef and JsRelease are called upon initialization and invalidation, * respectively. It also allows users to implicitly convert it into a JsRef. A * ChakraObjectRef must only be initialized once and invalidated once. - */ + */ class ChakraObjectRef { public: ChakraObjectRef() noexcept {} @@ -132,7 +132,7 @@ std::wstring ToStdWstring(const ChakraObjectRef &jsString); * * @remarks The content of utf8 is copied into JS engine owned memory. When * using Chakra instead of ChakraCore, this function incurs a UTF-8 to UTF-16 - * conversion. + * conversion. */ ChakraObjectRef ToJsString(const std::string_view &utf8); From e6afbb5f1e19fb56381ab24b09fd807072c92f02 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Fri, 18 Oct 2019 16:24:45 -0700 Subject: [PATCH 15/16] Fix Universal build. --- vnext/JSI/Shared/ChakraObjectRef.h | 2 ++ vnext/JSI/Shared/ChakraRuntime.cpp | 12 +++++++++++- vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp | 14 ++++++-------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/vnext/JSI/Shared/ChakraObjectRef.h b/vnext/JSI/Shared/ChakraObjectRef.h index d379ff4ad51..10243499e75 100644 --- a/vnext/JSI/Shared/ChakraObjectRef.h +++ b/vnext/JSI/Shared/ChakraObjectRef.h @@ -8,7 +8,9 @@ #ifdef CHAKRACORE #include "ChakraCore.h" #else +#ifndef USE_EDGEMODE_JSRT #define USE_EDGEMODE_JSRT +#endif #include #endif diff --git a/vnext/JSI/Shared/ChakraRuntime.cpp b/vnext/JSI/Shared/ChakraRuntime.cpp index 6d373bbeb64..179dee3145f 100644 --- a/vnext/JSI/Shared/ChakraRuntime.cpp +++ b/vnext/JSI/Shared/ChakraRuntime.cpp @@ -703,8 +703,18 @@ ChakraObjectRef ChakraRuntime::ToChakraObjectRef( VerifyJsErrorElseThrow(JsDoubleToNumber(value.asNumber(), &ref)); return ChakraObjectRef(ref); - } else { // value.isSymbol() || value.isString() || value.isObject() + } else if (value.isSymbol()) { + return GetChakraObjectRef(value.asSymbol(*this)); + + } else if (value.isString()) { + return GetChakraObjectRef(value.asString(*this)); + + } else if (value.isObject()) { return GetChakraObjectRef(value.asObject(*this)); + + } else { + // Control flow should never reach here. + std::terminate(); } } diff --git a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp index e8ebf9d5c90..54fb73e6e5c 100644 --- a/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp +++ b/vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp @@ -61,13 +61,11 @@ facebook::jsi::Value ChakraRuntime::evaluateJavaScriptSimple( const std::wstring url16 = Microsoft::Common::Unicode::Utf8ToUtf16(sourceURL); JsValueRef result; - ThrowUponJsError( - JsRunScript( - script16.c_str(), - JS_SOURCE_CONTEXT_NONE /*sourceContext*/, - url16.c_str(), - &result), - "JsRunScript"); + VerifyJsErrorElseThrow(JsRunScript( + script16.c_str(), + JS_SOURCE_CONTEXT_NONE /*sourceContext*/, + url16.c_str(), + &result)); return ToJsiValue(ChakraObjectRef(result)); } @@ -95,7 +93,7 @@ bool ChakraRuntime::evaluateSerializedScript( } else if (ret == JsErrorBadSerializedScript) { return false; } else { - ThrowUponChakraError(ret, "JsRunSerializedScript"); + VerifyChakraErrorElseThrow(ret); return true; } } From bda9d6c7c37547557230b4e6a41ad36c24cc0616 Mon Sep 17 00:00:00 2001 From: Yichen Yao Date: Sun, 20 Oct 2019 20:25:19 -0700 Subject: [PATCH 16/16] Trigger automation.