From 37349a9ac96a1c04e32093f6efb7b821a28a72ea Mon Sep 17 00:00:00 2001 From: Dan Nesfeder Date: Mon, 23 Jul 2018 11:52:03 -0400 Subject: [PATCH] Remove navigation listeners before clearing NavigationEngineFactory (#1140) --- .../navigation/ui/v5/NavigationViewModel.java | 2 +- .../v5/navigation/MapboxNavigation.java | 7 ++-- .../navigation/NavigationEngineFactory.java | 12 +++++++ .../v5/navigation/NavigationHelper.java | 2 +- .../v5/navigation/MapboxNavigationTest.java | 5 ++- .../NavigationEngineFactoryTest.java | 36 +++++++++++++++++++ 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java index fc8dc172bcc..5d06ad94406 100644 --- a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java +++ b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java @@ -94,8 +94,8 @@ public void onCreate() { public void onDestroy(boolean isChangingConfigurations) { if (!isChangingConfigurations) { locationEngineConductor.onDestroy(); - endNavigation(); deactivateInstructionPlayer(); + endNavigation(); } navigationViewEventDispatcher = null; } diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java index 6f17d051adc..2b73d340389 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java @@ -206,14 +206,13 @@ private void disableLocationEngine() { * also removes all listeners that have been attached. */ public void onDestroy() { - Timber.d("MapboxNavigation onDestroy."); stopNavigation(); disableLocationEngine(); - navigationEngineFactory.clearEngines(); - removeNavigationEventListener(null); + removeOffRouteListener(null); removeProgressChangeListener(null); removeMilestoneEventListener(null); - removeOffRouteListener(null); + removeNavigationEventListener(null); + navigationEngineFactory.clearEngines(); } // Public APIs diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactory.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactory.java index acd4bf48b14..4f493d864f6 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactory.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactory.java @@ -25,6 +25,9 @@ OffRoute retrieveOffRouteEngine() { } void updateOffRouteEngine(OffRoute offRouteEngine) { + if (offRouteEngine == null) { + return; + } this.offRouteEngine = offRouteEngine; } @@ -33,6 +36,9 @@ FasterRoute retrieveFasterRouteEngine() { } void updateFasterRouteEngine(FasterRoute fasterRouteEngine) { + if (fasterRouteEngine == null) { + return; + } this.fasterRouteEngine = fasterRouteEngine; } @@ -41,6 +47,9 @@ Snap retrieveSnapEngine() { } void updateSnapEngine(Snap snapEngine) { + if (snapEngine == null) { + return; + } this.snapEngine = snapEngine; } @@ -49,6 +58,9 @@ Camera retrieveCameraEngine() { } void updateCameraEngine(Camera cameraEngine) { + if (cameraEngine == null) { + return; + } this.cameraEngine = cameraEngine; } diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java index 589080c2503..dcb84ab9f94 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java @@ -472,9 +472,9 @@ static boolean isUserOffRoute(NavigationLocationUpdate navigationLocationUpdate, if (!options.enableOffRouteDetection()) { return false; } - Location location = navigationLocationUpdate.location(); OffRoute offRoute = navigationLocationUpdate.mapboxNavigation().getOffRouteEngine(); setOffRouteDetectorCallback(offRoute, callback); + Location location = navigationLocationUpdate.location(); return offRoute.isUserOffRoute(location, routeProgress, options); } diff --git a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigationTest.java b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigationTest.java index ce69ad4e90b..1f1bc6ab4df 100644 --- a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigationTest.java +++ b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigationTest.java @@ -10,7 +10,6 @@ import com.mapbox.services.android.navigation.v5.milestone.VoiceInstructionMilestone; import com.mapbox.services.android.navigation.v5.navigation.camera.SimpleCamera; import com.mapbox.services.android.navigation.v5.offroute.OffRoute; -import com.mapbox.services.android.navigation.v5.offroute.OffRouteDetector; import com.mapbox.services.android.navigation.v5.snap.Snap; import com.mapbox.services.android.navigation.v5.snap.SnapToRoute; @@ -246,7 +245,7 @@ public void setOffRouteEngine_doesReplaceDefaultEngine() throws Exception { OffRoute offRoute = mock(OffRoute.class); navigation.setOffRouteEngine(offRoute); - assertTrue(!(navigation.getOffRouteEngine() instanceof OffRouteDetector)); + assertEquals(offRoute, navigation.getOffRouteEngine()); } @Test @@ -262,7 +261,7 @@ public void getCameraEngine_returnsNonNullEngine() throws Exception { public void getCameraEngine_returnsSimpleCameraWhenNull() throws Exception { MapboxNavigation navigation = buildMapboxNavigation(); - navigation.setOffRouteEngine(null); + navigation.setCameraEngine(null); assertTrue(navigation.getCameraEngine() instanceof SimpleCamera); } diff --git a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactoryTest.java b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactoryTest.java index 2e8d4f6522a..72423a8a294 100644 --- a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactoryTest.java +++ b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactoryTest.java @@ -70,4 +70,40 @@ public void clearEngines_fasterRouteEngineIsRemoved() { assertNull(provider.retrieveFasterRouteEngine()); } + + @Test + public void updateFasterRouteEngine_ignoresNull() { + NavigationEngineFactory provider = new NavigationEngineFactory(); + + provider.updateFasterRouteEngine(null); + + assertNotNull(provider.retrieveFasterRouteEngine()); + } + + @Test + public void updateOffRouteEngine_ignoresNull() { + NavigationEngineFactory provider = new NavigationEngineFactory(); + + provider.updateOffRouteEngine(null); + + assertNotNull(provider.retrieveOffRouteEngine()); + } + + @Test + public void updateCameraEngine_ignoresNull() { + NavigationEngineFactory provider = new NavigationEngineFactory(); + + provider.updateCameraEngine(null); + + assertNotNull(provider.retrieveCameraEngine()); + } + + @Test + public void updateSnapEngine_ignoresNull() { + NavigationEngineFactory provider = new NavigationEngineFactory(); + + provider.updateSnapEngine(null); + + assertNotNull(provider.retrieveSnapEngine()); + } } \ No newline at end of file