[ios] Call snapshotter completion block with error if deallocated pri…

…or to snapshot completion

platform/darwin/src/MGLMapSnapshotter.mm

@@ -110,6 +110,8 @@ - (CLLocationCoordinate2D)coordinateForPoint:(NSPoint)point
 
 @interface MGLMapSnapshotter()
 @property (nonatomic) BOOL loading;
+@property (nonatomic) dispatch_queue_t queue;
+@property (nonatomic, copy) MGLMapSnapshotCompletionHandler completion;
 @end
 
 @implementation MGLMapSnapshotter {
@@ -118,6 +120,25 @@ @implementation MGLMapSnapshotter {
     std::unique_ptr<mbgl::Actor<mbgl::MapSnapshotter::Callback>> _snapshotCallback;
 }
 
+- (void)dealloc {
+    if (_snapshotCallback) {
+        NSAssert(_loading, @"Snapshot in progress - `loading` should = YES");
+
+        MGLMapSnapshotCompletionHandler completion = _completion;
+
+        // The snapshot hasn't completed, so we should alert the caller
+        if (completion && _queue) {
+            dispatch_async(_queue, ^{
+                NSDictionary *userInfo = @{NSLocalizedDescriptionKey: @"MGLMapSnapshotter deallocated prior to snapshot completion."};
+                NSError *error = [NSError errorWithDomain:MGLErrorDomain
+                                                     code:MGLErrorCodeSnapshotFailed
+                                                 userInfo:userInfo];
+                completion(NULL, error);
+            });
+        }
+    }
+}
+
 - (instancetype)initWithOptions:(MGLMapSnapshotOptions *)options
 {
     self = [super init];
@@ -147,7 +168,11 @@ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshot
     }
 
     self.loading = true;
-
+
+    self.completion = completion;
+    self.queue = queue;
+
+
     __weak __typeof__(self) weakSelf = self;
     // mbgl::Scheduler::GetCurrent() scheduler means "run callback on current (ie UI/main) thread"
     // capture weakSelf to avoid retain cycle if callback is never called (ie snapshot cancelled)
@@ -162,14 +187,17 @@ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshot
 
         strongSelf.loading = false;
 
+        MGLMapSnapshotCompletionHandler callback = strongSelf.completion;
+
         if (mbglError) {
             NSString *description = @(mbgl::util::toString(mbglError).c_str());
             NSDictionary *userInfo = @{NSLocalizedDescriptionKey: description};
             NSError *error = [NSError errorWithDomain:MGLErrorDomain code:MGLErrorCodeSnapshotFailed userInfo:userInfo];
 
             // Dispatch result to origin queue
             dispatch_async(queue, ^{
-                completion(nil, error);
+                strongSelf.completion(nil, error);
+                strongSelf.completion = nil;
             });
         } else {
 #if TARGET_OS_IPHONE
@@ -179,9 +207,10 @@ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshot
             mglImage.size = NSMakeSize(mglImage.size.width / strongSelf.options.scale,
                                        mglImage.size.height / strongSelf.options.scale);
 #endif
-            [strongSelf drawAttributedSnapshot:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn queue:queue completionHandler:completion];
+            [strongSelf drawAttributedSnapshot:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn];
         }
         strongSelf->_snapshotCallback = NULL;
+
     });
 
     // Launches snapshot on background Thread owned by mbglMapSnapshotter
@@ -190,7 +219,7 @@ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshot
     _mbglMapSnapshotter->snapshot(_snapshotCallback->self());
 }
 
-+ (void)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn queue:(dispatch_queue_t)queue scale:(CGFloat)scale size:(CGSize)size completionHandler:(MGLMapSnapshotCompletionHandler)completion {
++ (UIImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn scale:(CGFloat)scale size:(CGSize)size {
 
     NSArray<MGLAttributionInfo *>* attributionInfo = [MGLMapSnapshotter generateAttributionInfos:attributions];
 
@@ -251,6 +280,9 @@ + (void)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)attribu
     UIImage *compositedImage = UIGraphicsGetImageFromCurrentImageContext();
 
     UIGraphicsEndImageContext();
+
+    return compositedImage;
+
 #else
     NSSize targetSize = NSMakeSize(size.width, size.height);
     NSRect targetFrame = NSMakeRect(0, 0, targetSize.width, targetSize.height);
@@ -310,29 +342,42 @@ + (void)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)attribu
     [MGLMapSnapshotter drawAttributionTextWithStyle:attributionInfoStyle origin:attributionTextPosition attributionInfo:attributionInfo];
 
     [compositedImage unlockFocus];
-
+
+    return compositedImage;
+
 #endif
-    // Dispatch result to origin queue
-    dispatch_async(queue, ^{
-        MGLMapSnapshot* snapshot = [[MGLMapSnapshot alloc] initWithImage:compositedImage
-                                                                   scale:scale
-                                                              pointForFn:pointForFn
-                                                             latLngForFn:latLngForFn];
-        completion(snapshot, nil);
-    });
 }
 
-- (void)drawAttributedSnapshot:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn queue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshotCompletionHandler)completion {
+- (void)drawAttributedSnapshot:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn {
 
     // Process image watermark in a work queue
     dispatch_queue_t workQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+    dispatch_queue_t originQueue = self.queue;
+
     // Capture scale and size by value to avoid accessing self from another thread
     CGFloat scale = self.options.scale;
     CGSize size = self.options.size;
+
     // pointForFn is a copyable std::function that captures state by value: see MapSnapshotter::Impl::snapshot
+    __weak __typeof__(self) weakself = self;
+
     dispatch_async(workQueue, ^{
         // Call a class method to ensure we're not accidentally capturing self
-        [MGLMapSnapshotter drawAttributedSnapshotWorker:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn queue:queue scale:scale size:size completionHandler:completion];
+        UIImage *compositedImage = [MGLMapSnapshotter drawAttributedSnapshotWorker:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn scale:scale size:size];
+
+        // Dispatch result to origin queue
+        dispatch_async(originQueue, ^{
+            __typeof__(self) strongself = weakself;
+
+            if (strongself.completion) {
+                MGLMapSnapshot* snapshot = [[MGLMapSnapshot alloc] initWithImage:compositedImage
+                                                                           scale:scale
+                                                                      pointForFn:pointForFn
+                                                                     latLngForFn:latLngForFn];
+                strongself.completion(snapshot, nil);
+                strongself.completion = nil;
+            }
+        });
     });
 }
 

platform/ios/Integration Tests/MGLMapViewIntegrationTest.h

@@ -28,6 +28,7 @@
 @property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated);
 
 // Utility methods
+- (NSString*)validAccessToken;
 - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout;
 - (void)waitForMapViewToBeRenderedWithTimeout:(NSTimeInterval)timeout;
 @end

platform/ios/Integration Tests/MGLMapViewIntegrationTest.m

@@ -6,6 +6,17 @@ - (void)updateFromDisplayLink;
 
 @implementation MGLMapViewIntegrationTest
 
+- (NSString*)validAccessToken {
+    NSString *accessToken = [[NSProcessInfo processInfo] environment][@"MAPBOX_ACCESS_TOKEN"];
+    if (!accessToken) {
+        printf("warning: MAPBOX_ACCESS_TOKEN env var is required for this test - skipping.\n");
+        return nil;
+    }
+
+    [MGLAccountManager setAccessToken:accessToken];
+    return accessToken;
+}
+
 - (void)setUp {
     [super setUp];
 

platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterSwiftTests.swift

@@ -0,0 +1,62 @@
+import XCTest
+
+class MGLMapSnapshotterSwiftTests: MGLMapViewIntegrationTest {
+
+    // Create snapshot options
+    private class func snapshotterOptions(size: CGSize) -> MGLMapSnapshotOptions {
+        let camera = MGLMapCamera()
+
+        let options = MGLMapSnapshotOptions(styleURL: MGLStyle.satelliteStreetsStyleURL, camera: camera, size: size)
+
+        let sw = CLLocationCoordinate2D(latitude: 52.3, longitude: 13.0)
+        let ne = CLLocationCoordinate2D(latitude: 52.5, longitude: 13.2)
+        options.coordinateBounds = MGLCoordinateBounds(sw:sw, ne:ne)
+
+        return options
+    }
+
+    func testCapturingSnapshotterInSnapshotCompletion() {
+        // See the Obj-C testDeallocatingSnapshotterDuringSnapshot
+        // This Swift test, is essentially the same except for capturing the snapshotter
+        guard validAccessToken() != nil else {
+            return
+        }
+
+        let timeout: TimeInterval = 5.0
+        let expectation = self.expectation(description: "snapshot")
+
+        let options = MGLMapSnapshotterSwiftTests.snapshotterOptions(size: mapView.bounds.size)
+
+        let backgroundQueue = DispatchQueue.main
+
+        backgroundQueue.async {
+            let dg = DispatchGroup()
+            dg.enter()
+
+            DispatchQueue.main.async {
+
+                let snapshotter = MGLMapSnapshotter(options: options)
+
+                snapshotter.start(completionHandler: { (snapshot, error) in
+
+//                    // Without capturing snapshotter:
+//                    XCTAssertNil(snapshot)
+//                    XCTAssertNotNil(error)
+
+                    // Capture snapshotter
+                    dump(snapshotter)
+                    XCTAssertNotNil(snapshot)
+                    XCTAssertNil(error)
+
+                    dg.leave()
+                })
+            }
+
+            dg.notify(queue: .main) {
+                expectation.fulfill()
+            }
+        }
+
+        wait(for: [expectation], timeout: timeout)
+    }
+}