Test Map object destruction under different error scenarios

This is a work in progress.

test/resources/mock_file_source.cpp

@@ -5,78 +5,151 @@
 #include <mbgl/util/io.hpp>
 #include <mbgl/util/thread.hpp>
 
+#include <algorithm>
+
+namespace {
+
+const uint64_t timeout = 1000000;
+
+}
+
 namespace mbgl {
 
 class MockFileSource::Impl {
 public:
-    Impl(uv_loop_t*, Type type, const std::string& match) : type_(type), match_(match) {}
+    Impl(uv_loop_t* loop, Type type, const std::string& match)
+        : type_(type), match_(match), timer_(loop) {
+        timer_.start(timeout, timeout, [this] { dispatchPendingRequests(); });
+        timer_.unref();
+    }
 
-    void handleRequest(Request* req) const;
+    ~Impl() {
+        timer_.stop();
+    }
+
+    void setOnRequestDelayedCallback(std::function<void(void)> callback) {
+        requestEnqueuedCallback_ = callback;
+    }
+
+    void handleRequest(Request* req);
+    void cancelRequest(Request* req);
 
 private:
-    void replyWithFailure(Response* res) const;
-    void replyWithCorruptedData(Response* res, const std::string& url) const;
-    void replyWithSuccess(Response* res, const std::string& url) const;
+    void replyWithSuccess(Request* req) const;
+    void replyWithSuccessWithDelay(Request* req);
+    void replyWithFailure(Request* req) const;
+    void replyWithCorruptedData(Request* req) const;
+
+    void dispatchPendingRequests();
 
     Type type_;
     std::string match_;
-};
+    std::vector<Request*> pendingRequests_;
+    uv::timer timer_;
 
-void MockFileSource::Impl::replyWithFailure(Response* res) const {
-    res->status = Response::Status::Error;
-    res->message = "Failed by the test case";
-}
+    std::function<void(void)> requestEnqueuedCallback_;
+};
 
-void MockFileSource::Impl::replyWithCorruptedData(Response* res, const std::string& url) const {
+void MockFileSource::Impl::replyWithSuccess(Request* req) const {
+    std::shared_ptr<Response> res = std::make_shared<Response>();
     res->status = Response::Status::Successful;
-    res->data = util::read_file(url);
-    res->data.insert(0, "CORRUPTED");
+    res->data = util::read_file(req->resource.url);
+
+    req->notify(res);
 }
 
-void MockFileSource::Impl::replyWithSuccess(Response* res, const std::string& url) const {
-    res->status = Response::Status::Successful;
-    res->data = util::read_file(url);
+void MockFileSource::Impl::replyWithSuccessWithDelay(Request* req) {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
+        return;
+    }
+
+    pendingRequests_.push_back(req);
+    requestEnqueuedCallback_();
 }
 
-void MockFileSource::Impl::handleRequest(Request* req) const {
-    const std::string& url = req->resource.url;
-    std::shared_ptr<Response> response = std::make_shared<Response>();
+void MockFileSource::Impl::replyWithFailure(Request* req) const {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
+        return;
+    }
+
+    std::shared_ptr<Response> res = std::make_shared<Response>();
+    res->status = Response::Status::Error;
+    res->message = "Failed by the test case";
 
-    if (url.find(match_) == std::string::npos) {
-        replyWithSuccess(response.get(), url);
-        req->notify(response);
+    req->notify(res);
+}
+
+void MockFileSource::Impl::replyWithCorruptedData(Request* req) const {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
         return;
     }
 
+    std::shared_ptr<Response> res = std::make_shared<Response>();
+    res->status = Response::Status::Successful;
+    res->data = util::read_file(req->resource.url);
+    res->data.insert(0, "CORRUPTED");
+
+    req->notify(res);
+}
+
+void MockFileSource::Impl::handleRequest(Request* req) {
     switch (type_) {
     case Type::Success:
-        replyWithSuccess(response.get(), url);
+        replyWithSuccess(req);
+        break;
+    case Type::SuccessWithDelay:
+        replyWithSuccessWithDelay(req);
         break;
     case Type::RequestFail:
-        replyWithFailure(response.get());
+        replyWithFailure(req);
         break;
     case Type::RequestWithCorruptedData:
-        replyWithCorruptedData(response.get(), url);
+        replyWithCorruptedData(req);
         break;
     default:
         EXPECT_TRUE(false) << "Should never be reached.";
     }
+}
 
-    req->notify(response);
+void MockFileSource::Impl::cancelRequest(Request* req) {
+    auto it = std::find(pendingRequests_.begin(), pendingRequests_.end(), req);
+    if (it != pendingRequests_.end()) {
+        (*it)->destruct();
+        pendingRequests_.erase(it);
+    } else {
+        EXPECT_TRUE(false) << "Should never be reached.";
+    }
+}
+
+void MockFileSource::Impl::dispatchPendingRequests() {
+    for (auto req : pendingRequests_) {
+        replyWithSuccess(req);
+    }
+
+    pendingRequests_.clear();
 }
 
 MockFileSource::MockFileSource(Type type, const std::string& match)
     : thread_(std::make_unique<util::Thread<Impl>>("FileSource", util::ThreadPriority::Low, type, match)) {
 }
 
+void MockFileSource::setOnRequestDelayedCallback(std::function<void(void)> callback) {
+    thread_->invokeSync(&Impl::setOnRequestDelayedCallback, callback);
+}
+
 Request* MockFileSource::request(const Resource& resource, uv_loop_t* loop, Callback callback) {
     Request* req = new Request(resource, loop, std::move(callback));
-    thread_->invoke(&Impl::handleRequest, req);
+    thread_->invokeSync(&Impl::handleRequest, req);
 
     return req;
 }
 
-void MockFileSource::cancel(Request*) {
+void MockFileSource::cancel(Request* req) {
+    req->cancel();
+    thread_->invoke(&Impl::cancelRequest, req);
 }
 
 }

test/resources/mock_file_source.hpp

@@ -2,22 +2,37 @@
 #define TEST_RESOURCES_MOCK_FILE_SOURCE
 
 #include <mbgl/storage/file_source.hpp>
+#include <mbgl/util/thread.hpp>
 
 #include <string>
 #include <memory>
 
 namespace mbgl {
 
-namespace util {
-template <typename T> class Thread;
-}
-
-// This mock FileSource will read data from the disk and will fail
-// the request if the URL matches a string.
+// The MockFileSource is a FileSource that can simulate different
+// types of failures and it will work completely offline.
 class MockFileSource : public FileSource {
 public:
+    // Success:
+    //     Will reply to every request correctly with valid data.
+    //
+    // SuccessWithDelay:
+    //     Will reply to every request correctly with valid data,
+    //     but the ones that contains the "match" string on the
+    //     URL will be answered after a delay. This can be useful
+    //     for testing request cancellation.
+    //
+    // RequestFail:
+    //     Will reply with an error to requests that contains
+    //     the "match" string on the URL.
+    //
+    // RequestWithCorruptedData:
+    //     Will answer every request successfully but will return
+    //     corrupt data on the requests that contains the "match"
+    //     string on the URL.
     enum Type {
         Success,
+        SuccessWithDelay,
         RequestFail,
         RequestWithCorruptedData
     };
@@ -27,6 +42,10 @@ class MockFileSource : public FileSource {
     MockFileSource(Type type, const std::string& match);
     ~MockFileSource() override = default;
 
+    // Function that gets called when a delayed resource is enqueued. The
+    // callback must be safe to call from any thread.
+    void setOnRequestDelayedCallback(std::function<void(void)> callback);
+
     // FileSource implementation.
     Request* request(const Resource&, uv_loop_t*, Callback) override;
     void cancel(Request*) override;

test/resources/pending_resources.cpp

@@ -0,0 +1,61 @@
+#include "../fixtures/fixture_log_observer.hpp"
+#include "../fixtures/util.hpp"
+#include "mock_file_source.hpp"
+
+#include <mbgl/map/map.hpp>
+#include <mbgl/map/still_image.hpp>
+#include <mbgl/platform/default/headless_display.hpp>
+#include <mbgl/platform/default/headless_view.hpp>
+#include <mbgl/util/io.hpp>
+#include <mbgl/util/run_loop.hpp>
+#include <mbgl/util/uv_detail.hpp>
+
+using namespace mbgl;
+
+class PendingResourcesTest : public ::testing::TestWithParam<std::string> {
+};
+
+// This test will load a Style but one of the resources requested will not be
+// replied immediately like the others. We get an notification by the
+// MockFileSource when some resource is artificially delayed and we destroy
+// the Map object after that. The idea here is to test if these pending requests
+// are getting canceled correctly if on shutdown.
+TEST_P(PendingResourcesTest, DeleteMapObjectWithPendingRequest) {
+    // TODO: The glyphs test is blocked by the issue #1664.
+    if (GetParam() == "glyphs.pbf") {
+        return;
+    }
+
+    util::RunLoop loop(uv_default_loop());
+
+    auto display = std::make_shared<mbgl::HeadlessDisplay>();
+    HeadlessView view(display);
+    MockFileSource fileSource(MockFileSource::SuccessWithDelay, GetParam());
+
+    std::unique_ptr<Map> map = std::make_unique<Map>(view, fileSource, MapMode::Still);
+
+    uv::async endTest(loop.get(), [&map, &loop] {
+        map.reset();
+        loop.stop();
+    });
+
+    endTest.unref();
+    fileSource.setOnRequestDelayedCallback([&endTest] { endTest.send(); });
+
+    const std::string style = util::read_file("test/fixtures/resources/style.json");
+    map->resize(1000, 1000, 1.0);
+    map->setStyleJSON(style, ".");
+
+    map->renderStill([&endTest](std::exception_ptr, std::unique_ptr<const StillImage>) {
+        EXPECT_TRUE(false) << "Should never happen.";
+    });
+
+    uv_run(loop.get(), UV_RUN_DEFAULT);
+}
+
+// In the test data below, "sprite" will match both "sprite.json" and "sprite.png" and cause two
+// requests to be canceled. "resources" will match everything but in practice will only test the
+// cancellation of the sprites and "source.json" because we only load the rest after "source.json"
+// gets parsed.
+INSTANTIATE_TEST_CASE_P(ResourceLoader, PendingResourcesTest,
+    ::testing::Values("source.json", "sprite.json", "sprite.png", "sprite", "vector.pbf", "glyphs.pbf", "resources"));

test/test.gypi

@@ -62,6 +62,7 @@
         'resources/mock_file_source.cpp',
         'resources/mock_file_source.hpp',
         'resources/mock_view.hpp',
+        'resources/pending_resources.cpp',
         'resources/resource_loader.cpp',
 
         'storage/storage.hpp',