Introduce ChakraObjectRef and refactor ChakraRuntime #3445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

hansenyy merged 18 commits into microsoft:master from hansenyy:CharkaObjectRef

Oct 21, 2019

Contributor

hansenyy commented Oct 17, 2019 •

edited by ghost

Loading

This PR:

Introduces ChakraObjectRef, an RAII wrapper for JsRefs.
Replaces ChakraPropertyIdValue, ChakraStringValue, ChakraObjectValue, and ChakraWeakRefValue with a new class ChakraPointerValue, which is implemented on top of ChakraObjectRef.
Fix property ID comparison for ChakraRuntime.
Temporarily disables weak ref semantics for ChakraCore based Runtime. This functionality will be re-enabled in a future PR.

Microsoft Reviewers: Open in CodeFlow

hansenyy added 9 commits

October 11, 2019 17:01


          Add ChakraObjectRef.

746b982


          Add some ChakraObjectRef helper functions.

e7d6f4b


          Add more ChakraObjectRef helpers.

1e93842


          Introduce ChakraPointerValue and reimplement many Runtime functions w…

d6f050a

…ith it.


          Make ChakraJsiRuntime_core.cpp build.

b173de5


          Fix Desktop build.

9ff342c


          Bug fixes.

ca60dd1


          Fix Universal build.

af2be74


          Change files

7b8cdac

hansenyy requested review from a team, NikoAri, mganandraj and tudorms

October 17, 2019 00:56

hansenyy requested a review from a team as a code owner

October 17, 2019 00:56

ghost added the vnext label

hansenyy added 2 commits

October 16, 2019 17:59


          Remove some useless comments

de930cc


          Revert some reordering for more reviewable diff.

8b9c4d6

NikoAri reviewed

View reviewed changes

vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

hansenyy mentioned this pull request

Make sure that react-native-windows crashes generate Watson reports. #3454

Closed

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.cpp Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Outdated Show resolved Hide resolved

NikoAri approved these changes

View reviewed changes

NikoAri left a comment


          Merge branch 'master' into CharkaObjectRef

90c8044

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Outdated Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Outdated Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp

    
              }

              ChakraObjectRef ChakraRuntime::ToChakraObjectRef(

                  const facebook::jsi::Value &value) {

                if (value.isUndefined()) {

Member

tudorms Oct 18, 2019 •

edited by hansenyy

Loading

if (value.isUndefined()) { [](start = 2, length = 26)

(Nit) There is some duplication here for the various JsValueRef types; see if there's an opportunity to refactor #Pending

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraRuntime.cpp Show resolved Hide resolved

tudorms reviewed

View reviewed changes

vnext/JSI/Shared/ChakraObjectRef.h Show resolved Hide resolved

hansenyy added 6 commits

October 17, 2019 19:06


          Fix E2E test app.


          Address feedback from PR microsoft#3445.

7daa7d2


          yarn format.

bf16731


          Fix Universal build.

e6afbb5


          Merge branch 'master' into CharkaObjectRef

deb974e


          Trigger automation.

bda9d6c

hansenyy merged commit 715de93 into microsoft:master

hansenyy deleted the CharkaObjectRef branch

October 21, 2019 05:20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet