[android] Reuse Java Source objects by holding on to a strong referen…

…ce in the C++ peer.
mapbox · Nov 22, 2017 · f06ebcc · f06ebcc
1 parent 3067b77
commit f06ebcc
Show file tree

Hide file tree

Showing 21 changed files with 183 additions and 193 deletions.
diff --git a/...orm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java b/...orm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
@@ -733,22 +733,23 @@ public void addSource(@NonNull Source source) {
     if (isDestroyedOn("addSource")) {
       return;
     }
-    nativeAddSource(source.getNativePtr());
+    nativeAddSource(source, source.getNativePtr());
   }
 
   @Nullable
   public Source removeSource(@NonNull String sourceId) {
     if (isDestroyedOn("removeSource")) {
       return null;
     }
-    return nativeRemoveSourceById(sourceId);
+    Source source = getSource(sourceId);
+    return removeSource(source);
   }
 
   public Source removeSource(@NonNull Source source) {
     if (isDestroyedOn("removeSource")) {
       return null;
     }
-    nativeRemoveSource(source.getNativePtr());
+    nativeRemoveSource(source, source.getNativePtr());
     return source;
   }
 
@@ -1027,11 +1028,9 @@ private native void nativeFlyTo(double angle, double latitude, double longitude,
 
   private native Source nativeGetSource(String sourceId);
 
-  private native void nativeAddSource(long nativeSourcePtr) throws CannotAddSourceException;
+  private native void nativeAddSource(Source source, long sourcePtr) throws CannotAddSourceException;
 
-  private native Source nativeRemoveSourceById(String sourceId);
-
-  private native void nativeRemoveSource(long sourcePtr);
+  private native void nativeRemoveSource(Source source, long sourcePtr);
 
   private native void nativeAddImage(String name, int width, int height, float pixelRatio,
                                      byte[] array);

diff --git a/platform/android/config.cmake b/platform/android/config.cmake
@@ -181,8 +181,6 @@ add_library(mbgl-android STATIC
     platform/android/src/style/sources/custom_geometry_source.hpp
     platform/android/src/style/sources/source.cpp
     platform/android/src/style/sources/source.hpp
-    platform/android/src/style/sources/sources.cpp
-    platform/android/src/style/sources/sources.hpp
     platform/android/src/style/sources/raster_source.cpp
     platform/android/src/style/sources/raster_source.hpp
     platform/android/src/style/sources/unknown_source.cpp

diff --git a/platform/android/src/jni.cpp b/platform/android/src/jni.cpp
@@ -47,7 +47,7 @@
 #include "style/functions/interval_stops.hpp"
 #include "style/functions/stop.hpp"
 #include "style/layers/layers.hpp"
-#include "style/sources/sources.hpp"
+#include "style/sources/source.hpp"
 #include "style/light.hpp"
 #include "snapshotter/map_snapshotter.hpp"
 #include "snapshotter/map_snapshot.hpp"
@@ -161,7 +161,7 @@ void registerNatives(JavaVM *vm) {
     // Style
     TransitionOptions::registerNative(env);
     registerNativeLayers(env);
-    registerNativeSources(env);
+    Source::registerNative(env);
     Light::registerNative(env);
     Position::registerNative(env);
     Stop::registerNative(env);

diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
@@ -856,9 +856,7 @@ jni::Array<jni::Object<Source>> NativeMapView::getSources(JNIEnv& env) {
     jni::Array<jni::Object<Source>> jSources = jni::Array<jni::Object<Source>>::New(env, sources.size(), Source::javaClass);
     int index = 0;
     for (auto source : sources) {
-        auto jSource = jni::Object<Source>(createJavaSourcePeer(env, *rendererFrontend, *source));
-        jSources.Set(env, index, jSource);
-        jni::DeleteLocalRef(env, jSource);
+        jSources.Set(env, index, Source::peerForCoreSource(env, *source, *rendererFrontend));
         index++;
     }
 
@@ -874,38 +872,25 @@ jni::Object<Source> NativeMapView::getSource(JNIEnv& env, jni::String sourceId)
     }
 
     // Create and return the source's native peer
-    return jni::Object<Source>(createJavaSourcePeer(env, *rendererFrontend, *coreSource));
+    return Source::peerForCoreSource(env, *coreSource, *rendererFrontend);
 }
 
-void NativeMapView::addSource(JNIEnv& env, jni::jlong sourcePtr) {
+void NativeMapView::addSource(JNIEnv& env, jni::Object<Source> obj, jlong sourcePtr) {
     assert(sourcePtr != 0);
 
     Source *source = reinterpret_cast<Source *>(sourcePtr);
     try {
-        source->addToMap(*map);
-        source->setRendererFrontend(*rendererFrontend);
+        source->addToMap(env, obj, *map, *rendererFrontend);
     } catch (const std::runtime_error& error) {
         jni::ThrowNew(env, jni::FindClass(env, "com/mapbox/mapboxsdk/style/sources/CannotAddSourceException"), error.what());
     }
 }
 
-jni::Object<Source> NativeMapView::removeSourceById(JNIEnv& env, jni::String id) {
-    std::unique_ptr<mbgl::style::Source> coreSource = map->getStyle().removeSource(jni::Make<std::string>(env, id));
-    if (coreSource) {
-        return jni::Object<Source>(createJavaSourcePeer(env, *rendererFrontend, *coreSource));
-    } else {
-        return jni::Object<Source>();
-    }
-}
-
-void NativeMapView::removeSource(JNIEnv&, jlong sourcePtr) {
+void NativeMapView::removeSource(JNIEnv& env, jni::Object<Source> obj, jlong sourcePtr) {
     assert(sourcePtr != 0);
 
     mbgl::android::Source *source = reinterpret_cast<mbgl::android::Source *>(sourcePtr);
-    std::unique_ptr<mbgl::style::Source> coreSource = map->getStyle().removeSource(source->get().getID());
-    if (coreSource) {
-        source->setSource(std::move(coreSource));
-    }
+    source->removeFromMap(env, obj, *map);
 }
 
 void NativeMapView::addImage(JNIEnv& env, jni::String name, jni::jint w, jni::jint h, jni::jfloat scale, jni::Array<jbyte> pixels) {
@@ -1048,7 +1033,6 @@ void NativeMapView::registerNative(jni::JNIEnv& env) {
             METHOD(&NativeMapView::getSources, "nativeGetSources"),
             METHOD(&NativeMapView::getSource, "nativeGetSource"),
             METHOD(&NativeMapView::addSource, "nativeAddSource"),
-            METHOD(&NativeMapView::removeSourceById, "nativeRemoveSourceById"),
             METHOD(&NativeMapView::removeSource, "nativeRemoveSource"),
             METHOD(&NativeMapView::addImage, "nativeAddImage"),
             METHOD(&NativeMapView::addImages, "nativeAddImages"),

diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp
@@ -19,7 +19,7 @@
 #include "geometry/lat_lng.hpp"
 #include "geometry/projected_meters.hpp"
 #include "style/layers/layers.hpp"
-#include "style/sources/sources.hpp"
+#include "style/sources/source.hpp"
 #include "geometry/lat_lng_bounds.hpp"
 #include "map/camera_position.hpp"
 #include "map/image.hpp"
@@ -230,11 +230,11 @@ class NativeMapView : public MapObserver {
 
     jni::Object<Source> getSource(JNIEnv&, jni::String);
 
-    void addSource(JNIEnv&, jni::jlong);
+    void addSource(JNIEnv&, jni::Object<Source>, jlong nativePtr);
 
     jni::Object<Source> removeSourceById(JNIEnv&, jni::String);
 
-    void removeSource(JNIEnv&, jlong);
+    void removeSource(JNIEnv&, jni::Object<Source>, jlong nativePtr);
 
     void addImage(JNIEnv&, jni::String, jni::jint, jni::jint, jni::jfloat, jni::Array<jbyte>);
 

diff --git a/platform/android/src/style/sources/custom_geometry_source.cpp b/platform/android/src/style/sources/custom_geometry_source.cpp
@@ -46,6 +46,12 @@ namespace android {
         javaPeer(_obj.NewGlobalRef(env))  {
     }
 
+    CustomGeometrySource::CustomGeometrySource(jni::JNIEnv& env,
+                                               mbgl::style::Source& coreSource,
+                                               AndroidRendererFrontend& frontend)
+        : Source(env, coreSource, createJavaPeer(env), frontend) {
+    }
+
     CustomGeometrySource::~CustomGeometrySource() = default;
 
     void CustomGeometrySource::fetchTile (const mbgl::CanonicalTileID& tileID) {
@@ -94,9 +100,9 @@ namespace android {
 
     jni::Class<CustomGeometrySource> CustomGeometrySource::javaClass;
 
-    jni::jobject* CustomGeometrySource::createJavaPeer(jni::JNIEnv& env) {
+    jni::Object<Source> CustomGeometrySource::createJavaPeer(jni::JNIEnv& env) {
         static auto constructor = CustomGeometrySource::javaClass.template GetConstructor<jni::jlong>(env);
-        return CustomGeometrySource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this));
+        return jni::Object<Source>(CustomGeometrySource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this)).Get());
     }
 
     void CustomGeometrySource::registerNative(jni::JNIEnv& env) {

diff --git a/platform/android/src/style/sources/custom_geometry_source.hpp b/platform/android/src/style/sources/custom_geometry_source.hpp
@@ -27,6 +27,8 @@ class CustomGeometrySource : public Source {
                        jni::String,
                        jni::Object<>);
 
+    CustomGeometrySource(jni::JNIEnv&, mbgl::style::Source&, AndroidRendererFrontend&);
+
     ~CustomGeometrySource();
 
     void fetchTile(const mbgl::CanonicalTileID& tileID);
@@ -39,7 +41,8 @@ class CustomGeometrySource : public Source {
     jni::Array<jni::Object<geojson::Feature>> querySourceFeatures(jni::JNIEnv&,
                                                                   jni::Array<jni::Object<>> );
 
-    jni::jobject* createJavaPeer(jni::JNIEnv&);
+private:
+    jni::Object<Source> createJavaPeer(jni::JNIEnv&);
 
     jni::UniqueObject<CustomGeometrySource> javaPeer;
 }; // class CustomGeometrySource

diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp
@@ -43,8 +43,10 @@ namespace android {
             ) {
     }
 
-    GeoJSONSource::GeoJSONSource(mbgl::style::GeoJSONSource& coreSource)
-        : Source(coreSource) {
+    GeoJSONSource::GeoJSONSource(jni::JNIEnv& env,
+                                 mbgl::style::Source& coreSource,
+                                 AndroidRendererFrontend& frontend)
+            : Source(env, coreSource, createJavaPeer(env), frontend) {
     }
 
     GeoJSONSource::~GeoJSONSource() = default;
@@ -118,9 +120,9 @@ namespace android {
 
     jni::Class<GeoJSONSource> GeoJSONSource::javaClass;
 
-    jni::jobject* GeoJSONSource::createJavaPeer(jni::JNIEnv& env) {
+    jni::Object<Source> GeoJSONSource::createJavaPeer(jni::JNIEnv& env) {
         static auto constructor = GeoJSONSource::javaClass.template GetConstructor<jni::jlong>(env);
-        return GeoJSONSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this));
+        return jni::Object<Source>(GeoJSONSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this)).Get());
     }
 
     void GeoJSONSource::registerNative(jni::JNIEnv& env) {

diff --git a/platform/android/src/style/sources/geojson_source.hpp b/platform/android/src/style/sources/geojson_source.hpp
@@ -21,7 +21,7 @@ class GeoJSONSource : public Source {
 
     GeoJSONSource(jni::JNIEnv&, jni::String, jni::Object<>);
 
-    GeoJSONSource(mbgl::style::GeoJSONSource&);
+    GeoJSONSource(jni::JNIEnv&, mbgl::style::Source&, AndroidRendererFrontend&);
 
     ~GeoJSONSource();
 
@@ -40,7 +40,8 @@ class GeoJSONSource : public Source {
 
     jni::String getURL(jni::JNIEnv&);
 
-    jni::jobject* createJavaPeer(jni::JNIEnv&);
+private:
+    jni::Object<Source> createJavaPeer(jni::JNIEnv&);
 
 }; // class GeoJSONSource
 

diff --git a/platform/android/src/style/sources/image_source.cpp b/platform/android/src/style/sources/image_source.cpp
@@ -23,8 +23,10 @@ namespace android {
             ) {
     }
 
-    ImageSource::ImageSource(mbgl::style::ImageSource& coreSource)
-        : Source(coreSource) {
+    ImageSource::ImageSource(jni::JNIEnv& env,
+                             mbgl::style::Source& coreSource,
+                             AndroidRendererFrontend& frontend)
+            : Source(env, coreSource, createJavaPeer(env), frontend) {
     }
 
     ImageSource::~ImageSource() = default;
@@ -45,9 +47,9 @@ namespace android {
 
     jni::Class<ImageSource> ImageSource::javaClass;
 
-    jni::jobject* ImageSource::createJavaPeer(jni::JNIEnv& env) {
+    jni::Object<Source> ImageSource::createJavaPeer(jni::JNIEnv& env) {
         static auto constructor = ImageSource::javaClass.template GetConstructor<jni::jlong>(env);
-        return ImageSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this));
+        return jni::Object<Source>(ImageSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this)).Get());
     }
 
     void ImageSource::registerNative(jni::JNIEnv& env) {

diff --git a/platform/android/src/style/sources/image_source.hpp b/platform/android/src/style/sources/image_source.hpp
@@ -21,7 +21,7 @@ class ImageSource : public Source {
 
     ImageSource(jni::JNIEnv&, jni::String, jni::Object<LatLngQuad>);
 
-    ImageSource(mbgl::style::ImageSource&);
+    ImageSource(jni::JNIEnv&, mbgl::style::Source&, AndroidRendererFrontend&);
 
     ~ImageSource();
 
@@ -30,7 +30,8 @@ class ImageSource : public Source {
 
     void setImage(jni::JNIEnv&, jni::Object<Bitmap>);
 
-    jni::jobject* createJavaPeer(jni::JNIEnv&);
+private:
+    jni::Object<Source> createJavaPeer(jni::JNIEnv&);
 
 }; // class ImageSource
 

diff --git a/platform/android/src/style/sources/raster_source.cpp b/platform/android/src/style/sources/raster_source.cpp
@@ -22,8 +22,10 @@ namespace android {
         ) {
     }
 
-    RasterSource::RasterSource(mbgl::style::RasterSource& coreSource)
-        : Source(coreSource) {
+    RasterSource::RasterSource(jni::JNIEnv& env,
+                               mbgl::style::Source& coreSource,
+                               AndroidRendererFrontend& frontend)
+        : Source(env, coreSource, createJavaPeer(env), frontend) {
     }
 
     RasterSource::~RasterSource() = default;
@@ -35,9 +37,9 @@ namespace android {
 
     jni::Class<RasterSource> RasterSource::javaClass;
 
-    jni::jobject* RasterSource::createJavaPeer(jni::JNIEnv& env) {
+    jni::Object<Source> RasterSource::createJavaPeer(jni::JNIEnv& env) {
         static auto constructor = RasterSource::javaClass.template GetConstructor<jni::jlong>(env);
-        return RasterSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this));
+        return jni::Object<Source>(RasterSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this)).Get());
     }
 
     void RasterSource::registerNative(jni::JNIEnv& env) {

diff --git a/platform/android/src/style/sources/raster_source.hpp b/platform/android/src/style/sources/raster_source.hpp
@@ -18,13 +18,14 @@ class RasterSource : public Source {
 
     RasterSource(jni::JNIEnv&, jni::String, jni::Object<>, jni::jint);
 
-    RasterSource(mbgl::style::RasterSource&);
+    RasterSource(jni::JNIEnv&, mbgl::style::Source&, AndroidRendererFrontend&);
 
     ~RasterSource();
 
     jni::String getURL(jni::JNIEnv&);
 
-    jni::jobject* createJavaPeer(jni::JNIEnv&);
+private:
+    jni::Object<Source> createJavaPeer(jni::JNIEnv&);
 
 }; // class RasterSource