From 8ce5cdb0e5acc73379878d8fc1fd277e945c0315 Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Mon, 22 Jan 2024 11:20:12 -0800 Subject: [PATCH] Revert QE for ReactFeatureFlags.reduceDeleteCreateMutation (#42339) Summary: This diff cleans up the code added in D41900201 to run QE: https://fburl.com/qe2/ovgxlayy More context: https://fb.workplace.com/groups/react.technologies.discussions/permalink/3479283292303384/ Changelog: [Android][Internal] - Revert internal optimizations Reviewed By: mdvacca Differential Revision: D52750975 --- .../ReactAndroid/api/ReactAndroid.api | 1 - .../react/config/ReactFeatureFlags.java | 6 --- .../react/fabric/FabricMountingManager.cpp | 38 +------------------ .../jni/react/fabric/FabricMountingManager.h | 2 - 4 files changed, 1 insertion(+), 46 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index a426e8b256a4..656519cb86c2 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -1911,7 +1911,6 @@ public class com/facebook/react/config/ReactFeatureFlags { public static field enableViewRecycling Z public static field excludeYogaFromRawProps Z public static field fixStoppedSurfaceTagSetLeak Z - public static field reduceDeleteCreateMutation Z public static field reduceDeleteCreateMutationLayoutAnimation Z public static field rejectTurboModulePromiseOnNativeError Z public static field traceTurboModulePromiseRejections Z diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index ee4882d37f59..a244b1746f99 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -112,12 +112,6 @@ public class ReactFeatureFlags { */ public static boolean reduceDeleteCreateMutationLayoutAnimation = true; - /** - * Allow fix to drop delete...create mutations which could cause missing view state in Fabric - * SurfaceMountingManager. - */ - public static boolean reduceDeleteCreateMutation = false; - /** Report mount operations from the host platform to notify mount hooks. */ public static boolean enableMountHooks = false; diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp index 8374141f67fb..2a459f77167f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp @@ -30,22 +30,10 @@ namespace facebook::react { -constexpr static auto kReactFeatureFlagsJavaDescriptor = - "com/facebook/react/config/ReactFeatureFlags"; - -static bool getFeatureFlagValue(const char* name) { - static const auto reactFeatureFlagsClass = - jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); - const auto field = reactFeatureFlagsClass->getStaticField(name); - return reactFeatureFlagsClass->getStaticFieldValue(field); -} - FabricMountingManager::FabricMountingManager( std::shared_ptr& config, jni::global_ref& javaUIManager) - : javaUIManager_(javaUIManager), - reduceDeleteCreateMutation_( - getFeatureFlagValue("reduceDeleteCreateMutation")) {} + : javaUIManager_(javaUIManager) {} void FabricMountingManager::onSurfaceStart(SurfaceId surfaceId) { std::lock_guard lock(allocatedViewsMutex_); @@ -288,30 +276,6 @@ void FabricMountingManager::executeMount( case ShadowViewMutation::Create: { bool shouldCreateView = !allocatedViewTags.contains(newChildShadowView.tag); - if (reduceDeleteCreateMutation_) { - // Detect DELETE...CREATE situation on the same node and do NOT push - // back to the mount items. This is an edge case that may happen - // when for example animation runs while commit happened, and we - // want to filter them out here to capture all possible sources of - // such mutations. The re-ordering logic here assumes no - // DELETE...CREATE in the mutations, as we will re-order mutations - // and batch all DELETE instructions in the end. - auto it = std::remove_if( - cppDeleteMountItems.begin(), - cppDeleteMountItems.end(), - [&](auto& deletedMountItem) -> bool { - return deletedMountItem.oldChildShadowView.tag == - newChildShadowView.tag; - }); - bool hasDeletedViewsWithSameTag = it != cppDeleteMountItems.end(); - cppDeleteMountItems.erase(it, cppDeleteMountItems.end()); - - if (hasDeletedViewsWithSameTag) { - shouldCreateView = false; - LOG(ERROR) - << "XIN: Detect DELETE...CREATE on the same tag from mutations in the same batch. The DELETE and CREATE mutations are removed before sending to the native platforms"; - } - } if (shouldCreateView) { cppCommonMountItems.push_back( diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h index 9ee80750c165..0dff4e3daff2 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h @@ -63,8 +63,6 @@ class FabricMountingManager final { allocatedViewRegistry_{}; std::recursive_mutex allocatedViewsMutex_; - const bool reduceDeleteCreateMutation_{false}; - jni::local_ref getProps( const ShadowView& oldShadowView, const ShadowView& newShadowView);