Fix possible crash when terrain source is added multiple times using …

…runtime Style DSL.
mapbox · Oct 3, 2022 · a4da67f · a4da67f
1 parent d94fa14
commit a4da67f
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 4 deletions.
diff --git a/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt b/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt
@@ -4,7 +4,10 @@ import androidx.test.annotation.UiThreadTest
 import androidx.test.ext.junit.rules.ActivityScenarioRule
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.LargeTest
+import com.mapbox.maps.extension.style.expressions.dsl.generated.literal
+import com.mapbox.maps.extension.style.sources.generated.rasterDemSource
 import com.mapbox.maps.extension.style.style
+import com.mapbox.maps.extension.style.terrain.generated.terrain
 import junit.framework.TestCase.*
 import org.junit.*
 import org.junit.runner.RunWith
@@ -169,6 +172,53 @@ class StyleLoadTest {
     assertEquals(loadedStyles, 1)
   }
 
+  @Test
+  fun testTerrainDoesNotCrashOnMultipleStyleLoad() {
+    fun loadStyleWithTerrain(
+      uri: String = Style.DARK,
+      terrainSource: String = "TERRAIN_SOURCE",
+      demSourceUri : String= "mapbox://mapbox.mapbox-terrain-dem-v1"
+    ) {
+      mapboxMap.loadStyle(
+        styleExtension = style(uri) {
+          // we need terrain for the drape-mipmap parameter to be effective to reduce flickering
+          +rasterDemSource(terrainSource) {
+            url(demSourceUri)
+            // 514 specifies padded DEM tile and provides better performance than 512 tiles.
+            tileSize(514)
+            maxzoom(3)
+          }
+          +terrain(terrainSource) {
+            exaggeration(literal(0))
+          }
+        }
+      )
+    }
+
+    countDownLatch = CountDownLatch(1)
+    rule.scenario.onActivity {
+      it.runOnUiThread {
+        for (i in 0..100) {
+          loadStyleWithTerrain()
+        }
+
+        var i = 0
+        val runnable = object : Runnable {
+          override fun run() {
+            if (i++ < 100) {
+              loadStyleWithTerrain()
+              mapView.postDelayed(this, 1)
+            }
+          }
+        }
+        mapView.post(runnable)
+
+        mapView.onStart()
+      }
+    }
+    countDownLatch.throwExceptionOnTimeoutMs()
+  }
+
   @After
   @UiThreadTest
   fun teardown() {

diff --git a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt
@@ -326,7 +326,7 @@ class MapboxMap :
 
   private fun initializeStyleLoad(
     onStyleLoaded: Style.OnStyleLoaded? = null,
-    styleDataStyleLoadedListener: Style.OnStyleLoaded? = null,
+    styleDataStyleLoadedListener: Style.OnStyleLoaded,
     styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null,
     styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null,
     onMapLoadErrorListener: OnMapLoadErrorListener? = null,

diff --git a/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt b/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt
@@ -22,8 +22,15 @@ internal class StyleObserver(
 
   private var userStyleLoadedListener: Style.OnStyleLoaded? = null
 
+  // fired when [StyleDataType.STYLE] arrives, first of all style data loaded events
   private var styleDataStyleLoadedListener: Style.OnStyleLoaded? = null
+  // it the listener is not null - style load has been started
+  // but the initial event has NOT arrived yet
+  private val isWaitingStyleDataStyleEvent
+    get() = styleDataStyleLoadedListener == null
+  // fired when [StyleDataType.SPRITE] arrives
   private var styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null
+  // fired when [StyleDataType.SOURCES] arrives
   private var styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null
 
   private var loadStyleErrorListener: OnMapLoadErrorListener? = null
@@ -50,7 +57,7 @@ internal class StyleObserver(
    */
   fun setLoadStyleListener(
     userOnStyleLoaded: Style.OnStyleLoaded?,
-    styleDataStyleLoadedListener: Style.OnStyleLoaded? = null,
+    styleDataStyleLoadedListener: Style.OnStyleLoaded,
     styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null,
     styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null,
     onMapLoadErrorListener: OnMapLoadErrorListener?
@@ -115,10 +122,18 @@ internal class StyleObserver(
         styleDataStyleLoadedListener = null
       }
       StyleDataType.SPRITE -> {
-        onStyleSpritesReady()
+        // if we're waiting for the [StyleDataType.STYLE] event - then this [StyleDataType.SPRITE]
+        // is related to the previously loaded style, don't notify style extension DSL about it
+        if (!isWaitingStyleDataStyleEvent) {
+          onStyleSpritesReady()
+        }
       }
       StyleDataType.SOURCES -> {
-        onStyleSourcesReady()
+        // if we're waiting for the [StyleDataType.STYLE] event - then this [StyleDataType.SOURCES]
+        // is related to the previously loaded style, don't notify style extension DSL about it
+        if (!isWaitingStyleDataStyleEvent) {
+          onStyleSourcesReady()
+        }
       }
     }
   }