From 47b7d2c7c54af861e99b922aa258489d86c9c0b2 Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Sun, 10 Mar 2019 12:02:21 +0200 Subject: [PATCH] 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 --- .../anim/NavigationAnimator.java | 17 +++++++- .../stack/StackController.java | 3 +- .../stack/StackControllerTest.java | 42 +++++++++++++++++-- 3 files changed, 57 insertions(+), 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 index e8789ef1f7f..c2dbfeab528 100644 --- 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/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java b/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java index 740dd4576da..1473d402311 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java +++ b/lib/android/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 iterator = stack.iterator(); iterator.next(); while (stack.size() > 2) { diff --git a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java index b46d28555ac..c97349a800f 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java +++ b/lib/android/app/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();