Bug 2004094 - PingUploadScheduler does not correctly end background tasks#3347
Bug 2004094 - PingUploadScheduler does not correctly end background tasks#3347badboy merged 4 commits intomozilla:mainfrom
PingUploadScheduler does not correctly end background tasks#3347Conversation
|
cc @travis79 |
PingUploadScheduler does not correctly end background tasks
badboy
left a comment
There was a problem hiding this comment.
I would have prefered to split out the first two commits, as they are unrelated. But it's fine now.
Tests are failing unfortunately.
/Users/distiller/project/glean-core/ios/Glean/Net/BackgroundTaskScheduler.swift:16:1: warning: extension declares a conformance of imported type 'UIApplication' to imported protocol 'Sendable'; this will not behave correctly if the owners of 'UIKit' introduce this conformance in the future
extension UIApplication: BackgroundTaskScheduler {}
^
/Users/distiller/project/glean-core/ios/Glean/Net/BackgroundTaskScheduler.swift:16:1: note: add '@retroactive' to silence this warning
extension UIApplication: BackgroundTaskScheduler {}
^
/Users/distiller/project/glean-core/ios/Glean/Net/BackgroundTaskScheduler.swift:16:1: error: type 'UIApplication' does not conform to protocol 'BackgroundTaskScheduler'
extension UIApplication: BackgroundTaskScheduler {}
^
UIKit.UIApplication.beginBackgroundTask:3:25: note: candidate has non-matching type '(String?, (@Sendable () -> Void)?) -> UIBackgroundTaskIdentifier'
nonisolated open func beginBackgroundTask(withName taskName: String?, expirationHandler handler: (@Sendable () -> Void)? = nil) -> UIBackgroundTaskIdentifier}
I'm not the biggest fan of all that mocking and protocols. But I can see the value of testing the scheduler that way, so we can take it.
Can you squash the cleanup commits ("Add some documentation / cleanup.", "Remove unneeded throws. Swiftlint fixes.") into their respective commits?
|
Thanks @badboy! Regarding the test warnings, I understand why I wasn't seeing those locally. It's an Xcode version mismatch. You might want to look into updating your pipeline from Xcode 16.4 to Xcode 26. There were some significant strict concurrency changes between both versions (on the iOS side, we saw for a while warnings/errors in Xcode 16 that weren't in Xcode 26, and vice versa). The iOS team actually skipped 16.4 (stayed on 16.2) because of some additional issues with 16.4. I will look to see if I can resolve these without being able to reproduce them locally (since I don't have 16.4 and my internet is incredibly slow!). But, if I can't easily do that I'll just split my PR up into a fix and do minimal strict concurrency adjustments even if they will become warnings in Xcode 26. |
|
#3350 landed, updating to Xcode 26.2 -- that should probably fix the test failure here. |
Bugzilla
Dispatchersby making the typeSendable.SWIFT_STRICT_CONCURRENCY = complete.PingUploadSchedulerfor breaking out of while-switch and continuing execution to the background task handling.PingUploadSchedulermore testable with dependency injection via protocols.PingUploadScheduler.I wanted to be a good citizen and test this thoroughly since I'm not sure how to check this against my local iOS build, but if you are not happy with all the dependency injection changes that let me use mocks, I can back out those changes.