Summary:
This refactors MountingManager into a minimal API that shims into a more fully-featured SurfaceMountingManager. The SurfaceMountingManager keeps track of surface start/stop, surface ID, and surface Context.
This solves a number of issues around (1) race conditions around StopSurface/StartSurface, (2) memory management of Views, (3)
Concrete improvements:
1. Simpler to reason about race conditions around StopSurface/StartSurface.
2. 1:1 relationship between SurfaceId and all Views/tags.
3. When surface is stopped, all descendent Views can be GC'd immediately.
4. Fixed separation of concerns and leaky abstractions: surfaceId/rootTag and Surface Context are now stored and manipulated *only* in SurfaceMountingManager.
5. Simpler StopSurface flow: we simply remove references to all Views, and the Fragment (outside of the scope of this code) removes the RootView. This will trigger GC and we do ~0 work. Previously, we ran a REMOVE and DELETE instruction and kept track of each View in a HashMap. Now we can simply delete the map and move on.
The caveat: NativeAnimated (or other native modules that go through UIManager). APIs like `updateProps` currently uses only the ReactTag and does not store SurfaceId. This is a good argument for moving away from ReactTag, at least in its current incarnation, but: for now this requires that you do a lookup of a ReactTag across N surfaces (worst-case) to determine which Surface a ReactTag is in.
So, to summarize, the "con" of this approach is that now `getSurfaceManagerForViewEnforced` could be slower. It is used in:
* NativeAnimatedModule calls `updateProps` through UIManager
* FabricEventEmitter calls `receiveEvent` on FabricUIManager directly
* On audit, I could find zero native callsites to `sendAccessibilityEvent` through UIManager
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26000781
fbshipit-source-id: 386ae40c4333f8c584e05818c404868dbee6ce73
Summary:
In preparation for flipping the default, marking autoglob as False in places where it isn't explicitly specified.
Changelog: [Internal]
Reviewed By: strulovich
Differential Revision: D25497305
fbshipit-source-id: 142e5caca2d67efcb3c25067a36934f7f6dd4b21
Summary:
Props are transferred from C++ to Java using ReadableNativeMaps. The current implementation of ReadableNativeMaps creates an internal HashMap the first time one of its methods is executed.
During the update of props ReadableNativeMaps are consumed only once and they are accessed through an Iterator. That's why there is no reason to create the internal HashMap, which is inefficient from performance and memory point of view.
This diff creates an experiment that avoids the creation of the internal HashMap during prop updates, additionally it removes a lock that was being used in the creation of the internal HashMap. I expect this change to have a positive impact in TTRC and memory (in particular for ME devices)
This diff reduces the amount of ReadableNativeMaps's internal HashMaps created during initial render of MP Home by ~35%.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D25361169
fbshipit-source-id: 7b6efda11922d56127131494ca39b5cec75f1703
Summary:
These methods were being striped from proguard, which causes release builds to instacrash
Changelog: [Internal] Protect methods from Proguard
Reviewed By: RSNara
Differential Revision: D25314933
fbshipit-source-id: 173160eab953b7c24e02f5e6ef3bf335c1f85526
Summary:
Add three markers for ReactRootView perf logging: onMeasure, attachToReactInstanceManager, and updateLayoutSpecs.
It is suspected that one or all of these have regressed under Fabric in some cases.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24909635
fbshipit-source-id: 6b6c0cc792c4b2d72f2360d6cffc02ef00e8a88b
Summary:
This PR changes ReadableNativeMap.getNullableValue to return null if key not found, instead of throwing exception. This matches method signature and nullable annotation.
## Changelog
[Android] [Changed] - fix ReadableNativeMap.getNullableValue to match signature
Pull Request resolved: https://github.com/facebook/react-native/pull/30121
Test Plan: RNTester app builds and runs as expected, and getNullableValue will return null instead of throwing exception.
Reviewed By: JoshuaGross
Differential Revision: D24164302
Pulled By: fkgozali
fbshipit-source-id: 572c1d4ae5fd493aa0018c2df1dfc7fc91cb4b6b
Summary:
Fix ReadableArray annotations, because these methods throw ArrayIndexOutOfBoundsException instead of null if index is not found.
## Changelog
[Android] [Changed] - fix ReadableArray null annotations. Possibly breaking change for Kotlin apps.
Pull Request resolved: https://github.com/facebook/react-native/pull/30122
Test Plan: RNTester app builds and runs as expected, and show correct type in when used with Kotlin code.
Reviewed By: JoshuaGross
Differential Revision: D24164326
Pulled By: fkgozali
fbshipit-source-id: 0c3f8fa9accbd32cc71c50befe9330e5201643f6
Summary: Adding two new ReactMarkers for start and end of bridgeless initialization. Creating new ones instead of reusing existing ones to make it easier to differentiate data from the bridge vs. bridgeless, and also because our existing markers 1) aren't very easy to understand, and 2) don't map cleanly to the new architecture.
Reviewed By: PeteTheHeat
Differential Revision: D23789156
fbshipit-source-id: 2ed10769e08604e591503a2bc9566aeb1d0563ed
Summary:
New mechanism to soft-crash, or crash, and collect diagnostics in the mounting layer.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D22971260
fbshipit-source-id: 860cde3effa4a187f10f5dd1488dd41ace65e363
Summary:
After D22801173 (https://github.com/facebook/react-native/commit/9e6ba9ddb8608d4e3a4a80d0138600130b766d4c) has landed, the native mechanisms in NativeAnimated to delay queued items from immediate execution are no longer necessary.
Fabric and non-Fabric animations both look smooth after deleting this code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22807906
fbshipit-source-id: 9241fff84376f6aa9a35049cc40dfd6561effaa1
Summary:
As a followup to D22743723 (https://github.com/facebook/react-native/commit/d53fc8a3cd44c7ec7e6eade985daf3d4feb2d736) on the iOS side, I implement a BackgroundExecutor that can be used from C++ to schedule layouts on their own thread, off the JS and UI thread.
This is a potential perf improvement that we will experiment with.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22826795
fbshipit-source-id: 899bd67fe1b86f52910951e9536b59a1414a304c
Summary:
Enable registerSegment in Venice and verify bundle splitting works for Pokes with Venice.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D22666115
fbshipit-source-id: 74ef830b802634b1019d4371873aba599438de37
Summary:
This diff refactors the destruction of the ReactInstanceManager when the app is experiencing low memory.
As part of this refactor, I setup an experiment to understand at what level of memory pressure is convenient to destroy the RN bridge.
The experiment is divided in six levels described in the following table:
https://pxl.cl/1dzx8https://www.internalfb.com/intern/qe2/fb4a_react_native_memory/android_fb4a_instance_unload_pressure_v2/setup
changelog: [internal] Internal
Reviewed By: JoshuaGross
Differential Revision: D22577553
fbshipit-source-id: 37f8f561099a1ba6239795f5907090ced3b5dd18
Summary:
When a native module registers itself as a lifecycle event listener, ReactContext will immediately call `onHostResume` if the activity is already in a resumed state. We rely on this behavior for things like NativeAnimatedModule, which only enqueues the frame callback in onHostResume. However, ReactContext only does this if `hasActiveCatalystInstance()` returns true - which makes sense, because we don't want to notify listeners if the instance has been destroyed. But in bridgeless mode, `hasActiveCatalystInstance` returns false, and we never call `onHostResume` in this case.
This diff fixes an issue where native driver animations don't work the first time you navigate to a screen with bridgeless mode enabled.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22560314
fbshipit-source-id: 1a60cb482896308e21d6e438eb9a7314f580ad04
Summary:
`UIManagerHelper.getUIManager()` relies on the bridge (CatalystInstance) to get the proper UIManager depending on which renderer is being used. Unfortunately, this means it will always return null in bridgeless mode, where the CatalystInstance doesn't exist. This diff replaces the implementation of `BridgelessReactContext.getJSIModule()` to return the FabricUIManager from the ReactHost/Instance.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22480968
fbshipit-source-id: 640e3f22a5b39b315ed2f0397be3cba39e80529a
Summary:
Expose `resolveCustomDirectEventName` from UIManager interface for Bridgeless Mode+NativeAnimatedModule.
We cannot remove the interface from the Non-Fabric UIManagerModule yet, as it would break downstream open-source dependencies, so I just marked it deprecated.
Note that this still doesn't totally fix issues with Bridgeless mode: generally I have to navigate to a Bridgeless surface, exit it, and then navigate back, and only on the 2nd navigation does everything seem to work properly...
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483508
fbshipit-source-id: 685126e7e51aa5d0fd60ad5d4ecc44e8c6c3029d
Summary:
These flags haven't been used in months. They were useful to uncover some race conditions, but will not be iterated further. The Venice project will obviate the concerns that sparked these experiments in the first place.
These flags have been hardcoded to false for a while.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22319204
fbshipit-source-id: 09415f3bb1ca56e15f357210e966d0483ff384f2
Summary:
Switch between "Fabric" and "Non-Fabric" modes based on which types of native Views are being attached to animations. Don't allow non-Fabric to drive Fabric animations and vice-versa.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21985411
fbshipit-source-id: fb9bef1e38375b384430b4e0275e7b6d62eda7a4
Summary:
This diff exposes receiveEvent on the UIManager class. This is necessary to support backward compatibility between Fabric and classic RN
changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D21979687
fbshipit-source-id: 1ec75896687d55e699f79c520e21f05fac368ee6
Summary:
In the past, in Fabric (Android), we never called stopSurface. Ever! This is bad for memory and can cause other issues, so... let's not do that.
Instead, when the ReactRootView is being torn down, we check the View ID to see if it's a Fabric RootView, and if it is, we call Fabric's stopSurface method.
Fabric stopSurface only has impact on (1) Fabric mount instructions being executed after that point and (2) tells JS to stop running the surface, on the JS thread, asynchronously.
Anecdotally it looks like all the MountItems involved in deleting and removing views from the hierarchy are executed before stopSurface is called.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21965837
fbshipit-source-id: 5169c5a1e339fd9e016ae1234d8389b2bd30be70
Summary:
These logs are no longer necessary, because data indicates that the TurboModule eager init crash was fixed.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D21852743
fbshipit-source-id: ddeefd6396283ee5e15980a33fb006cb83a81532
Summary:
Removing logs related to T62192299 that we don't need anymore.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21773629
fbshipit-source-id: d16c01f87be3ed7512fe90b6e261b4c7efbd3068
Summary:
This is (part of) a rewrite of D15390384.
This implements the lifecycle interface only for Fabric to signal to NativeAnimatedModule when preOperations are about to run / operations are about to be dispatched.
We will likely want to remove this mechanism entirely and rewrite NativeAnimatedModule in C++ for Fabric/Venice, but for now, I'm not sure of a better solution to unblock.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698193
fbshipit-source-id: a13d445073911fd63d896202a7a1bfbe1167038a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28851
This diff creates a RuntimeExecutor that uses the bridge and exposes it on CatalystInstanceImpl.
Changelog: [Internal]
Reviewed By: mdvacca, RSNara
Differential Revision: D21051949
fbshipit-source-id: b3977fc14fa19089f33e297d29cedba0d067526d
Summary:
This diff refactors the initial render of Fabric in order to set the layout metrics as we start the surface.
This prevents to create an additional fabric commit during initial render. Also this migth help fixing T63495589 (I wasn't able to repro T63495589 again when using this diff)
changelog: [Internal][Android] Internal change to reduce the amount of commits during initial render of Fabric
Reviewed By: JoshuaGross
Differential Revision: D21330072
fbshipit-source-id: 758c49b52ea4c12d5623b7c7d68c7318f4a6cd83
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28779
Creating a JNI wrapper class for RuntimeExecutor so we can pass it to Fabric and TurboModules in Java.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D21049385
fbshipit-source-id: f833004225d9837acf6ffafd3988f89748cf12aa
Summary:
## Description
When the TurboModule and the NativeModule systems are alive at the same time, after RN cleanup, if a TurboModule is required, we return `null` from TurboModuleManager. This causes `CatalystInstanceImpl.getNativeModule` to do a lookup on NativeModule registry, which throws an `AssertionError`, because the TurboModule isn't found. Instead of throwing an `AssertionError` from `CatalystInstanceImpl.getNativeModule`, this diff instead has that method return `null` in this particular case.
## Rationale
This should eliminate the crashes we're seeing in T46487253.
## Future action
In the future, we should guard `CatalystInstanceImpl.getNativeModule` with an `if (mDestroyed) return null;` statement. This'll ensure that if NativeModules are required after React Native has started cleanup, they'll be returned as `null`. Right now, we either return the destroyed NativeModule object, or create/initialize the NativeMoulde and return it, which is wrong.
Changelog:
[Android][Changed] - Make CatalystInstance.getNativeModule nullable
Reviewed By: JoshuaGross
Differential Revision: D21272029
fbshipit-source-id: 099ad9ab9fa2146299df4cf7f86ae7a8e526bb15
Summary:
In bridgeless mode, the CatalystInstance doesn't exist, but we still need to be able to access the sourceURL in SourceCodeModule (which is needed to render the images in LogBox warnings and errors). This diff adds a new API for getting the sourceURL directly from ReactContext, instead of having to call context.getCatalystInstance().getSourceURL(), and updates SourceCodeModule to use it.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D20848700
fbshipit-source-id: 3ecda81a17121178b76bbb3e9b0f27f103c1961a
Summary:
This is the first of three PRs related to enabling multi-bundle support in React Native. More details, motivation and reasoning behind it can be found in RFC [here](https://github.com/react-native-community/discussions-and-proposals/issues/152).
Logic responsible for installing globals was pulled out from `loadApplicationScript` to `initializeRuntime` since it should be ran only once, what was left was renamed to `loadBundle`.
It's based on dratwas work from [here](https://github.com/callstack/react-native/tree/feat/multibundle/split-load-application), but applied to current `master` to avoid rebasing 3-months old branch and issues that come with that.
## Changelog
[Internal] [Changed] - split `loadApplicationScript` into `initializeRuntime` and `loadBundle` to enable multi-bundle support in the future
Pull Request resolved: https://github.com/facebook/react-native/pull/27844
Test Plan: Initialized new RN app with CLI, set RN to build from source and verified the still app builds and runs OK using code from this branch.
Reviewed By: rickhanlonii
Differential Revision: D19888605
Pulled By: ejanzer
fbshipit-source-id: 24ace48ffe8978796591fe7c6cf53a61b127cce6
Summary:
Instead of just blindly retrying all ViewCommands if they fail - which could be dangerous, since it's arbitrary imperative commands we'd be executing twice, potentially with bad app state - we only retry if the ViewCommand throws a "RetryableMountingLayerException".
Changelog: [Internal] Optimization to ViewCommands
Reviewed By: mdvacca
Differential Revision: D20529985
fbshipit-source-id: 0217b43f4bf92442bcc7ca48c8ae2b9a9e543dc9
Summary:
Marketplace eagerly initializes a few NativeModules. These NativeModules are TurboModule compatible, and the device/user is in the TurboModule test. So these NativeModuels should be returned from the TurboModule system. However, for some reason, we end up doing a lookup on the `NativeModuleRegistry` for these NativeModules.
This means that either:
1. The TurboModuleManager isn't attached to the CatalystInstance
2. The TurboModuleManager returned null from getModule.
These logs will help us get to the bottom of what's going on.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D20260150
fbshipit-source-id: bb554ead412ad3b0fa7502b77f575365608ebc98
Summary:
Currently the error messages lead users to assume that all problems are because we're trying to use the ReactContext too early; it could be because we're using it too late, after it's been destroyed. Because of D17944723, it could be because we're accessing too late, after teardown.
Changelog: [Internal] making some error messages more specific and helpful
Reviewed By: mdvacca
Differential Revision: D20251909
fbshipit-source-id: e236d57e4d7d9c778d41de95160c242bcd69b3c9
Summary:
Turns out that dispatchMountItems was reentrant, meaning that something (in particular, updateState) could cause mount items to be queued up which would then be executed synchronously, out-of-order, in the middle of the previous dispatchMountItems call.
We will continue to monitor this and see how often we're reentering: T63181639 and via any soft exceptions that are logged.
For context, there are currently three ways dispatchMountItems gets called: 1) On every UI Tick in the UI thread, in a loop; 2) via animations, via synchronouslyUpdateViewOnUIThread, which happens to fail a *lot* currently; 3) when there is a commit on the UI thread, like with a Java->C++ state update. We must account for reentrance and failure in all three cases and make sure the `mInDispatch` flag is reset after success or failure in all of those situations.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20170160
fbshipit-source-id: 834f1d9b65000caa7f2eea4849e5e7951d2b6be6
Summary:
This Pull Request implements the PlatformColor proposal discussed at https://github.com/react-native-community/discussions-and-proposals/issues/126. The changes include implementations for iOS and Android as well as a PlatformColorExample page in RNTester.
Every native platform has the concept of system defined colors. Instead of specifying a concrete color value the app developer can choose a system color that varies in appearance depending on a system theme settings such Light or Dark mode, accessibility settings such as a High Contrast mode, and even its context within the app such as the traits of a containing view or window.
The proposal is to add true platform color support to react-native by extending the Flow type `ColorValue` with platform specific color type information for each platform and to provide a convenience function, `PlatformColor()`, for instantiating platform specific ColorValue objects.
`PlatformColor(name [, name ...])` where `name` is a system color name on a given platform. If `name` does not resolve to a color for any reason, the next `name` in the argument list will be resolved and so on. If none of the names resolve, a RedBox error occurs. This allows a latest platform color to be used, but if running on an older platform it will fallback to a previous version.
The function returns a `ColorValue`.
On iOS the values of `name` is one of the iOS [UI Element](https://developer.apple.com/documentation/uikit/uicolor/ui_element_colors) or [Standard Color](https://developer.apple.com/documentation/uikit/uicolor/standard_colors) names such as `labelColor` or `systemFillColor`.
On Android the `name` values are the same [app resource](https://developer.android.com/guide/topics/resources/providing-resources) path strings that can be expressed in XML:
XML Resource:
`@ [<package_name>:]<resource_type>/<resource_name>`
Style reference from current theme:
`?[<package_name>:][<resource_type>/]<resource_name>`
For example:
- `?android:colorError`
- `?android:attr/colorError`
- `?attr/colorPrimary`
- `?colorPrimaryDark`
- `android:color/holo_purple`
- `color/catalyst_redbox_background`
On iOS another type of system dynamic color can be created using the `IOSDynamicColor({dark: <color>, light:<color>})` method. The arguments are a tuple containing custom colors for light and dark themes. Such dynamic colors are useful for branding colors or other app specific colors that still respond automatically to system setting changes.
Example: `<View style={{ backgroundColor: IOSDynamicColor({light: 'black', dark: 'white'}) }}/>`
Other platforms could create platform specific functions similar to `IOSDynamicColor` per the needs of those platforms. For example, macOS has a similar dynamic color type that could be implemented via a `MacDynamicColor`. On Windows custom brushes that tint or otherwise modify a system brush could be created using a platform specific method.
## Changelog
[General] [Added] - Added PlatformColor implementations for iOS and Android
Pull Request resolved: https://github.com/facebook/react-native/pull/27908
Test Plan:
The changes have been tested using the RNTester test app for iOS and Android. On iOS a set of XCTestCase's were added to the Unit Tests.
<img width="924" alt="PlatformColor-ios-android" src="https://user-images.githubusercontent.com/30053638/73472497-ff183a80-433f-11ea-90d8-2b04338bbe79.png">
In addition `PlatformColor` support has been added to other out-of-tree platforms such as macOS and Windows has been implemented using these changes:
react-native for macOS branch: https://github.com/microsoft/react-native/compare/master...tom-un:tomun/platformcolors
react-native for Windows branch: https://github.com/microsoft/react-native-windows/compare/master...tom-un:tomun/platformcolors
iOS
|Light|Dark|
|{F229354502}|{F229354515}|
Android
|Light|Dark|
|{F230114392}|{F230114490}|
{F230122700}
Reviewed By: hramos
Differential Revision: D19837753
Pulled By: TheSavior
fbshipit-source-id: 82ca70d40802f3b24591bfd4b94b61f3c38ba829
Summary:
We must use FLog instead of android.util.log, this diff moves the current callsites of android.util.log to FLog
changeLog:[internal]
Reviewed By: JoshuaGross
Differential Revision: D19884741
fbshipit-source-id: 300f7d691961aa51f0b525c37da7ae3d64fe5131
