From 8a975e5ea92db29c969de9e67f733b899821d070 Mon Sep 17 00:00:00 2001 From: Luis Nachtigall <31982496+LeLunZ@users.noreply.github.com> Date: Fri, 10 Apr 2026 17:56:35 +0200 Subject: [PATCH] refactor(mobile): cleanup iOS image loading pipeline (#27672) * refactor: replace DispatchQueue + DispatchSemaphore with OperationQueue for image processing * implement RequestRegistry and UnfairLock for managing cancellable requests * implement requests registry for local and remote image processing * remove Cancellable protocol and cancel method from request registry * refactor: introduce ImageRequest base class with unified cancellation and finish helpers * refactor: add get method to RequestRegistry and streamline request removal in image processing * add guard to cancel to prevent double onCancel calls * fix duplicate code merge issue * refactor(ios): enhance finish method to return callback status * remove unfitting methods form ImageRequest.swift and fix memory issue * revert bad merge * refactor(ios): resolve cancellation issues * refactor(ios): streamline image request completion handling * add return statements * refactor(ios): simplify image request cancellation and registry handling --------- Co-authored-by: Alex --- mobile/ios/Runner/Images/ImageRequest.swift | 27 ++++++ .../ios/Runner/Images/LocalImagesImpl.swift | 54 ++++------- .../ios/Runner/Images/RemoteImagesImpl.swift | 97 +++++++++---------- 3 files changed, 91 insertions(+), 87 deletions(-) diff --git a/mobile/ios/Runner/Images/ImageRequest.swift b/mobile/ios/Runner/Images/ImageRequest.swift index 5c05bafb83..6c8bb04c70 100644 --- a/mobile/ios/Runner/Images/ImageRequest.swift +++ b/mobile/ios/Runner/Images/ImageRequest.swift @@ -1,5 +1,32 @@ import Foundation +class ImageRequest: @unchecked Sendable { + private struct State: Sendable { + var isCancelled = false + } + + let completion: @Sendable (Result<[String: Int64]?, any Error>) -> Void + private let state: Mutex + + var isCancelled: Bool { + get { + state.withLock { $0.isCancelled } + } + set { + state.withLock { $0.isCancelled = newValue } + } + } + + init(completion: @escaping @Sendable (Result<[String: Int64]?, any Error>) -> Void) { + self.state = Mutex(State()) + self.completion = completion + } + + func cancel() { + isCancelled = true + } +} + struct RequestRegistry: ~Copyable, Sendable { private let requests = Mutex<[Int64: T]>([:]) diff --git a/mobile/ios/Runner/Images/LocalImagesImpl.swift b/mobile/ios/Runner/Images/LocalImagesImpl.swift index 713a24a2dd..9c142da054 100644 --- a/mobile/ios/Runner/Images/LocalImagesImpl.swift +++ b/mobile/ios/Runner/Images/LocalImagesImpl.swift @@ -3,21 +3,6 @@ import Flutter import MobileCoreServices import Photos -class LocalImageRequest { - weak var operation: Operation? - var isCancelled = false - let callback: (Result<[String: Int64]?, any Error>) -> Void - - init(callback: @escaping (Result<[String: Int64]?, any Error>) -> Void) { - self.callback = callback - } - - func cancel() { - isCancelled = true - operation?.cancel() - } -} - class LocalImageApiImpl: LocalImageApi { private static let imageManager = PHImageManager.default() private static let fetchOptions = { @@ -36,9 +21,9 @@ class LocalImageApiImpl: LocalImageApi { return requestOptions }() - private static let registry = RequestRegistry() + private static let registry = RequestRegistry() - private static var rgbaFormat = vImage_CGImageFormat( + private static let rgbaFormat = vImage_CGImageFormat( bitsPerComponent: 8, bitsPerPixel: 32, colorSpace: CGColorSpaceCreateDeviceRGB(), @@ -67,21 +52,20 @@ class LocalImageApiImpl: LocalImageApi { } func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, preferEncoded: Bool, completion: @escaping (Result<[String: Int64]?, any Error>) -> Void) { - let request = LocalImageRequest(callback: completion) + let request = ImageRequest(completion: completion) let operation = BlockOperation { if request.isCancelled { - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } guard let asset = Self.requestAsset(assetId: assetId) else { Self.registry.remove(requestId: requestId) - completion(.failure(PigeonError(code: "", message: "Could not get asset data for \(assetId)", details: nil))) - return + return request.completion(.failure(PigeonError(code: "", message: "Could not get asset data for \(assetId)", details: nil))) } if request.isCancelled { - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } if preferEncoded { @@ -100,12 +84,12 @@ class LocalImageApiImpl: LocalImageApi { ) if request.isCancelled { - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } guard let data = imageData else { Self.registry.remove(requestId: requestId) - return completion(.failure(PigeonError(code: "", message: "Could not get image data for \(assetId)", details: nil))) + return request.completion(.failure(PigeonError(code: "", message: "Could not get image data for \(assetId)", details: nil))) } let length = data.count @@ -114,15 +98,14 @@ class LocalImageApiImpl: LocalImageApi { if request.isCancelled { free(pointer) - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } - request.callback(.success([ + Self.registry.remove(requestId: requestId) + return request.completion(.success([ "pointer": Int64(Int(bitPattern: pointer)), "length": Int64(length), ])) - Self.registry.remove(requestId: requestId) - return } var image: UIImage? @@ -137,17 +120,17 @@ class LocalImageApiImpl: LocalImageApi { ) if request.isCancelled { - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } guard let image = image, let cgImage = image.cgImage else { Self.registry.remove(requestId: requestId) - return completion(.failure(PigeonError(code: "", message: "Could not get pixel data for \(assetId)", details: nil))) + return request.completion(.failure(PigeonError(code: "", message: "Could not get pixel data for \(assetId)", details: nil))) } if request.isCancelled { - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } do { @@ -155,23 +138,22 @@ class LocalImageApiImpl: LocalImageApi { if request.isCancelled { buffer.free() - return completion(ImageProcessing.cancelledResult) + return request.completion(ImageProcessing.cancelledResult) } - request.callback(.success([ + Self.registry.remove(requestId: requestId) + return request.completion(.success([ "pointer": Int64(Int(bitPattern: buffer.data)), "width": Int64(buffer.width), "height": Int64(buffer.height), "rowBytes": Int64(buffer.rowBytes), ])) - Self.registry.remove(requestId: requestId) } catch { Self.registry.remove(requestId: requestId) - return completion(.failure(PigeonError(code: "", message: "Failed to convert image for \(assetId): \(error)", details: nil))) + return request.completion(.failure(PigeonError(code: "", message: "Failed to convert image for \(assetId): \(error)", details: nil))) } } - request.operation = operation Self.registry.add(requestId: requestId, request: request) ImageProcessing.queue.addOperation(operation) } diff --git a/mobile/ios/Runner/Images/RemoteImagesImpl.swift b/mobile/ios/Runner/Images/RemoteImagesImpl.swift index 37d37f597b..de1f6dec89 100644 --- a/mobile/ios/Runner/Images/RemoteImagesImpl.swift +++ b/mobile/ios/Runner/Images/RemoteImagesImpl.swift @@ -3,27 +3,24 @@ import Flutter import MobileCoreServices import Photos -class RemoteImageRequest { - weak var task: URLSessionDataTask? +final class RemoteImageRequest: ImageRequest { + var task: URLSessionDataTask? let id: Int64 - var isCancelled = false - let completion: (Result<[String: Int64]?, any Error>) -> Void - init(id: Int64, task: URLSessionDataTask, completion: @escaping (Result<[String: Int64]?, any Error>) -> Void) { + init(id: Int64, completion: @escaping @Sendable (Result<[String: Int64]?, any Error>) -> Void) { self.id = id - self.task = task - self.completion = completion + super.init(completion: completion) } - func cancel() { - isCancelled = true + override func cancel() { + super.cancel() task?.cancel() } } class RemoteImageApiImpl: NSObject, RemoteImageApi { private static let registry = RequestRegistry() - private static var rgbaFormat = vImage_CGImageFormat( + private static let rgbaFormat = vImage_CGImageFormat( bitsPerComponent: 8, bitsPerPixel: 32, colorSpace: CGColorSpaceCreateDeviceRGB(), @@ -41,62 +38,58 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { var urlRequest = URLRequest(url: URL(string: url)!) urlRequest.cachePolicy = .returnCacheDataElseLoad + let request = RemoteImageRequest(id: requestId, completion: completion) + let task = URLSessionManager.shared.session.dataTask(with: urlRequest) { data, response, error in - Self.handleCompletion(requestId: requestId, encoded: preferEncoded, data: data, response: response, error: error) + Self.handleCompletion(request: request, encoded: preferEncoded, data: data, response: response, error: error) } - let request = RemoteImageRequest(id: requestId, task: task, completion: completion) - + request.task = task Self.registry.add(requestId: requestId, request: request) - task.resume() } - private static func handleCompletion(requestId: Int64, encoded: Bool, data: Data?, response: URLResponse?, error: Error?) { - guard let request = registry.remove(requestId: requestId) else { - return - } - - if let error = error { - if request.isCancelled || (error as NSError).code == NSURLErrorCancelled { - return request.completion(ImageProcessing.cancelledResult) - } - return request.completion(.failure(error)) - } - + private static func handleCompletion(request: RemoteImageRequest, encoded: Bool, data: Data?, response: URLResponse?, error: Error?) { if request.isCancelled { return request.completion(ImageProcessing.cancelledResult) } + if let error = error { + registry.remove(requestId: request.id) + return request.completion(.failure(error)) + } + guard let data = data else { + registry.remove(requestId: request.id) return request.completion(.failure(PigeonError(code: "", message: "No data received", details: nil))) } + if encoded { + let length = data.count + let pointer = malloc(length)! + data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length) + + if request.isCancelled { + free(pointer) + return request.completion(ImageProcessing.cancelledResult) + } + + registry.remove(requestId: request.id) + return request.completion( + .success([ + "pointer": Int64(Int(bitPattern: pointer)), + "length": Int64(length), + ])) + } + ImageProcessing.queue.addOperation { if request.isCancelled { return request.completion(ImageProcessing.cancelledResult) } - // Return raw encoded bytes when requested (for animated images) - if encoded { - let length = data.count - let pointer = malloc(length)! - data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length) - - if request.isCancelled { - free(pointer) - return request.completion(ImageProcessing.cancelledResult) - } - - return request.completion( - .success([ - "pointer": Int64(Int(bitPattern: pointer)), - "length": Int64(length), - ])) - } - guard let imageSource = CGImageSourceCreateWithData(data as CFData, nil), let cgImage = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, decodeOptions) else { + registry.remove(requestId: request.id) return request.completion(.failure(PigeonError(code: "", message: "Failed to decode image for request", details: nil))) } @@ -112,14 +105,16 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { return request.completion(ImageProcessing.cancelledResult) } - request.completion( - .success([ - "pointer": Int64(Int(bitPattern: buffer.data)), - "width": Int64(buffer.width), - "height": Int64(buffer.height), - "rowBytes": Int64(buffer.rowBytes), - ])) + registry.remove(requestId: request.id) + return request.completion( + .success([ + "pointer": Int64(Int(bitPattern: buffer.data)), + "width": Int64(buffer.width), + "height": Int64(buffer.height), + "rowBytes": Int64(buffer.rowBytes), + ])) } catch { + registry.remove(requestId: request.id) return request.completion(.failure(PigeonError(code: "", message: "Failed to convert image for request: \(error)", details: nil))) } }