From 0b560d3d3db72559f1e842bc4f3f521d8abd9467 Mon Sep 17 00:00:00 2001
From: Denis Koroskin <dkoroskin@fb.com>
Date: Mon, 29 Feb 2016 19:27:09 -0800
Subject: [PATCH] Don't try to remove Views that about to be removed with the
 root View

Summary:
We keep a list of FlatShadowNodes that mount to Views that we want to delete, and only flush it at the end of an update cycle. This results in a situation where a root view is being removed before children are removed, which results in a crash in NativeViewHierarchyManager because a View that we are trying to remove no longer exists. There are a few approaches to fix the issue:
a) make a check if a View exists before removing it. While works, it removes a bug protection when we erroneously trying to remove a View that no longer exists (such as this). I'd prefer to keep the check in place
b) flush the views-to-drop queue. This works, but does some extra work in UI thread (namely, removing Views that would be removed anyway)
c) trim the views-to-drop queue to remove any Views that will be removed anyway. This does a tiny bit of extra work in BG thread, but less work in UI thread.

This diff implements option c).

Reviewed By: ahmedre

Differential Revision: D2990105
---
 .../com/facebook/react/flat/FlatUIImplementation.java    | 6 ++++++
 .../main/java/com/facebook/react/flat/StateBuilder.java  | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java
index 969e53e2321..b7b30b50b22 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java
@@ -389,6 +389,12 @@ public class FlatUIImplementation extends UIImplementation {
     mStateBuilder.applyUpdates(eventDispatcher, (FlatRootShadowNode) cssNode);
   }
 
+  @Override
+  public void removeRootView(int rootViewTag) {
+    mStateBuilder.removeRootView(rootViewTag);
+    super.removeRootView(rootViewTag);
+  }
+
   @Override
   public void setJSResponder(int possiblyVirtualReactTag, boolean blockNativeResponder) {
     ReactShadowNode node = resolveShadowNode(possiblyVirtualReactTag);
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
index dee6124cc7e..b29e5e75d50 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
@@ -116,6 +116,15 @@ import com.facebook.react.uimanager.events.EventDispatcher;
     mOperationsQueue.enqueueProcessLayoutRequests();
   }
 
+  /* package */ void removeRootView(int rootViewTag) {
+    // Don't remove Views that are connected to a View that we are about to remove.
+    for (int i = mViewsToDrop.size() - 1; i >= 0; --i) {
+      if (mViewsToDrop.get(i).getRootNode().getReactTag() == rootViewTag) {
+        mViewsToDrop.remove(i);
+      }
+    }
+  }
+
   /**
    * Adds a DrawCommand for current mountable node.
    */