[ios,macos] Fix for infinite loop crash when setting shape to nil in MGLMapViewDelegate methods (#11614)

11 files changed, 630 insertions, 32 deletions

diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index ef89b20c43..609bf5f606 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -15,6 +15,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 ### Other changes
 
 * Unknown tokens in URLs are now preserved, rather than replaced with an empty string. ([#11787](https://github.com/mapbox/mapbox-gl-native/issues/11787))
+* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#11614](https://github.com/mapbox/mapbox-gl-native/pull/11614))
 
 ## 4.0.1 - May 14, 2018
 
diff --git a/platform/ios/Integration Tests/MGLCameraTransitionTests.mm b/platform/ios/Integration Tests/MGLCameraTransitionTests.mm
new file mode 100644
index 0000000000..d5c288fbb9
--- /dev/null
+++ b/platform/ios/Integration Tests/MGLCameraTransitionTests.mm
@@ -0,0 +1,398 @@
+#import "MGLMapViewIntegrationTest.h"
+#import "../../darwin/src/MGLGeometry_Private.h"
+
+@interface MBCameraTransitionTests : MGLMapViewIntegrationTest
+@end
+
+@implementation MBCameraTransitionTests
+
+- (void)testSetAndResetNorthWithDispatchAsyncInDelegateMethod {
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        MBCameraTransitionTests *strongSelf = weakself;
+
+        if (!strongSelf) return;
+
+        [expectation fulfill];
+
+        MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
+        if (mapView.direction != 0.0) {
+            dispatch_async(dispatch_get_main_queue(), ^{
+                [mapView resetNorth];
+            });
+        }
+    };
+
+    [self.mapView setDirection:90 animated:YES];
+
+    // loop, render, and wait
+    [self waitForExpectations:@[expectation] timeout:1.5];
+}
+
+
+- (void)testSetAndResetNorthInDelegateMethod {
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        MBCameraTransitionTests *strongSelf = weakself;
+
+        if (!strongSelf) return;
+
+        [expectation fulfill];
+
+        MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
+        if (mapView.direction != 0.0) {
+            NSLog(@"Reset to north");
+            [mapView resetNorth];
+        }
+    };
+
+    [self.mapView setDirection:90 animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.5];
+}
+
+- (void)testInterruptingAndResetNorthOnlyOnceInIsChanging {
+
+    // Reset to non-zero, prior to testing
+    [self.mapView setDirection:45 animated:NO];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+    __block BOOL startedReset = NO;
+    __block BOOL finishedReset = NO;
+
+    self.regionIsChanging = ^(MGLMapView *mapView) {
+        MBCameraTransitionTests *strongSelf = weakself;
+        if (!strongSelf) return;
+
+        if (!startedReset) {
+            NSLog(@"Reset to north, interrupting the previous transition");
+            startedReset = YES;
+            [mapView resetNorth];
+            finishedReset = YES;
+        }
+    };
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+        MBCameraTransitionTests *strongSelf = weakself;
+        if (!strongSelf) return;
+
+        MGLTestAssert(strongSelf, startedReset);
+
+        if (finishedReset) {
+            MGLTestAssert(strongSelf, !(reason & MGLCameraChangeReasonTransitionCancelled));
+            [expectation fulfill];
+        }
+        else {
+            MGLTestAssert(strongSelf, reason & MGLCameraChangeReasonTransitionCancelled);
+        }
+    };
+
+    [self.mapView setDirection:90 animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.5];
+
+    XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction);
+}
+
+- (void)testSetCenterCancelsTransitions {
+    XCTestExpectation *cameraIsInDCExpectation = [self expectationWithDescription:@"camera reset to DC"];
+
+    CLLocationCoordinate2D dc = CLLocationCoordinate2DMake(38.894368, -77.036487);
+    CLLocationCoordinate2D dc_west = CLLocationCoordinate2DMake(38.894368, -77.076487);
+
+    double zoomLevel = 15.0;
+
+    [self.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO];
+    [self.mapView setCenterCoordinate:dc_west zoomLevel:zoomLevel animated:YES];
+
+    __weak typeof(self) weakself = self;
+
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.15 * NSEC_PER_SEC),
+                   dispatch_get_main_queue(),
+                   ^{
+                       MBCameraTransitionTests *strongSelf = weakself;
+
+                       [strongSelf.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO];
+                       MGLTestAssertEqualWithAccuracy(strongSelf,
+                                                      dc.latitude,
+                                                      strongSelf.mapView.centerCoordinate.latitude,
+                                                      0.0005,
+                                                      @"setting center coordinate should cancel transitions");
+                       MGLTestAssertEqualWithAccuracy(strongSelf,
+                                                      dc.longitude,
+                                                      strongSelf.mapView.centerCoordinate.longitude,
+                                                      0.0005,
+                                                      @"setting center coordinate should cancel transitions");
+                       [cameraIsInDCExpectation fulfill];
+                   });
+
+    [self waitForExpectations:@[cameraIsInDCExpectation] timeout:10.0];
+}
+
+- (void)testSetCenterCoordinateInDelegateMethod {
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+    __block NSInteger delegateCallCount = 0;
+
+    CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0);
+    CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(-40.0, -40.0);
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        MBCameraTransitionTests *strongSelf = weakself;
+
+        if (!strongSelf) return;
+
+        MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
+
+        CLLocationCoordinate2D center = mapView.centerCoordinate;
+
+        switch(delegateCallCount) {
+            case 0:
+            {
+                // Our center coordinate should match our target (assuming we're not
+                // constrained by zoom level)
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target.longitude,
+                                               center.longitude,
+                                               0.0005,
+                                               @"center coordinate longitude should be at target");
+
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target.latitude,
+                                               center.latitude,
+                                               0.0005,
+                                               @"center coordinate latitude should be at target");
+
+                // Now set another coordinate.
+                // Should take MGLAnimationDuration seconds (0.3s)
+                [mapView setCenterCoordinate:target2 animated:YES];
+                break;
+            }
+
+            case 1:
+            {
+                // Our center coordinate should match our target (assuming we're not
+                // constrained by zoom level)
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target2.longitude,
+                                               center.longitude,
+                                               0.0005,
+                                               @"center coordinate longitude should be at target2");
+
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target2.latitude,
+                                               center.latitude,
+                                               0.0005,
+                                               @"center coordinate latitude should be at target2");
+                break;
+
+            }
+
+            default:
+                MGLTestFail(strongSelf);
+                break;
+        }
+
+        delegateCallCount++;
+
+        [expectation fulfill];
+    };
+
+    // Should take MGLAnimationDuration seconds (0.3)
+    [self.mapView setCenterCoordinate:target zoomLevel:15.0 animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.5];
+}
+
+- (void)testFlyToCameraInDelegateMethod {
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+
+    __weak typeof(self) weakself = self;
+    __block NSInteger delegateCallCount = 0;
+    expectation.expectedFulfillmentCount = 3;
+    expectation.assertForOverFulfill = YES;
+
+    CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0);
+    CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(30.0, 30.0);
+
+    __block BOOL runloop = YES;
+
+    NSTimeInterval stop0 = CACurrentMediaTime();
+    __block NSTimeInterval stop1 = 0.0;
+    __block NSTimeInterval stop2 = 0.0;
+
+    double zoomLevel = 5.0;
+    double altitude = MGLAltitudeForZoomLevel(zoomLevel, 0.0, target.latitude, self.mapView.frame.size);
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        MBCameraTransitionTests *strongSelf = weakself;
+
+        if (!strongSelf) return;
+
+        MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
+
+        CLLocationCoordinate2D center = mapView.centerCoordinate;
+
+        switch(delegateCallCount) {
+            case 0:
+            {
+                stop1 = CACurrentMediaTime();
+
+                // Our center coordinate should match our target (assuming we're not
+                // constrained by zoom level)
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target.longitude,
+                                               center.longitude,
+                                               0.0005,
+                                               @"center coordinate longitude should be at target");
+
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target.latitude,
+                                               center.latitude,
+                                               0.0005,
+                                               @"center coordinate latitude should be at target");
+
+                // Now set another coordinate.
+                MGLMapCamera *camera = [MGLMapCamera cameraLookingAtCenterCoordinate:target2
+                                                                        fromDistance:altitude
+                                                                               pitch:0.0
+                                                                             heading:0.0];
+
+                // flyToCamera can take a while...
+                [mapView flyToCamera:camera completionHandler:^{
+                    MGLTestAssert(strongSelf, !runloop, @"Completion block should be called after delegate method");
+                    [expectation fulfill];
+                    stop2 = CACurrentMediaTime();
+                }];
+                break;
+            }
+
+            case 1:
+            {
+                // Our center coordinate should match our target (assuming we're not
+                // constrained by zoom level)
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target2.longitude,
+                                               center.longitude,
+                                               0.0005,
+                                               @"center coordinate longitude should be at target2");
+
+                MGLTestAssertEqualWithAccuracy(strongSelf,
+                                               target2.latitude,
+                                               center.latitude,
+                                               0.0005,
+                                               @"center coordinate latitude should be at target2");
+
+                runloop = NO;
+                break;
+            }
+
+            default:
+                MGLTestFail(strongSelf);
+                break;
+        }
+
+        delegateCallCount++;
+
+        [expectation fulfill];
+    };
+
+    // Should take MGLAnimationDuration
+    [self.mapView setCenterCoordinate:target zoomLevel:zoomLevel animated:YES];
+
+    // Run the loop, so the camera can fly to the new camera
+    while (runloop) {
+        [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]];
+    }
+    [self waitForExpectations:@[expectation] timeout:0.5];
+
+    NSLog(@"setCenterCoordinate: %0.4fs", stop1 - stop0);
+    NSLog(@"flyToCamera: %0.4fs", stop2 - stop1);
+
+    XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", delegateCallCount); // Once for the setDirection and once for the reset north
+}
+
+#pragma mark - Pending tests
+
+- (void)disabled_testContinuallyResettingNorthInIsChangingPENDING {
+
+    // See https://github.com/mapbox/mapbox-gl-native/pull/11614
+    // This test currently fails, unsurprisingly, since we're continually
+    // setting the camera to the same parameters during its update.
+    //
+    // Possible solutions/expectations:
+    // - If you set camera parameters that match the *current* target parameters
+    //      then the transition could be a no-op. We'd need to consider any completion
+    //      block
+    //  - Ideally we would detect this case and disallow it.
+
+    // Reset to non-zero, prior to testing
+    [self.mapView setDirection:45 animated:NO];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
+
+    self.regionIsChanging = ^(MGLMapView *mapView) {
+        [mapView resetNorth];
+    };
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+        [expectation fulfill];
+    };
+
+    [self.mapView setDirection:90 animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.5];
+
+    XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction);
+}
+
+- (void)disabled_testContinuallySettingCoordinateInIsChangingPENDING {
+    // See above comment in `-disabled_testContinuallyResettingNorthInIsChangingPENDING`
+
+    // Reset to non-zero, prior to testing
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(0.0, 0.0) animated:NO];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+
+    self.regionIsChanging = ^(MGLMapView *mapView) {
+        [weakself.mapView setCenterCoordinate:CLLocationCoordinate2DMake(-40.0, -40.0) animated:YES];
+    };
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+        [expectation fulfill];
+    };
+
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(40.0, 40.0) animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.5];
+
+    XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction);
+}
+
+@end
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
index ab5d2cc46f..6c04ed9f84 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
@@ -1,18 +1,23 @@
 #import <XCTest/XCTest.h>
 #import <Mapbox/Mapbox.h>
 
-#define TestFailWithSelf(myself, ...) \
+#define MGLTestFail(myself, ...) \
     _XCTPrimitiveFail(myself, __VA_ARGS__)
 
+#define MGLTestAssert(myself, expression, ...) \
+    _XCTPrimitiveAssertTrue(myself, expression, @#expression, __VA_ARGS__)
+
+#define MGLTestAssertEqualWithAccuracy(myself, expression1, expression2, accuracy, ...) \
+    _XCTPrimitiveAssertEqualWithAccuracy(myself, expression1, @#expression1, expression2, @#expression2, accuracy, @#accuracy, __VA_ARGS__)
+
 @interface MGLMapViewIntegrationTest : XCTestCase <MGLMapViewDelegate>
 @property (nonatomic) MGLMapView *mapView;
 @property (nonatomic) MGLStyle *style;
 @property (nonatomic) XCTestExpectation *styleLoadingExpectation;
 @property (nonatomic) XCTestExpectation *renderFinishedExpectation;
-@property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, BOOL animated);
+@property (nonatomic) void (^regionWillChange)(MGLMapView *mapView, BOOL animated);
 @property (nonatomic) void (^regionIsChanging)(MGLMapView *mapView);
-
-
+@property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated);
 
 // Utility methods
 - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout;
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
index fc3229c83b..c42b8eef89 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
@@ -45,9 +45,9 @@
     self.renderFinishedExpectation = nil;
 }
 
-- (void)mapView:(MGLMapView *)mapView regionDidChangeAnimated:(BOOL)animated {
-    if (self.regionDidChange) {
-        self.regionDidChange(mapView, animated);
+- (void)mapView:(MGLMapView *)mapView regionWillChangeAnimated:(BOOL)animated {
+    if (self.regionWillChange) {
+        self.regionWillChange(mapView, animated);
     }
 }
 
@@ -57,6 +57,12 @@
     }
 }
 
+- (void)mapView:(MGLMapView *)mapView regionDidChangeWithReason:(MGLCameraChangeReason)reason animated:(BOOL)animated {
+    if (self.regionDidChange) {
+        self.regionDidChange(mapView, reason, animated);
+    }
+}
+
 #pragma mark - Utilities
 
 - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout {
diff --git a/platform/ios/Integration Tests/MGLShapeSourceTests.m b/platform/ios/Integration Tests/MGLShapeSourceTests.m
new file mode 100644
index 0000000000..088a9b011e
--- /dev/null
+++ b/platform/ios/Integration Tests/MGLShapeSourceTests.m
@@ -0,0 +1,126 @@
+//
+//  MBShapeSourceTests.m
+//  integration
+//
+//  Created by Julian Rex on 4/5/18.
+//  Copyright © 2018 Mapbox. All rights reserved.
+//
+
+#import "MGLMapViewIntegrationTest.h"
+
+@interface MGLShapeSourceTests : MGLMapViewIntegrationTest
+@end
+
+@implementation MGLShapeSourceTests
+
+- (void)testSettingShapeSourceToNilInRegionDidChange {
+
+    NSMutableArray *features = [[NSMutableArray alloc] init];
+
+    for (NSUInteger i = 0; i <= 180; i+=5) {
+        CLLocationCoordinate2D coord[4] = {
+            CLLocationCoordinate2DMake(round(0), round(i)),
+            CLLocationCoordinate2DMake(round(20), round(i)),
+            CLLocationCoordinate2DMake(round(0), round(i / 2 )),
+            CLLocationCoordinate2DMake(round(20), round(i / 2))};
+
+        MGLPolygonFeature *feature = [MGLPolygonFeature polygonWithCoordinates:coord count:4];
+        [features addObject:feature];
+    }
+
+    MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:features options:nil];
+    [self.style addSource:shapeSource];
+
+    MGLFillStyleLayer *layer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"layer" source:shapeSource];
+    layer.fillOpacity = [NSExpression expressionForConstantValue:@0.5];
+    [self.style addLayer:layer];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
+
+    __weak typeof(self) weakself = self;
+    __block NSInteger delegateCallCount = 0;
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        MGLShapeSourceTests *strongSelf = weakself;
+
+        if (!strongSelf)
+            return;
+
+        delegateCallCount++;
+
+        // Setting the shapeSource.shape = nil, was causing an infinite loop, so here
+        // we check for a runaway call. 10 here is arbitrary. We could argue that this
+        // should check that the call count is only 1, however in this case we particularly
+        // want to check for the infinite loop.
+        // See https://github.com/mapbox/mapbox-gl-native/issues/11180
+
+        if (delegateCallCount > 10) {
+            MGLTestFail(strongSelf);
+        }
+        else {
+            shapeSource.shape = nil;
+        }
+
+        [expectation fulfill];
+    };
+
+    // setCenterCoordinate is NOT animated here.
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0)];
+    [self waitForExpectations:@[expectation] timeout:5.0];
+}
+
+- (void)testSettingShapeSourceToNilInRegionIsChanging {
+
+    NSMutableArray *features = [[NSMutableArray alloc] init];
+
+    for (NSUInteger i = 0; i <= 180; i+=5) {
+        CLLocationCoordinate2D coord[4] = {
+            CLLocationCoordinate2DMake(round(0), round(i)),
+            CLLocationCoordinate2DMake(round(20), round(i)),
+            CLLocationCoordinate2DMake(round(0), round(i / 2 )),
+            CLLocationCoordinate2DMake(round(20), round(i / 2))};
+
+        MGLPolygonFeature *feature = [MGLPolygonFeature polygonWithCoordinates:coord count:4];
+        [features addObject:feature];
+    }
+
+    MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:features options:nil];
+    [self.style addSource:shapeSource];
+
+    MGLFillStyleLayer *layer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"layer" source:shapeSource];
+    layer.fillOpacity = [NSExpression expressionForConstantValue:@0.5];
+    [self.style addLayer:layer];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
+
+    __block NSInteger delegateCallCount = 0;
+    __weak typeof(self) weakself = self;
+
+    self.regionIsChanging = ^(MGLMapView *mapView) {
+        // See https://github.com/mapbox/mapbox-gl-native/issues/11180
+        shapeSource.shape = nil;
+    };
+
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        delegateCallCount++;
+
+        if (delegateCallCount > 1) {
+            MGLTestFail(weakself);
+        }
+
+        [expectation fulfill];
+    };
+
+    // Should take MGLAnimationDuration seconds (0.3)
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0) animated:YES];
+    [self waitForExpectations:@[expectation] timeout:1.0];
+}
+
+
+@end
diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
index 78cfff965c..aef2552b0b 100644
--- a/platform/ios/ios.xcodeproj/project.pbxproj
+++ b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -363,8 +363,10 @@
 		AC518E00201BB55A00EBC820 /* MGLTelemetryConfig.h in Headers */ = {isa = PBXBuildFile; fileRef = AC518DFD201BB55A00EBC820 /* MGLTelemetryConfig.h */; };
 		AC518E03201BB56000EBC820 /* MGLTelemetryConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */; };
 		AC518E04201BB56100EBC820 /* MGLTelemetryConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */; };
+		CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */; };
 		CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */; };
 		CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; };
+		CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */; };
 		CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		CA55CD42202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		CAA69DA4206DCD0E007279CD /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; };
@@ -996,9 +998,11 @@
 		96F3F73B1F5711F1003E2D2C /* MGLUserLocationHeadingIndicator.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLUserLocationHeadingIndicator.h; sourceTree = "<group>"; };
 		AC518DFD201BB55A00EBC820 /* MGLTelemetryConfig.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLTelemetryConfig.h; sourceTree = "<group>"; };
 		AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLTelemetryConfig.m; sourceTree = "<group>"; };
+		CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLShapeSourceTests.m; sourceTree = "<group>"; };
 		CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLMapViewIntegrationTest.m; sourceTree = "<group>"; };
 		CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLMapViewIntegrationTest.h; sourceTree = "<group>"; };
 		CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleLayerIntegrationTests.m; sourceTree = "<group>"; };
+		CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionTests.mm; sourceTree = "<group>"; };
 		CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; 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>"; };
@@ -1353,6 +1357,8 @@
 			children = (
 				16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */,
 				16376B0B1FFD9DAF0000563E /* Info.plist */,
+				CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */,
+				CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */,
 				CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */,
 				CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */,
 				CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */,
@@ -2809,8 +2815,10 @@
 			buildActionMask = 2147483647;
 			files = (
 				CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */,
+				CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */,
 				16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */,
 				CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
+				CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -3379,6 +3387,7 @@
 /* Begin XCBuildConfiguration section */
 		16376B0E1FFD9DAF0000563E /* Debug */ = {
 			isa = XCBuildConfiguration;
+			baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */;
 			buildSettings = {
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
@@ -3393,6 +3402,7 @@
 				CODE_SIGN_IDENTITY = "iPhone Developer";
 				CODE_SIGN_STYLE = Automatic;
 				GCC_C_LANGUAGE_STANDARD = gnu11;
+				HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)";
 				INFOPLIST_FILE = "Integration Tests/Info.plist";
 				LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
 				PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";
@@ -3404,6 +3414,7 @@
 		};
 		16376B0F1FFD9DAF0000563E /* Release */ = {
 			isa = XCBuildConfiguration;
+			baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */;
 			buildSettings = {
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
@@ -3418,6 +3429,7 @@
 				CODE_SIGN_IDENTITY = "iPhone Developer";
 				CODE_SIGN_STYLE = Automatic;
 				GCC_C_LANGUAGE_STANDARD = gnu11;
+				HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)";
 				INFOPLIST_FILE = "Integration Tests/Info.plist";
 				LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
 				PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";
diff --git a/platform/ios/src/MGLCameraChangeReason.h b/platform/ios/src/MGLCameraChangeReason.h
index 6c6b3636ba..f439d3e7ea 100644
--- a/platform/ios/src/MGLCameraChangeReason.h
+++ b/platform/ios/src/MGLCameraChangeReason.h
@@ -57,5 +57,9 @@ typedef NS_OPTIONS(NSUInteger, MGLCameraChangeReason)
     MGLCameraChangeReasonGestureOneFingerZoom = 1 << 7,
 
     // :nodoc: The user panned with two fingers to tilt the map (two finger drag).
-    MGLCameraChangeReasonGestureTilt = 1 << 8
+    MGLCameraChangeReasonGestureTilt = 1 << 8,
+
+    // :nodoc: Cancelled
+    MGLCameraChangeReasonTransitionCancelled = 1 << 16
+
 };
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 26b23abb4e..e16411f2c5 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -1271,7 +1271,7 @@ public:
     {
         [self setUserTrackingMode:MGLUserTrackingModeNone animated:NO];
     }
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 }
 
 - (void)notifyGestureDidBegin {
@@ -1321,7 +1321,7 @@ public:
 {
     if ( ! self.isScrollEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     MGLMapCamera *oldCamera = self.camera;
 
@@ -1390,7 +1390,7 @@ public:
 {
     if ( ! self.isZoomEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     CGPoint centerPoint = [self anchorPointForGesture:pinch];
     MGLMapCamera *oldCamera = self.camera;
@@ -1491,7 +1491,7 @@ public:
 {
     if ( ! self.isRotateEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     CGPoint centerPoint = [self anchorPointForGesture:rotate];
     MGLMapCamera *oldCamera = self.camera;
@@ -1696,7 +1696,7 @@ public:
 {
     if ( ! self.isZoomEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     if (doubleTap.state == UIGestureRecognizerStateEnded)
     {
@@ -1737,7 +1737,7 @@ public:
 
     if (_mbglMap->getZoom() == _mbglMap->getMinZoom()) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureZoomOut;
 
@@ -1776,7 +1776,7 @@ public:
 {
     if ( ! self.isZoomEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureOneFingerZoom;
 
@@ -1821,7 +1821,7 @@ public:
 {
     if ( ! self.isPitchEnabled) return;
 
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureTilt;
 
@@ -2981,7 +2981,7 @@ public:
         return;
     }
     
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
 
@@ -3006,7 +3006,7 @@ public:
 - (void)setZoomLevel:(double)zoomLevel animated:(BOOL)animated
 {
     if (zoomLevel == self.zoomLevel) return;
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
 
@@ -3149,7 +3149,7 @@ public:
     }
     
     [self willChangeValueForKey:@"visibleCoordinateBounds"];
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
 
@@ -3182,7 +3182,7 @@ public:
 - (void)_setDirection:(CLLocationDirection)direction animated:(BOOL)animated
 {
     if (direction == self.direction) return;
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     CGFloat duration = animated ? MGLAnimationDuration : 0;
 
@@ -3271,7 +3271,7 @@ public:
     }
 
     [self willChangeValueForKey:@"camera"];
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
 
@@ -3330,7 +3330,7 @@ public:
     }
 
     [self willChangeValueForKey:@"camera"];
-    _mbglMap->cancelTransitions();
+    [self cancelTransitions];
 
     self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
 
@@ -3339,6 +3339,12 @@ public:
     [self didChangeValueForKey:@"camera"];
 }
 
+- (void)cancelTransitions {
+    self.cameraChangeReasonBitmask |= MGLCameraChangeReasonTransitionCancelled;
+    _mbglMap->cancelTransitions();
+    self.cameraChangeReasonBitmask &= ~MGLCameraChangeReasonTransitionCancelled;
+}
+
 - (MGLMapCamera *)cameraThatFitsCoordinateBounds:(MGLCoordinateBounds)bounds
 {
     return [self cameraThatFitsCoordinateBounds:bounds edgePadding:UIEdgeInsetsZero];
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index 0b3db6becb..c711f6f352 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -13,6 +13,7 @@
 ### Other changes
 
 * Unknown tokens in URLs are now preserved, rather than replaced with an empty string. ([#11787](https://github.com/mapbox/mapbox-gl-native/issues/11787))
+* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#11614](https://github.com/mapbox/mapbox-gl-native/pull/11614))
 
 ## 0.7.1
 
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index 105adf0400..da8e243d91 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -594,13 +594,10 @@ void Transform::startTransition(const CameraOptions& camera,
                 animation.transitionFrameFn(t);
             }
             observer.onCameraIsChanging();
+            return false;
         } else {
-            transitionFinishFn();
-            transitionFinishFn = nullptr;
-
-            // This callback gets destroyed here,
-            // we can only return after this point.
-            transitionFrameFn = nullptr;
+            // Indicate that we need to terminate this transition
+            return true;
         }
     };
 
@@ -615,7 +612,14 @@ void Transform::startTransition(const CameraOptions& camera,
     };
 
     if (!isAnimated) {
-        transitionFrameFn(Clock::now());
+        auto update = std::move(transitionFrameFn);
+        auto finish = std::move(transitionFinishFn);
+
+        transitionFrameFn = nullptr;
+        transitionFinishFn = nullptr;
+
+        update(Clock::now());
+        finish();
     }
 }
 
@@ -624,8 +628,43 @@ bool Transform::inTransition() const {
 }
 
 void Transform::updateTransitions(const TimePoint& now) {
-    if (transitionFrameFn) {
-        transitionFrameFn(now);
+
+    // Use a temporary function to ensure that the transitionFrameFn lambda is
+    // called only once per update.
+
+    // This addresses the symptoms of https://github.com/mapbox/mapbox-gl-native/issues/11180
+    // where setting a shape source to nil (or similar) in the `onCameraIsChanging`
+    // observer function causes `Map::Impl::onUpdate()` to be called which
+    // in turn calls this function (before the current iteration has completed),
+    // leading to an infinite loop. See https://github.com/mapbox/mapbox-gl-native/issues/5833
+    // for a similar, related, issue.
+    //
+    // By temporarily nulling the `transitionFrameFn` (and then restoring it
+    // after the temporary has been called) we stop this recursion.
+    //
+    // It's important to note that the scope of this change is stop the above
+    // crashes. It doesn't address any potential deeper issue (for example
+    // user error, how often and when transition callbacks are called).
+
+    auto transition = std::move(transitionFrameFn);
+    transitionFrameFn = nullptr;
+
+    if (transition && transition(now)) {
+        // If the transition indicates that it is complete, then we should call
+        // the finish lambda (going via a temporary as above)
+        auto finish = std::move(transitionFinishFn);
+
+        transitionFinishFn = nullptr;
+        transitionFrameFn = nullptr;
+
+        if (finish) {
+            finish();
+        }
+    } else if (!transitionFrameFn) {
+        // We have to check `transitionFrameFn` is nil here, since a new transition
+        // may have been triggered in a user callback (from the transition call
+        // above)
+        transitionFrameFn = std::move(transition);
     }
 }
 
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index d429c57661..bff44a2dcd 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -165,7 +165,7 @@ private:
 
     TimePoint transitionStart;
     Duration transitionDuration;
-    std::function<void(const TimePoint)> transitionFrameFn;
+    std::function<bool(const TimePoint)> transitionFrameFn;
     std::function<void()> transitionFinishFn;
 };