Skip to content

ImageTaskDelegate #564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kean merged 10 commits into from
Jun 17, 2022
Merged

ImageTaskDelegate #564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9eae059
Remove ImageTaskDelegate calls from data(for:)
kean Jun 17, 2022
ea12a4a
Remove ImageTaskEvent.priorityUpdated
kean Jun 17, 2022
3d87b6e
Make ImageTaskDelegate Sendable
kean Jun 17, 2022
4ed41bd
ImagePipelineDelegate inherits from ImageTaskDelegate and ImageTaskEv…
kean Jun 17, 2022
ac920e6
Add imageTaskCreated event
kean Jun 17, 2022
ccf6257
Add isConfined option to startImageTask
kean Jun 17, 2022
9601b6f
Fix memory management tests
kean Jun 17, 2022
cf25598
Call imageTaskDidCancel on callback queue
kean Jun 17, 2022
4933489
Move isDataTask to ImageTaskContext
kean Jun 17, 2022
6912511
Add note about next steps
kean Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Nuke.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@
0C95FD542571B278008D4FC2 /* baseline.webp in Resources */ = {isa = PBXBuildFile; fileRef = 0C95FD532571B278008D4FC2 /* baseline.webp */; };
0C973E141D9FDB9F00C00AD9 /* Nuke.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = 0C9174901BAE99EE004A7905 /* Nuke.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
0C9B6E7620B9F3E2001924B8 /* ImagePipelineCoalescingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C9B6E7520B9F3E2001924B8 /* ImagePipelineCoalescingTests.swift */; };
0CA3BA63285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA3BA62285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift */; };
0CA3BA64285C15A90079A444 /* MockProgressiveDataLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CCBB533217D0B980026F552 /* MockProgressiveDataLoader.swift */; };
0CA4EC9626E67C7000BAC8E5 /* AVDataAsset.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA4EC9426E67C7000BAC8E5 /* AVDataAsset.swift */; };
0CA4EC9926E67CEC00BAC8E5 /* ImageDecoders+Default.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA4EC9826E67CEC00BAC8E5 /* ImageDecoders+Default.swift */; };
0CA4EC9B26E67D3000BAC8E5 /* ImageDecoders+Empty.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA4EC9A26E67D3000BAC8E5 /* ImageDecoders+Empty.swift */; };
Expand Down Expand Up @@ -484,6 +486,7 @@
0C95FD532571B278008D4FC2 /* baseline.webp */ = {isa = PBXFileReference; lastKnownFileType = file; path = baseline.webp; sourceTree = "<group>"; };
0C97DD9E284C00EA00F55FDA /* Nuke 11 Migration Guide.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = "Nuke 11 Migration Guide.md"; sourceTree = "<group>"; };
0C9B6E7520B9F3E2001924B8 /* ImagePipelineCoalescingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImagePipelineCoalescingTests.swift; sourceTree = "<group>"; };
0CA3BA62285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImagePipelineTaskDelegateTests.swift; sourceTree = "<group>"; };
0CA4EC9426E67C7000BAC8E5 /* AVDataAsset.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AVDataAsset.swift; sourceTree = "<group>"; };
0CA4EC9826E67CEC00BAC8E5 /* ImageDecoders+Default.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ImageDecoders+Default.swift"; sourceTree = "<group>"; };
0CA4EC9A26E67D3000BAC8E5 /* ImageDecoders+Empty.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ImageDecoders+Empty.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -928,6 +931,7 @@
0C6B5BE0257010D300D763F2 /* ImagePipelineFormatsTests.swift */,
0CD37C9925BA36D5006C2C36 /* ImagePipelineLoadDataTests.swift */,
0C53C8AE263C7B1700E62D03 /* ImagePipelineDelegateTests.swift */,
0CA3BA62285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift */,
0CE6202426543EC700AAB8C3 /* ImagePipelinePublisherTests.swift */,
0C5D5A9C2724773A0056B95B /* ImagePipelineAsyncAwaitTests.swift */,
0C78A2A8263F560A0051E0FF /* ImagePipelineCacheTests.swift */,
Expand Down Expand Up @@ -1595,6 +1599,7 @@
files = (
0C2CD7C725B7C0280017018F /* XCTestCaseExtensions.swift in Sources */,
0C2CD7BA25B7C0100017018F /* MockDataLoader.swift in Sources */,
0CA3BA64285C15A90079A444 /* MockProgressiveDataLoader.swift in Sources */,
0C2CD7C825B7C0280017018F /* NukeExtensions.swift in Sources */,
0C2CD7C625B7C0280017018F /* XCTestCase+Nuke.swift in Sources */,
0C2CD7C025B7C0200017018F /* Helpers.swift in Sources */,
Expand Down Expand Up @@ -1755,6 +1760,7 @@
0CF58FF726DAAC3800D2650D /* ImageDownsampleTests.swift in Sources */,
0C211E502856328500F48AA6 /* DataLoaderTests.swift in Sources */,
0C91B0EC2438E287007F9100 /* ResizeTests.swift in Sources */,
0CA3BA63285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift in Sources */,
0C6D0A8C20E57C810037B68F /* ImagePipelineDataCacheTests.swift in Sources */,
0C68F609208A1F40007DC696 /* ImageDecoderRegistryTests.swift in Sources */,
0CE6202526543EC700AAB8C3 /* ImagePipelinePublisherTests.swift in Sources */,
Expand Down
27 changes: 12 additions & 15 deletions Sources/Nuke/ImageTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ public final class ImageTask: Hashable, CustomStringConvertible, @unchecked Send
/// The original request.
public let request: ImageRequest

let isDataTask: Bool

/// Updates the priority of the task, even if it is already running.
public func setPriority(_ priority: ImageRequest.Priority) {
pipeline?.imageTaskUpdatePriorityCalled(self, priority: priority)
Expand Down Expand Up @@ -55,11 +53,10 @@ public final class ImageTask: Hashable, CustomStringConvertible, @unchecked Send
#endif
}

init(taskId: Int64, request: ImageRequest, isDataTask: Bool, pipeline: ImagePipeline) {
init(taskId: Int64, request: ImageRequest, pipeline: ImagePipeline) {
self.taskId = taskId
self.request = request
self._priority = request.priority
self.isDataTask = isDataTask
self.pipeline = pipeline

self._isCancelled = UnsafeMutablePointer<Int32>.allocate(capacity: 1)
Expand Down Expand Up @@ -107,10 +104,12 @@ public final class ImageTask: Hashable, CustomStringConvertible, @unchecked Send

/// A protocol that defines methods that image pipeline instances call on their
/// delegates to handle task-level events.
public protocol ImageTaskDelegate: AnyObject {
public protocol ImageTaskDelegate: AnyObject, Sendable {
func imageTaskCreated(_ task: ImageTask)

/// Gets called when the task is started. The caller can save the instance
/// of the class to update the task later.
func imageTaskWillStart(_ task: ImageTask)
func imageTaskStarted(_ task: ImageTask)

/// Gets called when the progress is updated.
func imageTask(_ task: ImageTask, didUpdateProgress progress: (completed: Int64, total: Int64))
Expand All @@ -121,32 +120,30 @@ public protocol ImageTaskDelegate: AnyObject {
func imageTaskDidCancel(_ task: ImageTask)

func imageTask(_ task: ImageTask, didCompleteWithResult result: Result<ImageResponse, ImagePipeline.Error>)

func dataTask(_ task: ImageTask, didCompleteWithResult result: Result<(data: Data, response: URLResponse?), ImagePipeline.Error>)
}

extension ImageTaskDelegate {
func imageTaskWillStart(_ task: ImageTask) {
// Do nothing
public func imageTaskCreated(_ task: ImageTask) {

}

func imageTask(_ task: ImageTask, didUpdateProgress progress: (completed: Int64, total: Int64)) {
public func imageTaskStarted(_ task: ImageTask) {
// Do nothing
}

func imageTask(_ task: ImageTask, didProduceProgressiveResponse response: ImageResponse) {
public func imageTask(_ task: ImageTask, didUpdateProgress progress: (completed: Int64, total: Int64)) {
// Do nothing
}

func imageTaskDidCancel(_ task: ImageTask) {
public func imageTask(_ task: ImageTask, didProduceProgressiveResponse response: ImageResponse) {
// Do nothing
}

func imageTask(_ task: ImageTask, didCompleteWithResult result: Result<ImageResponse, ImagePipeline.Error>) {
public func imageTaskDidCancel(_ task: ImageTask) {
// Do nothing
}

func dataTask(_ task: ImageTask, didCompleteWithResult result: Result<(data: Data, response: URLResponse?), ImagePipeline.Error>) {
public func imageTask(_ task: ImageTask, didCompleteWithResult result: Result<ImageResponse, ImagePipeline.Error>) {
// Do nothing
}
}
103 changes: 61 additions & 42 deletions Sources/Nuke/Pipeline/ImagePipeline.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class ImagePipeline: @unchecked Sendable {
let delegate: any ImagePipelineDelegate // swiftlint:disable:this all
let imageCache: ImageCache?

private var tasks = [ImageTask: TaskSubscription]()
private var tasks = [ImageTask: ImageTaskContext]()

private let tasksLoadData: TaskPool<ImageLoadKey, (Data, URLResponse?), Error>
private let tasksLoadImage: TaskPool<ImageLoadKey, ImageResponse, Error>
Expand Down Expand Up @@ -108,8 +108,9 @@ public final class ImagePipeline: @unchecked Sendable {

/// Loads an image for the given request.
public func image(for request: any ImageRequestConvertible, delegate: ImageTaskDelegate? = nil) async throws -> ImageResponse {
#warning("Move delegate to startImageTask")
let task = makeImageTask(request: request.asImageRequest())
delegate?.imageTaskWillStart(task)
delegate?.imageTaskCreated(task)

return try await withTaskCancellationHandler(handler: {
task.cancel()
Expand All @@ -120,7 +121,9 @@ public final class ImagePipeline: @unchecked Sendable {
continuation.resume(throwing: CancellationError())
}
queue.async { [weak delegate] in
self.startImageTask(task, callbackQueue: nil, progress: { [weak delegate] response, completed, total in
delegate?.imageTaskStarted(task)

self.startImageTask(task, isConfined: true, callbackQueue: nil, progress: { [weak delegate] response, completed, total in
if let response = response {
delegate?.imageTask(task, didProduceProgressiveResponse: response)
} else {
Expand All @@ -139,15 +142,17 @@ public final class ImagePipeline: @unchecked Sendable {
/// more data becomes available.
public func images(for request: any ImageRequestConvertible, delegate: ImageTaskDelegate? = nil) -> AsyncThrowingStream<ImageResponse, Swift.Error> {
let task = makeImageTask(request: request.asImageRequest())
delegate?.imageTaskWillStart(task)
delegate?.imageTaskCreated(task)

task.onCancel = { [weak delegate] in
delegate?.imageTaskDidCancel(task)
}

return AsyncThrowingStream { continuation in
queue.async { [weak delegate] in
self.startImageTask(task, callbackQueue: nil, progress: { [weak delegate] response, completed, total in
delegate?.imageTaskStarted(task)

self.startImageTask(task, isConfined: true, callbackQueue: nil, progress: { [weak delegate] response, completed, total in
if let response = response {
delegate?.imageTask(task, didProduceProgressiveResponse: response)
continuation.yield(response)
Expand Down Expand Up @@ -178,23 +183,17 @@ public final class ImagePipeline: @unchecked Sendable {
///
/// - parameter request: An image request.
@discardableResult
public func data(for request: any ImageRequestConvertible, delegate: ImageTaskDelegate? = nil) async throws -> (Data, URLResponse?) {
let task = makeImageTask(request: request.asImageRequest(), isDataTask: true)
delegate?.imageTaskWillStart(task)

public func data(for request: any ImageRequestConvertible) async throws -> (Data, URLResponse?) {
let task = makeImageTask(request: request.asImageRequest())
return try await withTaskCancellationHandler(handler: {
task.cancel()
}, operation: {
try await withUnsafeThrowingContinuation { continuation in
task.onCancel = { [weak delegate] in
delegate?.imageTaskDidCancel(task)
task.onCancel = {
continuation.resume(throwing: CancellationError())
}
queue.async { [weak delegate] in
self.startDataTask(task, callbackQueue: nil, progress: { [weak delegate] in
delegate?.imageTask(task, didUpdateProgress: ($0, $1))
}, completion: { [weak delegate] result in
delegate?.dataTask(task, didCompleteWithResult: result)
queue.async {
self.startDataTask(task, callbackQueue: nil, progress: nil, completion: { result in
continuation.resume(with: result.map { $0 })
})
}
Expand Down Expand Up @@ -240,32 +239,42 @@ public final class ImagePipeline: @unchecked Sendable {
completion: ((_ result: Result<ImageResponse, Error>) -> Void)?
) -> ImageTask {
let task = makeImageTask(request: request)
delegate.imageTaskCreated(task)
self.startImageTask(task, isConfined: isConfined, callbackQueue: queue, progress: progress, completion: completion)
return task
}

private func startImageTask(
_ task: ImageTask,
isConfined: Bool,
callbackQueue: DispatchQueue?,
progress: ((ImageResponse?, Int64, Int64) -> Void)?,
completion: ((_ result: Result<ImageResponse, Error>) -> Void)?
) {
if isConfined {
self.startImageTask(task, callbackQueue: queue, progress: progress, completion: completion)
_startImageTask(task, callbackQueue: callbackQueue, progress: progress, completion: completion)
} else {
self.queue.async {
self.startImageTask(task, callbackQueue: queue, progress: progress, completion: completion)
self._startImageTask(task, callbackQueue: callbackQueue, progress: progress, completion: completion)
}
}
return task
}

private func startImageTask(
private func _startImageTask(
_ task: ImageTask,
callbackQueue: DispatchQueue?,
progress progressHandler: ((ImageResponse?, Int64, Int64) -> Void)?,
completion: ((_ result: Result<ImageResponse, Error>) -> Void)?
) {
guard !isInvalidated else { return }

self.send(.started, task)
delegate.imageTaskStarted(task)

tasks[task] = makeTaskLoadImage(for: task.request)
let context = ImageTaskContext(callbackQueue: callbackQueue)
context.subscription = makeTaskLoadImage(for: task.request)
.subscribe(priority: task._priority.taskPriority, subscriber: task) { [weak self, weak task] event in
guard let self = self, let task = task else { return }

self.send(ImageTaskEvent(event), task)

if event.isCompleted {
self.tasks[task] = nil
}
Expand All @@ -276,22 +285,27 @@ public final class ImagePipeline: @unchecked Sendable {
switch event {
case let .value(response, isCompleted):
if isCompleted {
self.delegate.imageTask(task, didCompleteWithResult: .success(response))
completion?(.success(response))
} else {
self.delegate.imageTask(task, didProduceProgressiveResponse: response)
progressHandler?(response, task.completedUnitCount, task.totalUnitCount)
}
case let .progress(progress):
self.delegate.imageTask(task, didUpdateProgress: (progress.completed, progress.total))
task.setProgress(progress)
progressHandler?(nil, progress.completed, progress.total)
case let .error(error):
self.delegate.imageTask(task, didCompleteWithResult: .failure(error))
completion?(.failure(error))
}
}
}
tasks[task] = context
}

private func makeImageTask(request: ImageRequest, isDataTask: Bool = false) -> ImageTask {
ImageTask(taskId: nextTaskId, request: request, isDataTask: isDataTask, pipeline: self)
private func makeImageTask(request: ImageRequest) -> ImageTask {
ImageTask(taskId: nextTaskId, request: request, pipeline: self)
}

// MARK: - Loading Data (Closures)
Expand Down Expand Up @@ -335,7 +349,7 @@ public final class ImagePipeline: @unchecked Sendable {
progress: ((_ completed: Int64, _ total: Int64) -> Void)?,
completion: @escaping (Result<(data: Data, response: URLResponse?), Error>) -> Void
) -> ImageTask {
let task = makeImageTask(request: request, isDataTask: true)
let task = makeImageTask(request: request)
if isConfined {
self.startDataTask(task, callbackQueue: queue, progress: progress, completion: completion)
} else {
Expand All @@ -354,7 +368,8 @@ public final class ImagePipeline: @unchecked Sendable {
) {
guard !isInvalidated else { return }

tasks[task] = makeTaskLoadData(for: task.request)
let context = ImageTaskContext(callbackQueue: callbackQueue, isDataTask: true)
context.subscription = makeTaskLoadData(for: task.request)
.subscribe(priority: task._priority.taskPriority, subscriber: task) { [weak self, weak task] event in
guard let self = self, let task = task else { return }

Expand All @@ -378,6 +393,7 @@ public final class ImagePipeline: @unchecked Sendable {
}
}
}
tasks[task] = context
}

// MARK: - Loading Images (Combine)
Expand All @@ -398,24 +414,20 @@ public final class ImagePipeline: @unchecked Sendable {
}

private func cancel(_ task: ImageTask) {
guard let subscription = self.tasks.removeValue(forKey: task) else { return }
if !task.isDataTask {
self.send(.cancelled, task)
}
if let onCancel = task.onCancel {
dispatchCallback(to: nil, onCancel)
guard let context = self.tasks.removeValue(forKey: task) else { return }
dispatchCallback(to: context.callbackQueue) {
task.onCancel?()
if !context.isDataTask {
self.delegate.imageTaskDidCancel(task)
}
}
subscription.unsubscribe()
context.subscription?.unsubscribe()
}

func imageTaskUpdatePriorityCalled(_ task: ImageTask, priority: ImageRequest.Priority) {
queue.async {
task._priority = priority
guard let subscription = self.tasks[task] else { return }
if !task.isDataTask {
self.send(.priorityUpdated(priority: priority), task)
}
subscription.setPriority(priority.taskPriority)
self.tasks[task]?.subscription?.setPriority(priority.taskPriority)
}
}

Expand Down Expand Up @@ -479,8 +491,15 @@ public final class ImagePipeline: @unchecked Sendable {
TaskFetchWithPublisher(self, request)
}
}
}

private final class ImageTaskContext {
let callbackQueue: DispatchQueue?
let isDataTask: Bool
var subscription: TaskSubscription?

private func send(_ event: ImageTaskEvent, _ task: ImageTask) {
delegate.pipeline(self, imageTask: task, didReceiveEvent: event)
init(callbackQueue: DispatchQueue? = nil, isDataTask: Bool = false) {
self.callbackQueue = callbackQueue
self.isDataTask = isDataTask
}
}
Loading