make sure that Sprite isn't marked as loaded until we actually added everything to the SpriteStore

4 files changed, 65 insertions, 47 deletions

diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp
index 589f56f99e..e1ce23b2eb 100644
--- a/src/mbgl/map/sprite.cpp
+++ b/src/mbgl/map/sprite.cpp
@@ -15,62 +15,79 @@
 #include <string>
 #include <sstream>
 
-using namespace mbgl;
+namespace mbgl {
+
+struct Sprite::Loader {
+    bool loadedJSON = false;
+    bool loadedImage = false;
+    std::unique_ptr<Data> data = std::make_unique<Data>();
+
+    Request* jsonRequest = nullptr;
+    Request* spriteRequest = nullptr;
+
+    ~Loader() {
+        if (jsonRequest) {
+            util::ThreadContext::getFileSource()->cancel(jsonRequest);
+        }
+        if (spriteRequest) {
+            util::ThreadContext::getFileSource()->cancel(spriteRequest);
+        }
+    }
+};
 
 Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
     : pixelRatio(pixelRatio_ > 1 ? 2 : 1) {
     if (baseUrl.empty()) {
         // Treat a non-existent sprite as a successfully loaded empty sprite.
-        loadedImage = true;
-        loadedJSON = true;
         return;
     }
 
     std::string spriteURL(baseUrl + (pixelRatio_ > 1 ? "@2x" : "") + ".png");
     std::string jsonURL(baseUrl + (pixelRatio_ > 1 ? "@2x" : "") + ".json");
 
+    loader = std::make_unique<Loader>();
+
     FileSource* fs = util::ThreadContext::getFileSource();
-    jsonRequest = fs->request({ Resource::Kind::JSON, jsonURL }, util::RunLoop::getLoop(), [this, jsonURL](const Response &res) {
-        jsonRequest = nullptr;
+    loader->jsonRequest = fs->request({ Resource::Kind::JSON, jsonURL }, util::RunLoop::getLoop(),
+                                      [this, jsonURL](const Response& res) {
+        loader->jsonRequest = nullptr;
         if (res.status == Response::Successful) {
-            json = res.data;
-            loadedJSON = true;
+            loader->data->json = res.data;
+            loader->loadedJSON = true;
         } else {
             std::stringstream message;
-            message <<  "Failed to load [" << jsonURL << "]: " << res.message;
+            message << "Failed to load [" << jsonURL << "]: " << res.message;
             emitSpriteLoadingFailed(message.str());
             return;
         }
         emitSpriteLoadedIfComplete();
     });
 
-    spriteRequest = fs->request({ Resource::Kind::Image, spriteURL }, util::RunLoop::getLoop(), [this, spriteURL](const Response &res) {
-        spriteRequest = nullptr;
-        if (res.status == Response::Successful) {
-            image = res.data;
-            loadedImage = true;
-        } else {
-            std::stringstream message;
-            message <<  "Failed to load [" << spriteURL << "]: " << res.message;
-            emitSpriteLoadingFailed(message.str());
-            return;
-        }
-        emitSpriteLoadedIfComplete();
-    });
+    loader->spriteRequest =
+        fs->request({ Resource::Kind::Image, spriteURL }, util::RunLoop::getLoop(),
+                    [this, spriteURL](const Response& res) {
+            loader->spriteRequest = nullptr;
+            if (res.status == Response::Successful) {
+                loader->data->image = res.data;
+                loader->loadedImage = true;
+            } else {
+                std::stringstream message;
+                message << "Failed to load [" << spriteURL << "]: " << res.message;
+                emitSpriteLoadingFailed(message.str());
+                return;
+            }
+            emitSpriteLoadedIfComplete();
+        });
 }
 
 Sprite::~Sprite() {
-    if (jsonRequest) {
-        util::ThreadContext::getFileSource()->cancel(jsonRequest);
-    }
-
-    if (spriteRequest) {
-        util::ThreadContext::getFileSource()->cancel(spriteRequest);
-    }
 }
 
 void Sprite::emitSpriteLoadedIfComplete() {
-    if (isLoaded() && observer) {
+    assert(loader);
+    if (loader->loadedImage && loader->loadedJSON && observer) {
+        observer->onSpriteDataLoaded(std::move(loader->data));
+        loader.reset();
         observer->onSpriteLoaded();
     }
 }
@@ -84,10 +101,8 @@ void Sprite::emitSpriteLoadingFailed(const std::string& message) {
     observer->onSpriteLoadingFailed(error);
 }
 
-bool Sprite::isLoaded() const {
-    return loadedImage && loadedJSON;
-}
-
 void Sprite::setObserver(Observer* observer_) {
     observer = observer_;
 }
+
+} // namespace mbgl
diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp
index e4905ddbe9..4f69a53631 100644
--- a/src/mbgl/map/sprite.hpp
+++ b/src/mbgl/map/sprite.hpp
@@ -18,10 +18,16 @@ class Request;
 
 class Sprite : private util::noncopyable {
 public:
+    struct Data {
+        std::string image;
+        std::string json;
+    };
+
     class Observer {
     public:
         virtual ~Observer() = default;
 
+        virtual void onSpriteDataLoaded(std::unique_ptr<Data>) = 0;
         virtual void onSpriteLoaded() = 0;
         virtual void onSpriteLoadingFailed(std::exception_ptr error) = 0;
     };
@@ -29,30 +35,24 @@ public:
     Sprite(const std::string& baseUrl, float pixelRatio);
     ~Sprite();
 
-    bool isLoaded() const;
+    inline bool isLoaded() const {
+        return loader == nullptr;
+    }
 
     const float pixelRatio;
 
     void setObserver(Observer* observer);
 
-    inline const std::string& getImage() const { return image; }
-    inline const std::string& getJSON() const { return json; }
-
 private:
     void emitSpriteLoadedIfComplete();
     void emitSpriteLoadingFailed(const std::string& message);
 
-    bool loadedJSON = false;
-    std::string json;
-
-    bool loadedImage = false;
-    std::string image;
+    struct Loader;
+    std::unique_ptr<Loader> loader;
 
-    Request* jsonRequest = nullptr;
-    Request* spriteRequest = nullptr;
     Observer* observer = nullptr;
 };
 
-}
+} // namespace mbgl
 
 #endif
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index 4c05dac0b8..9b893822b8 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -191,10 +191,12 @@ void Style::onTileLoadingFailed(std::exception_ptr error) {
     emitResourceLoadingFailed(error);
 }
 
-void Style::onSpriteLoaded() {
+void Style::onSpriteDataLoaded(std::unique_ptr<Sprite::Data> spriteData) {
     // Add all sprite images to the SpriteStore object
-    spriteStore->setSprites(parseSprite(sprite->getImage(), sprite->getJSON()));
+    spriteStore->setSprites(parseSprite(spriteData->image, spriteData->json));
+}
 
+void Style::onSpriteLoaded() {
     shouldReparsePartialTiles = true;
 
     emitTileDataChanged();
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index 3362ce7f73..0490e93f32 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -89,6 +89,7 @@ private:
     void onTileLoadingFailed(std::exception_ptr error) override;
 
     // Sprite::Observer implementation.
+    void onSpriteDataLoaded(std::unique_ptr<Sprite::Data>) override;
     void onSpriteLoaded() override;
     void onSpriteLoadingFailed(std::exception_ptr error) override;