Bug 2004094 - PingUploadScheduler does not correctly end background tasks

glean-core/ios/Glean.xcodeproj/project.pbxproj

@@ -76,6 +76,12 @@
 		CD9DA7852BC809BE00E18F31 /* ObjectMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD9DA7842BC809BE00E18F31 /* ObjectMetricTests.swift */; };
 		CDBFB4DC27C3FA520045CCB9 /* Dispatchers.swift in Sources */ = {isa = PBXBuildFile; fileRef = CDBFB4DB27C3FA520045CCB9 /* Dispatchers.swift */; };
 		CDD08C8627E21104007C8400 /* gleanFFI.h in Headers */ = {isa = PBXBuildFile; fileRef = CDD08C8427E21104007C8400 /* gleanFFI.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		EDC21B942EE20B2C0042D53E /* PingUploadSchedulerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21B932EE20B2B0042D53E /* PingUploadSchedulerTests.swift */; };
+		EDC21C842EE213F10042D53E /* BackgroundTaskScheduler.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21C832EE213EF0042D53E /* BackgroundTaskScheduler.swift */; };
+		EDC21C892EE221EA0042D53E /* MockBackgroundTaskScheduler.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21C882EE221EA0042D53E /* MockBackgroundTaskScheduler.swift */; };
+		EDC21C8B2EE224060042D53E /* MockGleanUploadTaskProviderProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21C8A2EE224060042D53E /* MockGleanUploadTaskProviderProtocol.swift */; };
+		EDC21C8D2EE228BB0042D53E /* MockPingUploader.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21C8C2EE228BA0042D53E /* MockPingUploader.swift */; };
+		EDC21C8F2EE22CCB0042D53E /* GleanUploadTaskProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EDC21C8E2EE22CCA0042D53E /* GleanUploadTaskProvider.swift */; };
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
@@ -162,6 +168,12 @@
 		CDBFB4DB27C3FA520045CCB9 /* Dispatchers.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Dispatchers.swift; sourceTree = "<group>"; };
 		CDD08C8427E21104007C8400 /* gleanFFI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = gleanFFI.h; sourceTree = "<group>"; };
 		CDD08C8527E21104007C8400 /* glean.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = glean.swift; sourceTree = "<group>"; };
+		EDC21B932EE20B2B0042D53E /* PingUploadSchedulerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PingUploadSchedulerTests.swift; sourceTree = "<group>"; };
+		EDC21C832EE213EF0042D53E /* BackgroundTaskScheduler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackgroundTaskScheduler.swift; sourceTree = "<group>"; };
+		EDC21C882EE221EA0042D53E /* MockBackgroundTaskScheduler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockBackgroundTaskScheduler.swift; sourceTree = "<group>"; };
+		EDC21C8A2EE224060042D53E /* MockGleanUploadTaskProviderProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockGleanUploadTaskProviderProtocol.swift; sourceTree = "<group>"; };
+		EDC21C8C2EE228BA0042D53E /* MockPingUploader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockPingUploader.swift; sourceTree = "<group>"; };
+		EDC21C8E2EE22CCA0042D53E /* GleanUploadTaskProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GleanUploadTaskProvider.swift; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -225,6 +237,8 @@
 			children = (
 				60006D2A2E0DB174000C14E3 /* PingUploadScheduler.swift */,
 				6003AE242E0C6E9400BC8E07 /* PingUploader.swift */,
+				EDC21C832EE213EF0042D53E /* BackgroundTaskScheduler.swift */,
+				EDC21C8E2EE22CCA0042D53E /* GleanUploadTaskProvider.swift */,
 				1F605894231489AB00307A9F /* HttpPingUploader.swift */,
 			);
 			path = Net;
@@ -312,6 +326,7 @@
 		BF3DE39E2243A2F20018E23F /* GleanTests */ = {
 			isa = PBXGroup;
 			children = (
+				EDC21C872EE221E60042D53E /* Mocks */,
 				C27E756429D4B56500C6AADD /* Utils */,
 				1F39E7B1239F0741009B13B3 /* Debug */,
 				1FB8F8392326EBA500618E47 /* Config */,
@@ -385,6 +400,7 @@
 			children = (
 				8AF3BEA22E60EED50007A9ED /* makeCapablePingUploadRequestForTests.swift */,
 				8AF3BEA02E60EC620007A9ED /* PingUploaderTests.swift */,
+				EDC21B932EE20B2B0042D53E /* PingUploadSchedulerTests.swift */,
 				60691AEA28DD0BF200BDF31A /* BaselinePingTests.swift */,
 				CD3682F22CAC10FE00B02F04 /* RidealongPingTests.swift */,
 				BF80AA5E2399305200A8B172 /* DeletionRequestPingTests.swift */,
@@ -410,6 +426,16 @@
 			path = uniffi;
 			sourceTree = "<group>";
 		};
+		EDC21C872EE221E60042D53E /* Mocks */ = {
+			isa = PBXGroup;
+			children = (
+				EDC21C882EE221EA0042D53E /* MockBackgroundTaskScheduler.swift */,
+				EDC21C8A2EE224060042D53E /* MockGleanUploadTaskProviderProtocol.swift */,
+				EDC21C8C2EE228BA0042D53E /* MockPingUploader.swift */,
+			);
+			path = Mocks;
+			sourceTree = "<group>";
+		};
 /* End PBXGroup section */
 
 /* Begin PBXHeadersBuildPhase section */
@@ -610,6 +636,7 @@
 			files = (
 				CD0CADA427E216810015A997 /* glean.swift in Sources */,
 				CDBFB4DC27C3FA520045CCB9 /* Dispatchers.swift in Sources */,
+				EDC21C8F2EE22CCB0042D53E /* GleanUploadTaskProvider.swift in Sources */,
 				1F6058932314863400307A9F /* Configuration.swift in Sources */,
 				BF2E57052334B77D00364D92 /* EventMetric.swift in Sources */,
 				BF10008023548B0500064051 /* MemoryDistributionMetric.swift in Sources */,
@@ -627,6 +654,7 @@
 				1FB70AEF23301C1D00C7CF09 /* Logger.swift in Sources */,
 				CD062129284110970006370D /* TextMetric.swift in Sources */,
 				1F6A8FF0233C049D007837D5 /* BooleanMetric.swift in Sources */,
+				EDC21C842EE213F10042D53E /* BackgroundTaskScheduler.swift in Sources */,
 				1F6A8FF4233C0A91007837D5 /* DatetimeMetric.swift in Sources */,
 				AC06529C26E032E300D92D5E /* QuantityMetric.swift in Sources */,
 				BFFE18382350A5F50068D97B /* TimingDistributionMetric.swift in Sources */,
@@ -655,7 +683,10 @@
 				60691AEB28DD0BF200BDF31A /* BaselinePingTests.swift in Sources */,
 				BF890561232BC227003CA2BA /* StringMetricTests.swift in Sources */,
 				CD0F7CC226F0F28900EDA6A4 /* UrlMetricTests.swift in Sources */,
+				EDC21B942EE20B2C0042D53E /* PingUploadSchedulerTests.swift in Sources */,
+				EDC21C892EE221EA0042D53E /* MockBackgroundTaskScheduler.swift in Sources */,
 				BFCBD6AB246D55CC0032096D /* TestUtils.swift in Sources */,
+				EDC21C8B2EE224060042D53E /* MockGleanUploadTaskProviderProtocol.swift in Sources */,
 				AC06529E26E034BF00D92D5E /* QuantityMetricTypeTest.swift in Sources */,
 				CD3682F32CAC110300B02F04 /* RidealongPingTests.swift in Sources */,
 				1F58921223C923C4007D2D80 /* MetricsPingSchedulerTests.swift in Sources */,
@@ -664,6 +695,7 @@
 				BF80AA5B2399301300A8B172 /* HttpPingUploaderTests.swift in Sources */,
 				1FB8F8382326EABD00618E47 /* ConfigurationTests.swift in Sources */,
 				BF6C53B4232F872B00E3B43A /* PingTests.swift in Sources */,
+				EDC21C8D2EE228BB0042D53E /* MockPingUploader.swift in Sources */,
 				1F6A8FF6233C1555007837D5 /* DatetimeMetricTypeTests.swift in Sources */,
 				BF10008223548C4400064051 /* MemoryDistributionMetricTests.swift in Sources */,
 				BF43A8CD232A615200545310 /* CounterMetricTests.swift in Sources */,

glean-core/ios/Glean/Dispatchers.swift

@@ -5,9 +5,9 @@
 import Foundation
 
 /// This class manages a single background operation queue.
-class Dispatchers {
+public final class Dispatchers: Sendable {
     /// This is the shared singleton access to the Glean Dispatchers
-    static let shared = Dispatchers()
+    public static let shared = Dispatchers()
 
     // Don't let other instances be created, we only want singleton access through the static `shared`
     // property
@@ -17,14 +17,14 @@ class Dispatchers {
     // It is currently set to be a serial queue by setting the `maxConcurrentOperationsCount` to 1.
     // This queue is intended for API operations that are subject to the behavior and constraints of the
     // API.
-    lazy var serialOperationQueue: OperationQueue = {
+    let serialOperationQueue: OperationQueue = {
         var queue = OperationQueue()
         queue.name = "Glean serial dispatch queue"
         queue.maxConcurrentOperationCount = 1
         return queue
     }()
 
-    func launchAsync(block: @escaping () -> Void) {
+    func launchAsync(block: @escaping @Sendable () -> Void) {
         serialOperationQueue.addOperation(BlockOperation(block: block))
     }
 }

glean-core/ios/Glean/Net/BackgroundTaskScheduler.swift

@@ -0,0 +1,16 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import Foundation
+
+public protocol BackgroundTaskScheduler: Sendable {
+    func beginBackgroundTask(
+        withName taskName: String?,
+        expirationHandler handler: (@MainActor @Sendable () -> Void)?
+    ) -> UIBackgroundTaskIdentifier
+
+    func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier)
+}
+
+extension UIApplication: BackgroundTaskScheduler {}

glean-core/ios/Glean/Net/GleanUploadTaskProvider.swift

@@ -0,0 +1,18 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import Foundation
+
+public protocol GleanUploadTaskProviderProtocol: Sendable {
+    func getUploadTask() -> PingUploadTask
+}
+
+public final class GleanUploadTaskProvider: GleanUploadTaskProviderProtocol {
+    public init() {}
+
+    /// Calls the global `gleanGetUploadTask` and returns a `PingUploadTask`.
+    public func getUploadTask() -> PingUploadTask {
+        return gleanGetUploadTask()
+    }
+}

glean-core/ios/Glean/Net/PingUploadScheduler.swift

@@ -30,6 +30,9 @@ public class PingUploadScheduler {
     let httpUploader: PingUploader
     let httpEndpoint: String
 
+    let backgroundTaskScheduler: BackgroundTaskScheduler
+    let gleanUploadTaskProvider: GleanUploadTaskProviderProtocol
+
     // This struct is used for organizational purposes to keep the class constants in a single place
     struct Constants {
         // Since ping file names are UUIDs, this matches UUIDs for filtering purposes
@@ -46,9 +49,17 @@ public class PingUploadScheduler {
     ///
     /// - parameters:
     ///     * configuration: The Glean `Configuration` to use which contains the endpoint and http uploader
-    public init(configuration: Configuration) {
+    ///     * backgroundTaskScheduler: The `BackgroundTaskScheduler` which starts and ends background tasks.
+    ///     * gleanUploadTaskProvider: The `GleanUploadTaskProviderProtocol` wrapping the global `gleanGetUploadTask`.
+    public init(
+        configuration: Configuration,
+        backgroundTaskScheduler: BackgroundTaskScheduler = UIApplication.shared,
+        gleanUploadTaskProvider: GleanUploadTaskProviderProtocol = GleanUploadTaskProvider()
+    ) {
         self.httpUploader = configuration.httpClient
         self.httpEndpoint = configuration.serverEndpoint
+        self.backgroundTaskScheduler = backgroundTaskScheduler
+        self.gleanUploadTaskProvider = gleanUploadTaskProvider
     }
 
     /// This function gets an upload task from Glean and, if told so, uploads the data using the http uploader
@@ -65,7 +76,7 @@ public class PingUploadScheduler {
 
             // Begin the background task and save the id. We will reuse this same background task
             // for all the ping uploads
-            backgroundTaskId = UIApplication.shared.beginBackgroundTask(
+            backgroundTaskId = self.backgroundTaskScheduler.beginBackgroundTask(
                 withName: "Glean Upload Task"
             ) {
                 // End the background task if we run out of time
@@ -75,10 +86,10 @@ public class PingUploadScheduler {
                 }
             }
 
-            while true {
+            uploadTaskLoop: while true {
                 // Limits are enforced by glean-core to avoid an infinite loop here.
                 // Whenever a limit is reached, this binding will receive `.done` and step out.
-                switch gleanGetUploadTask() {
+                switch self.gleanUploadTaskProvider.getUploadTask() {
                 case let .upload(request):
                     var body = Data(capacity: request.body.count)
                     body.append(contentsOf: request.body)
@@ -99,12 +110,12 @@ public class PingUploadScheduler {
                 case .wait(let time):
                     sleep(UInt32(time) / 1000)
                 case .done:
-                    return
+                    break uploadTaskLoop
                 }
             }
 
             if backgroundTaskId != .invalid {
-                UIApplication.shared.endBackgroundTask(backgroundTaskId)
+                self.backgroundTaskScheduler.endBackgroundTask(backgroundTaskId)
                 backgroundTaskId = .invalid
             }
         }

glean-core/ios/GleanTests/Mocks/MockBackgroundTaskScheduler.swift

@@ -0,0 +1,31 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import UIKit
+import Glean
+
+final class MockBackgroundTaskScheduler: BackgroundTaskScheduler, @unchecked Sendable {
+    let withValidTaskIdentifier: Bool
+
+    var calledBeginBackgroundTask = 0
+    var calledEndBackgroundTask = 0
+
+    init(withValidTaskIdentifier: Bool) {
+        self.withValidTaskIdentifier = withValidTaskIdentifier
+    }
+
+    func beginBackgroundTask(
+        withName taskName: String?,
+        expirationHandler handler: (@MainActor @Sendable () -> Void)?
+    ) -> UIBackgroundTaskIdentifier {
+        calledBeginBackgroundTask += 1
+        return withValidTaskIdentifier
+               ? UIBackgroundTaskIdentifier(rawValue: Int.random(in: 0...Int.max))
+               : .invalid
+    }
+
+    func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier) {
+        calledEndBackgroundTask += 1
+    }
+}

glean-core/ios/GleanTests/Mocks/MockGleanUploadTaskProviderProtocol.swift

@@ -0,0 +1,25 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import UIKit
+import Glean
+
+final class MockGleanUploadTaskProviderProtocol: GleanUploadTaskProviderProtocol, @unchecked Sendable {
+    let task: PingUploadTask
+    var didCallGetUploadTask = false
+
+    init(returningTask: PingUploadTask) {
+        self.task = returningTask
+    }
+
+    func getUploadTask() -> PingUploadTask {
+        // Always return the expected task once, and then `.done` thereafter
+        if didCallGetUploadTask {
+            return .done(unused: 0)
+        } else {
+            didCallGetUploadTask = true
+            return task
+        }
+    }
+}

glean-core/ios/GleanTests/Mocks/MockPingUploader.swift

@@ -0,0 +1,21 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import XCTest
+
+@testable import Glean
+
+final class MockPingUploader: PingUploader, @unchecked Sendable {
+    var uploadRequested: (CapablePingUploadRequest) -> Void
+
+    init(uploadRequested: @escaping (CapablePingUploadRequest) -> Void) {
+        self.uploadRequested = uploadRequested
+    }
+
+    func upload(request: CapablePingUploadRequest, callback: @escaping @Sendable (UploadResult) -> Void) {
+        // Skip calling the regular callback for this mock's testing purposes; the global Glean object may not
+        // be initialized (which will cause a crash).
+        uploadRequested(request)
+    }
+}