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.

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);
     }
 }
 

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));
 

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();
 

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();
+    }
+}
+
 }

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;
 };
 
 }