Summary:
Fix warnings about implicit type truncation.
## Changelog
[Internal] [Fixed] - Fix various C++ warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/31002
Test Plan:
Almost all the changes here are simply making explicit conversions which are already occurring. With the exception of a couple of constants being changed from doubles to floats.
With these changes I am able to remove a bunch of warning suppressions in react-native-windows.
Reviewed By: shergin
Differential Revision: D26900502
Pulled By: rozele
fbshipit-source-id: d5e415282815c2212a840a863713287bbf118c10
Summary:
Removing unsafe cast from `int` to `uint16_t`.
Also, adding code to detect multiplication overflow during buffer resize.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Fix] - Fix unsafe cast and detect overflow in MapBuffer.
Pull Request resolved: https://github.com/facebook/react-native/pull/31106
Test Plan: Code compiles in Visual Studio 2019 without the unsafe cast warning (or error depending on the configuration).
Reviewed By: mdvacca
Differential Revision: D26865138
Pulled By: rozele
fbshipit-source-id: 4692a38b05fc873e31fbbe94d70803244e82de5d
Summary:
Changelog: [Internal]
Updates `ReactSurface` to use `SurfaceHandler` internally.
This removes most of the internal state in `ReactSurface` and propagates all the calls to the `SurfaceHandler`.
`FabricUIManager` now uses `SurfaceHandler` to start/stop the surface.
SurfaceId is still used for view operations. SurfaceId is also now mutable to play better with existing Android infra.
Reviewed By: shergin, mdvacca
Differential Revision: D26112992
fbshipit-source-id: 52e6860084d739381317035dc3011956d452063c
Summary:
Changelog: [internal]
The assumption was wrong. There can be more than one view on iOS in ScrollView. The other view is pull the refresh component.
Reviewed By: mdvacca
Differential Revision: D26880403
fbshipit-source-id: 69fb1668b5ee7d5f4a4ab3c89222587689c10591
Summary:
Changelog: [internal]
UIManager shouldn't be retained in lambda because if it outlives runtime it causes a crash.
UIManager -> ComponentDescriptorRegistry -> ParagraphComponentDescriptor -> TextLayoutManager's cache -> AttributedString::Fragment -> ShaviewView -> EventEmitter -> EventTarget -> Pointer
This is the chain of ownership that can cause a crash if UIManager is retained for longer than JS runtime.
{F459238235}
Reviewed By: JoshuaGross
Differential Revision: D26851194
fbshipit-source-id: 644cdee34ba9286618659d847fb2e78bc301d049
Summary:
Changelog: [internal]
StateWrapperImpl shouldn't retain State strongly because cleanup of `StateWrapperImpl` is triggered from Java and isn't guaranteed to take happen before runtime is destroyed.
This should resolve crash where `StateWrapperImpl`'s destruction causes a `~Pointer` to be called after runtime is destroyed.
Chain of ownership that will be broken by storing State weakly inside `StateWrapperImpl`.
`StateWrapperImpl -> ParagraphState -> TextLayourManager's cache -> AttributedString -> ShadowView -> EventEmitter -> EventTarget -> Pointer`
{F451105831}
Reviewed By: JoshuaGross
Differential Revision: D26815275
fbshipit-source-id: 0703c6dccc62c1d152923b786a83273fa8a03694
Summary:
This diff extracts ComponentNameRegistry out of Fabric modules
This is necessary to avoid depending on Fabric and regressing APK size for other RN apps (e.g. IG)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26765328
fbshipit-source-id: 0a22c4279146f5243473c74a84e78fad7f08f956
Summary:
Introduce the NativeComponentRegistryBinding class, the purpose of this class is to register the global function __nativeComponentRegistry__hasComponent into JS
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D26716902
fbshipit-source-id: d883b195b30600d3781d41158fd8c57cf22431df
Summary:
We want to be able to instrument touch processing delays in JS, which does not have access to systemUptime; therefore we want a UNIX timestamp, which JS has access to and can compare to the touch time.
It only matters that there is relative consistency between multiple touch events in JS, which is still the case; so this should have no impact on product code.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26705430
fbshipit-source-id: 2acd52ae5873a44edf1e0cb126a9a6c87203d8fa
Summary:
The is how it works:
* Text is a quite special component with special properties and constraints. Some of them are:
It's expensive to measure (layout) text. It's expensive to measure and expensive to pass AttributedString via JNI.
* When we measure text, we don't concerned about maximum height, only maximum height is important. (Even though theoretically, there are text layout systems that can balance these constraints (max height and width) trying to find a perfect result, we don't use such complex (and expensive) layout engines for building UIs).
Yoga, as a flexbox engine, does not aware of such constraints, so it requests remeasuring of text components quite often, so we have an RN built-in text measure cache system just for text measurements that suit these constraints. This way when Yoga requests a text measuring, we always measure with `Inf` height and store that result in the cache. And when Yoga requests another measure with the same width but a different height we retrieve the value from the cache and then just clamp it.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26696637
fbshipit-source-id: f65b275d33c77073bc2359cbf0a741ddcf05d8d4
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695253
fbshipit-source-id: b0ff02416ab208dfd71f71c2b3d83009f40be0a9
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695224
fbshipit-source-id: 3044049cb2447a733a9ecae84dcc099b26731acd
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695203
fbshipit-source-id: df09af5a62044c711368954b5e9b3a114491e2ed
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695184
fbshipit-source-id: fd98843f3485e13c9650c5a2576a1186ebb121db
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695162
fbshipit-source-id: 5615355f76b9c78d0f8981b3443b7c5900939ede
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695151
fbshipit-source-id: f5447e0a6d2b6bc06ff9456a35386a22106102f8
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695125
fbshipit-source-id: 3055dd3db7cd34bba9b3fc141032ac0f663523a1
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695097
fbshipit-source-id: 0a861b9f8a435267b16dcb9c37fd501901a544fd
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695073
fbshipit-source-id: f59b6ce7d8f2fd2c68bba41070228be981684ce0
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695055
fbshipit-source-id: d70fdda569c09277d511dc154605559ef3cd56e8
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695038
fbshipit-source-id: afcf2aecad8305b112e1b4ddcc1693380f3defcc
Summary:
For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D26695016
fbshipit-source-id: 63e6f6fc919076d94f04416f6821f21e0b3707a3
Summary:
This diff fixes a bug in the calculation of layout for text components
The rootcause of the bug is that fabric is not taking into consideration height constraints as part of the cache for text measurments.
The title text was being measured with a specific height constraint (22px) at the begining of the render, later there was a re-measure for the same Text component with a different height constraint, but fabric was reusing the result of the first calculation instead of re-measuring the text.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26676716
fbshipit-source-id: 3e769e0ca35b3e363b96d3a6d1626a091eaad908
Summary:
We recently fixed RTL scrolling in Fabric on iOS: D26608231 (https://github.com/facebook/react-native/commit/e5921f7f384af45df4f355fa3fa1b58a20a269d3)
Turns out, the mechanism for RTL scrolling on Android is completely different. It requires that content be wrapped in a "directional content view", which is `View` in LTR and `AndroidHorizontalScrollContentView` in RTL, backed by `ReactHorizontalScrollContainerView.java`.
iOS doesn't require that and just uses View and some custom logic in ScrollView itself.
In the future it would be great to align the platforms, but for now, for backwards-compat with non-Fabric and so we don't have to tear apart ScrollView.js, we codegen the AndroidHorizontalScrollContentView so it exists in C++, register the component, and stop mapping it to View explicitly in C++.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26659686
fbshipit-source-id: 3b9c646dbdb7fe9527d24d42bdc6acb1aca00945
Summary:
Changelog: [Internal]
Adds react_debug dependency in Android.mk where it was missing
Reviewed By: mdvacca
Differential Revision: D26617400
fbshipit-source-id: 5ac799269b106eadd881d30490ac34bd2134a9b7
Summary:
Changelog: [internal]
Check for null before passing it to `EventTarget` constructor.
Reviewed By: JoshuaGross
Differential Revision: D26605779
fbshipit-source-id: a2773c8123d83c25736bccefe656d1def8794091
Summary:
Changelog: [internal]
`shadowNodeFromValue` can return nullptr. Let's make sure it returns valid value before dispatching command.
Reviewed By: JoshuaGross
Differential Revision: D26605350
fbshipit-source-id: eb9a0347c95ba07fd7e9b7ddeca7e6d6011f50ad
Summary:
Changelog: [internal]
### Why does the crash happen?
The crash can happen if runtime is destroyed before background executor lambda is run. Destroying a shadow node after runtime leads to a crash through the chain of ownerships.
Chain of ownership:
`ShadowNode -> ShadowNodeFamily -> EventEmitter -> EventTarget -> Pointer`
Pointer tries to call `invalidate` method on raw pointer to the runtime which is gone.
https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/jsi/jsi/jsi.h?commit=2ee3ae0c6a64&lines=335-339
To work around this, weak pointers are passed to lambda. This way the lambda is less likely to be the last owner of shadow nodes. Possibility of race still exists but it less likely to happen.
## Other solution
Alternatively, we could make sure native Runnable queue in Java is emptied as part of tear down process. We can even implement both solutions as they are semantically correct.
Reviewed By: shergin
Differential Revision: D26582554
fbshipit-source-id: b1b8a92237902bc4c40376176f575caa24a41a05
Summary:
Surprisingly, it's not that trivial to pass `LayoutContrants` to `YGNodeCalculateLayout` in a way that always works. The problem is that `YGNodeCalculateLayout` does not allow expressing the constraints explicitly, so we need to pass them as `YGStyle` properties of a root node. With this approach, we unconditionally apply them as `YGStyle`s as actual values or `Undefined` value (which overrides some other values that can be previously set by calling this function or other code). We also intentionally preserve `height` and `width` values because it's a common use-case when a component explicitly specifies its size.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26583550
fbshipit-source-id: 2cd506fbdc9e6a1a8f119d09ccfd34f876a13625
Summary:
Changelog: [internal]
shergin found that folly's merge_patch implementation doesn't propagate `null` correctly (details in D26435620 (https://github.com/facebook/react-native/commit/1e9f63fe277c42d812ef007ced7eff1688602b62)). This is a requirement and needs to be adjusted in props forwarding on Android.
As far as we know this isn't causing any bugs but it is an error that should be fixed.
Reviewed By: shergin
Differential Revision: D26545821
fbshipit-source-id: 9edd24aecfcde17f5d9c1197f65db0e0f3f9e364
Summary:
We have a custom STUB_VIEW_ASSERT that helps debugging stub view issues on platforms (like iOS) where flushing logs around assert-time isn't 100% guaranteed.
Move that logic into react_native_assert since it's generally useful.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26567218
fbshipit-source-id: 79f5ae66fc65a0af48dbcf4c7204ac8245911cb0
Summary:
See react_native_assert.{h,cpp}. Because of the BUCK+Android issue where NDEBUG is always defined, we use react_native_assert instead of assert to enable xplat asserts in debug/dev mode.
This migrates most of the codebase, but probably not 100%. The goal is to increase assertion coverage on Android, not to get to 100% (yet).
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26562866
fbshipit-source-id: a7bf2055b973e1d3650ed8d68a6d02d556604af9
Summary:
These assertions will already run by default on iOS, macOS, Windows, etc, all OSes except Android because of a longstanding Buck+Android NDEBUG issue.
Align platforms by using REACT_NATIVE_DEBUG flag.
Only impacts debug/dev builds.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26561977
fbshipit-source-id: 324875c48b2a138e8ab55630c3e2ec43c2f768c3
Summary:
Use REACT_NATIVE_DEBUG for consistent branding and to prevent potential collisions with other codebases.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26517424
fbshipit-source-id: c85740d4e5320cc14023eb6f521bb1a242ae56fe
Summary:
Use react_native_assert in LayoutAnimations to enable asserts to fire on Android.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26517096
fbshipit-source-id: f000c4848f29c8779170625d357f547f2e9e6365
Summary:
This will allow these asserts to crash on Android debug builds.
We will migrate more sites as we confirm this is stable through testing.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26409354
fbshipit-source-id: fb35cd8de29890f7c2b761435eaa02de377bdd1e
Summary:
BUCK always defines NDEBUG on Android builds. This is a longstanding issue and it's tricky to work around.
Previous attempts to fix this within React Native were difficult because disabling NDEBUG caused lots of issues that were difficult to track down.
Instead, I am (1) introducing a new RN_DEBUG flag that can be used cross-platform, (2) whenever NDEBUG is *not* enabled, RN_DEBUG will automatically be defined, (3) enables debug-only code to be compiled on Android, (4) enables us to selectively, slowly migrate `assert` to `rn_assert` in a way that doesn't impact non-Android platforms, but allows us to maintain stability of Android debug builds.
Actually enabling the RN_DEBUG flag in debug builds is done in FB-internal code. I assume the NDEBUG issue is not a problem when compiling in open-source without BUCK.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26409355
fbshipit-source-id: 285b8073bba3756834925727bfa28d3c6bc06335
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/30988
We have a bunch of flags scattered throughout the codebase with poor hygiene and commenting. Consolidate.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26392518
fbshipit-source-id: 2823de123a5009d6b8c358e8a3f451b9fa0e05b7
Summary:
This diff defines an initial implementation of MapBuffer class. This is an unfinished implementation and the API and internals is going to change considerably in the next days.
The purpose of this stack is to experiment with ByteBuffers moving data from C++ into Java and learn about what're the performance implications of this model.
The format of serialization is going to change in the next few days. I'm going to follow a format similar to https://fb.quip.com/3ENaA782rkkC
I'm expecting to iterate on this API as we expand the development of the new JNI system, PLEASE read all the TODOs as you are reviewing the code.
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D26364354
fbshipit-source-id: 94e434f699a4250dd240342386eddeaa6acd3ba2
Summary:
The standard merge_patch (aka RFC7386) mechanism that we used before removes the key-value pairs from the original object in case if the patch has a `null` value. And we don't need it there because we should pass this null value down to the mounting layer to clean up this prop there. Besides that, the patch should not be recursive because props are not divisible.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26435620
fbshipit-source-id: 0d7612c6ca04dcbc122ff6add3777674e3868af8
Summary:
In D26292378 (https://github.com/facebook/react-native/commit/81147b6f793fbc00b81501393371bb332641f4c8) we changed the way the layout constraints are specified to Yoga for measuring and layout. This is a second iteration of the change that slightly more correct and fixes other problematic cases we discovered. See also the commend in the code.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26412484
fbshipit-source-id: 06011982a63cd4d3b61ae295f9aba6f8dab6ca02
Summary:
Building ReactAndroid from source on Windows has recently hit the limitation of maximum path lengths.
At build time, during the `:ReactAndroid:buildReactNdkLib` task, the linker tries to access several of the intermediate binaries located deep in the tmp folder hierarchy, eg.
```
D:\r\ReactAndroid\build\tmp\buildReactNdkLib/local/armeabi-v7a/objs/react_render_components_progressbar/D_/r/ReactAndroid/__/ReactCommon/react/renderer/components/progressbar/android/react/renderer/components/progressbar/AndroidProgressBarMeasurementsManager.o
```
**Suggested fix:** for modules such as `react_render_components_progressbar` and `react_render_components_picker`, rename them to `rrc_progressbar` etc.
**NOTE**: this assumes that the fix from https://github.com/facebook/react-native/issues/30535 is in place. This regression happened while https://github.com/facebook/react-native/issues/30535 has been pending checkin.
**Other mitigations I've tried:**
- setting [`LOCAL_SHORT_COMMANDS`](https://developer.android.com/ndk/guides/android_mk#local_short_commands) for the problematic modules or `APP_SHORT_COMMANDS` for the root project. Turns out those commands don't work on the NDK version RN requires, but even after manually applying a [patch ](https://android-review.googlesource.com/c/platform/ndk/+/1126440) to my local copy of the NDK, these flags had no effect.
- moving the repo directory higher in the file system tree, and using short directory names `D:\r\...` was not enough
- creating virtual drive letters for specific long paths with the [`sust`](https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/subst#examples) command is not workable, since they depend on the source folder structure, and get partly generated by the build system, which I can't plug into
- just enabling long path support on Windows is not enough, since the compiler toolchain doesn't support them.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[Android] [Fixed] - Fix source build on Windows machines vol. 2
Pull Request resolved: https://github.com/facebook/react-native/pull/30776
Test Plan:
Run `.\gradlew installArchives`
Before:
Now:
Differential Revision: D26194286
Pulled By: mdvacca
fbshipit-source-id: 778b5a0515148e2ace19e7902f74301831ebed94
Andrew Coates
Valentin Shergin
Igor Klemenski
Andrei Shikov
Samuel Susla
David Vacca
Joshua Gross
