Refactor native Map and Style to log error and not throw exception

CHANGELOG.md

@@ -5,9 +5,7 @@ Mapbox welcomes participation and contributions from everyone.
 # main
 ## Features ✨ and improvements 🏁
 * Optimize how plugins handle settings changes. Call `applySettings` only when settings value changes. ([#1189](https://github.com/mapbox/mapbox-maps-android/pull/1189))
-* Add exception type `MapboxMapMemoryLeakException` which will be thrown when users try to invoke functions related with `nativeMap` in `MapboxMap` and `Style` after onDestroy.
-  Users check the status of MapboxMap and Style by new method MapboxMap.isValid() and Style.isValid() to avoid such exception.
-  Automatically unsubscribe all observers in MapboxMap when `MapboxMap.onDestroy()` is invoked. ([1193](https://github.com/mapbox/mapbox-maps-android/pull/1193)) ([1202](https://github.com/mapbox/mapbox-maps-android/pull/1202))
+* Add `MapboxMap.isValid` and `Style.isValid` properties. Map and style become invalid when `MapView.onDestroy` is called: after that map and style objects are treated as not valid and accessing any method will cause printing error log and introduce a memory leak in user's code. Additionally unsubscribe all map observers when `MapboxMap.onDestroy()` is invoked. ([1193](https://github.com/mapbox/mapbox-maps-android/pull/1193)) ([1202](https://github.com/mapbox/mapbox-maps-android/pull/1202) ([1230](https://github.com/mapbox/mapbox-maps-android/pull/1230)))
 * Add `MapboxMap#coordinateBoundsForCameraUnwrapped` method for API consistency. ([1222](https://github.com/mapbox/mapbox-maps-android/pull/1222))
 * Add `LocationIndicatorLayer.bearingTransition` API to control transition of bearing property. (([1207](https://github.com/mapbox/mapbox-maps-android/pull/1207)))
 

app/src/androidTest/java/com/mapbox/maps/testapp/annotation/UpdateAnnotationTest.kt

@@ -37,7 +37,7 @@ class UpdateAnnotationTest : BaseMapTest() {
   private lateinit var pointAnnotation: PointAnnotation
   private lateinit var handler: Handler
   private val runnable = Runnable {
-    if (mapboxMap.isValid()) {
+    if (mapboxMap.isValid) {
       mapboxMap.loadStyleUri(AnnotationUtils.STYLES[index++ % AnnotationUtils.STYLES.size]) {
         runRunnable()
       }

app/src/main/java/com/mapbox/maps/testapp/examples/InsetMapActivity.kt

@@ -133,11 +133,6 @@ class InsetMapActivity : AppCompatActivity(), OnCameraChangeListener {
     )
   }
 
-  override fun onDestroy() {
-    super.onDestroy()
-    mainMapboxMap.removeOnCameraChangeListener(this)
-  }
-
   companion object {
     private const val STYLE_URL = "mapbox://styles/mapbox/cj5l80zrp29942rmtg0zctjto"
     private const val INSET_FRAGMENT_TAG = "com.mapbox.insetMapFragment"

app/src/main/java/com/mapbox/maps/testapp/examples/NinePatchImageActivity.kt

@@ -60,7 +60,7 @@ class NinePatchImageActivity : AppCompatActivity() {
   }
 
   private val runnable = {
-    if (style.isValid()) {
+    if (style.isValid) {
       val layer = (style.getLayer(LAYER_ID) as SymbolLayer)
       layer.textField("${layer.textField?.getTextAsString()} $TEXT_BASE")
       appendTextCounter++

app/src/main/java/com/mapbox/maps/testapp/examples/SimulateNavigationRouteActivity.kt

@@ -61,13 +61,13 @@ class SimulateNavigationRouteActivity : AppCompatActivity() {
 
   override fun onDestroy() {
     super.onDestroy()
+    handler.removeCallbacksAndMessages(null)
     if (this::navigationSimulator.isInitialized) {
       navigationSimulator.onDestroy()
     }
   }
 
   companion object {
-    private const val TAG = "NavigationRouteActivity"
     private const val NAVIGATION_ROUTE_JSON_NAME = "navigation_route.json"
     private const val INITIAL_OVERVIEW_DELAY_MS = 3000L
     private const val FIRST_FOLLOW_MODE_DELAY_MS = 8000L

app/src/main/java/com/mapbox/maps/testapp/examples/SpaceStationLocationActivity.kt

@@ -3,6 +3,7 @@ package com.mapbox.maps.testapp.examples
 import android.graphics.BitmapFactory
 import android.os.Bundle
 import android.os.Handler
+import android.os.Looper
 import android.widget.Toast
 import androidx.appcompat.app.AppCompatActivity
 import com.mapbox.common.Logger
@@ -35,7 +36,7 @@ import retrofit2.http.GET
  */
 class SpaceStationLocationActivity : AppCompatActivity() {
 
-  private val handler = Handler()
+  private val handler = Handler(Looper.getMainLooper())
   private var runnable: (() -> Unit) = {}
   private var call: Call<IssModel>? = null
   private lateinit var mapboxMap: MapboxMap
@@ -181,6 +182,7 @@ class SpaceStationLocationActivity : AppCompatActivity() {
     // When the user leaves the activity, there is no need in calling the API since the map
     // isn't in view.
     handler.removeCallbacksAndMessages(null)
+    call?.cancel()
   }
 
   companion object {

app/src/main/java/com/mapbox/maps/testapp/examples/fragment/MapFragment.kt

@@ -19,7 +19,7 @@ class MapFragment : Fragment() {
     inflater: LayoutInflater,
     container: ViewGroup?,
     savedInstanceState: Bundle?
-  ): View? {
+  ): View {
     mapView = MapView(
       inflater.context,
       MapInitOptions(inflater.context)
@@ -39,7 +39,7 @@ class MapFragment : Fragment() {
     callback.invoke(mapboxMap)
   } else this.onMapReady = callback
 
-  fun getMapView(): MapView? {
+  fun getMapView(): MapView {
     return mapView
   }
 }

extension-style/src/main/java/com/mapbox/maps/extension/style/sources/Source.kt

@@ -1,14 +1,17 @@
 package com.mapbox.maps.extension.style.sources
 
 import android.util.Log
+import androidx.annotation.VisibleForTesting
 import com.mapbox.bindgen.Value
 import com.mapbox.common.Logger
 import com.mapbox.maps.MapboxStyleException
 import com.mapbox.maps.StyleManagerInterface
 import com.mapbox.maps.extension.style.StyleContract
 import com.mapbox.maps.extension.style.StyleInterface
 import com.mapbox.maps.extension.style.layers.properties.PropertyValue
+import com.mapbox.maps.extension.style.sources.generated.GeoJsonSource
 import com.mapbox.maps.extension.style.utils.unwrap
+import java.lang.reflect.Method
 
 /**
  * Base class for sources.
@@ -48,13 +51,34 @@ abstract class Source(
 
   internal var delegate: StyleManagerInterface? = null
 
+  @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+  internal var styleObjectIsValidMethod: Method? = null
+
   /**
    * Add the source to the Style.
    *
    * @param delegate The style delegate
    */
   override fun bindTo(delegate: StyleInterface) {
     this.delegate = delegate
+    // geojson source is the only one holding async logic -
+    // in order to avoid accessing native style object when MapView is destroyed we need to check
+    // if delegate is still valid.
+    // Sadly due to current architecture it's now easily achievable with reflection only
+    // as this module does not have dependency on main SDK module and Style object
+    if (this is GeoJsonSource) {
+      try {
+        if (styleObjectIsValidMethod == null) {
+          val clazz = Class.forName(delegate.javaClass.name)
+          styleObjectIsValidMethod = clazz.getMethod("isValid")
+        }
+      } catch (e: Exception) {
+        // if exception did occur - nothing critical will happen,
+        // we will simply most likely access native object after MapView destruction leading
+        // to printing some logs, no actual leak will be introduced
+        Logger.e(TAG, e.message ?: "")
+      }
+    }
     val expected = delegate.addStyleSource(sourceId, getCachedSourceProperties())
     expected.error?.let {
       Log.e(TAG, getCachedSourceProperties().toString())
@@ -93,6 +117,14 @@ abstract class Source(
   private fun updateProperty(property: PropertyValue<*>, throwRuntimeException: Boolean = true) {
     delegate?.let { styleDelegate ->
       try {
+        if (this is GeoJsonSource) {
+          val isNativeStyleValid = styleObjectIsValidMethod?.invoke(styleDelegate)
+          // explicitly reset native reference and return
+          if (isNativeStyleValid == false) {
+            delegate = null
+            return@let
+          }
+        }
         val expected = styleDelegate.setStyleSourceProperty(
           sourceId,
           property.propertyName,