[core][android]Decouple offline storage vacuum and the deleteRegion API

include/mbgl/storage/default_file_source.hpp

@@ -121,6 +121,11 @@ class DefaultFileSource : public FileSource {
      * Eviction works by removing the least-recently requested resources not also required
      * by other regions, until the database shrinks below a certain size.
      *
+     * If the |pack| argument is `false` the database file packing is skipped and the
+     * database file does not shrink after this operation completes. Database file
+     * packing can be done later with `packDatabase()`. It is a useful optimization
+     * e.g. when several regions should be deleted in a row.
+     *
      * Note that this method takes ownership of the input, reflecting the fact that once
      * region deletion is initiated, it is not legal to perform further actions with the
      * region.
@@ -129,15 +134,15 @@ class DefaultFileSource : public FileSource {
      * executed on the database thread; it is the responsibility of the SDK bindings
      * to re-execute a user-provided callback on the main thread.
      */
-    void deleteOfflineRegion(OfflineRegion&&, std::function<void (std::exception_ptr)>);
+    void deleteOfflineRegion(OfflineRegion&&, std::function<void(std::exception_ptr)>, bool pack = true);
 
     /*
      * Invalidate all the tiles from an offline region forcing Mapbox GL to revalidate
      * the tiles with the server before using. This is more efficient than deleting the
      * offline region and downloading it again because if the data on the cache matches
      * the server, no new data gets transmitted.
      */
-    void invalidateOfflineRegion(OfflineRegion&, std::function<void (std::exception_ptr)>);
+    void invalidateOfflineRegion(OfflineRegion&, std::function<void(std::exception_ptr)>);
 
     /*
      * Changing or bypassing this limit without permission from Mapbox is prohibited
@@ -179,7 +184,16 @@ class DefaultFileSource : public FileSource {
      * executed on the database thread; it is the responsibility of the SDK bindings
      * to re-execute a user-provided callback on the main thread.
      */
-    void resetDatabase(std::function<void (std::exception_ptr)>);
+    void resetDatabase(std::function<void(std::exception_ptr)>);
+
+    /*
+     * Packs the existing database file into a minimal amount of disk space.
+     *
+     * When the operation is complete or encounters an error, the given callback will be
+     * executed on the database thread; it is the responsibility of the SDK bindings
+     * to re-execute a user-provided callback on the main thread.
+     */
+    void packDatabase(std::function<void(std::exception_ptr)> callback);
 
     /*
      * Forces revalidation of the ambient cache.

platform/android/CHANGELOG.md

@@ -14,6 +14,9 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
  - Convert GeoJSON features to tiles in a background thread and thus unblock the UI thread on updating the GeoJsonSource [#15871](https://github.com/mapbox/mapbox-gl-native/pull/15871)
  - Convert GeoJSON features to tiles for the loaded source description in a background thread and thus unblock the UI thread [#15885](https://github.com/mapbox/mapbox-gl-native/pull/15885)
 
+### Features
+- Introduce `OfflineManager#packDatabase` and `OfflineRegion#deleteAndSkipPackDatabase` API in order to decouple offline storage vacuum and delete region operations and thus to gain performance benefits e.g. when several regions should be deleted in a row [#15899](https://github.com/mapbox/mapbox-gl-native/pull/15899)
+
 ## 8.5.0-alpha.2 - October 10, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.5.0-alpha.1...android-v8.5.0-alpha.2) since [Mapbox Maps SDK for Android v8.5.0-alpha.1](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.5.0-alpha.1):
 

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java

@@ -306,6 +306,47 @@ public void run() {
     });
   }
 
+  /**
+   * Packs the existing database file into a minimal amount of disk space.
+   * <p>
+   * When the operation is complete or encounters an error, the given callback will be
+   * executed on the database thread; it is the responsibility of the SDK bindings
+   * to re-execute a user-provided callback on the main thread.
+   * </p>
+   *
+   * @param callback the callback to be invoked when the database was reset or when the operation erred.
+   */
+  public void packDatabase(@Nullable final FileSourceCallback callback) {
+    fileSource.activate();
+    nativePackDatabase(new FileSourceCallback() {
+      @Override
+      public void onSuccess() {
+        handler.post(new Runnable() {
+          @Override
+          public void run() {
+            fileSource.deactivate();
+            if (callback != null) {
+              callback.onSuccess();
+            }
+          }
+        });
+      }
+
+      @Override
+      public void onError(@NonNull final String message) {
+        handler.post(new Runnable() {
+          @Override
+          public void run() {
+            fileSource.deactivate();
+            if (callback != null) {
+              callback.onError(message);
+            }
+          }
+        });
+      }
+    });
+  }
+
   /**
    * Forces re-validation of the ambient cache.
    * <p>
@@ -648,6 +689,9 @@ private native void createOfflineRegion(FileSource fileSource, OfflineRegionDefi
   @Keep
   private native void nativeResetDatabase(@Nullable FileSourceCallback callback);
 
+  @Keep
+  private native void nativePackDatabase(@Nullable FileSourceCallback callback);
+
   @Keep
   private native void nativeInvalidateAmbientCache(@Nullable FileSourceCallback callback);
 

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegion.java

@@ -401,7 +401,55 @@ public void delete(@NonNull final OfflineRegionDeleteCallback callback) {
     if (!isDeleted) {
       isDeleted = true;
       fileSource.activate();
-      deleteOfflineRegion(new OfflineRegionDeleteCallback() {
+      deleteOfflineRegion(true /*pack*/, new OfflineRegionDeleteCallback() {
+        @Override
+        public void onDelete() {
+          handler.post(new Runnable() {
+            @Override
+            public void run() {
+              fileSource.deactivate();
+              callback.onDelete();
+              OfflineRegion.this.finalize();
+            }
+          });
+        }
+
+        @Override
+        public void onError(final String error) {
+          handler.post(new Runnable() {
+            @Override
+            public void run() {
+              isDeleted = false;
+              fileSource.deactivate();
+              callback.onError(error);
+            }
+          });
+        }
+      });
+    }
+  }
+
+  /**
+   * Same as {@link OfflineRegion#delete} but skipping database file packing for performance reasons.
+   * <p>
+   * Database file packing can be done later with {@link OfflineManager#packDatabase}.
+   * This method is a useful optimization e.g. when several regions should be deleted in a row.
+   * </p>
+   * <p>
+   * When the operation is complete or encounters an error, the given callback will be
+   * executed on the main thread.
+   * </p>
+   * <p>
+   * After you call this method, you may not call any additional methods on this object.
+   * </p>
+   *
+   * @param callback the callback to be invoked
+   */
+  public void deleteAndSkipPackDatabase(@NonNull final OfflineRegionDeleteCallback callback) {
+    if (!isDeleted) {
+      isDeleted = true;
+      fileSource.activate();
+      deleteOfflineRegion(false /*pack*/, new OfflineRegionDeleteCallback() {
         @Override
         public void onDelete() {
           handler.post(new Runnable() {
@@ -521,7 +569,7 @@ public void run() {
   private native void getOfflineRegionStatus(OfflineRegionStatusCallback callback);
 
   @Keep
-  private native void deleteOfflineRegion(OfflineRegionDeleteCallback callback);
+  private native void deleteOfflineRegion(boolean pack, OfflineRegionDeleteCallback callback);
 
   @Keep
   private native void updateOfflineRegionMetadata(byte[] metadata, OfflineRegionUpdateMetadataCallback callback);

platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/offline/CacheTest.kt

@@ -85,4 +85,20 @@ class CacheTest {
     }
     countDownLatch.await()
   }
+
+  @Test
+  fun testSetPackDatabase() {
+    rule.activity.runOnUiThread {
+      OfflineManager.getInstance(context).packDatabase(object : OfflineManager.FileSourceCallback {
+        override fun onSuccess() {
+          countDownLatch.countDown()
+        }
+
+        override fun onError(message: String) {
+          Assert.assertNull("onError should not be called", message)
+        }
+      })
+    }
+    countDownLatch.await()
+  }
 }

platform/android/src/offline/offline_manager.cpp

@@ -7,8 +7,21 @@
 namespace mbgl {
 namespace android {
 
-// OfflineManager //
+namespace {
+// Reattach, the callback comes from a different thread
+void handleException(std::exception_ptr exception,
+                     const jni::Object<OfflineManager::FileSourceCallback>& callback,
+                     android::UniqueEnv env = android::AttachEnv()) {
+    if (exception) {
+        OfflineManager::FileSourceCallback::onError(
+            *env, callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
+    } else {
+        OfflineManager::FileSourceCallback::onSuccess(*env, callback);
+    }
+}
+} // namespace
 
+// OfflineManager //
 OfflineManager::OfflineManager(jni::JNIEnv& env, const jni::Object<FileSource>& jFileSource)
     : fileSource(std::static_pointer_cast<DefaultFileSource>(mbgl::FileSource::getSharedFileSource(FileSource::getSharedResourceOptions(env, jFileSource)))) {}
 
@@ -107,77 +120,52 @@ void OfflineManager::mergeOfflineRegions(jni::JNIEnv& env_, const jni::Object<Fi
 void OfflineManager::resetDatabase(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->resetDatabase([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
+    fileSource->resetDatabase(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
+}
 
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
+void OfflineManager::packDatabase(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
+    auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->packDatabase(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::invalidateAmbientCache(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->invalidateAmbientCache([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->invalidateAmbientCache(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::clearAmbientCache(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->clearAmbientCache([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->clearAmbientCache(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::setMaximumAmbientCacheSize(jni::JNIEnv& env_, const jni::jlong size_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->setMaximumAmbientCacheSize(size_, [
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->setMaximumAmbientCacheSize(
+        size_,
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 // FileSource::FileSourceCallback //
@@ -207,7 +195,10 @@ void OfflineManager::registerNative(jni::JNIEnv& env) {
 
     #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)
 
-    jni::RegisterNativePeer<OfflineManager>( env, javaClass, "nativePtr",
+    jni::RegisterNativePeer<OfflineManager>(
+        env,
+        javaClass,
+        "nativePtr",
         jni::MakePeer<OfflineManager, const jni::Object<FileSource>&>,
         "initialize",
         "finalize",
@@ -216,6 +207,7 @@ void OfflineManager::registerNative(jni::JNIEnv& env) {
         METHOD(&OfflineManager::createOfflineRegion, "createOfflineRegion"),
         METHOD(&OfflineManager::mergeOfflineRegions, "mergeOfflineRegions"),
         METHOD(&OfflineManager::resetDatabase, "nativeResetDatabase"),
+        METHOD(&OfflineManager::packDatabase, "nativePackDatabase"),
         METHOD(&OfflineManager::invalidateAmbientCache, "nativeInvalidateAmbientCache"),
         METHOD(&OfflineManager::clearAmbientCache, "nativeClearAmbientCache"),
         METHOD(&OfflineManager::setMaximumAmbientCacheSize, "nativeSetMaximumAmbientCacheSize"),

platform/android/src/offline/offline_manager.hpp

@@ -95,6 +95,8 @@ class OfflineManager {
 
     void resetDatabase(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
 
+    void packDatabase(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
+
     void invalidateAmbientCache(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
 
     void clearAmbientCache(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);