diff options
author | John Firebaugh <john.firebaugh@gmail.com> | 2015-11-12 15:51:45 -0800 |
---|---|---|
committer | John Firebaugh <john.firebaugh@gmail.com> | 2015-11-16 12:25:47 -0800 |
commit | 7137239cbddb13e1c33240d13002973b5a222775 (patch) | |
tree | 99ad3ca2d8b5230eba3f5bacefe63098568dbdd4 | |
parent | 1caf89c32b80dc300b1fd349a2ece4557890c727 (diff) | |
download | qtlocation-mapboxgl-7137239cbddb13e1c33240d13002973b5a222775.tar.gz |
[core] Use std::unique_ptr for FileSource request
42 files changed, 298 insertions, 617 deletions
diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index 5d018b720e..8d2832b33b 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -18,13 +18,13 @@ public: void setAccessToken(const std::string& t) { accessToken = t; } std::string getAccessToken() const { return accessToken; } - // FileSource API - Request* request(const Resource&, Callback) override; - void cancel(Request*) override; + std::unique_ptr<FileRequest> request(const Resource&, Callback) override; -public: - class Impl; private: + friend class DefaultFileRequest; + void cancel(const Resource&, FileRequest*); + + class Impl; const std::unique_ptr<util::Thread<Impl>> thread; std::string accessToken; }; diff --git a/include/mbgl/storage/file_source.hpp b/include/mbgl/storage/file_source.hpp index 0bb5c0fcaf..0167eccc08 100644 --- a/include/mbgl/storage/file_source.hpp +++ b/include/mbgl/storage/file_source.hpp @@ -1,31 +1,33 @@ #ifndef MBGL_STORAGE_FILE_SOURCE #define MBGL_STORAGE_FILE_SOURCE -#include "response.hpp" -#include "resource.hpp" +#include <mbgl/storage/response.hpp> +#include <mbgl/storage/resource.hpp> #include <mbgl/util/noncopyable.hpp> -#include <mbgl/util/util.hpp> #include <functional> +#include <memory> namespace mbgl { -class Request; +class FileRequest : private util::noncopyable { +public: + virtual ~FileRequest() = default; +}; class FileSource : private util::noncopyable { -protected: - MBGL_STORE_THREAD(tid) - public: virtual ~FileSource() = default; using Callback = std::function<void (Response)>; - // These can be called from any thread. The callback will be invoked in the loop. - // You can only cancel a request from the same thread it was created in. - virtual Request* request(const Resource&, Callback) = 0; - virtual void cancel(Request*) = 0; + // Request a resource. The callback will be called asynchronously, in the same + // thread as the request was made. This thread must have an active RunLoop. The + // request may be cancelled before completion by releasing the returned FileRequest. + // If the request is cancelled before the callback is executed, the callback will + // not be executed. + virtual std::unique_ptr<FileRequest> request(const Resource&, Callback) = 0; }; } diff --git a/include/mbgl/storage/request.hpp b/include/mbgl/storage/request.hpp deleted file mode 100644 index 0f178af9bb..0000000000 --- a/include/mbgl/storage/request.hpp +++ /dev/null @@ -1,52 +0,0 @@ -#ifndef MBGL_STORAGE_DEFAULT_REQUEST -#define MBGL_STORAGE_DEFAULT_REQUEST - -#include <mbgl/storage/resource.hpp> - -#include <mbgl/util/util.hpp> -#include <mbgl/util/noncopyable.hpp> - -#include <mutex> -#include <thread> -#include <functional> -#include <memory> - -typedef struct uv_loop_s uv_loop_t; -namespace uv { class async; } - -namespace mbgl { - -class Response; - -class Request : private util::noncopyable { -public: - using Callback = std::function<void (Response)>; - Request(const Resource &resource, uv_loop_t *loop, Callback callback); - -public: - // May be called from any thread. - void notify(const std::shared_ptr<const Response> &response); - void destruct(); - - // May be called only from the thread the Request was created in. - void cancel(); - -private: - ~Request(); - void notifyCallback(); - -private: - std::mutex mtx; - bool canceled = false; - bool confirmed = false; - const std::unique_ptr<uv::async> async; - Callback callback; - std::shared_ptr<const Response> response; - -public: - const Resource resource; -}; - -} - -#endif diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp index aecbeb6a97..126e915e9b 100644 --- a/platform/default/sqlite_cache.cpp +++ b/platform/default/sqlite_cache.cpp @@ -1,5 +1,5 @@ #include "sqlite_cache_impl.hpp" -#include <mbgl/storage/request.hpp> +#include <mbgl/storage/resource.hpp> #include <mbgl/storage/response.hpp> #include <mbgl/util/compression.hpp> diff --git a/platform/node/README.md b/platform/node/README.md index 2d10736826..028da8dfb1 100644 --- a/platform/node/README.md +++ b/platform/node/README.md @@ -53,21 +53,18 @@ npm test ## Implementing a file source -When creating a `Map`, you must pass an options object (with a required `request` method, optional `cancel` method and optional 'ratio' number) as the first parameter. +When creating a `Map`, you must pass an options object (with a required `request` method and optional 'ratio' number) as the first parameter. ```js var map = new mbgl.Map({ request: function(req) { // TODO }, - cancel: function(req) { - // TODO - }, ratio: 2.0 }); ``` -The `request()` method starts a new request to a file, while `cancel()` tells the FileSource to cancel the request (if possible). The `ratio` sets the scale at which the map will render tiles, such as `2.0` for rendering images for high pixel density displays. The `req` parameter has two properties: +The `request()` method starts a new request to a file. The `ratio` sets the scale at which the map will render tiles, such as `2.0` for rendering images for high pixel density displays. The `req` parameter has two properties: ```json { diff --git a/platform/node/src/node_file_source.cpp b/platform/node/src/node_file_source.cpp index 8f88919638..9a1d7c055a 100644 --- a/platform/node/src/node_file_source.cpp +++ b/platform/node/src/node_file_source.cpp @@ -1,139 +1,44 @@ #include "node_file_source.hpp" #include "node_request.hpp" -#include "util/async_queue.hpp" - -#include <mbgl/storage/request.hpp> -#include <mbgl/util/run_loop.hpp> namespace node_mbgl { -struct NodeFileSource::Action { - const enum : bool { Add, Cancel } type; - mbgl::Resource const resource; +class NodeFileSourceRequest : public mbgl::FileRequest { +public: + std::unique_ptr<mbgl::WorkRequest> workRequest; }; -NodeFileSource::NodeFileSource(v8::Local<v8::Object> options_) : - queue(new Queue(uv_default_loop(), [this](Action &action) { - if (action.type == Action::Add) { - processAdd(action.resource); - } else if (action.type == Action::Cancel) { - processCancel(action.resource); - } - })) -{ +NodeFileSource::NodeFileSource(v8::Local<v8::Object> options_) + : nodeLoop(uv_default_loop()) { options.Reset(options_); - // Make sure that the queue doesn't block the loop from exiting. - queue->unref(); + // This has the effect of unreffing an async handle, which otherwise would keep the + // default loop running. You would think we could do this in the destructor instead, + // but in fact the destructor might not get called if there's no GC pressure which + // would cause the NodeMap object which owns us to get destroyed. + nodeLoop.stop(); } NodeFileSource::~NodeFileSource() { - queue->stop(); - queue = nullptr; - options.Reset(); } -mbgl::Request* NodeFileSource::request(const mbgl::Resource& resource, Callback callback) { - auto req = new mbgl::Request(resource, mbgl::util::RunLoop::getLoop(), std::move(callback)); - - std::lock_guard<std::mutex> lock(observersMutex); - - assert(observers.find(resource) == observers.end()); - observers[resource] = req; - - // This function can be called from any thread. Make sure we're executing the actual call in the - // file source loop by sending it over the queue. It will be processed in processAction(). - queue->send(Action{ Action::Add, resource }); - - return req; -} - -void NodeFileSource::cancel(mbgl::Request* req) { - req->cancel(); - - std::lock_guard<std::mutex> lock(observersMutex); - - auto it = observers.find(req->resource); - if (it == observers.end()) { - return; - } - - observers.erase(it); - - // This function can be called from any thread. Make sure we're executing the actual call in the - // file source loop by sending it over the queue. It will be processed in processAction(). - queue->send(Action{ Action::Cancel, req->resource }); - - req->destruct(); -} - -void NodeFileSource::processAdd(const mbgl::Resource& resource) { - Nan::HandleScope scope; - - // Make sure the loop stays alive as long as request is pending. - if (pending.empty()) { - queue->ref(); - } - - auto requestHandle = NodeRequest::Create(this, resource)->ToObject(); - pending.emplace(resource, requestHandle); - - auto callback = Nan::GetFunction(Nan::New<v8::FunctionTemplate>(NodeRequest::Respond, requestHandle)).ToLocalChecked(); - callback->SetName(Nan::New("respond").ToLocalChecked()); - - v8::Local<v8::Value> argv[] = { requestHandle, callback }; - Nan::MakeCallback(Nan::New(options), "request", 2, argv); -} - -void NodeFileSource::processCancel(const mbgl::Resource& resource) { - Nan::HandleScope scope; - - auto it = pending.find(resource); - if (it == pending.end()) { - // The response callback was already fired. There is no point in calling the cancelation - // callback because the request is already completed. - } else { - v8::Local<v8::Object> requestHandle = Nan::New(it->second); - it->second.Reset(); - pending.erase(it); - - // Make sure the the loop can exit when there are no pending requests. - if (pending.empty()) { - queue->unref(); - } - - if (Nan::Has(Nan::New(options), Nan::New("cancel").ToLocalChecked()).FromJust()) { - v8::Local<v8::Value> argv[] = { requestHandle }; - Nan::MakeCallback(Nan::New(options), "cancel", 1, argv); - } - - // Set the request handle in the request wrapper handle to null - Nan::ObjectWrap::Unwrap<NodeRequest>(requestHandle)->cancel(); - } -} - -void NodeFileSource::notify(const mbgl::Resource& resource, const std::shared_ptr<const mbgl::Response>& response) { - // First, remove the request, since it might be destructed at any point now. - auto it = pending.find(resource); - if (it != pending.end()) { - it->second.Reset(); - pending.erase(it); +std::unique_ptr<mbgl::FileRequest> NodeFileSource::request(const mbgl::Resource& resource, Callback callback) { + auto req = std::make_unique<NodeFileSourceRequest>(); - // Make sure the the loop can exit when there are no pending requests. - if (pending.empty()) { - queue->unref(); - } - } + // This function can be called from any thread. Make sure we're executing the + // JS implementation in the node event loop. + req->workRequest = nodeLoop.invokeWithCallback([this] (mbgl::Resource res, Callback cb) { + Nan::HandleScope scope; - std::lock_guard<std::mutex> lock(observersMutex); + auto requestHandle = NodeRequest::Create(res, cb)->ToObject(); + auto callbackHandle = Nan::GetFunction(Nan::New<v8::FunctionTemplate>(NodeRequest::Respond, requestHandle)).ToLocalChecked(); - auto observersIt = observers.find(resource); - if (observersIt == observers.end()) { - return; - } + v8::Local<v8::Value> argv[] = { requestHandle, callbackHandle }; + Nan::MakeCallback(Nan::New(options), "request", 2, argv); + }, callback, resource); - observersIt->second->notify(response); + return std::move(req); } } diff --git a/platform/node/src/node_file_source.hpp b/platform/node/src/node_file_source.hpp index dc1d06a3b0..4234b791ae 100644 --- a/platform/node/src/node_file_source.hpp +++ b/platform/node/src/node_file_source.hpp @@ -1,6 +1,7 @@ #pragma once #include <mbgl/storage/file_source.hpp> +#include <mbgl/util/run_loop.hpp> #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" @@ -9,47 +10,18 @@ #include <nan.h> #pragma GCC diagnostic pop -#include <memory> -#include <mutex> -#include <unordered_map> - namespace node_mbgl { -namespace util { template <typename T> class AsyncQueue; } - class NodeFileSource : public mbgl::FileSource { public: NodeFileSource(v8::Local<v8::Object>); ~NodeFileSource(); - mbgl::Request* request(const mbgl::Resource&, Callback); - void cancel(mbgl::Request*); - - // visiblity? - void notify(const mbgl::Resource&, const std::shared_ptr<const mbgl::Response>&); + std::unique_ptr<mbgl::FileRequest> request(const mbgl::Resource&, Callback); private: - struct Action; - using Queue = util::AsyncQueue<Action>; - - void processAdd(const mbgl::Resource&); - void processCancel(const mbgl::Resource&); - Nan::Persistent<v8::Object> options; - -private: - std::unordered_map<mbgl::Resource, Nan::Persistent<v8::Object>, mbgl::Resource::Hash> pending; - - // The observers list will hold pointers to all the requests waiting - // for a particular resource. The access must be guarded by a mutex - // because the list is also accessed by a thread from the mbgl::Map - // object and from the main thread when notifying requests of - // completion. Concurrent access is specially needed when - // canceling a request to avoid a deadlock (see #129). - std::unordered_map<mbgl::Resource, mbgl::Request*, mbgl::Resource::Hash> observers; - std::mutex observersMutex; - - Queue *queue = nullptr; + mbgl::util::RunLoop nodeLoop; }; } diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp index 14af3ad5eb..e08e4ffd17 100644 --- a/platform/node/src/node_request.cpp +++ b/platform/node/src/node_request.cpp @@ -1,6 +1,5 @@ #include "node_request.hpp" #include "node_file_source.hpp" -#include <mbgl/storage/request.hpp> #include <mbgl/storage/response.hpp> #include <cmath> @@ -24,27 +23,18 @@ NAN_MODULE_INIT(NodeRequest::Init) { } NAN_METHOD(NodeRequest::New) { - // Extract the pointer from the first argument - if (info.Length() < 2 || !info[0]->IsExternal() || !info[1]->IsExternal()) { - return Nan::ThrowTypeError("Cannot create Request objects explicitly"); - } - - auto source = reinterpret_cast<NodeFileSource*>(info[0].As<v8::External>()->Value()); - auto resource = reinterpret_cast<mbgl::Resource*>(info[1].As<v8::External>()->Value()); - auto req = new NodeRequest(source, *resource); + auto req = new NodeRequest(*reinterpret_cast<mbgl::FileSource::Callback*>(info[0].As<v8::External>()->Value())); req->Wrap(info.This()); - info.GetReturnValue().Set(info.This()); } -v8::Handle<v8::Object> NodeRequest::Create(NodeFileSource* source, const mbgl::Resource& resource) { +v8::Handle<v8::Object> NodeRequest::Create(const mbgl::Resource& resource, mbgl::FileSource::Callback callback) { Nan::EscapableHandleScope scope; v8::Local<v8::Value> argv[] = { - Nan::New<v8::External>(const_cast<NodeFileSource*>(source)), - Nan::New<v8::External>(const_cast<mbgl::Resource*>(&resource)) + Nan::New<v8::External>(const_cast<mbgl::FileSource::Callback*>(&callback)) }; - auto instance = Nan::New(constructor)->NewInstance(2, argv); + auto instance = Nan::New(constructor)->NewInstance(1, argv); Nan::Set(instance, Nan::New("url").ToLocalChecked(), Nan::New(resource.url).ToLocalChecked()); Nan::Set(instance, Nan::New("kind").ToLocalChecked(), Nan::New<v8::Integer>(int(resource.kind))); @@ -53,48 +43,35 @@ v8::Handle<v8::Object> NodeRequest::Create(NodeFileSource* source, const mbgl::R } NAN_METHOD(NodeRequest::Respond) { - auto nodeRequest = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Data().As<v8::Object>()); - - // Request has already been responded to, or was canceled, fail silently. - if (!nodeRequest->resource) { - return info.GetReturnValue().SetUndefined(); - } - - auto source = nodeRequest->source; - auto resource = std::move(nodeRequest->resource); + using Error = mbgl::Response::Error; + mbgl::Response response; if (info.Length() < 1) { - auto response = std::make_shared<mbgl::Response>(); - using Error = mbgl::Response::Error; - response->error = std::make_unique<Error>(Error::Reason::NotFound); - source->notify(*resource, response); - } else if (info[0]->BooleanValue()) { - auto response = std::make_shared<mbgl::Response>(); + response.error = std::make_unique<Error>(Error::Reason::NotFound); + } else if (info[0]->BooleanValue()) { // Store the error string. const Nan::Utf8String message { info[0]->ToString() }; - using Error = mbgl::Response::Error; - response->error = std::make_unique<Error>( + response.error = std::make_unique<Error>( Error::Reason::Other, std::string{ *message, size_t(message.length()) }); - source->notify(*resource, response); } else if (info.Length() < 2 || !info[1]->IsObject()) { return Nan::ThrowTypeError("Second argument must be a response object"); + } else { - auto response = std::make_shared<mbgl::Response>(); auto res = info[1]->ToObject(); if (Nan::Has(res, Nan::New("modified").ToLocalChecked()).FromJust()) { const double modified = Nan::Get(res, Nan::New("modified").ToLocalChecked()).ToLocalChecked()->ToNumber()->Value(); if (!std::isnan(modified)) { - response->modified = modified / 1000; // JS timestamps are milliseconds + response.modified = modified / 1000; // JS timestamps are milliseconds } } if (Nan::Has(res, Nan::New("expires").ToLocalChecked()).FromJust()) { const double expires = Nan::Get(res, Nan::New("expires").ToLocalChecked()).ToLocalChecked()->ToNumber()->Value(); if (!std::isnan(expires)) { - response->expires = expires / 1000; // JS timestamps are milliseconds + response.expires = expires / 1000; // JS timestamps are milliseconds } } @@ -102,14 +79,14 @@ NAN_METHOD(NodeRequest::Respond) { auto etagHandle = Nan::Get(res, Nan::New("etag").ToLocalChecked()).ToLocalChecked(); if (etagHandle->BooleanValue()) { const Nan::Utf8String etag { etagHandle->ToString() }; - response->etag = std::string { *etag, size_t(etag.length()) }; + response.etag = std::string { *etag, size_t(etag.length()) }; } } if (Nan::Has(res, Nan::New("data").ToLocalChecked()).FromJust()) { auto dataHandle = Nan::Get(res, Nan::New("data").ToLocalChecked()).ToLocalChecked(); if (node::Buffer::HasInstance(dataHandle)) { - response->data = std::make_shared<std::string>( + response.data = std::make_shared<std::string>( node::Buffer::Data(dataHandle), node::Buffer::Length(dataHandle) ); @@ -117,26 +94,17 @@ NAN_METHOD(NodeRequest::Respond) { return Nan::ThrowTypeError("Response data must be a Buffer"); } } - - // Send the response object to the NodeFileSource object - source->notify(*resource, response); } + // Send the response object to the NodeFileSource object + Nan::ObjectWrap::Unwrap<NodeRequest>(info.Data().As<v8::Object>())->callback(response); info.GetReturnValue().SetUndefined(); } //////////////////////////////////////////////////////////////////////////////////////////////// // Instance -NodeRequest::NodeRequest(NodeFileSource* source_, const mbgl::Resource& resource_) - : source(source_), - resource(std::make_unique<mbgl::Resource>(resource_)) {} - -NodeRequest::~NodeRequest() { -} - -void NodeRequest::cancel() { - resource.reset(); -} +NodeRequest::NodeRequest(mbgl::FileSource::Callback callback_) + : callback(callback_) {} } diff --git a/platform/node/src/node_request.hpp b/platform/node/src/node_request.hpp index 8e01c544eb..3a06a7b62f 100644 --- a/platform/node/src/node_request.hpp +++ b/platform/node/src/node_request.hpp @@ -8,8 +8,7 @@ #pragma GCC diagnostic pop #include <mbgl/storage/resource.hpp> - -#include <memory> +#include <mbgl/storage/file_source.hpp> namespace node_mbgl { @@ -22,17 +21,13 @@ public: static NAN_METHOD(New); static NAN_METHOD(Respond); - static v8::Handle<v8::Object> Create(NodeFileSource*, const mbgl::Resource&); + static v8::Handle<v8::Object> Create(const mbgl::Resource&, mbgl::FileSource::Callback); static Nan::Persistent<v8::Function> constructor; - NodeRequest(NodeFileSource* source, const mbgl::Resource& resource); - ~NodeRequest(); - - void cancel(); + NodeRequest(mbgl::FileSource::Callback); private: - NodeFileSource* source; - std::unique_ptr<mbgl::Resource> resource; + mbgl::FileSource::Callback callback; }; } diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp index 4b0f2a92ec..f9ab79e4f4 100644 --- a/src/mbgl/annotation/annotation_tile.cpp +++ b/src/mbgl/annotation/annotation_tile.cpp @@ -1,6 +1,7 @@ #include <mbgl/annotation/annotation_tile.hpp> #include <mbgl/util/constants.hpp> #include <mbgl/map/map_data.hpp> +#include <mbgl/storage/file_source.hpp> namespace mbgl { @@ -35,7 +36,7 @@ AnnotationTileMonitor::~AnnotationTileMonitor() { data.getAnnotationManager()->removeTileMonitor(*this); } -Request* AnnotationTileMonitor::monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)> callback_) { +std::unique_ptr<FileRequest> AnnotationTileMonitor::monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)> callback_) { callback = callback_; data.getAnnotationManager()->addTileMonitor(*this); return nullptr; diff --git a/src/mbgl/annotation/annotation_tile.hpp b/src/mbgl/annotation/annotation_tile.hpp index dcd843ed09..70c9f7265f 100644 --- a/src/mbgl/annotation/annotation_tile.hpp +++ b/src/mbgl/annotation/annotation_tile.hpp @@ -46,7 +46,7 @@ public: ~AnnotationTileMonitor(); void update(std::unique_ptr<GeometryTile>); - Request* monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) override; + std::unique_ptr<FileRequest> monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) override; TileID tileID; diff --git a/src/mbgl/map/geometry_tile.hpp b/src/mbgl/map/geometry_tile.hpp index 93312bcf48..d6717ddc47 100644 --- a/src/mbgl/map/geometry_tile.hpp +++ b/src/mbgl/map/geometry_tile.hpp @@ -46,7 +46,7 @@ public: virtual util::ptr<GeometryTileLayer> getLayer(const std::string&) const = 0; }; -class Request; +class FileRequest; class GeometryTileMonitor : private util::noncopyable { public: @@ -60,7 +60,7 @@ public: * * To cease monitoring, release the returned Request. */ - virtual Request* monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) = 0; + virtual std::unique_ptr<FileRequest> monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) = 0; }; class GeometryTileFeatureExtractor { diff --git a/src/mbgl/map/map_context.hpp b/src/mbgl/map/map_context.hpp index c2e0213dfb..433d5ecea5 100644 --- a/src/mbgl/map/map_context.hpp +++ b/src/mbgl/map/map_context.hpp @@ -25,10 +25,10 @@ class Sprite; class Worker; class StillImage; class SpriteImage; +class FileRequest; struct LatLng; struct LatLngBounds; - struct FrameData { std::array<uint16_t, 2> framebufferSize; }; @@ -92,7 +92,7 @@ private: std::string styleURL; std::string styleJSON; - RequestHolder styleRequest; + std::unique_ptr<FileRequest> styleRequest; Map::StillImageCallback callback; size_t sourceCacheSize; diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp index d03e3954ba..f19747c441 100644 --- a/src/mbgl/map/raster_tile_data.hpp +++ b/src/mbgl/map/raster_tile_data.hpp @@ -3,12 +3,11 @@ #include <mbgl/map/tile_data.hpp> #include <mbgl/renderer/raster_bucket.hpp> -#include <mbgl/storage/request_holder.hpp> namespace mbgl { class SourceInfo; -class Request; +class FileRequest; class StyleLayer; class TexturePool; class WorkRequest; @@ -29,7 +28,7 @@ private: TexturePool& texturePool; const SourceInfo& source; Worker& worker; - RequestHolder req; + std::unique_ptr<FileRequest> req; std::unique_ptr<Bucket> bucket; diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp index 3abb8f8a1f..661aa09e84 100644 --- a/src/mbgl/map/source.hpp +++ b/src/mbgl/map/source.hpp @@ -5,7 +5,6 @@ #include <mbgl/map/tile_data.hpp> #include <mbgl/map/tile_cache.hpp> #include <mbgl/style/types.hpp> -#include <mbgl/storage/request_holder.hpp> #include <mbgl/util/noncopyable.hpp> #include <mbgl/util/mat4.hpp> @@ -27,7 +26,7 @@ class MapData; class TexturePool; class Style; class Painter; -class Request; +class FileRequest; class TransformState; class Tile; struct ClipID; @@ -130,7 +129,7 @@ private: std::map<TileID, std::weak_ptr<TileData>> tile_data; TileCache cache; - RequestHolder req; + std::unique_ptr<FileRequest> req; Observer* observer_ = nullptr; }; diff --git a/src/mbgl/map/vector_tile.cpp b/src/mbgl/map/vector_tile.cpp index 1159c6fc22..8b06932615 100644 --- a/src/mbgl/map/vector_tile.cpp +++ b/src/mbgl/map/vector_tile.cpp @@ -181,7 +181,7 @@ VectorTileMonitor::VectorTileMonitor(const SourceInfo& source, const TileID& id, : url(source.tileURL(id, pixelRatio)) { } -Request* VectorTileMonitor::monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)> callback) { +std::unique_ptr<FileRequest> VectorTileMonitor::monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)> callback) { return util::ThreadContext::getFileSource()->request({ Resource::Kind::Tile, url }, [callback, this](Response res) { if (res.data && data == res.data) { // We got the same data again. Abort early. diff --git a/src/mbgl/map/vector_tile.hpp b/src/mbgl/map/vector_tile.hpp index 68ed6c2629..7c7f96abbd 100644 --- a/src/mbgl/map/vector_tile.hpp +++ b/src/mbgl/map/vector_tile.hpp @@ -63,7 +63,7 @@ class VectorTileMonitor : public GeometryTileMonitor { public: VectorTileMonitor(const SourceInfo&, const TileID&, float pixelRatio); - Request* monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) override; + std::unique_ptr<FileRequest> monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)>) override; private: std::string url; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index cd28fcbd41..28be9627ae 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -4,6 +4,7 @@ #include <mbgl/util/worker.hpp> #include <mbgl/util/work_request.hpp> #include <mbgl/style/style.hpp> +#include <mbgl/storage/file_source.hpp> #include <sstream> @@ -24,7 +25,7 @@ VectorTileData::VectorTileData(const TileID& id_, monitor(std::move(monitor_)) { state = State::loading; - req = monitor->monitorTile([callback, this](std::exception_ptr err, std::unique_ptr<GeometryTile> tile) { + tileRequest = monitor->monitorTile([callback, this](std::exception_ptr err, std::unique_ptr<GeometryTile> tile) { if (err) { try { std::rethrow_exception(err); @@ -177,10 +178,8 @@ void VectorTileData::redoPlacement() { } void VectorTileData::cancel() { - if (state != State::obsolete) { - state = State::obsolete; - } - req = nullptr; + state = State::obsolete; + tileRequest.reset(); workRequest.reset(); } diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 81064e3133..1b31f3db86 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -3,7 +3,6 @@ #include <mbgl/map/tile_data.hpp> #include <mbgl/map/tile_worker.hpp> -#include <mbgl/storage/request_holder.hpp> #include <mbgl/text/placement_config.hpp> #include <atomic> @@ -14,6 +13,7 @@ namespace mbgl { class Style; class WorkRequest; +class FileRequest; class GeometryTileMonitor; class VectorTileData : public TileData { @@ -41,8 +41,8 @@ private: TileWorker tileWorker; std::unique_ptr<GeometryTileMonitor> monitor; + std::unique_ptr<FileRequest> tileRequest; std::unique_ptr<WorkRequest> workRequest; - RequestHolder req; // Contains all the Bucket objects for the tile. Buckets are render // objects and they get added by tile parsing operations. diff --git a/src/mbgl/sprite/sprite_store.cpp b/src/mbgl/sprite/sprite_store.cpp index c1f4427959..8e00229050 100644 --- a/src/mbgl/sprite/sprite_store.cpp +++ b/src/mbgl/sprite/sprite_store.cpp @@ -4,7 +4,6 @@ #include <mbgl/storage/file_source.hpp> #include <mbgl/storage/resource.hpp> #include <mbgl/storage/response.hpp> -#include <mbgl/storage/request_holder.hpp> #include <mbgl/util/exception.hpp> #include <mbgl/util/thread_context.hpp> #include <mbgl/util/run_loop.hpp> @@ -17,8 +16,8 @@ namespace mbgl { struct SpriteStore::Loader { std::shared_ptr<const std::string> image; std::shared_ptr<const std::string> json; - RequestHolder jsonRequest; - RequestHolder spriteRequest; + std::unique_ptr<FileRequest> jsonRequest; + std::unique_ptr<FileRequest> spriteRequest; }; SpriteStore::SpriteStore(float pixelRatio_) diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index 47873f76f4..dd8f8370ec 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -1,5 +1,4 @@ #include <mbgl/storage/default_file_source_impl.hpp> -#include <mbgl/storage/request.hpp> #include <mbgl/storage/asset_context_base.hpp> #include <mbgl/storage/http_context_base.hpp> #include <mbgl/storage/network_status.hpp> @@ -35,11 +34,9 @@ DefaultFileSource::DefaultFileSource(FileCache* cache, const std::string& root) root)) { } -DefaultFileSource::~DefaultFileSource() { - MBGL_VERIFY_THREAD(tid); -} +DefaultFileSource::~DefaultFileSource() = default; -Request* DefaultFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr<FileRequest> DefaultFileSource::request(const Resource& resource, Callback callback) { if (!callback) { throw util::MisuseException("FileSource callback can't be empty"); } @@ -68,15 +65,14 @@ Request* DefaultFileSource::request(const Resource& resource, Callback callback) url = resource.url; } - auto req = new Request({ resource.kind, url }, util::RunLoop::getLoop(), std::move(callback)); - thread->invoke(&Impl::add, req); - return req; + Resource res { resource.kind, url }; + auto req = std::make_unique<DefaultFileRequest>(res, *this); + req->workRequest = thread->invokeWithCallback(&Impl::add, callback, res, req.get()); + return std::move(req); } -void DefaultFileSource::cancel(Request* req) { - assert(req); - req->cancel(); - thread->invoke(&Impl::cancel, req); +void DefaultFileSource::cancel(const Resource& res, FileRequest* req) { + thread->invoke(&Impl::cancel, res, req); } // ----- Impl ----- @@ -101,7 +97,7 @@ DefaultFileSource::Impl::~Impl() { void DefaultFileSource::Impl::networkIsReachableAgain() { for (auto& req : pending) { - auto& request = req.second; + auto& request = *req.second; auto& response = request.getResponse(); if (!request.realRequest && response && response->error && response->error->reason == Response::Error::Reason::Connection) { // We need all requests to fail at least once before we are going to start retrying @@ -111,18 +107,19 @@ void DefaultFileSource::Impl::networkIsReachableAgain() { } } -void DefaultFileSource::Impl::add(Request* req) { - auto& request = pending.emplace(req->resource, req->resource).first->second; +void DefaultFileSource::Impl::add(Resource resource, FileRequest* req, Callback callback) { + auto& request = *pending.emplace(resource, + std::make_unique<DefaultFileRequestImpl>(resource)).first->second; // Trigger a potentially required refresh of this Request update(request); // Add this request as an observer so that it'll get notified when something about this // request changes. - request.addObserver(req); + request.addObserver(req, callback); } -void DefaultFileSource::Impl::update(DefaultFileRequest& request) { +void DefaultFileSource::Impl::update(DefaultFileRequestImpl& request) { if (request.getResponse()) { // We've at least obtained a cache value, potentially we also got a final response. // The observers have been notified already; send what we have to the new one as well. @@ -154,7 +151,7 @@ void DefaultFileSource::Impl::update(DefaultFileRequest& request) { } } -void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest& request) { +void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequestImpl& request) { // Check the cache for existing data so that we can potentially // revalidate the information without having to redownload everything. request.cacheRequest = @@ -177,7 +174,7 @@ void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest& request) { }); } -void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest& request) { +void DefaultFileSource::Impl::startRealRequest(DefaultFileRequestImpl& request) { assert(!request.realRequest); // Cancel the timer if we have one. @@ -212,12 +209,12 @@ void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest& request) { } } -void DefaultFileSource::Impl::cancel(Request* req) { - auto it = pending.find(req->resource); +void DefaultFileSource::Impl::cancel(Resource resource, FileRequest* req) { + auto it = pending.find(resource); if (it != pending.end()) { // If the number of dependent requests of the DefaultFileRequest drops to zero, // cancel the request and remove it from the pending list. - auto& request = it->second; + auto& request = *it->second; request.removeObserver(req); if (!request.hasObservers()) { pending.erase(it); @@ -226,13 +223,9 @@ void DefaultFileSource::Impl::cancel(Request* req) { // There is no request for this URL anymore. Likely, the request already completed // before we got around to process the cancelation request. } - - // Send a message back to the requesting thread and notify it that this request has been - // canceled and is now safe to be deleted. - req->destruct(); } -void DefaultFileSource::Impl::reschedule(DefaultFileRequest& request) { +void DefaultFileSource::Impl::reschedule(DefaultFileRequestImpl& request) { if (request.realRequest) { // There's already a request in progress; don't start another one. return; @@ -257,7 +250,7 @@ void DefaultFileSource::Impl::reschedule(DefaultFileRequest& request) { // ----- DefaultFileRequest ----- -DefaultFileRequest::~DefaultFileRequest() { +DefaultFileRequestImpl::~DefaultFileRequestImpl() { if (realRequest) { realRequest->cancel(); realRequest = nullptr; @@ -265,32 +258,32 @@ DefaultFileRequest::~DefaultFileRequest() { // timerRequest and cacheRequest are automatically canceld upon destruction. } -void DefaultFileRequest::addObserver(Request* req) { - observers.insert(req); +void DefaultFileRequestImpl::addObserver(FileRequest* req, Callback callback) { + observers.emplace(req, callback); if (response) { // We've got a response, so send the (potentially stale) response to the requester. - req->notify(response); + callback(*response); } } -void DefaultFileRequest::removeObserver(Request* req) { +void DefaultFileRequestImpl::removeObserver(FileRequest* req) { observers.erase(req); } -bool DefaultFileRequest::hasObservers() const { +bool DefaultFileRequestImpl::hasObservers() const { return !observers.empty(); } -void DefaultFileRequest::notify() { +void DefaultFileRequestImpl::notify() { if (response) { - for (auto req : observers) { - req->notify(response); + for (auto& req : observers) { + req.second(*response); } } } -void DefaultFileRequest::setResponse(const std::shared_ptr<const Response>& response_) { +void DefaultFileRequestImpl::setResponse(const std::shared_ptr<const Response>& response_) { response = response_; if (response->error) { @@ -301,11 +294,11 @@ void DefaultFileRequest::setResponse(const std::shared_ptr<const Response>& resp } } -const std::shared_ptr<const Response>& DefaultFileRequest::getResponse() const { +const std::shared_ptr<const Response>& DefaultFileRequestImpl::getResponse() const { return response; } -int64_t DefaultFileRequest::getRetryTimeout() const { +int64_t DefaultFileRequestImpl::getRetryTimeout() const { if (!response) { // If we don't have a response, we should retry immediately. return 0; @@ -350,7 +343,7 @@ int64_t DefaultFileRequest::getRetryTimeout() const { return timeout; } -void DefaultFileRequest::checkResponseFreshness() { +void DefaultFileRequestImpl::checkResponseFreshness() { if (response && !response->stale && response->isExpired()) { // Create a new Response object with `stale = true`, but the same data, and // replace the current request object we have. diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp index 4ca8e42721..c063a7d1f8 100644 --- a/src/mbgl/storage/default_file_source_impl.hpp +++ b/src/mbgl/storage/default_file_source_impl.hpp @@ -4,6 +4,7 @@ #include <mbgl/storage/default_file_source.hpp> #include <mbgl/storage/asset_context_base.hpp> #include <mbgl/storage/http_context_base.hpp> +#include <mbgl/util/noncopyable.hpp> #include <set> #include <unordered_map> @@ -12,27 +13,41 @@ namespace mbgl { class RequestBase; -class DefaultFileRequest { +class DefaultFileRequest : public FileRequest { public: + DefaultFileRequest(const Resource& resource_, + DefaultFileSource& fileSource_) + : resource(resource_), + fileSource(fileSource_) { + } + + ~DefaultFileRequest() { + fileSource.cancel(resource, this); + } + + Resource resource; + DefaultFileSource& fileSource; + + std::unique_ptr<WorkRequest> workRequest; +}; + +class DefaultFileRequestImpl : public util::noncopyable { +public: + using Callback = std::function<void (Response)>; + const Resource resource; std::unique_ptr<WorkRequest> cacheRequest; RequestBase* realRequest = nullptr; std::unique_ptr<uv::timer> timerRequest; - inline DefaultFileRequest(const Resource& resource_) + inline DefaultFileRequestImpl(const Resource& resource_) : resource(resource_) {} -public: - // Make it movable-only. - DefaultFileRequest(const DefaultFileRequest&) = delete; - inline DefaultFileRequest(DefaultFileRequest&&) = default; - DefaultFileRequest& operator=(const DefaultFileRequest&) = delete; - inline DefaultFileRequest& operator=(DefaultFileRequest&&) = default; - ~DefaultFileRequest(); + ~DefaultFileRequestImpl(); // Observer accessors. - void addObserver(Request*); - void removeObserver(Request*); + void addObserver(FileRequest*, Callback); + void removeObserver(FileRequest*); bool hasObservers() const; // Updates/gets the response of this request object. @@ -54,7 +69,7 @@ public: private: // Stores a set of all observing Request objects. - std::set<Request*> observers; + std::unordered_map<FileRequest*, Callback> observers; // The current response data. We're storing it because we can satisfy requests for the same // resource directly by returning this response object. We also need it to create conditional @@ -68,21 +83,23 @@ private: class DefaultFileSource::Impl { public: + using Callback = std::function<void (Response)>; + Impl(FileCache*, const std::string& = ""); ~Impl(); void networkIsReachableAgain(); - void add(Request*); - void cancel(Request*); + void add(Resource, FileRequest*, Callback); + void cancel(Resource, FileRequest*); private: - void update(DefaultFileRequest&); - void startCacheRequest(DefaultFileRequest&); - void startRealRequest(DefaultFileRequest&); - void reschedule(DefaultFileRequest&); + void update(DefaultFileRequestImpl&); + void startCacheRequest(DefaultFileRequestImpl&); + void startRealRequest(DefaultFileRequestImpl&); + void reschedule(DefaultFileRequestImpl&); - std::unordered_map<Resource, DefaultFileRequest, Resource::Hash> pending; + std::unordered_map<Resource, std::unique_ptr<DefaultFileRequestImpl>, Resource::Hash> pending; uv_loop_t* const loop; FileCache* const cache; const std::string assetRoot; diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp deleted file mode 100644 index 5ed9278d0a..0000000000 --- a/src/mbgl/storage/request.cpp +++ /dev/null @@ -1,76 +0,0 @@ -#include <mbgl/storage/request.hpp> - -#include <mbgl/storage/response.hpp> -#include <mbgl/util/util.hpp> -#include <mbgl/util/uv_detail.hpp> - -#include <cassert> -#include <functional> -#include <memory> - -namespace mbgl { - -// Note: This requires that loop is running in the current thread (or not yet running). -Request::Request(const Resource &resource_, uv_loop_t *loop, Callback callback_) - : async(std::make_unique<uv::async>(loop, [this] { notifyCallback(); })), - callback(callback_), - resource(resource_) { -} - -// Called in the originating thread. -void Request::notifyCallback() { - std::unique_lock<std::mutex> lock(mtx); - if (!canceled) { - // Move the response object out so that we can't accidentally notify twice. - auto res = std::move(response); - assert(!response); - // Unlock before, since callbacks may call cancel, which also locks this mutex. - lock.unlock(); - // The user could supply a null pointer or empty std::function as a callback. In this case, we - // still do the file request, but we don't need to deliver a result. - // Similarly, two consecutive updates could trigger two notifyCallbacks, so we need to make - // sure - if (callback && res) { - callback(*res); - } - } else { - // Don't delete right way, because we have to unlock the mutex before deleting. - if (confirmed) { - lock.unlock(); - delete this; - } - } -} - -Request::~Request() = default; - -// Called in the FileSource thread. -void Request::notify(const std::shared_ptr<const Response> &response_) { - std::lock_guard<std::mutex> lock(mtx); - assert(response_); - response = response_; - async->send(); -} - -// Called in the originating thread. -void Request::cancel() { - std::lock_guard<std::mutex> lock(mtx); - assert(!canceled); - canceled = true; -} - - -// Called in the FileSource thread. -// Will only ever be invoked after cancel() was called in the original requesting thread. -void Request::destruct() { - std::lock_guard<std::mutex> lock(mtx); - assert(canceled); - confirmed = true; - // We need to extend the lock until after the async has been sent, otherwise the requesting - // thread could destroy the async while this call is still in progress. - async->send(); - // After this method returns, the FileSource thread has no knowledge of - // this object anymore. -} - -} diff --git a/src/mbgl/storage/request_holder.cpp b/src/mbgl/storage/request_holder.cpp deleted file mode 100644 index 3a038623c4..0000000000 --- a/src/mbgl/storage/request_holder.cpp +++ /dev/null @@ -1,12 +0,0 @@ -#include <mbgl/storage/request_holder.hpp> -#include <mbgl/storage/file_source.hpp> -#include <mbgl/util/thread_context.hpp> - -namespace mbgl { - -void RequestHolder::Deleter::operator()(Request* req) const { - // This function is called by the unique_ptr's Deleter. - util::ThreadContext::getFileSource()->cancel(req); -} - -} diff --git a/src/mbgl/storage/request_holder.hpp b/src/mbgl/storage/request_holder.hpp deleted file mode 100644 index 62edbfde7d..0000000000 --- a/src/mbgl/storage/request_holder.hpp +++ /dev/null @@ -1,26 +0,0 @@ -#ifndef MBGL_STORAGE_REQUEST_HOLDER -#define MBGL_STORAGE_REQUEST_HOLDER - -#include <memory> - -namespace mbgl { - -class Request; - -class RequestHolder { -public: - inline RequestHolder& operator=(Request* req) { - ptr = std::unique_ptr<Request, Deleter>(req); - return *this; - } - -private: - struct Deleter { - void operator()(Request*) const; - }; - std::unique_ptr<Request, Deleter> ptr; -}; - -} - -#endif diff --git a/src/mbgl/text/glyph_pbf.hpp b/src/mbgl/text/glyph_pbf.hpp index bf8567dbec..69d7747471 100644 --- a/src/mbgl/text/glyph_pbf.hpp +++ b/src/mbgl/text/glyph_pbf.hpp @@ -3,17 +3,17 @@ #include <mbgl/text/glyph.hpp> #include <mbgl/util/noncopyable.hpp> -#include <mbgl/storage/request_holder.hpp> #include <atomic> #include <functional> #include <string> +#include <memory> namespace mbgl { class GlyphStore; class FontStack; -class Request; +class FileRequest; class GlyphPBF : private util::noncopyable { public: @@ -45,7 +45,7 @@ private: std::shared_ptr<const std::string> data; std::atomic<bool> parsed; - RequestHolder req; + std::unique_ptr<FileRequest> req; Observer* observer = nullptr; }; diff --git a/test/fixtures/mock_file_source.cpp b/test/fixtures/mock_file_source.cpp index 33d7397d54..ea580276d6 100644 --- a/test/fixtures/mock_file_source.cpp +++ b/test/fixtures/mock_file_source.cpp @@ -1,11 +1,11 @@ #include "../fixtures/util.hpp" #include "mock_file_source.hpp" -#include <mbgl/storage/request.hpp> #include <mbgl/util/io.hpp> #include <mbgl/util/thread.hpp> #include <algorithm> +#include <unordered_map> namespace { @@ -15,6 +15,24 @@ const uint64_t timeout = 1000000; namespace mbgl { +class MockFileRequest : public FileRequest { +public: + MockFileRequest(const Resource& resource_, + MockFileSource& fileSource_) + : resource(resource_), + fileSource(fileSource_) { + } + + ~MockFileRequest() { + fileSource.cancel(this); + } + + Resource resource; + MockFileSource& fileSource; + + std::unique_ptr<WorkRequest> workRequest; +}; + class MockFileSource::Impl { public: Impl(Type type, const std::string& match) @@ -31,107 +49,97 @@ public: requestEnqueuedCallback_ = callback; } - void handleRequest(Request* req); - void cancelRequest(Request* req); + void handleRequest(FileRequest*, Resource, Callback); + void cancelRequest(FileRequest*); private: - void replyWithSuccess(Request* req) const; - void replyWithSuccessWithDelay(Request* req); - void replyWithFailure(Request* req) const; - void replyWithCorruptedData(Request* req) const; + void replyWithSuccess(Resource, Callback) const; + void replyWithSuccessWithDelay(FileRequest*, Resource, Callback); + void replyWithFailure(Resource, Callback) const; + void replyWithCorruptedData(Resource, Callback) const; void dispatchPendingRequests(); Type type_; std::string match_; - std::vector<Request*> pendingRequests_; + std::unordered_map<FileRequest*, std::pair<Resource, Callback>> pendingRequests_; uv::timer timer_; std::function<void(void)> requestEnqueuedCallback_; }; -void MockFileSource::Impl::replyWithSuccess(Request* req) const { - std::shared_ptr<Response> res = std::make_shared<Response>(); +void MockFileSource::Impl::replyWithSuccess(Resource resource, Callback callback) const { + Response res; try { - res->data = std::make_shared<const std::string>(std::move(util::read_file(req->resource.url))); + res.data = std::make_shared<const std::string>(std::move(util::read_file(resource.url))); } catch (const std::exception& err) { - res->error = std::make_unique<Response::Error>(Response::Error::Reason::Other, err.what()); + res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, err.what()); } - req->notify(res); + callback(res); } -void MockFileSource::Impl::replyWithSuccessWithDelay(Request* req) { - if (req->resource.url.find(match_) == std::string::npos) { - replyWithSuccess(req); +void MockFileSource::Impl::replyWithSuccessWithDelay(FileRequest* req, Resource resource, Callback callback) { + if (resource.url.find(match_) == std::string::npos) { + replyWithSuccess(resource, callback); return; } - pendingRequests_.push_back(req); + pendingRequests_.emplace(req, std::make_pair(resource, callback)); requestEnqueuedCallback_(); } -void MockFileSource::Impl::replyWithFailure(Request* req) const { - if (req->resource.url.find(match_) == std::string::npos) { - replyWithSuccess(req); +void MockFileSource::Impl::replyWithFailure(Resource resource, Callback callback) const { + if (resource.url.find(match_) == std::string::npos) { + replyWithSuccess(resource, callback); return; } - std::shared_ptr<Response> res = std::make_shared<Response>(); - res->error = std::make_unique<Response::Error>(Response::Error::Reason::Other, "Failed by the test case"); - - req->notify(res); + Response res; + res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, "Failed by the test case"); + callback(res); } -void MockFileSource::Impl::replyWithCorruptedData(Request* req) const { - if (req->resource.url.find(match_) == std::string::npos) { - replyWithSuccess(req); +void MockFileSource::Impl::replyWithCorruptedData(Resource resource, Callback callback) const { + if (resource.url.find(match_) == std::string::npos) { + replyWithSuccess(resource, callback); return; } - std::shared_ptr<Response> res = std::make_shared<Response>(); - auto data = std::make_shared<std::string>(std::move(util::read_file(req->resource.url))); + Response res; + auto data = std::make_shared<std::string>(std::move(util::read_file(resource.url))); data->insert(0, "CORRUPTED"); - res->data = std::move(data); - - req->notify(res); + res.data = std::move(data); + callback(res); } -void MockFileSource::Impl::handleRequest(Request* req) { +void MockFileSource::Impl::handleRequest(FileRequest* req, Resource resource, Callback callback) { switch (type_) { case Type::Success: - replyWithSuccess(req); + replyWithSuccess(resource, callback); break; case Type::SuccessWithDelay: - replyWithSuccessWithDelay(req); + replyWithSuccessWithDelay(req, resource, callback); break; case Type::RequestFail: - replyWithFailure(req); + replyWithFailure(resource, callback); break; case Type::RequestWithCorruptedData: - replyWithCorruptedData(req); + replyWithCorruptedData(resource, callback); break; default: EXPECT_TRUE(false) << "Should never be reached."; } } -void MockFileSource::Impl::cancelRequest(Request* req) { - auto it = std::find(pendingRequests_.begin(), pendingRequests_.end(), req); - if (it != pendingRequests_.end()) { - pendingRequests_.erase(it); - } else { - // There is no request for this URL anymore. Likely, the request already completed - // before we got around to process the cancelation request. - } - - req->destruct(); +void MockFileSource::Impl::cancelRequest(FileRequest* req) { + pendingRequests_.erase(req); } void MockFileSource::Impl::dispatchPendingRequests() { - for (auto req : pendingRequests_) { - replyWithSuccess(req); + for (auto& pair : pendingRequests_) { + replyWithSuccess(pair.second.first, pair.second.second); } pendingRequests_.clear(); @@ -145,15 +153,13 @@ void MockFileSource::setOnRequestDelayedCallback(std::function<void(void)> callb thread_->invokeSync(&Impl::setOnRequestDelayedCallback, callback); } -Request* MockFileSource::request(const Resource& resource, Callback callback) { - Request* req = new Request(resource, util::RunLoop::getLoop(), std::move(callback)); - thread_->invoke(&Impl::handleRequest, req); - - return req; +std::unique_ptr<FileRequest> MockFileSource::request(const Resource& res, Callback callback) { + auto req = std::make_unique<MockFileRequest>(res, *this); + req->workRequest = thread_->invokeWithCallback(&Impl::handleRequest, callback, req.get(), res); + return std::move(req); } -void MockFileSource::cancel(Request* req) { - req->cancel(); +void MockFileSource::cancel(FileRequest* req) { thread_->invoke(&Impl::cancelRequest, req); } diff --git a/test/fixtures/mock_file_source.hpp b/test/fixtures/mock_file_source.hpp index 716b0672e3..f41f808e36 100644 --- a/test/fixtures/mock_file_source.hpp +++ b/test/fixtures/mock_file_source.hpp @@ -47,10 +47,12 @@ public: void setOnRequestDelayedCallback(std::function<void(void)> callback); // FileSource implementation. - Request* request(const Resource&, Callback) override; - void cancel(Request*) override; + std::unique_ptr<FileRequest> request(const Resource&, Callback) override; private: + friend class MockFileRequest; + void cancel(FileRequest*); + const std::unique_ptr<util::Thread<Impl>> thread_; }; diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp index fb745d9ad4..d089659aeb 100644 --- a/test/storage/cache_response.cpp +++ b/test/storage/cache_response.cpp @@ -18,11 +18,11 @@ TEST_F(Storage, CacheResponse) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/cache" }; Response response; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(resource, [&](Response res) { - fs.cancel(req1); + req1.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -35,7 +35,7 @@ TEST_F(Storage, CacheResponse) { // Now test that we get the same values as in the previous request. If we'd go to the server // again, we'd get different values. req2 = fs.request(resource, [&](Response res2) { - fs.cancel(req2); + req2.reset(); EXPECT_EQ(response.error, res2.error); EXPECT_EQ(response.stale, res2.stale); ASSERT_TRUE(res2.data.get()); diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp index 0dfe765e7f..540a25acd6 100644 --- a/test/storage/cache_revalidate.cpp +++ b/test/storage/cache_revalidate.cpp @@ -16,8 +16,8 @@ TEST_F(Storage, CacheRevalidateSame) { util::RunLoop loop(uv_default_loop()); const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(revalidateSame, [&](Response res) { // This callback can get triggered multiple times. We only care about the first invocation. // It will get triggered again when refreshing the req2 (see below). @@ -41,13 +41,11 @@ TEST_F(Storage, CacheRevalidateSame) { return; } - ASSERT_TRUE(req1); - fs.cancel(req1); - req1 = nullptr; + ASSERT_TRUE(req1.get()); + req1.reset(); - ASSERT_TRUE(req2); - fs.cancel(req2); - req2 = nullptr; + ASSERT_TRUE(req2.get()); + req2.reset(); EXPECT_EQ(nullptr, res2.error); EXPECT_EQ(false, res2.stale); @@ -79,8 +77,8 @@ TEST_F(Storage, CacheRevalidateModified) { const Resource revalidateModified{ Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(revalidateModified, [&](Response res) { // This callback can get triggered multiple times. We only care about the first invocation. // It will get triggered again when refreshing the req2 (see below). @@ -104,13 +102,11 @@ TEST_F(Storage, CacheRevalidateModified) { return; } - ASSERT_TRUE(req1); - fs.cancel(req1); - req1 = nullptr; + ASSERT_TRUE(req1.get()); + req1.reset(); - ASSERT_TRUE(req2); - fs.cancel(req2); - req2 = nullptr; + ASSERT_TRUE(req2.get()); + req2.reset(); EXPECT_EQ(nullptr, res2.error); EXPECT_EQ(false, res2.stale); @@ -140,8 +136,8 @@ TEST_F(Storage, CacheRevalidateEtag) { util::RunLoop loop(uv_default_loop()); const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" }; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(revalidateEtag, [&](Response res) { // This callback can get triggered multiple times. We only care about the first invocation. // It will get triggered again when refreshing the req2 (see below). @@ -165,13 +161,11 @@ TEST_F(Storage, CacheRevalidateEtag) { return; } - ASSERT_TRUE(req1); - fs.cancel(req1); - req1 = nullptr; + ASSERT_TRUE(req1.get()); + req1.reset(); - ASSERT_TRUE(req2); - fs.cancel(req2); - req2 = nullptr; + ASSERT_TRUE(req2.get()); + req2.reset(); EXPECT_EQ(nullptr, res2.error); EXPECT_EQ(false, res2.stale); diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index ae8c0b42c1..fab52315fa 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -18,8 +18,8 @@ TEST_F(Storage, AssetReadDirectory) { util::RunLoop loop(uv_default_loop()); - Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, [&](Response res) { + req.reset(); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); EXPECT_EQ(false, res.stale); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index b9155c09c5..3fc30df524 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -19,8 +19,8 @@ TEST_F(Storage, AssetEmptyFile) { util::RunLoop loop(uv_default_loop()); - Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, [&](Response res) { + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -48,8 +48,8 @@ TEST_F(Storage, AssetNonEmptyFile) { util::RunLoop loop(uv_default_loop()); - Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, [&](Response res) { + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -79,8 +79,8 @@ TEST_F(Storage, AssetNonExistentFile) { util::RunLoop loop(uv_default_loop()); - Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, [&](Response res) { + req.reset(); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); EXPECT_EQ(false, res.stale); diff --git a/test/storage/http_cancel.cpp b/test/storage/http_cancel.cpp index 070c54b8b4..eabc895902 100644 --- a/test/storage/http_cancel.cpp +++ b/test/storage/http_cancel.cpp @@ -20,7 +20,7 @@ TEST_F(Storage, HTTPCancel) { fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, [&](Response) { ADD_FAILURE() << "Callback should not be called"; }); - fs.cancel(req); + req.reset(); HTTPCancel.finish(); uv_run(uv_default_loop(), UV_RUN_ONCE); @@ -36,11 +36,11 @@ TEST_F(Storage, HTTPCancelMultiple) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; - auto req2 = fs.request(resource, [&](Response) { + std::unique_ptr<FileRequest> req2 = fs.request(resource, [&](Response) { ADD_FAILURE() << "Callback should not be called"; }); - Request* req = fs.request(resource, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request(resource, [&](Response res) { + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -51,7 +51,7 @@ TEST_F(Storage, HTTPCancelMultiple) { loop.stop(); HTTPCancelMultiple.finish(); }); - fs.cancel(req2); + req2.reset(); uv_run(uv_default_loop(), UV_RUN_DEFAULT); } diff --git a/test/storage/http_coalescing.cpp b/test/storage/http_coalescing.cpp index 58f30baebb..38fc0151a3 100644 --- a/test/storage/http_coalescing.cpp +++ b/test/storage/http_coalescing.cpp @@ -43,10 +43,10 @@ TEST_F(Storage, HTTPCoalescing) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; - Request* reqs[total]; + std::unique_ptr<FileRequest> reqs[total]; for (int i = 0; i < total; i++) { reqs[i] = fs.request(resource, [&complete, &fs, &reqs, i] (Response res) { - fs.cancel(reqs[i]); + reqs[i].reset(); complete(res); }); } @@ -63,8 +63,8 @@ TEST_F(Storage, HTTPMultiple) { util::RunLoop loop(uv_default_loop()); const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?expires=2147483647" }; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(resource, [&] (Response res) { // Do not cancel the request right away. EXPECT_EQ(nullptr, res.error); @@ -80,8 +80,8 @@ TEST_F(Storage, HTTPMultiple) { EXPECT_EQ(res.data.get(), res2.data.get()); // Now cancel both requests after both have been notified. - fs.cancel(req1); - fs.cancel(req2); + req1.reset(); + req2.reset(); EXPECT_EQ(nullptr, res2.error); ASSERT_TRUE(res2.data.get()); @@ -111,8 +111,8 @@ TEST_F(Storage, HTTPStale) { int stale = 0; const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; - Request* req1 = nullptr; - Request* req2 = nullptr; + std::unique_ptr<FileRequest> req1; + std::unique_ptr<FileRequest> req2; req1 = fs.request(resource, [&] (Response res) { // Do not cancel the request right away. EXPECT_EQ(nullptr, res.error); @@ -144,8 +144,8 @@ TEST_F(Storage, HTTPStale) { stale++; } else { // Now cancel both requests after both have been notified. - fs.cancel(req1); - fs.cancel(req2); + req1.reset(); + req2.reset(); loop.stop(); HTTPStale.finish(); } diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index ce04e871eb..276793b77f 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -18,7 +18,7 @@ TEST_F(Storage, HTTPTemporaryError) { const auto start = uv_hrtime(); - Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, [&](Response res) { + std::unique_ptr<FileRequest> req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, [&](Response res) { static int counter = 0; switch (counter++) { case 0: { @@ -35,7 +35,7 @@ TEST_F(Storage, HTTPTemporaryError) { EXPECT_EQ("", res.etag); } break; case 1: { - fs.cancel(req1); + req1.reset(); const auto duration = double(uv_hrtime() - start) / 1e9; EXPECT_LT(0.99, duration) << "Backoff timer didn't wait 1 second"; EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; @@ -65,7 +65,7 @@ TEST_F(Storage, HTTPConnectionError) { const auto start = uv_hrtime(); - Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, [&](Response res) { + std::unique_ptr<FileRequest> req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, [&](Response res) { static int counter = 0; static int wait = 0; const auto duration = double(uv_hrtime() - start) / 1e9; @@ -91,7 +91,7 @@ TEST_F(Storage, HTTPConnectionError) { EXPECT_EQ("", res.etag); if (counter == 2) { - fs.cancel(req2); + req2.reset(); loop.stop(); HTTPConnectionError.finish(); } diff --git a/test/storage/http_header_parsing.cpp b/test/storage/http_header_parsing.cpp index 8048addb52..041b138b60 100644 --- a/test/storage/http_header_parsing.cpp +++ b/test/storage/http_header_parsing.cpp @@ -16,10 +16,10 @@ TEST_F(Storage, HTTPExpiresParsing) { DefaultFileSource fs(nullptr); util::RunLoop loop(uv_default_loop()); - Request* req1 = fs.request({ Resource::Unknown, + std::unique_ptr<FileRequest> req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?modified=1420794326&expires=1420797926&etag=foo" }, [&](Response res) { - fs.cancel(req1); + req1.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -45,9 +45,9 @@ TEST_F(Storage, HTTPCacheControlParsing) { int64_t now = std::chrono::duration_cast<std::chrono::seconds>( SystemClock::now().time_since_epoch()).count(); - Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" }, + std::unique_ptr<FileRequest> req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" }, [&](Response res) { - fs.cancel(req2); + req2.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); diff --git a/test/storage/http_issue_1369.cpp b/test/storage/http_issue_1369.cpp index 5c05aead31..0fe0f5fb1b 100644 --- a/test/storage/http_issue_1369.cpp +++ b/test/storage/http_issue_1369.cpp @@ -31,9 +31,9 @@ TEST_F(Storage, HTTPIssue1369) { auto req = fs.request(resource, [&](Response) { ADD_FAILURE() << "Callback should not be called"; }); - fs.cancel(req); + req.reset(); req = fs.request(resource, [&](Response res) { - fs.cancel(req); + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); diff --git a/test/storage/http_load.cpp b/test/storage/http_load.cpp index fef1f06e7b..0a6b0ef653 100644 --- a/test/storage/http_load.cpp +++ b/test/storage/http_load.cpp @@ -17,15 +17,14 @@ TEST_F(Storage, HTTPLoad) { const int max = 10000; int number = 1; - Request* reqs[concurrency]; + std::unique_ptr<FileRequest> reqs[concurrency]; std::function<void(int)> req = [&](int i) { const auto current = number++; reqs[i] = fs.request({ Resource::Unknown, std::string("http://127.0.0.1:3000/load/") + std::to_string(current) }, [&, i, current](Response res) { - fs.cancel(reqs[i]); - reqs[i] = nullptr; + reqs[i].reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); diff --git a/test/storage/http_other_loop.cpp b/test/storage/http_other_loop.cpp index 52475ae1a4..b432c39ac2 100644 --- a/test/storage/http_other_loop.cpp +++ b/test/storage/http_other_loop.cpp @@ -14,9 +14,9 @@ TEST_F(Storage, HTTPOtherLoop) { DefaultFileSource fs(nullptr); util::RunLoop loop(uv_default_loop()); - Request* req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, + std::unique_ptr<FileRequest> req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, [&](Response res) { - fs.cancel(req); + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index b72f060e63..b072a6eb53 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -18,9 +18,9 @@ TEST_F(Storage, HTTPTest) { const auto mainThread = uv_thread_self(); - Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, + std::unique_ptr<FileRequest> req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, [&](Response res) { - fs.cancel(req1); + req1.reset(); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); @@ -46,9 +46,9 @@ TEST_F(Storage, HTTP404) { const auto mainThread = uv_thread_self(); - Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/doesnotexist" }, + std::unique_ptr<FileRequest> req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/doesnotexist" }, [&](Response res) { - fs.cancel(req2); + req2.reset(); EXPECT_EQ(uv_thread_self(), mainThread); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); @@ -76,9 +76,9 @@ TEST_F(Storage, HTTP500) { const auto mainThread = uv_thread_self(); - Request* req3 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/permanent-error" }, + std::unique_ptr<FileRequest> req3 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/permanent-error" }, [&](Response res) { - fs.cancel(req3); + req3.reset(); EXPECT_EQ(uv_thread_self(), mainThread); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); diff --git a/test/storage/http_retry_network_status.cpp b/test/storage/http_retry_network_status.cpp index 66182f5be4..08e3be89ab 100644 --- a/test/storage/http_retry_network_status.cpp +++ b/test/storage/http_retry_network_status.cpp @@ -24,8 +24,8 @@ TEST_F(Storage, HTTPNetworkStatusChange) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/delayed" }; // This request takes 200 milliseconds to answer. - Request* req = fs.request(resource, [&](Response res) { - fs.cancel(req); + std::unique_ptr<FileRequest> req = fs.request(resource, [&](Response res) { + req.reset(); EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); @@ -64,7 +64,7 @@ TEST_F(Storage, HTTPNetworkStatusChangePreempt) { const auto start = uv_hrtime(); const Resource resource{ Resource::Unknown, "http://127.0.0.1:3001/test" }; - Request* req = fs.request(resource, [&](Response res) { + std::unique_ptr<FileRequest> req = fs.request(resource, [&](Response res) { static int counter = 0; const auto duration = double(uv_hrtime() - start) / 1e9; if (counter == 0) { @@ -95,7 +95,7 @@ TEST_F(Storage, HTTPNetworkStatusChangePreempt) { EXPECT_EQ("", res.etag); if (counter++ == 1) { - fs.cancel(req); + req.reset(); loop.stop(); HTTPNetworkStatusChangePreempt.finish(); } diff --git a/test/storage/http_timeout.cpp b/test/storage/http_timeout.cpp index d59ca3cb77..2c1a1f1f60 100644 --- a/test/storage/http_timeout.cpp +++ b/test/storage/http_timeout.cpp @@ -18,7 +18,7 @@ TEST_F(Storage, HTTPTimeout) { int counter = 0; const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=1" }; - Request* req = fs.request(resource, [&](Response res) { + std::unique_ptr<FileRequest> req = fs.request(resource, [&](Response res) { counter++; EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); @@ -28,7 +28,7 @@ TEST_F(Storage, HTTPTimeout) { EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); if (counter == 4) { - fs.cancel(req); + req.reset(); loop.stop(); HTTPTimeout.finish(); } |