Merge branch 'master' into jrex/13120-teardown-mbglmapupstream/jrex/13120-teardown-mbglmap

9 files changed, 84 insertions, 9 deletions

diff --git a/platform/darwin/resources/Base.lproj/Foundation.strings b/platform/darwin/resources/Base.lproj/Foundation.strings
index 19bfe750e6..360e3c8562 100644
--- a/platform/darwin/resources/Base.lproj/Foundation.strings
+++ b/platform/darwin/resources/Base.lproj/Foundation.strings
@@ -298,3 +298,9 @@
 /* OpenStreetMap short name attribution */
 "OSM_SHORT_NAME" = "OSM";
 
+/* User-friendly error description */
+"REMOVE_SRC_FAIL_IN_USE_FMT" = "Source '%@' is in use, cannot remove.";
+
+/* User-friendly error description */
+"REMOVE_SRC_FAIL_MISMATCH_FMT" = "Identifier '%1$@' does not match source identifier '%2$s'";
+
diff --git a/platform/darwin/src/MGLSource.mm b/platform/darwin/src/MGLSource.mm
index a32e223782..62d3cfa808 100644
--- a/platform/darwin/src/MGLSource.mm
+++ b/platform/darwin/src/MGLSource.mm
@@ -1,6 +1,7 @@
 #import "MGLSource_Private.h"
 #import "MGLStyle_Private.h"
 #import "MGLMapView_Private.h"
+#import "NSBundle+MGLAdditions.h"
 
 #include <mbgl/style/style.hpp>
 #include <mbgl/map/map.hpp>
@@ -57,11 +58,39 @@
     _mapView.style.rawStyle->addSource(std::move(_pendingSource));
 }
 
-- (void)removeFromMapView:(MGLMapView *)mapView {
+- (BOOL)removeFromMapView:(MGLMapView *)mapView error:(NSError * __nullable * __nullable)outError {
+    BOOL removed = NO;
+    
     if (self.rawSource == mapView.style.rawStyle->getSource(self.identifier.UTF8String)) {
-        _pendingSource = mapView.style.rawStyle->removeSource(self.identifier.UTF8String);
-        _mapView = nil;
+        
+        auto removedSource = mapView.style.rawStyle->removeSource(self.identifier.UTF8String);
+        
+        if (removedSource) {
+            removed = YES;
+            _pendingSource = std::move(removedSource);
+            _mapView = nil;
+        } else if (outError) {
+            NSString *format = NSLocalizedStringWithDefaultValue(@"REMOVE_SRC_FAIL_IN_USE_FMT", @"Foundation", nil, @"Source '%@' is in use, cannot remove.", @"User-friendly error description");
+            NSString *localizedDescription = [NSString stringWithFormat:format, self.identifier];
+
+            *outError = [NSError errorWithDomain:MGLErrorDomain
+                                            code:MGLErrorCodeSourceIsInUseCannotRemove
+                                        userInfo:@{ NSLocalizedDescriptionKey : localizedDescription }];
+        }
     }
+    else if (outError) {
+        // Consider raising an exception here
+        NSString *format = NSLocalizedStringWithDefaultValue(@"REMOVE_SRC_FAIL_MISMATCH_FMT", @"Foundation", nil, @"Identifier '%@' does not match source identifier '%s'", @"User-friendly error description");
+        NSString *localizedDescription = [NSString stringWithFormat:format,
+                                          self.identifier,
+                                          self.rawSource ? self.rawSource->getID().c_str() : "(null)"];
+        
+        *outError = [NSError errorWithDomain:MGLErrorDomain
+                                        code:MGLErrorCodeSourceIdentifierMismatch
+                                    userInfo:@{ NSLocalizedDescriptionKey : localizedDescription }];
+    }
+    
+    return removed;
 }
 
 - (NSString *)description {
diff --git a/platform/darwin/src/MGLSource_Private.h b/platform/darwin/src/MGLSource_Private.h
index d7d1f66641..af14c11b90 100644
--- a/platform/darwin/src/MGLSource_Private.h
+++ b/platform/darwin/src/MGLSource_Private.h
@@ -67,7 +67,7 @@ struct SourceWrapper {
  to the `MGLSource` instance and the unique_ptr reference is valid again. It is
  safe to add the source back to the style after it is removed.
  */
-- (void)removeFromMapView:(MGLMapView *)mapView;
+- (BOOL)removeFromMapView:(MGLMapView *)mapView error:(NSError * __nullable * __nullable)outError;
 
 @end
 
diff --git a/platform/darwin/src/MGLStyle.h b/platform/darwin/src/MGLStyle.h
index fcbd318b18..7621db0ad5 100644
--- a/platform/darwin/src/MGLStyle.h
+++ b/platform/darwin/src/MGLStyle.h
@@ -318,6 +318,27 @@ MGL_EXPORT
  */
 - (void)removeSource:(MGLSource *)source;
 
+/**
+ Removes a source from the current style.
+ 
+ @note Source identifiers are not guaranteed to exist across styles or different
+ versions of the same style. Applications that use this API must first set the
+ style URL to an explicitly versioned style using a convenience method like
+ `+[MGLStyle outdoorsStyleURLWithVersion:]`, `MGLMapView`’s “Style URL”
+ inspectable in Interface Builder, or a manually constructed `NSURL`. This
+ approach also avoids source identifer name changes that will occur in the default
+ style’s sources over time.
+ 
+ @param source The source to remove from the current style.
+ @param outError Upon return, if an error has occurred, a pointer to an `NSError`
+ object describing the error. Pass in `NULL` to ignore any error.
+ 
+ @return `YES` if `source` was removed successfully. If `NO`, `outError` contains
+ an `NSError` object describing the problem.
+ */
+- (BOOL)removeSource:(MGLSource *)source error:(NSError * __nullable * __nullable)outError;
+
+
 #pragma mark Managing Style Layers
 
 /**
diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm
index 4c98fd332b..244d09ea14 100644
--- a/platform/darwin/src/MGLStyle.mm
+++ b/platform/darwin/src/MGLStyle.mm
@@ -219,16 +219,23 @@ static_assert(6 == mbgl::util::default_styles::numOrderedStyles,
 
 - (void)removeSource:(MGLSource *)source
 {
+    [self removeSource:source error:nil];
+}
+
+- (BOOL)removeSource:(MGLSource *)source error:(NSError * __nullable * __nullable)outError {
     MGLLogDebug(@"Removing source: %@", source);
+    
     if (!source.rawSource) {
         [NSException raise:NSInvalidArgumentException format:
          @"The source %@ cannot be removed from the style. "
          @"Make sure the source was created as a member of a concrete subclass of MGLSource.",
          source];
     }
-    [source removeFromMapView:self.mapView];
+
+    return [source removeFromMapView:self.mapView error:outError];
 }
 
+
 - (nullable NSArray<MGLAttributionInfo *> *)attributionInfosWithFontSize:(CGFloat)fontSize linkColor:(nullable MGLColor *)linkColor {
     // It’d be incredibly convenient to use -sources here, but this operation
     // depends on the sources being sorted in ascending order by creation, as
diff --git a/platform/darwin/src/MGLTypes.h b/platform/darwin/src/MGLTypes.h
index 1cf52f20ce..5aee79bd82 100644
--- a/platform/darwin/src/MGLTypes.h
+++ b/platform/darwin/src/MGLTypes.h
@@ -49,7 +49,11 @@ 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,
+    /** Source is in use and cannot be removed */
+    MGLErrorCodeSourceIsInUseCannotRemove = 7,
+    /** Source is in use and cannot be removed */
+    MGLErrorCodeSourceIdentifierMismatch = 8
 };
 
 /** Options for enabling debugging features in an `MGLMapView` instance. */
diff --git a/platform/darwin/test/MGLStyleTests.mm b/platform/darwin/test/MGLStyleTests.mm
index d317292096..ce7e3036a1 100644
--- a/platform/darwin/test/MGLStyleTests.mm
+++ b/platform/darwin/test/MGLStyleTests.mm
@@ -237,7 +237,12 @@
     [self.style addLayer:fillLayer];
 
     // Attempt to remove the raster tile source
-    [self.style removeSource:rasterTileSource];
+    NSError *error;
+    BOOL result = [self.style removeSource:rasterTileSource error:&error];
+    
+    XCTAssertFalse(result);
+    XCTAssertEqualObjects(error.domain, MGLErrorDomain);
+    XCTAssertEqual(error.code, MGLErrorCodeSourceIsInUseCannotRemove);
     
     // Ensure it is still there
     XCTAssertTrue([[self.style sourceWithIdentifier:rasterTileSource.identifier] isMemberOfClass:[MGLRasterTileSource class]]);
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index a33aa63ea9..a93555ebe8 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * This SDK’s dynamic framework now has a bundle identifier of `com.mapbox.Mapbox`. ([#12857](https://github.com/mapbox/mapbox-gl-native/pull/12857))
 * Modified the behavior of the map view so that programmatic camera transitions can no longer be interrupted by user interaction when `MGLMapView.zoomEnabled`, `MGLMapView.rotateEnabled`, `MGLMapView.scrollEnabled`, and `MGLMapView.pitchEnabled` are set to false. ([#13362](https://github.com/mapbox/mapbox-gl-native/pull/13362))
 * Fixed random crashes during app termination. ([#13367](https://github.com/mapbox/mapbox-gl-native/pull/13367))
+* Added `-[MGLStyle removeSource:error:]` that returns a `BOOL` indicating success (and an optional `NSError` in case of failure). ([#13399](https://github.com/mapbox/mapbox-gl-native/pull/13399))
 
 ## 4.6.0 - November 7, 2018
 
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index 597f97d31c..58d08d1656 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -1,12 +1,14 @@
 # Changelog for Mapbox Maps SDK for macOS
 
+## master
+
+* Added `-[MGLStyle removeSource:error:]` that returns a `BOOL` indicating success (and an optional `NSError` in case of failure). ([#13399](https://github.com/mapbox/mapbox-gl-native/pull/13399))
+
 ## 0.12.0 - November 8, 2018
 
 * Renamed `-[MGLOfflineStorage putResourceWithUrl:data:modified:expires:etag:mustRevalidate:]` to `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:]`. ([#13318](https://github.com/mapbox/mapbox-gl-native/pull/13318))
 * This SDK’s dynamic framework now has a bundle identifier of `com.mapbox.Mapbox`. ([#12857](https://github.com/mapbox/mapbox-gl-native/pull/12857))
 
-## master
-
 ### Styles and rendering
 
 * `MGLSymbolStyleLayer.text` can now be set to rich text with varying fonts and text sizes. ([#12624](https://github.com/mapbox/mapbox-gl-native/pull/12624))