[ios, macos] Call snapshotter (Obj-C) completion block on dealloc/cancel if snapshot hasn't finished. (#12355)

10 files changed, 332 insertions, 64 deletions

diff --git a/platform/darwin/src/MGLMapSnapshotter.mm b/platform/darwin/src/MGLMapSnapshotter.mm
index d9d16cc5be..aa1a988b1b 100644
--- a/platform/darwin/src/MGLMapSnapshotter.mm
+++ b/platform/darwin/src/MGLMapSnapshotter.mm
@@ -109,7 +109,10 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
 @end
 
 @interface MGLMapSnapshotter()
-@property (nonatomic) BOOL loading;
+@property (nonatomic) BOOL cancelled;
+@property (nonatomic) dispatch_queue_t resultQueue;
+@property (nonatomic, copy) MGLMapSnapshotCompletionHandler completion;
++ (void)completeWithErrorCode:(MGLErrorCode)errorCode description:(nonnull NSString*)description onQueue:(dispatch_queue_t)queue completion:(MGLMapSnapshotCompletionHandler)completion;
 @end
 
 @implementation MGLMapSnapshotter {
@@ -118,13 +121,22 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
     std::unique_ptr<mbgl::Actor<mbgl::MapSnapshotter::Callback>> _snapshotCallback;
 }
 
+- (void)dealloc {
+    if (_completion) {
+        NSAssert(_snapshotCallback, @"Snapshot in progress - there should be a valid callback");
+
+        [MGLMapSnapshotter completeWithErrorCode:MGLErrorCodeSnapshotFailed
+                                     description:@"MGLMapSnapshotter deallocated prior to snapshot completion."
+                                         onQueue:_resultQueue
+                                      completion:_completion];
+    }
+}
+
 - (instancetype)initWithOptions:(MGLMapSnapshotOptions *)options
 {
     self = [super init];
     if (self) {
         [self setOptions:options];
-        _loading = false;
-
     }
     return self;
 }
@@ -141,13 +153,16 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
                     format:@"startWithQueue:completionHandler: must be called from a thread with an active run loop."];
     }
 
-    if ([self isLoading]) {
+    if (self.completion) {
+        // Consider replacing this exception with an error passed to the completion block.
         [NSException raise:NSInternalInconsistencyException
                     format:@"Already started this snapshotter."];
     }
     
-    self.loading = true;
-    
+    self.completion = completion;
+    self.resultQueue = queue;
+    self.cancelled = NO;
+
     __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)
@@ -160,16 +175,18 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
         // If self had died, _snapshotCallback would have been destroyed and this block would not be executed
         NSCAssert(strongSelf, @"Snapshot callback executed after being destroyed.");
 
-        strongSelf.loading = false;
+        if (!strongSelf.completion)
+            return;
 
         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 to result queue
             dispatch_async(queue, ^{
-                completion(nil, error);
+                strongSelf.completion(nil, error);
+                strongSelf.completion = nil;
             });
         } else {
 #if TARGET_OS_IPHONE
@@ -179,9 +196,10 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
             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,10 +208,10 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
     _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 {
-    
++ (MGLImage*)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];
-    
+
 #if TARGET_OS_IPHONE
     MGLAttributionInfoStyle attributionInfoStyle = MGLAttributionInfoStyleLong;
     for (NSUInteger styleValue = MGLAttributionInfoStyleLong; styleValue >= MGLAttributionInfoStyleShort; styleValue--) {
@@ -251,7 +269,11 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
     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 +332,41 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
     [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 resultQueue = self.resultQueue;
+
     // 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];
+        MGLImage *compositedImage = [MGLMapSnapshotter drawAttributedSnapshotWorker:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn scale:scale size:size];
+
+        // Dispatch result to origin queue
+        dispatch_async(resultQueue, ^{
+            __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;
+            }
+        });
     });
 }
 
@@ -466,10 +500,33 @@ const CGFloat MGLSnapshotterMinimumPixelSize = 64;
 
 - (void)cancel
 {
+    self.cancelled = YES;
+    
+    if (_snapshotCallback) {
+        [MGLMapSnapshotter completeWithErrorCode:MGLErrorCodeSnapshotFailed
+                                     description:[NSString stringWithFormat:@"MGLMapSnapshotter cancelled from %s", __PRETTY_FUNCTION__]
+                                         onQueue:self.resultQueue
+                                      completion:self.completion];
+        self.completion = nil;
+    }
+
     _snapshotCallback.reset();
     _mbglMapSnapshotter.reset();
 }
 
++ (void)completeWithErrorCode:(MGLErrorCode)errorCode description:(nonnull NSString*)description onQueue:(dispatch_queue_t)queue completion:(MGLMapSnapshotCompletionHandler)completion {
+    // The snapshot hasn't completed, so we should alert the caller
+    if (completion && queue) {
+        dispatch_async(queue, ^{
+            NSDictionary *userInfo = @{NSLocalizedDescriptionKey: description};
+            NSError *error = [NSError errorWithDomain:MGLErrorDomain
+                                                 code:errorCode
+                                             userInfo:userInfo];
+            completion(NULL, error);
+        });
+    }
+}
+
 - (void)setOptions:(MGLMapSnapshotOptions *)options
 {
     _options = options;
diff --git a/platform/darwin/src/MGLTypes.h b/platform/darwin/src/MGLTypes.h
index 1c90d7968b..035c6f6593 100644
--- a/platform/darwin/src/MGLTypes.h
+++ b/platform/darwin/src/MGLTypes.h
@@ -49,7 +49,7 @@ typedef NS_ENUM(NSInteger, MGLErrorCode) {
     /** An attempt to load the style failed. */
     MGLErrorCodeLoadStyleFailed = 5,
     /** An error occurred while snapshotting the map. */
-    MGLErrorCodeSnapshotFailed = 6,
+    MGLErrorCodeSnapshotFailed = 6
 };
 
 /** Options for enabling debugging features in an `MGLMapView` instance. */
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index 8c0875e1b0..bfa72d8ecf 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+### Other changes
+
+* Fixed bug where completion block passed to `-[MGLMapSnapshotter startWithQueue:completionHandler:]` was not being called in all code paths. ([#12355](https://github.com/mapbox/mapbox-gl-native/pull/12355))
+
 ## 4.4.0
 
 ### Styles and rendering
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
index 4884f30b77..f513df8b20 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
+++ b/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
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
index f67150ba9b..68023f3556 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
@@ -6,6 +6,17 @@
 
 @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];
 
diff --git a/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterSwiftTests.swift b/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterSwiftTests.swift
new file mode 100644
index 0000000000..bbdfce7531
--- /dev/null
+++ b/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)
+    }
+}
diff --git a/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m b/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m
index 22f7fc5911..fd5b3db978 100644
--- a/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m
+++ b/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m
@@ -1,5 +1,10 @@
 #import "MGLMapViewIntegrationTest.h"
 
+@interface MGLMapSnapshotter ()
+@property (nonatomic) BOOL cancelled;
+@end
+
+
 @interface MGLMapSnapshotterTest : MGLMapViewIntegrationTest
 @end
 
@@ -19,21 +24,10 @@ MGLMapSnapshotter* snapshotterWithCoordinates(CLLocationCoordinate2D coordinates
     return snapshotter;
 }
 
-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;
-}
-
 @implementation MGLMapSnapshotterTest
 
 - (void)testMultipleSnapshotsWithASingleSnapshotter {
-    if (!validAccessToken()) {
+    if (![self validAccessToken]) {
         return;
     }
 
@@ -66,8 +60,137 @@ NSString* validAccessToken() {
     [self waitForExpectations:@[expectation] timeout:10.0];
 }
 
+- (void)testDeallocatingSnapshotterDuringSnapshot {
+    // See also https://github.com/mapbox/mapbox-gl-native/issues/12336
+    if (![self validAccessToken]) {
+        return;
+    }
+
+    NSTimeInterval timeout         = 5.0;
+    XCTestExpectation *expectation = [self expectationWithDescription:@"snapshot"];
+    CGSize size                    = self.mapView.bounds.size;
+    CLLocationCoordinate2D coord   = CLLocationCoordinate2DMake(30.0, 30.0);
+
+    // Test triggering to main queue
+    dispatch_queue_t backgroundQueue = dispatch_get_main_queue();
+//  dispatch_queue_t backgroundQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+
+    __weak __typeof__(self) weakself = self;
+
+    dispatch_async(backgroundQueue, ^{
+
+        dispatch_group_t dg = dispatch_group_create();
+        dispatch_group_enter(dg);
+
+        dispatch_async(dispatch_get_main_queue(), ^{
+            MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
+            __weak MGLMapSnapshotter *weakSnapshotter = snapshotter;
+            
+            [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
+                // We expect this completion block to be called with an error
+                __typeof__(self) strongself = weakself;
+
+                MGLTestAssertNil(strongself, snapshot);
+                MGLTestAssert(strongself,
+                              ([error.domain isEqualToString:MGLErrorDomain] && error.code == MGLErrorCodeSnapshotFailed),
+                              @"Should have errored");
+                MGLTestAssertNil(strongself, weakSnapshotter, @"Snapshotter should have been deallocated");
+
+                dispatch_group_leave(dg);
+            }];
+        });
+
+        dispatch_group_notify(dg, dispatch_get_main_queue(), ^{
+            [expectation fulfill];
+        });
+    });
+
+    [self waitForExpectations:@[expectation] timeout:timeout];
+}
+
+- (void)testSnapshotterUsingNestedDispatchQueues {
+    // This is the opposite pair to the above test `testDeallocatingSnapshotterDuringSnapshot`
+    // The only significant difference is that the snapshotter is a `__block` variable, so
+    // its lifetime should continue until it's set to nil in the completion block.
+    // See also https://github.com/mapbox/mapbox-gl-native/issues/12336
+
+    if (![self validAccessToken]) {
+        return;
+    }
+
+    NSTimeInterval timeout         = 5.0;
+    XCTestExpectation *expectation = [self expectationWithDescription:@"snapshot"];
+    CGSize size                    = self.mapView.bounds.size;
+    CLLocationCoordinate2D coord   = CLLocationCoordinate2DMake(30.0, 30.0);
+
+    // Test triggering to main queue
+    dispatch_queue_t backgroundQueue = dispatch_get_main_queue();
+    //  dispatch_queue_t backgroundQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+
+    __weak __typeof__(self) weakself = self;
+
+    dispatch_async(backgroundQueue, ^{
+
+        dispatch_group_t dg = dispatch_group_create();
+        dispatch_group_enter(dg);
+
+        dispatch_async(dispatch_get_main_queue(), ^{
+
+            __block MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
+
+            [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
+                // We expect this completion block to be called with an error
+                __typeof__(self) strongself = weakself;
+                MGLTestAssertNotNil(strongself, snapshot);
+                MGLTestAssertNil(strongself, error, @"Snapshotter should have completed");
+                dispatch_group_leave(dg);
+                snapshotter = nil;
+            }];
+        });
+
+        dispatch_group_notify(dg, dispatch_get_main_queue(), ^{
+            [expectation fulfill];
+        });
+    });
+
+    [self waitForExpectations:@[expectation] timeout:timeout];
+}
+
+- (void)testCancellingSnapshot {
+    if (![self validAccessToken]) {
+        return;
+    }
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"snapshots"];
+    expectation.assertForOverFulfill = YES;
+    expectation.expectedFulfillmentCount = 1;
+
+    CGSize size                    = self.mapView.bounds.size;
+    CLLocationCoordinate2D coord   = CLLocationCoordinate2DMake(30.0, 30.0);
+
+    MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
+
+    __weak __typeof__(self) weakself = self;
+
+    [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
+        // We expect this completion block to be called with an error
+        __typeof__(self) strongself = weakself;
+
+        MGLTestAssertNil(strongself, snapshot);
+        MGLTestAssert(strongself,
+                      ([error.domain isEqualToString:MGLErrorDomain] && error.code == MGLErrorCodeSnapshotFailed),
+                      @"Should have been cancelled");
+        MGLTestAssert(strongself, snapshotter.cancelled, @"Should have been cancelled");
+        [expectation fulfill];
+    }];
+
+    [snapshotter cancel];
+
+    [self waitForExpectations:@[expectation] timeout:0.5];
+}
+
 - (void)testAllocatingSnapshotOnBackgroundQueue {
-    if (!validAccessToken()) {
+    if (![self validAccessToken]) {
         return;
     }
 
@@ -79,7 +202,6 @@ NSString* validAccessToken() {
     dispatch_queue_attr_t attr = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INITIATED, QOS_MIN_RELATIVE_PRIORITY);
     dispatch_queue_t backgroundQueue = dispatch_queue_create(__PRETTY_FUNCTION__, attr);
 
-    // This crashes maybe 1 in 10 times.
     dispatch_async(backgroundQueue, ^{
 
         // Create the snapshotter - DO NOT START.
@@ -104,8 +226,8 @@ NSString* validAccessToken() {
     [self waitForExpectations:@[expectation] timeout:2.0];
 }
 
-- (void)testSnapshotterFromBackgroundQueue {
-    if (!validAccessToken()) {
+- (void)testSnapshotterFromBackgroundQueueShouldFail {
+    if (![self validAccessToken]) {
         return;
     }
 
@@ -121,41 +243,25 @@ NSString* validAccessToken() {
     dispatch_queue_attr_t attr = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INITIATED, QOS_MIN_RELATIVE_PRIORITY); // also for concurrent
     dispatch_queue_t backgroundQueue = dispatch_queue_create(__PRETTY_FUNCTION__, attr);
 
-
     // Use dispatch_group to keep the backgroundQueue block around (and
     // so also the MGLMapSnapshotter
     dispatch_group_t group = dispatch_group_create();
     dispatch_group_enter(group);
 
-
     dispatch_async(backgroundQueue, ^{
 
         MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
         XCTAssertNotNil(snapshotter);
 
         MGLMapSnapshotCompletionHandler completion = ^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
-
-            // This should be the main queue
-            __typeof__(self) strongself = weakself;
-
-            MGLTestAssertNotNil(strongself, strongself);
-
-            MGLTestAssertNotNil(strongself, snapshot);
-            MGLTestAssertNotNil(strongself, snapshot.image);
-            MGLTestAssertNil(strongself, error, @"Snapshot should not error with: %@", error);
-
-            // Change this to XCTAttachmentLifetimeKeepAlways to be able to look at the snapshots after running
-            XCTAttachment *attachment = [XCTAttachment attachmentWithImage:snapshot.image];
-            attachment.lifetime = XCTAttachmentLifetimeDeleteOnSuccess;
-            [strongself addAttachment:attachment];
-
+            // The completion block should not be called
+            MGLTestFail(weakself);
             dispatch_group_leave(group);
         };
 
-        // untested
         @try {
             [snapshotter startWithCompletionHandler:completion];
-            MGLTestFail(weakself);
+            MGLTestFail(weakself, @"startWithCompletionHandler: should raise an exception");
         }
         @catch (NSException *exception) {
             MGLTestAssert(weakself, exception.name == NSInvalidArgumentException);
@@ -177,7 +283,7 @@ NSString* validAccessToken() {
 
 - (void)testMultipleSnapshottersPENDING {
     MGL_CHECK_IF_PENDING_TEST_SHOULD_RUN();
-    if (!validAccessToken()) {
+    if (![self validAccessToken]) {
         return;
     }
 
@@ -235,7 +341,7 @@ NSString* validAccessToken() {
 }
 
 - (void)testSnapshotPointConversion {
-    if (!validAccessToken()) {
+    if (![self validAccessToken]) {
         return;
     }
 
@@ -277,7 +383,7 @@ NSString* validAccessToken() {
 }
 
 - (void)testSnapshotPointConversionCoordinateOrdering {
-    if (!validAccessToken()) {
+    if (![self validAccessToken]) {
         return;
     }
 
diff --git a/platform/ios/Integration Tests/integration-Bridging-Header.h b/platform/ios/Integration Tests/integration-Bridging-Header.h
new file mode 100644
index 0000000000..55b44ea57b
--- /dev/null
+++ b/platform/ios/Integration Tests/integration-Bridging-Header.h
@@ -0,0 +1,5 @@
+//
+//  Use this file to import your target's public headers that you would like to expose to Swift.
+//
+#import "MGLMapViewIntegrationTest.h"
+
diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
index ca84ff295d..d9d15f76be 100644
--- a/platform/ios/ios.xcodeproj/project.pbxproj
+++ b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -393,6 +393,7 @@
 		CAA69DA4206DCD0E007279CD /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; };
 		CAA69DA5206DCD0E007279CD /* Mapbox.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		CABE5DAD2072FAB40003AF3C /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA8847D21CBAF91600AB86E3 /* Mapbox.framework */; };
+		CAE7AD5520F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAE7AD5420F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift */; };
 		DA00FC8E1D5EEB0D009AABC8 /* MGLAttributionInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		DA00FC8F1D5EEB0D009AABC8 /* MGLAttributionInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		DA00FC901D5EEB0D009AABC8 /* MGLAttributionInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = DA00FC8D1D5EEB0D009AABC8 /* MGLAttributionInfo.mm */; };
@@ -991,7 +992,7 @@
 		927FBCFA1F4DAA8300F8BF1F /* MBXSnapshotsViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MBXSnapshotsViewController.h; sourceTree = "<group>"; };
 		927FBCFB1F4DAA8300F8BF1F /* MBXSnapshotsViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MBXSnapshotsViewController.m; sourceTree = "<group>"; };
 		927FBCFD1F4DB05500F8BF1F /* MGLMapSnapshotter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLMapSnapshotter.h; sourceTree = "<group>"; };
-		927FBCFE1F4DB05500F8BF1F /* MGLMapSnapshotter.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLMapSnapshotter.mm; sourceTree = "<group>"; };
+		927FBCFE1F4DB05500F8BF1F /* MGLMapSnapshotter.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLMapSnapshotter.mm; sourceTree = "<group>"; wrapsLines = 0; };
 		92F2C3EC1F0E3C3A00268EC0 /* MGLRendererFrontend.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLRendererFrontend.h; sourceTree = "<group>"; };
 		92FC0AE7207CEE16007B6B54 /* MGLShapeOfflineRegion.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLShapeOfflineRegion.h; sourceTree = "<group>"; };
 		92FC0AE8207CEE16007B6B54 /* MGLShapeOfflineRegion_Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLShapeOfflineRegion_Private.h; sourceTree = "<group>"; };
@@ -1041,6 +1042,8 @@
 		CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = "<group>"; };
 		CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = "<group>"; };
 		CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationViewIntegrationTests.m; path = "Annotation Tests/MGLAnnotationViewIntegrationTests.m"; sourceTree = "<group>"; };
+		CAE7AD5320F46EF5003B6782 /* integration-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "integration-Bridging-Header.h"; sourceTree = "<group>"; };
+		CAE7AD5420F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MGLMapSnapshotterSwiftTests.swift; sourceTree = "<group>"; };
 		DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = "<group>"; };
 		DA00FC8D1D5EEB0D009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = "<group>"; };
 		DA0CD58F1CF56F6A00A5F5A5 /* MGLFeatureTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLFeatureTests.mm; path = ../../darwin/test/MGLFeatureTests.mm; sourceTree = "<group>"; };
@@ -1393,6 +1396,7 @@
 			isa = PBXGroup;
 			children = (
 				CA6914B320E67F07002DB0EE /* Annotations */,
+				CAE7AD5320F46EF5003B6782 /* integration-Bridging-Header.h */,
 				CA1B4A4F2099FA2800EDD491 /* Snapshotter Tests */,
 				16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */,
 				16376B0B1FFD9DAF0000563E /* Info.plist */,
@@ -1755,6 +1759,7 @@
 			isa = PBXGroup;
 			children = (
 				CA1B4A502099FB2200EDD491 /* MGLMapSnapshotterTest.m */,
+				CAE7AD5420F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift */,
 			);
 			path = "Snapshotter Tests";
 			sourceTree = "<group>";
@@ -2693,6 +2698,7 @@
 				TargetAttributes = {
 					16376B061FFD9DAF0000563E = {
 						CreatedOnToolsVersion = 9.2;
+						LastSwiftMigration = 0940;
 						ProvisioningStyle = Automatic;
 						TestTargetID = DA1DC9491CB6C1C2006E619F;
 					};
@@ -2881,6 +2887,7 @@
 				CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */,
 				16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */,
 				CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
+				CAE7AD5520F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift in Sources */,
 				CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */,
 				CA6914B520E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m in Sources */,
 				CA1B4A512099FB2200EDD491 /* MGLMapSnapshotterTest.m in Sources */,
@@ -3469,6 +3476,7 @@
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
 				CLANG_CXX_LANGUAGE_STANDARD = "gnu++14";
+				CLANG_ENABLE_MODULES = YES;
 				CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES;
 				CLANG_WARN_COMMA = YES;
 				CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES;
@@ -3484,6 +3492,9 @@
 				LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
 				PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";
 				PRODUCT_NAME = "$(TARGET_NAME)";
+				SWIFT_OBJC_BRIDGING_HEADER = "Integration Tests/integration-Bridging-Header.h";
+				SWIFT_OPTIMIZATION_LEVEL = "-Onone";
+				SWIFT_VERSION = 3.0;
 				TARGETED_DEVICE_FAMILY = "1,2";
 				TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Integration Test Harness.app/Integration Test Harness";
 			};
@@ -3496,6 +3507,7 @@
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
 				CLANG_CXX_LANGUAGE_STANDARD = "gnu++14";
+				CLANG_ENABLE_MODULES = YES;
 				CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES;
 				CLANG_WARN_COMMA = YES;
 				CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES;
@@ -3511,6 +3523,8 @@
 				LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
 				PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";
 				PRODUCT_NAME = "$(TARGET_NAME)";
+				SWIFT_OBJC_BRIDGING_HEADER = "Integration Tests/integration-Bridging-Header.h";
+				SWIFT_VERSION = 3.0;
 				TARGETED_DEVICE_FAMILY = "1,2";
 				TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Integration Test Harness.app/Integration Test Harness";
 			};
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index 8cc93d3447..2fb601bf6f 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog for Mapbox Maps SDK for macOS
 
+## master
+
+### Other changes
+
+* Fixed bug where completion block passed to `-[MGLMapSnapshotter startWithQueue:completionHandler:]` was not being called in all code paths. ([#12355](https://github.com/mapbox/mapbox-gl-native/pull/12355))
+
 # 0.11.0
 
 ### Styles and rendering