[core][android] Introduce OfflineManager.runPackDatabaseAutomatically(boolean) API

include/mbgl/storage/default_file_source.hpp

@@ -121,20 +121,18 @@ 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.
      *
+     * Note that this operation can be potentially slow if packing the database occurs
+     * automatically (see runPackDatabaseAutomatically() and packDatabase()).
+     *
      * 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 deleteOfflineRegion(OfflineRegion&&, std::function<void(std::exception_ptr)>, bool pack = true);
+    void deleteOfflineRegion(OfflineRegion&&, std::function<void(std::exception_ptr)>);
 
     /*
      * Invalidate all the tiles from an offline region forcing Mapbox GL to revalidate
@@ -189,12 +187,25 @@ class DefaultFileSource : public FileSource {
     /*
      * Packs the existing database file into a minimal amount of disk space.
      *
+     * This operation has a performance impact as it will vacuum the database,
+     * forcing it to move pages on the filesystem.
+     *
      * 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);
 
+    /*
+     * Sets whether packing the database file occurs automatically after an offline
+     * region is deleted (deleteOfflineRegion()) or the ambient cache is cleared
+     * (clearAmbientCache()).
+     *
+     * By default, packing is enabled. If disabled, disk space will not be freed
+     * after resources are removed unless packDatabase() is explicitly called.
+     */
+    void runPackDatabaseAutomatically(bool);
+
     /*
      * Forces revalidation of the ambient cache.
      *
@@ -212,9 +223,10 @@ class DefaultFileSource : public FileSource {
     /*
      * Erase resources from the ambient cache, freeing storage space.
      *
-     * Erases the ambient cache, freeing resources. This operation can be
-     * potentially slow because it will trigger a VACUUM on SQLite,
-     * forcing the database to move pages on the filesystem.
+     * Erases the ambient cache, freeing resources.
+     *
+     * Note that this operation can be potentially slow if packing the database
+     * occurs automatically (see runPackDatabaseAutomatically() and packDatabase()).
      *
      * Resources overlapping with offline regions will not be affected
      * by this call.

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

@@ -309,6 +309,9 @@ public void run() {
   /**
    * Packs the existing database file into a minimal amount of disk space.
    * <p>
+   * This operation has a performance impact as it will vacuum the database,
+   * forcing it to move pages on the filesystem.
+   * <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.
@@ -396,9 +399,10 @@ public void run() {
   /**
    * Erase resources from the ambient cache, freeing storage space.
    * <p>
-   * Erases the ambient cache, freeing resources. This operation can be
-   * potentially slow because it will trigger a VACUUM on SQLite,
-   * forcing the database to move pages on the filesystem.
+   * Erases the ambient cache, freeing resources.
+   * <p>
+   * Note that this operation can be potentially slow if packing the database
+   * occurs automatically ({@link OfflineManager#runPackDatabaseAutomatically(boolean)}).
    * </p>
    * <p>
    * Resources overlapping with offline regions will not be affected
@@ -661,14 +665,32 @@ private boolean isValidOfflineRegionDefinition(OfflineRegionDefinition definitio
    * <p>
    * Once this limit is reached, {@link OfflineRegion.OfflineRegionObserver#mapboxTileCountLimitExceeded(long)}
    * fires every additional attempt to download additional tiles until already downloaded tiles are removed
-   * by calling {@link OfflineRegion#deleteOfflineRegion(OfflineRegion.OfflineRegionDeleteCallback)}.
+   * by calling {@link OfflineRegion#delete(OfflineRegion.OfflineRegionDeleteCallback)}.
    * </p>
    *
    * @param limit the maximum number of tiles allowed to be downloaded
    */
   @Keep
   public native void setOfflineMapboxTileCountLimit(long limit);
 
+  /**
+   * Sets whether database file packing occurs automatically.
+   * By default, the automatic database file packing is enabled.
+   * <p>
+   * If packing is enabled, database file packing occurs automatically
+   * after an offline region is deleted by calling
+   * {@link OfflineRegion#delete(OfflineRegion.OfflineRegionDeleteCallback)}
+   * or the ambient cache is cleared by calling {@link OfflineManager#clearAmbientCache()}.
+   *
+   * If packing is disabled, disk space will not be freed after
+   * resources are removed unless {@link OfflineManager#packDatabase()} is explicitly called.
+   * </p>
+   *
+   * @param autopack flag setting the automatic database file packing.
+   */
+  @Keep
+  public native void runPackDatabaseAutomatically(boolean autopack);
+
   @Keep
   private native void initialize(FileSource fileSource);
 

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

@@ -388,52 +388,8 @@ public void run() {
    * by other regions, until the database shrinks below a certain size.
    * </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 delete(@NonNull final OfflineRegionDeleteCallback callback) {
-    if (!isDeleted) {
-      isDeleted = true;
-      fileSource.activate();
-      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.
+   * Note that this operation can be potentially slow if packing the database
+   * occurs automatically ({@link OfflineManager#runPackDatabaseAutomatically(boolean)}).
    * </p>
    * <p>
    * When the operation is complete or encounters an error, the given callback will be
@@ -445,11 +401,11 @@ public void run() {
    *
    * @param callback the callback to be invoked
    */
-  public void deleteAndSkipPackDatabase(@NonNull final OfflineRegionDeleteCallback callback) {
+  public void delete(@NonNull final OfflineRegionDeleteCallback callback) {
     if (!isDeleted) {
       isDeleted = true;
       fileSource.activate();
-      deleteOfflineRegion(false /*pack*/, new OfflineRegionDeleteCallback() {
+      deleteOfflineRegion(new OfflineRegionDeleteCallback() {
         @Override
         public void onDelete() {
           handler.post(new Runnable() {
@@ -569,7 +525,7 @@ public void run() {
   private native void getOfflineRegionStatus(OfflineRegionStatusCallback callback);
 
   @Keep
-  private native void deleteOfflineRegion(boolean pack, OfflineRegionDeleteCallback callback);
+  private native void deleteOfflineRegion(OfflineRegionDeleteCallback callback);
 
   @Keep
   private native void updateOfflineRegionMetadata(byte[] metadata, OfflineRegionUpdateMetadataCallback callback);

platform/android/src/offline/offline_manager.cpp

@@ -168,6 +168,10 @@ void OfflineManager::setMaximumAmbientCacheSize(jni::JNIEnv& env_, const jni::jl
             std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
+void OfflineManager::runPackDatabaseAutomatically(jni::JNIEnv&, jboolean autopack) {
+    fileSource->runPackDatabaseAutomatically(autopack);
+}
+
 // FileSource::FileSourceCallback //
 
 void OfflineManager::FileSourceCallback::onSuccess(jni::JNIEnv& env,
@@ -211,6 +215,7 @@ void OfflineManager::registerNative(jni::JNIEnv& env) {
         METHOD(&OfflineManager::invalidateAmbientCache, "nativeInvalidateAmbientCache"),
         METHOD(&OfflineManager::clearAmbientCache, "nativeClearAmbientCache"),
         METHOD(&OfflineManager::setMaximumAmbientCacheSize, "nativeSetMaximumAmbientCacheSize"),
+        METHOD(&OfflineManager::runPackDatabaseAutomatically, "runPackDatabaseAutomatically"),
         METHOD(&OfflineManager::putResourceWithUrl, "putResourceWithUrl"));
 }
 

platform/android/src/offline/offline_manager.hpp

@@ -103,6 +103,8 @@ class OfflineManager {
 
     void setMaximumAmbientCacheSize(jni::JNIEnv&, const jni::jlong size, const jni::Object<FileSourceCallback>& callback_);
 
+    void runPackDatabaseAutomatically(jni::JNIEnv&, jboolean autopack);
+
 private:
     std::shared_ptr<mbgl::DefaultFileSource> fileSource;
 };

platform/android/src/offline/offline_region.cpp

@@ -101,31 +101,23 @@ void OfflineRegion::getOfflineRegionStatus(jni::JNIEnv& env_, const jni::Object<
     });
 }
 
-namespace {
-// Reattach, the callback comes from a different thread
-void handleException(std::exception_ptr exception,
-                     const jni::Object<OfflineRegion::OfflineRegionDeleteCallback>& callback,
-                     android::UniqueEnv env = android::AttachEnv()) {
-    if (exception) {
-        OfflineRegion::OfflineRegionDeleteCallback::onError(*env, callback, exception);
-    } else {
-        OfflineRegion::OfflineRegionDeleteCallback::onDelete(*env, callback);
-    }
-}
-} // namespace
-
-void OfflineRegion::deleteOfflineRegion(jni::JNIEnv& env_,
-                                        jni::jboolean pack,
-                                        const jni::Object<OfflineRegionDeleteCallback>& callback_) {
+void OfflineRegion::deleteOfflineRegion(jni::JNIEnv& env_, const jni::Object<OfflineRegionDeleteCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->deleteOfflineRegion(
-        std::move(*region),
-        [
-            // Ensure the object is not gc'd in the meanwhile
-            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
-            std::exception_ptr error) mutable { handleException(error, *callback); },
-        pack);
+    fileSource->deleteOfflineRegion(std::move(*region),
+                                    [
+                                        // Ensure the object is not gc'd in the meanwhile
+                                        callback = std::make_shared<decltype(globalCallback)>(
+                                            std::move(globalCallback))](std::exception_ptr error) mutable {
+                                        // Reattach, the callback comes from a different thread
+                                        android::UniqueEnv env = android::AttachEnv();
+
+                                        if (error) {
+                                            OfflineRegionDeleteCallback::onError(*env, *callback, error);
+                                        } else {
+                                            OfflineRegionDeleteCallback::onDelete(*env, *callback);
+                                        }
+                                    });
 }
 
 void OfflineRegion::invalidateOfflineRegion(jni::JNIEnv& env_,

platform/android/src/offline/offline_region.hpp

@@ -69,7 +69,7 @@ class OfflineRegion {
 
     void getOfflineRegionStatus(jni::JNIEnv&, const jni::Object<OfflineRegion::OfflineRegionStatusCallback>&);
 
-    void deleteOfflineRegion(jni::JNIEnv&, jni::jboolean pack, const jni::Object<OfflineRegionDeleteCallback>&);
+    void deleteOfflineRegion(jni::JNIEnv&, const jni::Object<OfflineRegionDeleteCallback>&);
 
     void invalidateOfflineRegion(jni::JNIEnv&, const jni::Object<OfflineRegionInvalidateCallback>&);
 

platform/default/include/mbgl/storage/offline_database.hpp

@@ -74,7 +74,7 @@ class OfflineDatabase : private util::noncopyable {
     expected<OfflineRegionMetadata, std::exception_ptr>
     updateMetadata(const int64_t regionID, const OfflineRegionMetadata&);
 
-    std::exception_ptr deleteRegion(OfflineRegion&&, bool pack = true);
+    std::exception_ptr deleteRegion(OfflineRegion&&);
     std::exception_ptr invalidateRegion(int64_t regionID);
 
     // Return value is (response, stored size)
@@ -94,6 +94,7 @@ class OfflineDatabase : private util::noncopyable {
     bool exceedsOfflineMapboxTileCountLimit(const Resource&);
     void markUsedResources(int64_t regionID, const std::list<Resource>&);
     std::exception_ptr pack();
+    void runPackDatabaseAutomatically(bool autopack_) { autopack = autopack_; }
 
 private:
     void initialize();
@@ -149,6 +150,7 @@ class OfflineDatabase : private util::noncopyable {
     optional<uint64_t> offlineMapboxTileCount;
 
     bool evict(uint64_t neededFreeSize);
+    bool autopack = true;
 };
 
 } // namespace mbgl

platform/default/src/mbgl/storage/default_file_source.cpp

@@ -82,9 +82,9 @@ class DefaultFileSource::Impl {
         }
     }
 
-    void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback, bool pack) {
+    void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
         downloads.erase(region.getID());
-        callback(offlineDatabase->deleteRegion(std::move(region), pack));
+        callback(offlineDatabase->deleteRegion(std::move(region)));
     }
 
     void invalidateRegion(int64_t regionID, std::function<void (std::exception_ptr)> callback) {
@@ -208,6 +208,8 @@ class DefaultFileSource::Impl {
 
     void packDatabase(std::function<void(std::exception_ptr)> callback) { callback(offlineDatabase->pack()); }
 
+    void runPackDatabaseAutomatically(bool autopack) { offlineDatabase->runPackDatabaseAutomatically(autopack); }
+
 private:
     expected<OfflineDownload*, std::exception_ptr> getDownload(int64_t regionID) {
         auto it = downloads.find(regionID);
@@ -316,10 +318,8 @@ void DefaultFileSource::updateOfflineMetadata(const int64_t regionID,
     impl->actor().invoke(&Impl::updateMetadata, regionID, metadata, callback);
 }
 
-void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region,
-                                            std::function<void(std::exception_ptr)> callback,
-                                            bool pack) {
-    impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback, pack);
+void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
+    impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback);
 }
 
 void DefaultFileSource::invalidateOfflineRegion(OfflineRegion& region,
@@ -363,6 +363,10 @@ void DefaultFileSource::packDatabase(std::function<void(std::exception_ptr)> cal
     impl->actor().invoke(&Impl::packDatabase, std::move(callback));
 }
 
+void DefaultFileSource::runPackDatabaseAutomatically(bool autopack) {
+    impl->actor().invoke(&Impl::runPackDatabaseAutomatically, autopack);
+}
+
 void DefaultFileSource::invalidateAmbientCache(std::function<void (std::exception_ptr)> callback) {
     impl->actor().invoke(&Impl::invalidateAmbientCache, std::move(callback));
 }

platform/default/src/mbgl/storage/offline_database.cpp

@@ -154,7 +154,7 @@ void OfflineDatabase::removeExisting() {
 void OfflineDatabase::removeOldCacheTable() {
     assert(db);
     db->exec("DROP TABLE IF EXISTS http_cache");
-    vacuum();
+    if (autopack) vacuum();
 }
 
 void OfflineDatabase::createSchema() {
@@ -705,7 +705,7 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try {
 
     resourceQuery.run();
 
-    vacuum();
+    if (autopack) vacuum();
 
     return nullptr;
 } catch (...) {
@@ -884,7 +884,7 @@ OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetad
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
-std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pack) try {
+std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try {
     {
         mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") };
         query.bind(1, region.getID());
@@ -893,7 +893,7 @@ std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pa
 
     evict(0);
     assert(db);
-    if (pack) vacuum();
+    if (autopack) vacuum();
 
     // Ensure that the cached offlineTileCount value is recalculated.
     offlineMapboxTileCount = nullopt;
@@ -1235,7 +1235,7 @@ std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
 
         if (databaseSize > maximumAmbientCacheSize) {
             evict(0);
-            vacuum();
+            if (autopack) vacuum();
         }
 
         return nullptr;