[core] Schedule invocation of onStyleImageMissing completion callback on the same thread

Before this change, ImageManger's 'done' callback for onStyleImageMissing observer
notification that was created on renderer thread, could be called from different thread,
therefore, is not thread safe. For example, on Android platform, callback was invoked
from UI thread. This change makes callback to be scheduled on originating thread.

3 files changed, 39 insertions, 9 deletions

diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp
index 25acd6bd25..2ea47225f8 100644
--- a/src/mbgl/renderer/image_manager.cpp
+++ b/src/mbgl/renderer/image_manager.cpp
@@ -1,4 +1,6 @@
 #include <mbgl/renderer/image_manager.hpp>
+#include <mbgl/actor/actor.hpp>
+#include <mbgl/actor/scheduler.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/gfx/context.hpp>
 #include <mbgl/renderer/image_manager_observer.hpp>
@@ -116,7 +118,7 @@ void ImageManager::removeRequestor(ImageRequestor& requestor) {
 
 void ImageManager::notifyIfMissingImageAdded() {
     for (auto it = missingImageRequestors.begin(); it != missingImageRequestors.end();) {
-        if (it->second.callbacksRemaining == 0) {
+        if (it->second.callbacks.empty()) {
             notify(*it->first, it->second.pair);
             missingImageRequestors.erase(it++);
         } else {
@@ -137,19 +139,29 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR
     if (missing > 0) {
         ImageRequestor* requestorPtr = &requestor;
 
-        missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { std::move(pair), missing });
+        auto emplaced = missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { pair, {} });
+        assert(emplaced.second);
 
         for (const auto& dependency : pair.first) {
             auto it = images.find(dependency.first);
             if (it == images.end()) {
                 assert(observer != nullptr);
-                observer->onStyleImageMissing(dependency.first, [this, requestorPtr]() {
-                    auto requestorIt = missingImageRequestors.find(requestorPtr);
-                    if (requestorIt != missingImageRequestors.end()) {
-                        assert(requestorIt->second.callbacksRemaining > 0);
-                        requestorIt->second.callbacksRemaining--;
-                    }
+                auto callback = std::make_unique<ActorCallback>(
+                        *Scheduler::GetCurrent(),
+                        [this, requestorPtr, imageId = dependency.first] {
+                            auto requestorIt = missingImageRequestors.find(requestorPtr);
+                            if (requestorIt != missingImageRequestors.end()) {
+                                assert(requestorIt->second.callbacks.find(imageId) != requestorIt->second.callbacks.end());
+                                requestorIt->second.callbacks.erase(imageId);
+                            }
+                        });
+
+                auto actorRef = callback->self();
+                emplaced.first->second.callbacks.emplace(dependency.first, std::move(callback));
+                observer->onStyleImageMissing(dependency.first, [actorRef]() mutable {
+                    actorRef.invoke(&Callback::operator());
                 });
+
             }
         }
 
diff --git a/src/mbgl/renderer/image_manager.hpp b/src/mbgl/renderer/image_manager.hpp
index 99887ae384..bec811317e 100644
--- a/src/mbgl/renderer/image_manager.hpp
+++ b/src/mbgl/renderer/image_manager.hpp
@@ -15,6 +15,9 @@
 
 namespace mbgl {
 
+template <class T>
+class Actor;
+
 namespace gfx {
 class Context;
 } // namespace gfx
@@ -66,9 +69,11 @@ private:
     bool loaded = false;
 
     std::map<ImageRequestor*, ImageRequestPair> requestors;
+    using Callback = std::function<void()>;
+    using ActorCallback = Actor<Callback>;
     struct MissingImageRequestPair {
         ImageRequestPair pair;
-        unsigned int callbacksRemaining;
+        std::map<std::string, std::unique_ptr<ActorCallback>> callbacks;
     };
     std::map<ImageRequestor*, MissingImageRequestPair> missingImageRequestors;
     ImageMap images;
diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp
index d124e67e10..b9ec099f93 100644
--- a/test/renderer/image_manager.test.cpp
+++ b/test/renderer/image_manager.test.cpp
@@ -117,6 +117,7 @@ public:
 };
 
 TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) {
+    util::RunLoop runLoop;
     ImageManager imageManager;
     StubImageRequestor requestor;
     bool notified = false;
@@ -132,11 +133,15 @@ TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) {
     ImageDependencies dependencies;
     dependencies.emplace("one", ImageType::Icon);
     imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID));
+    runLoop.runOnce();
+
     ASSERT_FALSE(notified);
 
     imageManager.setLoaded(true);
+    runLoop.runOnce();
     ASSERT_FALSE(notified);
     imageManager.notifyIfMissingImageAdded();
+    runLoop.runOnce();
     ASSERT_TRUE(notified);
 }
 
@@ -169,6 +174,7 @@ class StubImageManagerObserver : public ImageManagerObserver {
 };
 
 TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
+    util::RunLoop runLoop;
     ImageManager imageManager;
     StubImageRequestor requestor;
     StubImageManagerObserver observer;
@@ -185,16 +191,19 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
     ImageDependencies dependencies;
     dependencies.emplace("pre", ImageType::Icon);
     imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID));
+    runLoop.runOnce();
 
     EXPECT_EQ(observer.count, 0);
     ASSERT_FALSE(notified);
 
     imageManager.setLoaded(true);
+    runLoop.runOnce();
 
     EXPECT_EQ(observer.count, 1);
     ASSERT_FALSE(notified);
 
     imageManager.notifyIfMissingImageAdded();
+    runLoop.runOnce();
 
     EXPECT_EQ(observer.count, 1);
     ASSERT_TRUE(notified);
@@ -202,6 +211,7 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
 }
 
 TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {
+    util::RunLoop runLoop;
     ImageManager imageManager;
     StubImageRequestor requestor;
     StubImageManagerObserver observer;
@@ -218,16 +228,19 @@ TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {
     ASSERT_FALSE(notified);
 
     imageManager.setLoaded(true);
+    runLoop.runOnce();
 
     uint64_t imageCorrelationID = 0;
     ImageDependencies dependencies;
     dependencies.emplace("after", ImageType::Icon);
     imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID));
+    runLoop.runOnce();
 
     EXPECT_EQ(observer.count, 1);
     ASSERT_FALSE(notified);
 
     imageManager.notifyIfMissingImageAdded();
+    runLoop.runOnce();
 
     EXPECT_EQ(observer.count, 1);
     ASSERT_TRUE(notified);