[android] use a single DefaultFileSource for all mapviews + jni peer abstractionupstream/tvn-wip-state-keeping-sync

9 files changed, 164 insertions, 23 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/fs/FileSource.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/fs/FileSource.java
new file mode 100644
index 0000000000..1ec6a8db61
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/fs/FileSource.java
@@ -0,0 +1,41 @@
+package com.mapbox.mapboxsdk.fs;
+
+import android.content.Context;
+
+import com.mapbox.mapboxsdk.offline.OfflineManager;
+
+import java.lang.ref.WeakReference;
+
+/**
+ * Holds a central reference to the core's DefaultFileSource for as long as
+ * there are active mapviews / offline managers
+ */
+public class FileSource {
+
+  // Use weak reference to avoid blocking GC on this reference.
+  // Should only block when mapview / Offline manager instances
+  // are alive
+  private static WeakReference<FileSource> INSTANCE;
+
+  public static synchronized FileSource getInstance(Context context) {
+    if (INSTANCE == null || INSTANCE.get() == null) {
+      String cachePath = OfflineManager.getDatabasePath(context);
+      String apkPath = context.getPackageCodePath();
+      INSTANCE = new WeakReference<>(new FileSource(cachePath, apkPath));
+    }
+
+    return INSTANCE.get();
+  }
+
+  private long nativePtr;
+
+  private FileSource(String cachePath, String apkPath) {
+    initialize(cachePath, apkPath);
+  }
+
+  private native void initialize(String cachePath, String apkPath);
+
+  @Override
+  protected native void finalize() throws Throwable;
+
+}
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
index e6c08ef021..0cf8124c4b 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
@@ -36,6 +36,7 @@ import com.mapbox.mapboxsdk.annotations.MarkerViewManager;
 import com.mapbox.mapboxsdk.camera.CameraPosition;
 import com.mapbox.mapboxsdk.constants.MapboxConstants;
 import com.mapbox.mapboxsdk.constants.Style;
+import com.mapbox.mapboxsdk.fs.FileSource;
 import com.mapbox.mapboxsdk.maps.widgets.CompassView;
 import com.mapbox.mapboxsdk.maps.widgets.MyLocationView;
 import com.mapbox.mapboxsdk.maps.widgets.MyLocationViewSettings;
@@ -137,6 +138,9 @@ public class MapView extends FrameLayout {
     attrView = (ImageView) view.findViewById(R.id.attributionView);
     logoView = view.findViewById(R.id.logoView);
 
+    // Get a file source reference on the main thread
+    final FileSource fileSource = FileSource.getInstance(context);
+
     glSurfaceView = (GLSurfaceView) findViewById(R.id.surfaceView);
     glSurfaceView.setEGLConfigChooser(8, 8, 8, 0 /** TODO: What alpha value do we need here?? */, 16, 8);
     glSurfaceView.setEGLContextClientVersion(2);
@@ -148,7 +152,7 @@ public class MapView extends FrameLayout {
         glSurfaceView.setRenderMode(RENDERMODE_WHEN_DIRTY);
 
         // Create native Map object
-        nativeMapView = new NativeMapView(MapView.this);
+        nativeMapView = new NativeMapView(MapView.this, fileSource);
         nativeMapView.setAccessToken(Mapbox.getAccessToken());
         nativeMapView.setReachability(isConnected());
 
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
index 31a5d4046f..720eaa64df 100755
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
@@ -13,9 +13,9 @@ import android.util.DisplayMetrics;
 import com.mapbox.mapboxsdk.annotations.Marker;
 import com.mapbox.mapboxsdk.annotations.Polygon;
 import com.mapbox.mapboxsdk.annotations.Polyline;
+import com.mapbox.mapboxsdk.fs.FileSource;
 import com.mapbox.mapboxsdk.geometry.LatLng;
 import com.mapbox.mapboxsdk.geometry.ProjectedMeters;
-import com.mapbox.mapboxsdk.offline.OfflineManager;
 import com.mapbox.mapboxsdk.style.layers.CannotAddLayerException;
 import com.mapbox.mapboxsdk.style.layers.Layer;
 import com.mapbox.mapboxsdk.style.layers.NoSuchLayerException;
@@ -41,6 +41,9 @@ final class NativeMapView {
   // Used for callbacks
   private MapView mapView;
 
+  //Hold a reference to prevent it from being GC'd as long as it's used on the native side
+  private final FileSource fileSource;
+
   // Device density
   private final float pixelRatio;
 
@@ -62,14 +65,13 @@ final class NativeMapView {
   // Constructors
   //
 
-  NativeMapView(MapView mapView) {
+  NativeMapView(MapView mapView, FileSource fileSource) {
     this.mapView = mapView;
+    this.fileSource = fileSource;
 
     Context context = mapView.getContext();
-    String cachePath = OfflineManager.getDatabasePath(context);
 
-    pixelRatio = context.getResources().getDisplayMetrics().density;
-    String apkPath = context.getPackageCodePath();
+    this.pixelRatio = context.getResources().getDisplayMetrics().density;
 
     int availableProcessors = Runtime.getRuntime().availableProcessors();
 
@@ -90,14 +92,14 @@ final class NativeMapView {
       throw new IllegalArgumentException("totalMemory cannot be negative.");
     }
 
-    initialize(this, cachePath, apkPath, pixelRatio, availableProcessors, totalMemory);
+    initialize(this, fileSource, pixelRatio, availableProcessors, totalMemory);
   }
 
   //
   // Methods
   //
 
-  private native void initialize(NativeMapView nativeMapView, String cachePath, String apkPath, float pixelRatio,
+  private native void initialize(NativeMapView nativeMapView, FileSource fileSource, float pixelRatio,
                                  int availableProcessors, long totalMemory);
 
   native void destroy();
diff --git a/platform/android/config.cmake b/platform/android/config.cmake
index c24b816c04..0bdb5640fd 100644
--- a/platform/android/config.cmake
+++ b/platform/android/config.cmake
@@ -129,6 +129,10 @@ macro(mbgl_platform_core)
         platform/android/src/style/functions/interval_stops.cpp
         platform/android/src/style/functions/interval_stops.hpp
 
+        # FileSource holder
+        platform/android/src/file_source.cpp
+        platform/android/src/file_source.hpp
+
         # Connectivity
         platform/android/src/connectivity_listener.cpp
         platform/android/src/connectivity_listener.hpp
diff --git a/platform/android/src/file_source.cpp b/platform/android/src/file_source.cpp
new file mode 100644
index 0000000000..9ba413c352
--- /dev/null
+++ b/platform/android/src/file_source.cpp
@@ -0,0 +1,53 @@
+#include "file_source.hpp"
+
+#include <mbgl/util/logging.hpp>
+
+#include <string>
+
+
+namespace mbgl {
+namespace android {
+
+FileSource::FileSource(jni::JNIEnv& _env, jni::String _cachePath, jni::String _apkPath) {
+    Log::Info(Event::JNI, "FileSource::FileSource");
+
+    // Create a core default file source
+    fileSource = std::make_unique<mbgl::DefaultFileSource>(
+        jni::Make<std::string>(_env, _cachePath) + "/mbgl-offline.db",
+        jni::Make<std::string>(_env, _apkPath));
+}
+
+FileSource::~FileSource() {
+    Log::Info(Event::JNI, "FileSource::~FileSource");
+}
+
+jni::Class<FileSource> FileSource::javaClass;
+
+FileSource* FileSource::getNativePeer(jni::JNIEnv& env, jni::Object<FileSource> jFileSource) {
+    static auto field = FileSource::javaClass.GetField<jlong>(env, "nativePtr");
+    return reinterpret_cast<FileSource *>(jFileSource.Get(env, field));
+}
+
+mbgl::DefaultFileSource& FileSource::getDefaultFileSource(jni::JNIEnv& env, jni::Object<FileSource> jFileSource) {
+    FileSource* fileSource = FileSource::getNativePeer(env, jFileSource);
+    assert(fileSource != nullptr);
+    return *fileSource->fileSource;
+}
+
+void FileSource::registerNative(jni::JNIEnv& env) {
+    FileSource::javaClass = *jni::Class<FileSource>::Find(env).NewGlobalRef(env).release();
+
+    #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)
+
+    // Register the peer
+    jni::RegisterNativePeer<FileSource>(
+        env, FileSource::javaClass, "nativePtr",
+        std::make_unique<FileSource, JNIEnv&, jni::String, jni::String>,
+        "initialize",
+        "finalize"
+    );
+}
+
+
+} // namespace android
+} // namespace mbgl
\ No newline at end of file
diff --git a/platform/android/src/file_source.hpp b/platform/android/src/file_source.hpp
new file mode 100644
index 0000000000..8bc4639047
--- /dev/null
+++ b/platform/android/src/file_source.hpp
@@ -0,0 +1,37 @@
+#pragma once
+
+#include <mbgl/storage/default_file_source.hpp>
+
+#include <jni/jni.hpp>
+
+namespace mbgl {
+namespace android {
+
+/**
+ * Peer class for the Android FileSource holder. Ensures that a single DefaultFileSource is used
+ */
+class FileSource {
+public:
+
+    static constexpr auto Name() { return "com/mapbox/mapboxsdk/fs/FileSource"; };
+
+    FileSource(jni::JNIEnv&, jni::String, jni::String);
+
+    ~FileSource();
+
+    static jni::Class<FileSource> javaClass;
+
+    static FileSource* getNativePeer(jni::JNIEnv&, jni::Object<FileSource>);
+
+    static mbgl::DefaultFileSource& getDefaultFileSource(jni::JNIEnv&, jni::Object<FileSource>);
+
+    static void registerNative(jni::JNIEnv&);
+
+private:
+
+    std::unique_ptr<mbgl::DefaultFileSource> fileSource;
+};
+
+
+} // namespace android
+} // namespace mbgl
\ No newline at end of file
diff --git a/platform/android/src/jni.cpp b/platform/android/src/jni.cpp
index e1164d3d2a..7676b29293 100755
--- a/platform/android/src/jni.cpp
+++ b/platform/android/src/jni.cpp
@@ -9,6 +9,7 @@
 #include <sys/system_properties.h>
 
 #include "jni.hpp"
+#include "file_source.hpp"
 #include "java_types.hpp"
 #include "native_map_view.hpp"
 #include "bitmap.hpp"
@@ -706,8 +707,9 @@ void registerNatives(JavaVM *vm) {
 
     jni::JNIEnv& env = jni::GetEnv(*vm, jni::jni_version_1_6);
 
-    //For the DefaultFileSource
+    // For the DefaultFileSource
     static mbgl::util::RunLoop mainRunLoop;
+    FileSource::registerNative(env);
 
     //Basic types
     java::registerNatives(env);
diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
index 91f932ed2f..6ba836758b 100755
--- a/platform/android/src/native_map_view.cpp
+++ b/platform/android/src/native_map_view.cpp
@@ -39,9 +39,10 @@ namespace android {
 
 using DebugOptions = mbgl::MapDebugOptions;
 
-NativeMapView::NativeMapView(jni::JNIEnv& _env, jni::Object<NativeMapView> _obj, jni::String _cachePath, jni::String _apkPath,
+NativeMapView::NativeMapView(jni::JNIEnv& _env, jni::Object<NativeMapView> _obj, jni::Object<FileSource> jFileSource,
                              jni::jfloat _pixelRatio, jni::jint _availableProcessors, jni::jlong _totalMemory) :
     javaPeer(_obj.NewWeakGlobalRef(_env)),
+    fileSource(mbgl::android::FileSource::getDefaultFileSource(_env, jFileSource)),
     pixelRatio(_pixelRatio),
     availableProcessors(_availableProcessors),
     totalMemory(_totalMemory),
@@ -61,15 +62,10 @@ NativeMapView::NativeMapView(jni::JNIEnv& _env, jni::Object<NativeMapView> _obj,
         return;
     }
 
-    // Create a default file source for this map instance
-    fileSource = std::make_unique<mbgl::DefaultFileSource>(
-        jni::Make<std::string>(_env, _cachePath) + "/mbgl-offline.db",
-        jni::Make<std::string>(_env, _apkPath));
-
     // Create the core map
     map = std::make_unique<mbgl::Map>(
         *this, mbgl::Size{ static_cast<uint32_t>(width), static_cast<uint32_t>(height) },
-        pixelRatio, *fileSource, threadPool, MapMode::Continuous);
+        pixelRatio, fileSource, threadPool, MapMode::Continuous);
 
     //Calculate a fitting cache size based on device parameters
     float zoomFactor   = map->getMaxZoom() - map->getMinZoom() + 1;
@@ -91,8 +87,8 @@ NativeMapView::~NativeMapView() {
     mbgl::util::RunLoop::Impl* loop = reinterpret_cast<mbgl::util::RunLoop::Impl*>(mbgl::util::RunLoop::getLoopHandle());
     loop->setWakeFunction(nullptr);
 
+    //Destroy map instance
     map.reset();
-    fileSource.reset();
 
     vm = nullptr;
 }
@@ -176,7 +172,7 @@ void NativeMapView::render(jni::JNIEnv& env) {
 }
 
 void NativeMapView::setAPIBaseUrl(jni::JNIEnv& env, jni::String url) {
-    fileSource->setAPIBaseURL(jni::Make<std::string>(env, url));
+    fileSource.setAPIBaseURL(jni::Make<std::string>(env, url));
 }
 
 jni::String NativeMapView::getStyleUrl(jni::JNIEnv& env) {
@@ -196,11 +192,11 @@ void NativeMapView::setStyleJson(jni::JNIEnv& env, jni::String json) {
 }
 
 jni::String NativeMapView::getAccessToken(jni::JNIEnv& env) {
-    return jni::Make<jni::String>(env, fileSource->getAccessToken());
+    return jni::Make<jni::String>(env, fileSource.getAccessToken());
 }
 
 void NativeMapView::setAccessToken(jni::JNIEnv& env, jni::String token) {
-    fileSource->setAccessToken(jni::Make<std::string>(env, token));
+    fileSource.setAccessToken(jni::Make<std::string>(env, token));
 }
 
 void NativeMapView::cancelTransitions(jni::JNIEnv&) {
@@ -812,7 +808,7 @@ void NativeMapView::registerNative(jni::JNIEnv& env) {
 
     // Register the peer
     jni::RegisterNativePeer<NativeMapView>(env, NativeMapView::javaClass, "nativePtr",
-            std::make_unique<NativeMapView, JNIEnv&, jni::Object<NativeMapView>, jni::String, jni::String, jni::jfloat, jni::jint, jni::jlong>,
+            std::make_unique<NativeMapView, JNIEnv&, jni::Object<NativeMapView>, jni::Object<FileSource>, jni::jfloat, jni::jint, jni::jlong>,
             "initialize",
             "destroy",
             METHOD(&NativeMapView::render, "render"),
diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp
index ad7fbbc0a1..b42c873a51 100755
--- a/platform/android/src/native_map_view.hpp
+++ b/platform/android/src/native_map_view.hpp
@@ -10,6 +10,7 @@
 #include <mbgl/storage/default_file_source.hpp>
 #include <mbgl/storage/network_status.hpp>
 
+#include "file_source.hpp"
 #include "annotation/marker.hpp"
 #include "annotation/polygon.hpp"
 #include "annotation/polyline.hpp"
@@ -37,7 +38,7 @@ public:
 
     static void registerNative(jni::JNIEnv&);
 
-    NativeMapView(jni::JNIEnv&, jni::Object<NativeMapView>, jni::String, jni::String, jni::jfloat, jni::jint, jni::jlong);
+    NativeMapView(jni::JNIEnv&, jni::Object<NativeMapView>, jni::Object<FileSource>, jni::jfloat, jni::jint, jni::jlong);
 
     virtual ~NativeMapView();
 
@@ -225,6 +226,8 @@ private:
     JavaVM *vm = nullptr;
     jni::UniqueWeakObject<NativeMapView> javaPeer;
 
+    mbgl::DefaultFileSource& fileSource;
+
     std::string styleUrl;
     std::string apiKey;
 
@@ -245,7 +248,6 @@ private:
 
     // Ensure these are initialised last
     std::unique_ptr<mbgl::util::RunLoop> runLoop;
-    std::unique_ptr<mbgl::DefaultFileSource> fileSource;
     mbgl::ThreadPool threadPool;
     std::unique_ptr<mbgl::Map> map;
     mbgl::EdgeInsets insets;