Summary:
## Problem
For efficiency reasons, we'd only check whether the current class or its superclass implemented the `TurboModule` interface. However, it's possible for NativeModules to exist that use inheritance, and have their base class extend a code-generated spec. In this case, the superclass of the superclass of the NativeModule will implement `TurboModule`.
## Solution
To fix this problem, I'm relying on the `Types.isAssignable` API and checking whether the NativeModule can be assigned to the `TurboModule` interface. This is a more reliable way of knowing whether a NativeModule is a TurboModule or not.
**Note:** Had to adjust the buck dependencies of FrescoModule to make the `mTypes.isAssignable` check work.
Changelog:
[Android][Fixed] - Correct TurboModule detection logic in ReactModuleSpecProcessor
Reviewed By: fkgozali
Differential Revision: D19183671
fbshipit-source-id: ad21881453fe7027d9432048108f6ba344fd7e63
Summary:
We currently have a method in ReactContextBaseJavaModule that logs a warning if the native module is trying to access a ReactContext without an active Catalyst instance (because if you try to access it directly, it throws).
For bridgeless mode, we never have a CatalystInstance, but it's safe to call certain methods on the context that would normally require one. For this case, let's just return the context when the context is in bridgeless mode.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D19133988
fbshipit-source-id: cae0bd397aa24d9ad416491cbc32676870cc70b0
Summary:
The class was deprecated in 2016 and is blocking an upgrade to fbjni
which no longer supports the `Countable` class this extends.
Changelog:
[Android] [Removed] - NativeRunnableDeprecated
Pull Request resolved: https://github.com/facebook/react-native/pull/27529
Test Plan: No references to it in this codebase.
Reviewed By: mdvacca
Differential Revision: D19087074
Pulled By: passy
fbshipit-source-id: a4ee73be7c13cedf1d86d2643f8e788ad4a2e31f
Summary:
This diff refactors ReactContext to use IllegalStateException instead of RuntimeException when applicable.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18901845
fbshipit-source-id: 51ec36824c8402fa2c17e76c55578be44ec8aa15
Summary:
ExceptionManagers are created before the `ReactApplicationContext`. Once we make them all TurboModule-compatible, they'll also subclasses `ReactContextBaseJavaModule`. This means that they'll need to be created with `ReactApplicationContext`. Since one isn't available, we'll have to call `super(null)` in their constructor.
Changelog:
[Android][Changed] - Make ReactApplicationContext nullable as the constructor argument of ReactContextBaseJavaModule
Reviewed By: PeteTheHeat
Differential Revision: D18935950
fbshipit-source-id: a643a10a42cf36a2a2d6fde87795965f16295d43
Summary:
This diff exposes the getJSIModule on the ReactContext class.
This class already has methods to obtain NativeModules and JSModules, it make sense to expose the getJSIModule method too.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18862858
fbshipit-source-id: 95fe48c065266060c96fc40a002041ba398b3134
Summary:
This method exposes a method to ReactContext to determine if we are running in Bridgeless mode or not
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18901149
fbshipit-source-id: bdd5ba747381f3bde5f592276b42244ca01ccbb9
Summary:
This diff promotes the UIManagerModule.getEventDispatcher() to the interface UIManager and it implements this method in FabricUIManager class.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18862862
fbshipit-source-id: 424f0e601ed1807dbd5d33048daf7dc3bb200dcd
Summary: Removing the methods I recently added for storing/retrieving an instance key on a ReactContext.
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D18710637
fbshipit-source-id: d34683ec660bd999db8112865e15392606fc9237
Summary:
This diff adds extra logging in the method that handles exceptions for RN Android
Changelog: internal
Reviewed By: JoshuaGross
Differential Revision: D18694123
fbshipit-source-id: e275445b65473ed55eec9d4b823938e32fa805e5
Summary:
We need to make our ExceptionsManagers into TurboModules. To do so, we need to first make them type-safe. This requires extending the base class code-generated from the NativeModule JavaScript spec. That base class extends `ReactContextBaseJavaModule`, which requires `ReactApplicationContext` to instantiate. Unfortunatley, our ExceptionsManagers need to be created before the `ReactInstanceManager`. This means that the `ReactApplicationContext` isn't available by the time they're created.
In this diff, I make the `ReactContextBaseJavaModule` have a 0 argument constructor that simply sets its ReactApplicationContext to null. This will allow us to eventually migrate our ExceptionsManagers to the TurboModule system.
Changelog:
[Android][Added] - Give ReactContextBaseJavaModule a 0 arg ctor
Reviewed By: mdvacca
Differential Revision: D18717908
fbshipit-source-id: 203ffc49f2ec0b32a809402801795879d3b3a64b
Summary:
We read from / write to `CatalystInstanceImpl.mTurboModuleRegistry` from multiple threads. To ensure that we directly read/write from memory, and not a stale cache, we should make this variable `volatile`.
Changelog:
[Android][Fixed] Make TurboModuleRegistry in CatalystInstanceImpl.java volatile
Reviewed By: fkgozali
Differential Revision: D18542954
fbshipit-source-id: 0a47f05e0653b4f7f58503c678bdf31c68eab9bf
Summary:
Document which methods can be called on UI thread or ANY thread.
In the future we should see if we can use only `ThreadConfined` or the AndroidX annotations instead of using both / choosing between them at each site.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18532542
fbshipit-source-id: 3b5406ea5035615a0ebf83484bf8ec0747a6b6f7
Summary:
Fixes T54997838 by preventing any view mutations during `onMeasure` calls.
There might still be places where this is possible, but this is where I'm seeing all the crashes currently.
See comments in ReactRootView for why views were mutated during onMeasure.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18518591
fbshipit-source-id: 1406af8a6b0bfcc86f4cc5b451b3967f312dfd85
Summary:
Adding a new String field for `instanceKey` to ReactContext, which is set via a new constructor on ReactApplicationContext. Also adding getters to ReactContext and ThemedReactContext so that it's accessible from any instance/subclass of ReactContext.
This will only be used in bridgeless mode.
Reviewed By: mdvacca
Differential Revision: D18316556
fbshipit-source-id: 9757da72fde4ba36034c1e129326461fed496229
Summary:
Turns out that `SoftAssertions.java` has always been a lie - it actually always throws exceptions. Migrate it to using `ReactSoftException`.
This file hasn't been touched at all since it was originally open-sourced, besides codemods!
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18336020
fbshipit-source-id: cba3db25a9f9d61325dd3f7843e92e984ae56281
Summary:
Looking at the crash reports from T46487253:
1. This crash happens only with TurboModule-compatible NativeModules.
2. Users who experience this crash are in the TurboModules test group.
Therefore, the crash happens while trying to load TurboModules.
The stack trace of the crash includes [this lookup via the NativeModule system](https://fburl.com/diffusion/vxj9goz5). When TurboModules are enabled, we can only start executing this line if one of two things are true:
1. The TurboModuleRegistry is null in CatalystInstanceImpl.
2. The TurboModuleRegistry isn't null but the NativeModule returned by the TurboModuleRegistry is null.
We can protect against 1 by asserting that when `ReactFeatureFlags.useTurboModules` is `true`, `mTurboModuleRegistry` is not null. Once this check lands, unless there's a race with setting `ReactFeatureFlags.useTurboModules`, we should be able to rule out 1.
Changelog:
[Added][Android] - Assert TurboModuleRegistry isn't null before using it in CatalystInstanceImpl
Reviewed By: PeteTheHeat
Differential Revision: D18211935
fbshipit-source-id: de88c033425c474ef80b73386b7182b1d3bb382f
Summary:
Mostly for easing open-source migration and not making a backwards-incompatible change (yet), we'll set this to false by default. Every app can opt-in to this if wanted but it's not necessary. This change is part of experiments surrounding more-aggressive teardown for Fabric and Bridgeless mode.
Changelog: [Internal] - This has the effect of (by default) disabling the previous diff which caused ReactContext teardown to always set mCatalystInstance to null. Now that is opt-in behavior and off by default, so it's not longer a breaking change.
Reviewed By: mdvacca
Differential Revision: D18207302
fbshipit-source-id: 7acfc894415e966f652c7049849eef79c440a135
Summary:
Simplify the API of `getReactApplicationContextIfActiveOrWarn`. We don't need to pass so much information into this method to collect good SoftExceptions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18134400
fbshipit-source-id: 0a250ab0252a44121f3339a31506a0a6c4c7cd35
Summary:
In D18032458 we introduce getReactApplicationContextIfActiveOrWarn. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing.
Modules that don't derive from `ReactContextBaseJavaModule` manually check for the catalyst impl and log their own SoftExceptions.
In this diff we also introduce SoftExceptions that by contract never cause crashes, even in debug mode.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18112989
fbshipit-source-id: 868f01f388aa2db3518db9f873f2afc2a62eed45
Summary:
In three previous diffs (D18020359 D17998627 D17969056), I implemented this logic in three different modules. There are potentially hundreds of modules where we should be implementing this check, so I'm moving the important logic into ReactContextBaseJavaModule.
Additionally, `WebSocketModule` was retaining its own copy of ReactApplicationContext instead of using the built-in `getReactApplicationContext`, so I removed that ivar from `WebSocketModule`.
Changelog:
[Internal]
Reviewed By: mdvacca
Differential Revision: D18032458
fbshipit-source-id: 9114120d3b80334df8d2e0813e36d21c667fc1bd
Summary:
CatalystInstanceImpl.destroy does a bunch of stuff in each of the relevant threads (Native Module thread, JS thread, UI thread).
This change creates a V1 destroy method (unchanged) and a V2 destroy method. The goal is to resolve (and catch!) race conditions in native modules and JSI modules that could occur during teardown; and mitigate race conditions that occur in RN teardown, like deallocation of C++ objects (scheduler, JS VM, and UIManager for Fabric).
Changelog:
[Internal] Experiment to fix deallocation race conditions
Reviewed By: mdvacca
Differential Revision: D18001677
fbshipit-source-id: 5955da0a7b726491c7d749642475f0fba74cce5a
Summary:
We want ReactContext to blow up if it tries to use `mCatalystInstance` after `destroy` is called.
Changelog:
[[Internal]]
Reviewed By: mdvacca
Differential Revision: D17944723
fbshipit-source-id: cfe8a8b98473f53dd68bbcd91a71f58bd7a0c503
Summary:
This already has an assert that `destroy` should only be called on the UI thread. Add an annotation.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D17989113
fbshipit-source-id: fd44f321cbcb7f0a18383ca6226cce72e5991eea
Summary: Oleksandr made the point that making `sDidInit` volatile isn't quite enough because the flag is set at the beginning of the function; it's possible that it will return true while it's still in the process of initializing the bridge. Moving where we set the flag to the end of the function should address the issue.
Reviewed By: JoshuaGross
Differential Revision: D17770128
fbshipit-source-id: 695d3edc582d9dce1884d8698d400dd147ca5cae
Summary: In D17747958 I added a function to check the value of a static variable, but forgot to make sure it was accessed in a thread-safe manner. Adding `volatile` to the flag
Reviewed By: makovkastar
Differential Revision: D17763888
fbshipit-source-id: f227b69de1a0df6493424da3b276529555999f70
Summary: Adding a hook to check if ReactBridge is initialized so we can avoid loading the .so on the main thread.
Reviewed By: fkgozali
Differential Revision: D17747958
fbshipit-source-id: 5969afa57dc1b446c03bc68eaa9e1385fe17c461
Summary:
`ReactRootView.startReactApplication` takes a `Bundle` argument called `initialProperties`. This is translated to a `ReadableMap` via `Arguments.fromBundle`. If the bundle contains an array of bundles, this gets translated by `Arguments.fromArray`.
If the bundle was passed from one activity to another via intent extras, however, there is a problem. After the bundle has been marshaled and unmarshaled, the array of `Bundle`s come out the other end as an arrap of `Parcelable`s, although each array element remains a `Bundle`. This results in an "Unknown array type" exception.
This PR fixes this by adding support for `Parcelable` arrays – provided they contain only members of type `Bundle`.
## Changelog
[Android] [Fixed] - Don't throw "Unknown array type" exception when passing serialized bundle arrays in ReactRootView.startReactApplication's initialProperties parameter
Pull Request resolved: https://github.com/facebook/react-native/pull/26379
Test Plan: Added test class `ArgumentsTest`. The test method `testFromMarshaledBundle` fails when used on the old version of `Arguments`.
Differential Revision: D17661203
Pulled By: cpojer
fbshipit-source-id: 63612d78f49bdf9cc53f6f21ae883dba6cebce84
Summary:
In `CatalystInstanceImpl.destroy()`, we require the TurboModuleManager using the [following lines](https://fburl.com/diffusion/a4y6wbft):
```
final JSIModule turboModuleManager =
ReactFeatureFlags.useTurboModules
? mJSIModuleRegistry.getModule(JSIModuleType.TurboModuleManager)
: null;
```
For some strange reason, even though `ReactFeatureFlags.useTurboModules` is true, the TurboModuleManager isn't registered with mJSIModuleRegistry. I spent some time looking through the code, but I couldn't figure out why. These lines actually aren't necessary, so it's possible to fix the issue by simply working around it, which is what this diff does. We shouldn't have been double requiring the TurboModuleManager anyways, since `CatalystInstance.java` has a method to set the TurboModuleManager, which we call in `ReactInstanceManager.createReactContext`.
## Alternative approach
I could push this diff to the next cut, and instead land a diff that adds debug information to the native crash. At the cost of a week, it may help us figure out why we're seeing the crash. Thoughts? cc fkgozali
Reviewed By: fkgozali
Differential Revision: D17636604
fbshipit-source-id: ecfff593dc6eb4ec4d5e331348b308bc7ab37966
Summary: CatalystInstanceImpl is responsible for creating the NativeModules thread. We therefore expose a method `getNativeCallInvokerHolder()` on this hybrid class to create and give us access to the `CallInvokerHolder` for the NativeModules thread.
Reviewed By: PeteTheHeat
Differential Revision: D17422164
fbshipit-source-id: 316423847518124115643549fa73a8533d493cd0
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:
We probably don't always need to crash in these cases.
This should replace all prod/dev crashes with logged SoftExceptions.
Note that this is currently only used for Fabric.
Changelog:
[Internal]
Reviewed By: mdvacca
Differential Revision: D17170459
fbshipit-source-id: 70757ae88deb8c893b36971d879174e25a194fb9
Summary:
## Summary
When an exception is raised by Java in a synchronous call from JS to Java, the Java portion of the stack trace is simply ignored when the exception is forwarded to JS. This problem doesn't exist for async calls. I did some digging to figure out why this works with async calls, but not sync calls, to get to the bottom of T52585087.
In T52585087, `TurboModuleRegistry.get<Spec>('I18nAssets')` fails because of a `NullPointerException` in Java. However, since there's no stack trace associated with the `NullPointerException`, it's hard to diagnose the problem.
## Asynchronous calls
ReactNative's NativeModules thread is a background thread on which we schedule asynchronous NativeModule method invocations. We spawn our background threads in Java using the [`MessageQueueThreadImpl`](https://fburl.com/diffusion/u0vdm5k3). Each thread is associated with a queue on which you can schedule some work. These `MessageQueueThread`s have a [C++ API](https://fburl.com/diffusion/596740d8) that we can use to schedule some work from C++.
NativeModule method invocations in JS produce a C++/Java boundry, because our JS executes C++, which executes the Java NativeModule method. But since the method is asynchronous, instead of invoking it immediately, we wrap it in a C++ lambda and use the C++ API of `MessageQueueThread` to schedule it to be invoked later. Therefore, when it actually invokes, we'll get Java invoking C++, which invokes Java.
When the NativeModule method (implemented in Java) throws an exception, fbjni will convert that exception into a `jni::JniException` and start unwinding the C++ stack. Eventually, this exception will reach the outermost Java/C++ boundry. Typically, at this point, the program would crash, but because we used fbjni to register all our native functions, our `jni::JniException` will automatically be converted into a regular Java exception. This exception will be caught by MessageQueueThreadHandler [here](https://fburl.com/diffusion/c4thoca7), and handled using our ExceptionHandler NativeModule.
## Synchronous calls
In synchronous execution, JS uses a `JSI::HostObject` to call the Java method directly. If the Java method throws an error, because we're using fbjni, that Java exception will be converted to a `jni::JniException` object, which will contain the stack tract of the Java object. However, from what I could gather, Hermes doesn't know about `jni::JniException`. So, simply ignore the stack trace associated with the Java exception, understanding only the exception message. Hence, all synchronous calls into Java only display the JS stack trace. What we really want is to build an platform-agnostic abstraction that understands `jni::JniException` (and whatever its analogue is in iOS, if any) and use that to bridge between Native and JS.
## Temporary Solution
We know that when NativeModules are created, we do a synchronous call from JS to C++ to Java. This synchronous call happens when you do a property access on the `NativeModules` object. Therefore, at the very least, to get to the bottom of T52585087, we could log all exceptions that are thrown whenever a NativeModule is created. This should help us get to the bottom of T52585087.
Reviewed By: mdvacca
Differential Revision: D17126667
fbshipit-source-id: a6fb27aaea094b9559939ddcc260d3a2c6e71492
Summary: Support for `sendAccessibilityEvent` in the FabricUIManager.
Reviewed By: shergin
Differential Revision: D17142507
fbshipit-source-id: 5c131d7caa1e4189fd41ecfb558d0027394b6a15
Summary: This was an experiment to patch individual deltas in development instead of reloading the whole JS bundle. With improvements such as Fast Refresh that reduces the need for reloads and bundle splitting that reduces the number of modules and memory by 10x, we won't be needing this complex optimization that we never properly made work. This diff removes that code and I will be removing the JS side of things in Metro in a follow-up diff.
Reviewed By: fkgozali
Differential Revision: D16832709
fbshipit-source-id: 46596a3126d52d7d74f4b9ffc9a6ee9d82ec9522
Summary: Some surfaces throw ConcurrentModificationException when logging detailed perf for Fabric. I've refactored the ReactMarker class to use a threadsafe ArrayList and removed synchronization, which is safer and should improve perf everywhere the markers are used, even if there are zero listeners.
Reviewed By: mdvacca
Differential Revision: D16656139
fbshipit-source-id: 34572f9ad19028a273e0837b0b895c5e8a47976a
Summary:
## The Problem
1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`.
2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever.
3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt
### jsi.h
```
/// .................................................... In addition, to
/// make shutdown safe, destruction of objects associated with the Runtime
/// must be destroyed before the Runtime is destroyed, or from the
/// destructor of a managed HostObject or HostFunction. Informally, this
/// means that the main source of unsafe behavior is to hold a jsi object
/// in a non-Runtime-managed object, and not clean it up before the Runtime
/// is shut down. If your lifecycle is such that avoiding this is hard,
/// you will probably need to do use your own locks.
class Runtime {
public:
virtual ~Runtime();
```
Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks.
## The Solution
If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread.
For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233.
Reviewed By: mdvacca
Differential Revision: D16623340
fbshipit-source-id: 3a4c3efc70b9b3c8d329f19fdf4b4423c489695b
Summary:
This diff adds the new API required to implment JSResponderHandler in FabricUIManager
The new API differs from the old API, but since setJSResponder is called ONLY from JS it's not necessary to have this method as part of UIManager interface.
Reviewed By: JoshuaGross
Differential Revision: D16543440
fbshipit-source-id: ca4bd4c1e4df706cda0eb16798e01f3350558d06
Summary:
## The Problem
1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`.
2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever.
3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt
### jsi.h
```
/// .................................................... In addition, to
/// make shutdown safe, destruction of objects associated with the Runtime
/// must be destroyed before the Runtime is destroyed, or from the
/// destructor of a managed HostObject or HostFunction. Informally, this
/// means that the main source of unsafe behavior is to hold a jsi object
/// in a non-Runtime-managed object, and not clean it up before the Runtime
/// is shut down. If your lifecycle is such that avoiding this is hard,
/// you will probably need to do use your own locks.
class Runtime {
public:
virtual ~Runtime();
```
Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks.
## The Solution
If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread.
For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233.
Reviewed By: mhorowitz
Differential Revision: D16589168
fbshipit-source-id: a1c0786999c22bef55d416beb0fc40261447a807
Summary:
FabricUIManager.removeRootView() isn't currently used, removing it from the UIManager interface.
It looks like this is called from JS in paper renderers, but not Fabric, so we should be good to delete it.
Reviewed By: shergin, mdvacca
Differential Revision: D16275118
fbshipit-source-id: b8f3ae1dc7574ce17d8cc9e7fee72ef5dcc9b323
Summary:
Step 1 in removing the dependency on ReactContext from GuardedRunnable and other related classes. These are extensively by native modules and view managers, so in order to remove the bridge dependency from those modules we'll need to first decouple these classes from ReactContext. It turns out they only need ReactContext for its handleException method, which delegates out to product code. For backwards compatibility I'm exposing another NativeModuleExceptionManager in ReactContext that simply wraps its handleException method (since this interface already does everything we need).
I figured I'd keep around an extra constructor that still uses ReactContext for now instead of trying to migrate everything over at once.
Reviewed By: makovkastar
Differential Revision: D16270995
fbshipit-source-id: c9a8714bea7ac2a98e78234a0bae49140c00980d
Summary:
This diff migrates the usages Nullable and NonNull annotations to AndroidX instead of javax.
The purpose of this change is to bring consistency in the annotations used by the core of RN
Reviewed By: makovkastar
Differential Revision: D16054504
fbshipit-source-id: 21d888854da088d2a14615a90d4dc058e5286b91
Ramanpreet Nara
Emily Janzer
Pascal Hartig
David Vacca
Joshua Gross
Emily Janzer
Andres Suarez
masaaki1915
Petter Hesselberg
Christoph Nakazawa
Eric Williamson
Ram N