Summary:
## Summary
Please check out D21035208.
## Changes
- Update `RCTTurboModuleManagerDelegate getTurboModule:instance:jsInvoker:nativeInvoker:perfLogger` to use `RCTTurboModuleManagerDelegate getTurboModule:(const ObjCTurboModule::InitParams)`
- Update all implementations of `RCTTurboModuleManagerDelegate` in accordance with this API change
Changelog:
[iOS][Changed] - Make RCTTurboModuleManagerDelegate create TurboModules via ObjCTurboModuleManager::InitParams
Reviewed By: PeteTheHeat
Differential Revision: D21036272
fbshipit-source-id: c16002c47db26e2ba143fc1080afe9e2fe1e7816
Summary:
## Summary
Please check out D21035208.
## Changes
- `ObjCTurboModule::ObjCTurboModule` changed to accept a bag of arguments `const ObjCTurboModule::InitParams` instead of an argument list.
- TurboModule iOS codegen scripts updated to generated `ObjCTurboModule` subclasses that accept a `const ObjCTurboModule::InitParams` object in their constructor, and forward it to `ObjCTurboModule::ObjCTurboModule`.
- All manually checked in code-generated ObjC++ classes (i.e: RCTNativeSampleTurboModule, RCTTestModule, FBReactNativeSpec) are updated.
## Rationale
This way, the code-gen can remain constant while we add, remove, or modify the arguments passed to ObjCTurboModule.
## Commands run
```
function update-codegen() {
pushd ~/fbsource && js1 build oss-native-modules-specs -p ios && js1 build oss-native-modules-specs -p android && popd;
}
> update-codegen
```
Changelog:
[iOS][Changed] Update ObjCTurboModule to use ObjCTurboModule::InitParams
Reviewed By: PeteTheHeat
Differential Revision: D21036266
fbshipit-source-id: 6584b0838dca082a69e8c14c7ca50c3568b95086
Summary:
## Problem
Every time we want to add, remove, or change the data passed to JavaTurboModule's constructor, we have to modify the C++ TurboModule codegen. (The same is true of `ObjCTurboModule`).
**Why was this necessary?**
- `JavaTurboModule` is effectively an abstract class whose constructor is always invoked by code-generated C++ classes. These C++ code-generated class constructors accept an argument list, and manually foward each and every item in that list to `JavaTurboModule::JavaTurboModule`.
## The fix
In this diff, I introduce a struct `JavaTurboModule::InitParams`, to represent a bag of arguments:
```
class JSI_EXPORT JavaTurboModule : public TurboModule {
public:
struct InitParams {
std::string moduleName;
jni::alias_ref<JTurboModule> instance;
std::shared_ptr<CallInvoker> jsInvoker;
std::shared_ptr<CallInvoker> nativeInvoker;
};
```
All `JavaTurboModules` will be created with an instance of this `InitParams` struct, instead of a list of arguments. Our code-generated C++ `jsi::HostObject` sublcasses will simply accept `InitParams` in their constructor, and forward it to `JavaTurboModule`'s constructor. This way, the codegen remains oblivious to what arguments JavaTurboModule requires.
## Okay, but why do we need this change now?
In the future, I plan to modify the constructor for `JavaTurboModule` to accept a performance logger, and a `RuntimeExecutor`. Similar modifications are planned for ObjC. For this reason, to avoid these four codemods, and any potential other codemods that occur because we're making modifications to `JavaTurboModule` or `ObjCTurboModule`, I'm launching this codemod, and the codemods in this stack.
## Misc Fix
- Previously, we were generating the TurboModule name from the Spec filename. This is incorrect because that name represents the spec name. Now, the name will be forwarded from TurboModuleManager in the `JavaTurboModule::InitParams` struct.
## Alternative implementations
I initially considered using `ContextContainer`, but decided against it because:
1. There are no type-safety guarantees.
2. I think it's a bit overkill for this scenario. We just need an opaque bag of data, and for our purposes a simple struct does the job fine.
## Commands run
Reviewed By: fkgozali
Differential Revision: D21035208
fbshipit-source-id: 9542cafea192081bc34d337ab3a7a783083eb06c
Summary:
This diff:
1. Has ObjC NativeModules use the native `CallInvoker` to invoke JS -> native sync/async calls.
2. Integrates the native `CallInvoker` for each ObjC NativeModule with the bridge. This way, the bridge is informed of all JS -> native TurboModule method calls, and dispatches `onBatchComplete` appropriately.
Changelog:
[iOS][Fixed] Integrate ObjC TurboModules async method calls with the bridge
Reviewed By: fkgozali
Differential Revision: D20831545
fbshipit-source-id: da1cbb4ecef4cae85841ca7ef625ab8e380760cd
Summary:
## Problem:
Let `A` be the set of all ObjC NativeModules that neither provide nor reqeust a method queue.
The TurboModule system dispatches all method calls to NativeModules in `A` synchronously to the JS thread. Here is the relevant logic:
**RCTTurboModule.mm:**
Link: https://fburl.com/diffusion/nz9gqje8
```
jsi::Value performMethodInvocation(
// ...
)
{
// ...
dispatch_queue_t methodQueue = NULL;
if ([instance_ conformsToProtocol:protocol(RCTBridgeModule)] &&
[instance_ respondsToSelector:selector(methodQueue)]) {
methodQueue = [instance_ performSelector:selector(methodQueue)];
}
if (methodQueue == NULL || methodQueue == RCTJSThread) {
// This is the default mode of execution: on JS thread.
block();
} else if (methodQueue == dispatch_get_main_queue()) {
```
**Why does this end up happening?**
1. NativeModules that request a method queue have `synthesize methodQueue = _methodQueue` in their `implementation` section. This generates a `methodQueue` getter for the NativeModule, and also creates an ivar to back that getter. The TurboModule system generates a `dispatch_queue_t` and uses ObjC's KVC API to write to the ivar. So in the above logic, for NativeModules that provide a method queue, methodQueue will neither be `NULL` nor `RCTJSThread`, so we don't dispatch synchronously to the JS thread.
2. NativeModules that provide a method queue will return something that is not `NULL` or something that is `RCTJSThread`. If they return `NULL`, the infra will throw an error early. If they return `RCTJSThread`, we'll dispatch synchronously to the JS thread, as we should (...wait. For async NativeModule methods that dispatch to `RCTJSThread`, should we dispatch asynchronously to the JS thread, via jsInvoker? **Edit:** Nope: https://fburl.com/diffusion/ivt9b40s.). In all other cases, we dispatch to appropriately to the respective method queue.
3. For NativeModules that neither provide nor request a method queue (i.e: NativeModules in `A`), they don't implement the `methodQueue` selector. Therefore, we dispatch synchronously to the JS thread.
## The fix (Part 1):
The first step towards fixing this problem is to generate `dispatch_queue_t`s for NativeModules in `A`.
That's what this diff accomplishes.
Changelog:
[iOS][Fixed] - Create method queue for NativeModules that don't provide nor request one.
Reviewed By: fkgozali
Differential Revision: D20821054
fbshipit-source-id: 17a73550ad96766c5c7e719e28e1cc879e36465c
Summary:
`RCTTurboModuleManager` will create a native `CallInvoker` for each ObjC NativeModule. This `CallInvoker` will be used to dispatch calls from JS to native. Before passing the native `CallInvoker` to the `ObjCTurboModule`, it'll first use `RCTCxxBridge decorateNativeCallInvoker` to get a bridge-aware decorated native `CallInvoker`. That way, the bridge remains informed about async TurboModule method calls that took place since the last time it was flushed. This ensures that we don't end up dispatching `onBatchComplete` any less with TurboModules on than we do with TurboModules off.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D20831546
fbshipit-source-id: b2eb4e0097e0dabf8c4bd8fdc4c850a0858af699
Summary:
Might be worthwhile to just kill this method instead, since we're having all NativeModules provide their TurboModule jsi::HostObjects. But I'll leave that decision to a later time.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D20809201
fbshipit-source-id: ee73d4b5454a76460832a54f9b864841e5b2b9c0
Summary:
This is necessary to integrate TurboModule async method dispatch with the bridge's `onBatchComplete` event. See D20717931 for more details.
This diff is similar to D20480971.
**Note:** This stack doesn't really make any functional changes, since the native CallInvoker is `nullptr` right now.
Changelog:
[Internal]
Reviewed By: fkgozali
Differential Revision: D20809199
fbshipit-source-id: bf465a3a51bdddb8b56d1e696ca510fdf071f9ec
Summary:
## Context
For now, assume TurboModules doesn't exist.
**What happens when we call an async NativeModule method?**
Everytime JS calls an async NativeModule method, we don't immediately execute it. The legacy infra pushes the call into some queue managed by `MessageQueue.js`. This queue is "flushed" or "emptied" by the following events:
- **Flushed:** A C++ -> JS call. NativeModule async methods can called with an `onSuccess` and/or `onFail` callback(s). Calling `NativeToJsBridge::invokeCallback` to invoke one of these callbacks is one way for ObjC++/C++/Java to call into JS. Another way is via JSModule method calls, which are initiated by `NativeToJsBridge::callFunction`.
- **Flushed:** When `JSIExecutor::flush` is called. Since TurboModules don't exist, this only happens when we call `JSIExecutor::loadApplicationScript`.
- **Emptied:** When more than 5 ms have passed, and the queue hasn't been flushed/emptied, on the next async NativeModule method call, we add to the queue. Afterwards, we empty it, and invoke all the NativeModule method calls.
**So, what's the difference between flushed and emptied?**
> Note: These are two terms I just made up, but the distinction is important.
If the queue was "flushed", and it contained at least one NativeModule method call, `JsToNativeBridge` dispatches the `onBatchComplete` event. On Android, the UIManager module is the only module that listens to this event. This `onBatchComplete` event doesn't fire if the queue was "emptied".
**Why does any of this matter?**
1. TurboModules exist.
2. We need the TurboModules infra to have `JsToNativeBridge` dispatch `onBatchComplete`, which depends on:
- **Problem 1:** The queue being flushed on calls into JS from Java/C++/ObjC++.
- **Problem 2:** There being queued up NativeModule async method calls when the queue is flushed.
In D14656466, fkgozali fixed Problem 1 by making every C++/Java/Obj -> JS call from TurboModules also execute `JSIExecutor::flush()`. This means that, with TurboModules, we flush the NativeModule async method call queue as often as we do without TurboModules. So far, so good. However, we still have one big problem: As we convert more NativeModules to TurboModules, the average size of the queue of NativeModule method calls will become smaller and smaller, because more NativeModule method calls will be TurboModule method calls. This queue will more often be empty than not. Therefore, we'll end up dispatching the `onBatchComplete` event less often with TurboModules enabled. So, somehow, when we're about to flush the NativeModule method call queue, we need `JsToNativeBridge` to understand that we've executed TurboModule method calls in the batch. These calls would have normally been queued, which would have led the queue size to be non-zero. So if, during a batch, some TurboModule async method calls were executed, `JsToNativeBridge` should dispatch `onBatchComplete`.
**So, what does this diff do?**
1. Make `Instance` responsible for creating the JS `CallInvoker`.
2. Make `NativeToJsBridge` responsible for creating the native `CallInvoker`. `Instance` calls into `NativeToJsBridge` to get the native `CallInvoker`.
3. Hook up `CatalystInstanceImpl`, the Android bridge, with the new JS `CallInvoker`, and the new native `CallInvoker`. This fixes `onBatchComplete` on Android. iOS work is pending.
Changelog:
[Android][Fixed] - Ensure `onBatchComplete` is dispatched correctly with TurboModules
Reviewed By: mdvacca
Differential Revision: D20717931
fbshipit-source-id: bc3ccbd6c135b7f084edbc6ddb4d1e3c0c7e0875
Summary:
`fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency.
Changelog: [Internal]
Reviewed By: scottrice
Differential Revision: D20495655
fbshipit-source-id: a57b72f694c533e2e16dffe74eccb8fdec1f55f5
Summary:
Just includes the manual changes to get the perf logger moved to the ObjCTurboModule constructor.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D20498443
fbshipit-source-id: e607588f47ccc8ae199cd97043b09dfaff7a72dd
Summary:
Previously, I had logic inside `RCTTurboModuleManager` to attach the `id<RCTTurboModulePerformanceLogger>` to the `ObjCTurboModule` object
```
/**
* By default, all TurboModules are long-lived.
* Additionally, if a TurboModule with the name `name` isn't found, then we
* trigger an assertion failure.
*/
auto turboModule = [strongSelf provideTurboModule:moduleName];
/**
* TODO(T63718299): Move this setter into the ObjCTurboModule constructor
*/
if (performanceLogger) {
if (auto objCTurboModule = std::dynamic_pointer_cast<facebook::react::ObjCTurboModule>(turboModule)) {
objCTurboModule->setRCTTurboModulePerformanceLogger(performanceLogger);
};
}
```
This diff removes that logic in `RCTTurboModuleManager`, and it also removes `ObjCTurboModule::setRCTTurboModulePerformanceLogger`. Henceforth, we'll instead pass the `id<RCTTurboModulePerformanceLogger>` into `ObjCTurboModule`'s constructor. I've made all the necessary changes to the codegen scripts in this diff as well.
This should also resolve T63903079 by simply eliminating the code that's crashing production FB apps.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D20480971
fbshipit-source-id: c3899981f880aa5d1354b5c3f4018c8fd57c3bae
Summary:
In production, we'd like to be able to track the performance metrics of TurboModules. So, I'm introducing a new protocol called `RCTTurboModulePerformanceLogger`. It allows the application to be notified at points during stages of TurboModule creation, and TurboModule method invocation.
**Why can't we use `RCTPerformanceLogger`?**
`RCTPerformanceLogger` is meant to be used to profile RN initialization. It supports a fixed list of markers. Each marker has a type, and there can only be one instance of it. This isn't suitable for logging events in the TurboModule system.
Reviewed By: mdvacca
Differential Revision: D20310976
fbshipit-source-id: a49251b325c94f912cd35ab1cd8fc010f2cfc08f
Summary:
This can cause a deadlock if the main thread synchronously calls into some JS that creates a TurboModule. However, this is also the behaviour of legacy NativeModules. Furthermore, this also greatly simplifies how we'll fix async method invocation in TurboModules: T55255146.
Changelog:
[Internal]
Reviewed By: fkgozali
Differential Revision: D20364251
fbshipit-source-id: d0db85665506f08c51c2f33a123e69960923e7f3
Summary:
The new name is get_preprocessor_flags_for_build_mode.
Changelog: [Internal]
Reviewed By: d16r
Differential Revision: D20351718
fbshipit-source-id: 67628ce81e7244f0f72af2d00d92842a649ff619
Summary:
Updating react-native-windows to build with Visual Studio 2019 and the v142 toolset.
## Changelog
[Internal] [Fixed] - Fix MSVC build break in VS 2019
Pull Request resolved: https://github.com/facebook/react-native/pull/28253
Test Plan: No real change, only adding a missing include for `stdexcept`.
Reviewed By: cpojer
Differential Revision: D20321273
Pulled By: shergin
fbshipit-source-id: ffe8b4d3a96ce4ed1db303020d77a806637e88bc
Summary:
Adding a `.clang-tidy` to a bunch of dirs under `react-native-github/ReactAndroid` and `react-native-github/ReactCommon`.
I don't want to add a single `.clang-tidy` at the root because we'll need more fine-grained control over what checks are enabled in different parts of the codebase; for example, fabric will and TM will probably have more checks enabled than older parts of the codebase that we're not actively modernizing, and the Hermes team probably wants its own config to be consistent with the rest of their codebase.
Starting off each `.clang-tidy` by only enabling clang-diagnostic; this is just to test that it's working. In the future, we'll work with the community to gradually enable more checks.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19705749
fbshipit-source-id: 979cea053b645ac4a9790340033bfcfb49ca0f97
Summary:
After D19565499, the `LongLivedObjectCollection` will be cleared on the JS thread when the jsi::Runtime is deleted. This diff makes it so that we never hold strong references to `CallbackWrapper`s in our Android TurboModules infra. Therefore, we can leverage the changes in D19565499 to ensure that our `jsi::Function`s are deleted before the `jsi::Runtime`.
## Caveat
If you delete a TurboModule by itself, it's jsi::Functions that haven't been invoked won't be released. This is also the case for iOS. I plan to fix this for both iOS and Android at a later point in time.
Changelog:
[Android][Fixed] - Refactor jsi::Function cleanup in TurboModules
Reviewed By: mdvacca
Differential Revision: D19589151
fbshipit-source-id: efa3cc6c83634014159ac7500dcf6bef9c925762
Summary:
## Description
You're not supposed to hold on to JSI objects (ex: `jsi::Function`) past the point where their `jsi::Runtime` is deleted. Otherwise, we get a dangling pointer crash, like this: T60262810! Historically, this cleanup problem has always been really tricky to get right. With this diff, I hope to fix that problem once and for all by deleting all `jsi::Function`s when we delete the global `__turboModuleProxy` function.
## Current Setup
- The TurboModules infra uses weak references to `CallbackWrapper`s to hold on to the `jsi::Function`s passed from JS to ObjC.
- The LongLivedObjectCollection holds on to strong references to `CallbackWrapper`s. This ensures that the `jsi::Function`s aren't deleted prematurely. This also means that we can use `LongLivedObjectCollection` to delete all `CallbackWrappers`.
- `TurboModuleBinding` is the abstraction we use to install the global `__turboModuleProxy` function. It is owned by `TurboModuleManager`, and `TurboModuleManager` uses it to clear all references to `jsi::Function`s, when we delete all NativeModules.
## Solution
1. Transfer ownership of `TurboModuleBinding` from `TurboModuleManager` to the `__turboModuleProxy` function.
2. Clear the `LongLivedObjectCollection` when `TurboModuleBinding` is deleted.
Changelog:
[iOS][Fixed] - Clear all held jsi::Functions when jsi::Runtime is deleted
Reviewed By: JoshuaGross
Differential Revision: D19565499
fbshipit-source-id: e3510ea04e72f6bda363a8fc3ee2be60303b70a6
Summary:
The reason for this change is that it is the primary root that we want people to be using and the naming should reflect that.
#nocancel
build-break
overriding_review_checks_triggers_an_audit_and_retroactive_review
Changelog: [Internal]
Oncall Short Name: fbobjc_sheriff
Differential Revision: D19431128
fbshipit-source-id: c7208e20ed0f5f5eb6c2849428c09a6d4af9b6f3
Summary:
This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that `fbjni` no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.
**A non-exhaustive list of tasks:**
* [X] Gradle builds the SDK and RNTester for Android.
* [X] Buck build for rntester works in OSS.
* [ ] Move from `java-only` release to full `fbjni` release. This requires finding a solution for stripping out `.so` files that the old `Android.mk` insists on including in the final artifacts and will clash with the full distribution.
* [ ] Import this and fix potential internal build issues.
* [ ] Verify that the changes made to the Hermes integration don't have any unintended consequences.
## Changelog
[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls
Pull Request resolved: https://github.com/facebook/react-native/pull/27729
Test Plan:
- CI is already passing again for Gradle and Buck in OSS.
- After applying the following patch, RNTester builds and works with the latest Flipper SDK:
```
diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
--- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
debugImplementation files(hermesPath + "hermes-debug.aar")
releaseImplementation files(hermesPath + "hermes-release.aar")
- debugImplementation("com.facebook.flipper🐬0.23.4") {
+ debugImplementation("com.facebook.flipper🐬+") {
exclude group:'com.facebook.yoga'
- exclude group:'com.facebook.flipper', module: 'fbjni'
- exclude group:'com.facebook.litho', module: 'litho-annotations'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
}
if (useIntlJsc) {
```
Reviewed By: mdvacca
Differential Revision: D19345270
Pulled By: passy
fbshipit-source-id: 33811e7f97f44f2ec5999e1c35339909dc4fd3b1
Summary:
In bridgeless mode, `RCTTurboModuleManager` is initialized with a nil bridge. This has mostly worked, since `RCTBridge` doesn't do too many things for TMM (some notifs and perf markers). The one important thing it provides is a `_jsInvoker`.
In bridgeless mode, up until this point `_jsInvoker` has been nil, and turbo modules were not able to call functions on the JS thread. This diff fixes that.
Reviewed By: RSNara
Differential Revision: D19437174
fbshipit-source-id: 86bfc0a47bd9576e7d3203b860e86446eb0b63dd
Summary:
RCTTiming was the only NativeModule that relied on converting `double`s to other `double`s via `RCTConvert`. RCTTiming was made into a regular NativeModule in D18410788, so it's safe to strip out this logic.
Hopefully, this reduces the memory consumption enough to reduce the OOMs reported in T45151932.
Changelog:
[iOS][Removed] - Stop using RCTConvert to convert between primitive types
Reviewed By: fkgozali
Differential Revision: D18506069
fbshipit-source-id: 7316ad86bc84d47fb383735126d5b00e5491b371
Summary:
PushNotificationiOS wasn't used by anything in ReactInternal, so I just removed it from the target.
## Codemod
Everywhere we required `ReactInternal`, we now also require `RCTPushNotification`:
```
> xbgr -f 'BUCK$' 'ReactInternal"' -l | xargs -I {} sed -i '' $'s|ReactInternal",|ReactInternal",\"fbsource//xplat/js:RCTPushNotification",|g' $HOME/{}
> xbgr -f 'BUCK$' 'ReactInternalApple"' -l | xargs -I {} sed -i '' $'s|ReactInternalApple",|ReactInternalApple",\"fbsource//xplat/js:RCTPushNotificationApple",|g' $HOME/{}
> arc f
```
Changelog:
[Internal] - Separate RCTPushNotification from ReactInternal
Reviewed By: PeteTheHeat
Differential Revision: D18363643
fbshipit-source-id: b8d123f40741c6d200dc9e736e64e885c2572e15
Summary:
None of the code inside `ReactInternal` depended on the `RCTLinkingManager` NativeModule. So I extracted `RCTLinking` into its own BUCK target. This will make it easier to make `RCTLinkingManager` TurboModule-compatible.
## Codemod
Everywhere we required `ReactInternal`, we now also require `RCTLinking`:
```
> xbgr -f 'BUCK$' 'ReactInternal"' -l | xargs -I {} sed -i '' $'s|ReactInternal",|ReactInternal",\"fbsource//xplat/js:RCTLinking",|g' $HOME/{}
> xbgr -f 'BUCK$' 'ReactInternalApple"' -l | xargs -I {} sed -i '' $'s|ReactInternalApple",|ReactInternalApple",\"fbsource//xplat/js:RCTLinkingApple",|g' $HOME/{}
> arc f
```
Changelog:
[Internal] - Separate RCTLinking from ReactInternal
Reviewed By: fkgozali
Differential Revision: D18314747
fbshipit-source-id: d9b5f536a6e93a0aca8721801a2ee5d446e0d4a6
Summary:
A very common pattern I've seen in RN codebase:
- (instancetype) init {
[[NSNotificationCenter defaultCenter] addObserver:self ...]
}
- (void) dealloc {
[[NSNotificationCenter defaultCenter] removeObserver:self ...]
}
From Apple:
https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc
> If your app targets iOS 9.0 and later or macOS 10.11 and later, you don't need to unregister an observer in its dealloc method.
RN targets iOS9+
Changelog: [Internal][Cleanup] Remove unneeded NSNotification center removeObserver
Reviewed By: shergin
Differential Revision: D18264235
fbshipit-source-id: 684e5f5555cec96b055b13cd83daaeb393f4fac9
Summary:
`xplat` targets add different deps based on what platform the target is being built for.
for anything using `fb_xplat`, we can put all ios supermodules in `fbobjc_labels` and all android sms in `fbandroid_labels`
There's some weirdness with python targets like `thrift_gen` in `/xplat/mobileconfig/tools/generator/gen-py/BUCK` that don't have platform-specific labels because the except_for list for `fbandroid` doesn't need the `fbsource//` prefix (see changes in `/ios/isolation/infra.mobileconfig.sm`)
Changelog: [Internal]
Reviewed By: shergin, joshleibsly
Differential Revision: D17884952
fbshipit-source-id: e245364cf515b75682990094d24f789d53b1f3f5
Summary:
**Context**
For method calls from JS to Objective C, we have to convert JS values to ObjC objects/primitives. Before we can call our ObjC methods, we need to run both the ObjC primitives and objects through `RCTConvert`. This is necessary, because we sometimes convert `NSDictionary`s to special Objective C objects. Apparently, in `RCTTiming`, we also do the same with `double` (i.e: we convert a `double` arg to another `double` type with different meaning).
**Problem**
`RCTTiming` used `RCTConvert` to convert `double`s into `NSTimeInterval` (also a double). The conversion is defined like this:
```
// i.e: division by 1000
RCT_CUSTOM_CONVERTER(NSTimeInterval, NSTimeInterval, [self double:json] / 1000.0)
```
This diff implements the support necessary to make this work. For completeness, I also implemented the same functionality for `BOOL`s.
Changelog: [iOS][Fixed] Improve method argument RCTConvert logic
Reviewed By: mdvacca
Differential Revision: D17887915
fbshipit-source-id: 3246fdbf4db7e96911f16460d92448b1f1e99444
Summary:
In the legacy system, when NativeModules are supposed to be initialized on the main queue, we do the following synchronously on the main thread:
1. Done: Attach bridge on main queue
2. Register the NativeModule for frame updates on main queue
3. Post Notification that NativeModule was created on main queue
4. Attach methodQueue on main queue
5. Call new on main queue
`[RCTModuleData instance]` is the entrypoint for all of this logic.
We probably shouldn't synchronously execute all this initialization on the main queue, because it can lead to deadlocks down the road. Therefore, this diff makes it so that we still call `new` on the same thread. However, we do all other initialization in the main thread, if that's required.
Changelog: [iOS][Fixed] TurboModule initialization on the main queue
Reviewed By: PeteTheHeat
Differential Revision: D17867583
fbshipit-source-id: a88412ee1e3d93a4f9b5ab0b4dd8fc5213fa91f8
Summary: I was trying to debug an issue with a method signature mismatch last week, but was having trouble figuring out the problem because the error I was getting was unrelated - apparently JNI will sometimes swallow the error and just fail mysteriously later on. Ramanpreet showed me this macro that will throw any pending exceptions, so let's do that after we try to lookup the Java method in case it fails.
Reviewed By: RSNara
Differential Revision: D17680121
fbshipit-source-id: 1f23e49014f7cc1616e111386d440637e6a74677
Summary:
There are cases where the CallbackWrapper instances were added from different thread, potentially crashing the inner std::unordered_set<> we're using to keep the wrappers alive for extended time.
To avoid it, let's just use std::mutex.
Reviewed By: shergin
Differential Revision: D17631233
fbshipit-source-id: e8f98004e45a68be31f8f0cda118fb67dcb06d45
Summary: Now that all the plumbing is done, this diff finally implements async method dispatch on the NativeModule thread.
Reviewed By: mdvacca
Differential Revision: D17480605
fbshipit-source-id: 992aab99954c488a0327144d84a1668a2b158d04
Summary:
Self explanatory.
1. **Existing:** We create the NativeModules thread in CatalystInstanceImpl.cpp.
2. D17422165: We wrap this thread in a `BridgeNativeCallInvoker`.
3. D17422164: We use `CatalystInstanceImpl::getNativeCallInvokerHolder()` to get a hold of the `BridgeNitiveCallInvoker` in Java.
4. D17422163: From Java, we pass this `CallInvokerHolder` to `TurboModuleManager`'s constructor.
5. **This diff:** `TurboModuleManager` then unwraps the `CallInvoker` from `CallInvokerHolder`, and passes it to all TurboModules in their constructor.
Reviewed By: PeteTheHeat
Differential Revision: D17422160
fbshipit-source-id: c0a76dfe5fdedac2e0e21f7a562bc7588dc190fb
Summary:
## Motivation
The concept behind JSCallInvoker doesn't necessarily have to apply only to the JS thread. On Android, we need to re-use this abstraction to allow execution of async method calls on the NativeModules thread.
Reviewed By: PeteTheHeat
Differential Revision: D17377313
fbshipit-source-id: 3d9075cbfce0b908d800a366947cfd16a3013d1c
Summary: This is just a minor fix to D17380360. Basically, we want to make sure that callbacks can only be called once on the ObjC side.
Reviewed By: fkgozali
Differential Revision: D17403852
fbshipit-source-id: b0d2bbd539daef4837a345bc2953dd9687b3147b
Summary: There is a race condition between tearing down the bridge vs CallbackWrapper invoking the wrapped jsi::Function. This means the wrapper can be stale and unsafe to access after the bridge dies. To avoid unsafe access, let's clean it up properly using weak ref.
Reviewed By: RSNara
Differential Revision: D17380360
fbshipit-source-id: f91ce75d945bf8ed0b141c593bcc674ff465aa8c
Summary:
This diff improves on D17244061 by moving the `Jni::JniLocalScope` to earlier on in `JavaTurboModule::invokeJavaMethod`. The problem with D17244061 was two-fold:
1. It completely ignored the local references created by just calling `getConstants()`. So, you could spam `getConstants()` and still get an overflow.
2. The capacity was incorrect. Basically, two things happen when you call `PushLocalFrame`. One, we push an actual Jni frame onto a stack somewhere in the dvm: https://fburl.com/f16wfrxi, https://fburl.com/jhpha563. Popping off this stack is how the local references are cleared when we call `PopLocalFrame`. Two, we do a check to make sure that we can support at least `capacity` more local references: https://fburl.com/jucvew8j. The max is 512. The problem before was that, I was just using the argument count for the capacity. This was inaccurate because we create many `jclass` objects, and several misc. intermediate java objects when we invoke `JavaTurboModule::invokeJavaMethod`. So, I've refined the calculation of capacity to be a bit more accurate. This should make sure that capacity check works, which should help us fail faster if our local reference count approaches 512.
Reviewed By: shergin
Differential Revision: D17337354
fbshipit-source-id: 45574bae4748c52d8f919c1480b9a0936d970628
Ramanpreet Nara
Kevin Gozali
Max Ovtsin
Héctor Ramos
Adam Ernst
Jon Thysell (JAUNTY)
Janette Cheng
Josh Leibsly
Emily Janzer
Pascal Hartig
Peter Argany
Andres Suarez
Emily Janzer