[core] Fix offline status reporting with pre-existing tiles (#4147)

3 files changed, 28 insertions, 18 deletions

diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp
index df3e3f4dc3..a42591d60e 100644
--- a/platform/default/mbgl/storage/offline_database.cpp
+++ b/platform/default/mbgl/storage/offline_database.cpp
@@ -103,6 +103,11 @@ OfflineDatabase::Statement OfflineDatabase::getStatement(const char * sql) {
 }
 
 optional<Response> OfflineDatabase::get(const Resource& resource) {
+    auto result = getInternal(resource);
+    return result ? result->first : optional<Response>();
+}
+
+optional<std::pair<Response, uint64_t>> OfflineDatabase::getInternal(const Resource& resource) {
     if (resource.kind == Resource::Kind::Tile) {
         assert(resource.tileData);
         return getTile(*resource.tileData);
@@ -151,7 +156,7 @@ std::pair<bool, uint64_t> OfflineDatabase::putInternal(const Resource& resource,
     return { inserted, size };
 }
 
-optional<Response> OfflineDatabase::getResource(const Resource& resource) {
+optional<std::pair<Response, uint64_t>> OfflineDatabase::getResource(const Resource& resource) {
     Statement accessedStmt = getStatement(
         "UPDATE resources SET accessed = ?1 WHERE url = ?2");
 
@@ -172,6 +177,7 @@ optional<Response> OfflineDatabase::getResource(const Resource& resource) {
     }
 
     Response response;
+    uint64_t size = 0;
 
     response.etag     = stmt->get<optional<std::string>>(0);
     response.expires  = stmt->get<optional<SystemTimePoint>>(1);
@@ -182,11 +188,13 @@ optional<Response> OfflineDatabase::getResource(const Resource& resource) {
         response.noContent = true;
     } else if (stmt->get<int>(4)) {
         response.data = std::make_shared<std::string>(util::decompress(*data));
+        size = data->length();
     } else {
         response.data = std::make_shared<std::string>(*data);
+        size = data->length();
     }
 
-    return response;
+    return std::make_pair(response, size);
 }
 
 bool OfflineDatabase::putResource(const Resource& resource,
@@ -263,7 +271,7 @@ bool OfflineDatabase::putResource(const Resource& resource,
     return true;
 }
 
-optional<Response> OfflineDatabase::getTile(const Resource::TileData& tile) {
+optional<std::pair<Response, uint64_t>> OfflineDatabase::getTile(const Resource::TileData& tile) {
     Statement accessedStmt = getStatement(
         "UPDATE tiles "
         "SET accessed       = ?1 "
@@ -302,6 +310,7 @@ optional<Response> OfflineDatabase::getTile(const Resource::TileData& tile) {
     }
 
     Response response;
+    uint64_t size = 0;
 
     response.etag     = stmt->get<optional<std::string>>(0);
     response.expires  = stmt->get<optional<SystemTimePoint>>(1);
@@ -312,11 +321,13 @@ optional<Response> OfflineDatabase::getTile(const Resource::TileData& tile) {
         response.noContent = true;
     } else if (stmt->get<int>(4)) {
         response.data = std::make_shared<std::string>(util::decompress(*data));
+        size = data->length();
     } else {
         response.data = std::make_shared<std::string>(*data);
+        size = data->length();
     }
 
-    return response;
+    return std::make_pair(response, size);
 }
 
 bool OfflineDatabase::putTile(const Resource::TileData& tile,
@@ -452,8 +463,8 @@ void OfflineDatabase::deleteRegion(OfflineRegion&& region) {
     offlineMapboxTileCount = {};
 }
 
-optional<Response> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) {
-    auto response = get(resource);
+optional<std::pair<Response, uint64_t>> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) {
+    auto response = getInternal(resource);
 
     if (response) {
         markUsed(regionID, resource);
diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp
index fc3f729bff..eb18cc18d2 100644
--- a/platform/default/mbgl/storage/offline_database.hpp
+++ b/platform/default/mbgl/storage/offline_database.hpp
@@ -44,7 +44,8 @@ public:
 
     void deleteRegion(OfflineRegion&&);
 
-    optional<Response> getRegionResource(int64_t regionID, const Resource&);
+    // Return value is (response, stored size)
+    optional<std::pair<Response, uint64_t>> getRegionResource(int64_t regionID, const Resource&);
     uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&);
 
     OfflineRegionDefinition getRegionDefinition(int64_t regionID);
@@ -75,14 +76,15 @@ private:
 
     Statement getStatement(const char *);
 
-    optional<Response> getTile(const Resource::TileData&);
+    optional<std::pair<Response, uint64_t>> getTile(const Resource::TileData&);
     bool putTile(const Resource::TileData&, const Response&,
                  const std::string&, bool compressed);
 
-    optional<Response> getResource(const Resource&);
+    optional<std::pair<Response, uint64_t>> getResource(const Resource&);
     bool putResource(const Resource&, const Response&,
                      const std::string&, bool compressed);
 
+    optional<std::pair<Response, uint64_t>> getInternal(const Resource&);
     std::pair<bool, uint64_t> putInternal(const Resource&, const Response&, bool evict);
 
     // Return value is true iff the resource was previously unused by any other regions.
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index 8aa58d6c34..c748c1e572 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -134,7 +134,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
 }
 
 void OfflineDownload::activateDownload() {
-    status = offlineDatabase.getRegionCompletedStatus(id);
+    status = OfflineRegionStatus();
     requiredSourceURLs.clear();
 
     ensureResource(Resource::style(definition.styleURL), [&] (Response styleResponse) {
@@ -188,10 +188,6 @@ void OfflineDownload::activateDownload() {
             ensureResource(resource);
         }
     });
-
-    // This will be the initial notification, after we've incremented requiredResourceCount
-    // to the reflect the extent to which required resources are already in the database.
-    observer->statusChanged(status);
 }
 
 void OfflineDownload::deactivateDownload() {
@@ -207,14 +203,15 @@ void OfflineDownload::ensureTiles(SourceType type, uint16_t tileSize, const Sour
 void OfflineDownload::ensureResource(const Resource& resource, std::function<void (Response)> callback) {
     status.requiredResourceCount++;
 
-    optional<Response> offlineResponse = offlineDatabase.getRegionResource(id, resource);
+    optional<std::pair<Response, uint64_t>> offlineResponse = offlineDatabase.getRegionResource(id, resource);
     if (offlineResponse) {
         if (callback) {
-            callback(*offlineResponse);
+            callback(offlineResponse->first);
         }
 
-        // Not incrementing status.completedResource{Size,Count} here because previously-existing
-        // resources are already accounted for by offlineDatabase.getRegionCompletedStatus();
+        status.completedResourceCount++;
+        status.completedResourceSize += offlineResponse->second;
+        observer->statusChanged(status);
 
         return;
     }