Remove navigation listeners before clearing NavigationEngineFactory (#…

…1140)
mapbox · Jul 23, 2018 · 37349a9 · 37349a9
1 parent 3a951eb
commit 37349a9
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 9 deletions.
diff --git a/...on-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java b/...on-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/.../src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java b/.../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/...in/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactory.java b/...in/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/.../src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java b/.../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/.../test/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigationTest.java b/.../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/...ava/com/mapbox/services/android/navigation/v5/navigation/NavigationEngineFactoryTest.java b/...ava/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());
+  }
 }