tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[core] Fix offline status reporting regressions

Browse Source 
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.
This commit is contained in:
John Firebaugh
2016-03-08 09:34:19 -08:00
parent 8fee0f33c0
commit e49c1e3da3
3 changed files with 77 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
platform/default/mbgl/storage/offline_download.cpp
View File

@@ -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;
}
auto it = requests.insert(requests.begin(), nullptr);
*it = onlineFileSource.request(resource, [=] (Response onlineResponse) {
if (onlineResponse.error) {
observer->responseError(*onlineResponse.error);
return;
}
requests.erase(it);
if (callback) {
callback(onlineResponse);
if (resource.kind == Resource::Kind::Tile
&& util::mapbox::isMapboxURL(resource.url)
&& offlineDatabase.offlineMapboxTileCountLimitExceeded()) {
observer->mapboxTileCountLimitExceeded(offlineDatabase.getOfflineMapboxTileCountLimit());
return;
}
status.completedResourceCount++;
status.completedResourceSize += offlineDatabase.putRegionResource(id, resource, onlineResponse);
auto fileRequestsIt = fileRequests.insert(fileRequests.begin(), nullptr);
*fileRequestsIt = onlineFileSource.request(resource, [=] (Response onlineResponse) {
if (onlineResponse.error) {
observer->responseError(*onlineResponse.error);
return;
}
observer->statusChanged(status);
fileRequests.erase(fileRequestsIt);
if (callback) {
callback(onlineResponse);
}
status.completedResourceCount++;
status.completedResourceSize += offlineDatabase.putRegionResource(id, resource, onlineResponse);
observer->statusChanged(status);
});
});
}

4
platform/default/mbgl/storage/offline_download.hpp
View File

@@ -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;
};

50
test/storage/offline_download.cpp
View File

@@ -435,19 +435,8 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) {
test.response("offline/0-0-0.vector.pbf"));
auto observer = std::make_unique<MockObserver>();
bool completedOnce = false;
observer->statusChangedFn = [&] (OfflineRegionStatus status) {
if (!status.complete()) {
return;
} else if (!completedOnce) {
completedOnce = true;
download.setState(OfflineRegionDownloadState::Inactive);
download.setState(OfflineRegionDownloadState::Active);
} else {
EXPECT_EQ(2, status.completedResourceCount);
EXPECT_EQ(test.size, status.completedResourceSize);
EXPECT_TRUE(status.requiredResourceCountIsPrecise);
if (status.complete()) {
test.loop.stop();
}
};
@@ -456,6 +445,43 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) {
download.setState(OfflineRegionDownloadState::Active);
test.loop.run();
OfflineDownload redownload(
region.getID(),
OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/offline/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0),
test.db, test.fileSource);
std::vector<OfflineRegionStatus> statusesAfterReactivate;
observer = std::make_unique<MockObserver>();
observer->statusChangedFn = [&] (OfflineRegionStatus status) {
statusesAfterReactivate.push_back(status);
if (status.complete()) {
test.loop.stop();
}
};
redownload.setObserver(std::move(observer));
redownload.setState(OfflineRegionDownloadState::Active);
test.loop.run();
ASSERT_EQ(3, statusesAfterReactivate.size());
EXPECT_EQ(OfflineRegionDownloadState::Active, statusesAfterReactivate[0].downloadState);
EXPECT_FALSE(statusesAfterReactivate[0].requiredResourceCountIsPrecise);
EXPECT_EQ(1, statusesAfterReactivate[0].requiredResourceCount);
EXPECT_EQ(0, statusesAfterReactivate[0].completedResourceCount);
EXPECT_EQ(OfflineRegionDownloadState::Active, statusesAfterReactivate[1].downloadState);
EXPECT_TRUE(statusesAfterReactivate[1].requiredResourceCountIsPrecise);
EXPECT_EQ(2, statusesAfterReactivate[1].requiredResourceCount);
EXPECT_EQ(1, statusesAfterReactivate[1].completedResourceCount);
EXPECT_EQ(OfflineRegionDownloadState::Active, statusesAfterReactivate[2].downloadState);
EXPECT_TRUE(statusesAfterReactivate[2].requiredResourceCountIsPrecise);
EXPECT_EQ(2, statusesAfterReactivate[2].requiredResourceCount);
EXPECT_EQ(2, statusesAfterReactivate[2].completedResourceCount);
}
TEST(OfflineDownload, Deactivate) {