Create BGContinuedProcessingTask to upload media
#24891
Conversation
| <string>spectrum-'22-icon-app-83.5x83.5</string> | ||
| <string>spectrum-'22-icon-app-60x60</string> | ||
| <string>spectrum-'22-icon-app-76x76</string> | ||
| <string>spectrum-'22-icon-app-83.5x83.5</string> |
There was a problem hiding this comment.
Xcode made these changes. Xcode also made the original changes. Somehow, Xcode decides to revert changes that were made by itself...
There was a problem hiding this comment.
What version of Xcode are you on?
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 29235 | |
| Version | PR #24891 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 1e19fd0 | |
| Installation URL | 2vpnkn5hq25ug |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 29235 | |
| Version | PR #24891 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 1e19fd0 | |
| Installation URL | 6k1sseh9ahdjo |
There was a problem hiding this comment.
I'm glad we're doing this, but I'm concerned about how many errors we're ignoring.
Any IO error shouldn't be ignored, otherwise we end up with impossible-to-debug issues.
I'm also curious what happens if a user schedules an image for upload then immediately deletes it before it starts uploading (or while it's in progress)
|
Version |
There was a problem hiding this comment.
It looks pretty neat! LGTM, but disclaimer I'm not familiar with this API.
I left one main comment about actor vs MainActor and I'd also consider monitoring using MediaCoordinator (latest example).
In terms of features, I'm not sure it should be shown for quick operations like uploading a site image or a featured image for a post – it may be too overwhelming, and other apps don't do this. I think it should be reserved only for actually long background operations like uploading large videos or multiple files.
| <string>spectrum-'22-icon-app-83.5x83.5</string> | ||
| <string>spectrum-'22-icon-app-60x60</string> | ||
| <string>spectrum-'22-icon-app-76x76</string> | ||
| <string>spectrum-'22-icon-app-83.5x83.5</string> |
There was a problem hiding this comment.
What version of Xcode are you on?
|
|
||
| @available(iOS 26.0, *) | ||
| /// Utilize `BGContinuedProcessingTask` to show the uploading media activity. | ||
| private actor ConcreteMediaUploadBackgroundTracker: MediaUploadBackgroundTracker { |
There was a problem hiding this comment.
Mixing actor isolation with @MainActor functions (updateMessaging, updateResult) adds complexity. I would suggest to synchronize ConcreteMediaUploadBackgroundTracker itself on the main thread as it doesn't perform any work and potentially creates more CPU overhead by adding context switches and by re-reading stuff from the Core Data. MediaCoordinator, PostMediaUploadsViewModel, etc are also all synchronized on main also for simplicity.
| private var state: BGTaskState = .idle | ||
|
|
||
| private init?() { | ||
| let taskId = (Bundle.main.infoDictionary?["BGTaskSchedulerPermittedIdentifiers"] as? [String])?.first { |
There was a problem hiding this comment.
(nit) I'd suggest making init non-optional to remove the need for the code using this class to deal with the optional.
| } | ||
|
|
||
| let success = mediaItems.allSatisfy { $0.remoteStatus == .sync } | ||
| await setTaskCompleted(success: success) |
There was a problem hiding this comment.
(not) this is the kind of complexity – one function with three concurrent code sections – I mentioned earlier that could be easily eliminated by synchronizing the class on main. You may end up code sections from updateMessaging and updateResult being intertwined in any order, so it's hard to reason about.
Description
Here is a recording.
ScreenRecording_09-26-2025.22-53-19_1.MP4