notifications on request cancellation

include/mbgl/storage/base_request.hpp

@@ -1,6 +1,7 @@
 #ifndef MBGL_STORAGE_BASE_REQUEST
 #define MBGL_STORAGE_BASE_REQUEST
 
+#include <mbgl/storage/request_callback.hpp>
 #include <mbgl/util/ptr.hpp>
 
 #include <string>
@@ -15,8 +16,6 @@ namespace mbgl {
 
 class Response;
 class Request;
-using Callback = std::function<void(const Response &)>;
-
 
 class BaseRequest {
 private:
@@ -43,7 +42,6 @@ private:
     // This object may hold a shared_ptr to itself. It does this to prevent destruction of this object
     // while a request is in progress.
     util::ptr<BaseRequest> self;
-
     std::forward_list<std::unique_ptr<Callback>> callbacks;
 };
 

include/mbgl/storage/request.hpp

@@ -1,10 +1,10 @@
 #ifndef MBGL_STORAGE_REQUEST
 #define MBGL_STORAGE_REQUEST
 
+#include <mbgl/storage/request_callback.hpp>
 #include <mbgl/storage/response.hpp>
 #include <mbgl/util/ptr.hpp>
 
-#include <functional>
 #include <forward_list>
 
 typedef struct uv_loop_s uv_loop_t;
@@ -12,7 +12,6 @@ typedef struct uv_loop_s uv_loop_t;
 namespace mbgl {
 
 class BaseRequest;
-using Callback = std::function<void(const Response &)>;
 
 class Request {
 private:
@@ -25,7 +24,8 @@ public:
     Request(const util::ptr<BaseRequest> &base);
     ~Request();
 
-    void onload(Callback cb);
+    void onload(CompletedCallback cb);
+    void oncancel(AbortedCallback cb);
     void cancel();
 
 private:

include/mbgl/storage/request_callback.hpp

@@ -0,0 +1,22 @@
+#ifndef MBGL_STORAGE_REQUEST_CALLBACK
+#define MBGL_STORAGE_REQUEST_CALLBACK
+
+#include <mbgl/util/variant.hpp>
+
+#include <functional>
+
+namespace mbgl {
+
+class Response;
+
+using CompletedCallback = std::function<void(const Response &)>;
+using AbortedCallback = std::function<void()>;
+
+using Callback = mapbox::util::variant<
+    CompletedCallback,
+    AbortedCallback
+>;
+
+}
+
+#endif

src/storage/base_request.cpp

@@ -8,34 +8,41 @@
 
 namespace mbgl {
 
+template <typename T, typename ...Args>
+void invoke(const std::forward_list<std::unique_ptr<Callback>> &list, Args&& ...args) {
+    for (const std::unique_ptr<Callback> &callback : list) {
+        assert(callback);
+        if (callback->is<T>()) {
+            callback->get<T>()(::std::forward<Args>(args)...);
+        }
+    }
+}
+
 BaseRequest::BaseRequest(const std::string &path_) : thread_id(uv_thread_self()), path(path_) {
 }
 
+// A base request can only be "canceled" by destroying the object. In that case, we'll have to
+// notify all cancel callbacks.
 BaseRequest::~BaseRequest() {
     assert(thread_id == uv_thread_self());
+    invoke<AbortedCallback>(callbacks);
 }
 
 void BaseRequest::notify() {
     assert(thread_id == uv_thread_self());
 
+    // We are only supposed to call notify when a result exists.
+    assert(response);
+
     // The parameter exists solely so that any calls to ->remove()
     // are not going to cause deallocation of this object while this call is in progress.
     util::ptr<BaseRequest> retain = self;
 
     // Swap the lists so that it's safe for callbacks to call ->cancel()
     // on the request object, which would modify the list.
-    std::forward_list<std::unique_ptr<Callback>> list;
-    list.swap(callbacks);
-
-    // Only notify when we actually have a response. When the response code is 0, the request was
-    // canceled and we don't need to notify.
-    if (response) {
-        // Now that we have our private copy, notify all observers.
-        for (std::unique_ptr<Callback> &callback : list) {
-            assert(callback);
-            (*callback)(*response);
-        }
-    }
+    const std::forward_list<std::unique_ptr<Callback>> list = std::move(callbacks);
+    callbacks.clear();
+    invoke<CompletedCallback>(list, *response);
 
     self.reset();
 }
@@ -43,9 +50,15 @@ void BaseRequest::notify() {
 Callback *BaseRequest::add(Callback &&callback, const util::ptr<BaseRequest> &request) {
     assert(thread_id == uv_thread_self());
     assert(this == request.get());
+
     if (response) {
         // We already have a response. Notify right away.
-        callback(*response);
+        if (callback.is<CompletedCallback>()) {
+            callback.get<CompletedCallback>()(*response);
+        } else {
+            // We already know that this request was successful. The AbortedCallback will be discarded
+            // here since it would never be called.
+        }
         return nullptr;
     } else {
         self = request;

src/storage/request.cpp

@@ -15,7 +15,17 @@ Request::~Request() {
     assert(thread_id == uv_thread_self());
 }
 
-void Request::onload(Callback cb) {
+void Request::onload(CompletedCallback cb) {
+    assert(thread_id == uv_thread_self());
+    if (base) {
+        Callback *callback = base->add(std::move(cb), base);
+        if (callback) {
+            callbacks.push_front(callback);
+        }
+    }
+}
+
+void Request::oncancel(AbortedCallback cb) {
     assert(thread_id == uv_thread_self());
     if (base) {
         Callback *callback = base->add(std::move(cb), base);

src/text/glyph_store.cpp

@@ -149,7 +149,8 @@ GlyphPBF::GlyphPBF(const std::string &glyphURL, const std::string &fontStack, Gl
 
     // The prepare call jumps back to the main thread.
     fileSource->prepare([&, url, fileSource] {
-        fileSource->request(ResourceType::Glyphs, url)->onload([&, url](const Response &res) {
+        auto request = fileSource->request(ResourceType::Glyphs, url);
+        request->onload([&, url](const Response &res) {
             if (res.code != 200) {
                 // Something went wrong with loading the glyph pbf. Pass on the error to the future listeners.
                 const std::string msg = util::sprintf<255>("[ERROR] failed to load glyphs (%d): %s\n", res.code, res.message.c_str());
@@ -163,6 +164,9 @@ GlyphPBF::GlyphPBF(const std::string &glyphURL, const std::string &fontStack, Gl
                 promise.set_value(*this);
             }
         });
+        request->oncancel([&]() {
+            promise.set_exception(std::make_exception_ptr(std::runtime_error("Loading glyphs was canceled")));
+        });
     });
 }