-
Notifications
You must be signed in to change notification settings - Fork 285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/bg fetch #1303
base: trunk
Are you sure you want to change the base?
Feature/bg fetch #1303
Conversation
Okay, so it turns out when I leave iOS alone and let it do it's thing the BG Fetch does happen when iOS is ready for it. I was able to see it do a few cycles of refreshing in the background and from what I am seeing in the console logs it seems to be working as expected. The app wakes up, reconnects to the web socket, does some updating, and sits quietly till we shut it down. The way I tested this was:
|
private func handleAppRefresh(task: BGAppRefreshTask) { | ||
NSLog("Did fire handle app refresh") | ||
guard BuildConfiguration.current == .debug else { | ||
return | ||
} | ||
|
||
NSLog("Background refresh intiated") | ||
scheduleAppRefresh() | ||
|
||
DispatchQueue.main.asyncAfter(deadline: .now() + BackgroundRefreshConstants.timeOut) { | ||
NSLog("Background refresh finishing") | ||
task.setTaskCompleted(success: true) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also set an expiration handler, because according to Apple Docs:
If you don’t set an expiration handler, the system will mark your task as complete and unsuccessful instead of sending a warning.
If we set an expiration handler I think there is no reason to have our own timeout.
Other ideas:
- We can extract all BG related code to a separate class to clean up app delegate
- To avoid always waiting for BG task to expire we can try to be smart: re-set a timer for x seconds on every change that we receive (
- (void)bucket:(SPBucket *)bucket didChangeObjectForKey:(NSString *)key....
). So when all changes are received and processed, we wait a bit and finish BG task.
You can trigger an installable build for these changes by visiting CircleCI here. |
var finished: Bool = false { | ||
didSet { | ||
if finished == true { | ||
handler?() | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to track finished
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A totally. The finished variable was something I had put in because I was having issues making an instance of the backgroundFetchManager in the app delegate. Was trying to use changing it to trigger ending the refresh, but using the handler instead can serve that same purpose. I have removed the finished var.
Thanks for the suggestions @eshurakov I have updated the PR per these suggestions. Did some more testing today and it is working for me again! Seems that I made iOS mad yesterday by not shutting down the BG task |
Fix
This is a continuation of the original PR #1297 I moved the changes to a new branch, so created a new PR to isolate those features from the widgets changes.
Note that in this new PR I have changed what the fetch is doing. Now the fetch simply wakes the app and lets it run for a time, shutting down before we hit the apple limit. I am still testing if it is actually doing the bg fetch on it's own. If triggered from lldb everything seems to work fine, but iOS hasn't wanted to do background updates for me today, so still trying to see how it responds. Will comment as I know more about what happens if I can get it to actually log a fetch
This is a draft PR for adding background sync back into SN iOS. At this point all this does is implement the BackgroundTasks API into the app and setup Simperium to do a background sync every 30 minutes or more (sync will be triggered by apple when they feel like it, but not before 30 minutes)
Test
The background sync will print to the logs, so there are two options for testing. First, run the simulator to a device (must be at least iOS 13) and leave it alone for a while. As mentioned above the earliest begin date on the refresh is 30 minutes, if you want to make that faster go to
@objc func scheduleAppRefresh() { let request = BGAppRefreshTaskRequest(identifier: BackgroundRefreshConstants.identifier) request.earliestBeginDate = Date(timeIntervalSinceNow: BackgroundRefreshConstants.earliestBeginDate) do { try BGTaskScheduler.shared.submit(request) } catch { print("Couldn't schedule app refersh: \(error)") } }
and change request.earliestBeginDate to Date(timeIntervalSinceNow: 60) which will make it happen in a minute or more. After giving it some time you should see some events appear in the logs for showing how the background sync went
OR,
e -l objc -- (void)[[BGTaskScheduler sharedScheduler] _simulateLaunchForTaskWithIdentifier:@"com.codality.NotationalFlow.backgroundRefresh"]
Review
(Required) Add instructions for reviewers. For example:
Release
(Required) Add a concise statement to
RELEASE-NOTES.txt
if the changes should be included in release notes. Include details about updating the notes in this section. For example:If the changes should not be included in release notes, add a statement to this section. For example: