[core] Dismiss actors from sprite loaderupstream/mikhail_no_actors_in_sprite_loader

and get rid of `SpriteLoaderWorker`, use `scheduleAndReplyValue()` API instead.

5 files changed, 40 insertions, 89 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1ca0a15aa..fab5319f74 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -532,8 +532,6 @@ add_library(
     ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_loader.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_loader.hpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_loader_observer.hpp
-    ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_loader_worker.cpp
-    ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_loader_worker.hpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_parser.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/sprite/sprite_parser.hpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/storage/asset_file_source.hpp
diff --git a/src/mbgl/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp
index 60bab2bf6e..da05714242 100644
--- a/src/mbgl/sprite/sprite_loader.cpp
+++ b/src/mbgl/sprite/sprite_loader.cpp
@@ -2,7 +2,6 @@
 #include <mbgl/actor/scheduler.hpp>
 #include <mbgl/sprite/sprite_loader.hpp>
 #include <mbgl/sprite/sprite_loader_observer.hpp>
-#include <mbgl/sprite/sprite_loader_worker.hpp>
 #include <mbgl/sprite/sprite_parser.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/storage/resource.hpp>
@@ -20,24 +19,15 @@ namespace mbgl {
 
 static SpriteLoaderObserver nullObserver;
 
-struct SpriteLoader::Loader {
-    Loader(SpriteLoader& imageManager)
-        : mailbox(std::make_shared<Mailbox>(*Scheduler::GetCurrent())),
-          worker(Scheduler::GetBackground(), ActorRef<SpriteLoader>(imageManager, mailbox)) {
-    }
-
+struct SpriteLoader::Data {
     std::shared_ptr<const std::string> image;
     std::shared_ptr<const std::string> json;
     std::unique_ptr<AsyncRequest> jsonRequest;
     std::unique_ptr<AsyncRequest> spriteRequest;
-    std::shared_ptr<Mailbox> mailbox;
-    Actor<SpriteLoaderWorker> worker;
 };
 
 SpriteLoader::SpriteLoader(float pixelRatio_)
-        : pixelRatio(pixelRatio_)
-        , observer(&nullObserver) {
-}
+    : pixelRatio(pixelRatio_), observer(&nullObserver), threadPool(Scheduler::GetBackground()) {}
 
 SpriteLoader::~SpriteLoader() = default;
 
@@ -48,56 +38,70 @@ void SpriteLoader::load(const std::string& url, FileSource& fileSource) {
         return;
     }
 
-    loader = std::make_unique<Loader>(*this);
+    data = std::make_unique<Data>();
 
-    loader->jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) {
+    data->jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) {
         if (res.error) {
             observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
         } else if (res.notModified) {
             return;
         } else if (res.noContent) {
-            loader->json = std::make_shared<std::string>();
+            data->json = std::make_shared<std::string>();
             emitSpriteLoadedIfComplete();
         } else {
             // Only trigger a sprite loaded event we got new data.
-            assert(loader->json != res.data);
-            loader->json = std::move(res.data);
+            assert(data->json != res.data);
+            data->json = std::move(res.data);
             emitSpriteLoadedIfComplete();
         }
     });
 
-    loader->spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) {
+    data->spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) {
         if (res.error) {
             observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
         } else if (res.notModified) {
             return;
         } else if (res.noContent) {
-            loader->image = std::make_shared<std::string>();
+            data->image = std::make_shared<std::string>();
             emitSpriteLoadedIfComplete();
         } else {
-            assert(loader->image != res.data);
-            loader->image = std::move(res.data);
+            assert(data->image != res.data);
+            data->image = std::move(res.data);
             emitSpriteLoadedIfComplete();
         }
     });
 }
 
 void SpriteLoader::emitSpriteLoadedIfComplete() {
-    assert(loader);
-
-    if (!loader->image || !loader->json) {
+    assert(data);
+    if (!data->image || !data->json) {
         return;
     }
 
-    loader->worker.self().invoke(&SpriteLoaderWorker::parse, loader->image, loader->json);
-}
+    struct ParseResult {
+        std::vector<Immutable<style::Image::Impl>> images;
+        std::exception_ptr error;
+    };
 
-void SpriteLoader::onParsed(std::vector<Immutable<style::Image::Impl>> result) {
-    observer->onSpriteLoaded(std::move(result));
-}
+    auto parseClosure = [image = data->image, json = data->json]() -> ParseResult {
+        try {
+            return {parseSprite(*image, *json), nullptr};
+        } catch (...) {
+            return {{}, std::current_exception()};
+        }
+    };
+
+    auto resultClosure = [this, weak = weakFactory.makeWeakPtr()](ParseResult result) {
+        if (!weak) return; // This instance has been deleted.
+
+        if (result.error) {
+            observer->onSpriteError(result.error);
+            return;
+        }
+        observer->onSpriteLoaded(std::move(result.images));
+    };
 
-void SpriteLoader::onError(std::exception_ptr err) {
-    observer->onSpriteError(err);
+    threadPool->scheduleAndReplyValue(parseClosure, resultClosure);
 }
 
 void SpriteLoader::setObserver(SpriteLoaderObserver* observer_) {
diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp
index d82c51e2da..c22c030cf9 100644
--- a/src/mbgl/sprite/sprite_loader.hpp
+++ b/src/mbgl/sprite/sprite_loader.hpp
@@ -13,6 +13,7 @@ namespace mbgl {
 
 class FileSource;
 class SpriteLoaderObserver;
+class Scheduler;
 
 class SpriteLoader {
 public:
@@ -28,15 +29,15 @@ private:
 
     // Invoked by SpriteAtlasWorker
     friend class SpriteLoaderWorker;
-    void onParsed(std::vector<Immutable<style::Image::Impl>>);
-    void onError(std::exception_ptr);
 
     const float pixelRatio;
 
-    struct Loader;
-    std::unique_ptr<Loader> loader;
+    struct Data;
+    std::unique_ptr<Data> data;
 
     SpriteLoaderObserver* observer = nullptr;
+    std::shared_ptr<Scheduler> threadPool;
+    mapbox::base::WeakPtrFactory<SpriteLoader> weakFactory{this};
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader_worker.cpp b/src/mbgl/sprite/sprite_loader_worker.cpp
deleted file mode 100644
index 4bded33d53..0000000000
--- a/src/mbgl/sprite/sprite_loader_worker.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-#include <mbgl/sprite/sprite_loader_worker.hpp>
-#include <mbgl/sprite/sprite_loader.hpp>
-#include <mbgl/sprite/sprite_parser.hpp>
-
-namespace mbgl {
-
-SpriteLoaderWorker::SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader> parent_)
-    : parent(std::move(parent_)) {
-}
-
-void SpriteLoaderWorker::parse(std::shared_ptr<const std::string> image,
-                              std::shared_ptr<const std::string> json) {
-    try {
-        if (!image) {
-            // This shouldn't happen, since we always invoke it with a non-empty pointer.
-            throw std::runtime_error("missing sprite image");
-        }
-        if (!json) {
-            // This shouldn't happen, since we always invoke it with a non-empty pointer.
-            throw std::runtime_error("missing sprite metadata");
-        }
-
-        parent.invoke(&SpriteLoader::onParsed, parseSprite(*image, *json));
-    } catch (...) {
-        parent.invoke(&SpriteLoader::onError, std::current_exception());
-    }
-}
-
-} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader_worker.hpp b/src/mbgl/sprite/sprite_loader_worker.hpp
deleted file mode 100644
index d61e07d14f..0000000000
--- a/src/mbgl/sprite/sprite_loader_worker.hpp
+++ /dev/null
@@ -1,23 +0,0 @@
-#pragma once
-
-#include <mbgl/actor/actor_ref.hpp>
-#include <mbgl/sprite/sprite_parser.hpp>
-
-#include <memory>
-#include <string>
-
-namespace mbgl {
-
-class SpriteLoader;
-
-class SpriteLoaderWorker {
-public:
-    SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader>);
-
-    void parse(std::shared_ptr<const std::string> image, std::shared_ptr<const std::string> json);
-
-private:
-    ActorRef<SpriteLoader> parent;
-};
-
-} // namespace mbgl