Merge pull request #738 from Esri/Caleb/Backport-Fix-Crashes

[Fix] Backport: Download Offline Resources bugs

Author: CalebRas

Shared/Supporting Files/Models/OnDemandResource.swift

@@ -33,7 +33,7 @@ final class OnDemandResource {
     }
 
     /// The progress of the on-demand resource request.
-    var progress: Progress { request.progress }
+    let progress = Progress(totalUnitCount: 100)
 
     /// A Boolean value indicating whether a resource request can be initiated.
     var isDownloadable: Bool {
@@ -47,20 +47,30 @@ final class OnDemandResource {
     private(set) var error: (any Error)?
 
     /// The on-demand resource request.
-    private let request: NSBundleResourceRequest
+    private var request: NSBundleResourceRequest
+    
+    /// A task for monitoring `request.progress` to update `self.progress`.
+    ///
+    /// This is needed because passing `request.progress` to a `ProgressView` can cause race condition crashes.
+    @ObservationIgnored private var progressTask: Task<Void, Never>?
 
     /// Initializes a request with a set of Resource Tags.
     init(tags: Set<String>) async {
-        request = NSBundleResourceRequest(tags: tags)
+        let request = NSBundleResourceRequest(tags: tags)
         request.loadingPriority = NSBundleResourceRequestLoadingPriorityUrgent
+        self.request = request
 
         let isResourceAvailable = await request.conditionallyBeginAccessingResources()
         requestState = isResourceAvailable ? .downloaded : .notStarted
     }
 
+    deinit {
+        progressTask?.cancel()
+    }
+    
     /// Cancels the on-demand resource request.
     func cancel() {
-        progress.cancel()
+        progressTask?.cancel()
         request.endAccessingResources()
         requestState = .cancelled
     }
@@ -69,8 +79,38 @@ final class OnDemandResource {
     func download() async {
         guard isDownloadable else { return }
 
-        requestState = .inProgress
+        // Recreates the request if a download has been attempted before so
+        // beginAccessingResources() can be called again without crashing.
+        if requestState == .cancelled || requestState == .error {
+            request = NSBundleResourceRequest(tags: request.tags)
+            request.loadingPriority = NSBundleResourceRequestLoadingPriorityUrgent
+            
+            error = nil
+        }
+        
+        // Monitors `request.progress` to update `self.progress`.
+        progressTask = Task { @MainActor [weak self] in
+            guard let self else { return }
+            
+            // A stream is used here because `progress.publisher` doesn't always produce values.
+            let stream = AsyncStream { continuation in
+                let observation = request.progress
+                    .observe(\.fractionCompleted, options: [.initial, .new]) { _, change in
+                        guard let newValue = change.newValue else { return }
+                        continuation.yield(newValue)
+                    }
+                continuation.onTermination = { _ in
+                    observation.invalidate()
+                }
+            }
+            
+            for await fractionCompleted in stream {
+                self.progress.completedUnitCount = Int64(fractionCompleted * 100)
+            }
+        }
+        
         do {
+            requestState = .inProgress
             try await request.beginAccessingResources()
             requestState = .downloaded
         } catch {
@@ -81,6 +121,8 @@ final class OnDemandResource {
                 cancel()
             }
         }
+        
+        progressTask?.cancel()
     }
 }
 

Shared/Supporting Files/Views/DownloadOfflineResourcesView.swift

@@ -16,70 +16,58 @@ import SwiftUI
 
 /// A view with controls for downloading the app's on-demand resources.
 struct DownloadOfflineResourcesView: View {
-    /// The samples that require offline resources.
-    let samples = SamplesApp.samples.filter(\.hasDependencies)
-    
     /// The action to dismiss the view.
     @Environment(\.dismiss) private var dismiss
 
+    /// The progress of a download-all operation.
+    @State private var downloadAllProgress: Progress?
+    
     /// The view models for the on-demand resource requests.
     @State private var onDemandResources: [String: OnDemandResource] = [:]
 
-    /// The current state of the "download all on-demand resources" request.
-    @State private var downloadAllRequestState: OnDemandResource.RequestState = .notStarted
+    /// A Boolean value indicating whether all of the `onDemandResources` have successfully downloaded.
+    private var allResourcesAreDownloaded: Bool { uniqueRequestStates == [.downloaded] }
 
-    /// A Boolean value indicating whether confirm cancel alert is showing.
-    @State private var confirmCancelAlertIsShowing = false
+    /// A Boolean value indicating whether there is an on going download-all operation.
+    private var isDownloadingAll: Bool { downloadAllProgress != nil }
 
-    /// A Boolean value indicating whether all of the `onDemandResources` have successfully downloaded.
-    private var allResourcesAreDownloaded: Bool {
-        guard !onDemandResources.isEmpty else { return false }
-        return onDemandResources.values
-            .allSatisfy { $0.requestState == .downloaded }
-    }
+    /// A Boolean value indicating whether there is an in progress download.
+    private var isDownloadingResource: Bool { uniqueRequestStates.contains(.inProgress) }
 
-    /// Returns the on-demand resource for the given sample.
-    func resource(for sample: Sample) -> OnDemandResource? {
-        return onDemandResources[sample.name]
+    /// The distinct request states of all on-demand resources.
+    private var uniqueRequestStates: Set<OnDemandResource.RequestState> {
+        onDemandResources.values.reduce(into: Set()) { result, resource in
+            result.insert(resource.requestState)
+        }
     }
 
     var body: some View {
         NavigationStack {
             Form {
                 Section {
                     Button {
-                        downloadAllRequestState = .inProgress
+                        downloadAllProgress = Progress()
                     } label: {
                         Label {
                             Text("Download All")
                         } icon: {
-                            switch downloadAllRequestState {
-                            case .inProgress:
-                                ProgressView()
-                            case .downloaded:
+                            if let downloadAllProgress {
+                                ProgressView(downloadAllProgress)
+                                    .progressViewStyle(GaugeProgressViewStyle())
+                            } else if allResourcesAreDownloaded {
                                 Image(systemName: "checkmark.circle")
                                     .foregroundStyle(.secondary)
-                            default:
+                            } else {
                                 Image(systemName: "arrow.down.circle")
                             }
                         }
                         .frame(maxWidth: .infinity)
                     }
-                    .disabled(onDemandResources.isEmpty || downloadAllRequestState != .notStarted || allResourcesAreDownloaded)
-                    .task(id: downloadAllRequestState) {
-                        guard downloadAllRequestState == .inProgress else { return }
-                        await downloadAll()
-                    }
-                    .onChange(of: allResourcesAreDownloaded) {
-                        guard allResourcesAreDownloaded else { return }
-                        downloadAllRequestState = .downloaded
-                    }
+                    .disabled(onDemandResources.isEmpty || isDownloadingAll || allResourcesAreDownloaded)
                 }
                 Section {
-                    List(samples, id: \.name) { sample in
-                        if let resource = resource(for: sample) {
-                            DownloadOnDemandResourceView(name: sample.name, resource: resource)
-                        }
+                    List(onDemandResources.sorted(by: { $0.key < $1.key }), id: \.key) { name, resource in
+                        DownloadOnDemandResourceView(name: name, resource: resource)
                     }
                 } footer: {
                     Text("**Note**: The system may purge downloads at any time.")
@@ -89,29 +77,24 @@ struct DownloadOfflineResourcesView: View {
             .navigationBarTitleDisplayMode(.inline)
             .toolbar {
                 ToolbarItem(placement: .confirmationAction) {
-                    Button("Done") {
-                        if onDemandResources.values.allSatisfy({ $0.requestState != .inProgress }) {
-                            dismiss()
-                        } else {
-                            confirmCancelAlertIsShowing = true
-                        }
-                    }
-                    .alert("Cancel downloads?", isPresented: $confirmCancelAlertIsShowing) {
-                        Button("Don't Cancel", role: .cancel, action: {})
-                        
+                    if isDownloadingResource {
                         Button("Cancel") {
                             for resource in onDemandResources.values where resource.requestState == .inProgress {
                                 resource.cancel()
                             }
                             dismiss()
                         }
+                    } else {
+                        Button("Done") {
+                            dismiss()
+                        }
                     }
                 }
             }
             .task {
                 guard onDemandResources.isEmpty else { return }
                 onDemandResources = await withTaskGroup { group in
-                    for sample in samples {
+                    for sample in SamplesApp.samples where sample.hasDependencies {
                         group.addTask {
                             let resource = await OnDemandResource(tags: sample.odrTags)
                             return (sample.name, resource)
@@ -124,22 +107,30 @@ struct DownloadOfflineResourcesView: View {
                     return resources
                 }
             }
+            .task(id: isDownloadingAll) {
+                guard isDownloadingAll else { return }
+                await downloadAll()
+            }
+            .onChange(of: isDownloadingResource) {
+                guard isDownloadingAll, !isDownloadingResource else { return }
+                downloadAllProgress = nil
+            }
         }
     }
 
     /// Downloads all of the on-demand resources that haven't started a request yet.
     /// - Note: The system may purge the resources at any time after the request object is deallocated.
     private func downloadAll() async {
         await withTaskGroup { group in
-            for resource in onDemandResources.values where resource.isDownloadable {
-                group.addTask(operation: resource.download)
+            for resource in onDemandResources.values {
+                if resource.isDownloadable {
+                    group.addTask(operation: resource.download)
+                    downloadAllProgress?.addChildUnits(resource.progress)
+                } else if resource.requestState == .inProgress {
+                    downloadAllProgress?.addChildUnits(resource.progress)
+                }
             }
         }
-        
-        // Automatically dismisses the view if all of the resources have downloaded successfully.
-        if allResourcesAreDownloaded {
-            dismiss()
-        }
     }
 }
 
@@ -205,3 +196,12 @@ private struct GaugeProgressViewStyle: ProgressViewStyle {
         }
     }
 }
+
+private extension Progress {
+    /// Adds a process object and its unit count as a suboperation of a progress tree.
+    /// - Parameter progress: The progress instance to add to the progress tree.
+    func addChildUnits(_ progress: Progress) {
+        addChild(progress, withPendingUnitCount: progress.totalUnitCount)
+        totalUnitCount += progress.totalUnitCount
+    }
+}