[core] Pass a value to response callback, not a reference
This commit is contained in:
@@ -20,7 +20,7 @@ protected:
|
||||
public:
|
||||
virtual ~FileSource() = default;
|
||||
|
||||
using Callback = std::function<void(const Response &)>;
|
||||
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.
|
||||
|
||||
@@ -20,7 +20,7 @@ class Response;
|
||||
|
||||
class Request : private util::noncopyable {
|
||||
public:
|
||||
using Callback = std::function<void(const Response &)>;
|
||||
using Callback = std::function<void (Response)>;
|
||||
Request(const Resource &resource, uv_loop_t *loop, Callback callback);
|
||||
|
||||
public:
|
||||
|
||||
@@ -105,7 +105,7 @@ void MapContext::setStyleURL(const std::string& url) {
|
||||
}
|
||||
|
||||
FileSource* fs = util::ThreadContext::getFileSource();
|
||||
styleRequest = fs->request({ Resource::Kind::Style, styleURL }, [this, base](const Response &res) {
|
||||
styleRequest = fs->request({ Resource::Kind::Style, styleURL }, [this, base](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
|
||||
@@ -30,7 +30,7 @@ void RasterTileData::request(float pixelRatio,
|
||||
state = State::loading;
|
||||
|
||||
FileSource* fs = util::ThreadContext::getFileSource();
|
||||
req = fs->request({ Resource::Kind::Tile, url }, [url, callback, this](const Response &res) {
|
||||
req = fs->request({ Resource::Kind::Tile, url }, [url, callback, this](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
|
||||
@@ -147,7 +147,7 @@ void Source::load() {
|
||||
}
|
||||
|
||||
FileSource* fs = util::ThreadContext::getFileSource();
|
||||
req = fs->request({ Resource::Kind::Source, info.url }, [this](const Response &res) {
|
||||
req = fs->request({ Resource::Kind::Source, info.url }, [this](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
|
||||
@@ -182,7 +182,7 @@ VectorTileMonitor::VectorTileMonitor(const SourceInfo& source, const TileID& id,
|
||||
}
|
||||
|
||||
Request* VectorTileMonitor::monitorTile(std::function<void (std::exception_ptr, std::unique_ptr<GeometryTile>)> callback) {
|
||||
return util::ThreadContext::getFileSource()->request({ Resource::Kind::Tile, url }, [callback, this](const Response& res) {
|
||||
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.
|
||||
return;
|
||||
|
||||
@@ -41,7 +41,7 @@ void SpriteStore::setURL(const std::string& url) {
|
||||
|
||||
FileSource* fs = util::ThreadContext::getFileSource();
|
||||
loader->jsonRequest = fs->request({ Resource::Kind::SpriteJSON, jsonURL },
|
||||
[this, jsonURL](const Response& res) {
|
||||
[this, jsonURL](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
@@ -61,7 +61,7 @@ void SpriteStore::setURL(const std::string& url) {
|
||||
|
||||
loader->spriteRequest =
|
||||
fs->request({ Resource::Kind::SpriteImage, spriteURL },
|
||||
[this, spriteURL](const Response& res) {
|
||||
[this, spriteURL](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
|
||||
@@ -74,7 +74,7 @@ GlyphPBF::GlyphPBF(GlyphStore* store,
|
||||
return "";
|
||||
});
|
||||
|
||||
auto requestCallback = [this, store, fontStack, url](const Response &res) {
|
||||
auto requestCallback = [this, store, fontStack, url](Response res) {
|
||||
if (res.stale) {
|
||||
// Only handle fresh responses.
|
||||
return;
|
||||
|
||||
@@ -21,7 +21,7 @@ TEST_F(Storage, CacheResponse) {
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
|
||||
req1 = fs.request(resource, [&](const Response &res) {
|
||||
req1 = fs.request(resource, [&](Response res) {
|
||||
fs.cancel(req1);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
@@ -34,7 +34,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, [&](const Response &res2) {
|
||||
req2 = fs.request(resource, [&](Response res2) {
|
||||
fs.cancel(req2);
|
||||
EXPECT_EQ(response.error, res2.error);
|
||||
EXPECT_EQ(response.stale, res2.stale);
|
||||
|
||||
@@ -18,7 +18,7 @@ TEST_F(Storage, CacheRevalidateSame) {
|
||||
const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" };
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
req1 = fs.request(revalidateSame, [&](const Response &res) {
|
||||
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).
|
||||
static bool first = true;
|
||||
@@ -35,7 +35,7 @@ TEST_F(Storage, CacheRevalidateSame) {
|
||||
EXPECT_EQ(0, res.modified);
|
||||
EXPECT_EQ("snowfall", res.etag);
|
||||
|
||||
req2 = fs.request(revalidateSame, [&, res](const Response &res2) {
|
||||
req2 = fs.request(revalidateSame, [&, res](Response res2) {
|
||||
if (res2.stale) {
|
||||
// Discard stale responses, if any.
|
||||
return;
|
||||
@@ -81,7 +81,7 @@ TEST_F(Storage, CacheRevalidateModified) {
|
||||
"http://127.0.0.1:3000/revalidate-modified" };
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
req1 = fs.request(revalidateModified, [&](const Response& res) {
|
||||
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).
|
||||
static bool first = true;
|
||||
@@ -98,7 +98,7 @@ TEST_F(Storage, CacheRevalidateModified) {
|
||||
EXPECT_EQ(1420070400, res.modified);
|
||||
EXPECT_EQ("", res.etag);
|
||||
|
||||
req2 = fs.request(revalidateModified, [&, res](const Response &res2) {
|
||||
req2 = fs.request(revalidateModified, [&, res](Response res2) {
|
||||
if (res2.stale) {
|
||||
// Discard stale responses, if any.
|
||||
return;
|
||||
@@ -142,7 +142,7 @@ TEST_F(Storage, CacheRevalidateEtag) {
|
||||
const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" };
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
req1 = fs.request(revalidateEtag, [&](const Response &res) {
|
||||
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).
|
||||
static bool first = true;
|
||||
@@ -159,7 +159,7 @@ TEST_F(Storage, CacheRevalidateEtag) {
|
||||
EXPECT_EQ(0, res.modified);
|
||||
EXPECT_EQ("response-1", res.etag);
|
||||
|
||||
req2 = fs.request(revalidateEtag, [&, res](const Response &res2) {
|
||||
req2 = fs.request(revalidateEtag, [&, res](Response res2) {
|
||||
if (res2.stale) {
|
||||
// Discard stale responses, if any.
|
||||
return;
|
||||
|
||||
@@ -18,8 +18,7 @@ TEST_F(Storage, AssetReadDirectory) {
|
||||
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" },
|
||||
[&](const Response &res) {
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
ASSERT_NE(nullptr, res.error);
|
||||
EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason);
|
||||
|
||||
@@ -19,8 +19,7 @@ TEST_F(Storage, AssetEmptyFile) {
|
||||
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" },
|
||||
[&](const Response &res) {
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
@@ -49,8 +48,7 @@ TEST_F(Storage, AssetNonEmptyFile) {
|
||||
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" },
|
||||
[&](const Response &res) {
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
@@ -81,8 +79,7 @@ TEST_F(Storage, AssetNonExistentFile) {
|
||||
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" },
|
||||
[&](const Response &res) {
|
||||
Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
ASSERT_NE(nullptr, res.error);
|
||||
EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason);
|
||||
|
||||
@@ -18,7 +18,7 @@ TEST_F(Storage, HTTPCancel) {
|
||||
|
||||
auto req =
|
||||
fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" },
|
||||
[&](const Response &) { ADD_FAILURE() << "Callback should not be called"; });
|
||||
[&](Response) { ADD_FAILURE() << "Callback should not be called"; });
|
||||
|
||||
fs.cancel(req);
|
||||
HTTPCancel.finish();
|
||||
@@ -36,10 +36,10 @@ TEST_F(Storage, HTTPCancelMultiple) {
|
||||
|
||||
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" };
|
||||
|
||||
auto req2 = fs.request(resource, [&](const Response &) {
|
||||
auto req2 = fs.request(resource, [&](Response) {
|
||||
ADD_FAILURE() << "Callback should not be called";
|
||||
});
|
||||
Request* req = fs.request(resource, [&](const Response &res) {
|
||||
Request* req = fs.request(resource, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
|
||||
@@ -45,7 +45,7 @@ TEST_F(Storage, HTTPCoalescing) {
|
||||
|
||||
Request* reqs[total];
|
||||
for (int i = 0; i < total; i++) {
|
||||
reqs[i] = fs.request(resource, [&complete, &fs, &reqs, i] (const Response &res) {
|
||||
reqs[i] = fs.request(resource, [&complete, &fs, &reqs, i] (Response res) {
|
||||
fs.cancel(reqs[i]);
|
||||
complete(res);
|
||||
});
|
||||
@@ -62,15 +62,10 @@ TEST_F(Storage, HTTPMultiple) {
|
||||
DefaultFileSource fs(nullptr);
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
const Response *reference = nullptr;
|
||||
|
||||
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?expires=2147483647" };
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
req1 = fs.request(resource, [&] (const Response &res) {
|
||||
EXPECT_EQ(nullptr, reference);
|
||||
reference = &res;
|
||||
|
||||
req1 = fs.request(resource, [&] (Response res) {
|
||||
// Do not cancel the request right away.
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
ASSERT_TRUE(res.data.get());
|
||||
@@ -80,9 +75,9 @@ TEST_F(Storage, HTTPMultiple) {
|
||||
EXPECT_EQ("", res.etag);
|
||||
|
||||
// Start a second request for the same resource after the first one has been completed.
|
||||
req2 = fs.request(resource, [&] (const Response &res2) {
|
||||
// Make sure we get the same object ID as before.
|
||||
EXPECT_EQ(reference, &res2);
|
||||
req2 = fs.request(resource, [&, res] (Response res2) {
|
||||
// Make sure we get the same data as before.
|
||||
EXPECT_EQ(res.data.get(), res2.data.get());
|
||||
|
||||
// Now cancel both requests after both have been notified.
|
||||
fs.cancel(req1);
|
||||
@@ -118,7 +113,7 @@ TEST_F(Storage, HTTPStale) {
|
||||
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" };
|
||||
Request* req1 = nullptr;
|
||||
Request* req2 = nullptr;
|
||||
req1 = fs.request(resource, [&] (const Response &res) {
|
||||
req1 = fs.request(resource, [&] (Response res) {
|
||||
// Do not cancel the request right away.
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
ASSERT_TRUE(res.data.get());
|
||||
@@ -136,7 +131,7 @@ TEST_F(Storage, HTTPStale) {
|
||||
updates++;
|
||||
|
||||
// Start a second request for the same resource after the first one has been completed.
|
||||
req2 = fs.request(resource, [&] (const Response &res2) {
|
||||
req2 = fs.request(resource, [&] (Response res2) {
|
||||
EXPECT_EQ(nullptr, res2.error);
|
||||
ASSERT_TRUE(res2.data.get());
|
||||
EXPECT_EQ("Hello World!", *res2.data);
|
||||
|
||||
@@ -18,8 +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" },
|
||||
[&](const Response &res) {
|
||||
Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, [&](Response res) {
|
||||
static int counter = 0;
|
||||
switch (counter++) {
|
||||
case 0: {
|
||||
@@ -66,8 +65,7 @@ TEST_F(Storage, HTTPConnectionError) {
|
||||
|
||||
const auto start = uv_hrtime();
|
||||
|
||||
Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" },
|
||||
[&](const Response &res) {
|
||||
Request* 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;
|
||||
|
||||
@@ -18,7 +18,7 @@ TEST_F(Storage, HTTPExpiresParsing) {
|
||||
|
||||
Request* req1 = fs.request({ Resource::Unknown,
|
||||
"http://127.0.0.1:3000/test?modified=1420794326&expires=1420797926&etag=foo" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req1);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
@@ -46,7 +46,7 @@ TEST_F(Storage, HTTPCacheControlParsing) {
|
||||
SystemClock::now().time_since_epoch()).count();
|
||||
|
||||
Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req2);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
|
||||
@@ -28,11 +28,11 @@ TEST_F(Storage, HTTPIssue1369) {
|
||||
|
||||
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" };
|
||||
|
||||
auto req = fs.request(resource, [&](const Response&) {
|
||||
auto req = fs.request(resource, [&](Response) {
|
||||
ADD_FAILURE() << "Callback should not be called";
|
||||
});
|
||||
fs.cancel(req);
|
||||
req = fs.request(resource, [&](const Response &res) {
|
||||
req = fs.request(resource, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
|
||||
@@ -23,7 +23,7 @@ TEST_F(Storage, HTTPLoad) {
|
||||
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](const Response &res) {
|
||||
[&, i, current](Response res) {
|
||||
fs.cancel(reqs[i]);
|
||||
reqs[i] = nullptr;
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
|
||||
@@ -15,7 +15,7 @@ TEST_F(Storage, HTTPOtherLoop) {
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
Request* req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
|
||||
@@ -19,7 +19,7 @@ TEST_F(Storage, HTTPTest) {
|
||||
const auto mainThread = uv_thread_self();
|
||||
|
||||
Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req1);
|
||||
EXPECT_EQ(uv_thread_self(), mainThread);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
@@ -47,7 +47,7 @@ TEST_F(Storage, HTTP404) {
|
||||
const auto mainThread = uv_thread_self();
|
||||
|
||||
Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/doesnotexist" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req2);
|
||||
EXPECT_EQ(uv_thread_self(), mainThread);
|
||||
ASSERT_NE(nullptr, res.error);
|
||||
@@ -77,7 +77,7 @@ TEST_F(Storage, HTTP500) {
|
||||
const auto mainThread = uv_thread_self();
|
||||
|
||||
Request* req3 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/permanent-error" },
|
||||
[&](const Response &res) {
|
||||
[&](Response res) {
|
||||
fs.cancel(req3);
|
||||
EXPECT_EQ(uv_thread_self(), mainThread);
|
||||
ASSERT_NE(nullptr, res.error);
|
||||
|
||||
@@ -24,7 +24,7 @@ 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, [&](const Response& res) {
|
||||
Request* req = fs.request(resource, [&](Response res) {
|
||||
fs.cancel(req);
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
@@ -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, [&](const Response& res) {
|
||||
Request* req = fs.request(resource, [&](Response res) {
|
||||
static int counter = 0;
|
||||
const auto duration = double(uv_hrtime() - start) / 1e9;
|
||||
if (counter == 0) {
|
||||
|
||||
@@ -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, [&](const Response &res) {
|
||||
Request* req = fs.request(resource, [&](Response res) {
|
||||
counter++;
|
||||
EXPECT_EQ(nullptr, res.error);
|
||||
EXPECT_EQ(false, res.stale);
|
||||
|
||||
Reference in New Issue
Block a user