- LayoutAnimations cause invalid view operations

Summary:
[Android] [Fixed] - LayoutAnimations cause invalid view operations

The dating team has found a consistent repro where following an order of steps will get you the following exception:

https://lookaside.facebook.com/intern/pixelcloudnew/asset/?id=2113362972287761

The exception is due to the fact that the following operation
 `delete child tag 17 from parent tag 481 which is located at index 2`
cannot be performed because parent tag 481 only has 2 children.. and they also happen to not have the tag 17 as a child. Somehow the operations and the tree they act upon are out of sync.

RN receives operations from React via the native module `UIManagerModule`. The operations use tags (an identifier for a view) and indices to determine what view is updated. These operations (grouped together as a batch) are then passed to the UI thread where we perform them on the platform views.

LayoutAnimations are implemented per batch in RN. When LayoutAnimations are on, qualified view updates are animated. Because the delete operation is animated, RN doesn't remove the view from the platform view hierarchy immediately but asynchronously -- after the animation is complete. This is problematic for other view operations that rely on an index on where to insert or delete a view because during the creation of those operations, it was assumed all prior operations were performed synchronously.

This is what was occurring in the repro and there were silent view errors occurring before the exception was thrown.

This diff proposes a solution to track all pending delete operations across operations.
We introduce a map that is keyed on view tags and has a value of a sparse array that represents child index -> count of views that are pending deletion.
`{11: [0=1], 481: [2=1]}` where this would be interpreted as for index 0 under view tag 11, there is one async view deletion that has not completed and under tag 481 there is one async view deletion at index 2.

We use the map to adjust indices on add / delete operations. We also update the count when the deletion animation is complete and remove the tag from the map when it is deleted.

Regarding worst case sizing of the map => O(# of platform views rendered)

Reviewed By: mdvacca

Differential Revision: D14529038

fbshipit-source-id: 86d8982845e25a2b23d6d89ce27852fd77dbb060

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java

@@ -11,6 +11,7 @@
 import android.os.Build;
 import android.util.SparseArray;
 import android.util.SparseBooleanArray;
+import android.util.SparseIntArray;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
@@ -35,6 +36,8 @@
 import com.facebook.react.uimanager.layoutanimation.LayoutAnimationListener;
 import com.facebook.systrace.Systrace;
 import com.facebook.systrace.SystraceMessage;
+import java.util.HashMap;
+import java.util.Map;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
@@ -73,6 +76,7 @@ public class NativeViewHierarchyManager {
   private final JSResponderHandler mJSResponderHandler = new JSResponderHandler();
   private final RootViewManager mRootViewManager;
   private final LayoutAnimationController mLayoutAnimator = new LayoutAnimationController();
+  private final Map<Integer, SparseIntArray> mTagsToPendingIndicesToDelete = new HashMap<>();
 
   private boolean mLayoutAnimationEnabled;
   private PopupMenu mPopupMenu;
@@ -336,19 +340,49 @@ private static String constructManageChildrenErrorMessage(
     return stringBuilder.toString();
   }
 
+  /**
+   * Given an index to action on under synchronous deletes, return an updated index factoring in
+   * asynchronous deletes (where the async delete operations have not yet been performed)
+   */
+  private int normalizeIndex(int index, SparseIntArray pendingIndices) {
+    int normalizedIndex = index;
+    for (int i = 0; i <= index; i++) {
+      normalizedIndex += pendingIndices.get(i);
+    }
+    return normalizedIndex;
+  }
+
+  /**
+   * Given React tag, return sparse array of direct child indices that are pending deletion (due to
+   * async view deletion)
+   */
+  private SparseIntArray getOrCreatePendingIndicesToDelete(int tag) {
+    SparseIntArray pendingIndicesToDelete = mTagsToPendingIndicesToDelete.get(tag);
+    if (pendingIndicesToDelete ==  null) {
+      pendingIndicesToDelete = new SparseIntArray();
+      mTagsToPendingIndicesToDelete.put(tag, pendingIndicesToDelete);
+    }
+    return pendingIndicesToDelete;
+  }
+
   /**
    * @param tag react tag of the node we want to manage
    * @param indicesToRemove ordered (asc) list of indicies at which view should be removed
    * @param viewsToAdd ordered (asc based on mIndex property) list of tag-index pairs that represent
    * a view which should be added at the specified index
    * @param tagsToDelete list of tags corresponding to views that should be removed
+   * @param indicesToDelete parallel list to tagsToDelete, list of indices of those tags
    */
   public synchronized void manageChildren(
       int tag,
       @Nullable int[] indicesToRemove,
       @Nullable ViewAtIndex[] viewsToAdd,
-      @Nullable int[] tagsToDelete) {
+      @Nullable int[] tagsToDelete,
+      @Nullable int[] indicesToDelete) {
     UiThreadUtil.assertOnUiThread();
+
+    final SparseIntArray pendingIndicesToDelete = getOrCreatePendingIndicesToDelete(tag);
+
     final ViewGroup viewToManage = (ViewGroup) mTagsToViews.get(tag);
     final ViewGroupManager viewManager = (ViewGroupManager) resolveViewManager(tag);
     if (viewToManage == null) {
@@ -405,15 +439,16 @@ public synchronized void manageChildren(
                       tagsToDelete));
         }
 
-        View viewToRemove = viewManager.getChildAt(viewToManage, indexToRemove);
+        int normalizedIndexToRemove = normalizeIndex(indexToRemove, pendingIndicesToDelete);
+        View viewToRemove = viewManager.getChildAt(viewToManage, normalizedIndexToRemove);
 
         if (mLayoutAnimationEnabled &&
             mLayoutAnimator.shouldAnimateLayout(viewToRemove) &&
             arrayContains(tagsToDelete, viewToRemove.getId())) {
           // The view will be removed and dropped by the 'delete' layout animation
           // instead, so do nothing
         } else {
-          viewManager.removeViewAt(viewToManage, indexToRemove);
+          viewManager.removeViewAt(viewToManage, normalizedIndexToRemove);
         }
 
         lastIndexToRemove = indexToRemove;
@@ -435,13 +470,15 @@ public synchronized void manageChildren(
                       viewsToAdd,
                       tagsToDelete));
         }
-        viewManager.addView(viewToManage, viewToAdd, viewAtIndex.mIndex);
+        int normalizedIndexToAdd = normalizeIndex(viewAtIndex.mIndex, pendingIndicesToDelete);
+        viewManager.addView(viewToManage, viewToAdd, normalizedIndexToAdd);
       }
     }
 
     if (tagsToDelete != null) {
       for (int i = 0; i < tagsToDelete.length; i++) {
         int tagToDelete = tagsToDelete[i];
+        final int indexToDelete = indicesToDelete[i];
         final View viewToDestroy = mTagsToViews.get(tagToDelete);
         if (viewToDestroy == null) {
           throw new IllegalViewOperationException(
@@ -457,13 +494,20 @@ public synchronized void manageChildren(
 
         if (mLayoutAnimationEnabled &&
             mLayoutAnimator.shouldAnimateLayout(viewToDestroy)) {
-          mLayoutAnimator.deleteView(viewToDestroy, new LayoutAnimationListener() {
-            @Override
-            public void onAnimationEnd() {
-              viewManager.removeView(viewToManage, viewToDestroy);
-              dropView(viewToDestroy);
-            }
-          });
+          int updatedCount = pendingIndicesToDelete.get(indexToDelete, 0) + 1;
+          pendingIndicesToDelete.put(indexToDelete, updatedCount);
+          mLayoutAnimator.deleteView(
+              viewToDestroy,
+              new LayoutAnimationListener() {
+                @Override
+                public void onAnimationEnd() {
+                  viewManager.removeView(viewToManage, viewToDestroy);
+                  dropView(viewToDestroy);
+
+                  int count = pendingIndicesToDelete.get(indexToDelete, 0);
+                  pendingIndicesToDelete.put(indexToDelete, Math.max(0, count - 1));
+                }
+              });
         } else {
           dropView(viewToDestroy);
         }
@@ -580,6 +624,7 @@ protected synchronized void dropView(View view) {
       }
       viewGroupManager.removeAllViews(viewGroup);
     }
+    mTagsToPendingIndicesToDelete.remove(view.getId());
     mTagsToViews.remove(view.getId());
     mTagsToViewManagers.remove(view.getId());
   }

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyOptimizer.java

@@ -145,13 +145,15 @@ public void handleManageChildren(
       int[] indicesToRemove,
       int[] tagsToRemove,
       ViewAtIndex[] viewsToAdd,
-      int[] tagsToDelete) {
+      int[] tagsToDelete,
+      int[] indicesToDelete) {
     if (!ENABLED) {
       mUIViewOperationQueue.enqueueManageChildren(
           nodeToManage.getReactTag(),
           indicesToRemove,
           viewsToAdd,
-          tagsToDelete);
+          tagsToDelete,
+          indicesToDelete);
       return;
     }
 
@@ -276,9 +278,10 @@ private void removeNodeFromParent(ReactShadowNode nodeToRemove, boolean shouldDe
 
       mUIViewOperationQueue.enqueueManageChildren(
           nativeNodeToRemoveFrom.getReactTag(),
-          new int[]{index},
+          new int[] {index},
           null,
-          shouldDelete ? new int[]{nodeToRemove.getReactTag()} : null);
+          shouldDelete ? new int[] {nodeToRemove.getReactTag()} : null,
+          shouldDelete ? new int[] {index} : null);
     } else {
       for (int i = nodeToRemove.getChildCount() - 1; i >= 0; i--) {
         removeNodeFromParent(nodeToRemove.getChildAt(i), shouldDelete);
@@ -301,7 +304,8 @@ private void addNonLayoutNode(
     mUIViewOperationQueue.enqueueManageChildren(
         parent.getReactTag(),
         null,
-        new ViewAtIndex[]{new ViewAtIndex(child.getReactTag(), index)},
+        new ViewAtIndex[] {new ViewAtIndex(child.getReactTag(), index)},
+        null,
         null);
   }
 

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java

@@ -347,6 +347,7 @@ public void manageChildren(
     int[] indicesToRemove = new int[numToMove + numToRemove];
     int[] tagsToRemove = new int[indicesToRemove.length];
     int[] tagsToDelete = new int[numToRemove];
+    int[] indicesToDelete = new int[numToRemove];
 
     if (numToMove > 0) {
       Assertions.assertNotNull(moveFrom);
@@ -380,6 +381,7 @@ public void manageChildren(
         indicesToRemove[numToMove + i] = indexToRemove;
         tagsToRemove[numToMove + i] = tagToRemove;
         tagsToDelete[i] = tagToRemove;
+        indicesToDelete[i] = indexToRemove;
       }
     }
 
@@ -423,7 +425,8 @@ public void manageChildren(
           indicesToRemove,
           tagsToRemove,
           viewsToAdd,
-          tagsToDelete);
+          tagsToDelete,
+          indicesToDelete);
     }
 
     for (int i = 0; i < tagsToDelete.length; i++) {

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java

@@ -210,16 +210,19 @@ private final class ManageChildrenOperation extends ViewOperation {
     private final @Nullable int[] mIndicesToRemove;
     private final @Nullable ViewAtIndex[] mViewsToAdd;
     private final @Nullable int[] mTagsToDelete;
+    private final @Nullable int[] mIndicesToDelete;
 
     public ManageChildrenOperation(
         int tag,
         @Nullable int[] indicesToRemove,
         @Nullable ViewAtIndex[] viewsToAdd,
-        @Nullable int[] tagsToDelete) {
+        @Nullable int[] tagsToDelete,
+        @Nullable int[] indicesToDelete) {
       super(tag);
       mIndicesToRemove = indicesToRemove;
       mViewsToAdd = viewsToAdd;
       mTagsToDelete = tagsToDelete;
+      mIndicesToDelete = indicesToDelete;
     }
 
     @Override
@@ -228,7 +231,8 @@ public void execute() {
           mTag,
           mIndicesToRemove,
           mViewsToAdd,
-          mTagsToDelete);
+          mTagsToDelete,
+          mIndicesToDelete);
     }
   }
 
@@ -778,9 +782,10 @@ public void enqueueManageChildren(
       int reactTag,
       @Nullable int[] indicesToRemove,
       @Nullable ViewAtIndex[] viewsToAdd,
-      @Nullable int[] tagsToDelete) {
+      @Nullable int[] tagsToDelete,
+      @Nullable int[] indicesToDelete) {
     mOperations.add(
-        new ManageChildrenOperation(reactTag, indicesToRemove, viewsToAdd, tagsToDelete));
+        new ManageChildrenOperation(reactTag, indicesToRemove, viewsToAdd, tagsToDelete, indicesToDelete));
   }
 
   public void enqueueSetChildren(