Cancel in-flight push animation on pop (#4842)

When popToRoot or popTo where called while a screen was pushed, at the end of the push animation RNN tried to destroy an already destroyed ViewController. Related to #3767
wix · Mar 10, 2019 · 47b7d2c · 47b7d2c
1 parent 5775096
commit 47b7d2c
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 5 deletions.
diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/anim/NavigationAnimator.java b/lib/android/app/src/main/java/com/reactnativenavigation/anim/NavigationAnimator.java
@@ -4,6 +4,7 @@
 import android.animation.AnimatorListenerAdapter;
 import android.animation.AnimatorSet;
 import android.content.Context;
+import android.support.annotation.RestrictTo;
 import android.view.View;
 import android.view.ViewGroup;
 
@@ -19,7 +20,7 @@
 import java.util.List;
 import java.util.Map;
 
-import static com.reactnativenavigation.utils.CollectionUtils.merge;
+import static com.reactnativenavigation.utils.CollectionUtils.*;
 
 @SuppressWarnings("ResourceType")
 public class NavigationAnimator extends BaseAnimator {
@@ -108,4 +109,18 @@ public void onAnimationEnd(Animator animation) {
         });
         set.start();
     }
+
+    public void cancelPushAnimations() {
+        for (View view : runningPushAnimations.keySet()) {
+            runningPushAnimations.get(view).cancel();
+            runningPushAnimations.remove(view);
+        }
+    }
+
+    @RestrictTo(RestrictTo.Scope.TESTS)
+    public void endPushAnimation(View view) {
+        if (runningPushAnimations.containsKey(view)) {
+            runningPushAnimations.get(view).end();
+        }
+    }
 }
diff --git a/...id/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java b/...id/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java
@@ -275,7 +275,7 @@ public void popTo(ViewController viewController, Options mergeOptions, CommandLi
             return;
         }
 
-
+        animator.cancelPushAnimations();
         String currentControlId;
         for (int i = stack.size() - 2; i >= 0; i--) {
             currentControlId = stack.get(i).getId();
@@ -297,6 +297,7 @@ public void popToRoot(Options mergeOptions, CommandListener listener) {
             return;
         }
 
+        animator.cancelPushAnimations();
         Iterator<String> iterator = stack.iterator();
         iterator.next();
         while (stack.size() > 2) {

diff --git a/...pp/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java b/...pp/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java
@@ -42,9 +42,7 @@
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.junit.Test;
-import org.mockito.ArgumentCaptor;
-import org.mockito.InOrder;
-import org.mockito.Mockito;
+import org.mockito.*;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -297,6 +295,18 @@ public void onSuccess(String childId) {
         });
     }
 
+    @Test
+    public void pop_screenCurrentlyBeingPushedIsPopped() {
+        disablePushAnimation(child1, child2);
+        uut.push(child1, Mockito.mock(CommandListenerAdapter.class));
+        uut.push(child2, Mockito.mock(CommandListenerAdapter.class));
+
+        uut.push(child3, Mockito.mock(CommandListenerAdapter.class));
+        uut.pop(Options.EMPTY, Mockito.mock(CommandListenerAdapter.class));
+        assertThat(uut.size()).isEqualTo(2);
+        assertContainsOnlyId(child1.getId(), child2.getId());
+    }
+
     @Test
     public void pop_appliesOptionsAfterPop() {
         uut.push(child1, new CommandListenerAdapter());
@@ -622,6 +632,19 @@ public void onSuccess(String childId) {
         });
     }
 
+    @Test
+    public void popTo_pushAnimationIsCancelled() {
+        disablePushAnimation(child1, child2);
+        uut.push(child1, Mockito.mock(CommandListenerAdapter.class));
+        uut.push(child2, Mockito.mock(CommandListenerAdapter.class));
+
+        ViewGroup pushed = child3.getView();
+        uut.push(child3, Mockito.mock(CommandListenerAdapter.class));
+        uut.popTo(child1, Options.EMPTY, Mockito.mock(CommandListenerAdapter.class));
+        animator.endPushAnimation(pushed);
+        assertContainsOnlyId(child1.getId());
+    }
+
     @Test
     public void popToRoot_PopsEverythingAboveFirstController() {
         child1.options.animations.push.enabled = new Bool(false);
@@ -707,6 +730,19 @@ public void popToRoot_optionsAreMergedOnTopChild() {
         verify(child1, times(0)).mergeOptions(mergeOptions);
     }
 
+    @Test
+    public void popToRoot_screenPushedBeforePopAnimationCompletesIsPopped() {
+        disablePushAnimation(child1, child2);
+        uut.push(child1, Mockito.mock(CommandListenerAdapter.class));
+        uut.push(child2, Mockito.mock(CommandListenerAdapter.class));
+
+        ViewGroup pushed = child3.getView();
+        uut.push(child3, Mockito.mock(CommandListenerAdapter.class));
+        uut.popToRoot(Options.EMPTY, Mockito.mock(CommandListenerAdapter.class));
+        animator.endPushAnimation(pushed);
+        assertContainsOnlyId(child1.getId());
+    }
+
     @Test
     public void findControllerById_ReturnsSelfOrChildrenById() {
         assertThat(uut.findController("123")).isNull();