Summary:
Earlier this week I introduced a change in the old, non-Fabric renderer (D20378633 D20427803) that (gated behind a feature-flag) executes ViewCommands before all other types of commands, as a perf optimization and (I think) a potential fix for a category of race conditions.
I've added more details in comments here.
The Fabric renderer uses the same feature-flag that I introduced for the non-Fabric renderer.
Changelog: [Internal] Fabric
Reviewed By: mdvacca
Differential Revision: D20449186
fbshipit-source-id: bb3649f565f32c417a6247369902333989a043aa
Summary:
Simplifying the dispatchMountItems reentrance and retry logic.
Motivation: cleanup so I can work on dispatching ViewCommands before anything else.
Importantly, this gives us the properties that:
1) Only one function is responsible for calling dispatchMountItems
2) Only one function is responsible for deciding if we shouldn't call dispatchMountItems due to reentrance
3) Only one function is responsible for all cleanup
4) Only one function maintains all of the relevant flags (except dispatchPreMountItems... two total now, instead of 4 before)
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20437035
fbshipit-source-id: 5370366790eb25f653bee6c1950e747458374a61
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 diff changes the Fabric measure API in order to support attachments parameters
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D20087252
fbshipit-source-id: 20e1526aaa3695d38d0805416df8a32adb8296ad
Summary:
Until now, there were two measure functions that differ in only one parameter (rootTag). The rootTag is used to use the context associated to the tag as part of the calculation of layout, otherwise it just uses the ReactApplicationContext.
This diff unifies both method into an unique method that
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D20081281
fbshipit-source-id: b1f6a6cedbf78f36f36fd0f93407c23c6996d76b
Summary:
Both the UIManagers use ReactConstants.TAG currently, so certain logs are ambiguous. Use FabricUIManager's tag instead for all logs to disambiguate.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19945587
fbshipit-source-id: 0a9d62b5c0276688b2ad3acf8d893b523c26560a
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:
This diff exposes the surfaceID as part of ReactRootView.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18474014
fbshipit-source-id: 98f651b211d3cc4df88c6b1eb257bc12129eff57
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:
This diff introduces the flag IS_DEVELOPMENT_ENVIRONMENT that will be used in Fabric to control the logging of props, localData and state ONLY during development.
Using DEBUG mode to control the logging of this kind of data is not enough.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18290351
fbshipit-source-id: cf0824bd15b9f1c509bbb284b85761166099bc42
Summary:
Easy diff to extend logging in FabricUIManager class
Changelog: Add extra logging in Fabric Android
Reviewed By: shergin
Differential Revision: D18277487
fbshipit-source-id: 387bdb4b237bdbdc0d65263c1f125ad5c9e26b18
Summary:
This diff annotates FabricUIManager class with NonNull annotations, this will help analysis of nullability plus improving integration with Kotlin clients
Changelog: Add NonNull annotation to FabricUIManager API
Reviewed By: JoshuaGross
Differential Revision: D18010917
fbshipit-source-id: 760ba04b78693cb184172c0fe613c7f808a49031
Summary:
This diff refactors the execution of the logging of Fabric React markers to be executed after the MountItems are executed on the UI Thred
Changelog: Improve logging of Fabric react markers
Reviewed By: JoshuaGross
Differential Revision: D18010920
fbshipit-source-id: e36306102d190119a89c16e660b855acab1528fe
Summary:
This diff refactors the stopping of DispatchUIFrameCallback on FabricUIManager to make it thread safe
Changelog: Refactor the cancellation of dispatching of Mounting operations for Fabric
Reviewed By: JoshuaGross
Differential Revision: D18010922
fbshipit-source-id: 305bc65576698cb785a2a2308cbd03db4a9a97e4
Summary:
While unlikely, in exceptional cases it's possible that `onCatalystInstanceDestroy` is called multiple times, especially since there's no restrictions as to which thread this is called on (currently it's only called on the JS thread from CatalystInstanceImpl) and no synchronization. Make sure it tears down cleanly if called multiple times, and report soft exceptions if that's detected.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18001678
fbshipit-source-id: 4a7c37b5b3c2207ba1dd9c7d85690391f799518d
Summary:
This diff changes updateState method to support null stateWrappers, before this method would crash with a NullPointerException, now it allows the null object to reach the view manager.
Changelog: Add support for update of nullable localState in Fabric
Reviewed By: JoshuaGross
Differential Revision: D17939651
fbshipit-source-id: c62555905e39f9e0db75b9e1d1b93f33d0560266
Summary: Tear down FabricUIManager more aggressively. The rest of the ivars here we can't set to null (yet) because they're marked as final.
Reviewed By: mdvacca
Differential Revision: D17878342
fbshipit-source-id: a1c8ace2603750ac39c3d97e1aca6c486ba5fb79
Summary: These methods are (or should be) only called on the UI thread. Make those assumptions explicit.
Reviewed By: mdvacca
Differential Revision: D17865205
fbshipit-source-id: 9b3acf8f3215a07b1a667ced55e50e99a488de79
Summary: Although `mBinding` is unregistered which means the connection to the JNI-bridged Cxx object can be destructed, we still hold onto the `mBinding` Java object after unregistering. That doesn't seem desirable, I think we should just clear it out here for consistency.
Reviewed By: mdvacca
Differential Revision: D17865206
fbshipit-source-id: 1ad8643c48ba0b2d52620a7b8ebe8a52928648ef
Summary:
We use `ViewManager.onMeasure` to perform measurements of Android views and pass the measured size back to Yoga. For Android in order to the report correct dimensions of a View, this View must be created using a Context that has a theme associated with it. Before, `onMeasure` only had ReactApplicationContext passed as the first parameter and ReactSwitch, for example, could not be measured correctly (because it uses the size of the thumb drawable, which is extracted from the current theme). This diff adds surfaceId as the first parameter of `FabricUIManager.measure`, so that we can retrieve ThemedReactContext and pass it to `ViewManager.onMeasure`.
The size of the Switch component is still incorrect, but at least the size reported back to Yoga is the same as in Paper. So there is more investigation necessary why this happens in Fabric. I will investigate and publish another diff with the fix.
Reviewed By: JoshuaGross, shergin
Differential Revision: D17625959
fbshipit-source-id: 48197a61240fb13042bef3e9f5d681acacc702fb
Summary:
Collapse many Remove/Delete mount items into a single batched item.
Since a delete is always preceded by a remove mountitem, we can batch these into one instruction. Since deletes tend to come in large blocks, it might make sense to batch many into a single instruction.
Reviewed By: mdvacca
Differential Revision: D17254631
fbshipit-source-id: abfd54cdb0bbb9a4c0880ec8e8bbd681367aecd4
Summary:
Log every item in a BatchMountItem. There's a lot of useful debug information being hidden there currently.
Changelog:
[Internal]
Reviewed By: mdvacca
Differential Revision: D17254629
fbshipit-source-id: c72f0aa8506059da5225ebead24d3f8ead5bdebd
Summary: Support for `sendAccessibilityEvent` in the FabricUIManager.
Reviewed By: shergin
Differential Revision: D17142507
fbshipit-source-id: 5c131d7caa1e4189fd41ecfb558d0027394b6a15
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: For Litho interop and to resolve T47926405, always pass props and state to Create mount item so that any ViewManager can create view instances with knowledge of initial props and state.
Reviewed By: mdvacca
Differential Revision: D16554082
fbshipit-source-id: 3b19a43347b0fa201a054eec60e82fb77cad3625
Summary: I'm only renaming the new `addRootView` that I added (which takes the moduleName, and uses startSurfaceWithConstraints), since the other one implements the UIManager interface method that's shared with paper.
Reviewed By: shergin
Differential Revision: D16432425
fbshipit-source-id: 392af42690052551504676df776bac6d1a968785
Summary: Right now we register ReactFabric as a callable module with the bridge so that we can call `ReactFabric.unmountComponentAtNode` in `ReactInstanceManager.detachViewFromInstance`. In bridgeless mode we don't have callable modules, so I'm just setting a global variable that can be called from C++ instead. Using this in a new `unmount` method in FabricUIManager.
Reviewed By: shergin, mdvacca
Differential Revision: D16273720
fbshipit-source-id: 95edb16da6566113a58babda3ebdf0fc4e39f8b0
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: The method FabricUIManager.scheduleMountItems receives only one MountItem, is makes more sense to be called FabricUIManager.scheduleMountItem
Reviewed By: JoshuaGross, makovkastar
Differential Revision: D16062749
fbshipit-source-id: a27063be33b644af83ede6a9198edbfb1c3296e1
Summary: This diff exposes LayoutDirection as part of UpdateLayoutMountItem
Reviewed By: JoshuaGross
Differential Revision: D16060521
fbshipit-source-id: 163bf2a0bdca62dcecb03a8aaa2f4bf595b18c8f
Summary:
This diff formats the Java class files inside xplat/js/react-native-github. Since google-java-format was enabled in D16071401 we want to codemode the existing code so that users don't have to deal with formatter lint noise at diff-time.
```arc f --paths-cmd 'hg files -I "**/*.java"'```
drop-conflicts
Reviewed By: cpojer
Differential Revision: D16071725
fbshipit-source-id: fc6e3852e45742c109f0c5ac4065d64201c74204
Summary:
Right now JS triggers a view manager command with the following code:
```
UIManager.dispatchViewManagerCommand(
ReactNative.findNodeHandle(this),
UIManager.getViewManagerConfig('RCTView').Commands.hotspotUpdate,
[destX || 0, destY || 0],
);
```
As we want to get rid of calls to UIManager, we need to stop looking for the integer defined in native from JavaScript. We will be changing methods like this to be:
```
UIManager.dispatchViewManagerCommand(
ReactNative.findNodeHandle(this),
'hotspotUpdate',
[destX || 0, destY || 0],
);
```
We need to support ints and Strings to be backwards compatible, but ints will be deprecated.
Reviewed By: shergin
Differential Revision: D15955444
fbshipit-source-id: d1c488975ae03404f8f851a7035b58a90ed34163
Summary: This diff adds extra logging in FabricUIManager, this will be useful to debug production issues
Reviewed By: JoshuaGross
Differential Revision: D15907520
fbshipit-source-id: 94e16444af3c023b6c4837c4797404d3debe8e95
Summary: I added this function in D15810990, not sure why lint didn't complain about this...
Reviewed By: zackargyle
Differential Revision: D15880129
fbshipit-source-id: 4d207ba26be46186dc3904c82d82a4a83d4d4eb7
Summary: This diff forces the method: scheduler.constraintSurfaceLayout to run on the JS thread.
Reviewed By: JoshuaGross
Differential Revision: D15845768
fbshipit-source-id: de2aa69f301770aaf6cb7c3f5670548a3b6110df
Summary: Scheduler.startSurface accepts LayoutConstraints and LayoutContext, but for some reason we don't use these on Android. This diff adds new methods to Binding and FabricUIManager to start a surface with the provided measurespecs. I created new methods to avoid affecting the functionality of any surfaces already using Fabric, but if we want this behavior everywhere then I can just add it to the existing `addRootView` and `startSurface`.
Reviewed By: JoshuaGross, shergin
Differential Revision: D15810990
fbshipit-source-id: 6cd9a58b125461f91253458905405298cfb723ce
Summary: Easy diff to create a ReactFeatureFlag to enabled logging in Fabric
Reviewed By: JoshuaGross
Differential Revision: D15803582
fbshipit-source-id: d735f24850bddf43c27b97d006100cbb8f0cc6e3
Summary:
We currently have two different codepaths for actually rendering a surface with Fabric on iOS and Android: on iOS we use Fabric's `UIManagerBinding.startSurface` to call `AppRegistry.runApplication`, but on Android we don't; instead we use the same codepath as paper, calling `ReactRootView.runApplication`.
This diff does a few different things:
1. Unify iOS and Android by removing the `#ifndef` for Android so that we call `startSurface` for both
2. Pass through the JS module name on Android so that this actually works (it currently passes in an empty string)
3. Remove the call to `ReactRootView.runApplication` for Fabric so that we don't end up doing this twice
4. Copy over some logic that we need from `ReactRootView.runApplication` (make sure that root layout specs get updated, and that content appeared gets logged)
Reviewed By: mdvacca
Differential Revision: D15501666
fbshipit-source-id: 5c96c8cf036261cb99729b1dbdff0f7c09a32d76
Samuel Susla
Joshua Gross
David Vacca
Andres Suarez
Oleksandr Melnykov
Emily Janzer
Eli White