wipupstream/lp-detached-13521

11 files changed, 113 insertions, 48 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
index d6bb0a9f01..c58bab19f5 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
@@ -7,6 +7,7 @@ import android.support.annotation.IntRange;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.StringDef;
+import android.support.annotation.VisibleForTesting;
 import android.util.DisplayMetrics;
 import android.util.Pair;
 
@@ -51,7 +52,8 @@ public class Style {
    * @param builder   the builder used for creating this style
    * @param nativeMap the map object used to load this style
    */
-  private Style(@NonNull Builder builder, @NonNull NativeMap nativeMap) {
+  @VisibleForTesting
+  Style(@NonNull Builder builder, @NonNull NativeMap nativeMap) {
     this.builder = builder;
     this.nativeMap = nativeMap;
   }
@@ -442,18 +444,13 @@ public class Style {
    */
   void onWillStartLoadingMap() {
     fullyLoaded = false;
-    for (Source source : sources.values()) {
-      if (source != null) {
-        source.setDetached();
-        nativeMap.removeSource(source);
-      }
+
+    for (Source source : nativeMap.getSources()) {
+      source.nativeSetDetached();
     }
 
-    for (Layer layer : layers.values()) {
-      if (layer != null) {
-        layer.setDetached();
-        nativeMap.removeLayer(layer);
-      }
+    for (Layer layer : nativeMap.getLayers()) {
+      layer.nativeSetDetached();
     }
 
     for (Map.Entry<String, Bitmap> bitmapEntry : images.entrySet()) {
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java
index d290c093a7..6ef331c131 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java
@@ -17,9 +17,6 @@ public abstract class Layer {
 
   @Keep
   private long nativePtr;
-  @Keep
-  private boolean invalidated;
-  private boolean detached;
 
   static {
     LibraryLoader.load();
@@ -43,10 +40,6 @@ public abstract class Layer {
   }
 
   public void setProperties(@NonNull PropertyValue<?>... properties) {
-    if (detached) {
-      return;
-    }
-
     checkThread();
     if (properties.length == 0) {
       return;
@@ -142,6 +135,12 @@ public abstract class Layer {
   @Keep
   protected native void nativeSetMaxZoom(float zoom);
 
+  @Keep
+  public native void nativeSetDetached();
+
+  @Keep
+  public native boolean nativeIsDetached();
+
   public long getNativePtr() {
     return nativePtr;
   }
@@ -156,12 +155,4 @@ public abstract class Layer {
       return value;
     }
   }
-
-  public void setDetached() {
-    detached = true;
-  }
-
-  public boolean isDetached() {
-    return detached;
-  }
 }
\ No newline at end of file
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java
index 9e809c2ce0..b7b979958d 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java
@@ -194,9 +194,6 @@ public class GeoJsonSource extends Source {
    * @param feature the GeoJSON {@link Feature} to set
    */
   public void setGeoJson(Feature feature) {
-    if (detached) {
-      return;
-    }
     checkThread();
     nativeSetFeature(feature);
   }
@@ -208,9 +205,6 @@ public class GeoJsonSource extends Source {
    * @param geometry the GeoJSON {@link Geometry} to set
    */
   public void setGeoJson(Geometry geometry) {
-    if (detached) {
-      return;
-    }
     checkThread();
     nativeSetGeometry(geometry);
   }
@@ -222,9 +216,6 @@ public class GeoJsonSource extends Source {
    * @param features the GeoJSON FeatureCollection
    */
   public void setGeoJson(FeatureCollection features) {
-    if (detached) {
-      return;
-    }
     checkThread();
     nativeSetFeatureCollection(features);
   }
@@ -236,9 +227,6 @@ public class GeoJsonSource extends Source {
    * @param json the raw GeoJson FeatureCollection string
    */
   public void setGeoJson(String json) {
-    if (detached) {
-      return;
-    }
     checkThread();
     nativeSetGeoJsonString(json);
   }
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java
index 9667a33e09..ef2454d58e 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java
@@ -1,7 +1,6 @@
 package com.mapbox.mapboxsdk.style.sources;
 
 import android.support.annotation.Keep;
-
 import android.support.annotation.NonNull;
 
 import com.mapbox.mapboxsdk.LibraryLoader;
@@ -15,8 +14,6 @@ public abstract class Source {
   @Keep
   private long nativePtr;
 
-  protected boolean detached;
-
   static {
     LibraryLoader.load();
   }
@@ -85,7 +82,9 @@ public abstract class Source {
   @Keep
   protected native String nativeGetAttribution();
 
-  public void setDetached() {
-    detached = true;
-  }
+  @Keep
+  public native void nativeSetDetached();
+
+  @Keep
+  public native boolean nativeIsDetached();
 }
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt
index 84af279bd0..10be2f36ec 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt
@@ -7,10 +7,11 @@ import com.mapbox.mapboxsdk.maps.MapboxMap
 import com.mapbox.mapboxsdk.maps.Style
 import com.mapbox.mapboxsdk.style.layers.SymbolLayer
 import com.mapbox.mapboxsdk.style.sources.GeoJsonSource
-import com.mapbox.mapboxsdk.testapp.action.MapboxMapAction
+import com.mapbox.mapboxsdk.testapp.action.MapboxMapAction.invoke
 import com.mapbox.mapboxsdk.testapp.activity.EspressoTest
 import com.mapbox.mapboxsdk.testapp.activity.espresso.EspressoTestActivity
 import com.mapbox.mapboxsdk.testapp.utils.TestingAsyncUtils
+import junit.framework.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -29,7 +30,7 @@ class StyleLoadTest : EspressoTest() {
     @Test
     fun updateSourceAfterStyleLoad() {
         validateTestSetup()
-        MapboxMapAction.invoke(mapboxMap) { uiController: UiController, mapboxMap: MapboxMap ->
+        invoke(mapboxMap) { uiController: UiController, mapboxMap: MapboxMap ->
             val source = GeoJsonSource("id")
             val layer = SymbolLayer("id", "id")
             mapboxMap.setStyle(Style.Builder().withSource(source).withLayer(layer))
@@ -39,4 +40,26 @@ class StyleLoadTest : EspressoTest() {
             source.setGeoJson("{}")
         }
     }
+
+    @Test
+    fun loadingNewStyle_sourcesDetached() {
+        invoke(mapboxMap) { _: UiController, mapboxMap: MapboxMap ->
+            val sources = mapboxMap.style!!.sources
+            mapboxMap.setStyle(Style.DARK)
+            for (source in sources) {
+                Assert.assertTrue(source.nativeIsDetached())
+            }
+        }
+    }
+
+    @Test
+    fun loadingNewStyle_layersDetached() {
+        invoke(mapboxMap) { _: UiController, mapboxMap: MapboxMap ->
+            val layers = mapboxMap.style!!.layers
+            mapboxMap.setStyle(Style.DARK)
+            for (layer in layers) {
+                Assert.assertTrue(layer.nativeIsDetached())
+            }
+        }
+    }
 }
\ No newline at end of file
diff --git a/platform/android/src/style/layers/layer.cpp b/platform/android/src/style/layers/layer.cpp
index 2f335b6ff3..ee8949328d 100644
--- a/platform/android/src/style/layers/layer.cpp
+++ b/platform/android/src/style/layers/layer.cpp
@@ -91,7 +91,19 @@ namespace android {
         return layer;
     }
 
+    void Layer::setDetached(jni::JNIEnv&) {
+        detached = true;
+    }
+
+    jboolean Layer::isDetached(jni::JNIEnv&) {
+        return detached ? jni::jni_true : jni::jni_false;
+    }
+
     void Layer::setLayoutProperty(jni::JNIEnv& env, const jni::String& jname, const jni::Object<>& jvalue) {
+        if (detached) {
+            return;
+        }
+
         // Convert and set property
         optional<mbgl::style::conversion::Error> error = layer.setLayoutProperty(jni::Make<std::string>(env, jname), Value(env, jvalue));
         if (error) {
@@ -101,6 +113,10 @@ namespace android {
     }
 
     void Layer::setPaintProperty(jni::JNIEnv& env, const jni::String& jname, const jni::Object<>& jvalue) {
+        if (detached) {
+            return;
+        }
+
         // Convert and set property
         optional<mbgl::style::conversion::Error> error = layer.setPaintProperty(jni::Make<std::string>(env, jname), Value(env, jvalue));
         if (error) {
@@ -110,6 +126,10 @@ namespace android {
     }
 
     void Layer::setFilter(jni::JNIEnv& env, const jni::Array<jni::Object<>>& jfilter) {
+        if (detached) {
+            return;
+        }
+
         using namespace mbgl::style;
         using namespace mbgl::style::conversion;
 
@@ -137,6 +157,9 @@ namespace android {
     }
 
     void Layer::setSourceLayer(jni::JNIEnv& env, const jni::String& sourceLayer) {
+        if (detached) {
+            return;
+        }
         layer.setSourceLayer(jni::Make<std::string>(env, sourceLayer));
     }
 
@@ -157,10 +180,16 @@ namespace android {
     }
 
     void Layer::setMinZoom(jni::JNIEnv&, jni::jfloat zoom) {
+        if (detached) {
+            return;
+        }
         layer.setMinZoom(zoom);
     }
 
     void Layer::setMaxZoom(jni::JNIEnv&, jni::jfloat zoom) {
+        if (detached) {
+            return;
+        }
         layer.setMaxZoom(zoom);
     }
 
@@ -189,7 +218,9 @@ namespace android {
             METHOD(&Layer::getMaxZoom, "nativeGetMaxZoom"),
             METHOD(&Layer::setMinZoom, "nativeSetMinZoom"),
             METHOD(&Layer::setMaxZoom, "nativeSetMaxZoom"),
-            METHOD(&Layer::getVisibility, "nativeGetVisibility")
+            METHOD(&Layer::getVisibility, "nativeGetVisibility"),
+            METHOD(&Layer::setDetached, "nativeSetDetached"),
+            METHOD(&Layer::isDetached, "nativeIsDetached")
         );
 
     }
diff --git a/platform/android/src/style/layers/layer.hpp b/platform/android/src/style/layers/layer.hpp
index 0fb679152b..67baf789aa 100644
--- a/platform/android/src/style/layers/layer.hpp
+++ b/platform/android/src/style/layers/layer.hpp
@@ -35,6 +35,10 @@ public:
 
     style::Layer& get();
 
+    void setDetached(jni::JNIEnv&);
+
+    jboolean isDetached(jni::JNIEnv&);
+
     void setLayoutProperty(jni::JNIEnv&, const jni::String&, const jni::Object<>& value);
 
     void setPaintProperty(jni::JNIEnv&, const jni::String&, const jni::Object<>& value);
@@ -90,6 +94,9 @@ protected:
 
     // Map is set when the layer is retrieved or after adding to the map
     mbgl::Map* map;
+
+    // new style has started loading, making this layer invalid
+    bool detached;
 };
 
 /**
diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp
index a9307afe67..ccd068da0c 100644
--- a/platform/android/src/style/sources/geojson_source.cpp
+++ b/platform/android/src/style/sources/geojson_source.cpp
@@ -177,6 +177,9 @@ namespace android {
     }
 
     void GeoJSONSource::setAsync(Update::Converter converterFn) {
+        if (detached) {
+            return;
+        }
         awaitingUpdate = std::make_unique<Update>(
                 std::move(converterFn),
                 std::make_unique<Actor<Callback>>(
diff --git a/platform/android/src/style/sources/image_source.cpp b/platform/android/src/style/sources/image_source.cpp
index b42e0e5a51..7adc03d794 100644
--- a/platform/android/src/style/sources/image_source.cpp
+++ b/platform/android/src/style/sources/image_source.cpp
@@ -32,6 +32,9 @@ namespace android {
     ImageSource::~ImageSource() = default;
 
     void ImageSource::setURL(jni::JNIEnv& env, const jni::String& url) {
+        if (detached) {
+            return;
+        }
         // Update the core source
         source.as<mbgl::style::ImageSource>()->ImageSource::setURL(jni::Make<std::string>(env, url));
     }
@@ -42,10 +45,16 @@ namespace android {
     }
 
     void ImageSource::setImage(jni::JNIEnv& env, const jni::Object<Bitmap>& bitmap) {
+        if (detached) {
+            return;
+        }
         source.as<mbgl::style::ImageSource>()->setImage(Bitmap::GetImage(env, bitmap));
     }
 
     void ImageSource::setCoordinates(jni::JNIEnv& env, const jni::Object<LatLngQuad>& coordinatesObject) {
+        if (detached) {
+            return;
+        }
         source.as<mbgl::style::ImageSource>()->setCoordinates(
                 LatLngQuad::getLatLngArray(env, coordinatesObject));
     }
diff --git a/platform/android/src/style/sources/source.cpp b/platform/android/src/style/sources/source.cpp
index be4c0367fc..e9f0f4e4b8 100644
--- a/platform/android/src/style/sources/source.cpp
+++ b/platform/android/src/style/sources/source.cpp
@@ -142,6 +142,14 @@ namespace android {
         rendererFrontend = nullptr;
     }
 
+    void Source::setDetached(jni::JNIEnv&) {
+        detached = true;
+    }
+
+    jboolean Source::isDetached(jni::JNIEnv&) {
+        return detached ? jni::jni_true : jni::jni_false;
+    }
+
     void Source::registerNative(jni::JNIEnv& env) {
         // Lookup the class
         static auto& javaClass = jni::Class<Source>::Singleton(env);
@@ -151,7 +159,9 @@ namespace android {
         // Register the peer
         jni::RegisterNativePeer<Source>(env, javaClass, "nativePtr",
             METHOD(&Source::getId, "nativeGetId"),
-            METHOD(&Source::getAttribution, "nativeGetAttribution")
+            METHOD(&Source::getAttribution, "nativeGetAttribution"),
+            METHOD(&Source::setDetached, "nativeSetDetached"),
+            METHOD(&Source::isDetached, "nativeIsDetached")
         );
 
         // Register subclasses
diff --git a/platform/android/src/style/sources/source.hpp b/platform/android/src/style/sources/source.hpp
index 93b706425a..e93a1cd735 100644
--- a/platform/android/src/style/sources/source.hpp
+++ b/platform/android/src/style/sources/source.hpp
@@ -43,6 +43,10 @@ public:
 
     jni::Local<jni::String> getAttribution(jni::JNIEnv&);
 
+    void setDetached(jni::JNIEnv&);
+
+    jboolean isDetached(jni::JNIEnv&);
+
 protected:
     // Set on newly created sources until added to the map.
     std::unique_ptr<mbgl::style::Source> ownedSource;
@@ -55,6 +59,9 @@ protected:
 
     // RendererFrontend pointer is valid only when added to the map.
     AndroidRendererFrontend* rendererFrontend { nullptr };
+
+    // new style has started loading, making this source invalid
+    bool detached;
 };
 
 } // namespace android