[core] keep tiles renderable even if a subsequent error occurs

Since 9a9408e8111bcdcd0fcb9a93112d61ab8fce0601, we marked tiles as non-renderable if an error occured. This lead to situations where a tile was loaded + parsed successfully, then a revalidation attempt occured (e.g. because the resource was stale) which failed. In this case, we used to mark the tile as non-renderable although we could've used the perfectly parsed (stale) resource.

4 files changed, 51 insertions, 5 deletions

diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 017861884b..fafd9f7ae7 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -79,7 +79,6 @@ void GeometryTile::markObsolete() {
 
 void GeometryTile::setError(std::exception_ptr err) {
     loaded = true;
-    renderable = false;
     observer->onTileError(*this, err);
 }
 
@@ -174,7 +173,6 @@ void GeometryTile::onPlacement(PlacementResult result) {
 void GeometryTile::onError(std::exception_ptr err) {
     loaded = true;
     pending = false;
-    renderable = false;
     observer->onTileError(*this, err);
 }
     
diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp
index b940e342d7..76c74ab964 100644
--- a/src/mbgl/tile/raster_tile.cpp
+++ b/src/mbgl/tile/raster_tile.cpp
@@ -29,7 +29,6 @@ void RasterTile::cancel() {
 
 void RasterTile::setError(std::exception_ptr err) {
     loaded = true;
-    renderable = false;
     observer->onTileError(*this, err);
 }
 
@@ -49,9 +48,7 @@ void RasterTile::onParsed(std::unique_ptr<RasterBucket> result) {
 }
 
 void RasterTile::onError(std::exception_ptr err) {
-    bucket.reset();
     loaded = true;
-    renderable = false;
     observer->onTileError(*this, err);
 }
 
diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp
index 31fb8c1fd0..bb2da1ec62 100644
--- a/test/tile/geojson_tile.test.cpp
+++ b/test/tile/geojson_tile.test.cpp
@@ -77,3 +77,40 @@ TEST(GeoJSONTile, Issue7648) {
         test.loop.runOnce();
     }
 }
+
+// Tests that tiles remain renderable if they have been renderable and then had an error sent to
+// them, e.g. when revalidating/refreshing the request.
+TEST(GeoJSONTile, Issue9927) {
+    GeoJSONTileTest test;
+
+    CircleLayer layer("circle", "source");
+
+    mapbox::geometry::feature_collection<int16_t> features;
+    features.push_back(mapbox::geometry::feature<int16_t> {
+        mapbox::geometry::point<int16_t>(0, 0)
+    });
+
+    GeoJSONTile tile(OverscaledTileID(0, 0, 0), "source", test.tileParameters, features);
+
+    tile.setLayers({{ layer.baseImpl }});
+    tile.setPlacementConfig({});
+
+    while (!tile.isComplete()) {
+        test.loop.runOnce();
+    }
+
+    ASSERT_TRUE(tile.isRenderable());
+    ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
+
+    // Make sure that once we've had a renderable tile and then receive erroneous data, we retain
+    // the previously rendered data and keep the tile renderable.
+    tile.setError(std::make_exception_ptr(std::runtime_error("Connection offline")));
+    ASSERT_TRUE(tile.isRenderable());
+    ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
+
+    // Then simulate a parsing failure and make sure that we keep it renderable in this situation
+    // as well.
+    tile.onError(std::make_exception_ptr(std::runtime_error("Parse error")));
+    ASSERT_TRUE(tile.isRenderable());
+    ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
+ }
diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp
index e96e09bacf..efdc4767d9 100644
--- a/test/tile/raster_tile.test.cpp
+++ b/test/tile/raster_tile.test.cpp
@@ -66,6 +66,20 @@ TEST(RasterTile, onParsed) {
     EXPECT_TRUE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());
+
+    // Make sure that once we've had a renderable tile and then receive erroneous data, we retain
+    // the previously rendered data and keep the tile renderable.
+    tile.setError(std::make_exception_ptr(std::runtime_error("Connection offline")));
+    EXPECT_TRUE(tile.isRenderable());
+    EXPECT_TRUE(tile.isLoaded());
+    EXPECT_TRUE(tile.isComplete());
+
+    // Then simulate a parsing failure and make sure that we keep it renderable in this situation
+    // as well.
+    tile.onError(std::make_exception_ptr(std::runtime_error("Parse error")), 0);
+    ASSERT_TRUE(tile.isRenderable());
+    EXPECT_TRUE(tile.isLoaded());
+    EXPECT_TRUE(tile.isComplete());
 }
 
 TEST(RasterTile, onParsedEmpty) {