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

MGLOfflineStorage iCloud Backup #5112

Closed
suvov opened this issue May 24, 2016 · 9 comments · Fixed by #5124
Closed

MGLOfflineStorage iCloud Backup #5112

suvov opened this issue May 24, 2016 · 9 comments · Fixed by #5124
Assignees
Labels
bug iOS Mapbox Maps SDK for iOS offline
Milestone

Comments

@suvov
Copy link

suvov commented May 24, 2016

We're using MGLOfflineStorage to download offline maps as shown in your example code. And it seems that downloaded data gets backed up to iCloud. Which is against Apple guidelines. Actually we found it out after being rejected.

Steps to reproduce:

・Check iCloud backup size for an app in settings before downloading offline region.
・Check iCloud backup size after

@tobrun tobrun added the iOS Mapbox Maps SDK for iOS label May 24, 2016
@friedbunny friedbunny added the bug label May 24, 2016
@friedbunny
Copy link
Contributor

friedbunny commented May 24, 2016

Thanks for the report.

We already try to exclude offline downloads and ambient cache from backups — that happened in #4493 and v3.2.0, the first release that offline was available.

This appears to be insufficient for some applications — in my testing, an enterprise app did not backup offline data to iCloud, but an App Store app did.

/cc @1ec5

@1ec5 1ec5 added this to the ios-v3.3.0 milestone May 24, 2016
@1ec5
Copy link
Contributor

1ec5 commented May 24, 2016

#4493 excluded the cache from backups using NSURLIsExcludedFromBackupKey, as recommended in this Technical Q&A. However, the documentation for that option also says:

You can use this property to exclude cache and other app support files which are not needed in a backup. Some operations commonly made to user documents cause this property to be reset to false; consequently, do not use this property on user documents.

We used to store the cache in Caches, which does not get backed up, but as explained in #4371, we moved it to Application Support so that offline downloads wouldn’t get purged by the system.

@suvov, did you upgrade your application from iOS SDK v3.1.x? Looking through the code, I wonder if what happened is that we set that flag on a nonexistent cache, then moved the legacy cache from Caches to the new location in Application Support. We should reverse the order so that the flag is set on the cache no matter where it came from. Hopefully that’s the only case in which the cache could be backed up.

/cc @jfirebaugh

@friedbunny
Copy link
Contributor

friedbunny commented May 24, 2016

@1ec5 I just retested the App Store app (that had been updated from some earlier version to v3.2.0) and a fresh install still uploaded the cache to iCloud.

Another option may be to create our own subdirectory and set NSURLIsExcludedFromBackupKey on that (rather than individual files).

@1ec5
Copy link
Contributor

1ec5 commented May 24, 2016

I’m not sure how that’s any different from setting it on the one file we create, but it’s worth a try. The migration code is going to get a bit messier, though.

@1ec5
Copy link
Contributor

1ec5 commented May 24, 2016

Actually, that makes sense: in a fresh installation, the cache doesn’t exist until after the mbgl::DefaultFileSource is initialized. So let’s set NSURLIsExcludedFromBackupKey after initializing mbgl::DefaultFileSource.

@friedbunny friedbunny self-assigned this May 24, 2016
@friedbunny
Copy link
Contributor

friedbunny commented May 24, 2016

It’s cumbersome to test, but I believe this could be limited to the first launch of an app. NSURLIsExcludedFromBackupKey is definitely not set the first time the database is initialized on v3.2.x and, following these steps, I was able to backup to iCloud without including the database:

  1. Delete app.
  2. Turn off iCloud backup for app.
  3. Manually trigger iCloud backup.
  4. Download app.
  5. Launch, kill, and then relaunch the app.
  6. Download offline region.
  7. Re-enable iCloud backup for app.
  8. Trigger iCloud backup.
  9. App backup size does not include offline database.

😬

@friedbunny
Copy link
Contributor

@suvov Thanks again for reporting this issue. When we put out another pre-release build later this week, it would be great if you could attempt to verify that our patch actually fixes the problem (because it’s not such an easy thing to verify!).

@suvov
Copy link
Author

suvov commented May 26, 2016

@friedbunny Sure, will try it. Thank you for offline support in first place.

@1ec5
Copy link
Contributor

1ec5 commented Jul 7, 2016

v3.3.0-rc.1 is out.

ChrisLoer added a commit that referenced this issue Aug 25, 2017
Ports fix for GL JS issue #5112.
Line label projection can't be based on tile geometry that's behind the plane of the camera.
The relevant tests are still ignored because the overzoomed collision behavior is different between native and JS.
ChrisLoer added a commit that referenced this issue Aug 25, 2017
Ports fix for GL JS issue #5112.
Line label projection can't be based on tile geometry that's behind the plane of the camera.
The relevant tests are still ignored because the overzoomed collision behavior is different between native and JS.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS offline
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants