From c38b506d7a6609559c70cb3951d62c53c4d78d29 Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Thu, 23 Jul 2020 19:48:37 -0700
Subject: [PATCH] LayoutAnimations: fix index consistency crash

Summary:
Unfortunately LayoutAnimations index adjustment is a bit tricky. There was one (hopefully only one!) remaining issue where a specific series of delayed removes, inserts, and removes would trip up index adjustment. Namely in this case: Preexisting delayed removes, and then a later animated commit with immediate inserts followed by higher-index removes.

This case is now resolved and I took time to test and verify other index adjustment paths.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22709915

fbshipit-source-id: 16ba5bb358608d384f44628dbb6cc691deb8164a
---
 .../LayoutAnimationKeyFrameManager.cpp        | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp
index 9d97fb1d533e41..6da7ddcdf890a9 100644
--- a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp
+++ b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp
@@ -600,7 +600,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
             mutation.type == ShadowViewMutation::Type::Insert) {
           // Indices for immediate INSERT mutations must be adjusted to insert
           // at higher indices if previous animations have deferred removals
-          // before the insertion indect
+          // before the insertion index
           // TODO: refactor to reduce code duplication
           if (mutation.type == ShadowViewMutation::Type::Insert) {
             int adjustedIndex = mutation.index;
@@ -766,12 +766,14 @@ LayoutAnimationKeyFrameManager::pullTransaction(
             // tree hierarchy).
             {
               int adjustedIndex = mutation.index;
+              int adjustment = 0;
               for (auto &otherMutation : mutations) {
                 if (otherMutation.type == ShadowViewMutation::Type::Insert &&
-                    otherMutation.parentShadowView.tag == parentTag) {
-                  if (otherMutation.index <= adjustedIndex &&
-                      !mutatedViewIsVirtual(otherMutation)) {
+                    otherMutation.parentShadowView.tag == parentTag &&
+                    !mutatedViewIsVirtual(otherMutation)) {
+                  if (otherMutation.index <= adjustedIndex) {
                     adjustedIndex++;
+                    adjustment++;
                   } else {
                     // If we are delaying this remove instruction, conversely,
                     // we must adjust upward the insertion index of any INSERT
@@ -802,9 +804,16 @@ LayoutAnimationKeyFrameManager::pullTransaction(
                   }
                   const auto &delayedFinalMutation =
                       *animatedKeyFrame.finalMutationForKeyFrame;
+
+                  // Note: we add the "adjustment" we've accumulated to the
+                  // `delayedFinalMutation.index` before comparing. Since
+                  // "adjustment" is caused by Insert MountItems that we are
+                  // about to execute, but haven't yet, the delayed mutation's
+                  // index *will* be adjusted right after this.
                   if (delayedFinalMutation.type ==
                           ShadowViewMutation::Type::Remove &&
-                      delayedFinalMutation.index <= adjustedIndex) {
+                      (delayedFinalMutation.index + adjustment) <=
+                          adjustedIndex) {
                     adjustedIndex++;
                   }
                 }