Commit Graph
65 Commits
Author SHA1 Message Date
Samuel Susla 74d3559924 Clean up extract_uimanagerbinding_on_demand experiment
Summary:
Changelog: [internal]

Cleanup the experiment.

Reviewed By: mdvacca

Differential Revision: D27995976

fbshipit-source-id: dd6b25f5ad225243765d64b7d92b97f4423005a2
2021-04-28 04:19:24 -07:00
Peter Argany 9729d4dfeb Revert a small change to UIManagerBinding
Summary:
Changelog: [Internal]

This reverts a small change from D26885562 (https://github.com/facebook/react-native/commit/ea1f9531f00b5cd834e03f58cdfa117a93634624). I'm not sure if this change was made intentionally (feel free to reject this diff if it was luluwu).

Before D26885562 (https://github.com/facebook/react-native/commit/ea1f9531f00b5cd834e03f58cdfa117a93634624), if `RN$Bridgeless` was truthy and `RN$stopSurface` was falsey, then the function returned. After D26885562 (https://github.com/facebook/react-native/commit/ea1f9531f00b5cd834e03f58cdfa117a93634624), it ran the else clause (which shouldn't be run in bridgeless mode IIUC).

Reviewed By: mdvacca

Differential Revision: D27751583

fbshipit-source-id: daaf5c72fec11f4b2e21d1d2926a5026b984461f
2021-04-14 10:37:50 -07:00
David Vacca b28ddede7b Transfer 'DisplayMode' value from Native -> JS
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
2021-04-12 00:05:43 -07:00
David Vacca bdb9a1e094 Add displayMode parameter into startSurface and setSurfaceProps methods
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
2021-04-12 00:05:43 -07:00
David Vacca 680dd36fd1 Introduce new Fabric API setSurfaceProps
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
2021-04-12 00:05:43 -07:00
Samuel Susla 3a3b61ce71 Use nested namespaces in UIManager module
Summary:
Changelog: [internal]

Use nested namespaces in UIManager module.

Reviewed By: JoshuaGross

Differential Revision: D27462611

fbshipit-source-id: 626510bf72a18303de55e5db3e13db4f415d799e
2021-04-01 04:50:59 -07:00
Samuel Susla 9ef995a14a Enable C++ 17 in React Native
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
2021-04-01 04:50:59 -07:00
Samuel Susla a038179b9b Add return value to JavaScript calls in UIManagerBinding
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
2021-03-31 14:32:02 -07:00
Valentin Shergin 336876509c Introducing Timeline: A time-travel debugging tool (cross-platform part)
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
2021-03-24 11:30:24 -07:00
Valentin Shergin 3dd92513c5 Fabric: Aligning UIManager::startSurface and UIManager::stopSurface signatures
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
2021-03-23 13:41:47 -07:00
Samuel Susla 34d189ae09 Introducing Leak Checker
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
2021-03-17 02:57:37 -07:00
Joshua Gross 9c1926051e Pass blockNativeResponder param to setIsJSResponder
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
2021-03-15 18:37:36 -07:00
Lulu Wu ea1f9531f0 Add additional checks to prevent throwing native java errors in UIManagerBinding.cpp
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
2021-03-15 03:09:30 -07:00
Valentin Shergin fa64427236 Fabric: Fixed deadlock in Surface Handler
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
2021-03-12 08:57:57 -08:00
Joshua Gross 3b0800a49a EventTarget (native)/InstanceHandle (JS) is not optional, assert that it's non-null
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
2021-03-11 20:49:53 -08:00
Samuel Susla 077e434d77 Do not retain UIManager inside background executor
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
2021-03-07 04:40:26 -08:00
Andrei Shikov ac704690dc Update Android.mk to ensure the rn-tester compiles in debug mode
Summary:
Changelog: [Internal]

Adds react_debug dependency in Android.mk where it was missing

Reviewed By: mdvacca

Differential Revision: D26617400

fbshipit-source-id: 5ac799269b106eadd881d30490ac34bd2134a9b7
2021-02-24 12:29:00 -08:00
Samuel Susla c060f368b9 Check for null in eventTargetFromValue
Summary:
Changelog: [internal]

Check for null before passing it to `EventTarget` constructor.

Reviewed By: JoshuaGross

Differential Revision: D26605779

fbshipit-source-id: a2773c8123d83c25736bccefe656d1def8794091
2021-02-24 06:07:16 -08:00
Samuel Susla f7eecc68cb Nullcheck before dispatching view command
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
2021-02-24 01:59:44 -08:00
Samuel Susla 270060e1d9 Pass weak pointers to background executor lambda
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
2021-02-22 17:02:19 -08:00
Joshua Gross b3930f935f Convert most Fabric Cxx code to use react_native_assert instead of assert
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
2021-02-19 20:52:52 -08:00
Igor Klemenski c37d49492b Work around max path length during source build of RN Android (#30776)
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:
![image](https://user-images.githubusercontent.com/12816515/105435667-a1e15d00-5c12-11eb-9fcd-d0c278aaf477.png)
Now:
![image](https://user-images.githubusercontent.com/12816515/105435741-c2a9b280-5c12-11eb-88d5-a69ae56bbf50.png)

Differential Revision: D26194286

Pulled By: mdvacca

fbshipit-source-id: 778b5a0515148e2ace19e7902f74301831ebed94
2021-02-11 12:32:33 -08:00
Ramanpreet Nara ab21226457 Migrate away from "@fbsource//tools/build_defs/apple:flag_defs.bzl"
Summary:
We still have usages of "fbsource//tools/build_defs/apple:flag_defs.bzl" in react-native-github. But this should get us closer towards not using the fbsource cell. Hopefully, this is enough to unbreak the  test_docker CircleCI build.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D26289304

fbshipit-source-id: 1c6464bb84df4f82f8a797321a73a1ed324e319a
2021-02-08 16:04:51 -08:00
Valentin Shergin fc24bb4af0 Fabric: Support for setIsJSResponder, all the native changes
Summary:
This is a Fabric-compliant implementation of `JSResponder` feature. To make it work e2e we also need to update FabricRenderer in React repository. But before we can do this, we need to ship the native changes.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: mdvacca

Differential Revision: D24630027

fbshipit-source-id: 70c30e1250b554d83862956b536714704093072f
2021-02-04 13:16:29 -08:00
Valentin Shergin 7a290d1596 Fabric: Introducing SurfaceHandler
Summary:
This implements `SurfaceHandler`, a new primitive for controlling the Surface life-cycle that ensures ownership, preserves state, maintains internal invariants, and simplifies surface manipulation from the application side.

For now, all this is an unused code. The coming diff will introduce an experiment that will route all surfaceId-based APIs to SurfaceHandler-based ones.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D24290777

fbshipit-source-id: f2ff2b58e6d46e971a548f9f02113a1c783c4940
2021-01-29 07:54:19 -08:00
Samuel Susla 6b1a899ef7 BackgroundExecutor: rename shouldCancel to shouldYield
Summary:
Changelog: [internal]

Name `shouldCancel` is misleading. It implies the commit is cancelled and doesn't happen.
`shouldYield` expresses the intent better, because the commit's changes do eventually reach mounting layer but the current commit is rather yielding to the next one.

Reviewed By: mdvacca

Differential Revision: D26126121

fbshipit-source-id: 18988f217cbc651f0010f6e2381682bdbaed8bd4
2021-01-28 12:40:12 -08:00
Samuel Susla f92cc5ba77 Background Executor: only cancel commit if it is for the same surface
Summary:
Changelog: [internal]

In theory, a commit from different surface could cancel a commit for unrelated surface. Adding surfaceId to cancellation logic assures this doesn't happen.

Reviewed By: shergin

Differential Revision: D26126003

fbshipit-source-id: 15b9b73f1000a2b4ae882e0a5129fe26fbb53fd2
2021-01-28 12:40:11 -08:00
Samuel Susla d5a1bbe64a Background Executor: Move complete root event counting to static variable
Summary:
Changelog: [internal]

This simplifies logic, there is no need to keep the counter as an ivar of UIManager.

Reviewed By: JoshuaGross, shergin

Differential Revision: D26125928

fbshipit-source-id: bd266586463a9f9b85d6dc189cdab19f79e3d107
2021-01-28 12:40:11 -08:00
Valentin Shergin a0ab688216 Fabric: Using UIManagerBinding::getBinding() to access the binding on demand
Summary:
This changes how we access UIManagerBinding in Fabric code. Instead of storing a pointer to it and accessing it, now we just request it from JavaScript runtime on demand. After a week of testing, I hope we will be able to simplify retaining and referencing logic between UIManager and UIManagerBinding. The change is gated just in case to have the ability to turn this new implementation off.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D26082788

fbshipit-source-id: 211c551fe1aaba9b0bff18478e40e1e1f32e999c
2021-01-26 14:58:23 -08:00
Valentin Shergin 9e16ac93ed Fabric: Simplifications in UIManagerBinding::completeRoot
Summary:
This is a small simplification of the logic we have in UIManagerBinding choosing the way to call `completeRoot`. It removes an additional check for for `sharedUIManager->backgroundExecutor_` not being empty. We don't need it because it never changes and we already have the same check several lines above.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D25953674

fbshipit-source-id: 683b3e51a792160d480551a65c40492ce28938e4
2021-01-25 21:57:58 -08:00
Andres Suarez 0f4f917663 Apply clang-format update fixes
Reviewed By: igorsugak

Differential Revision: D25861683

fbshipit-source-id: 616afca13ae64c76421053ce49286035e0687e36
2021-01-09 22:11:00 -08:00
Joshua Gross c15f907f67 Core native implementation of sendAccessibilityEvent in Fabric
Summary:
This is the core (Cxx) Fabric implementation of sendAccessibilityEvent. No platform support is added in this diff (Android and iOS do not build yet).

See following diffs.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D25852412

fbshipit-source-id: 88b7d8e2e251183dfe7fb377a4cffbac8f458656
2021-01-08 18:10:59 -08:00
Andrew Coates 47bb3be6ce Minor code tweaks to work around a compiler bug in MSVC. (#30696)
Summary:
While pulling in additional react-native code into react-native-windows to support fabric, I hit some bugs with the MSVC compiler.  These minor changes work around those bugs and shouldn't have any functional effect on the code

## Changelog

[Internal] [Fixed] - Minor code tweaks to work around a compiler bug in MSVC.

Pull Request resolved: https://github.com/facebook/react-native/pull/30696

Test Plan: Code compiles in MSVC and clang.

Reviewed By: shergin

Differential Revision: D25830710

Pulled By: appden

fbshipit-source-id: 796bd8191ea62fe62e09dd16cc56b8edc092297f
2021-01-08 13:33:37 -08:00
Valentin Shergin f379b1e583 Fabric: Shipping updateStateWithAutorepeat as the only way to update a state
Summary:
This replaces the internal core implementation of `setState` with the new `updateStateWithAutorepeat` which is now the only option.
In short, `updateStateWithAutorepeat` works as `setState` with the following features:
* The state update might be performed several times until it succeeds.
* The callback is being called on every retry with actual previous data provided (can be different on every call).
* In case of a static value is provided (simple case, not lambda, the only case on Android for now), the same *new*/provided value will be used for all state updates. In this case, the state update cannot fail.
* If a callback is provided, the update operation can be canceled via returning `nullptr` from the callback.

This diff removes all mentions of the previous state update approach from the core; some other leftovers will be removed separatly.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D25695600

fbshipit-source-id: 14b3d4bad7ee69e024a9b0b9fc018f7d58bf060c
2020-12-23 21:49:44 -08:00
Samuel Susla da10179238 Pass x and y to measure callback
Summary:
Changelog: [internal]

Previous implementation of "measure" in UIManagerBinding returned 0, 0 for x and y coordinates.

Reviewed By: shergin

Differential Revision: D25681586

fbshipit-source-id: fa69b6d4803f083a8299e00cae8bb59932c8bf78
2020-12-23 04:45:55 -08:00
Samuel Susla a1e9a2d81e Avoid for loop in UIManager::getNewestCloneOfShadowNode
Summary:
Changelog: [internal]

The loop inside `UIManager::getNewestCloneOfShadowNode` is unnecessary as ancestorList already has index of the children.

Reviewed By: shergin

Differential Revision: D25681296

fbshipit-source-id: e8fab6c6d945064b0a7e63b6fd22697054fb7ea2
2020-12-23 04:45:54 -08:00
Valentin Shergin fb4e06b1a5 Fabric: Introducing UIManagerCommitHook, unified way to alter commits
Summary:
The new UIManagerCommitHook can be used to implement commit-atering features without modifying the core.
E.g. State Reconciation seems can be implemented as a coomit hook but first we will use it for a new feature called Timeline (see the coming diffs).

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D25221312

fbshipit-source-id: dbe41b475bc8b36e1780d81447ab43b32758bdff
2020-12-18 16:01:30 -08:00
Valentin Shergin d2ae775bf7 Fabric: Introducing ShadowTreeDelegate::shadowTreeWillCommit()
Summary:
With the change, a new delegate method allows a receiver to alter a new (proposed) shadow tree with another tree by returning the altered tree.
We will it use in future diffs to implement Commit Hooks.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D25221313

fbshipit-source-id: 9f83577d862b713fff71fa365ce660cc1de87c84
2020-12-18 16:01:30 -08:00
Emily Janzer 81096901a8 Use bridgeless flag instead of specific binding in UIManagerBinding
Summary:
This fix is still a little hypothetical. We have a few different JS errors that we're seeing with bridgeless mode that seem to be caused by Fabric trying to access `__fbBatchedBridge` from C++. I think what's happening is:

1. User encounters an unrelated JS error very early in rendering a new surface (possibly while the bundle is still loading?)
2. In release builds, BridgelessReactFragment handles the error by stopping the surface and rendering a retry button (actually, the surface is stopped in a bunch of places in BaseFbReactFragment, which might be why this is popping up now - I recently refactored that class to share more of its logic in bridgeless mode)
3. Fabric stops the surface by first checking to see if the custom binding `RN$stopSurface` exists; if not, it falls back to calling the registered callable module `ReactFabric`.

I think #3 is where things are going wrong for bridgeless mode; if you call stopSurface before `RN$stopSurface` is installed (which happens when ReactFabric shim is required) then you'll fall back to the bridge version.

My solution here is to instead rely on a flag set in C++ to determine whether we're in bridgeless mode, and then check to see if the stopSurface binding has been installed. If not, we just noop - if the ReactFabric shim hasn't been required, we probably don't actually have a React surface that needs to be stopped.

At least, that's my current theory. We'll see if this actually works.

Changelog: [Fixed][iOS] Fix an issue calling stopSurface in bridgeless mode before surface is started

Reviewed By: mdvacca

Differential Revision: D25453696

fbshipit-source-id: bff76675c43989101d0ba5ae0aba60089db230bf
2020-12-10 17:17:23 -08:00
Samuel Susla 6b16f2e7a7 Add throttling mechanism to background executor
Summary:
Changelog: [internal]

Background executor performs unnecessary operations when second `completeRoot` message from React arrives before first `completeRoot` was finished. This produces unnecessary `ShadowViewMutations`.

Mechanism:
Everytime `completeRoot` is received, before the call is dispatched on the background queue, `completeRootEventCounter_DO_NOT_USE_` is incremented.
Inside `ShadowTree::tryCommit` we check if the value has been incremented to determine if another `completeRoot` is queued.

Reviewed By: JoshuaGross

Differential Revision: D24419160

fbshipit-source-id: 11e19026feca01db6c8981b093a691a6b58a006f
2020-10-26 05:04:53 -07:00
Samuel Susla 668cc2fbde Remove setNativeProps from core
Summary:
Changelog: [internal]

Fabric uses view commands instead of setNativeProps. This diff removes what's left of setNativeProps from the core.

Reviewed By: JoshuaGross

Differential Revision: D24309999

fbshipit-source-id: 70e54f0a984f8c36f77ba2cd59f59fc6923bc832
2020-10-15 10:41:08 -07:00
generatedunixname89002005287564 3f85b83653 Daily arc lint --take CLANGFORMAT
Reviewed By: zertosh

Differential Revision: D23811656

fbshipit-source-id: 4104948d2e4db786998320bcfdb1598d4a497f2d
2020-09-21 04:38:18 -07:00
Valentin Shergin 57dd48b246 Fabric: Marking all JS function lambdas noexcept in UIManagerBinding
Summary:
Exceptions in C++ work quite differently from exceptions in other languages. To make exceptions actually work **correctly** all the code needs to be written with "exceptions in mind" (e.g., see https://www.stroustrup.com/except.pdf). In short, if the code is not "exceptions ready", throwing an exception causes memory leaks, dangling pointers, and invariant violations all over the place, which will probably cause another crashes down the road (which will be especially hard to investigate and attribute to the original issue).
Fabric Core (Layout, Props parsing, ShadowNodes management, and so on) does not use exceptions because in most (all?) the cases the exception is now recoverable. So, if a program detects some internal state invariant violation or missing some resource, *logically* it's fatal. We also don't want to pay code-size and performance tax for exception support, so that's why we don't use them. It's just not the right fit for Fabric Core.

This does not mean that exceptions don't happen though. C++ standard library can throw them... sometimes. And if our library is compiled with exceptions enabled (still the case, unfortunately), an exception can bubble to JavaScript code and losing all context down the road. And it's hard to investigate such crashes. To isolate those occasional exceptions inside C++ core we are marking all C++/JS boundaries with `noexcept` that stops the bubbling.

I hope that will give us much more informative crash reports.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D23787492

fbshipit-source-id: 0822dbf36fc680c15b02b5cd0f2d87328296b642
2020-09-18 17:07:29 -07:00
Valentin Shergin 1ddc6542c7 Fabric: Removing catching all exceptions in UIManager::completeRoot
Summary:
This is a revert of D23529233 (https://github.com/facebook/react-native/commit/902611f14837752353e919dd1740812ec7260fb8).
It turns out it was a bad idea. With this catch-all thing, we don't get new information. Yeah, we crash earlier now but seems we have even less information about the crash. :(

I think D23754284 (https://github.com/facebook/react-native/commit/04c874bd9c6b15274fd87acf10cb3533b2eabc0d) should fix the issue.

Changelog: [Internal] Fabric-specific internal change.

Original commit changeset: 7ac7fb26ac08

Reviewed By: sammy-SC

Differential Revision: D23786086

fbshipit-source-id: 86784df0193abfb7328c4d5a16a9af4214e9a161
2020-09-18 17:07:29 -07:00
Valentin Shergin c453dbc4cc Fabric: Enabling state auto-repeating for all state updates (gated)
Summary:
This enables a new state auto repeating mechanism built-in mechanism for all state updates which we already use for CK interop. This experiment is supposed to help with T74769670 and co.

This change is gated with MC.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D23762508

fbshipit-source-id: f535513c724ace9ede570177281324eb507329c5
2020-09-17 14:47:12 -07:00
Joshua Gross ee38751975 Layout Events: throttle layout events sent to same node repeatedly
Summary:
Under Fabric only, we can enter an infinite layout loop where the emitted layout event oscillates between two height values that are off by a very small amount.

The cause is, in part, components that use layoutEvents to determine their dimensions: for example, using onLayout event "height" parameters to determine the height of a child. If the onLayout height changes rapidly, the child's height will change, causing another layout, ad infinitum.

This might seem like an extreme case but there are product use-cases where this is done in non-Fabric and layout stabilizes quickly. In Fabric, currently it may never stabilize.

Part of this is due to a longstanding issue that exists in Fabric and non-Fabric, that we cannot immediately fix: If in a single frame, C++ emits 100 layout events to ReactJS, ReactJS may only process 50 before committing the root. That will trigger more layout events, even though product code has only partially processed the layout events. At the next frame, the next 50 events will be processed which may again change the layout, emitting more events... etc. In most cases the tree will converge and layout values will stabilize, but in extreme cases in Fabric, it might not.

Part of this is because Fabric does not drop *stale* layout events. If 10 layout events are dispatched to the same node, it will process all 10 events in older. Non-Fabric does not have this behavior, so we're changing Fabric to drop stale events when they queue up.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D23719494

fbshipit-source-id: e44a3b3e40585b59680299db3a4efdc63cdf0de8
2020-09-17 13:20:23 -07:00
Valentin Shergin 13bc3c87ef Fabric: Removing shared_ptr from ShadowTreeCommitTransaction's argument
Summary:
We don't need a shared_ptr here and without it the code will be faster and simpler.
This change is aligned with any clone-line callbacks we have in the Core which accepts a `const &` and return `shared_ptr<>`.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross, sammy-SC

Differential Revision: D23725687

fbshipit-source-id: 1cd959f4273913175d342302e2f12752f0114768
2020-09-16 23:56:00 -07:00
Valentin Shergin 0118cbf1d1 Fabric: Introducing ShadowTree::getCurrentRevision()
Summary:
Previously, to get a current root shadow node for a shadow tree, we called `tryCommit` method and stole a pointer from this. That was not a very straightforward method to get things done, and most importantly we need to do this to change the shape of the ShadowTreeCommitTransaction signature (remove a shared pointer from the callback) to make it simpler, faster and allow future improvements (see the next diff).

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross, sammy-SC

Differential Revision: D23725689

fbshipit-source-id: 51950b843a0e401828b6c6a38e5e2aaaf21ec166
2020-09-16 23:56:00 -07:00
Valentin Shergin a8b090b128 Fabric: Another attempt to deal with failing state updates
Summary:
This is *another* attempt to solve a failed state update problem.

Unfortunately, some applications are inherently not compatible with the "let's recommit the update on the application side in case it failed" approach. The problem is that if we call `updateState` on the application side, we miss the original event window. E.g. if we need to deliver some state update with AsyncBatched priority and if the update fails, we lose the opportunity to commit it on time. These issues can be critical for some complex use-cases as ComponentKit interop.

This diff adds implementation for `updateState` that does the work a bit differently. For all failed state updates it tres to recommit them asap using `ShadowTree::commit` and calling lambda on every attempt. With this approach the update might fail in two cases:
The node disappeared from the tree, so there is no way to update it.
The lambda returned `nullptr` indicating that the update is no longer needed.

We need this for the ComponentKit interoperability layer that is very sensitive for missing state updates.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D23603958

fbshipit-source-id: a3b8c09fb2f1c8302583aa5880b48fc0840224e3
2020-09-11 09:22:08 -07:00
Valentin Shergin c19b3ffae9 Fabric: Communicating a reason why a commit was unsuccessful via ShadowTree::CommitStatus
Summary:
We need it to stop repeating to commit new shadow tree in `ShadowTree::commit` when a transaction cancels the commit.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross, sammy-SC

Differential Revision: D23603959

fbshipit-source-id: d279fb3bf4190e860740a6450595d6f2fc3117f7
2020-09-11 09:22:08 -07:00