Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Fix hang observing user defaults on background queue #4745

Merged
merged 3 commits into from
Apr 19, 2016
Merged

Fix hang observing user defaults on background queue #4745

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d76ef6e
[ios] Fixed hang observing user defaults on background queue
1ec5 Apr 19, 2016
e7ddaa9
[ios] Assert that defaults change is on main thread
1ec5 Apr 19, 2016
3c4049c
[ios] Toggle telemetry on main queue
1ec5 Apr 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Mapbox welcomes participation and contributions from everyone. If you’d like
- The user dot now moves smoothly between user location updates while user location tracking is disabled. ([#1582](https://github.com/mapbox/mapbox-gl-native/pull/1582))
- User location heading updates now resume properly when an app becomes active again. ([#4674](https://github.com/mapbox/mapbox-gl-native/pull/4674))
- Fixed an issue preventing KVO change notifications from being generated on MGLMapView’s `userTrackingMode` key path when `-setUserTrackingMode:animated:` is called. ([#4724](https://github.com/mapbox/mapbox-gl-native/pull/4724))
- Fixed a hang that could occur if the host application attempts to set user defaults on a background queue. ([#4745](https://github.com/mapbox/mapbox-gl-native/pull/4745))
- Added a `-reloadStyle:` action to MGLMapView to force a reload of the current style. ([#4728](https://github.com/mapbox/mapbox-gl-native/pull/4728))
- A more specific user agent string is now sent with style and tile requests. ([#4012](https://github.com/mapbox/mapbox-gl-native/pull/4012))
- Mapbox Telemetry is automatically disabled while the host application is running in the iOS Simulator. ([#4726](https://github.com/mapbox/mapbox-gl-native/pull/4726))
Expand Down
19 changes: 7 additions & 12 deletions platform/ios/src/MGLMapboxEvents.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ @interface MGLMapboxEvents () <MGLLocationManagerDelegate>
@end

@implementation MGLMapboxEvents {
id _userDefaultsObserver;
NSString *_instanceID;
}

Expand Down Expand Up @@ -199,17 +198,8 @@ - (instancetype) init {
self.canEnableDebugLogging = YES;
}


// Watch for changes to telemetry settings by the user
__weak MGLMapboxEvents *weakSelf = self;
_userDefaultsObserver = [[NSNotificationCenter defaultCenter] addObserverForName:NSUserDefaultsDidChangeNotification
object:nil
queue:[NSOperationQueue mainQueue]
usingBlock:
^(NSNotification *notification) {
MGLMapboxEvents *strongSelf = weakSelf;
[strongSelf pauseOrResumeMetricsCollectionIfRequired];
}];
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(userDefaultsDidChange:) name:NSUserDefaultsDidChangeNotification object:nil];

[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(pauseOrResumeMetricsCollectionIfRequired) name:UIApplicationDidEnterBackgroundNotification object:nil];
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(pauseOrResumeMetricsCollectionIfRequired) name:UIApplicationWillEnterForegroundNotification object:nil];
Expand All @@ -233,7 +223,7 @@ + (nullable instancetype)sharedManager {
}

- (void)dealloc {
[[NSNotificationCenter defaultCenter] removeObserver:_userDefaultsObserver];
[[NSNotificationCenter defaultCenter] removeObserver:self];
[self pauseMetricsCollection];
}

Expand All @@ -249,6 +239,11 @@ - (NSString *)instanceID {
return _instanceID;
}

- (void)userDefaultsDidChange:(NSNotification *)notification {
MGLAssertIsMainThread();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the assert here? Could pauseOrResumeMetricsCollectionIfRequired not just be dispatched back onto main thread if needed instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed in 3c4049c.

[self pauseOrResumeMetricsCollectionIfRequired];
}

- (void)pauseOrResumeMetricsCollectionIfRequired {
// Prevent blue status bar when host app has `when in use` permission only and it is not in foreground
if ([CLLocationManager authorizationStatus] == kCLAuthorizationStatusAuthorizedWhenInUse &&
Expand Down