diff options
author | Minh Nguyễn <mxn@1ec5.org> | 2016-03-09 15:57:21 -0800 |
---|---|---|
committer | Minh Nguyễn <mxn@1ec5.org> | 2016-03-10 17:14:14 -0800 |
commit | 7fa3b92429185eb3749c9f547839712c8a0faf2b (patch) | |
tree | 89de99fa34cc6f17ee1bf85961defc372384cc0b /platform | |
parent | 77d99584cec58ea2c6c8b58257ca5e13d0c02834 (diff) | |
download | qtlocation-mapboxgl-7fa3b92429185eb3749c9f547839712c8a0faf2b.tar.gz |
[ios, osx] Tightened offline task state
Fixed a race condition that could occur if a progress update comes after a call to remove the task but before the removal is complete. Avoid synchronously setting the task’s state inside -resume and -suspend; instead, rely on the observer to set the state asynchronously.
Diffstat (limited to 'platform')
-rw-r--r-- | platform/darwin/src/MGLOfflineStorage.mm | 5 | ||||
-rw-r--r-- | platform/darwin/src/MGLOfflineTask.mm | 32 | ||||
-rw-r--r-- | platform/darwin/src/MGLOfflineTask_Private.h | 10 | ||||
-rw-r--r-- | platform/darwin/src/MGLTilePyramidOfflineRegion.mm | 8 |
4 files changed, 43 insertions, 12 deletions
diff --git a/platform/darwin/src/MGLOfflineStorage.mm b/platform/darwin/src/MGLOfflineStorage.mm index 274cef7688..b245273954 100644 --- a/platform/darwin/src/MGLOfflineStorage.mm +++ b/platform/darwin/src/MGLOfflineStorage.mm @@ -104,8 +104,9 @@ } - (void)removeTask:(MGLOfflineTask *)task withCompletionHandler:(MGLOfflineTaskRemovalCompletionHandler)completion { + mbgl::OfflineRegion *mbglOfflineRegion = task.mbglOfflineRegion; [task invalidate]; - self.mbglFileSource->deleteOfflineRegion(std::move(*task.mbglOfflineRegion), [&, task, completion](std::exception_ptr exception) { + self.mbglFileSource->deleteOfflineRegion(std::move(*mbglOfflineRegion), [&, completion](std::exception_ptr exception) { NSError *error; if (exception) { error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:@{ @@ -113,7 +114,7 @@ }]; } if (completion) { - dispatch_async(dispatch_get_main_queue(), [&, task, completion, error](void) { + dispatch_async(dispatch_get_main_queue(), [&, completion, error](void) { completion(error); }); } diff --git a/platform/darwin/src/MGLOfflineTask.mm b/platform/darwin/src/MGLOfflineTask.mm index 9f6aa3312b..4698a492fe 100644 --- a/platform/darwin/src/MGLOfflineTask.mm +++ b/platform/darwin/src/MGLOfflineTask.mm @@ -7,6 +7,12 @@ #include <mbgl/storage/default_file_source.hpp> #include <mbgl/util/string.hpp> +/** + Assert that the current offline task is valid. + + This macro should be used at the beginning of any public-facing instance method + of `MGLOfflineTask`. For private methods, an assertion is more appropriate. + */ #define MGLAssertOfflineTaskIsValid() \ do { \ if (_state == MGLOfflineTaskStateInvalid) { \ @@ -32,7 +38,7 @@ private: @interface MGLOfflineTask () -@property (nonatomic, readwrite) mbgl::OfflineRegion *mbglOfflineRegion; +@property (nonatomic, nullable, readwrite) mbgl::OfflineRegion *mbglOfflineRegion; @property (nonatomic, readwrite) MGLOfflineTaskState state; @property (nonatomic, readwrite) MGLOfflineTaskProgress progress; @@ -54,8 +60,7 @@ private: _state = MGLOfflineTaskStateUnknown; mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; - MBGLOfflineRegionObserver *mbglObserver = new MBGLOfflineRegionObserver(self); - mbglFileSource->setOfflineRegionObserver(*_mbglOfflineRegion, std::make_unique<MBGLOfflineRegionObserver>(*mbglObserver)); + mbglFileSource->setOfflineRegionObserver(*_mbglOfflineRegion, std::make_unique<MBGLOfflineRegionObserver>(self)); } return self; } @@ -85,7 +90,6 @@ private: mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; mbglFileSource->setOfflineRegionDownloadState(*_mbglOfflineRegion, mbgl::OfflineRegionDownloadState::Active); - self.state = MGLOfflineTaskStateActive; } - (void)suspend { @@ -93,13 +97,27 @@ private: mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; mbglFileSource->setOfflineRegionDownloadState(*_mbglOfflineRegion, mbgl::OfflineRegionDownloadState::Inactive); - self.state = MGLOfflineTaskStateInactive; } - (void)invalidate { - MGLAssertOfflineTaskIsValid(); + NSAssert(_state != MGLOfflineTaskStateInvalid, @"Cannot invalidate an already invalid offline task."); self.state = MGLOfflineTaskStateInvalid; + self.mbglOfflineRegion = nil; +} + +- (void)setState:(MGLOfflineTaskState)state { + if (!self.mbglOfflineRegion) { + // A progress update has arrived after the call to + // -[MGLOfflineStorage removeTask:withCompletionHandler:] but before the + // removal is complete and the completion handler is called. + NSAssert(_state == MGLOfflineTaskStateInvalid, @"A valid MGLOfflineTask has no mbgl::OfflineRegion."); + return; + } + + NSAssert(_state != MGLOfflineTaskStateInvalid, @"Cannot change the state of an invalid offline task."); + + _state = state; } - (void)requestProgress { @@ -120,7 +138,7 @@ private: } - (void)offlineRegionStatusDidChange:(mbgl::OfflineRegionStatus)status { - MGLAssertOfflineTaskIsValid(); + NSAssert(_state != MGLOfflineTaskStateInvalid, @"Cannot change update progress of an invalid offline task."); switch (status.downloadState) { case mbgl::OfflineRegionDownloadState::Inactive: diff --git a/platform/darwin/src/MGLOfflineTask_Private.h b/platform/darwin/src/MGLOfflineTask_Private.h index 1c1ff59b84..7a425d7647 100644 --- a/platform/darwin/src/MGLOfflineTask_Private.h +++ b/platform/darwin/src/MGLOfflineTask_Private.h @@ -2,12 +2,20 @@ #include <mbgl/storage/default_file_source.hpp> +NS_ASSUME_NONNULL_BEGIN + @interface MGLOfflineTask (Private) -@property (nonatomic) mbgl::OfflineRegion *mbglOfflineRegion; +@property (nonatomic, nullable) mbgl::OfflineRegion *mbglOfflineRegion; - (instancetype)initWithMBGLRegion:(mbgl::OfflineRegion *)region; +/** + Invalidates the task and ensures that no future progress update can ever + revalidate it. + */ - (void)invalidate; @end + +NS_ASSUME_NONNULL_END diff --git a/platform/darwin/src/MGLTilePyramidOfflineRegion.mm b/platform/darwin/src/MGLTilePyramidOfflineRegion.mm index 3d51917c3e..6507ceef77 100644 --- a/platform/darwin/src/MGLTilePyramidOfflineRegion.mm +++ b/platform/darwin/src/MGLTilePyramidOfflineRegion.mm @@ -33,8 +33,12 @@ } if (!styleURL.scheme) { - // Assume a relative path into the application bundle. - styleURL = [NSURL URLWithString:[@"asset://" stringByAppendingString:styleURL.absoluteString]]; + [NSException raise:@"Invalid style URL" format: + @"%@ does not support setting a relative file URL as the style URL. " + @"To download the online resources required by this style, " + @"specify a URL to an online copy of this style. " + @"For Mapbox-hosted styles, use the mapbox: scheme.", + NSStringFromClass([self class])]; } _styleURL = styleURL; |