Commit Graph
26 Commits
Author SHA1 Message Date
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
Valentin Shergin 902611f148 Fabric: Catching exceptions in UIManager::completeRoot earlier, in C++
Summary:
Fabric Core does not support exception safety, so technically exceptions are unrecoverable and binaries should be built without exception support.
While it's not the case, some exceptions might bubble to JavaScript realm and be caught as JavaScript exceptions. In this case, we lose the stack trace. To prevent that and to investigate one particular error we have we add this try-catch block in this diff.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: mdvacca

Differential Revision: D23529233

fbshipit-source-id: 7ac7fb26ac08ad26af8790172de471ac178c3a37
2020-09-04 22:53:40 -07:00
Joshua Gross fe7ff13fcf LayoutAnimations: stopSurface
Summary:
Implementing stopSurface to stop ongoing animations.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D23382974

fbshipit-source-id: 478e0b1ad443ceeb771b03bd1689ec2bdbe02979
2020-08-27 19:37:06 -07:00
David Vacca 25e0a15b77 Delete activityindicator module
Summary:
This diff deletes the activityindicator buck module because these files are created by the code gen.

changelog: [internal]

Reviewed By: PeteTheHeat

Differential Revision: D23227860

fbshipit-source-id: 133315a44c9181be3263e0c6227884ed15487c1f
2020-08-26 12:33:31 -07:00
Valentin Shergin f4fa79a2e3 Fabric: Fixed dangling pointer issue in UIManager::getRelativeLayoutMetrics
Summary:
The same as D21464834 (https://github.com/facebook/react-native/commit/604402678bb0c0c1dc8e55a07da86dee0a53da23).
It should help with T71784916.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D23297325

fbshipit-source-id: 6f14f9b1d1e7d251e53819207bac26dde5afe020
2020-08-24 12:41:44 -07:00
Lulu Wu bdbb040382 Fix Logbox Fullscreen View
Summary:
Logbox inside bridgeless surfaces was crashing and not able to open, this diff should fix it.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D23043773

fbshipit-source-id: 6e584f97e53e626b9bfedd6dc997ba6ba8c08b8f
2020-08-18 12:29:45 -07:00
Samuel Susla 4f94d7da81 Back out "Use ShadowTree::commit if failureCallback is nullptr"
Summary:
Changelog: [internal]

Original commit changeset: 5cb78a3a75a7

In D23151218 (https://github.com/facebook/react-native/commit/dffec8bc7bee4d05b72d19b4efe4aeab980f4cd5) we switched from `ShadowTree::tryCommit` to `ShadowTree::commit` inside `UIManager::updateState`.
It fixes update state being dropped but can cause an infinite loop inside `ShadowTree::commit` because the shadow node that triggered `UIManager::updateState` can been removed before the `updateState` call is dispatched..

Reviewed By: JoshuaGross

Differential Revision: D23155228

fbshipit-source-id: f3339a4e4798880972366d6f894c14a58be1b9b2
2020-08-16 10:33:11 -07:00
Samuel Susla dffec8bc7b Use ShadowTree::commit if failureCallback is nullptr
Summary:
Changelog: [internal]

In D22940187 (https://github.com/facebook/react-native/commit/774dec1e17f6f250172a6d4d944121b82fa36efb) we introduced a mechanism to retry failed state updates from view layer.
The mechanism fixes an issue where state update is occasionally dropped with background executor enabled.

# Why is state dropped?

The state is dropped because with background executor enabled, it is possible to enter `ShadowTree::tryCommit` for the same tree from 2 different threads at once. One of thread changes `_rootShadowNode` causing the other commit to fail.

The code goes like this:

```
{
    Lock mutex
    grab reference to `rootShadowNode_` and call it `oldRootShadowNode`
    Unlock mutex
}

State reconciliation
Trigger layout
`rootShadowNode_` is untouched in this section.

{
    Lock mutex
    Check if `oldRootShadowNode` is equal to `rootShadowNode_`, if not, return false signalling failure. Now this is what happens when
    state update fails.

..... not relevant

    Unlock mutex
}

..... not relevant
```

However, in D22940187 (https://github.com/facebook/react-native/commit/774dec1e17f6f250172a6d4d944121b82fa36efb), we have taken another path. Instead of retrying to commit transaction, client is informed about the failure and it is left up to them to retry. This is correct and works. But I think it is unnecessary to this retry can be done inside UIManager::updateState.

In this diff I call `ShadowTree::commit` (version with retries) in case no `stateUpdate.failureCallback` is provided.
This makes sure that we do retry if state update fails but if Android implements `stateUpdate.failureCallback`, it is left up to view layer to retry.

Eventually we might decide to converge these two approaches.

Reviewed By: shergin

Differential Revision: D23151218

fbshipit-source-id: 5cb78a3a75a754429a8e33bd7736e683e9ed34d4
2020-08-16 03:49:45 -07:00
Joshua Gross 77accf2380 Log more diagnostics when JSI values cannot be cast to Object
Summary:
There are a few places where we cast JSI values to objects without much validation and without proper error logging, and in some places the crashes aren't symbolicated well. To make debugging easier in the short-term, I'm adding some additional logs.

Changelog: [Internal]

Reviewed By: mdvacca, RSNara

Differential Revision: D23033222

fbshipit-source-id: 9343d693a441f0af728e560a0c245bcc4eb97869
2020-08-10 16:08:50 -07:00
Samuel Susla 154ce78972 Take viewport offset into account in UIManager.measureInWindow
Summary:
Changelog: [Internal]

Fabric's UIManager.measureInWindow didn't take viewport's offset into account. This diff fixes it by including viewport's offset in `LayoutContext`.

Reviewed By: JoshuaGross

Differential Revision: D23021903

fbshipit-source-id: 9106a8789d66fe19d8cb0a9378ee5bc8f2c83005
2020-08-10 12:52:23 -07:00
David Vacca 2fa8829305 Extend 'uimanager' module to compile in OSS
Summary:
This diff extends the 'uimanager' module to compile in OSS

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22918206

fbshipit-source-id: 2783ec6d9e53cdafab5c77a3f217b32c1c7f0b41
2020-08-07 11:12:02 -07:00
Joshua Gross 774dec1e17 Introduce general API for setting C++ State from the View layer and getting a notification if it fails, with Android impl
Summary:
iOS will need to be implemented separately, but the shared C++ bits are in place.

Explanation: there is currently no way for the View layer to /know/ if an UpdateState call has succeeded or failed. Generally we just assume it succeeds, but if it fails we have no way of knowing or retrying.

This can cause some UI bugs. To mitigate this, I'm introducing a "failure" notification callback mechanism. The JNI bridging for this is a little complicated to avoid passing Runnable across the JNI, but it
should be much simpler on iOS.

In development this seems to make View components much more reliable.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22940187

fbshipit-source-id: 917f2932ae22d421f91fe8f4fca3f07dc089f820
2020-08-05 06:35:41 -07:00
Samuel Susla 6c01f26d27 Fix typo in UIManagerBinding's destructor
Summary: Changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D22897454

fbshipit-source-id: 73cc02b9645c04dd0e15e255d7277cd87d56fde3
2020-08-04 11:16:16 -07:00
David Vacca 8616f868d5 Create android oss build system for react/config module
Summary:
This diff creates the Android OSS build system for the module react/config

As part of this diff I also moved the module to react/config folder

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22877264

fbshipit-source-id: 5b3c42580d2b1d73dc0abb48bcf4ff063b2f581f
2020-08-03 14:21:26 -07:00
Joshua Gross e3302eeeab LayoutAnimations: call onSuccess, onFailure callbacks
Summary:
Hook up onSuccess and onFailure callbacks to LayoutAnimations.

Note that in non-Fabric RN, onSuccess is not known to work in Android. I could not find any uses of onFailure and it's not documented, so for now, it is only called if the setup of the animation fails.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D22889352

fbshipit-source-id: 4306debb350388dd2b7a2cbfe295eb99723988e2
2020-08-02 16:37:03 -07:00
David Vacca 3093010ea5 move fabric to ReactCommon/react/renderer
Summary:
This diff moves fabric C++ code from ReactCommon/fabric to ReactCommon/react/renderer
As part of this diff I also refactored components, codegen and callsites on CatalystApp, FB4A and venice

Script: P137350694

changelog: [internal] internal refactor

Reviewed By: fkgozali

Differential Revision: D22852139

fbshipit-source-id: f85310ba858b6afd81abfd9cbe6d70b28eca7415
2020-07-31 13:34:29 -07:00