Fix duplicate didFinishDownloadingImageForURL delegate callback

[onevcat]

Summary

• Remove the duplicate invocation path of imageDownloader(_:didFinishDownloadingImageForURL:with:error:): the onDownloadingFinished forwarding in ImageDownloader.setupSessionHandler() and its hook in SessionDelegate are deleted; the reportDidDownloadImageData call in startDownloadTask's onTaskDone handler remains as the single notification point.
• Add regression tests pinning "exactly one delegate notification per download" across success, network error, invalid status code, and data-modifying failure.

The bug

A completed download notified the delegate twice, back to back, from the same urlSession(_:task:didCompleteWithError:) flow:

1. SessionDelegate line 211 → onDownloadingFinished → forwarding closure in setupSessionHandler()
2. SessionDelegate line 224 → onCompleted → onTaskDone → reportDidDownloadImageData

The call count was also inconsistent, verified by the new tests (before the fix):

Scenario	Calls	Notes
Success	2
Network error	2
Invalid status code (404)	1	path 1 coincidentally filtered: the task was already removed
Cancellation	1	same
Data-modifying failure (didDownload data returns nil)	2, contradictory	first call reported error: nil (a "success"), second reported the failure

History

The duplication shipped with 5.0 and survived for over seven years:

• 5e26180 (2018-10-11, the 5.0 downloader rewrite): only the onTaskDone path existed.
• 8b6d412 (2018-10-13): the onDownloadingFinished hook was added with the forwarding commented out — the duplication was evidently known at the time.
• 83cfe18 (2018-10-20): the forwarding was uncommented without removing the onTaskDone call. The duplicate was born.
• 93f62c4 (5.10) and befd36c (6.0) refactored both paths along, unchanged.

It went unnoticed because the callback is typically used for logging/metrics where duplication is low-symptom, and the method had zero test coverage.

Why keep the onTaskDone path (and not the other one)

The onTaskDone path covers every termination — success, session error, cancellation, invalid status code, and data-modifying failure — with the correct error value. The onDownloadingFinished path only fired when didCompleteWithError still found the task registered, and reported error: nil for data-modifying failures because it only saw the transport-level result.

Note: KingfisherError.ResponseErrorReason.noURLResponse (error code 2005) no longer has a producer after this change — it was only ever synthesized to feed the duplicate call and never reached a completion handler. The public enum case remains for API compatibility.

Tests

• New: testDidFinishDownloadingDelegateCalledOncePerSuccessfulDownload, testDidFinishDownloadingDelegateCalledOncePerFailedDownload, testDidFinishDownloadingDelegateCalledOncePerErroredDownload, testDidFinishDownloadingDelegateConsistentWhenDataModifyingFails. All four fail against master (counting 2 calls / a nil error on the first event) and pass with this change.
• Full macOS suite passes (670 tests).

Known remaining wrinkles (documented, intentionally not addressed here)

• On a shared download with multiple attached callers, didDownload image for url is notified once per attached callback (processing itself is deduplicated per processor identifier, the notification is not).
• Cancelling one caller of a shared download emits a didFinishDownloadingImageForURL with taskCancelled while the download continues for remaining callers, followed by a second (correct) notification at actual completion — a consequence of onTaskDone serving both per-callback delivery and terminal notification. Both are pre-existing, low-impact, and left for a separate discussion.