Summary:
There are races between BackgroundExecutor and Fabric/View teardown, where, because of the way things are set up currently, a View will strongly retain a pointer into some native State object which can keep a host of other objects alive in C++, even after stopSurface, and even after RN itself starts tearing down.
To alleviate this, we more aggressively clear State from Java, without waiting for Java GC: 1) on stopSurface, 2) whenever a State object is stale from Java's perspective.
This should allow us to keep all common updateState semantics, while only introducing a new edge-case that stateWrapper can be destroyed during mounting if stopSurface happens at the same time. In those cases, checking for NPEs should be sufficient.
The possible race condition only really happens with updateState, so it's easier to check for. There should practically be no cases where there's a race between `stopSurface` and `getState`, because `getState` is only really called around state updates and view preallocation, and never after; we still check for NPEs in those cases, but it shouldn't be an issue.
Changelog: [Internal]
Reviewed By: thurn, sammy-SC, mdvacca
Differential Revision: D26858584
fbshipit-source-id: 2ef7467220865380037d69d8de322fe8797f6a12
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
Summary:
In practice this has been enabled in production for months and is fine. Remove feature-flag and gating.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451697
fbshipit-source-id: 9e70db3ed4e00de7b8295d9225d43ba09e4e68e9
Summary:
Make sure to check incoming state values before calling SetState, or we call back and forth forever.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D23389355
fbshipit-source-id: 9cf6110cf654fe93f555a6fbfd9b20f112214e0a
Summary:
Update FabricViewStateManager so that the caller can bail out of updates by returning null.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22966024
fbshipit-source-id: 31cd9ec8a9a9918fbb94844e30ed1a2fcc61997d
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
Andres Suarez
generatedunixname89002005287564
Joshua Gross
Valentin Shergin