Skip to content

Commit

Permalink
remove enableFixForClippedSubviewsCrash experiment (#43963)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #43963

changelog: [internal]

This experiment did work out. Let's clean it up

Reviewed By: cortinico

Differential Revision: D55797519

fbshipit-source-id: a5da97a7d31b9395b25bfd37db567054721599b0
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Apr 9, 2024
1 parent 49b0d26 commit 3e73dfb
Show file tree
Hide file tree
Showing 21 changed files with 31 additions and 154 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.facebook.react.fabric.events.EventEmitterWrapper;
import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor;
import com.facebook.react.fabric.mounting.mountitems.MountItem;
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
import com.facebook.react.modules.core.ReactChoreographer;
import com.facebook.react.touch.JSResponderHandler;
import com.facebook.react.uimanager.IViewGroupManager;
Expand All @@ -50,7 +49,6 @@
import com.facebook.react.uimanager.ViewManager;
import com.facebook.react.uimanager.ViewManagerRegistry;
import com.facebook.react.uimanager.events.EventCategoryDef;
import com.facebook.react.views.view.ReactViewGroup;
import java.util.ArrayDeque;
import java.util.HashSet;
import java.util.LinkedList;
Expand Down Expand Up @@ -384,25 +382,8 @@ public void addViewAt(final int parentTag, final int tag, final int index) {
// should be impossible - we mark this as a "readded" View and
// thus prevent the RemoveDeleteTree worker from deleting this
// View in the future.
if (ReactNativeFeatureFlags.enableFixForClippedSubviewsCrash()) {
if (viewParent instanceof ReactViewGroup) {
ReactViewGroup viewParentGroup = (ReactViewGroup) viewParent;
// If the parent group has subview clipping enabled, we need to use the specialized
// method.
// Otherwise, ReactViewGroup's member variables managing subview clipping
// will get out of sync with Android's view hierarchy, leading to a crash.
if (viewParentGroup.getRemoveClippedSubviews()) {
viewParentGroup.removeViewWithSubviewClippingEnabled(view);
} else {
viewParentGroup.removeView(view);
}
} else if (viewParent instanceof ViewGroup) {
((ViewGroup) viewParent).removeView(view);
}
} else {
if (viewParent instanceof ViewGroup) {
((ViewGroup) viewParent).removeView(view);
}
if (viewParent instanceof ViewGroup) {
((ViewGroup) viewParent).removeView(view);
}
mErroneouslyReaddedReactTags.add(tag);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<cc44495e0b264e2d56e155a164a774e7>>
* @generated SignedSource<<4c0ede4fa927bc8361d0355bc7cddb10>>
*/

/**
Expand Down Expand Up @@ -58,12 +58,6 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun enableCustomDrawOrderFabric(): Boolean = accessor.enableCustomDrawOrderFabric()

/**
* Attempt at fixing a crash related to subview clipping on Android. This is a kill switch for the fix
*/
@JvmStatic
public fun enableFixForClippedSubviewsCrash(): Boolean = accessor.enableFixForClippedSubviewsCrash()

/**
* Enables the use of microtasks in Hermes (scheduling) and RuntimeScheduler (execution).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<99c459dce42767a7da84810fbceea73e>>
* @generated SignedSource<<aa5b9eac9da720ff1a1bc44325a92c46>>
*/

/**
Expand All @@ -25,7 +25,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var enableBackgroundExecutorCache: Boolean? = null
private var enableCleanTextInputYogaNodeCache: Boolean? = null
private var enableCustomDrawOrderFabricCache: Boolean? = null
private var enableFixForClippedSubviewsCrashCache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
private var enableMountHooksAndroidCache: Boolean? = null
private var enableSpannableBuildingUnificationCache: Boolean? = null
Expand Down Expand Up @@ -81,15 +80,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun enableFixForClippedSubviewsCrash(): Boolean {
var cached = enableFixForClippedSubviewsCrashCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.enableFixForClippedSubviewsCrash()
enableFixForClippedSubviewsCrashCache = cached
}
return cached
}

override fun enableMicrotasks(): Boolean {
var cached = enableMicrotasksCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<e3a1a041fc9d4f8ea504d064ea01607a>>
* @generated SignedSource<<69c4a4aa9621adfe02c15f5f011b3804>>
*/

/**
Expand Down Expand Up @@ -38,8 +38,6 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun enableCustomDrawOrderFabric(): Boolean

@DoNotStrip @JvmStatic public external fun enableFixForClippedSubviewsCrash(): Boolean

@DoNotStrip @JvmStatic public external fun enableMicrotasks(): Boolean

@DoNotStrip @JvmStatic public external fun enableMountHooksAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<cc645d933e253361d5933f56501a35e9>>
* @generated SignedSource<<299901489d5059ed7124f6738be0e48d>>
*/

/**
Expand Down Expand Up @@ -33,8 +33,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun enableCustomDrawOrderFabric(): Boolean = false

override fun enableFixForClippedSubviewsCrash(): Boolean = false

override fun enableMicrotasks(): Boolean = false

override fun enableMountHooksAndroid(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<dc699311bdbebb8db4ee313611d192b5>>
* @generated SignedSource<<1c5e5d965ec746a3d06748ef922b8460>>
*/

/**
Expand All @@ -29,7 +29,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var enableBackgroundExecutorCache: Boolean? = null
private var enableCleanTextInputYogaNodeCache: Boolean? = null
private var enableCustomDrawOrderFabricCache: Boolean? = null
private var enableFixForClippedSubviewsCrashCache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
private var enableMountHooksAndroidCache: Boolean? = null
private var enableSpannableBuildingUnificationCache: Boolean? = null
Expand Down Expand Up @@ -90,16 +89,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun enableFixForClippedSubviewsCrash(): Boolean {
var cached = enableFixForClippedSubviewsCrashCache
if (cached == null) {
cached = currentProvider.enableFixForClippedSubviewsCrash()
accessedFeatureFlags.add("enableFixForClippedSubviewsCrash")
enableFixForClippedSubviewsCrashCache = cached
}
return cached
}

override fun enableMicrotasks(): Boolean {
var cached = enableMicrotasksCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<51dd25e88ebf2ac9929c477e710d3d8d>>
* @generated SignedSource<<2412f7c99bf424397e091a529ac05f32>>
*/

/**
Expand Down Expand Up @@ -33,8 +33,6 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun enableCustomDrawOrderFabric(): Boolean

@DoNotStrip public fun enableFixForClippedSubviewsCrash(): Boolean

@DoNotStrip public fun enableMicrotasks(): Boolean

@DoNotStrip public fun enableMountHooksAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,9 +692,7 @@ public void run() {
}
}

// TODO: make this method package only once we remove Android's mounting layer retry mechanism.
@VisibleForTesting
public void removeViewWithSubviewClippingEnabled(View view) {
/*package*/ void removeViewWithSubviewClippingEnabled(View view) {
UiThreadUtil.assertOnUiThread();

Assertions.assertCondition(mRemoveClippedSubviews);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<ade99ab28f82affa77445231caed9e9d>>
* @generated SignedSource<<0756d56ae2ce45f020ac9a87e9a3d1c3>>
*/

/**
Expand Down Expand Up @@ -69,12 +69,6 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool enableFixForClippedSubviewsCrash() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("enableFixForClippedSubviewsCrash");
return method(javaProvider_);
}

bool enableMicrotasks() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("enableMicrotasks");
Expand Down Expand Up @@ -158,11 +152,6 @@ bool JReactNativeFeatureFlagsCxxInterop::enableCustomDrawOrderFabric(
return ReactNativeFeatureFlags::enableCustomDrawOrderFabric();
}

bool JReactNativeFeatureFlagsCxxInterop::enableFixForClippedSubviewsCrash(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::enableFixForClippedSubviewsCrash();
}

bool JReactNativeFeatureFlagsCxxInterop::enableMicrotasks(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::enableMicrotasks();
Expand Down Expand Up @@ -240,9 +229,6 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"enableCustomDrawOrderFabric",
JReactNativeFeatureFlagsCxxInterop::enableCustomDrawOrderFabric),
makeNativeMethod(
"enableFixForClippedSubviewsCrash",
JReactNativeFeatureFlagsCxxInterop::enableFixForClippedSubviewsCrash),
makeNativeMethod(
"enableMicrotasks",
JReactNativeFeatureFlagsCxxInterop::enableMicrotasks),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<ca2e44ed44d1ba779bb4ac49d795299f>>
* @generated SignedSource<<1ae5c51e5b1c1725565e57a02d19c3a7>>
*/

/**
Expand Down Expand Up @@ -45,9 +45,6 @@ class JReactNativeFeatureFlagsCxxInterop
static bool enableCustomDrawOrderFabric(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool enableFixForClippedSubviewsCrash(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool enableMicrotasks(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<f1a871bce940e7558363902e56a1ed5e>>
* @generated SignedSource<<520ae882fb57dec58ededd47411c0b1d>>
*/

/**
Expand Down Expand Up @@ -41,10 +41,6 @@ bool ReactNativeFeatureFlags::enableCustomDrawOrderFabric() {
return getAccessor().enableCustomDrawOrderFabric();
}

bool ReactNativeFeatureFlags::enableFixForClippedSubviewsCrash() {
return getAccessor().enableFixForClippedSubviewsCrash();
}

bool ReactNativeFeatureFlags::enableMicrotasks() {
return getAccessor().enableMicrotasks();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<881b149aca14f4f73e45e02089787a88>>
* @generated SignedSource<<e9fbc07cb8f1c50e015e34035d50b8d1>>
*/

/**
Expand Down Expand Up @@ -62,11 +62,6 @@ class ReactNativeFeatureFlags {
*/
RN_EXPORT static bool enableCustomDrawOrderFabric();

/**
* Attempt at fixing a crash related to subview clipping on Android. This is a kill switch for the fix
*/
RN_EXPORT static bool enableFixForClippedSubviewsCrash();

/**
* Enables the use of microtasks in Hermes (scheduling) and RuntimeScheduler (execution).
*/
Expand Down
Loading

0 comments on commit 3e73dfb

Please sign in to comment.