From 305e688a3a1d255fe34269cc327284275a0a3044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20Ka=CC=88fer?= Date: Mon, 14 Apr 2014 11:47:57 -0400 Subject: [PATCH] clean up os x + ios request code fixes #129 --- common/foundation_request.mm | 63 +++++++++++++++++++++++++++++ include/llmr/map/tile_data.hpp | 4 +- include/llmr/platform/platform.hpp | 14 +++++-- ios/MBXViewController.mm | 64 ------------------------------ ios/llmr-app.gyp | 3 +- linux/main.cpp | 28 ++++++------- linux/request.cpp | 4 +- linux/request.hpp | 4 +- macosx/llmr-app.gyp | 3 +- macosx/main.mm | 59 +-------------------------- src/map/tile_data.cpp | 11 +++-- src/style/sprite.cpp | 12 +++--- 12 files changed, 112 insertions(+), 157 deletions(-) create mode 100644 common/foundation_request.mm diff --git a/common/foundation_request.mm b/common/foundation_request.mm new file mode 100644 index 000000000..d7ab26cef --- /dev/null +++ b/common/foundation_request.mm @@ -0,0 +1,63 @@ +#import +#include +#include +#include +#include + + +namespace llmr { + +class platform::Request { +public: + Request(NSURLSessionDataTask *task, const std::string &original_url) + : task(task), original_url(original_url) {} + NSURLSessionDataTask *task; + std::string original_url; +}; + +std::shared_ptr +platform::request_http(const std::string &url, std::function background_function, + std::function foreground_callback) { + __block std::shared_ptr req; + NSURLSessionDataTask *task = [[NSURLSession sharedSession] + dataTaskWithURL:[NSURL URLWithString:@(url.c_str())] + completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) { + // Make sure we clear the shared_ptr to resolve the circular reference. We're referencing + // this shared_ptr by value so that the object stays around until this completion handler is + // invoked. + req.reset(); + + if ([error code] == NSURLErrorCancelled) { + // We intentionally cancelled this request. Do nothing. + return; + } + + Response res; + + if (!error && [response isKindOfClass:[NSHTTPURLResponse class]]) { + res.code = [(NSHTTPURLResponse *)response statusCode]; + res.body = {(const char *)[data bytes], [data length]}; + } else { + res.code = -1; + res.error_message = [[error localizedDescription] UTF8String]; + } + + background_function(&res); + + dispatch_async(dispatch_get_main_queue(), ^(void) { + foreground_callback(); + }); + }]; + + req = std::make_shared(task, url); + [task resume]; + return req; +} + +void platform::cancel_request_http(const std::shared_ptr &req) { + if (req) { + [req->task cancel]; + } +} + +} diff --git a/include/llmr/map/tile_data.hpp b/include/llmr/map/tile_data.hpp index b0312b5e3..955c1ffbc 100644 --- a/include/llmr/map/tile_data.hpp +++ b/include/llmr/map/tile_data.hpp @@ -95,7 +95,9 @@ private: std::string data; const Style& style; GlyphAtlas& glyphAtlas; - platform::Request *req = nullptr; + + // Stores a request that is in progress. + std::weak_ptr req; }; } diff --git a/include/llmr/platform/platform.hpp b/include/llmr/platform/platform.hpp index 1724ebd35..ef5feb8ba 100644 --- a/include/llmr/platform/platform.hpp +++ b/include/llmr/platform/platform.hpp @@ -20,16 +20,24 @@ void restart(); class Request; + struct Response { + int16_t code = -1; std::string body; + + std::string error_message; }; -// Makes an HTTP request of a URL on a background thread, calls a function with the results on the same thread, and finally calls a callback function on the main thread. Returns a cancellable request. -Request *request_http(std::string url, std::function background_function, std::function foreground_callback); +// Makes an HTTP request of a URL on a background thread, calls a function with the results in +// another thread, then calls the foreground callback in the original main thread. +// Returns a cancellable request. +std::shared_ptr request_http(const std::string &url, + std::function background_function, + std::function foreground_callback); // Cancels an HTTP request. -void cancel_request_http(Request *request); +void cancel_request_http(const std::shared_ptr &req); // Returns a relative timestamp in seconds. This value must be monotonic. double time(); diff --git a/ios/MBXViewController.mm b/ios/MBXViewController.mm index 3e52db752..b25d55bdf 100644 --- a/ios/MBXViewController.mm +++ b/ios/MBXViewController.mm @@ -470,70 +470,6 @@ namespace llmr [[NSNotificationCenter defaultCenter] postNotificationName:MBXNeedsRenderNotification object:nil]; } - Request *request_http(std::string url, std::function background_function, std::function foreground_callback) - { - if (!session) { - session = [[NSURLSession alloc] init]; - } - NSURLSessionDataTask *task = [session dataTaskWithURL:[NSURL URLWithString:@(url.c_str())] completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) - { - dispatch_async(queue, ^(){ - [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil]; - if ([error code] == NSURLErrorCancelled) { - // We intentionally cancelled this request. Do nothing. - return; - } - - Response res; - - if ( ! error && [response isKindOfClass:[NSHTTPURLResponse class]]) - { - res.code = [(NSHTTPURLResponse *)response statusCode]; - res.body = { (const char *)[data bytes], [data length] }; - } else { - NSLog(@"http error (%s): %@", url.c_str(), [error localizedDescription]); - } - - background_function(res); - - dispatch_async(dispatch_get_main_queue(), ^(void) - { - foreground_callback(); - }); - - }); - }]; - - [task resume]; - - [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil]; - - Request *req = new Request(); - - req->identifier = task.taskIdentifier; - req->original_url = url; - - return req; - } - - void cancel_request_http(Request *request) - { - [session getTasksWithCompletionHandler:^(NSArray *dataTasks, NSArray *uploadTasks, NSArray *downloadTasks) - { - for (NSURLSessionDownloadTask *task in downloadTasks) - { - if (task.taskIdentifier == request->identifier) - { - [task cancel]; - - [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil]; - - return; - } - } - }]; - } - double time() { return [displayLink timestamp]; diff --git a/ios/llmr-app.gyp b/ios/llmr-app.gyp index b7000d555..288e2bcac 100644 --- a/ios/llmr-app.gyp +++ b/ios/llmr-app.gyp @@ -12,7 +12,8 @@ "./main.m", "./MBXAppDelegate.m", "./MBXSettings.mm", - "./MBXViewController.mm" + "./MBXViewController.mm", + "../common/foundation_request.mm", ], 'product_extension': 'app', 'mac_bundle': 1, diff --git a/linux/main.cpp b/linux/main.cpp index 51d31bd0b..bb32029bb 100644 --- a/linux/main.cpp +++ b/linux/main.cpp @@ -11,7 +11,7 @@ MapView *mapView = nullptr; -std::forward_list requests; +std::forward_list> requests; @@ -74,14 +74,12 @@ int main(int argc, char *argv[]) { } namespace llmr { -namespace platform { -void cleanup() { +void platform::cleanup() { bool& dirty = mapView->dirty; - requests.remove_if([&dirty](llmr::platform::Request * req) { + requests.remove_if([&dirty](std::shared_ptr &req) { if (req->done) { req->foreground_callback(); - delete req; dirty = true; return true; } else { @@ -90,25 +88,27 @@ void cleanup() { }); } -void restart() { +void platform::restart() { if (mapView) { mapView->dirty = true; } } -Request *request_http(std::string url, std::function background_function, std::function foreground_callback) { - Request *req = new Request(url, background_function, foreground_callback); +std::shared_ptr +platform::request_http(const std::string &url, std::function background_function, + std::function foreground_callback) { + std::shared_ptr req = + std::make_shared(url, background_function, foreground_callback); requests.push_front(req); return req; } -void cancel_request_http(Request *request) { - for (Request *req : requests) { - if (req == request) { - req->cancel(); - } +// Cancels an HTTP request. +void platform::cancel_request_http(const std::shared_ptr &req) { + if (req) { + req->cancel(); } } -} + } diff --git a/linux/request.cpp b/linux/request.cpp index 651dcd2d1..558195760 100644 --- a/linux/request.cpp +++ b/linux/request.cpp @@ -44,7 +44,7 @@ void Request::finish() { curl_share_cleanup(curl_share); } -Request::Request(std::string url, std::function bg, std::function fg) +Request::Request(std::string url, std::function bg, std::function fg) : done(false), cancelled(false), url(url), @@ -102,7 +102,7 @@ void Request::request(void *ptr) { curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &res.code); if (code != CURLE_ABORTED_BY_CALLBACK) { - req->background_function(res); + req->background_function(&res); } req->done = true; diff --git a/linux/request.hpp b/linux/request.hpp index 78348d4bf..8350d9d45 100644 --- a/linux/request.hpp +++ b/linux/request.hpp @@ -14,7 +14,7 @@ struct Response; class Request { public: - Request(std::string url, std::function bg, std::function fg); + Request(std::string url, std::function bg, std::function fg); static void initialize(); static void finish(); @@ -36,7 +36,7 @@ public: std::atomic done; std::atomic cancelled; const std::string url; - const std::function background_function; + const std::function background_function; const std::function foreground_callback; private: diff --git a/macosx/llmr-app.gyp b/macosx/llmr-app.gyp index dd6f29c5b..3c1bfedf7 100644 --- a/macosx/llmr-app.gyp +++ b/macosx/llmr-app.gyp @@ -11,7 +11,8 @@ 'sources': [ './main.mm', './settings.mm', - './settings.hpp' + './settings.hpp', + '../common/foundation_request.mm' ], 'product_extension': 'app', 'mac_bundle': 1, diff --git a/macosx/main.mm b/macosx/main.mm index 269d2bc6e..6a40f22e1 100644 --- a/macosx/main.mm +++ b/macosx/main.mm @@ -23,13 +23,12 @@ int main() { namespace llmr { -namespace platform { -void cleanup() { +void platform::cleanup() { // noop } -void restart() { +void platform::restart() { if (mapView) { mapView->dirty = true; CGEventRef event = CGEventCreate(NULL); @@ -38,58 +37,4 @@ void restart() { } } -class Request { -public: - int16_t identifier; - std::string original_url; -}; - -Request *request_http(std::string url, std::function background_function, std::function foreground_callback) -{ - NSURLSessionDataTask *task = [[NSURLSession sharedSession] dataTaskWithURL:[NSURL URLWithString:@(url.c_str())] completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) - { - if ([error code] == NSURLErrorCancelled) { - // We intentionally cancelled this request. Do nothing. - return; - } - - Response res; - - if ( ! error && [response isKindOfClass:[NSHTTPURLResponse class]]) - { - res.code = [(NSHTTPURLResponse *)response statusCode]; - res.body = { (const char *)[data bytes], [data length] }; - } else { - NSLog(@"http error (%s): %@", url.c_str(), [error localizedDescription]); - } - - background_function(res); - - dispatch_async(dispatch_get_main_queue(), ^(void) - { - foreground_callback(); - }); - }]; - - [task resume]; - - Request *req = new Request(); - - req->identifier = task.taskIdentifier; - req->original_url = url; - - return req; -} - -void cancel_request_http(Request *request) -{ - [[NSURLSession sharedSession] getTasksWithCompletionHandler:^(NSArray *dataTasks, NSArray *uploadTasks, NSArray *downloadTasks) - { - for (NSURLSessionDownloadTask *task in downloadTasks) - if (task.taskIdentifier == request->identifier) - return [task cancel]; - }]; -} - -} } diff --git a/src/map/tile_data.cpp b/src/map/tile_data.cpp index aa9fd06f5..e35fc0645 100644 --- a/src/map/tile_data.cpp +++ b/src/map/tile_data.cpp @@ -48,28 +48,27 @@ void TileData::request() { // Note: Somehow this feels slower than the change to request_http() std::weak_ptr weak_tile = shared_from_this(); - platform::Request *request = platform::request_http(url, [=](platform::Response& res) { + req = platform::request_http(url, [weak_tile, url](platform::Response *res) { std::shared_ptr tile = weak_tile.lock(); if (!tile || tile->state == State::obsolete) { // noop. Tile is obsolete and we're now just waiting for the refcount // to drop to zero for destruction. - } else if (res.code == 200) { + } else if (res->code == 200) { tile->state = State::loaded; - tile->data.swap(res.body); + tile->data.swap(res->body); tile->parse(); } else { - fprintf(stderr, "tile loading failed (%s): %d\n", url.c_str(), res.code); + fprintf(stderr, "tile loading failed (%s): %d\n", url.c_str(), res->code); } }, []() { platform::restart(); }); - req = request; } void TileData::cancel() { if (state != State::obsolete) { state = State::obsolete; - platform::cancel_request_http(req); + platform::cancel_request_http(req.lock()); } } diff --git a/src/style/sprite.cpp b/src/style/sprite.cpp index 9b0c09167..e7d48e5b7 100644 --- a/src/style/sprite.cpp +++ b/src/style/sprite.cpp @@ -36,17 +36,17 @@ void Sprite::load(const std::string& base_url) { std::string suffix = (pixelRatio > 1 ? "@2x" : ""); - platform::request_http(base_url + suffix + ".json", [sprite](const platform::Response & res) { - if (res.code == 200) { - sprite->parseJSON(res.body); + platform::request_http(base_url + suffix + ".json", [sprite](const platform::Response *res) { + if (res->code == 200) { + sprite->parseJSON(res->body); } else { fprintf(stderr, "failed to load sprite\n"); } }, complete); - platform::request_http(base_url + suffix + ".png", [sprite](const platform::Response & res) { - if (res.code == 200) { - sprite->raster->load(res.body); + platform::request_http(base_url + suffix + ".png", [sprite](const platform::Response *res) { + if (res->code == 200) { + sprite->raster->load(res->body); } else { fprintf(stderr, "failed to load sprite image\n"); }