diff --git a/include/mbgl/storage/base_request.hpp b/include/mbgl/storage/base_request.hpp index c903742f4..832ecbc23 100644 --- a/include/mbgl/storage/base_request.hpp +++ b/include/mbgl/storage/base_request.hpp @@ -1,6 +1,7 @@ #ifndef MBGL_STORAGE_BASE_REQUEST #define MBGL_STORAGE_BASE_REQUEST +#include #include #include @@ -15,8 +16,6 @@ namespace mbgl { class Response; class Request; -using Callback = std::function; - 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 self; - std::forward_list> callbacks; }; diff --git a/include/mbgl/storage/request.hpp b/include/mbgl/storage/request.hpp index 10c938ed1..e603ee527 100644 --- a/include/mbgl/storage/request.hpp +++ b/include/mbgl/storage/request.hpp @@ -1,10 +1,10 @@ #ifndef MBGL_STORAGE_REQUEST #define MBGL_STORAGE_REQUEST +#include #include #include -#include #include 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; class Request { private: @@ -25,7 +24,8 @@ public: Request(const util::ptr &base); ~Request(); - void onload(Callback cb); + void onload(CompletedCallback cb); + void oncancel(AbortedCallback cb); void cancel(); private: diff --git a/include/mbgl/storage/request_callback.hpp b/include/mbgl/storage/request_callback.hpp new file mode 100644 index 000000000..01427bd96 --- /dev/null +++ b/include/mbgl/storage/request_callback.hpp @@ -0,0 +1,22 @@ +#ifndef MBGL_STORAGE_REQUEST_CALLBACK +#define MBGL_STORAGE_REQUEST_CALLBACK + +#include + +#include + +namespace mbgl { + +class Response; + +using CompletedCallback = std::function; +using AbortedCallback = std::function; + +using Callback = mapbox::util::variant< + CompletedCallback, + AbortedCallback +>; + +} + +#endif diff --git a/src/storage/base_request.cpp b/src/storage/base_request.cpp index 9698da73a..8eeda8179 100644 --- a/src/storage/base_request.cpp +++ b/src/storage/base_request.cpp @@ -8,34 +8,41 @@ namespace mbgl { +template +void invoke(const std::forward_list> &list, Args&& ...args) { + for (const std::unique_ptr &callback : list) { + assert(callback); + if (callback->is()) { + callback->get()(::std::forward(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(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 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> 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 : list) { - assert(callback); - (*callback)(*response); - } - } + const std::forward_list> list = std::move(callbacks); + callbacks.clear(); + invoke(list, *response); self.reset(); } @@ -43,9 +50,15 @@ void BaseRequest::notify() { Callback *BaseRequest::add(Callback &&callback, const util::ptr &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()) { + callback.get()(*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; diff --git a/src/storage/request.cpp b/src/storage/request.cpp index 65d1d7eb3..42bf87a84 100644 --- a/src/storage/request.cpp +++ b/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); diff --git a/src/text/glyph_store.cpp b/src/text/glyph_store.cpp index ecf78a17d..1723bd3d9 100644 --- a/src/text/glyph_store.cpp +++ b/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"))); + }); }); }