Notify data changes using Observer pattern

This pattern, in contrast with callbacks, allows us to clearly see
the dependency and hierarchy between the objects. It will also help
on writing unit tests as we can count how many times a notification
was triggered.

5 files changed, 64 insertions, 24 deletions

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index c14cedb033..146d7ec4e7 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -168,10 +168,7 @@ void MapContext::updateTiles() {
     if (!style) return;
     for (const auto& source : style->sources) {
         source->update(data, transformState, *style, *glyphAtlas, *glyphStore, *spriteAtlas,
-                       getSprite(), *texturePool, [this]() {
-            assert(Environment::currentlyOn(ThreadType::Map));
-            triggerUpdate();
-        });
+                       getSprite(), *texturePool);
     }
 }
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 5186c665fb..91f183d69b 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -5,7 +5,6 @@
 #include <mbgl/style/style.hpp>
 
 #include <cassert>
-#include <functional>
 
 namespace mbgl {
 
@@ -15,6 +14,10 @@ ResourceLoader::ResourceLoader() {
 
 ResourceLoader::~ResourceLoader() {
     assert(Environment::currentlyOn(ThreadType::Map));
+
+    for (const auto& source : style_->sources) {
+        source->setObserver(nullptr);
+    }
 }
 
 void ResourceLoader::setObserver(Observer* observer) {
@@ -29,7 +32,8 @@ void ResourceLoader::setStyle(Style* style) {
 
     Environment& env = Environment::Get();
     for (const auto& source : style->sources) {
-        source->load(accessToken_, env, std::bind(&ResourceLoader::emitTileDataChanged, this));
+        source->setObserver(this);
+        source->load(accessToken_, env);
     }
 }
 
@@ -37,6 +41,14 @@ void ResourceLoader::setAccessToken(const std::string& accessToken) {
     accessToken_ = accessToken;
 }
 
+void ResourceLoader::onSourceLoaded() {
+    emitTileDataChanged();
+}
+
+void ResourceLoader::onTileLoaded() {
+    emitTileDataChanged();
+}
+
 void ResourceLoader::emitTileDataChanged() {
     assert(Environment::currentlyOn(ThreadType::Map));
 
diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp
index 6d2089e3ab..f03ba791dd 100644
--- a/src/mbgl/map/resource_loader.hpp
+++ b/src/mbgl/map/resource_loader.hpp
@@ -1,6 +1,7 @@
 #ifndef MBGL_MAP_RESOURCE_LOADER
 #define MBGL_MAP_RESOURCE_LOADER
 
+#include <mbgl/map/source.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <string>
@@ -13,7 +14,7 @@ class Style;
 // by the Style. The Source object currently owns all the tiles, thus this
 // class will notify its observers of any change on these tiles which will
 // ultimately cause a new rendering to be triggered.
-class ResourceLoader : private util::noncopyable {
+class ResourceLoader : public Source::Observer, private util::noncopyable {
 public:
     class Observer {
     public:
@@ -34,6 +35,10 @@ public:
     // Set the access token to be used for loading the tile data.
     void setAccessToken(const std::string& accessToken);
 
+    // Source::Observer implementation.
+    void onSourceLoaded() override;
+    void onTileLoaded() override;
+
 private:
     void emitTileDataChanged();
 
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 708ff6b27a..c63cd026de 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -136,9 +136,7 @@ bool Source::isLoaded() const {
 // Note: This is a separate function that must be called exactly once after creation
 // The reason this isn't part of the constructor is that calling shared_from_this() in
 // the constructor fails.
-void Source::load(const std::string& accessToken,
-                  Environment& env,
-                  std::function<void()> callback) {
+void Source::load(const std::string& accessToken, Environment& env) {
     if (info.url.empty()) {
         loaded = true;
         return;
@@ -147,7 +145,7 @@ void Source::load(const std::string& accessToken,
     util::ptr<Source> source = shared_from_this();
 
     const std::string url = util::mapbox::normalizeSourceURL(info.url, accessToken);
-    env.request({ Resource::Kind::JSON, url }, [source, callback](const Response &res) {
+    env.request({ Resource::Kind::JSON, url }, [source](const Response &res) {
         if (res.status != Response::Successful) {
             Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str());
             return;
@@ -164,7 +162,7 @@ void Source::load(const std::string& accessToken,
         source->info.parseTileJSONProperties(d);
         source->loaded = true;
 
-        callback();
+        source->emitSourceLoaded();
     });
 }
 
@@ -226,8 +224,7 @@ TileData::State Source::addTile(MapData& data,
                                 SpriteAtlas& spriteAtlas,
                                 util::ptr<Sprite> sprite,
                                 TexturePool& texturePool,
-                                const TileID& id,
-                                std::function<void()> callback) {
+                                const TileID& id) {
     const TileData::State state = hasTile(id);
 
     if (state != TileData::State::invalid) {
@@ -256,6 +253,8 @@ TileData::State Source::addTile(MapData& data,
         new_tile.data = cache.get(normalized_id.to_uint64());
     }
 
+    auto callback = std::bind(&Source::emitTileLoaded, this);
+
     if (!new_tile.data) {
         // If we don't find working tile data, we're just going to load it.
         if (info.type == SourceType::Vector) {
@@ -366,8 +365,7 @@ void Source::update(MapData& data,
                     GlyphStore& glyphStore,
                     SpriteAtlas& spriteAtlas,
                     util::ptr<Sprite> sprite,
-                    TexturePool& texturePool,
-                    std::function<void()> callback) {
+                    TexturePool& texturePool) {
     if (!loaded || data.getAnimationTime() <= updated) {
         return;
     }
@@ -387,7 +385,7 @@ void Source::update(MapData& data,
     // Add existing child/parent tiles if the actual tile is not yet loaded
     for (const auto& id : required) {
         const TileData::State state = addTile(data, transformState, style, glyphAtlas, glyphStore,
-                                              spriteAtlas, sprite, texturePool, id, callback);
+                                              spriteAtlas, sprite, texturePool, id);
 
         if (state != TileData::State::parsed) {
             // The tile we require is not yet loaded. Try to find a parent or
@@ -477,4 +475,20 @@ void Source::onLowMemory() {
     cache.clear();
 }
 
+void Source::setObserver(Observer* observer) {
+    observer_ = observer;
+}
+
+void Source::emitSourceLoaded() {
+    if (observer_) {
+        observer_->onSourceLoaded();
+    }
+}
+
+void Source::emitTileLoaded() {
+    if (observer_) {
+        observer_->onTileLoaded();
+    }
+}
+
 }
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index b15de0e642..255e972086 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -52,12 +52,18 @@ public:
 
 class Source : public std::enable_shared_from_this<Source>, private util::noncopyable {
 public:
+    class Observer {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void onSourceLoaded() = 0;
+        virtual void onTileLoaded() = 0;
+    };
+
     Source();
     ~Source();
 
-    void load(const std::string& accessToken,
-              Environment&,
-              std::function<void()> callback);
+    void load(const std::string& accessToken, Environment&);
     bool isLoaded() const;
 
     void load(MapData&, Environment&, std::function<void()> callback);
@@ -68,8 +74,7 @@ public:
                 GlyphStore&,
                 SpriteAtlas&,
                 util::ptr<Sprite>,
-                TexturePool&,
-                std::function<void()> callback);
+                TexturePool&);
 
     void invalidateTiles(const std::vector<TileID>&);
 
@@ -83,10 +88,15 @@ public:
     void setCacheSize(size_t);
     void onLowMemory();
 
+    void setObserver(Observer* observer);
+
     SourceInfo info;
     bool enabled;
 
 private:
+    void emitSourceLoaded();
+    void emitTileLoaded();
+
     bool findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     bool findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
     int32_t coveringZoomLevel(const TransformState&) const;
@@ -100,8 +110,7 @@ private:
                             SpriteAtlas&,
                             util::ptr<Sprite>,
                             TexturePool&,
-                            const TileID&,
-                            std::function<void()> callback);
+                            const TileID&);
 
     TileData::State hasTile(const TileID& id);
     void updateTilePtrs();
@@ -117,6 +126,9 @@ private:
     std::vector<Tile*> tilePtrs;
     std::map<TileID, std::weak_ptr<TileData>> tile_data;
     TileCache cache;
+
+
+    Observer* observer_ = nullptr;
 };
 
 }