From 5d42321b73dcafbfd76d38331558e5e60f7d9f1e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Feb 2016 16:16:37 -0800 Subject: [PATCH] [core] Make {Default,Online}FileSource::Impl more similar --- include/mbgl/storage/online_file_source.hpp | 3 - platform/default/default_file_source.cpp | 32 +++---- platform/default/online_file_source.cpp | 101 +++++++------------- 3 files changed, 53 insertions(+), 83 deletions(-) diff --git a/include/mbgl/storage/online_file_source.hpp b/include/mbgl/storage/online_file_source.hpp index 1f346b5e7..aceb141fb 100644 --- a/include/mbgl/storage/online_file_source.hpp +++ b/include/mbgl/storage/online_file_source.hpp @@ -20,11 +20,8 @@ public: std::unique_ptr request(const Resource&, Callback) override; private: - friend class OnlineFileRequest; friend class OnlineFileRequestImpl; - void cancel(FileRequest*); - class Impl; const std::unique_ptr> thread; std::string accessToken; diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp index 5bb171f7c..cdf5e1e44 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -114,7 +114,7 @@ public: getDownload(regionID).setState(state); } - void add(FileRequest* req, Resource resource, Callback callback) { + void request(FileRequest* req, Resource resource, Callback callback) { tasks[req] = std::make_unique(resource, callback, this); } @@ -147,21 +147,6 @@ private: bool offline = false; }; -class DefaultFileRequest : public FileRequest { -public: - DefaultFileRequest(Resource resource, FileSource::Callback callback, util::Thread& thread_) - : thread(thread_), - workRequest(thread.invokeWithCallback(&DefaultFileSource::Impl::add, callback, this, resource)) { - } - - ~DefaultFileRequest() { - thread.invoke(&DefaultFileSource::Impl::cancel, this); - } - - util::Thread& thread; - std::unique_ptr workRequest; -}; - DefaultFileSource::DefaultFileSource(const std::string& cachePath, const std::string& assetRoot) : thread(std::make_unique>(util::ThreadContext{"DefaultFileSource", util::ThreadType::Unknown, util::ThreadPriority::Low}, cachePath)), assetFileSource(std::make_unique(assetRoot)) { @@ -186,6 +171,21 @@ void DefaultFileSource::setMaximumCacheEntrySize(uint64_t) { } std::unique_ptr DefaultFileSource::request(const Resource& resource, Callback callback) { + class DefaultFileRequest : public FileRequest { + public: + DefaultFileRequest(Resource resource_, FileSource::Callback callback_, util::Thread& thread_) + : thread(thread_), + workRequest(thread.invokeWithCallback(&DefaultFileSource::Impl::request, callback_, this, resource_)) { + } + + ~DefaultFileRequest() { + thread.invoke(&DefaultFileSource::Impl::cancel, this); + } + + util::Thread& thread; + std::unique_ptr workRequest; + }; + if (isAssetURL(resource.url)) { return assetFileSource->request(resource, callback); } else { diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp index 01dc23bae..3614501ee 100644 --- a/platform/default/online_file_source.cpp +++ b/platform/default/online_file_source.cpp @@ -20,20 +20,6 @@ namespace mbgl { -class OnlineFileRequest : public FileRequest { -public: - OnlineFileRequest(OnlineFileSource& fileSource_) - : fileSource(fileSource_) { - } - - ~OnlineFileRequest() { - fileSource.cancel(this); - } - - OnlineFileSource& fileSource; - std::unique_ptr workRequest; -}; - class OnlineFileRequestImpl : public util::noncopyable { public: using Callback = std::function; @@ -59,22 +45,35 @@ private: class OnlineFileSource::Impl { public: - using Callback = std::function; + // Dummy parameter is a workaround for a gcc 4.9 bug. + Impl(int) { + NetworkStatus::Subscribe(&reachability); + } - Impl(int); - ~Impl(); + ~Impl() { + NetworkStatus::Unsubscribe(&reachability); + } - void networkIsReachableAgain(); + void request(FileRequest* req, Resource resource, Callback callback) { + pending[req] = std::make_unique(resource, callback, *this); + } - void add(Resource, FileRequest*, Callback); - void cancel(FileRequest*); + void cancel(FileRequest* req) { + pending.erase(req); + } private: friend OnlineFileRequestImpl; + void networkIsReachableAgain() { + for (auto& req : pending) { + req.second->networkIsReachableAgain(*this); + } + } + std::unordered_map> pending; - const std::unique_ptr httpContext; - util::AsyncTask reachability; + const std::unique_ptr httpContext { HTTPContextBase::createContext() }; + util::AsyncTask reachability { std::bind(&Impl::networkIsReachableAgain, this) }; }; OnlineFileSource::OnlineFileSource() @@ -85,10 +84,6 @@ OnlineFileSource::OnlineFileSource() OnlineFileSource::~OnlineFileSource() = default; std::unique_ptr OnlineFileSource::request(const Resource& resource, Callback callback) { - if (!callback) { - throw util::MisuseException("FileSource callback can't be empty"); - } - Resource res = resource; switch (resource.kind) { @@ -117,44 +112,24 @@ std::unique_ptr OnlineFileSource::request(const Resource& resource, break; } - auto req = std::make_unique(*this); - req->workRequest = thread->invokeWithCallback(&Impl::add, callback, res, req.get()); - return std::move(req); + class OnlineFileRequest : public FileRequest { + public: + OnlineFileRequest(Resource resource_, FileSource::Callback callback_, util::Thread& thread_) + : thread(thread_), + workRequest(thread.invokeWithCallback(&OnlineFileSource::Impl::request, callback_, this, resource_)) { + } + + ~OnlineFileRequest() { + thread.invoke(&OnlineFileSource::Impl::cancel, this); + } + + util::Thread& thread; + std::unique_ptr workRequest; + }; + + return std::make_unique(res, callback, *thread); } -void OnlineFileSource::cancel(FileRequest* req) { - thread->invoke(&Impl::cancel, req); -} - -// ----- Impl ----- - -// Dummy parameter is a workaround for a gcc 4.9 bug. -OnlineFileSource::Impl::Impl(int) - : httpContext(HTTPContextBase::createContext()), - reachability(std::bind(&Impl::networkIsReachableAgain, this)) { - NetworkStatus::Subscribe(&reachability); -} - -OnlineFileSource::Impl::~Impl() { - NetworkStatus::Unsubscribe(&reachability); -} - -void OnlineFileSource::Impl::networkIsReachableAgain() { - for (auto& req : pending) { - req.second->networkIsReachableAgain(*this); - } -} - -void OnlineFileSource::Impl::add(Resource resource, FileRequest* req, Callback callback) { - pending[req] = std::make_unique(resource, callback, *this); -} - -void OnlineFileSource::Impl::cancel(FileRequest* req) { - pending.erase(req); -} - -// ----- OnlineFileRequest ----- - OnlineFileRequestImpl::OnlineFileRequestImpl(const Resource& resource_, Callback callback_, OnlineFileSource::Impl& impl) : resource(resource_), callback(callback_) { @@ -165,9 +140,7 @@ OnlineFileRequestImpl::OnlineFileRequestImpl(const Resource& resource_, Callback OnlineFileRequestImpl::~OnlineFileRequestImpl() { if (request) { request->cancel(); - request = nullptr; } - // timer is automatically canceled upon destruction. } static Duration errorRetryTimeout(Response::Error::Reason failedRequestReason, uint32_t failedRequests) {