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:
changelog: [internal]
### When does leak happen?
Leak happens anytime a callback isn't executed inside native module, it will never get cleaned up.
Imagine a native module with method that takes onSuccess and onFail callbacks. Only one of them will be called at any time and the other one will leak.
### Why does it leak?
It leaks because when `CallbackWrapper` is created using `CallbackWrapper::createWeak`. Inside `CallbackWrapper::createWeak`, the newly created object is inserted into `LongLivedObjectCollection`. This object collection will keep it alive until `CallbackWrapper::destroy` is called, which isn't called in case closure isn't executed.
### Solution
Introduce class RCTBlockGuard which ties cleanup of resources to lifetime of the block.
Reviewed By: RSNara
Differential Revision: D26664173
fbshipit-source-id: 9348f7c39eb317cf1e8e5d59e77a378e5e04f3eb
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: D26695275
fbshipit-source-id: 85aae94105a2817d345d25f736386e545dff0a9a
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
Summary:
This diff increases the severity for yoga logs to match all other logs in Fabric
changelog: [internal] internal
Differential Revision: D26315760
fbshipit-source-id: 1de3c23513ad8ce1630e3d0e3576f60608aac7de
Summary:
iOS and Android platform code already explicitly check this invariant: nodes cannot have parents when they're inserted into the View hierarchy.
Check this in the core so we get these checks in unit tests, and earlier in the core before platform code runs.
Changelog: [Internal]
Reviewed By: shergin, mdvacca
Differential Revision: D26331842
fbshipit-source-id: c12bc9066d280cb85ccc9e754c9fa475927e6080
Summary:
This hack was introduced to fix T63560216. See before/after, it no longer repros.
Changelog: [Internal]
Differential Revision: D26306134
fbshipit-source-id: 7e6f886d76f3c54912fbb548069c31faaac08786
Summary:
During earlier testing I didn't fully realize that Android disables a core bit of View Flattening: Views can be concrete or non-concrete; and their children can be flattened or not. None of these properties are mutually exclusive with each other.
Except on Android - that functionality is currently disabled. A View can be either flattened and non-concrete, or non-flat and concrete. So there are some flattening edge-cases hit on iOS but not Android, due to the larger state-space on iOS.
To test, I forced Android to align with iOS and tested; and then tested on iOS; and ensured no mounting errors, assertions, or crashes were hit during some specific tests.
Changelog: [Internal]
Differential Revision: D26298872
fbshipit-source-id: 2f0f78127a7bf057c7cf109005f1dae74f0ff6ba
Summary:
On iOS, log lines are not entirely flushed when an assert is hit right after (or during) this mutate function. Make sure to flush log lines regularly during this function so that debugging is easier.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26291789
fbshipit-source-id: 47af109cdc3dcfc6bf08cbb41db06e9260bfaa08
Summary:
remove dead code; this flag is always defaulted to false, and not used anywhere
Changelog: [Internal]
Reviewed By: sammy-SC, mdvacca
Differential Revision: D26271507
fbshipit-source-id: e2277cc24f164c53f2e8a0aa72456ac400834d70
Summary:
Add LA_ASSERT macro, this just makes debugging easier on Android since these asserts are compiled out for us even in debug.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D26271508
fbshipit-source-id: 9be8c71e273d762a4f31ff1fcc629ce48218b98d
Summary:
These asserts don't run in prod; only set starting/final view props if the result props object is non-null.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26271512
fbshipit-source-id: b495c014a062cf255fd4b5cb8609582f23edcec8
Summary:
In this case where there's an ongoing UPDATE animation and an INSERT of the same node, make sure the ongoing animation type is what we expect, and that the `newChildShadowView` is valid.
We were already guarding against these, but we should (1) crash more in debug and (2) fail more elegantly in prod when the asserts don't run.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26271514
fbshipit-source-id: 48e7d37a2493241f16099d9fe5ecb0d247707ca7
Joshua Gross
Valentin Shergin
Samuel Susla
David Vacca
Andrei Shikov
Igor Klemenski
