From 067d2eee033561b3455f10b40af629c6056124b2 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 1 Dec 2020 19:50:55 -0800 Subject: [PATCH] Remove `enableDrawMutationFix` feature flag and ship it Summary: This was added to prevent mutating the UI during draw or measure, and appears to have been effective. Keep the comments, ship the feature, remove flags. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D25258409 fbshipit-source-id: 36ad8a03d1eb82bc9dcd769372c03f1ebe8b8da8 --- .../com/facebook/react/config/ReactFeatureFlags.java | 3 --- .../com/facebook/react/fabric/FabricUIManager.java | 12 +++++------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index d972c83330e..58ffe7db73a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -86,9 +86,6 @@ public class ReactFeatureFlags { /** Disable customDrawOrder in ReactViewGroup under Fabric only. */ public static boolean disableCustomDrawOrderFabric = false; - /** Potential bugfix for crashes caused by mutating the view hierarchy during onDraw. */ - public static boolean enableDrawMutationFix = true; - /** Use lock-free data structures for Fabric MountItems. */ public static boolean enableLockFreeMountInstructions = false; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 76ab25a5463..b8d3dc7e609 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -586,11 +586,11 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { int commitNumber = mCurrentSynchronousCommitNumber++; - // We are on the UI thread so this would otherwise be safe to call, *BUT* we don't know - // where we are on the callstack. Why isn't this safe, and why do we have additional safeguards - // here? + // We are on the UI thread so it would otherwise be safe to call `tryDispatchMountItems` here to + // flush previously-queued mountitems, *BUT* we don't know where we are on the callstack. + // Why isn't it safe, and why do we have additional safeguards here? // - // A tangible example where this will cause a crash: + // A tangible example where it would cause a crash, and did in the past: // 1. There are queued "delete" mutations // 2. We're called by this stack trace: // FabricUIManager.synchronouslyUpdateViewOnUIThread(FabricUIManager.java:574) @@ -607,9 +607,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // android.widget.ScrollView.overScrollBy(ScrollView.java:2040) // android.widget.ScrollView.computeScroll(ScrollView.java:1481) // android.view.View.updateDisplayListIfDirty(View.java:20466) - if (!ReactFeatureFlags.enableDrawMutationFix) { - tryDispatchMountItems(); - } + // 3. A view is deleted while its parent is being drawn, causing a crash. MountItem synchronousMountItem = new MountItem() {