mapbox / mapbox-navigation-android

Mapbox Navigation SDK for Android
https://docs.mapbox.com/android/navigation/overview/
Other
621 stars 318 forks source link

Crash when changing map style during navigation in 0.42.4 #2305

Closed Ranafugaz closed 4 years ago

Ranafugaz commented 4 years ago

Android API: 28 Mapbox Navigation SDK version: 0.42.4

In the last version 0.42.4 when the map style is changed during navigation the app crashes. The version 0.42.3 works fine.

Steps to trigger behavior

  1. Change map style during navigation Code:
    navigationMap.addProgressChangeListener(this);
    ...
    @Override
    public void onProgressChange(Location location, RouteProgress routeProgress) {
        navigationMap.retrieveMap().setStyle(Style.TRAFFIC_NIGHT, new Style.OnStyleLoaded() {
            @Override
            public void onStyleLoaded(@NonNull Style style) { //TODO takeout
            }
        });
    }

Expected behavior

v0.42.3

ezgif com-video-to-gif (3)

Actual behavior: Crash

v0.42.4 ezgif com-video-to-gif (2)

Guardiola31337 commented 4 years ago

Hey @Ranafugaz 👋 thanks for reaching out and report your issue.

I was able to reproduce using the EmbeddedNavigationActivity from the test app and I got the following native 💥

12-12 14:52:01.235 200-200/? A/DEBUG: *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
12-12 14:52:01.235 200-200/? A/DEBUG: Build fingerprint: 'google/hammerhead/hammerhead:6.0.1/M4B30Z/3437181:user/release-keys'
12-12 14:52:01.235 200-200/? A/DEBUG: Revision: '11'
12-12 14:52:01.235 200-200/? A/DEBUG: ABI: 'arm'
12-12 14:52:01.235 200-200/? A/DEBUG: pid: 16139, tid: 16139, name: .navigation.app  >>> com.mapbox.services.android.navigation.app <<<
12-12 14:52:01.235 200-200/? A/DEBUG: signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x62
12-12 14:52:01.268 200-200/? A/DEBUG:     r0 00000002  r1 00000000  r2 00000000  r3 bb5ae5a8
12-12 14:52:01.268 200-200/? A/DEBUG:     r4 b4dd6a80  r5 1342b6a0  r6 b6d9bec0  r7 bef55068
12-12 14:52:01.268 200-200/? A/DEBUG:     r8 1339fdc0  r9 b4df6500  sl 12cc4510  fp 00000000
12-12 14:52:01.268 200-200/? A/DEBUG:     ip b4d3c94c  sp bef55030  lr b49ddc29  pc 9f9ae90a  cpsr 600b0030
12-12 14:52:01.285 200-200/? A/DEBUG: backtrace:
12-12 14:52:01.285 200-200/? A/DEBUG:     #00 pc 0006990a  /data/app/com.mapbox.services.android.navigation.app-1/lib/arm/libmapbox-gl.so
12-12 14:52:01.285 200-200/? A/DEBUG:     #01 pc 014c70a9  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (java.lang.Object com.mapbox.mapboxsdk.style.layers.Layer.nativeGetVisibility()+76)
12-12 14:52:01.285 200-200/? A/DEBUG:     #02 pc 014c6ba9  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (com.mapbox.mapboxsdk.style.layers.PropertyValue com.mapbox.mapboxsdk.style.layers.Layer.getVisibility()+84)
12-12 14:52:01.285 200-200/? A/DEBUG:     #03 pc 021f2d6d  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.ui.v5.route.MapRouteArrow.updateVisibilityTo(boolean)+488)
12-12 14:52:01.285 200-200/? A/DEBUG:     #04 pc 021f29d9  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.ui.v5.route.MapRouteArrow.addUpcomingManeuverArrow(com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress)+948)
12-12 14:52:01.285 200-200/? A/DEBUG:     #05 pc 02202f01  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.ui.v5.route.MapRouteProgressChangeListener.onProgressChange(android.location.Location, com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress)+500)
12-12 14:52:01.286 200-200/? A/DEBUG:     #06 pc 02274acd  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.v5.internal.navigation.NavigationEventDispatcher.onProgressChange(android.location.Location, com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress)+600)
12-12 14:52:01.286 200-200/? A/DEBUG:     #07 pc 0234093f  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.v5.internal.navigation.RouteProcessorThreadListener.onNewRouteProgress(android.location.Location, com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress)+378)
12-12 14:52:01.286 200-200/? A/DEBUG:     #08 pc 0228cf73  /data/app/com.mapbox.services.android.navigation.app-1/oat/arm/base.odex (offset 0xf22000) (void com.mapbox.services.android.navigation.v5.internal.navigation.RouteProcessorRunnable$sendUpdateToResponseHandler$1.run()+150)
12-12 14:52:01.286 200-200/? A/DEBUG:     #09 pc 7364653f  /data/dalvik-cache/arm/system@framework@boot.oat (offset 0x1ed6000)

    --------- beginning of system

In https://github.com/mapbox/mapbox-navigation-android/pull/2262 we made some changes in MapRouteLine to persist the route lines across style changes but we didn't adapt MapRouteArrow (also part of NavigationMapRoute). We should also persist the upcoming maneuver arrow across style changes to avoid this issue.

Attaching a diff with the changes needed to reproduce the crash

diff --git a/app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationApplication.kt b/app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationApplication.kt
index 3ebbd578..2a3df563 100644
--- a/app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationApplication.kt
+++ b/app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationApplication.kt
@@ -27,7 +27,7 @@ class NavigationApplication : MultiDexApplication() {
         setupStrictMode()
         setupCanary()
         setupMapbox()
-        setupCrashMonitor()
+        // setupCrashMonitor()
     }

     private fun setupTimber() {
diff --git a/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/navigationui/EmbeddedNavigationActivity.java b/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/navigationui/EmbeddedNavigationActivity.java
index 8a150484..d2008d26 100644
--- a/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/navigationui/EmbeddedNavigationActivity.java
+++ b/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/navigationui/EmbeddedNavigationActivity.java
@@ -28,6 +28,7 @@ import com.mapbox.geojson.Point;
 import com.mapbox.mapboxsdk.Mapbox;
 import com.mapbox.mapboxsdk.camera.CameraPosition;
 import com.mapbox.mapboxsdk.geometry.LatLng;
+import com.mapbox.mapboxsdk.maps.Style;
 import com.mapbox.services.android.navigation.testapp.R;
 import com.mapbox.services.android.navigation.ui.v5.NavigationView;
 import com.mapbox.services.android.navigation.ui.v5.NavigationViewOptions;
@@ -284,7 +285,7 @@ public class EmbeddedNavigationActivity extends AppCompatActivity implements OnN
   }

   private void setupNightModeFab() {
-    fabNightModeToggle.setOnClickListener(view -> toggleNightMode());
+    fabNightModeToggle.setOnClickListener(view -> navigationView.retrieveNavigationMapboxMap().retrieveMap().setStyle(Style.SATELLITE_STREETS));
   }

   private void toggleNightMode() {

Could you take a look at it @LukasPaczos when you have a chance?

@Ranafugaz We will prioritize this internally and then report back here on this ticket. Thanks again!

LukasPaczos commented 4 years ago

This is unrelated to the drawing refactor from https://github.com/mapbox/mapbox-navigation-android/pull/2262. The only relevant change is bumping the Maps SDK version - previously, the Maps SDK would drop all layer references when style was about to change, however, in order to not blink, the Map SDK is not doing that anymore.

This exposed MapRouteArrow's misuse of the Maps SDK which is interacting with the layer after the style is changed because of the constantly incoming stream of RouteProgress updates.