Commit Graph
5 Commits
Author SHA1 Message Date
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 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
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