diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java index 2d96776f882..a40bcaca7ba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java @@ -8,11 +8,13 @@ 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; @@ -28,6 +30,9 @@ 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() { @@ -39,8 +44,35 @@ 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) { + mUpdateStateId++; updateStateImpl((NativeMap) map); } + + @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); + } + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.cpp deleted file mode 100644 index 40bf0c392d6..00000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.cpp +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include "NodeStateWrapper.h" -#include -#include - -using namespace facebook::jni; - -namespace facebook { -namespace react { - -jni::local_ref NodeStateWrapper::initHybrid( - jni::alias_ref) { - return makeCxxInstance(); -} - -jni::local_ref NodeStateWrapper::getState() { - folly::dynamic map = state_->getDynamic(); - local_ref readableNativeMap = - ReadableNativeMap::newObjectCxxArgs(map); - return readableNativeMap; -} - -void NodeStateWrapper::updateState(ReadableNativeMap *map) { - // Get folly::dynamic from map - auto dynamicMap = map->consume(); - // Set state - state_->updateState(dynamicMap); -} - -void NodeStateWrapper::registerNatives() { - registerHybrid({ - makeNativeMethod("getState", NodeStateWrapper::getState), - makeNativeMethod("updateState", NodeStateWrapper::updateState), - }); -} - -} // namespace react -} // namespace facebook diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.h deleted file mode 100644 index 672b8e95b16..00000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/NodeStateWrapper.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include -#include - -namespace facebook { -namespace react { - -class NodeStateWrapper : public jni::HybridClass { - public: - constexpr static const char *const kJavaDescriptor = - "Lcom/facebook/react/fabric/NodeStateWrapper;"; - - NodeStateWrapper() {} - - static void registerNatives(); - - jni::local_ref getState(); - void updateState(ReadableNativeMap *map); - - const State *state_; - - private: - static jni::local_ref initHybrid(jni::alias_ref); -}; - -} // namespace react -} // namespace facebook diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp index 9a5b929e9af..afbe9c71552 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.cpp @@ -33,7 +33,31 @@ void StateWrapperImpl::updateStateImpl(NativeMap *map) { // Get folly::dynamic from map auto dynamicMap = map->consume(); // Set state - state_->updateState(dynamicMap); + state_->updateState(dynamicMap, nullptr); +} + +void StateWrapperImpl::updateStateWithFailureCallbackImpl( + NativeMap *map, + jni::alias_ref 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 globalSelf = make_global(self); + // Set state + state_->updateState( + dynamicMap, [globalSelf = std::move(globalSelf), callbackRefId]() { + static auto method = + jni::findClassStatic( + StateWrapperImpl::StateWrapperImplJavaDescriptor) + ->getMethod("updateStateFailed"); + method(globalSelf, callbackRefId); + }); } void StateWrapperImpl::registerNatives() { @@ -41,6 +65,9 @@ void StateWrapperImpl::registerNatives() { makeNativeMethod("initHybrid", StateWrapperImpl::initHybrid), makeNativeMethod("getState", StateWrapperImpl::getState), makeNativeMethod("updateStateImpl", StateWrapperImpl::updateStateImpl), + makeNativeMethod( + "updateStateWithFailureCallbackImpl", + StateWrapperImpl::updateStateWithFailureCallbackImpl), }); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h index 056a11738ea..9a88eb0271b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h @@ -20,11 +20,17 @@ class StateWrapperImpl : public jni::HybridClass { public: constexpr static const char *const kJavaDescriptor = "Lcom/facebook/react/fabric/StateWrapperImpl;"; + constexpr static auto StateWrapperImplJavaDescriptor = + "com/facebook/react/fabric/StateWrapperImpl"; static void registerNatives(); jni::local_ref getState(); void updateStateImpl(NativeMap *map); + void updateStateWithFailureCallbackImpl( + NativeMap *map, + jni::alias_ref self, + int callbackRefId); State::Shared state_; diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricStateBaseViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricStateBaseViewGroup.java deleted file mode 100644 index af24da786bc..00000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricStateBaseViewGroup.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.uimanager; - -import android.view.ViewGroup; -import com.facebook.react.bridge.UiThreadUtil; -import com.facebook.react.bridge.WritableMap; -import com.facebook.react.config.ReactFeatureFlags; - -/** This is a helper base class for ViewGroups that use Fabric State. */ -public abstract class FabricStateBaseViewGroup extends ViewGroup { - public interface StateUpdateCallback { - WritableMap getStateUpdate(); - } - - private StateWrapper mStateWrapper = null; - - public FabricStateBaseViewGroup(ThemedReactContext context) { - super(context); - } - - public void setStateWrapper(StateWrapper stateWrapper) { - mStateWrapper = stateWrapper; - } - - private static void setState( - final FabricStateBaseViewGroup view, - final StateWrapper stateWrapper, - final StateUpdateCallback stateUpdateCallback, - final int numTries) { - // The StateWrapper will change, breaking this loop, whenever the UpdateState MountItem - // is executed. - // The caller is responsible for detecting if data is up-to-date, and doing nothing, or - // detecting if state is stale and calling setState again. - if (stateWrapper != view.mStateWrapper) { - return; - } - // We arbitrarily bail out after a certain number of retries. - // This is a pretty large number: in practice I've seen this number go over 50 - // with minimal/no visual jank. - if (numTries > 5 * 60) { - return; - } - - stateWrapper.updateState(stateUpdateCallback.getStateUpdate()); - - // An `updateState` call can fail, and there's no way to verify if it succeeds besides - // waiting for a corresponding `StateUpdate` MountItem to be executed on some future UI tick. - // So.... to resolve conflicts with updateState, we just keep firing it until it succeeds or - // the View goes away. - if (ReactFeatureFlags.enableExperimentalStateUpdateRetry) { - UiThreadUtil.runOnUiThread( - new Runnable() { - @Override - public void run() { - setState(view, stateWrapper, stateUpdateCallback, numTries + 1); - } - }); - } - } - - public static void setState( - final FabricStateBaseViewGroup view, - final StateWrapper stateWrapper, - final StateUpdateCallback stateUpdateCallback) { - setState(view, stateWrapper, stateUpdateCallback, 0); - } -} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java new file mode 100644 index 00000000000..454fd2bbb7d --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java @@ -0,0 +1,90 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.uimanager; + +import androidx.annotation.Nullable; +import com.facebook.common.logging.FLog; +import com.facebook.react.bridge.WritableMap; +import com.facebook.react.config.ReactFeatureFlags; + +/** + * This is a helper base class for ViewGroups that use Fabric State. + * + *

Reason to use this: UpdateState calls from the View layer to the Fabric core can fail, and + * optionally Fabric will call a "failure callback" if that happens. This class abstracts that and + * makes it easier ensure that State in Fabric is always up-to-date. + * + *

1. Whenever ViewManager.updateState is called, call View.setStateWrapper. 2. Instead of + * calling StateWrapper.updateState directly, call View.setState and it will automatically keep + * retrying the UpdateState call until it succeeds; or you call setState again; or the View layer is + * updated with a newer StateWrapper. + */ +public class FabricViewStateManager { + private static final String TAG = "FabricViewStateManager"; + + public interface HasFabricViewStateManager { + FabricViewStateManager getFabricViewStateManager(); + } + + public interface StateUpdateCallback { + WritableMap getStateUpdate(); + } + + @Nullable private StateWrapper mStateWrapper = null; + + public void setStateWrapper(StateWrapper stateWrapper) { + mStateWrapper = stateWrapper; + } + + public boolean hasStateWrapper() { + return mStateWrapper != null; + } + + private void setState( + @Nullable final StateWrapper stateWrapper, + final StateUpdateCallback stateUpdateCallback, + final int numTries) { + // The StateWrapper will change, breaking the async loop, whenever the UpdateState MountItem + // is executed. + // The caller is responsible for detecting if data is up-to-date, and doing nothing, or + // detecting if state is stale and calling setState again. + if (stateWrapper == null) { + FLog.e(TAG, "setState called without a StateWrapper"); + return; + } + if (stateWrapper != mStateWrapper) { + return; + } + // We bail out after an arbitrary number of tries. In practice this should never go higher + // than 2 or 3, but there's nothing guaranteeing that. + if (numTries > 60) { + return; + } + + Runnable failureRunnable = null; + if (ReactFeatureFlags.enableExperimentalStateUpdateRetry) { + failureRunnable = + new Runnable() { + @Override + // Run on the UI thread + public void run() { + FLog.e(TAG, "UpdateState failed - retrying! " + numTries); + setState(stateWrapper, stateUpdateCallback, numTries + 1); + } + }; + } + stateWrapper.updateState( + stateUpdateCallback.getStateUpdate(), + // Failure callback - this is run if the updateState call fails + failureRunnable); + } + + public void setState(final StateUpdateCallback stateUpdateCallback) { + setState(mStateWrapper, stateUpdateCallback, 0); + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java index 6ac419a62d9..e56a16cd50f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java @@ -23,4 +23,10 @@ public interface StateWrapper { /** Pass a map of values back to the C++ layer. */ void updateState(WritableMap map); + + /** + * Pass a map of values back to the C++ layer. /Last/ runnable passed into updateState is called + * if an updateState call fails. + */ + void updateState(WritableMap map, Runnable failureCallback); } diff --git a/ReactCommon/react/renderer/core/ConcreteState.h b/ReactCommon/react/renderer/core/ConcreteState.h index 6b8c73cbc35..c57a6518bc6 100644 --- a/ReactCommon/react/renderer/core/ConcreteState.h +++ b/ReactCommon/react/renderer/core/ConcreteState.h @@ -60,11 +60,13 @@ class ConcreteState : public State { */ void updateState( Data &&newData, + std::function failureCallback = nullptr, EventPriority priority = EventPriority::AsynchronousUnbatched) const { updateState( [data = std::move(newData)](Data const &oldData) mutable -> Data && { return std::move(data); }, + failureCallback, priority); } @@ -78,6 +80,7 @@ class ConcreteState : public State { */ void updateState( std::function callback, + std::function failureCallback = nullptr, EventPriority priority = EventPriority::AsynchronousBatched) const { auto family = family_.lock(); @@ -88,11 +91,13 @@ class ConcreteState : public State { } auto stateUpdate = StateUpdate{ - family, [=](StateData::Shared const &oldData) -> StateData::Shared { + family, + [=](StateData::Shared const &oldData) -> StateData::Shared { assert(oldData); return std::make_shared( callback(*std::static_pointer_cast(oldData))); - }}; + }, + failureCallback}; family->dispatchRawState(std::move(stateUpdate), priority); } @@ -102,8 +107,9 @@ class ConcreteState : public State { return getData().getDynamic(); } - void updateState(folly::dynamic data) const override { - updateState(std::move(Data(getData(), data))); + void updateState(folly::dynamic data, std::function failureCallback) + const override { + updateState(std::move(Data(getData(), data)), failureCallback); } #endif }; diff --git a/ReactCommon/react/renderer/core/State.h b/ReactCommon/react/renderer/core/State.h index c0472fec057..ae636f37e42 100644 --- a/ReactCommon/react/renderer/core/State.h +++ b/ReactCommon/react/renderer/core/State.h @@ -65,7 +65,9 @@ class State { #ifdef ANDROID virtual folly::dynamic getDynamic() const = 0; - virtual void updateState(folly::dynamic data) const = 0; + virtual void updateState( + folly::dynamic data, + std::function failureCallback) const = 0; #endif protected: diff --git a/ReactCommon/react/renderer/core/StateUpdate.h b/ReactCommon/react/renderer/core/StateUpdate.h index 5e52cdc575a..a77abb90dc6 100644 --- a/ReactCommon/react/renderer/core/StateUpdate.h +++ b/ReactCommon/react/renderer/core/StateUpdate.h @@ -21,9 +21,11 @@ class StateUpdate { public: using Callback = std::function; + using FailureCallback = std::function; SharedShadowNodeFamily family; Callback callback; + FailureCallback failureCallback; }; } // namespace react diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp index 980de3b5414..91827e3fb94 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -238,23 +238,27 @@ void UIManager::updateState(StateUpdate const &stateUpdate) const { shadowTreeRegistry_.visit( family->getSurfaceId(), [&](ShadowTree const &shadowTree) { - shadowTree.tryCommit([&](RootShadowNode::Shared const - &oldRootShadowNode) { - return std::static_pointer_cast< - RootShadowNode>(oldRootShadowNode->cloneTree( - *family, [&](ShadowNode const &oldShadowNode) { - auto newData = - callback(oldShadowNode.getState()->getDataPointer()); - auto newState = - componentDescriptor.createState(*family, newData); + bool updateSucceeded = shadowTree.tryCommit( + [&](RootShadowNode::Shared const &oldRootShadowNode) { + return std::static_pointer_cast< + RootShadowNode>(oldRootShadowNode->cloneTree( + *family, [&](ShadowNode const &oldShadowNode) { + auto newData = + callback(oldShadowNode.getState()->getDataPointer()); + auto newState = + componentDescriptor.createState(*family, newData); - return oldShadowNode.clone({ - /* .props = */ ShadowNodeFragment::propsPlaceholder(), - /* .children = */ ShadowNodeFragment::childrenPlaceholder(), - /* .state = */ newState, - }); - })); - }); + return oldShadowNode.clone({ + /* .props = */ ShadowNodeFragment::propsPlaceholder(), + /* .children = */ + ShadowNodeFragment::childrenPlaceholder(), + /* .state = */ newState, + }); + })); + }); + if (!updateSucceeded && stateUpdate.failureCallback) { + stateUpdate.failureCallback(); + } }); }