Summary:
Renaming the `better` utilities to `butter`:
- to prevent claims that this library is superior to others - it really depends on use cases
- to indicate ease of use throughout the codebase, easily spread like butter
Changelog: [C++][Changed] Renaming C++ better util to butter, used by Fabric internals
Reviewed By: JoshuaGross
Differential Revision: D33242764
fbshipit-source-id: 26dc95d9597c61ce8e66708e44ed545e0fc5cff5
Summary:
changelog: [internal]
Just moving code that doesn't belong to UIManagerBinding out of the class.
Reviewed By: philIip
Differential Revision: D33060412
fbshipit-source-id: 2d54929072cef14fd1fa6b70bde382ae21ecff45
Summary:
Applies suggestions from default preset that make sense in our codebase
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33010231
fbshipit-source-id: 6bc9edf01fd9bd9938d211e3494dd1a127b24eaa
Summary:
changelog: [internal]
Provide `UIManager` to `UIManagerBinding` in constructor to make the API safer.
Reviewed By: philIip
Differential Revision: D32668892
fbshipit-source-id: a15cd295196a60c3f46997e59c05c4f90503e18d
Summary:
The IDE warning suggests that passing folly::dynamic by value will create a copy on each call.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D32978154
fbshipit-source-id: a47a60c332a9d299eb2110d3537dfab0bc2398b6
Summary:
changelog: [internal]
Exposes a new flag on RuntimeScheduler: `unstable_getIsSynchronous`. Flag indicates if the current code is run synchronously and therefore commit phase should be synchronous.
Unit tests will be added later, to keep this diff short. This code path is not executed yet.
Reviewed By: mdvacca, ShikaSD
Differential Revision: D32677814
fbshipit-source-id: e01d4fff7e716d627ff99fe104965851138c3aef
Summary:
changelog: [internal]
Background executor has been shipped on both platforms for a long time.
I've kept the flag around because I wanted to run tests and compare Concurrent Mode vs Background Executor. The intention was to see if we can get rid of Background Executor to simplify the threading model.
Since then, React team has moved away from Concurrent Mode towards more gradual rollout of concurrent rendering and it no longer makes sense to do this comparison. Right now, we don't have a concern with concurrent rendering and Background Executor. If we ever want to run the an experiment, this gating will need to be added again.
Reviewed By: javache
Differential Revision: D32674798
fbshipit-source-id: a1e51c9c5b8e48efa4cb0f25379d58e7eb80ccd9
Summary:
changelog: [internal]
UIManager::animationTick is thread safe. Let's make it obvious by marking the method const.
Reviewed By: javache
Differential Revision: D32669102
fbshipit-source-id: 49e35d0f0a5c5d1b03baa1cbf9cdece082909e85
Summary:
Changelog: [internal]
Nothing was using `stop` parameter, let's get rid of it.
Reviewed By: philIip
Differential Revision: D32669018
fbshipit-source-id: dc2d52048a2f7dd3785dd959270087001c778962
Summary:
We have `LOCAL_SHARED_LIBRARIES` that are getting longer and are
making reviewing them on Diffs quite hard.
Having all the list of the dependency on a single line is suboptimal
and it makes hard to find duplicated entries.
I've updated the longest `LOCAL_SHARED_LIBRARIES` to be multilines and
I've sorted the entries here.
Changelog:
[Internal] [Changed] - LOCAL_SHARED_LIBRARIES
Reviewed By: ShikaSD
Differential Revision: D32695127
fbshipit-source-id: f5b381c501ddff083ef9f4baaca6c4c8c9523368
Summary:
Nearly all of these are identical and these compiler_flags are now centralized in rn_defs.bzl. This should have NO CHANGE on build configuration, the flags have just moved for now.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D31631766
fbshipit-source-id: be40ebeb70ae52b7ded07ca08c4a29f10a0ed925
Summary:
changelog: [internal]
This is a pre-condition to get rid of `shared_ptr` from `EventEmitterWrapper`. Also saves us a few copies of shared_ptr, this is negligible though.
Reviewed By: mdvacca
Differential Revision: D31307048
fbshipit-source-id: b84654bed2359b66faf3995795e135e88fe51cb6
Summary:
Unfortunately, parsing some props requires stateful context - namely, PlatformColor on Android. We explored several different options but they all seemed inferior to the approach of using ContextContainer, and most would require using global state.
By introducing this change everywhere as early as possible, we can avoid later pain. It is likely that some prop, on some platform, will require this mechanism. We'll be ready for it!
Because we can pass a constref of the ContextContainer through to all props and because the context and context data is never retained by prop parsers, perf and memory hit should be ~0.
This diff contains core changes only. Leaf changes to all props structs and conversions files will be in next diff(s).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D29838789
fbshipit-source-id: f5090e7f02eb6e8fbe0ef4dd201e7d12104a3e3c
Summary:
Changelog: [internal]
Original commit changeset: 390e773283a7
Original diff didn't resolve crash T82119127. Let's back this out and investigate again.
Reviewed By: mdvacca
Differential Revision: D29909878
fbshipit-source-id: 54b054f4a5c97b8f6369f8790c2cf32a56c4fbd2
Summary:
This diff exposes a new API in FabricUIManager called getInspectorDataForInstance. The goal of this method is to return React metadata for a Native view.
This data will be used from partner teams to build tools that uses React metadata in RN
Data returned from React: P429701924
changelog: [internal] internal
allow-large-files
Reviewed By: JoshuaGross
Differential Revision: D29747864
fbshipit-source-id: 8cb55573be08cb530f7e3c83eed8b4fcf43e7781
Summary:
I'm hunting down the source of a perf regression on a screen and think that having these systrace sections could be handy for this and future investigations.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D29802969
fbshipit-source-id: f4030261da8888ddeb32ae41b9cf2b25af6a5583
Summary:
#import is common in Objective C, but is a vendor-specific extension of C++, which breaks MSVC compat. Fix an #import that made its way into C++ code.
## 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
-->
[Internal] [Fixed] - Replace #import in C++ Header with #include
Pull Request resolved: https://github.com/facebook/react-native/pull/31805
Reviewed By: sammy-SC
Differential Revision: D29553429
Pulled By: javache
fbshipit-source-id: d609981f0c6a756eafb58d50e98975625d08de21
Summary:
Changelog: [internal]
[This assumption](https://fburl.com/diffusion/yeqtvxru) in `~Scheduler` might not be completely correct. There can be a race between JS thread and main thread. With main thread settings animationDelegate_ to nullptr and JS thread reading the value. Even though they always happen one after the other, JS thread can have the value of animationDelegate_ in its cache line and changing it from main thread might not invalidate it right away.
Reviewed By: JoshuaGross
Differential Revision: D29237290
fbshipit-source-id: 6cb898caf7c225cf8162e7560921b563dec514b1
Summary:
Changelog: [internal]
[This assumption](https://fburl.com/diffusion/yeqtvxru) in `~Scheduler` might not be completely correct. There can be a race between JS thread and main thread. With main thread settings delegate_ to nullptr and JS thread reading the value. Even though they always happen one after the other, JS thread can have the value of delegate_ in its cache line and changing it from main thread might not invalidate it right away.
Reviewed By: mdvacca
Differential Revision: D29233092
fbshipit-source-id: 390e773283a745742d78152b9921c9ce21148a4c
Summary:
In the refactor in D28933824 (https://github.com/facebook/react-native/commit/6b601db8b83337415113a123d780fb64aa67af42), an early return was accidentally deleted. This causes incorrect measurements to be returned, which can break a few things including Text Inline Views.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D29122235
fbshipit-source-id: 76c8991132e22cd4e0fc1c277447c4dba751adfb
Summary:
PreAllocation currently always happens at revision 0 (after ShadowNode creation), and all CREATE mutations are triggered for ShadowNodes at revision 1 or higher (since CREATE mutations are generated by the differ, it means that all ShadowNodes have revision 1 or higher when CompleteRoot is called). This means that between PreAllocation and CREATE, we /always/ expect at least one clone.
It is possible for a node to be "non-view-forming" at revision 0, causing view preallocation to be skipped, and "view-forming" at revision 1 (causing the CREATE mutation to be thrown away, since all CREATE mutations of revision 0 or 1 are thrown away). This causes a crash. It is extremely marginal, but there are repros in the wild.
Thus, I'm introducing one new UIManager and Scheduler delegate method that allows the mounting layer to be notified of clones. If a clone from rev 0->1 results in a node going from non-view-forming to view-forming, we can preallocate then, as well.
This resolves this crash, and allows us to keep experimenting safely with this View PreAllocation optimization. I believe all edge-cases are accounted for.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D29043426
fbshipit-source-id: dff11d3140ded1cbb02a5518a3aeb52dc812cc50
Summary:
Changelog: [internal]
Add an option to make all measure calls asynchronous.
Reviewed By: JoshuaGross
Differential Revision: D28934444
fbshipit-source-id: 57a320b03add0182b4646b13ed4b692b899ddea3
Summary:
Changelog: [internal]
Move call of getRelativeLayoutMetrics to lambda.
The reason for this is so we can make all of "measure" calls asynchronous.
Reviewed By: JoshuaGross
Differential Revision: D28933824
fbshipit-source-id: f77fc1d06d5cd4706d52a02253a3e754e8e8fa44
Summary:
Changelog: [internal]
Use getter function instead of accessing ivar. This makes for clearer APIs.
React part is implemented in https://github.com/facebook/react/pull/21553
Reviewed By: JoshuaGross
Differential Revision: D28675372
fbshipit-source-id: cf99f8482067bfc0fd57d39fda9656abd665bb69
Summary:
Changelog: [internal]
Extend LeakChecker so it is available on Android (or any other platform as it is completely in C++ now).
Reviewed By: JoshuaGross
Differential Revision: D28600243
fbshipit-source-id: c77a003e3ffc6171e61c998508c9f34f10bb65ca
Summary:
This diff adds a new variable called "DisplayMode" into SurfaceHandler.cpp and FacebookAppRouteHandler.js. The purpose of DisplayMode is for the native pre-render system to notify React that the a surface is either being "pre-rendered" or "rendered"
When the surface is being "pre-rendered" (displayMode == "SUSPENDED"), react will create and commit React Trees, but it will not execute use-effect callbacks
When the surface is being "rendered" (displayMode == "VISIBLE"), react will create and commit React Trees and it will not execute all use-effect callbacks that weren't executed during "pre-rendering"
By default surfaces are going to be rendered with displayMode == "VISIBLE".
This diff should not create any change of behavior for now, this is the infra required to integrate the new offScreen API the react team is working on for pre-rendering system
changelog: [internal] internal
Reviewed By: yungsters
Differential Revision: D27614664
fbshipit-source-id: f1f42fdf174c2ffa74174feb1873f1d5d46e7a95
Summary:
This diff extends startSurface and setSurfaceProps methods with the new parameter called displayMode
changelog: [internal] internal
Reviewed By: yungsters
Differential Revision: D27669847
fbshipit-source-id: c2ddb690ca897e46e00f07b491b91bb2bc8e847d
Summary:
This diff introduces a new fabric API called setSurfaceProps that will be used to call the new JS api "setSurfaceProps"
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27607588
fbshipit-source-id: 36a19f728af244d7e72687d9305b1c568e2b8ec6
Summary:
Changelog: [Internal] enable support for C++ 17.
C++ 17 in React Native targets.
Short and comprehensive list of C++ features:
https://github.com/AnthonyCalandra/modern-cpp-features#c17-language-features
Reviewed By: JoshuaGross
Differential Revision: D27431145
fbshipit-source-id: e8da6fe9d70e9b7343a8caec21cdbeb043478575
Summary:
Changelog: [internal]
Add support for a return value when calling JavaScript functions in UIManagerBinding.
Reviewed By: mdvacca
Differential Revision: D27470817
fbshipit-source-id: 38de92dd913af61d879f1cc5962d417f51da73b0
Summary:
This is a core part of the Timeline feature (aka Time Travel Debugger). With these new primitives, any external library can initiate "saving" all the previous interface changes (commits) and unwind to any previous one (in order to introspect and validate visual side-effects).
The next diff in the stack will implement UI for this feature integrated into Debug menu on iOS.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25926660
fbshipit-source-id: 2e5f6892351d3053db8f64c1cf6ff445b0867ad7
Summary:
In the previous diff, we moved the destruction of a ShadowTree object from `UIManager::stopSurface` method. And the next logical step is to align `UIManager::startSurface` method with the symmetrical approach. Now `stopSurface` accepts a unique pointer to a ShadowTree and `stopSurface` returns it back to the caller; this way we can avoid returning a raw pointer from `startSurface`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D27010533
fbshipit-source-id: b9211fb7e67763cc190d6b8c86cb866d20c6693d
Summary:
Changelog: [internal]
Introducing LeakChecker. A tool that checks if all native components have been cleaned up when surface is stopped.
**Known shortcomings**:
- LeakChecker is only enabled in debug builds and the existence of leaks is logged to console.
- For now, Leak Checker looks at N-1 screen. This is intentional as there is a known limitation of React that doesn't free up all shadow nodes when surface is stopped. Because of this, the use of LeakChecker is not intuitive and I'll work with React team to try to work around this.
- It doesn't help locating the leak, it only informs that leak is present. I'll be looking into ways to help locate the leak.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D26727461
fbshipit-source-id: 8350190b99f24642f8e15a3c2e1d79cfaa810d3d
Summary:
The non-Fabric API has a `blockNativeResponder` param in setJSResponder. Make sure to pass that along in Fabric.
On Android this allows us to respect the flag and do the same thing non-Fabric was doing if `blockNativeResponder` is false. It's not clear yet what the impact is on iOS.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27058806
fbshipit-source-id: aa5074fa46191d78f5292a93d9040ab4bb58ca66
Summary:
Right now there are places in UIManagerBinding.cpp where native java exceptions will be thrown if calling JSI functions results in errors, such as:
```Trying to report error getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
Error: getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
```
https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/358181062b47b9561e60427bbb3816a9
In this diff I added LOG(ERROR) and checks because:
1, Throwing errors neither prevents the JSI errors nor handles them properly, checks prevent JSI errors from happening.
2, Errors are aggregated in Logview with other JSI errors as "Error: android_crash:com.facebook.react.devsupport.JSException:facebook::jni::throwNewJavaException" which keeps the SLA task open forever, checks can prevent JSI errors so they won't lead to exceptions, and LOG(ERROR) will make sure we have enough info for debugging.
Changelog:
[General][Changed] - Add checks and logs to for better error handling
Reviewed By: JoshuaGross
Differential Revision: D26885562
fbshipit-source-id: c0c1c057342e9efc0ff708188703f4332036e7a9
Summary:
As part of stoping a Surface, we have to commit an empty shadow node tree. And to avoid a deadlock we have to do it not under a `linkMutex_`. To do so, we need to move the part that commits an empty tree from UIManager to SurfaceHandler. And this diff does exactly this.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D27010535
fbshipit-source-id: 60cc79b1c81d7340b550e78653737d2cc5bec24d
Summary:
When EventTarget is created in createNode, it's currently treated as optional in native, but it's not optional in JS. We should assert that it's non-null to make the contract more clear.
For now, we keep it as an assert so we catch issues in debug/dev, but it won't impact prod.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27002506
fbshipit-source-id: c541dc02d3de22df19292deaef1a3b68fe57ba1f
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]
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
Kevin Gozali
Samuel Susla
Andrei Shikov
Nicola Corti
Joshua Gross
David Vacca
Nick Gerleman
Peter Argany
Valentin Shergin
Lulu Wu