Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056685
fbshipit-source-id: a9abb451e62c9e378b0a5667fa4c5f82952bd02b
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056680
fbshipit-source-id: 2a57f15c01a585af9ddbe08cf6dd60922d4f64b3
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056676
fbshipit-source-id: 6baa5d9ba0ef15218ce02cbb51862f18d98b465c
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056672
fbshipit-source-id: 2f3e4b9bbed339f15b22fe87e27d81af083fd481
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26055276
fbshipit-source-id: f38f9e94ab313fb842d1e3037ab667cd460f1043
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26054855
fbshipit-source-id: 9592d854962d53f64d836b8361256cac5c4325df
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26054856
fbshipit-source-id: 228cc08a624e793aff4caf36e1df8285f3b3519d
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26043861
fbshipit-source-id: 37757508c835cbd1181f0e0e774abc62fdbfee2b
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class. This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26043393
fbshipit-source-id: b126658d818a18b7ffb2794de8e59a49c0e3d258
Summary:
The `Event` interface has been improved such that:
1. `getEventData` is now a default method on Event that returns null
2. `dispatch` has a default implementation that relies on getEventName() and getEventData()
3. `dispatchModern` can detect if surfaceId and event data are present; if not, it falls back to dispatch
This will dramatically ease future migrations: at some point in the (distant) future, we can simply delete RCTEventEmitter and
all use-cases will be supported by the current `Event` class without needing to introduce a 3rd transitional interface; and 99%
of all Event classes can be simplified, delet their `dispatch` implementation and need no further work.
At their core, all Events are simply: (1) a name, (2) data, (3) a target (surfaceId and tag). The interface now reflects that but still
allows for flexibility of the data and names being generated on-demand if necessary; but for the vast majority of Event classes, code
will be dramatically simplified.
I also migrate a single Event class, ContentSizeChangeEvent, to use this new method of dispatch.
Changelog: [Android][Changed] Added convenience methods to simplify native Event classes and ease migrations
Reviewed By: mdvacca
Differential Revision: D26043325
fbshipit-source-id: bc308105f7f6e654d45fd156dbf4a2bcbc45819c
Summary:
We assume that startSurface is always called off the UI thread; see if we can synchronously call setId and setSurfaceId, and in general, call setSurfaceId sooner.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26053050
fbshipit-source-id: a657584502bb0018e9591fe610eac0fc9fab2ca9
Summary:
Before this change, `mountingOverrideDelegate` was proxied via `Scheduler::startSurface` and `ShadowTree::ShadowTree` constructor down to `MountingCordinator`. Now we set it on the `MountingCoordinator` directly.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26049076
fbshipit-source-id: 7f1ecf2c8b6f264a7e59d19881464fe529c53d30
Summary:
The RootView being managed by Fabric should have an id of View.NO_ID when it is "handed over" to RN. This is true for Fabric and non-Fabric
and setting a custom id on the ReactRootView has never been supported. I'm temporarily (?) adding an additional check earlier and into ReactRootView to hopefully
catch any of these issues early.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26077509
fbshipit-source-id: 59e1ec080504e50698acc654c29120f039238a96
Summary:
There is a race between PreAllocateMountItems executing and killing off stale SurfaceMountingManagers.
For now I'm simplifying the "eviction" mechanism. We just keep up to 15 SurfaceMountingManagers around and start evicting them from the least-recently-referenced one.
Hopefully this logic can be simplified in the future.
I'm also sneaking in a small perf optimization for PreAllocateMountItem: don't queue them if the associated surface is already stopped, because chewing through a bunch of dead PreAllocateMountItems on the UI thread can be expensive.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26059378
fbshipit-source-id: 3b2503d7d8e5f045ae741d0d4a5880d1b37758d2
Summary:
In some cases, onMeasure/onLayout are called on the RootView before `startSurface` in Fabric has been able to set surfaceId on the RootView.
With the new SurfaceMountingManager, this causes a crash because we need a valid surfaceId to perform an operation. Before the SurfaceMountingManager refactor, a surfaceId of 0 would be passed to `mBinding.setConstraints` in the FabricUIManager, and setConstraints in C++ noops if there's an invalid surfaceId.
For now, FabricUIManager will also fail silently if the surfaceId is invalid when updateRootLayoutSpecs is called, just to be conservative and to be consistent with previous behavior. This will be upgraded to a hard-crash in the future.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26051266
fbshipit-source-id: ca2d80f899cdba9b3962af68546bd83b77be0680
Summary:
This diff removes references to FabricUIManager from UIManagerModule, these callsited were originally used for NativeAnimatedDriver, but they are not used anymore
changelog: [internal] internal
Reviewed By: JoshuaGross, shergin
Differential Revision: D26035388
fbshipit-source-id: d4825af17f6948d922c42670f2c7b02498c12039
Summary:
See title. dispatchV2 was just introduced yesterday, so nothing relies on it yet / it's safe to rename (there are no released versions of RN with `dispatchV2`).
And... dispatchModern is a better name.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26039008
fbshipit-source-id: 009ae721b8caef23a389c33b51f6f8952a6a73da
Summary:
In SurfaceMountingManager/MountingManager/FabricUIManager infra, we try to blackhole events that are sent to a stopped surface. In this case I just forgot to add a null check. Add here to protect against events sent to stopped surfaces - for example, if a TextInput is focused when the surface is stopped, a "blur" event will be sent and will crash here otherwise. Now it blackholes silently, as expected.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26031260
fbshipit-source-id: 9936466ca2d00267efaf7fa594c9bcd59f7aad2a
Summary:
Support RCTEventEmitterV2 in ReactTextInput*Event classes for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ReactTextInput*Event creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26031261
fbshipit-source-id: 7c972ecfd1e395104c4639995bb5ecc5f7c6baae
Summary:
Support RCTEventEmitterV2 in ImageLoadEvent for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ImageLoadEvent creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26029773
fbshipit-source-id: c8e00e06a2f9d6682367f9099bdf7f5fc12890e0
Summary:
Motivation: perf, simplicity, adhering to new SurfaceMountingManager APIs available to us. Backwards-compatible with events sent through old system or Fabric, to Fabric or non-Fabric Views.
Changelog: [Changed][Android] Old Native method to create ScrollEvent has been deprecated and will be removed at some point in the (distant) future
Reviewed By: mdvacca
Differential Revision: D26027105
fbshipit-source-id: b9dba5b56c2bfed3b8fc4488c54b271b85ab5fa0
Summary:
Refactor EventEmitters to take an optional SurfaceId that Fabric will use, and non-Fabric will not.
Migrating touches is a good proof-of-concept for how this could be used generally, and as it turns out, TouchEvent's API is more flexible than most other event APIs (because it uses a dictionary to pass data around, so we can just stuff SurfaceId into it - not efficient, but flexible).
All new APIs are backwards-compatible and designed to work with old-style events, with Fabric and non-Fabric. Native Views that migrate to the new API will be backwards-compatible and get an efficiency boost in Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26025135
fbshipit-source-id: 5b418951e9d0a3882f2d67398f2aaadac8a3a556
Summary:
Create V2 EventEmitter that surfaceId can be passed into, with a backwards-compat layer, and some debug-only logging to help assist with migration.
Changelog: [Changed][Android] RCTEventEmitter has been deprecated in favor of RCTModernEventEmitter for optimal Fabric support; RCTEventEmitter will continue to work in Fabric, but there are minor perf implications.
Reviewed By: mdvacca
Differential Revision: D26027104
fbshipit-source-id: 784ca092bbc88d19c82f6c42537c34460d96de86
Summary:
There's a field called `surfaceID` in a couple of classes that isn't the same as the integer `surfaceId` in Fabric.
For consistency, I've deprecated a couple of them, or renamed when appropriate.
In addition, now we're passing the actual integer surfaceId into the ThemedReactContext. This means that every single View created in Fabric gets annotated with the surfaceId it's in. Currently this isn't used, but the idea is that now each View has a mapping back to its surface, which could be used to simplify / optimize operations with SurfaceMountingManager. In particular, we might be able to use this in the future to optimize animations and/or event emitters.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26021571
fbshipit-source-id: b7db7de123db07fa928a6f815be86bdbb030e62c
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:
MountingManager keeps a map of tags to Views, and attempts to clean it up by (1) deleting tags when they're explicitly deleted, (2) recursively deleting all Views when the View hierarchy is torn down.
However, there appear to be.... substantial gaps here. In tests, when navigating between screen X -> screen Y -> back to X (triggering a StopSurface), each "StopSurface" resulted in 200-600 Views being leaked (!).
What is causing these leaks? Well, for one, the "dropView" mechanism isn't perfect, so it might be missed Views. Second, Views don't always guarantee that `reactTag == getId()`, so that could result in leaks. Third, View preallocation on Android complicates things: Views can be preallocated and then never even inserted into the View hierarchy, so DELETE mutations could never be issued. Fourth, StopSurface is also complicated on Android (largely because of View preallocatioAndroid (largely because of View preallocation).
So, I introduce a new mechanism: keep a list of all tags for a surface, and remove all tags for a surface when the surface is torn down. This should be fool-proof: it handles preallocation and normal creation; it can handle deletes; and we're guaranteed that tags cannot be added after a surface is stopped.
Is this overly complicating things? Well, hopefully we can simplify all of this in the longterm. But until we get rid of View Preallocation, it seems like we need this mechanism - and View Preallocation might be around for a while, or forever.
Other thoughts: it's possible that using other data-structures could be more efficient, but I'm guessing the perf implications here are marginal (compared to the insane amount of memory leaks we're fixing). It could also simplify things to have a SurfaceMountingManager interface that implies all actions happen on a specific surface, including teardown.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25985409
fbshipit-source-id: f55b533770b1630c6c2a9b7a694d953aa3324428
Summary:
Original commit changeset: 3ed8e78e31b0
Backing-out D25938851 (https://github.com/facebook/react-native/commit/69b3016171bb2f994dd4a62c34c2c4645b5a7d56) and D25935785 (https://github.com/facebook/react-native/commit/bdea479a1faa0f1f7d7c9d9162212cce94bc9720). Based on analysis documented in T83141606, I believe this issue should be fixed in JS.
Additionally, this crash actually has nothing to do with (un)flattening or the differ; it is a side-effect of stale ShadowNodes being cloned, which I believe is either UB or a contract violation. Either way, it should probably be fixed either in JS, or in node cloning. So this isn't the right solution for this issue and should be reverted.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25949569
fbshipit-source-id: 8cf1094a767da98fff4430da60d223412e029545
Summary:
This diff changes the type of the SwipeRefreshLayoutManager.size prop from Int to String in Fabric.
The current implementation of this prop allows JS developers to use "int" type when fabric is enables and "int or string" types when using Fabric is disabled.
Since long term we want to only support "string" type for this prop, I'm changing the type of the prop to be String.
After my diff Fabric will start supporting only "string" types, non fabric screens will keep supporting "int or string" values.
**Will this break production?**
No, because there are no usages of RefreshControl.Size prop in fbsource
**What about if someone start using this prop next week?**
IMO It's very unlikely because of the nature of this prop, I will be monitoring next week and if there's an usage it will be detected by flow when trying to land D25933457.
Changelog: [Android][Changed] - RefreshControl.size prop changed its type to string, the valid values are: 'default' and 'large'
Reviewed By: JoshuaGross
Differential Revision: D25933458
fbshipit-source-id: 55067d7405b063f1e8d9bb7a5fd7731f5f168960
Summary:
Android has some optimizations around view allocation and pre-allocation that, in the case of View Unflattening, can cause "Create" mutations to be skipped.
To make sure that doesn't happen, we add a flag to ShadowViewMutation (in the core) that any platform can consume, that indicates if the mutation is a "recreation" mutation.
It is still a bit unclear why this is needed, in the sense that I would expect props revision to increment if a view is unflattened. However, there is at least one documented reproduction where that is *not* the case. So for now, we'll have a hack pending further investigation.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25935785
fbshipit-source-id: 6fb4f0a6dedba0fe46ba3cd558ac1daa70f671f5
Summary:
This diff refactors the intialization of Fabric in order to avoid loading UIManagerModule as part of the creation of FabricJSIModuleProvider.
One caveat is that now we are not taking into consideration the flag mLazyViewManagersEnabled
master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/CoreModulesPackage.java177
if (mLazyViewManagersEnabled) {
As a side effect of this diff view managers will be initialized twice if the user has fabric and paper enabled
This diff was originally backed out in D25739854 (https://github.com/facebook/react-native/commit/4984c1e525e310f15c7d89230fdb2fa8fea91f05) because it produced a couple of bugs:
https://fb.workplace.com/groups/rn.support/permalink/4917641074951135/https://fb.workplace.com/groups/rn.support/permalink/4918163014898941/
These bugs are fixed by D25667987 (https://github.com/facebook/react-native/commit/2e631471092090e743245377742166ecae1d7e26).
This diff was reverted a couple of times because of the change in the registration of eventDispatcher. That's why I'm gating that behavior change as part of the "StaticViewConfig" QE.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D25858934
fbshipit-source-id: a632799ccac728d4efca44ee685519713b4a7cbb
Summary:
These methods can all throw exceptions that get caught and reported by JS. The logviews aren't currently very helpful; hopefully adding additional information will make batch debugging a little easier.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D25870788
fbshipit-source-id: a1cab225b11a3d2868f098d4575e475ee4064e65
Summary:
This diff migrates all the lookups of EventDispatcher to not depend on UIManagerModule anymore.
This refactor is necessary because:
- Users running in Fabric / Venice should not load on the UIManagerModule class
- D25858934 will introduce a change that will break all of these callsites
In the migration I'm relying on the method UIManagerHelper.getEventDispatcherFromReactTag() that returns the correct EventDispatcher for a reactTag.
I'm planning to land this change early in the week (to catch potential errors in alpha / beta versions)
As a followup we need to deprecate and prevent developers to continue using getNativeModule(UIManagerModule.class) moving forward. That will be part of another diff
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D25858933
fbshipit-source-id: e26c99759307517b5bef483274fe0e0d71bb4c6c
Summary:
Hermes has a way to set up a callback that is invoked when a fatal error
such as Out of Memory occurs. It is a static API that should be called at
most once, so it uses `std::call_once` to achieve that.
The fatal error handler is simple, it just uses glog to log an error message
to logcat, then aborts (using `__android_log_assert`).
The reason is typically very helpful for understanding why `hermes_fatal` was called.
Changelog:
[Android][Internal] - Print a logcat message when Hermes has a fatal error
Reviewed By: mhorowitz
Differential Revision: D25792805
fbshipit-source-id: 45de70d71e9bd8eaa880526d8835b4e32aab8fe3
Summary:
This diff logs a SoftError when there is not EventDispatcher associated to UIManager
The app will crash in Debug mode, this will not affect production users
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D25859546
fbshipit-source-id: 8045bcd67f613ea6286f30fe6f3c66113c700b0b
Summary:
The purpose of this diff is to ensure that visibility changes are handled correctly when the value of "display" for a View changes from 'flex' to 'none'.
RNTester is nesting several Views with different kind of visibilities. When the user tap on an item there's a state update that changes the visibility styles for some of these views. Fabric does not reflect the right changes of visibility on the screen.
changelog: internal
Reviewed By: shergin
Differential Revision: D25841763
fbshipit-source-id: 769b97afb72939d346a4c6f2669ff938b35596bc
Summary:
This is the Android native implementation of sendAccessibilityEvent for Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25852418
fbshipit-source-id: cb51e667a7f673da6b9c9e539770225b02bdc902
Summary:
This diff refactors the createViewInstance method in order to ensure that viewID is set before props are updated in the view.
This is necessary because there are components that deliver events at the same time their props are set. This means that some components might not have their viewId set correctly when events are delivered.
Since viewId is used to determine if a view belongs to Fabric or Paper, there are cases when the events are not delivered to the right renderer
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D25667987
fbshipit-source-id: 4acfa8f80d66e9e59514354481957d7d3b571248
Joshua Gross
Valentin Shergin
generatedunixname89002005325674
David Vacca
generatedunixname89002005325676
Ramanpreet Nara
Riley Dulin
Igor Klemenski
