Enable array buffers in JSCRuntime.cpp

[ryantrem]

Summary

The JavaScriptCore implementation of JSI does not currently support array buffers. The comments in the code suggest the JSC version used by React Native does not work with array buffers, but this seems to be out of date since the current version of JSC used by React Native does indeed support array buffers. This change just enables array buffers in JSCRuntime.cpp.

NOTE: See react-native-community/discussions-and-proposals#91 (comment) for more background on this change.

Changelog

[General] [Added] - Support for array buffers in the JavaScriptCore implementation of JSI

Test Plan

To test these changes, I just made some temporary changes to RNTester to use JSI to inject a test function into the JS runtime that reads from and writes to an array buffer, and call that function from the JS of the RNTester app (see ryantrem@28152ce).

For the JS side of this, specifically look at https://github.com/ryantrem/react-native/blob/28152ce3f4ae0fa906557415d106399b3f072118/RNTester/js/RNTesterApp.android.js#L13-L18

For the native side of this, specifically look at https://github.com/ryantrem/react-native/blob/28152ce3f4ae0fa906557415d106399b3f072118/RNTester/android/app/src/main/cpp/JSITest.cpp#L22-L38

[ryantrem]

FYI @mhorowitz, @tmikov, @bghgary.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	10,738,000	16,304

Base commit: f438a6e

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,780,258	15,628
android	hermes	armeabi-v7a	6,442,477	16,108
android	hermes	x86	7,165,449	16,074
android	hermes	x86_64	7,056,016	16,084
android	jsc	arm64-v8a	8,948,058	14,971
android	jsc	armeabi-v7a	8,602,657	15,439
android	jsc	x86	8,776,347	15,405
android	jsc	x86_64	9,352,558	15,417

Base commit: f438a6e

[facebook-github-bot]

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[ryantrem]

Worth noting, this existing define looks wrong to me. I believe _JSC_FAST_IS_ARRAY will never be defined on Android, so will always use the slow version. Presumably this should be fixed, though I didn't really want to overload this PR.

[facebook-github-bot]

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[ryantrem]

From what I can tell, there is no way for me to see what the warning is that is causing the "Facebook Internal - Builds & Tests" check to fail, is that right? Assuming yes, can someone (@shergin?) provide some details if there is something more I need to address for this?

[ryantrem]

Ping! @shergin, @mhorowitz, @tmikov - can anyone help here? I'd like to see this PR merged, but I don't know what the build warning is. I don't get any build warnings in either XCode or Android Studio when building locally.

[tmikov]

AFAICT, it is a clang-format warning on line 944. The line is too long and needs to be reformatted.

[ryantrem]

@shergin or @tmikov - thanks for your help so far on this PR. Can one of you re-"import" this PR after my latest change (which I believe should address the issue @tmikov mentioned above)?

[tmikov]

@ryantrem the original importer "owns" the diff internally, so ordinarily only they can re-import it. I will reach out to @shergin internally.

[ryantrem]

Hi again @tmikov and @shergin - sorry to nag, but since another week has passed I wanted to follow up on this and see if there has been any progress, or if you need anything else from me.

[facebook-github-bot]

@tmikov has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[tmikov]

@ryantrem sorry for the delay. We discussed it with @shergin and we will proceed to land it shortly.

[react-native-bot]

This pull request was successfully merged by @ryantrem in 9c32140.

^{When will my fix make it into a release? | Upcoming Releases}