Summary:
sDidInit can be accessed from different threads, this diff refactors the definition of this variable to be volatile and also to be assigned at the end of the staticInit() method.
Changelog:
Ensure proper initialization of FabricSoLoder
Reviewed By: ejanzer
Differential Revision: D18010919
fbshipit-source-id: 3ec7b19fdc15056b90fc01281b8c3888e93a7dd3
Summary:
In `tearDownReactContext`, `reactContext.destroy()` sets `mCatalystInstance` to null. We cannot call `reactContext.getCatalystInstance()` after that without hitting an assertion. Change ordering so that doesn't happen in reload or teardown.
Changelog: [Internal]
Reviewed By: makovkastar
Differential Revision: D18041279
fbshipit-source-id: 22658dc506b76cf58aee1008841abacfe9410c9d
Summary:
This diff adds an experiment to disable the preallocation of views on the mounting layer of Fabric
Changelog:
Add a ReactNativeConfig to configure the preallocation of views in the mounting layer of Fabric
Reviewed By: shergin
Differential Revision: D17949681
fbshipit-source-id: 0af63df22aff9e94289bc8a8217c79222f1fd61c
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:
WebSocketModule can be called asynchronously while the ReactContext/CatalystInstance is being torn down. Trying to get a JS module at this time will result in a crash if the ReactContext has already torn down Catalyst. Check explicitly that Catalyst is still alive before trying to emit an event to JS via some JS module.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18020359
fbshipit-source-id: 1b77abd457c7d97bd241389251890bb682b6fde3
Summary:
AppStateModule can be called indirectly when the ReactContext is torn down. Trying to get a JS module at this time will result in a crash if the ReactContext has already torn down Catalyst. Check explicitly that Catalyst is still alive before trying to emit an event to JS via some JS module.
Changelog: [Internal]
Reviewed By: ejanzer, mdvacca
Differential Revision: D17969056
fbshipit-source-id: dd446de57280e588a73f3e228bac82b3d67ecdc0
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:
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 diff checks in the output from `fbsource//xplat/js:FBReactNativeSpec-flow-type-android-files` into `react-native-github/ReactAndroid/src/main/java/com/facebook/fbreact/specs/`.
To update the codegen output, please run:
```
js1 build oss-native-modules-specs -p android
```
To use the codegen, add the following as a buck dep to your target:
```
fbsource//xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/fbreact/specs:FBReactNativeSpecAndroid
```
Changelog: [Android][Added] Check in TurboModules codegen
Reviewed By: fkgozali
Differential Revision: D17990380
fbshipit-source-id: 94895e375311e9360c308dffadf1558a5ff04212
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:
This PR fixes Android CI.
## Changelog
[Android] [Fixed] - fix Android CI
Pull Request resolved: https://github.com/facebook/react-native/pull/26862
Test Plan: Android CI is green again
Differential Revision: D17925576
Pulled By: mdvacca
fbshipit-source-id: 90bdccda96a11210254a9eaab3c373151057f332
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:
The Android makefiles had hard-coded paths to hermes-engine, sometimes looking in `node_modules` and other times looking in `..`. This commit implements the Node module resolution algorithm (see common.mk), which handles both of these cases and also looks further up the root if necessary, handling the case when the `hermes-engine` npm package is hoisted.
This commit does three things:
- Defines `find-node-module` and uses it in the makefiles to find `hermes-engine`
- Removes the unused `/path/to/hermes-engine/include` paths since this directory does not exist and should be `/path/to/hermes-engine/android/include` (`android`)
- Moves the definition of `REACT_NATIVE` in the makefiles to the top. It was defined after every `$(CLEAR_VARS)` invocation but was not actually cleared anyway. `$(CLEAR_VARS)` resets only `LOCAL_*` variables in this list: https://android.googlesource.com/platform/build/+/7dc45a8/core/clear_vars.mk
## Changelog
[Internal] [Changed] - Android Makefiles look for hermes-engine using Node's module resolution algorithm
Pull Request resolved: https://github.com/facebook/react-native/pull/26820
Test Plan: Run `./gradlew :ReactAndroid:installArchives`, which requires the hermes-engine paths to be correct in order to find the headers.
Differential Revision: D17923671
Pulled By: cpojer
fbshipit-source-id: 9238b8718a94080db1abbba6375a6a1d484c871d
Summary:
Currently the react native framework doesn't handle the accessibility state changes of the focused item that happen not upon double tapping. Screen reader doesn't get notified when the state of the focused item changes in the background.
To fix this problem, post a layout change notification for every state changes on iOS.
On Android, send a click event whenever state "checked", "selected" or "disabled" changes. In the case that such states changes upon user's clicking, the duplicated click event will be skipped by Talkback.
## Changelog:
[General][Fixed] - Announce accessibility state changes happening in the background
Pull Request resolved: https://github.com/facebook/react-native/pull/26624
Test Plan: Add a nested checkbox example which state changes after a delay in the AccessibilityExample.
Differential Revision: D17903205
Pulled By: cpojer
fbshipit-source-id: 9245ee0b79936cf11b408b52d45c59ba3415b9f9
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: UseVanillaJNI flag was missing for measure and baseline functions
Reviewed By: amir-shalem
Differential Revision: D17868201
fbshipit-source-id: 95d6843d643e90157a51550d6efbf059f0ca2c39
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:
Add UI thread assertions and annotations to ReactRootView. Shouldn't have any immediate effect since these methods already call other methods that assert they're on the UI thread. Doing this to hoist assumptions higher up.
Doing some very simple refactoring to the way cleanup happens to ensure aggressive cleanup in more instances.
Reviewed By: shergin
Differential Revision: D17860291
fbshipit-source-id: c87e0336594fa2327271b648eb8340e250235250
Summary: set missing useVanillaJNI, it was causing part of the unit-tests to run in fbjni instead of vanillajni.
Reviewed By: SidharthGuglani
Differential Revision: D17852635
fbshipit-source-id: 5bc187d90fbdc430015be55a015a7d1e0ba0ebc6
Summary: This PR caused a regression with shadows, see https://github.com/facebook/react-native/issues/26544.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D17809320
fbshipit-source-id: 0c83cd211425622ada0fd8c492f43df0536a4b8a
Summary: Use TestParametrization to test both fbjni and vanilla jni versions
Reviewed By: amir-shalem
Differential Revision: D17788718
fbshipit-source-id: 0f3317b7403cadca7b7ccd9140f1933d746bf433
Summary: This diff moves methods related to actions on YGNode like free node, reset node etc. to vanilla JNI
Reviewed By: amir-shalem
Differential Revision: D17668008
fbshipit-source-id: 03bfc51ec1fcf06569713400f984d551827e22fe
Summary:
This PR addresses issue https://github.com/facebook/react-native/issues/23870 (`View.getGlobalVisibleRect()` is broken in some use cases)
The issue affects the following Android APIs:
- ViewGroup.getChildVisibleRect()
- View.getGlobalVisibleRect() (Which calls into ViewGroup.getChildVisibleRect)
- View.getLocalVisibleRect() (Which calls into View.getGlobalVisibleRect())
According to Android documentation, View.getGlobalVisibleRect() should provide a rect for a given view that has been clipped by the bounds of all of its parent views up the view hierarchy. It does so through the use of the recursive function ViewGroup.getChildVisibleRect().
Since React Native has a separate clipping mechanism that does not rely on Android UI's clipping implementation, ViewGroup.getChildVisibleRect() is unable to determine that a rect should be clipped if the clipping view is a ReactClippingViewGroup. This resultantly breaks some important use cases for things like testing with Detox, which relies on this functionality to tell when a component is on-screen, as explained in the above referenced issue.
The rationale of the fix is essentially to implement logic analogous to [ViewGroup.getChildVisibleRect()](https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/view/ViewGroup.java#6176), discarding irrelevant Android clipping modes, and instead testing against the 'overflow' property, restoring the originally intended functionality. This is implemented as an override to ViewGroup.getChildVisibleRect() in the following classes:
- ReactViewGroup
- ReactScrollView
- ReactHorizontalScrollView
Unfortunately, since the public ViewGroup.getChildVisibleRect() API recurses into a `hide` annotated API which cannot be overridden, it was necessary to provide this override in each of the above React Native classes to ensure the superclass implementation would not be called, which would break the recursion.
## Changelog
[Android] [Fixed] - View.getGlobalVisibleRect() clips result rect properly when overflow is 'hidden'
Pull Request resolved: https://github.com/facebook/react-native/pull/26334
Test Plan:
The functionality in question is neither used internally nor exposed by React Native, and thus only affects Android native modules that use the above referenced APIs.
As such, I have primarily performed testing with a forked example project that had been submitted with issue https://github.com/facebook/react-native/issues/23870, originally by d4vidi.
The example project can be found here:
- [Configured to build against RN Master](https://github.com/davidbiedenbach/RNClipVisibilityBugDemo/tree/rn-master)
- [Configured to build against PR branch](https://github.com/davidbiedenbach/RNClipVisibilityBugDemo/tree/fix-23870)
(Original project here: https://github.com/d4vidi/RNClipVisibilityBugDemo)
### Bug in effect:
When built against RN master, it can be observed that fully clipped views are reported as visible, as in the below screenshots.
#### Views inside a ReactViewGroup do not report as clipped
#### Views inside a ReactScrollView do not report as clipped
#### Views inside a ReactHorizontalScrollView do not report clipping properly
### Bug fixed
When built against the PR branch, fully-clipped views no longer report visible.
#### Views inside a ReactViewGroup report clipping properly
#### Views inside a ReactScrollView report clipping properly
#### Views inside a ReactHorizontalScrollView report clipping properly
Reviewed By: mdvacca
Differential Revision: D17782658
Pulled By: yungsters
fbshipit-source-id: 0cd0d385898579a7a8a3e453f6ba681679ebe496
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: Fabric doesn't support setNativeProps, so we have to use commands instead to set the value of the native component.
Reviewed By: JoshuaGross
Differential Revision: D17736274
fbshipit-source-id: 18c47365926c3c2cfc3551f4b5b6cc72e4162367
Summary: Adding a second exception with a different error message so we can tell how the ViewManagerRegistry was created when it fails to find a view manager.
Reviewed By: rickhanlonii, JoshuaGross
Differential Revision: D17759060
fbshipit-source-id: 4415fa7440395f7e5730d2b77088f16a1a6a819d
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:
Currently, horizontal `ScrollViews` don't properly support `removeClippedSubviews`.
This is because the container view, `ReactHorizontalScrollContainerView` extends from regular `ViewGroup` instead of `ReactViewGroup` so doesn't have all the logic around clipping children.
Moreover, the `ReactHorizontalScrollContainerViewManager` doesn't actually support the `removeClippedSubviews` prop
This change:
- Makes `ReactHorizontalScrollContainerView` extend from `ReactViewGroup` while maintaining the special logic around RTL scrolling
- Factors out a common `ReactClippingViewManager` which will be used to bridge all components which extend `ReactViewGroup` and support clipping. It has the logic for adding/removing children and getting child counts while considering clipped subviews
- `ReactViewManager` now extends this new `ReactClippingViewManager`
- `ReactHorizontalScrollContainerViewManager` also extends `ReactClippingViewManager`
Reviewed By: JoshuaGross
Differential Revision: D17708630
fbshipit-source-id: d257566ee54ad46f6d62f176a657c596bba96aa4
Summary:
Fabric expects the measure method to return the size in density-independent pixels, but getMeasuredWidth and getMeasuredHeight return pixels on Android, so we have to convert these values before returning to C++.
Check the method createUpdateLayoutMountItem in Binding.cpp:
```
local_ref<JMountItem::javaobject> createUpdateLayoutMountItem(
const jni::global_ref<jobject> &javaUIManager,
const ShadowViewMutation &mutation) {
...
int x = round(frame.origin.x * pointScaleFactor);
int y = round(frame.origin.y * pointScaleFactor);
int w = round(frame.size.width * pointScaleFactor);
int h = round(frame.size.height * pointScaleFactor);
auto layoutDirection = toInt(newChildShadowView.layoutMetrics.layoutDirection);
return updateLayoutInstruction(
javaUIManager, newChildShadowView.tag, x, y, w, h, layoutDirection);
}
return nullptr;
}
```
We are interested in the next two lines:
```
int w = round(frame.size.width * pointScaleFactor);
int h = round(frame.size.height * pointScaleFactor);
```
`frame.size.width` and `frame.size.height` are the values returned from the measure method in Java and they are multiplied by the screen density to get the size in pixels, which means Fabric expects these values to be DIPs.
Reviewed By: shergin
Differential Revision: D17626834
fbshipit-source-id: f9856b5d0796c75c26c84adf11e1652b22a1ddef
David Vacca
Joshua Gross
Ramanpreet Nara
Dulmandakh
Andres Suarez
James Ide
Sharon Gong
Sidharth Guglani
Pavol Fulop
Amir Shalem
Ram N
Oleksandr Melnykov
David Biedenbach
Emily Janzer
Alexander Kawrykow
