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
This commit is contained in:
Valentin Shergin
2020-12-23 21:46:56 -08:00
committed by Facebook GitHub Bot
parent 3ade096f02
commit f379b1e583
12 changed files with 25 additions and 170 deletions
@@ -8,13 +8,11 @@
package com.facebook.react.fabric;
import android.annotation.SuppressLint;
import androidx.annotation.AnyThread;
import androidx.annotation.NonNull;
import com.facebook.jni.HybridData;
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.NativeMap;
import com.facebook.react.bridge.ReadableNativeMap;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.uimanager.StateWrapper;
@@ -30,9 +28,6 @@ public class StateWrapperImpl implements StateWrapper {
@DoNotStrip private final HybridData mHybridData;
private Runnable mFailureCallback = null;
private int mUpdateStateId = 0;
private static native HybridData initHybrid();
private StateWrapperImpl() {
@@ -44,29 +39,8 @@ public class StateWrapperImpl implements StateWrapper {
public native void updateStateImpl(@NonNull NativeMap map);
public native void updateStateWithFailureCallbackImpl(
@NonNull NativeMap map, Object self, int updateStateId);
@Override
public void updateState(@NonNull WritableMap map, Runnable failureCallback) {
mUpdateStateId++;
mFailureCallback = failureCallback;
updateStateWithFailureCallbackImpl((NativeMap) map, this, mUpdateStateId);
}
@DoNotStrip
@AnyThread
public void updateStateFailed(int callbackRefId) {
// If the callback ref ID doesn't match the ID of the most-recent updateState call,
// then it's an outdated failure callback and we ignore it.
if (callbackRefId != mUpdateStateId) {
return;
}
final Runnable failureCallback = mFailureCallback;
mFailureCallback = null;
if (failureCallback != null) {
UiThreadUtil.runOnUiThread(failureCallback);
}
public void updateState(@NonNull WritableMap map) {
updateStateImpl((NativeMap) map);
}
}
@@ -33,31 +33,7 @@ void StateWrapperImpl::updateStateImpl(NativeMap *map) {
// Get folly::dynamic from map
auto dynamicMap = map->consume();
// Set state
state_->updateState(dynamicMap, nullptr);
}
void StateWrapperImpl::updateStateWithFailureCallbackImpl(
NativeMap *map,
jni::alias_ref<jobject> self,
int callbackRefId) {
// Get folly::dynamic from map
auto dynamicMap = map->consume();
// Turn the alias into a global_ref
// Note: this whole thing feels really janky, making StateWrapperImpl.java
// pass "this" into a function it's calling on "this". But after struggling
// for a while I couldn't figure out how to get a reference to the Java side
// of "this" in C++ in a way that's reasonably safe, and it maybe is even
// discouraged. Anyway, it might be weird, but this seems to work and be safe.
jni::global_ref<jobject> globalSelf = make_global(self);
// Set state
state_->updateState(
dynamicMap, [globalSelf = std::move(globalSelf), callbackRefId]() {
static auto method =
jni::findClassStatic(
StateWrapperImpl::StateWrapperImplJavaDescriptor)
->getMethod<void(jint)>("updateStateFailed");
method(globalSelf, callbackRefId);
});
state_->updateState(dynamicMap);
}
void StateWrapperImpl::registerNatives() {
@@ -65,9 +41,6 @@ void StateWrapperImpl::registerNatives() {
makeNativeMethod("initHybrid", StateWrapperImpl::initHybrid),
makeNativeMethod("getState", StateWrapperImpl::getState),
makeNativeMethod("updateStateImpl", StateWrapperImpl::updateStateImpl),
makeNativeMethod(
"updateStateWithFailureCallbackImpl",
StateWrapperImpl::updateStateWithFailureCallbackImpl),
});
}
@@ -79,10 +79,9 @@ public class FabricViewStateManager {
if (stateUpdate == null) {
return;
}
stateWrapper.updateState(
stateUpdate,
// Failure callback - this is run if the updateState call fails
failureRunnable);
// TODO: State update cannot fail; remove `failureRunnable` and custom retrying logic.
stateWrapper.updateState(stateUpdate);
}
public void setState(final StateUpdateCallback stateUpdateCallback) {
@@ -22,8 +22,8 @@ public interface StateWrapper {
ReadableNativeMap getState();
/**
* Pass a map of values back to the C++ layer. /Last/ runnable passed into updateState is called
* if an updateState call fails.
* Pass a map of values back to the C++ layer. The operation is performed synchronously and cannot
* fail.
*/
void updateState(WritableMap map, Runnable failureCallback);
void updateState(WritableMap map);
}