From e3711407a14ac37a592537c6704c86c21f8a29fe Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Thu, 27 Aug 2020 19:34:40 -0700
Subject: [PATCH] Noop when removing views from empty parent

Summary:
In some cases (BottomSheet?) the parent/ViewManager removes all children of the View before Fabric gets a chance to remove the children.

Apparently prior to D23368229 (https://github.com/facebook/react-native/commit/d344fb4e29a827d1e7d233672a3efe3b2b981a8a) (landed just today!) this sequence of operations happened and just noop'ed. Since we've been doing that happily as long as Fabric
has existed, we'll keep doing that for now.

I suspect that on *some* versions of Android this crashes and others it doesn't, based on logviews and my inability to repro certain crashes.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D23387044

fbshipit-source-id: 88a46191adef4f6816bd7babd9103d103ddcef33
---
 .../fabric/mounting/MountingManager.java      | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java
index a62cf858c2b..4629668d494 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java
@@ -369,13 +369,35 @@ public class MountingManager {
     int actualTag = (view != null ? view.getId() : -1);
     if (actualTag != tag) {
       int tagActualIndex = -1;
-      for (int i = 0; i < parentView.getChildCount(); i++) {
+      int parentChildrenCount = parentView.getChildCount();
+      for (int i = 0; i < parentChildrenCount; i++) {
         if (parentView.getChildAt(i).getId() == tag) {
           tagActualIndex = i;
           break;
         }
       }
 
+      // TODO T74425739: previously, we did not do this check and `removeViewAt` would be executed
+      // below, sometimes crashing there. *However*, interestingly enough, `removeViewAt` would not
+      // complain if you removed views from an already-empty parent. This seems necessary currently
+      // for certain ViewManagers that remove their own children - like BottomSheet?
+      // This workaround seems not-great, but for now, we just return here for
+      // backwards-compatibility. Essentially, if a view has already been removed from the
+      // hierarchy, we treat it as a noop.
+      if (tagActualIndex == -1) {
+        FLog.e(
+            TAG,
+            "removeViewAt: ["
+                + tag
+                + "] -> ["
+                + parentTag
+                + "] @"
+                + index
+                + ": view already removed from parent! Children in parent: "
+                + parentChildrenCount);
+        return;
+      }
+
       throw new IllegalStateException(
           "Tried to delete view ["
               + tag