clean up os x + ios request code

fixes #129
mapbox · Apr 14, 2014 · 305e688 · 305e688 · incanus · Apr 21, 2014
1 parent b4ad515
commit 305e688
Show file tree

Hide file tree

Showing 12 changed files with 112 additions and 157 deletions.
diff --git a/common/foundation_request.mm b/common/foundation_request.mm
@@ -0,0 +1,63 @@
+#import <Foundation/Foundation.h>
+#include <memory>
+#include <string>
+#include <functional>
+#include <llmr/platform/platform.hpp>
+
+
+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>
+platform::request_http(const std::string &url, std::function<void(Response *)> background_function,
+                       std::function<void()> foreground_callback) {
+    __block std::shared_ptr<Request> 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<Request>(task, url);
+    [task resume];
+    return req;
+}
+
+void platform::cancel_request_http(const std::shared_ptr<Request> &req) {
+    if (req) {
+        [req->task cancel];
+    }
+}
+
+}
diff --git a/include/llmr/map/tile_data.hpp b/include/llmr/map/tile_data.hpp
@@ -95,7 +95,9 @@ class TileData : public std::enable_shared_from_this<TileData>,
     std::string data;
     const Style& style;
     GlyphAtlas& glyphAtlas;
-    platform::Request *req = nullptr;
+
+    // Stores a request that is in progress.
+    std::weak_ptr<platform::Request> req;
 };
 
 }

diff --git 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<void(Response&)> background_function, std::function<void()> 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> request_http(const std::string &url,
+                                      std::function<void(Response *)> background_function,
+                                      std::function<void()> foreground_callback);
 
 // Cancels an HTTP request.
-void cancel_request_http(Request *request);
+void cancel_request_http(const std::shared_ptr<Request> &req);
 
 // Returns a relative timestamp in seconds. This value must be monotonic.
 double time();

diff --git a/ios/MBXViewController.mm b/ios/MBXViewController.mm
@@ -470,70 +470,6 @@ void restart()
             [[NSNotificationCenter defaultCenter] postNotificationName:MBXNeedsRenderNotification object:nil];
         }
 
-        Request *request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> 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
@@ -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
@@ -11,7 +11,7 @@
 
 
 MapView *mapView = nullptr;
-std::forward_list<llmr::platform::Request *> requests;
+std::forward_list<std::shared_ptr<llmr::platform::Request>> 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<llmr::platform::Request> &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<void(Response&)> background_function, std::function<void()> foreground_callback) {
-    Request *req = new Request(url, background_function, foreground_callback);
+std::shared_ptr<platform::Request>
+platform::request_http(const std::string &url, std::function<void(Response *)> background_function,
+                       std::function<void()> foreground_callback) {
+    std::shared_ptr<Request> req =
+        std::make_shared<Request>(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<Request> &req) {
+    if (req) {
+        req->cancel();
     }
 }
 
-}
+
 }
diff --git 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<void(platform::Response&)> bg, std::function<void()> fg)
+Request::Request(std::string url, std::function<void(platform::Response *)> bg, std::function<void()> 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
@@ -14,7 +14,7 @@ struct Response;
 
 class Request {
 public:
-    Request(std::string url, std::function<void(platform::Response&)> bg, std::function<void()> fg);
+    Request(std::string url, std::function<void(platform::Response *)> bg, std::function<void()> fg);
 
     static void initialize();
     static void finish();
@@ -36,7 +36,7 @@ class Request {
     std::atomic<bool> done;
     std::atomic<bool> cancelled;
     const std::string url;
-    const std::function<void(platform::Response&)> background_function;
+    const std::function<void(platform::Response *)> background_function;
     const std::function<void()> foreground_callback;
 
 private:

diff --git 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
@@ -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<void(Response&)> background_function, std::function<void()> 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];
-    }];
-}
-
-}
 }