[ios, osx] More accurate KVO, private delegate, descriptive fallback name, fewer reloads

Made MGLOfflineStorage.packs strong rather than copy so that KVO notifications on that key path include index paths. Replaced custom KVO mutable collection methods with calls to -mutableArrayValueForKey:, avoiding redundant casts.

Privatized MGLOfflinePackDelegate and made its methods required, since MGLOfflineStorage is the only conforming implementation. Require a pack to be explicitly invalidated before deallocation by its owner (MGLOfflineStorage). Added a method to reload the canonical collection of packs.

Previously, if the user adds an offline pack without specifying a name in the alert, the pack would be named “Untitled n” for the nth pack created since the application was launched. This logic could result in identically named packs (for example, if the user relaunches and creates another unnamed pack). Instead, use the visible coordinate bounds as a placeholder and fallback name.

Resume a pack upon successful addition. Removed an extraneous weakSelf/strongSelf dance. When possible, update individual labels in a cell instead of reloading the entire cell. That way, if the user swipes to the left to reveal the Delete accessory button while the pack is actively downloading, the accessory button remains on screen as progress updates come in.

1 files changed, 9 insertions, 15 deletions

diff --git a/platform/darwin/src/MGLOfflinePack.mm b/platform/darwin/src/MGLOfflinePack.mm
index 42bd871e54..279261e9be 100644
--- a/platform/darwin/src/MGLOfflinePack.mm
+++ b/platform/darwin/src/MGLOfflinePack.mm
@@ -38,6 +38,7 @@ private:
 
 @interface MGLOfflinePack ()
 
+@property (nonatomic, weak, nullable) id <MGLOfflinePackDelegate> delegate;
 @property (nonatomic, nullable, readwrite) mbgl::OfflineRegion *mbglOfflineRegion;
 @property (nonatomic, readwrite) MGLOfflinePackState state;
 @property (nonatomic, readwrite) MGLOfflinePackProgress progress;
@@ -66,10 +67,7 @@ private:
 }
 
 - (void)dealloc {
-    if (_mbglOfflineRegion && _state != MGLOfflinePackStateInvalid) {
-        mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource];
-        mbglFileSource->setOfflineRegionObserver(*_mbglOfflineRegion, nullptr);
-    }
+    NSAssert(_state == MGLOfflinePackStateInvalid, @"MGLOfflinePack was not invalided prior to deallocation.");
 }
 
 - (id <MGLOfflineRegion>)region {
@@ -105,6 +103,8 @@ private:
     NSAssert(_state != MGLOfflinePackStateInvalid, @"Cannot invalidate an already invalid offline pack.");
     
     self.state = MGLOfflinePackStateInvalid;
+    mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource];
+    mbglFileSource->setOfflineRegionObserver(*self.mbglOfflineRegion, nullptr);
     self.mbglOfflineRegion = nil;
 }
 
@@ -159,9 +159,7 @@ private:
     progress.maximumResourcesExpected = status.requiredResourceCountIsPrecise ? status.requiredResourceCount : UINT64_MAX;
     self.progress = progress;
     
-    if ([self.delegate respondsToSelector:@selector(offlinePack:progressDidChange:)]) {
-        [self.delegate offlinePack:self progressDidChange:progress];
-    }
+    [self.delegate offlinePack:self progressDidChange:progress];
 }
 
 NSError *MGLErrorFromResponseError(mbgl::Response::Error error) {
@@ -187,6 +185,8 @@ NSError *MGLErrorFromResponseError(mbgl::Response::Error error) {
     }];
 }
 
+@end
+
 void MBGLOfflineRegionObserver::statusChanged(mbgl::OfflineRegionStatus status) {
     dispatch_async(dispatch_get_main_queue(), ^{
         [pack offlineRegionStatusDidChange:status];
@@ -195,18 +195,12 @@ void MBGLOfflineRegionObserver::statusChanged(mbgl::OfflineRegionStatus status)
 
 void MBGLOfflineRegionObserver::responseError(mbgl::Response::Error error) {
     dispatch_async(dispatch_get_main_queue(), ^{
-        if ([pack.delegate respondsToSelector:@selector(offlinePack:didReceiveError:)]) {
-            [pack.delegate offlinePack:pack didReceiveError:MGLErrorFromResponseError(error)];
-        }
+        [pack.delegate offlinePack:pack didReceiveError:MGLErrorFromResponseError(error)];
     });
 }
 
 void MBGLOfflineRegionObserver::mapboxTileCountLimitExceeded(uint64_t limit) {
     dispatch_async(dispatch_get_main_queue(), ^{
-        if ([pack.delegate respondsToSelector:@selector(offlinePack:didReceiveMaximumAllowedMapboxTiles:)]) {
-            [pack.delegate offlinePack:pack didReceiveMaximumAllowedMapboxTiles:limit];
-        }
+        [pack.delegate offlinePack:pack didReceiveMaximumAllowedMapboxTiles:limit];
     });
 }
-
-@end