[core] Fix offline status reporting regressions

The core of the change is ensuring that ensureResource doesn't release Zalgo: it should be consistently async, rather than async in the case that the resource doesn't exist in the database, but sync if it does.

This ensures that status is reported in a more consistent sequence, regardless of the database state.

2 files changed, 38 insertions, 29 deletions

diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index 178bdcbfeb..8aacc29c56 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -8,6 +8,7 @@
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/tile_cover.hpp>
 #include <mbgl/util/mapbox.hpp>
+#include <mbgl/util/run_loop.hpp>
 
 #include <set>
 
@@ -195,7 +196,8 @@ void OfflineDownload::activateDownload() {
 }
 
 void OfflineDownload::deactivateDownload() {
-    requests.clear();
+    workRequests.clear();
+    fileRequests.clear();
 }
 
 void OfflineDownload::ensureTiles(SourceType type, uint16_t tileSize, const SourceInfo& info) {
@@ -207,43 +209,48 @@ void OfflineDownload::ensureTiles(SourceType type, uint16_t tileSize, const Sour
 void OfflineDownload::ensureResource(const Resource& resource, std::function<void (Response)> callback) {
     status.requiredResourceCount++;
 
-    optional<std::pair<Response, uint64_t>> offlineResponse = offlineDatabase.getRegionResource(id, resource);
-    if (offlineResponse) {
-        if (callback) {
-            callback(offlineResponse->first);
-        }
+    auto workRequestsIt = workRequests.insert(workRequests.begin(), nullptr);
+    *workRequestsIt = util::RunLoop::Get()->invokeCancellable([=] () {
+        workRequests.erase(workRequestsIt);
 
-        status.completedResourceCount++;
-        status.completedResourceSize += offlineResponse->second;
-        observer->statusChanged(status);
+        optional<std::pair<Response, uint64_t>> offlineResponse = offlineDatabase.getRegionResource(id, resource);
+        if (offlineResponse) {
+            if (callback) {
+                callback(offlineResponse->first);
+            }
 
-        return;
-    }
+            status.completedResourceCount++;
+            status.completedResourceSize += offlineResponse->second;
+            observer->statusChanged(status);
 
-    if (resource.kind == Resource::Kind::Tile
-        && util::mapbox::isMapboxURL(resource.url)
-        && offlineDatabase.offlineMapboxTileCountLimitExceeded()) {
-        observer->mapboxTileCountLimitExceeded(offlineDatabase.getOfflineMapboxTileCountLimit());
-        return;
-    }
+            return;
+        }
 
-    auto it = requests.insert(requests.begin(), nullptr);
-    *it = onlineFileSource.request(resource, [=] (Response onlineResponse) {
-        if (onlineResponse.error) {
-            observer->responseError(*onlineResponse.error);
+        if (resource.kind == Resource::Kind::Tile
+            && util::mapbox::isMapboxURL(resource.url)
+            && offlineDatabase.offlineMapboxTileCountLimitExceeded()) {
+            observer->mapboxTileCountLimitExceeded(offlineDatabase.getOfflineMapboxTileCountLimit());
             return;
         }
 
-        requests.erase(it);
+        auto fileRequestsIt = fileRequests.insert(fileRequests.begin(), nullptr);
+        *fileRequestsIt = onlineFileSource.request(resource, [=] (Response onlineResponse) {
+            if (onlineResponse.error) {
+                observer->responseError(*onlineResponse.error);
+                return;
+            }
+
+            fileRequests.erase(fileRequestsIt);
 
-        if (callback) {
-            callback(onlineResponse);
-        }
+            if (callback) {
+                callback(onlineResponse);
+            }
 
-        status.completedResourceCount++;
-        status.completedResourceSize += offlineDatabase.putRegionResource(id, resource, onlineResponse);
+            status.completedResourceCount++;
+            status.completedResourceSize += offlineDatabase.putRegionResource(id, resource, onlineResponse);
 
-        observer->statusChanged(status);
+            observer->statusChanged(status);
+        });
     });
 }
 
diff --git a/platform/default/mbgl/storage/offline_download.hpp b/platform/default/mbgl/storage/offline_download.hpp
index 4200020487..d5ba801540 100644
--- a/platform/default/mbgl/storage/offline_download.hpp
+++ b/platform/default/mbgl/storage/offline_download.hpp
@@ -11,6 +11,7 @@ namespace mbgl {
 
 class OfflineDatabase;
 class FileSource;
+class WorkRequest;
 class FileRequest;
 class Resource;
 class Response;
@@ -55,7 +56,8 @@ private:
     FileSource& onlineFileSource;
     OfflineRegionStatus status;
     std::unique_ptr<OfflineRegionObserver> observer;
-    std::list<std::unique_ptr<FileRequest>> requests;
+    std::list<std::unique_ptr<WorkRequest>> workRequests;
+    std::list<std::unique_ptr<FileRequest>> fileRequests;
     std::set<std::string> requiredSourceURLs;
 };