[android] fix ownership of style layer after delete

19 files changed, 184 insertions, 18 deletions

diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
index af1473ca34..e188a98c6e 100755
--- a/platform/android/src/native_map_view.cpp
+++ b/platform/android/src/native_map_view.cpp
@@ -780,10 +780,15 @@ void NativeMapView::addLayer(JNIEnv& env, jlong nativeLayerPtr, jni::String befo
 }
 
 /**
- * Remove by layer id. Ownership is not transferred back
+ * Remove by layer id.
  */
-void NativeMapView::removeLayerById(JNIEnv& env, jni::String id) {
-    map->removeLayer(jni::Make<std::string>(env, id));
+jni::Object<Layer> NativeMapView::removeLayerById(JNIEnv& env, jni::String id) {
+    std::unique_ptr<mbgl::style::Layer> coreLayer = map->removeLayer(jni::Make<std::string>(env, id));
+    if (coreLayer) {
+        return jni::Object<Layer>(createJavaLayerPeer(env, *map, std::move(coreLayer)));
+    } else {
+        return jni::Object<Layer>();
+    }
 }
 
 /**
diff --git a/platform/android/src/style/layers/background_layer.cpp b/platform/android/src/style/layers/background_layer.cpp
index 6b013ae086..9915f3894e 100644
--- a/platform/android/src/style/layers/background_layer.cpp
+++ b/platform/android/src/style/layers/background_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     BackgroundLayer::BackgroundLayer(jni::JNIEnv& env, jni::String layerId)
         : Layer(env, std::make_unique<mbgl::style::BackgroundLayer>(jni::Make<std::string>(env, layerId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     BackgroundLayer::BackgroundLayer(mbgl::Map& map, mbgl::style::BackgroundLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    BackgroundLayer::BackgroundLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::BackgroundLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     BackgroundLayer::~BackgroundLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/background_layer.hpp b/platform/android/src/style/layers/background_layer.hpp
index bd62c024f4..2fdc948892 100644
--- a/platform/android/src/style/layers/background_layer.hpp
+++ b/platform/android/src/style/layers/background_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     BackgroundLayer(mbgl::Map&, mbgl::style::BackgroundLayer&);
 
+    BackgroundLayer(mbgl::Map&, std::unique_ptr<mbgl::style::BackgroundLayer>);
+
     ~BackgroundLayer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/circle_layer.cpp b/platform/android/src/style/layers/circle_layer.cpp
index 2b33a2dd47..948c397829 100644
--- a/platform/android/src/style/layers/circle_layer.cpp
+++ b/platform/android/src/style/layers/circle_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     CircleLayer::CircleLayer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::CircleLayer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     CircleLayer::CircleLayer(mbgl::Map& map, mbgl::style::CircleLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    CircleLayer::CircleLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::CircleLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     CircleLayer::~CircleLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/circle_layer.hpp b/platform/android/src/style/layers/circle_layer.hpp
index d45984f23b..ee988d7c57 100644
--- a/platform/android/src/style/layers/circle_layer.hpp
+++ b/platform/android/src/style/layers/circle_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     CircleLayer(mbgl::Map&, mbgl::style::CircleLayer&);
 
+    CircleLayer(mbgl::Map&, std::unique_ptr<mbgl::style::CircleLayer>);
+
     ~CircleLayer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/fill_layer.cpp b/platform/android/src/style/layers/fill_layer.cpp
index d818314f89..fc1dfccfcc 100644
--- a/platform/android/src/style/layers/fill_layer.cpp
+++ b/platform/android/src/style/layers/fill_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     FillLayer::FillLayer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::FillLayer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     FillLayer::FillLayer(mbgl::Map& map, mbgl::style::FillLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    FillLayer::FillLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::FillLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     FillLayer::~FillLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/fill_layer.hpp b/platform/android/src/style/layers/fill_layer.hpp
index 7609a5742f..f43c263ab8 100644
--- a/platform/android/src/style/layers/fill_layer.hpp
+++ b/platform/android/src/style/layers/fill_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     FillLayer(mbgl::Map&, mbgl::style::FillLayer&);
 
+    FillLayer(mbgl::Map&, std::unique_ptr<mbgl::style::FillLayer>);
+
     ~FillLayer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/layer.cpp b/platform/android/src/style/layers/layer.cpp
index bfc09be302..dbf71fd2af 100644
--- a/platform/android/src/style/layers/layer.cpp
+++ b/platform/android/src/style/layers/layer.cpp
@@ -26,9 +26,22 @@ namespace android {
         , layer(*ownedLayer) {
     }
 
+    /**
+     * Takes a non-owning reference. For lookup methods
+     */
     Layer::Layer(mbgl::Map& coreMap, mbgl::style::Layer& coreLayer) : layer(coreLayer) , map(&coreMap) {
     }
 
+    /**
+     * Takes a owning reference. Ownership is transfered to this peer, eg after removing
+     * from the map
+     */
+    Layer::Layer(mbgl::Map& coreMap, std::unique_ptr<mbgl::style::Layer> coreLayer)
+        : ownedLayer(std::move(coreLayer))
+        , layer(*ownedLayer)
+        , map(&coreMap) {
+    }
+
     Layer::~Layer() {
     }
 
diff --git a/platform/android/src/style/layers/layer.cpp.ejs b/platform/android/src/style/layers/layer.cpp.ejs
index 9a1274965b..5da397d77d 100644
--- a/platform/android/src/style/layers/layer.cpp.ejs
+++ b/platform/android/src/style/layers/layer.cpp.ejs
@@ -14,18 +14,34 @@ namespace mbgl {
 namespace android {
 
 <% if (type === 'background') { -%>
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     <%- camelize(type) %>Layer::<%- camelize(type) %>Layer(jni::JNIEnv& env, jni::String layerId)
         : Layer(env, std::make_unique<mbgl::style::<%- camelize(type) %>Layer>(jni::Make<std::string>(env, layerId))) {
 <% } else { -%>
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     <%- camelize(type) %>Layer::<%- camelize(type) %>Layer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::<%- camelize(type) %>Layer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
 <% } -%>
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     <%- camelize(type) %>Layer::<%- camelize(type) %>Layer(mbgl::Map& map, mbgl::style::<%- camelize(type) %>Layer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    <%- camelize(type) %>Layer::<%- camelize(type) %>Layer(mbgl::Map& map, std::unique_ptr<mbgl::style::<%- camelize(type) %>Layer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     <%- camelize(type) %>Layer::~<%- camelize(type) %>Layer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/layer.hpp b/platform/android/src/style/layers/layer.hpp
index bf07adad67..deea7a6613 100644
--- a/platform/android/src/style/layers/layer.hpp
+++ b/platform/android/src/style/layers/layer.hpp
@@ -21,11 +21,16 @@ public:
     static void registerNative(jni::JNIEnv&);
 
     /*
-     * Called when a Java object is created on the c++ side
+     * Called when a non-owning peer object is created on the c++ side
      */
     Layer(mbgl::Map&, mbgl::style::Layer&);
 
     /*
+     * Called when a owning peer object is created on the c++ side
+     */
+    Layer(mbgl::Map&, std::unique_ptr<mbgl::style::Layer>);
+
+    /*
      * Called when a Java object was created from the jvm side
      */
     Layer(jni::JNIEnv&, std::unique_ptr<mbgl::style::Layer>);
diff --git a/platform/android/src/style/layers/layer.hpp.ejs b/platform/android/src/style/layers/layer.hpp.ejs
index 3d715746ff..102efd2d4d 100644
--- a/platform/android/src/style/layers/layer.hpp.ejs
+++ b/platform/android/src/style/layers/layer.hpp.ejs
@@ -30,6 +30,8 @@ public:
 
     <%- camelize(type) %>Layer(mbgl::Map&, mbgl::style::<%- camelize(type) %>Layer&);
 
+    <%- camelize(type) %>Layer(mbgl::Map&, std::unique_ptr<mbgl::style::<%- camelize(type) %>Layer>);
+
     ~<%- camelize(type) %>Layer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/layers.cpp b/platform/android/src/style/layers/layers.cpp
index 328ef94ec8..5c6ee1ae8f 100644
--- a/platform/android/src/style/layers/layers.cpp
+++ b/platform/android/src/style/layers/layers.cpp
@@ -21,36 +21,65 @@
 namespace mbgl {
 namespace android {
 
-Layer* initializeLayerPeer(mbgl::Map& map, mbgl::style::Layer& coreLayer) {
-    Layer* layer;
+static Layer* initializeLayerPeer(mbgl::Map& map, mbgl::style::Layer& coreLayer) {
     if (coreLayer.is<mbgl::style::BackgroundLayer>()) {
-        layer = new BackgroundLayer(map, *coreLayer.as<mbgl::style::BackgroundLayer>());
+        return new BackgroundLayer(map, *coreLayer.as<mbgl::style::BackgroundLayer>());
     } else if (coreLayer.is<mbgl::style::CircleLayer>()) {
-        layer = new CircleLayer(map, *coreLayer.as<mbgl::style::CircleLayer>());
+        return new CircleLayer(map, *coreLayer.as<mbgl::style::CircleLayer>());
     } else if (coreLayer.is<mbgl::style::FillLayer>()) {
-            layer = new FillLayer(map, *coreLayer.as<mbgl::style::FillLayer>());
+        return new FillLayer(map, *coreLayer.as<mbgl::style::FillLayer>());
     } else if (coreLayer.is<mbgl::style::LineLayer>()) {
-            layer = new LineLayer(map, *coreLayer.as<mbgl::style::LineLayer>());
+        return new LineLayer(map, *coreLayer.as<mbgl::style::LineLayer>());
     } else if (coreLayer.is<mbgl::style::RasterLayer>()) {
-            layer = new RasterLayer(map, *coreLayer.as<mbgl::style::RasterLayer>());
+        return new RasterLayer(map, *coreLayer.as<mbgl::style::RasterLayer>());
     } else if (coreLayer.is<mbgl::style::SymbolLayer>()) {
-        layer = new SymbolLayer(map, *coreLayer.as<mbgl::style::SymbolLayer>());
+        return new SymbolLayer(map, *coreLayer.as<mbgl::style::SymbolLayer>());
     } else if (coreLayer.is<mbgl::style::CustomLayer>()) {
-        layer = new CustomLayer(map, *coreLayer.as<mbgl::style::CustomLayer>());
+        return new CustomLayer(map, *coreLayer.as<mbgl::style::CustomLayer>());
     } else {
-        layer = new UnknownLayer(map, coreLayer);
+        return new UnknownLayer(map, coreLayer);
     }
+}
+
+template <class LayerType, class PeerType>
+static PeerType* createPeer(Map& map, std::unique_ptr<mbgl::style::Layer> layer) {
+    return new PeerType(map, std::move(std::unique_ptr<LayerType>(layer.release()->as<LayerType>())));
+}
 
-    return layer;
+static Layer* initializeLayerPeer(Map& map, std::unique_ptr<mbgl::style::Layer> coreLayer) {
+    if (coreLayer->is<style::BackgroundLayer>()) {
+        return createPeer<style::BackgroundLayer, BackgroundLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<style::CircleLayer>()) {
+        return createPeer<style::CircleLayer, CircleLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<style::FillLayer>()) {
+        return createPeer<style::FillLayer, FillLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<style::LineLayer>()) {
+        return createPeer<style::LineLayer, LineLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<style::RasterLayer>()) {
+        return createPeer<style::RasterLayer, RasterLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<style::SymbolLayer>()) {
+        return createPeer<style::SymbolLayer, SymbolLayer>(map, std::move(coreLayer));
+    } else if (coreLayer->is<mbgl::style::CustomLayer>()) {
+        return createPeer<style::SymbolLayer, SymbolLayer>(map, std::move(coreLayer));
+    } else {
+        return new UnknownLayer(map, std::move(coreLayer));
+    }
 }
 
-jni::jobject* createJavaLayerPeer(jni::JNIEnv& env, mbgl::Map& map, mbgl::style::Layer& coreLayer) {
+jni::jobject* createJavaLayerPeer(jni::JNIEnv& env, Map& map, style::Layer& coreLayer) {
     std::unique_ptr<Layer> peerLayer = std::unique_ptr<Layer>(initializeLayerPeer(map, coreLayer));
     jni::jobject* result = peerLayer->createJavaPeer(env);
     peerLayer.release();
     return result;
 }
 
+jni::jobject* createJavaLayerPeer(jni::JNIEnv& env, mbgl::Map& map, std::unique_ptr<mbgl::style::Layer> coreLayer) {
+    std::unique_ptr<Layer> peerLayer = std::unique_ptr<Layer>(initializeLayerPeer(map, std::move(coreLayer)));
+    jni::jobject* result = peerLayer->createJavaPeer(env);
+    peerLayer.release();
+    return result;
+}
+
 void registerNativeLayers(jni::JNIEnv& env) {
     Layer::registerNative(env);
     BackgroundLayer::registerNative(env);
diff --git a/platform/android/src/style/layers/layers.hpp b/platform/android/src/style/layers/layers.hpp
index 111b521c26..75863a324a 100644
--- a/platform/android/src/style/layers/layers.hpp
+++ b/platform/android/src/style/layers/layers.hpp
@@ -10,10 +10,16 @@
 namespace mbgl {
 namespace android {
 
-mbgl::android::Layer* initializeLayerPeer(mbgl::Map&, mbgl::style::Layer&);
-
+/**
+ * Create a non-owning peer
+ */
 jni::jobject* createJavaLayerPeer(jni::JNIEnv&, mbgl::Map&, mbgl::style::Layer&);
 
+/**
+ * Create an owning peer
+ */
+jni::jobject* createJavaLayerPeer(jni::JNIEnv& env, mbgl::Map& map, std::unique_ptr<mbgl::style::Layer>);
+
 void registerNativeLayers(jni::JNIEnv&);
 
 }
diff --git a/platform/android/src/style/layers/line_layer.cpp b/platform/android/src/style/layers/line_layer.cpp
index e53e2060c8..1a3a666a7b 100644
--- a/platform/android/src/style/layers/line_layer.cpp
+++ b/platform/android/src/style/layers/line_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     LineLayer::LineLayer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::LineLayer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     LineLayer::LineLayer(mbgl::Map& map, mbgl::style::LineLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    LineLayer::LineLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::LineLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     LineLayer::~LineLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/line_layer.hpp b/platform/android/src/style/layers/line_layer.hpp
index e2fc93329e..a79c8b9021 100644
--- a/platform/android/src/style/layers/line_layer.hpp
+++ b/platform/android/src/style/layers/line_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     LineLayer(mbgl::Map&, mbgl::style::LineLayer&);
 
+    LineLayer(mbgl::Map&, std::unique_ptr<mbgl::style::LineLayer>);
+
     ~LineLayer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/raster_layer.cpp b/platform/android/src/style/layers/raster_layer.cpp
index 69fd634bce..8a324b88f2 100644
--- a/platform/android/src/style/layers/raster_layer.cpp
+++ b/platform/android/src/style/layers/raster_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     RasterLayer::RasterLayer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::RasterLayer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     RasterLayer::RasterLayer(mbgl::Map& map, mbgl::style::RasterLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    RasterLayer::RasterLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::RasterLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     RasterLayer::~RasterLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/raster_layer.hpp b/platform/android/src/style/layers/raster_layer.hpp
index 3cc2d96dde..2f5d4f6fcd 100644
--- a/platform/android/src/style/layers/raster_layer.hpp
+++ b/platform/android/src/style/layers/raster_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     RasterLayer(mbgl::Map&, mbgl::style::RasterLayer&);
 
+    RasterLayer(mbgl::Map&, std::unique_ptr<mbgl::style::RasterLayer>);
+
     ~RasterLayer();
 
     // Property getters
diff --git a/platform/android/src/style/layers/symbol_layer.cpp b/platform/android/src/style/layers/symbol_layer.cpp
index 14803ae278..e42eeb4c77 100644
--- a/platform/android/src/style/layers/symbol_layer.cpp
+++ b/platform/android/src/style/layers/symbol_layer.cpp
@@ -9,14 +9,27 @@
 namespace mbgl {
 namespace android {
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map) from the JVM side
+     */
     SymbolLayer::SymbolLayer(jni::JNIEnv& env, jni::String layerId, jni::String sourceId)
         : Layer(env, std::make_unique<mbgl::style::SymbolLayer>(jni::Make<std::string>(env, layerId), jni::Make<std::string>(env, sourceId))) {
     }
 
+    /**
+     * Creates a non-owning peer object (for layers currently attached to the map)
+     */
     SymbolLayer::SymbolLayer(mbgl::Map& map, mbgl::style::SymbolLayer& coreLayer)
         : Layer(map, coreLayer) {
     }
 
+    /**
+     * Creates an owning peer object (for layers not attached to the map)
+     */
+    SymbolLayer::SymbolLayer(mbgl::Map& map, std::unique_ptr<mbgl::style::SymbolLayer> coreLayer)
+        : Layer(map, std::move(coreLayer)) {
+    }
+
     SymbolLayer::~SymbolLayer() = default;
 
     // Property getters
diff --git a/platform/android/src/style/layers/symbol_layer.hpp b/platform/android/src/style/layers/symbol_layer.hpp
index 1048b01b14..98ce5572e9 100644
--- a/platform/android/src/style/layers/symbol_layer.hpp
+++ b/platform/android/src/style/layers/symbol_layer.hpp
@@ -22,6 +22,8 @@ public:
 
     SymbolLayer(mbgl::Map&, mbgl::style::SymbolLayer&);
 
+    SymbolLayer(mbgl::Map&, std::unique_ptr<mbgl::style::SymbolLayer>);
+
     ~SymbolLayer();
 
     // Property getters