diff --git a/React/Fabric/Mounting/ComponentViews/SafeAreaView/RCTSafeAreaViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/SafeAreaView/RCTSafeAreaViewComponentView.mm index 1180a898cb0..fa362f134a4 100644 --- a/React/Fabric/Mounting/ComponentViews/SafeAreaView/RCTSafeAreaViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/SafeAreaView/RCTSafeAreaViewComponentView.mm @@ -61,7 +61,7 @@ using namespace facebook::react; auto newPadding = RCTEdgeInsetsFromUIEdgeInsets(insets); auto threshold = 1.0 / RCTScreenScale() + 0.01; // Size of a pixel plus some small threshold. - _state->updateStateWithAutorepeat( + _state->updateState( [=](SafeAreaViewShadowNode::ConcreteState::Data const &oldData) -> SafeAreaViewShadowNode::ConcreteState::SharedData { auto oldPadding = oldData.padding; @@ -74,7 +74,7 @@ using namespace facebook::react; auto newData = oldData; newData.padding = newPadding; - return std::make_shared(newData); + return std::make_shared(newData); }); } diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 889452e5d16..8ff8770a97a 100644 --- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -311,7 +311,7 @@ static void RCTSendPaperScrollEvent_DEPRECATED(UIScrollView *scrollView, NSInteg _state->updateState([contentOffset](ScrollViewShadowNode::ConcreteState::Data const &data) { auto newData = data; newData.contentOffset = contentOffset; - return newData; + return std::make_shared(newData); }); } 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 6a461623e99..2d96776f882 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/StateWrapperImpl.java @@ -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); } } 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 afbe9c71552..9a5b929e9af 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,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 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); - }); + 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), }); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java index f6bdd767801..f0b7cfb8ff6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/FabricViewStateManager.java @@ -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) { 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 51bbc14349a..c21d2414d0b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/StateWrapper.java @@ -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); } diff --git a/ReactCommon/react/renderer/core/ConcreteState.h b/ReactCommon/react/renderer/core/ConcreteState.h index 33fa5dac788..9ee3b21ec51 100644 --- a/ReactCommon/react/renderer/core/ConcreteState.h +++ b/ReactCommon/react/renderer/core/ConcreteState.h @@ -60,55 +60,23 @@ 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); + [data = std::move(newData)](Data const &oldData) mutable -> SharedData { + return std::make_shared(std::move(data)); }, - failureCallback, priority); } /* * Initiate a state update process with a given function (that transforms an - * old data value to a new one) and priority. The update function can be - * called from any thread any moment later. The function can be called only - * once or not called at all (in the case where the node was already unmounted - * and updating makes no sense). The state update operation might fail in case - * of conflict. + * old data value to a new one) and priority. The callback function can be + * called from any thread any moment later. + * In case of a conflict, the `callback` might be called several times until + * it succeeded. To cancel the state update operation, the callback needs to + * return `nullptr`. */ void updateState( - std::function callback, - std::function failureCallback = nullptr, - EventPriority priority = EventPriority::AsynchronousBatched) const { - auto family = family_.lock(); - - if (!family) { - // No more nodes of this family exist anymore, - // updating state is impossible. - return; - } - - auto stateUpdate = StateUpdate{ - family, - [=](StateData::Shared const &oldData) -> StateData::Shared { - assert(oldData); - return std::make_shared( - callback(*std::static_pointer_cast(oldData))); - }, - failureCallback, - false}; - - family->dispatchRawState(std::move(stateUpdate), priority); - } - - /* - * An experimental version of `updateState` function that re-commit the state - * update over and over again until it succeeded. To cancel the state update - * operation, the state update lambda needs to return `nullptr`. - */ - void updateStateWithAutorepeat( std::function callback, EventPriority priority = EventPriority::AsynchronousBatched) const { auto family = family_.lock(); @@ -120,14 +88,10 @@ class ConcreteState : public State { } auto stateUpdate = StateUpdate{ - family, - [=](StateData::Shared const &oldData) -> StateData::Shared { + family, [=](StateData::Shared const &oldData) -> StateData::Shared { assert(oldData); return callback(*std::static_pointer_cast(oldData)); - }, - nullptr, - true, - }; + }}; family->dispatchRawState(std::move(stateUpdate), priority); } @@ -137,9 +101,8 @@ class ConcreteState : public State { return getData().getDynamic(); } - void updateState(folly::dynamic data, std::function failureCallback) - const override { - updateState(std::move(Data(getData(), data)), failureCallback); + void updateState(folly::dynamic data) const override { + updateState(std::move(Data(getData(), data))); } #endif }; diff --git a/ReactCommon/react/renderer/core/State.h b/ReactCommon/react/renderer/core/State.h index ae636f37e42..c0472fec057 100644 --- a/ReactCommon/react/renderer/core/State.h +++ b/ReactCommon/react/renderer/core/State.h @@ -65,9 +65,7 @@ class State { #ifdef ANDROID virtual folly::dynamic getDynamic() const = 0; - virtual void updateState( - folly::dynamic data, - std::function failureCallback) const = 0; + virtual void updateState(folly::dynamic data) const = 0; #endif protected: diff --git a/ReactCommon/react/renderer/core/StateUpdate.h b/ReactCommon/react/renderer/core/StateUpdate.h index 7931f5e0800..890e56d1410 100644 --- a/ReactCommon/react/renderer/core/StateUpdate.h +++ b/ReactCommon/react/renderer/core/StateUpdate.h @@ -25,8 +25,6 @@ class StateUpdate { SharedShadowNodeFamily family; Callback callback; - FailureCallback failureCallback; - bool autorepeat; }; } // namespace react diff --git a/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 9fe70284e2f..313b4e201ec 100644 --- a/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -109,15 +109,9 @@ Scheduler::Scheduler( #ifdef ANDROID removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( "react_fabric:remove_outstanding_surfaces_on_destruction_android"); - uiManager_->experimentEnableStateUpdateWithAutorepeat = - reactNativeConfig_->getBool( - "react_fabric:enable_state_update_with_autorepeat_android"); #else removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( "react_fabric:remove_outstanding_surfaces_on_destruction_ios"); - uiManager_->experimentEnableStateUpdateWithAutorepeat = - reactNativeConfig_->getBool( - "react_fabric:enable_state_update_with_autorepeat_ios"); #endif } diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp index 3da07b811a7..de353321bf2 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -191,8 +191,7 @@ LayoutMetrics UIManager::getRelativeLayoutMetrics( shadowNode.getFamily(), *layoutableAncestorShadowNode, policy); } -void UIManager::updateStateWithAutorepeat( - StateUpdate const &stateUpdate) const { +void UIManager::updateState(StateUpdate const &stateUpdate) const { auto &callback = stateUpdate.callback; auto &family = stateUpdate.family; auto &componentDescriptor = family->getComponentDescriptor(); @@ -230,43 +229,6 @@ void UIManager::updateStateWithAutorepeat( }); } -void UIManager::updateState(StateUpdate const &stateUpdate) const { - if (stateUpdate.autorepeat || experimentEnableStateUpdateWithAutorepeat) { - updateStateWithAutorepeat(stateUpdate); - return; - } - - auto &callback = stateUpdate.callback; - auto &family = stateUpdate.family; - auto &componentDescriptor = family->getComponentDescriptor(); - - shadowTreeRegistry_.visit( - family->getSurfaceId(), [&](ShadowTree const &shadowTree) { - auto status = shadowTree.tryCommit([&](RootShadowNode const - &oldRootShadowNode) { - return std::static_pointer_cast( - 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, - }); - })); - }); - if (status != ShadowTree::CommitStatus::Succeeded && - stateUpdate.failureCallback) { - stateUpdate.failureCallback(); - } - }); -} - void UIManager::dispatchCommand( const ShadowNode::Shared &shadowNode, std::string const &commandName, diff --git a/ReactCommon/react/renderer/uimanager/UIManager.h b/ReactCommon/react/renderer/uimanager/UIManager.h index 62f3e6ba5db..c2c76cc03b5 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.h +++ b/ReactCommon/react/renderer/uimanager/UIManager.h @@ -83,11 +83,6 @@ class UIManager final : public ShadowTreeDelegate { ShadowTree const &shadowTree, MountingCoordinator::Shared const &mountingCoordinator) const override; - /* - * Temporary flags. - */ - bool experimentEnableStateUpdateWithAutorepeat{false}; - RootShadowNode::Unshared shadowTreeWillCommit( ShadowTree const &shadowTree, RootShadowNode::Shared const &oldRootShadowNode, @@ -143,7 +138,6 @@ class UIManager final : public ShadowTreeDelegate { * and performs a commit. */ void updateState(StateUpdate const &stateUpdate) const; - void updateStateWithAutorepeat(StateUpdate const &stateUpdate) const; void dispatchCommand( const ShadowNode::Shared &shadowNode,