[core] make sure tiles are not treated as complete until all worker operations completed

Previously, when we started a worker operation that eventually throws an exception (e.g. due to the tile not being parseable), and then enqueue another worker operation while the first one is processing, we treated the worker as idle once the first operation's error callback fired, even though the second operation was still in progress. Due to our use of coalescing, I was unable to come up with a reliable test since we'd need to reproduce the behavior described above, which is timing dependent.

4 files changed, 17 insertions, 21 deletions

diff --git a/test/tile/annotation_tile.test.cpp b/test/tile/annotation_tile.test.cpp
index 4d71c5b0b4..ac3e064c50 100644
--- a/test/tile/annotation_tile.test.cpp
+++ b/test/tile/annotation_tile.test.cpp
@@ -52,8 +52,7 @@ TEST(AnnotationTile, Issue8289) {
         {},
             std::make_unique<FeatureIndex>(),
             std::move(data),
-            0
-    });
+    }, 0);
 
     auto collisionTile = std::make_unique<CollisionTile>(PlacementConfig());
 
@@ -65,16 +64,14 @@ TEST(AnnotationTile, Issue8289) {
     tile.onPlacement(GeometryTile::PlacementResult {
         {},
             std::move(collisionTile),
-            0
-    });
+    }, 0);
 
     // Simulate a second layout with empty data.
     tile.onLayout(GeometryTile::LayoutResult {
         {},
             std::make_unique<FeatureIndex>(),
             std::make_unique<AnnotationTileData>(),
-            0
-    });
+    }, 0);
 
     std::unordered_map<std::string, std::vector<Feature>> result;
     GeometryCoordinates queryGeometry {{ Point<int16_t>(0, 0) }};
diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp
index 80e215a98d..18d11c2c14 100644
--- a/test/tile/geojson_tile.test.cpp
+++ b/test/tile/geojson_tile.test.cpp
@@ -76,7 +76,7 @@ TEST(GeoJSONTile, Issue7648) {
 TEST(GeoJSONTile, Issue9927) {
     GeoJSONTileTest test;
 
-    CircleLayer layer("circle", "source");
+    test.style.addLayer(std::make_unique<CircleLayer>("circle", "source"));
 
     mapbox::geometry::feature_collection<int16_t> features;
     features.push_back(mapbox::geometry::feature<int16_t> {
@@ -85,7 +85,6 @@ TEST(GeoJSONTile, Issue9927) {
 
     GeoJSONTile tile(OverscaledTileID(0, 0, 0), "source", test.tileParameters, features);
 
-    tile.setLayers({{ layer.baseImpl }});
     tile.setPlacementConfig({});
 
     while (!tile.isComplete()) {
@@ -93,17 +92,18 @@ TEST(GeoJSONTile, Issue9927) {
     }
 
     ASSERT_TRUE(tile.isRenderable());
-    ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
+    ASSERT_NE(nullptr, tile.getBucket(*test.style.getRenderLayer("circle")));
 
     // 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));
+    ASSERT_NE(nullptr, tile.getBucket(*test.style.getRenderLayer("circle")));
 
     // 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")));
+    // as well. We're using 3 as a correlationID since we've done two three calls that increment
+    // this counter (as part of the GeoJSONTile constructor, setLayers, and setPlacementConfig).
+    tile.onError(std::make_exception_ptr(std::runtime_error("Parse error")), 3);
     ASSERT_TRUE(tile.isRenderable());
-    ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
+    ASSERT_NE(nullptr, tile.getBucket(*test.style.getRenderLayer("circle")));
  }
diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp
index 596f97884c..a606066e9b 100644
--- a/test/tile/raster_tile.test.cpp
+++ b/test/tile/raster_tile.test.cpp
@@ -47,7 +47,7 @@ TEST(RasterTile, setError) {
 TEST(RasterTile, onError) {
     RasterTileTest test;
     RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
-    tile.onError(std::make_exception_ptr(std::runtime_error("test")));
+    tile.onError(std::make_exception_ptr(std::runtime_error("test")), 0);
     EXPECT_FALSE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());
@@ -56,7 +56,7 @@ TEST(RasterTile, onError) {
 TEST(RasterTile, onParsed) {
     RasterTileTest test;
     RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
-    tile.onParsed(std::make_unique<RasterBucket>(UnassociatedImage{}));
+    tile.onParsed(std::make_unique<RasterBucket>(UnassociatedImage{}), 0);
     EXPECT_TRUE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());
@@ -79,7 +79,7 @@ TEST(RasterTile, onParsed) {
 TEST(RasterTile, onParsedEmpty) {
     RasterTileTest test;
     RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
-    tile.onParsed(nullptr);
+    tile.onParsed(nullptr, 0);
     EXPECT_FALSE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());
diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp
index 37bfe8512d..336076ecc2 100644
--- a/test/tile/vector_tile.test.cpp
+++ b/test/tile/vector_tile.test.cpp
@@ -53,7 +53,8 @@ TEST(VectorTile, setError) {
 TEST(VectorTile, onError) {
     VectorTileTest test;
     VectorTile tile(OverscaledTileID(0, 0, 0), "source", test.tileParameters, test.tileset);
-    tile.onError(std::make_exception_ptr(std::runtime_error("test")));
+    tile.onError(std::make_exception_ptr(std::runtime_error("test")), 0);
+
     EXPECT_FALSE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());
@@ -78,16 +79,14 @@ TEST(VectorTile, Issue7615) {
             symbolBucket
         }},
         nullptr,
-        0
-    });
+    }, 0);
 
     // Subsequent onLayout should not cause the existing symbol bucket to be discarded.
     tile.onLayout(GeometryTile::LayoutResult {
         {},
         nullptr,
         nullptr,
-        0
-    });
+    }, 0);
 
     EXPECT_EQ(symbolBucket.get(), tile.getBucket(*symbolLayer.baseImpl->createRenderLayer()));
 }