From 28ccb0ccc08416b9e8ced25ae938a50ca4a00151 Mon Sep 17 00:00:00 2001
From: Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>
Date: Wed, 18 Dec 2019 16:30:32 +0200
Subject: [PATCH] [core] Offline download must clear batch buffer at load
 interrupt

Otherwise, the stale buffer data are processed again at the
repeated load. It is causing various problems like missing load
completion notification or crashes (if the previously used observer
instance has been deleted).
---
 .../src/mbgl/storage/offline_download.cpp     |  1 +
 test/storage/offline_download.test.cpp        | 83 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/platform/default/src/mbgl/storage/offline_download.cpp b/platform/default/src/mbgl/storage/offline_download.cpp
index d5cf7e4fe..98eb1d388 100644
--- a/platform/default/src/mbgl/storage/offline_download.cpp
+++ b/platform/default/src/mbgl/storage/offline_download.cpp
@@ -379,6 +379,7 @@ void OfflineDownload::deactivateDownload() {
     requiredSourceURLs.clear();
     resourcesRemaining.clear();
     requests.clear();
+    buffer.clear();
 }
 
 void OfflineDownload::queueResource(Resource&& resource) {
diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp
index ebe3f82ee..c1a9bb73f 100644
--- a/test/storage/offline_download.test.cpp
+++ b/test/storage/offline_download.test.cpp
@@ -757,7 +757,6 @@ TEST(OfflineDownload, Deactivate) {
     test.loop.run();
 }
 
-
 TEST(OfflineDownload, AllOfflineRequestsHaveLowPriorityAndOfflineUsage) {
     OfflineTest test;
     auto region = test.createRegion();
@@ -947,3 +946,85 @@ TEST(OfflineDownload, ResourceOfflineUsageUnset) {
     test.loop.run();
 }
 #endif // __QT__
+
+// Test verifies that resource batch buffer is cleared at loading interrupt.
+TEST(OfflineDownload, InterruptAndResume) {
+    OfflineTest test;
+    auto region = test.createRegion();
+    ASSERT_TRUE(region);
+    OfflineDownload download(region->getID(),
+                             OfflineTilePyramidRegionDefinition(
+                                 "http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true),
+                             test.db,
+                             test.fileSource);
+
+    test.fileSource.styleResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/style.json", resource.url);
+        return test.response("style.json");
+    };
+
+    test.fileSource.spriteImageResponse = [&](const Resource& resource) {
+        EXPECT_TRUE(resource.url == "http://127.0.0.1:3000/sprite.png" ||
+                    resource.url == "http://127.0.0.1:3000/sprite@2x.png");
+        return test.response("sprite.png");
+    };
+
+    test.fileSource.imageResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/radar.gif", resource.url);
+        return test.response("radar.gif");
+    };
+
+    test.fileSource.spriteJSONResponse = [&](const Resource& resource) {
+        EXPECT_TRUE(resource.url == "http://127.0.0.1:3000/sprite.json" ||
+                    resource.url == "http://127.0.0.1:3000/sprite@2x.json");
+        return test.response("sprite.json");
+    };
+
+    test.fileSource.glyphsResponse = [&](const Resource&) { return test.response("glyph.pbf"); };
+
+    test.fileSource.sourceResponse = [&](const Resource& resource) {
+        EXPECT_EQ("http://127.0.0.1:3000/streets.json", resource.url);
+        return test.response("streets.json");
+    };
+
+    test.fileSource.tileResponse = [&](const Resource& resource) {
+        const Resource::TileData& tile = *resource.tileData;
+        EXPECT_EQ("http://127.0.0.1:3000/{z}-{x}-{y}.vector.pbf", tile.urlTemplate);
+        EXPECT_EQ(1, tile.pixelRatio);
+        EXPECT_EQ(0, tile.x);
+        EXPECT_EQ(0, tile.y);
+        EXPECT_EQ(0, tile.z);
+        return test.response("0-0-0.vector.pbf");
+    };
+
+    auto observer = std::make_unique<MockObserver>();
+    bool interrupted = false;
+    observer->statusChangedFn = [&](OfflineRegionStatus status) {
+        if (!interrupted && status.completedResourceCount > 0) {
+            interrupted = true;
+            assert(!status.complete());
+            test.loop.schedule([&]() {
+                download.setState(OfflineRegionDownloadState::Inactive);
+                test.loop.stop();
+            });
+        }
+    };
+
+    download.setObserver(std::move(observer));
+    download.setState(OfflineRegionDownloadState::Active);
+    test.loop.run();
+
+    auto newObserver = std::make_unique<MockObserver>();
+    newObserver->statusChangedFn = [&](OfflineRegionStatus status) {
+        if (status.complete()) {
+            EXPECT_EQ(status.completedTileCount, status.requiredTileCount);
+            EXPECT_EQ(264u, status.completedResourceCount); // 256 glyphs, 2 sprite images, 2 sprite jsons, 1 tile, 1
+                                                            // style, source, image
+            test.loop.stop();
+        }
+    };
+
+    download.setObserver(std::move(newObserver));
+    download.setState(OfflineRegionDownloadState::Active);
+    test.loop.run();
+}